[PATCH 0/2] leds: freeze delayed hardware access during suspend

[PATCH 0/2] leds: freeze delayed hardware access during suspend

[Jiale Yao]

The TPS6131x torch watchdog refresh and CR0014114 firmware recount
callbacks perform bus transfers from delayed work. Both callbacks use the
non-freezable system workqueue and can run after their I2C or SPI controller
has been suspended.

Queue both callbacks on system_freezable_wq. This freezes their hardware
access before device suspend and resumes it after devices become available
again. Each patch is independent and can be applied separately.

Jiale Yao (2):
  leds: tps6131x: queue torch refresh work on freezable workqueue
  leds: cr0014114: queue recount work on freezable workqueue

 drivers/leds/flash/leds-tps6131x.c | 11 +++++++----
 drivers/leds/leds-cr0014114.c      |  6 ++++--
 2 files changed, 11 insertions(+), 6 deletions(-)

-- 
2.34.1

[PATCH 1/2] leds: tps6131x: queue torch refresh work on freezable workqueue

[Jiale Yao]

The torch watchdog refresh work performs I2C register writes and
reschedules itself every ten seconds while the torch is enabled. Since
schedule_delayed_work() uses the non-freezable system workqueue, the
callback can run after the I2C adapter has been suspended.

Queue the refresh on system_freezable_wq so it is frozen before devices
are suspended and resumed only after they become available again.

Signed-off-by: Jiale Yao <yaojiale02@163.com>
---
 drivers/leds/flash/leds-tps6131x.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/leds/flash/leds-tps6131x.c b/drivers/leds/flash/leds-tps6131x.c
index 5c9a5af5af05..841a6538e75e 100644
--- a/drivers/leds/flash/leds-tps6131x.c
+++ b/drivers/leds/flash/leds-tps6131x.c
@@ -11,6 +11,7 @@
 #include <linux/leds.h>
 #include <linux/module.h>
 #include <linux/regmap.h>
+#include <linux/workqueue.h>
 #include <media/v4l2-flash-led-class.h>
 
 #define TPS6131X_REG_0				0x00
@@ -307,8 +308,9 @@ static void tps6131x_torch_refresh_handler(struct work_struct *work)
 		return;
 	}
 
-	schedule_delayed_work(&tps6131x->torch_refresh_work,
-			      TPS6131X_TORCH_REFRESH_INTERVAL_JIFFIES);
+	queue_delayed_work(system_freezable_wq,
+			   &tps6131x->torch_refresh_work,
+			   TPS6131X_TORCH_REFRESH_INTERVAL_JIFFIES);
 }
 
 static int tps6131x_brightness_set(struct led_classdev *cdev, enum led_brightness brightness)
@@ -367,8 +369,9 @@ static int tps6131x_brightness_set(struct led_classdev *cdev, enum led_brightnes
 	 * To prevent this, the mode must be refreshed within the watchdog timeout.
 	 */
 	if (brightness)
-		schedule_delayed_work(&tps6131x->torch_refresh_work,
-				      TPS6131X_TORCH_REFRESH_INTERVAL_JIFFIES);
+		queue_delayed_work(system_freezable_wq,
+				   &tps6131x->torch_refresh_work,
+				   TPS6131X_TORCH_REFRESH_INTERVAL_JIFFIES);
 
 	return 0;
 }
-- 
2.34.1

[PATCH 2/2] leds: cr0014114: queue recount work on freezable workqueue

[Jiale Yao]

The firmware workaround periodically recounts the LED state by issuing
SPI transfers. Its delayed work runs on the non-freezable system
workqueue and can therefore access the controller after it has been
suspended.

Queue the recount work on system_freezable_wq so hardware access is
deferred until devices have resumed.

Signed-off-by: Jiale Yao <yaojiale02@163.com>
---
 drivers/leds/leds-cr0014114.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/leds/leds-cr0014114.c b/drivers/leds/leds-cr0014114.c
index 3f6931ae0bcc..afe3730e1222 100644
--- a/drivers/leds/leds-cr0014114.c
+++ b/drivers/leds/leds-cr0014114.c
@@ -155,7 +155,8 @@ static void cr0014114_recount_work(struct work_struct *work)
 	if (ret)
 		dev_warn(priv->dev, "sync of LEDs failed %d\n", ret);
 
-	schedule_delayed_work(&priv->work, CR_RECOUNT_DELAY);
+	queue_delayed_work(system_freezable_wq, &priv->work,
+			   CR_RECOUNT_DELAY);
 }
 
 static int cr0014114_set_sync(struct led_classdev *ldev,
@@ -256,7 +257,8 @@ static int cr0014114_probe(struct spi_device *spi)
 		return ret;
 
 	/* setup recount work to workaround buggy firmware */
-	schedule_delayed_work(&priv->work, CR_RECOUNT_DELAY);
+	queue_delayed_work(system_freezable_wq, &priv->work,
+			   CR_RECOUNT_DELAY);
 
 	spi_set_drvdata(spi, priv);
 
-- 
2.34.1

Re: [PATCH 2/2] leds: cr0014114: queue recount work on freezable workqueue

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] leds: Guideline violation: Uncapitalized commit message description
--

commit 2ba0c5cc8cf57633496ac16ae32b61b6ab9d6609
Author: Jiale Yao <yaojiale02@163.com>

leds: cr0014114: queue recount work on freezable workqueue

This commit changes the recount delayed work to run on the
system_freezable_wq instead of the standard workqueue. This defers hardware
access from the workaround until the devices have fully resumed.

> leds: cr0014114: queue recount work on freezable workqueue

[Severity: Low]
This isn't a bug, but the LED subsystem guidelines require the commit
message description after the subsystem prefix to be capitalized. Should
"queue" be changed to "Queue" here to match standard conventions and assist
with Git history categorization?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261016100435.2581303-1-yaojiale02@163.com?part=2

Re: [PATCH 1/2] leds: tps6131x: queue torch refresh work on freezable workqueue

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] leds: tps6131x: torch unintentionally turns back on upon system resume
- [Low] leds: Commit message description is not capitalized
--

commit f183582cffa51c16f099031455716678dcb8eab8
Author: Jiale Yao <yaojiale02@163.com>

leds: tps6131x: queue torch refresh work on freezable workqueue

This commit changes the torch refresh watchdog work to use the
system_freezable_wq instead of the non-freezable system workqueue. This
prevents the work from running and attempting I2C register writes after
the I2C adapter has been suspended.

> leds: tps6131x: queue torch refresh work on freezable workqueue

[Severity: Low]
This isn't a bug, but the description after the subsystem prefix should
be capitalized. Should "queue" be changed to "Queue" to align with the
subsystem conventions?

> diff --git a/drivers/leds/flash/leds-tps6131x.c b/drivers/leds/flash/leds-tps6131x.c
> index 5c9a5af5af05c..841a6538e75e1 100644
> --- a/drivers/leds/flash/leds-tps6131x.c
> +++ b/drivers/leds/flash/leds-tps6131x.c
[ ... ]
> @@ -307,8 +308,9 @@ static void tps6131x_torch_refresh_handler(struct work_struct *work)
>  		return;
>  	}
>  
> -	schedule_delayed_work(&tps6131x->torch_refresh_work,
> -			      TPS6131X_TORCH_REFRESH_INTERVAL_JIFFIES);
> +	queue_delayed_work(system_freezable_wq,
> +			   &tps6131x->torch_refresh_work,
> +			   TPS6131X_TORCH_REFRESH_INTERVAL_JIFFIES);
>  }

[Severity: Medium]
Does this unintentionally cause the torch to turn back on upon system resume?

Because the driver does not set the LED_CORE_SUSPENDRESUME flag, the LED is
not cleanly turned off and its workqueue canceled during suspend. Instead,
during suspend, the hardware watchdog will automatically shut off the LED
after 13 seconds.

When using the previous non-freezable system workqueue, the handler would
execute during suspend, fail to perform I2C transfers because the adapter
is suspended, and return without rescheduling itself. This cleanly broke the
refresh loop.

Now that the workqueue is freezable, won't it simply pause during suspend
and thaw upon resume? When it thaws, tps6131x_torch_refresh_handler() will
execute and force the torch back on via I2C, which seems like an unintended
behavioral regression.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261016100435.2581303-1-yaojiale02@163.com?part=1