Re: [PATCH V1 5/5] backlight: qcom-wled: Add auto string detection logic

[kgunda]

On 2018-05-14 22:32, Bjorn Andersson wrote:
> On Wed 09 May 00:14 PDT 2018, kgunda@xxxxxxxxxxxxxx wrote:
>
> > On 2018-05-07 23:40, Bjorn Andersson wrote:
> > > On Thu 03 May 02:57 PDT 2018, Kiran Gunda wrote:
> > >
> > > [..]
> > > > +
> > > > +#define WLED_AUTO_DETECT_OVP_COUNT		5
> > > > +#define WLED_AUTO_DETECT_CNT_DLY_US		HZ /* 1 second */
> > > > +static bool wled_auto_detection_required(struct wled *wled)
> > >
> > > So cfg.auto_detection_enabled is set, but we didn't have a fault during
> > > wled_auto_detection_at_init(), which I presume indicates that the boot
> > > loader configured the strings appropriately (or didn't enable the BL).
> > > Then first time we try to enable the backlight we will hit the ovp irq,
> > > which will  enter here a few times to figure out that the strings are
> > > incorrectly configured and then we will do the same thing that would
> > > have been done if we probed with a fault.
> > >
> > > This is convoluted!
> > >
> > > If auto-detection is a feature allowing the developer to omit the string
> > > configuration then just do the auto detection explicitly in probe when
> > > the developer did so and then never do it again.
> > >
> > As explained in the previous patch, the auto-detection is needed 
> > later,
> > because are also cases where one/more of the connected LED string of 
> > the
> > display-backlight is malfunctioning (because of damage) and requires 
> > the
> > damaged string to be turned off to prevent the complete panel and/or 
> > board
> > from being damaged.
>
> Okay, that sounds very reasonable. Please ensure that it's clearly
> described in the commit message, so that we have this documented if
> someone wonders in the future.
>
> Regards,
> Bjorn
> --
Thanks for that ! Sure I will describe it in the commit message.
> To unsubscribe from this list: send the line "unsubscribe 
> linux-arm-msm" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html