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

[sashiko-bot@kernel.org]

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