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

2018-05-14 Thread kgunda

On 2018-05-14 22:32, Bjorn Andersson wrote:

On Wed 09 May 00:14 PDT 2018, kgu...@codeaurora.org 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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2018-05-14 Thread kgunda

On 2018-05-14 22:32, Bjorn Andersson wrote:

On Wed 09 May 00:14 PDT 2018, kgu...@codeaurora.org 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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2018-05-14 Thread Bjorn Andersson
On Wed 09 May 00:14 PDT 2018, kgu...@codeaurora.org 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


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

2018-05-14 Thread Bjorn Andersson
On Wed 09 May 00:14 PDT 2018, kgu...@codeaurora.org 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


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

2018-05-09 Thread kgunda

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_USHZ /* 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.

+{
+   s64 elapsed_time_us;
+
+   if (*wled->version == WLED_PM8941)
+   return false;
+   /*
+* Check if the OVP fault was an occasional one
+* or if it's firing continuously, the latter qualifies
+* for an auto-detection check.
+*/
+   if (!wled->auto_detection_ovp_count) {
+   wled->start_ovp_fault_time = ktime_get();
+   wled->auto_detection_ovp_count++;
+   } else {
+   elapsed_time_us = ktime_us_delta(ktime_get(),
+wled->start_ovp_fault_time);
+   if (elapsed_time_us > WLED_AUTO_DETECT_CNT_DLY_US)
+   wled->auto_detection_ovp_count = 0;
+   else
+   wled->auto_detection_ovp_count++;
+
+   if (wled->auto_detection_ovp_count >=
+   WLED_AUTO_DETECT_OVP_COUNT) {
+   wled->auto_detection_ovp_count = 0;
+   return true;
+   }
+   }
+
+   return false;
+}
+
+static int wled_auto_detection_at_init(struct wled *wled)
+{
+   int rc;
+   u32 fault_status = 0, rt_status = 0;
+
+   if (*wled->version == WLED_PM8941)
+   return 0;


cfg.auto_detection_enabled will be false in this case, so there's no
need for the extra check.


Ok. I will remove it in the next series.

+
+   if (!wled->cfg.auto_detection_enabled)
+   return 0;
+
+   rc = regmap_read(wled->regmap,
+wled->ctrl_addr + WLED3_CTRL_REG_INT_RT_STS,
+_status);
+   if (rc < 0) {
+   pr_err("Failed to read RT status rc=%d\n", rc);
+   return rc;
+   }
+
+   rc = regmap_read(wled->regmap,
+wled->ctrl_addr + WLED3_CTRL_REG_FAULT_STATUS,
+_status);
+   if (rc < 0) {
+   pr_err("Failed to read fault status rc=%d\n", rc);
+   return rc;
+   }
+
+   if ((rt_status & WLED3_CTRL_REG_OVP_FAULT_STATUS) ||
+   (fault_status & WLED3_CTRL_REG_OVP_FAULT_BIT)) {


So this would only happen if the boot loader set an invalid string
configuration, as we have yet to enable the module here?


Yes.

+   mutex_lock(>lock);
+   rc = wled_auto_string_detection(wled);
+   if (!rc)
+   wled->auto_detection_done = true;
+   mutex_unlock(>lock);
+   }
+
+   return rc;
+}
+
+static void handle_ovp_fault(struct wled *wled)
+{
+   if (!wled->cfg.auto_detection_enabled)


As this is the only reason for requesting the ovp_irq, how about just
not requesting it in this case?


This is also needed for information purpose if there is any other reason
and also discussing with HW systems what needs to do if the OVP keep on 
triggering.

+   return;
+
+   mutex_lock(>lock);
+   if (wled->ovp_irq > 0 && !wled->ovp_irq_disabled) {


The logic here is unnecessary, the only way handle_ovp_fault() is ever
executed is if wled_ovp_irq_handler() was called, which is a really 
good

indication that ovp_irq is valid and !ovp_irq_disabled. So this is
always going to be entered.


Ok. I will remove this logic in the next series.

+   disable_irq_nosync(wled->ovp_irq);
+   wled->ovp_irq_disabled = true;
+   }
+
+   if (wled_auto_detection_required(wled))
+   wled_auto_string_detection(wled);
+
+   if (wled->ovp_irq > 0 && wled->ovp_irq_disabled) {


Again, ovp_irq is valid and we entered the function with either
ovp_irq_disabled = true due to some bug or it was set to true above. 

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

2018-05-09 Thread kgunda

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_USHZ /* 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.

+{
+   s64 elapsed_time_us;
+
+   if (*wled->version == WLED_PM8941)
+   return false;
+   /*
+* Check if the OVP fault was an occasional one
+* or if it's firing continuously, the latter qualifies
+* for an auto-detection check.
+*/
+   if (!wled->auto_detection_ovp_count) {
+   wled->start_ovp_fault_time = ktime_get();
+   wled->auto_detection_ovp_count++;
+   } else {
+   elapsed_time_us = ktime_us_delta(ktime_get(),
+wled->start_ovp_fault_time);
+   if (elapsed_time_us > WLED_AUTO_DETECT_CNT_DLY_US)
+   wled->auto_detection_ovp_count = 0;
+   else
+   wled->auto_detection_ovp_count++;
+
+   if (wled->auto_detection_ovp_count >=
+   WLED_AUTO_DETECT_OVP_COUNT) {
+   wled->auto_detection_ovp_count = 0;
+   return true;
+   }
+   }
+
+   return false;
+}
+
+static int wled_auto_detection_at_init(struct wled *wled)
+{
+   int rc;
+   u32 fault_status = 0, rt_status = 0;
+
+   if (*wled->version == WLED_PM8941)
+   return 0;


cfg.auto_detection_enabled will be false in this case, so there's no
need for the extra check.


Ok. I will remove it in the next series.

+
+   if (!wled->cfg.auto_detection_enabled)
+   return 0;
+
+   rc = regmap_read(wled->regmap,
+wled->ctrl_addr + WLED3_CTRL_REG_INT_RT_STS,
+_status);
+   if (rc < 0) {
+   pr_err("Failed to read RT status rc=%d\n", rc);
+   return rc;
+   }
+
+   rc = regmap_read(wled->regmap,
+wled->ctrl_addr + WLED3_CTRL_REG_FAULT_STATUS,
+_status);
+   if (rc < 0) {
+   pr_err("Failed to read fault status rc=%d\n", rc);
+   return rc;
+   }
+
+   if ((rt_status & WLED3_CTRL_REG_OVP_FAULT_STATUS) ||
+   (fault_status & WLED3_CTRL_REG_OVP_FAULT_BIT)) {


So this would only happen if the boot loader set an invalid string
configuration, as we have yet to enable the module here?


Yes.

+   mutex_lock(>lock);
+   rc = wled_auto_string_detection(wled);
+   if (!rc)
+   wled->auto_detection_done = true;
+   mutex_unlock(>lock);
+   }
+
+   return rc;
+}
+
+static void handle_ovp_fault(struct wled *wled)
+{
+   if (!wled->cfg.auto_detection_enabled)


As this is the only reason for requesting the ovp_irq, how about just
not requesting it in this case?


This is also needed for information purpose if there is any other reason
and also discussing with HW systems what needs to do if the OVP keep on 
triggering.

+   return;
+
+   mutex_lock(>lock);
+   if (wled->ovp_irq > 0 && !wled->ovp_irq_disabled) {


The logic here is unnecessary, the only way handle_ovp_fault() is ever
executed is if wled_ovp_irq_handler() was called, which is a really 
good

indication that ovp_irq is valid and !ovp_irq_disabled. So this is
always going to be entered.


Ok. I will remove this logic in the next series.

+   disable_irq_nosync(wled->ovp_irq);
+   wled->ovp_irq_disabled = true;
+   }
+
+   if (wled_auto_detection_required(wled))
+   wled_auto_string_detection(wled);
+
+   if (wled->ovp_irq > 0 && wled->ovp_irq_disabled) {


Again, ovp_irq is valid and we entered the function with either
ovp_irq_disabled = true due to some bug or it was set to true above. 

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

2018-05-07 Thread Bjorn Andersson
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.

> +{
> + s64 elapsed_time_us;
> +
> + if (*wled->version == WLED_PM8941)
> + return false;
> + /*
> +  * Check if the OVP fault was an occasional one
> +  * or if it's firing continuously, the latter qualifies
> +  * for an auto-detection check.
> +  */
> + if (!wled->auto_detection_ovp_count) {
> + wled->start_ovp_fault_time = ktime_get();
> + wled->auto_detection_ovp_count++;
> + } else {
> + elapsed_time_us = ktime_us_delta(ktime_get(),
> +  wled->start_ovp_fault_time);
> + if (elapsed_time_us > WLED_AUTO_DETECT_CNT_DLY_US)
> + wled->auto_detection_ovp_count = 0;
> + else
> + wled->auto_detection_ovp_count++;
> +
> + if (wled->auto_detection_ovp_count >=
> + WLED_AUTO_DETECT_OVP_COUNT) {
> + wled->auto_detection_ovp_count = 0;
> + return true;
> + }
> + }
> +
> + return false;
> +}
> +
> +static int wled_auto_detection_at_init(struct wled *wled)
> +{
> + int rc;
> + u32 fault_status = 0, rt_status = 0;
> +
> + if (*wled->version == WLED_PM8941)
> + return 0;

cfg.auto_detection_enabled will be false in this case, so there's no
need for the extra check.

> +
> + if (!wled->cfg.auto_detection_enabled)
> + return 0;
> +
> + rc = regmap_read(wled->regmap,
> +  wled->ctrl_addr + WLED3_CTRL_REG_INT_RT_STS,
> +  _status);
> + if (rc < 0) {
> + pr_err("Failed to read RT status rc=%d\n", rc);
> + return rc;
> + }
> +
> + rc = regmap_read(wled->regmap,
> +  wled->ctrl_addr + WLED3_CTRL_REG_FAULT_STATUS,
> +  _status);
> + if (rc < 0) {
> + pr_err("Failed to read fault status rc=%d\n", rc);
> + return rc;
> + }
> +
> + if ((rt_status & WLED3_CTRL_REG_OVP_FAULT_STATUS) ||
> + (fault_status & WLED3_CTRL_REG_OVP_FAULT_BIT)) {

So this would only happen if the boot loader set an invalid string
configuration, as we have yet to enable the module here?

> + mutex_lock(>lock);
> + rc = wled_auto_string_detection(wled);
> + if (!rc)
> + wled->auto_detection_done = true;
> + mutex_unlock(>lock);
> + }
> +
> + return rc;
> +}
> +
> +static void handle_ovp_fault(struct wled *wled)
> +{
> + if (!wled->cfg.auto_detection_enabled)

As this is the only reason for requesting the ovp_irq, how about just
not requesting it in this case?

> + return;
> +
> + mutex_lock(>lock);
> + if (wled->ovp_irq > 0 && !wled->ovp_irq_disabled) {

The logic here is unnecessary, the only way handle_ovp_fault() is ever
executed is if wled_ovp_irq_handler() was called, which is a really good
indication that ovp_irq is valid and !ovp_irq_disabled. So this is
always going to be entered.

> + disable_irq_nosync(wled->ovp_irq);
> + wled->ovp_irq_disabled = true;
> + }
> +
> + if (wled_auto_detection_required(wled))
> + wled_auto_string_detection(wled);
> +
> + if (wled->ovp_irq > 0 && wled->ovp_irq_disabled) {

Again, ovp_irq is valid and we entered the function with either
ovp_irq_disabled = true due to some bug or it was set to true above. So
this check is useless - which renders ovp_irq_disabled unnecessary as
well.

> + enable_irq(wled->ovp_irq);
> + wled->ovp_irq_disabled = false;
> + }
> + mutex_unlock(>lock);
> +}
> +
>  static irqreturn_t wled_ovp_irq_handler(int irq, void *_wled)
>  {
>   struct wled *wled = _wled;
> @@ -413,6 +706,9 @@ static irqreturn_t wled_ovp_irq_handler(int irq, void 
> *_wled)
>   dev_dbg(wled->dev, "WLED OVP fault detected, int_sts=%x 
> fault_sts= %x\n",
>   int_sts, fault_sts);
>  
> + if (fault_sts & 

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

2018-05-07 Thread Bjorn Andersson
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.

> +{
> + s64 elapsed_time_us;
> +
> + if (*wled->version == WLED_PM8941)
> + return false;
> + /*
> +  * Check if the OVP fault was an occasional one
> +  * or if it's firing continuously, the latter qualifies
> +  * for an auto-detection check.
> +  */
> + if (!wled->auto_detection_ovp_count) {
> + wled->start_ovp_fault_time = ktime_get();
> + wled->auto_detection_ovp_count++;
> + } else {
> + elapsed_time_us = ktime_us_delta(ktime_get(),
> +  wled->start_ovp_fault_time);
> + if (elapsed_time_us > WLED_AUTO_DETECT_CNT_DLY_US)
> + wled->auto_detection_ovp_count = 0;
> + else
> + wled->auto_detection_ovp_count++;
> +
> + if (wled->auto_detection_ovp_count >=
> + WLED_AUTO_DETECT_OVP_COUNT) {
> + wled->auto_detection_ovp_count = 0;
> + return true;
> + }
> + }
> +
> + return false;
> +}
> +
> +static int wled_auto_detection_at_init(struct wled *wled)
> +{
> + int rc;
> + u32 fault_status = 0, rt_status = 0;
> +
> + if (*wled->version == WLED_PM8941)
> + return 0;

cfg.auto_detection_enabled will be false in this case, so there's no
need for the extra check.

> +
> + if (!wled->cfg.auto_detection_enabled)
> + return 0;
> +
> + rc = regmap_read(wled->regmap,
> +  wled->ctrl_addr + WLED3_CTRL_REG_INT_RT_STS,
> +  _status);
> + if (rc < 0) {
> + pr_err("Failed to read RT status rc=%d\n", rc);
> + return rc;
> + }
> +
> + rc = regmap_read(wled->regmap,
> +  wled->ctrl_addr + WLED3_CTRL_REG_FAULT_STATUS,
> +  _status);
> + if (rc < 0) {
> + pr_err("Failed to read fault status rc=%d\n", rc);
> + return rc;
> + }
> +
> + if ((rt_status & WLED3_CTRL_REG_OVP_FAULT_STATUS) ||
> + (fault_status & WLED3_CTRL_REG_OVP_FAULT_BIT)) {

So this would only happen if the boot loader set an invalid string
configuration, as we have yet to enable the module here?

> + mutex_lock(>lock);
> + rc = wled_auto_string_detection(wled);
> + if (!rc)
> + wled->auto_detection_done = true;
> + mutex_unlock(>lock);
> + }
> +
> + return rc;
> +}
> +
> +static void handle_ovp_fault(struct wled *wled)
> +{
> + if (!wled->cfg.auto_detection_enabled)

As this is the only reason for requesting the ovp_irq, how about just
not requesting it in this case?

> + return;
> +
> + mutex_lock(>lock);
> + if (wled->ovp_irq > 0 && !wled->ovp_irq_disabled) {

The logic here is unnecessary, the only way handle_ovp_fault() is ever
executed is if wled_ovp_irq_handler() was called, which is a really good
indication that ovp_irq is valid and !ovp_irq_disabled. So this is
always going to be entered.

> + disable_irq_nosync(wled->ovp_irq);
> + wled->ovp_irq_disabled = true;
> + }
> +
> + if (wled_auto_detection_required(wled))
> + wled_auto_string_detection(wled);
> +
> + if (wled->ovp_irq > 0 && wled->ovp_irq_disabled) {

Again, ovp_irq is valid and we entered the function with either
ovp_irq_disabled = true due to some bug or it was set to true above. So
this check is useless - which renders ovp_irq_disabled unnecessary as
well.

> + enable_irq(wled->ovp_irq);
> + wled->ovp_irq_disabled = false;
> + }
> + mutex_unlock(>lock);
> +}
> +
>  static irqreturn_t wled_ovp_irq_handler(int irq, void *_wled)
>  {
>   struct wled *wled = _wled;
> @@ -413,6 +706,9 @@ static irqreturn_t wled_ovp_irq_handler(int irq, void 
> *_wled)
>   dev_dbg(wled->dev, "WLED OVP fault detected, int_sts=%x 
> fault_sts= %x\n",
>   int_sts, fault_sts);
>  
> + if (fault_sts &