[PATCH] backlight: qcom-wled: Fix unbalanced OVP IRQ enable at probe

[PATCH] backlight: qcom-wled: Fix unbalanced OVP IRQ enable at probe

[David Heidelberg via B4 Relay]

From: David Heidelberg <david@ixit.cz>

wled_configure_ovp_irq() derives the initial state of the OVP interrupt
from the hardware:

        /* Keep OVP irq disabled until module is enabled */
        if (!(val & WLED3_CTRL_REG_MOD_EN_MASK))
                disable_irq(wled->ovp_irq);

but wled->brightness, which is what the rest of the driver uses to tell
whether the module is on, is left at zero.

On boards where the bootloader hands the kernel a lit backlight the two
disagree. MOD_EN is already set, so the interrupt is left enabled, while
wled_update_status() still believes the backlight is off and takes

        if (!!brightness != !!wled->brightness)
                rc = wled_module_enable(wled, !!brightness);

on the first backlight update. wled_module_enable() then schedules
wled_ovp_work(), which calls enable_irq() on the already enabled
interrupt:

  Unbalanced enable for IRQ 176
  WARNING: CPU: 0 PID: 160 at kernel/irq/manage.c:774 __enable_irq+0x50/0x80
  Hardware name: Xiaomi Pocophone F1 (DT)
  Workqueue: events wled_ovp_work
  Call trace:
   __enable_irq+0x50/0x80
   enable_irq+0x48/0xa0
   wled_ovp_work+0x18/0x24
   process_one_work+0x1d0/0x350
   worker_thread+0x13c/0x460
   kthread+0x110/0x114
   ret_from_fork+0x10/0x20

The bootloader is not the only way to get there. The readback runs after
wledN_setup(), and wled4_setup() sets MOD_EN itself on the path where the
sink configuration does not already match, as does the tail of
wled_auto_string_detection(), which all three setup paths can reach
through wled_auto_detection_at_init(). A cold-booted board with a dark
panel can therefore reach the same disagreement.

Move the MOD_EN readback into wled_probe() and use it to seed
wled->brightness, so the driver starts out agreeing with the hardware,
and key the OVP interrupt off wled->brightness instead. The first
backlight update then only reprograms the brightness registers and
leaves both the module and the interrupt alone. The OVP interrupt also
stays armed from probe whenever the module is already enabled, rather
than being disabled at probe and only enabled once something writes
brightness.

Note that a backlight update requesting brightness 0 before any non-zero
one now really does turn the module off wherever MOD_EN was already set,
where before it was silently ignored.

wled->brightness is seeded with default-brightness rather than the level
the bootloader actually programmed, so the first update can still step
the brightness. Reading that level back needs a per-version accessor and
is left for later.

Assisted-by: LLM
Fixes: 8663c188beea ("backlight: qcom-wled: Add auto string detection logic")
Cc: stable@vger.kernel.org
Signed-off-by: David Heidelberg <david@ixit.cz>
---
this is slightly annoying bug sdm845-mainline had patched for a years,
but not very well. This should be the proper patch, got ack from people
with Xiaomi Poco F1 (ebbg variant) which does use this so it works.

Adding MSM8953 folks into Cc too, which use the early fix.
---
 drivers/video/backlight/qcom-wled.c | 32 +++++++++++++++++++++-----------
 1 file changed, 21 insertions(+), 11 deletions(-)

diff --git a/drivers/video/backlight/qcom-wled.c b/drivers/video/backlight/qcom-wled.c
index 650dd95f06ef5..344b8cad90105 100644
--- a/drivers/video/backlight/qcom-wled.c
+++ b/drivers/video/backlight/qcom-wled.c
@@ -1622,54 +1622,49 @@ static int wled_configure_short_irq(struct wled *wled,
 
 	return rc;
 }
 
 static int wled_configure_ovp_irq(struct wled *wled,
 				  struct platform_device *pdev)
 {
 	int rc;
-	u32 val;
 
 	wled->ovp_irq = platform_get_irq_byname(pdev, "ovp");
 	if (wled->ovp_irq < 0) {
 		dev_dbg(&pdev->dev, "OVP IRQ not found - disabling automatic string detection\n");
 		return 0;
 	}
 
 	rc = devm_request_threaded_irq(wled->dev, wled->ovp_irq, NULL,
 				       wled_ovp_irq_handler, IRQF_ONESHOT,
 				       "wled_ovp_irq", wled);
 	if (rc < 0) {
 		wled->ovp_irq = 0;
 		return 0;
 	}
 
-	rc = regmap_read(wled->regmap, wled->ctrl_addr +
-			 WLED3_CTRL_REG_MOD_EN, &val);
-	if (rc < 0)
-		return rc;
-
-	/* Keep OVP irq disabled until module is enabled */
-	if (!(val & WLED3_CTRL_REG_MOD_EN_MASK))
+	/* Keep the OVP irq disabled until the module is enabled */
+	if (!wled->brightness)
 		disable_irq(wled->ovp_irq);
 
 	return 0;
 }
 
 static const struct backlight_ops wled_ops = {
 	.update_status = wled_update_status,
 };
 
 static int wled_probe(struct platform_device *pdev)
 {
 	struct backlight_properties props;
 	struct backlight_device *bl;
 	struct wled *wled;
 	struct regmap *regmap;
+	u32 mod_en;
 	u32 val;
 	int rc;
 
 	regmap = dev_get_regmap(pdev->dev.parent, NULL);
 	if (!regmap) {
 		dev_err(&pdev->dev, "Unable to get regmap\n");
 		return -EINVAL;
 	}
@@ -1729,27 +1724,42 @@ static int wled_probe(struct platform_device *pdev)
 
 	default:
 		dev_err(wled->dev, "Invalid WLED version\n");
 		break;
 	}
 
 	INIT_DELAYED_WORK(&wled->ovp_work, wled_ovp_work);
 
+	val = WLED_DEFAULT_BRIGHTNESS;
+	of_property_read_u32(pdev->dev.of_node, "default-brightness", &val);
+
+	/*
+	 * The module may already be enabled, either by a bootloader that left
+	 * the backlight lit or by the setup above. Record that, so that the
+	 * first brightness update does not enable an already enabled module,
+	 * and so that the OVP irq is armed from probe rather than from that
+	 * first update.
+	 */
+	rc = regmap_read(wled->regmap, wled->ctrl_addr + WLED3_CTRL_REG_MOD_EN,
+			 &mod_en);
+	if (rc < 0)
+		return rc;
+
+	if (mod_en & WLED3_CTRL_REG_MOD_EN_MASK)
+		wled->brightness = val;
+
 	rc = wled_configure_short_irq(wled, pdev);
 	if (rc < 0)
 		return rc;
 
 	rc = wled_configure_ovp_irq(wled, pdev);
 	if (rc < 0)
 		return rc;
 
-	val = WLED_DEFAULT_BRIGHTNESS;
-	of_property_read_u32(pdev->dev.of_node, "default-brightness", &val);
-
 	memset(&props, 0, sizeof(struct backlight_properties));
 	props.type = BACKLIGHT_RAW;
 	props.brightness = val;
 	props.max_brightness = wled->max_brightness;
 	bl = devm_backlight_device_register(&pdev->dev, wled->name,
 					    &pdev->dev, wled,
 					    &wled_ops, &props);
 	return PTR_ERR_OR_ZERO(bl);

---
base-commit: 944a035ecca915ae947905dcfb03f2b9dc6d032c
change-id: 20260908-qcom-wled-backlight-fd9574027353

Best regards,
--  
David Heidelberg <david@ixit.cz>

[PATCH] backlight: qcom-wled: Fix unbalanced OVP IRQ enable at probe

[David Heidelberg]

wled_configure_ovp_irq() derives the initial state of the OVP interrupt
from the hardware:

        /* Keep OVP irq disabled until module is enabled */
        if (!(val & WLED3_CTRL_REG_MOD_EN_MASK))
                disable_irq(wled->ovp_irq);

but wled->brightness, which is what the rest of the driver uses to tell
whether the module is on, is left at zero.

On boards where the bootloader hands the kernel a lit backlight the two
disagree. MOD_EN is already set, so the interrupt is left enabled, while
wled_update_status() still believes the backlight is off and takes

        if (!!brightness != !!wled->brightness)
                rc = wled_module_enable(wled, !!brightness);

on the first backlight update. wled_module_enable() then schedules
wled_ovp_work(), which calls enable_irq() on the already enabled
interrupt:

  Unbalanced enable for IRQ 176
  WARNING: CPU: 0 PID: 160 at kernel/irq/manage.c:774 __enable_irq+0x50/0x80
  Hardware name: Xiaomi Pocophone F1 (DT)
  Workqueue: events wled_ovp_work
  Call trace:
   __enable_irq+0x50/0x80
   enable_irq+0x48/0xa0
   wled_ovp_work+0x18/0x24
   process_one_work+0x1d0/0x350
   worker_thread+0x13c/0x460
   kthread+0x110/0x114
   ret_from_fork+0x10/0x20

The bootloader is not the only way to get there. The readback runs after
wledN_setup(), and wled4_setup() sets MOD_EN itself on the path where the
sink configuration does not already match, as does the tail of
wled_auto_string_detection(), which all three setup paths can reach
through wled_auto_detection_at_init(). A cold-booted board with a dark
panel can therefore reach the same disagreement.

Move the MOD_EN readback into wled_probe() and use it to seed
wled->brightness, so the driver starts out agreeing with the hardware,
and key the OVP interrupt off wled->brightness instead. The first
backlight update then only reprograms the brightness registers and
leaves both the module and the interrupt alone. The OVP interrupt also
stays armed from probe whenever the module is already enabled, rather
than being disabled at probe and only enabled once something writes
brightness.

Note that a backlight update requesting brightness 0 before any non-zero
one now really does turn the module off wherever MOD_EN was already set,
where before it was silently ignored.

wled->brightness is seeded with default-brightness rather than the level
the bootloader actually programmed, so the first update can still step
the brightness. Reading that level back needs a per-version accessor and
is left for later.

Assisted-by: LLM
Fixes: 8663c188beea ("backlight: qcom-wled: Add auto string detection logic")
Cc: stable@vger.kernel.org
Signed-off-by: David Heidelberg <david@ixit.cz>
---
this is slightly annoying bug sdm845-mainline had patched for a years,
but not very well. This should be the proper patch, got ack from people
with Xiaomi Poco F1 (ebbg variant) which does use this so it works.

Adding MSM8953 folks into Cc too, which use the early fix.
---
 drivers/video/backlight/qcom-wled.c | 32 +++++++++++++++++++++-----------
 1 file changed, 21 insertions(+), 11 deletions(-)

diff --git a/drivers/video/backlight/qcom-wled.c b/drivers/video/backlight/qcom-wled.c
index 650dd95f06ef5..344b8cad90105 100644
--- a/drivers/video/backlight/qcom-wled.c
+++ b/drivers/video/backlight/qcom-wled.c
@@ -1622,54 +1622,49 @@ static int wled_configure_short_irq(struct wled *wled,
 
 	return rc;
 }
 
 static int wled_configure_ovp_irq(struct wled *wled,
 				  struct platform_device *pdev)
 {
 	int rc;
-	u32 val;
 
 	wled->ovp_irq = platform_get_irq_byname(pdev, "ovp");
 	if (wled->ovp_irq < 0) {
 		dev_dbg(&pdev->dev, "OVP IRQ not found - disabling automatic string detection\n");
 		return 0;
 	}
 
 	rc = devm_request_threaded_irq(wled->dev, wled->ovp_irq, NULL,
 				       wled_ovp_irq_handler, IRQF_ONESHOT,
 				       "wled_ovp_irq", wled);
 	if (rc < 0) {
 		wled->ovp_irq = 0;
 		return 0;
 	}
 
-	rc = regmap_read(wled->regmap, wled->ctrl_addr +
-			 WLED3_CTRL_REG_MOD_EN, &val);
-	if (rc < 0)
-		return rc;
-
-	/* Keep OVP irq disabled until module is enabled */
-	if (!(val & WLED3_CTRL_REG_MOD_EN_MASK))
+	/* Keep the OVP irq disabled until the module is enabled */
+	if (!wled->brightness)
 		disable_irq(wled->ovp_irq);
 
 	return 0;
 }
 
 static const struct backlight_ops wled_ops = {
 	.update_status = wled_update_status,
 };
 
 static int wled_probe(struct platform_device *pdev)
 {
 	struct backlight_properties props;
 	struct backlight_device *bl;
 	struct wled *wled;
 	struct regmap *regmap;
+	u32 mod_en;
 	u32 val;
 	int rc;
 
 	regmap = dev_get_regmap(pdev->dev.parent, NULL);
 	if (!regmap) {
 		dev_err(&pdev->dev, "Unable to get regmap\n");
 		return -EINVAL;
 	}
@@ -1729,27 +1724,42 @@ static int wled_probe(struct platform_device *pdev)
 
 	default:
 		dev_err(wled->dev, "Invalid WLED version\n");
 		break;
 	}
 
 	INIT_DELAYED_WORK(&wled->ovp_work, wled_ovp_work);
 
+	val = WLED_DEFAULT_BRIGHTNESS;
+	of_property_read_u32(pdev->dev.of_node, "default-brightness", &val);
+
+	/*
+	 * The module may already be enabled, either by a bootloader that left
+	 * the backlight lit or by the setup above. Record that, so that the
+	 * first brightness update does not enable an already enabled module,
+	 * and so that the OVP irq is armed from probe rather than from that
+	 * first update.
+	 */
+	rc = regmap_read(wled->regmap, wled->ctrl_addr + WLED3_CTRL_REG_MOD_EN,
+			 &mod_en);
+	if (rc < 0)
+		return rc;
+
+	if (mod_en & WLED3_CTRL_REG_MOD_EN_MASK)
+		wled->brightness = val;
+
 	rc = wled_configure_short_irq(wled, pdev);
 	if (rc < 0)
 		return rc;
 
 	rc = wled_configure_ovp_irq(wled, pdev);
 	if (rc < 0)
 		return rc;
 
-	val = WLED_DEFAULT_BRIGHTNESS;
-	of_property_read_u32(pdev->dev.of_node, "default-brightness", &val);
-
 	memset(&props, 0, sizeof(struct backlight_properties));
 	props.type = BACKLIGHT_RAW;
 	props.brightness = val;
 	props.max_brightness = wled->max_brightness;
 	bl = devm_backlight_device_register(&pdev->dev, wled->name,
 					    &pdev->dev, wled,
 					    &wled_ops, &props);
 	return PTR_ERR_OR_ZERO(bl);

---
base-commit: 944a035ecca915ae947905dcfb03f2b9dc6d032c
change-id: 20260908-qcom-wled-backlight-fd9574027353

Best regards,
--  
David Heidelberg <david@ixit.cz>

Re: [PATCH] backlight: qcom-wled: Fix unbalanced OVP IRQ enable at probe

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:

New issues:
- [High] If default-brightness is 0, the OVP IRQ is incorrectly disabled while the hardware module remains active, and the module cannot be subsequently disabled.

Pre-existing issues:
- [High] Missing platform_set_drvdata() in probe causes a guaranteed NULL pointer dereference in wled_remove().
- [High] wled_remove() calls disable_irq() with negative or uninitialized IRQ values if the IRQs failed to map during probe.
- [High] Tearing down synchronization primitives in wled_remove() before the devm-managed backlight is unregistered creates a use-after-free window.
- [High] WLED3 devices lack the wled_auto_detection_required function handler, leading to a NULL pointer dereference if the OVP IRQ fires.
- [High] The short IRQ handler unconditionally re-enables the hardware module, overriding software intentions.
--

commit 5bacd29d54de18ff6f2680d6c09d873e1a8a3c3a
Author: David Heidelberg <david@ixit.cz>

    backlight: qcom-wled: Fix unbalanced OVP IRQ enable at probe

This commit fixes an issue where the OVP IRQ was unbalanced at probe. It reads
the hardware module enable state to initialize the software brightness level,
and keys the OVP IRQ enable off this software state rather than the hardware
state to avoid mismatches.

> diff --git a/drivers/video/backlight/qcom-wled.c b/drivers/video/backlight/qcom-wled.c
> index 650dd95f06ef5..344b8cad90105 100644
> --- a/drivers/video/backlight/qcom-wled.c
> +++ b/drivers/video/backlight/qcom-wled.c
> @@ -1627,7 +1627,6 @@ static int wled_configure_ovp_irq(struct wled *wled,
>  				  struct platform_device *pdev)
>  {
>  	int rc;
> -	u32 val;
>  
>  	wled->ovp_irq = platform_get_irq_byname(pdev, "ovp");
>  	if (wled->ovp_irq < 0) {

[Severity: High]
This is a pre-existing issue, but if platform_get_irq_byname() fails and
returns a negative error code, it is left in wled->ovp_irq without being
reset to 0.

Could this cause a regression during module removal?

drivers/video/backlight/qcom-wled.c:wled_remove() {
...
    disable_irq(wled->ovp_irq);
...
}

It appears wled_remove() calls disable_irq() unconditionally, which would
result in an out-of-bounds IRQ descriptor lookup or inadvertently disabling a
system IRQ if wled->ovp_irq is negative.

[ ... ]
> @@ -1643,13 +1642,8 @@ static int wled_configure_ovp_irq(struct wled *wled,
>  		return 0;
>  	}
>  
> -	rc = regmap_read(wled->regmap, wled->ctrl_addr +
> -			 WLED3_CTRL_REG_MOD_EN, &val);
> -	if (rc < 0)
> -		return rc;
> -
> -	/* Keep OVP irq disabled until module is enabled */
> -	if (!(val & WLED3_CTRL_REG_MOD_EN_MASK))
> +	/* Keep the OVP irq disabled until the module is enabled */
> +	if (!wled->brightness)
>  		disable_irq(wled->ovp_irq);

[Severity: High]
If default-brightness is 0, wled->brightness will be seeded with 0 later in
wled_probe(). Will this incorrectly disable the OVP IRQ while the hardware
module remains active?

If the bootloader leaves the backlight ON, but default-brightness in the device
tree is 0, checking !wled->brightness instead of the hardware MOD_EN state
disables the OVP IRQ while hardware is still ON.

Additionally, a user space request to set brightness to 0 evaluates
!!0 != !!0 as false, silently ignoring the request and leaving the hardware
permanently ON without OVP protection.

[ ... ]
> @@ -1734,6 +1729,24 @@ static int wled_probe(struct platform_device *pdev)
>  
>  	INIT_DELAYED_WORK(&wled->ovp_work, wled_ovp_work);
>  
> +	val = WLED_DEFAULT_BRIGHTNESS;
> +	of_property_read_u32(pdev->dev.of_node, "default-brightness", &val);
> +
> +	/*
> +	 * The module may already be enabled, either by a bootloader that left
> +	 * the backlight lit or by the setup above. Record that, so that the
> +	 * first brightness update does not enable an already enabled module,
> +	 * and so that the OVP irq is armed from probe rather than from that
> +	 * first update.
> +	 */
> +	rc = regmap_read(wled->regmap, wled->ctrl_addr + WLED3_CTRL_REG_MOD_EN,
> +			 &mod_en);
> +	if (rc < 0)
> +		return rc;
> +
> +	if (mod_en & WLED3_CTRL_REG_MOD_EN_MASK)
> +		wled->brightness = val;
> +

[Severity: High]
This isn't a bug introduced by this patch, but WLED3 devices lack the
wled_auto_detection_required function handler:

drivers/video/backlight/qcom-wled.c:wled_configure() {
...
    case 3:
        u32_opts = wled3_opts;
        size = ARRAY_SIZE(wled3_opts);
        *cfg = wled3_config_defaults;
        wled->wled_set_brightness = wled3_set_brightness;
        wled->wled_sync_toggle = wled3_sync_toggle;
...
}

Does this lead to a NULL pointer dereference if the OVP IRQ fires?

drivers/video/backlight/qcom-wled.c:wled_ovp_irq_handler() {
...
    if (fault_sts & WLED3_CTRL_REG_OVP_FAULT_BIT) {
        if (wled->wled_auto_detection_required(wled)) {
            mutex_lock(&wled->lock);
...
}

[Severity: High]
This is a pre-existing issue, but the short IRQ handler unconditionally
re-enables the hardware module:

drivers/video/backlight/qcom-wled.c:wled_short_irq_handler() {
...
    rc = wled_module_enable(wled, true);
...
}

Does this override the software intentions if the backlight is logically OFF
(or being turned OFF) in wled->brightness when the short-circuit IRQ triggers?

This could cause the hardware backlight to become stuck ON, leading to permanent
battery drain and incorrect state.

[ ... ]
> @@ -1742,9 +1755,6 @@ static int wled_probe(struct platform_device *pdev)
>  	if (rc < 0)
>  		return rc;
>  
> -	val = WLED_DEFAULT_BRIGHTNESS;
> -	of_property_read_u32(pdev->dev.of_node, "default-brightness", &val);
> -
>  	memset(&props, 0, sizeof(struct backlight_properties));
>  	props.type = BACKLIGHT_RAW;
>  	props.brightness = val;

[Severity: High]
This is a pre-existing issue, but wled_probe() never calls
platform_set_drvdata() before returning successfully:

drivers/video/backlight/qcom-wled.c:wled_probe() {
...
    bl = devm_backlight_device_register(&pdev->dev, wled->name,
                                        &pdev->dev, wled,
                                        &wled_ops, &props);
    return PTR_ERR_OR_ZERO(bl);
}

Will this cause a guaranteed NULL pointer dereference in wled_remove() when
unbinding the driver?

drivers/video/backlight/qcom-wled.c:wled_remove() {
    struct wled *wled = platform_get_drvdata(pdev);

    mutex_destroy(&wled->lock);
...
}

[Severity: High]
This is another pre-existing issue, but tearing down synchronization primitives
in wled_remove() before the devm-managed backlight is unregistered creates a
use-after-free window.

drivers/video/backlight/qcom-wled.c:wled_remove() {
...
    mutex_destroy(&wled->lock);
    cancel_delayed_work_sync(&wled->ovp_work);
...
}

The backlight device is registered via devm_backlight_device_register(), meaning
it remains active until devres cleanup after wled_remove() returns.

Could this lead to lock corruption or use-after-free if userspace accesses the
sysfs interface concurrently while the driver is being removed?

drivers/video/backlight/qcom-wled.c:wled_update_status() {
...
    mutex_lock(&wled->lock);
...
}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260908-qcom-wled-backlight-v1-1-c4dd4eabda07@ixit.cz?part=1

Re: [PATCH] backlight: qcom-wled: Fix unbalanced OVP IRQ enable at probe

[Konrad Dybcio]

On 9/8/26 10:53 PM, David Heidelberg via B4 Relay wrote:
> From: David Heidelberg <david@ixit.cz>
> 
> wled_configure_ovp_irq() derives the initial state of the OVP interrupt
> from the hardware:

[...]

> +	val = WLED_DEFAULT_BRIGHTNESS;
> +	of_property_read_u32(pdev->dev.of_node, "default-brightness", &val);
> +
> +	/*
> +	 * The module may already be enabled, either by a bootloader that left
> +	 * the backlight lit or by the setup above. Record that, so that the
> +	 * first brightness update does not enable an already enabled module,
> +	 * and so that the OVP irq is armed from probe rather than from that
> +	 * first update.
> +	 */
> +	rc = regmap_read(wled->regmap, wled->ctrl_addr + WLED3_CTRL_REG_MOD_EN,
> +			 &mod_en);
> +	if (rc < 0)
> +		return rc;
> +
> +	if (mod_en & WLED3_CTRL_REG_MOD_EN_MASK)
> +		wled->brightness = val;

This won't work for WLED5, you can implement backlight_ops.get_brightness()
and take advantage of having that function pointer

Otherwise I believe what this patch is good

Konrad