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

[Lee Jones <lee@kernel.org>]

On Fri, 16 Oct 2026, sashiko-bot@kernel.org wrote:

> 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

This needs an answer.

> - [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

-- 
Lee Jones