Re: [PATCH v4 09/12] leds: trigger: Add led_trigger_notify_hw_control_changed() interface

Thank you for your contribution! Sashiko AI review found 1 potential issue(=
s) to consider:
- [Medium] The `led_trigger_notify_hw_control_changed()` API performs sleep=
able operations without an internal deferral mechanism, creating a trap for=
 driver authors who might call it directly from hardware interrupt handlers.
--

--- Patch [9]: [PATCH v4 09/12] leds: trigger: Add led_trigger_notify_hw_co=
ntrol_changed() interface ---
commit 21d51858b3771f2be6da6b9704657f9342549cfc
Author: Rong Zhang <[email protected]>

leds: trigger: Add led_trigger_notify_hw_control_changed() interface

This commit introduces a new interface, led_trigger_notify_hw_control_chang=
ed(),
allowing LED drivers to notify the LED core about autonomous hardware
control transitions. The core will then transition the trigger between
"none" and the device's private trigger.

> diff --git a/drivers/leds/led-triggers.c b/drivers/leds/led-triggers.c
> index 64bc95d0bfeb7..2db9fcd1350b8 100644
> --- a/drivers/leds/led-triggers.c
> +++ b/drivers/leds/led-triggers.c
[ ... ]
> @@ -471,6 +492,63 @@ int devm_led_trigger_register(struct device *dev,
[ ... ]
> +void led_trigger_notify_hw_control_changed(struct led_classdev *led_cdev=
, bool activate)
> +{
> +	struct led_trigger *trig;
> +
> +	/* Restricted to private triggers. */
> +	if (WARN_ON(!(led_cdev->flags & LED_TRIG_HW_CHANGED) ||
> +		    !led_cdev->hw_control_trigger || !led_cdev->trigger_type))
> +		return;
> +
> +	scoped_guard(rwsem_read, &triggers_list_lock) {

[Severity: Medium]
Will driver authors likely call this API directly from hardware interrupt
handlers when an autonomous hardware control transition occurs?

This function acquires a sleepable rw_semaphore (triggers_list_lock) and
can subsequently call __led_trigger_set() which uses synchronize_rcu().
Calling this from an atomic context like a hardirq handler would trigger a
'sleeping in atomic context' bug or a deadlock.

Since there is no internal workqueue deferral mechanism (unlike
led_trigger_event()) and no might_sleep() annotation, could this become a
trap for driver authors?

> +		list_for_each_entry(trig, &trigger_list, next_trig) {
> +			if (trig->trigger_type =3D=3D led_cdev->trigger_type &&
> +			    !strcmp(trig->name, led_cdev->hw_control_trigger)) {
> +				guard(rwsem_write)(&led_cdev->trigger_lock);
> +
> +				led_trigger_do_hw_control_transition(led_cdev, activate, trig);
> +				return;
> +			}
> +		}
> +	}
> +
> +	dev_err(led_cdev->dev,
> +		"%s() is called, but the private trigger (%s) is not properly register=
ed\n",
> +		__func__, led_cdev->hw_control_trigger);
> +}

--=20
Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/20260802-leds-trigg=
[email protected]?part=3D9