Re: [PATCH 1/2] dt-bindings: leds: add generic LED consumer documentation

[Hans de Goede <hansg@kernel.org>]

Hi, 
On 4-Sep-25 8:41 AM, Krzysztof Kozlowski wrote:
> On Thu, Sep 04, 2025 at 01:56:15AM +0200, Aleksandrs Vinarskis wrote:
>>> On Tue, Sep 02, 2025 at 11:10:51AM +0000, Aleksandrs Vinarskis wrote:
>>>> Currently supports passing 'led-names' used to map LED devices to their
>>>> respective functions.
>>>>
>>>> Signed-off-by: Aleksandrs Vinarskis <alex@vinarskis.com>
>>>> ---
>>>>  .../devicetree/bindings/leds/leds-consumer.yaml    | 69 ++++++++++++++++++++++
>>>>  1 file changed, 69 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/leds/leds-consumer.yaml b/Documentation/devicetree/bindings/leds/leds-consumer.yaml
>>>> new file mode 100644
>>>> index 0000000000000000000000000000000000000000..a63e78417df84609e279835f7dae62e3ad2f0bf5
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/leds/leds-consumer.yaml
>>>> @@ -0,0 +1,69 @@
>>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>>> +%YAML 1.2
>>>> +---
>>>> +$id: http://devicetree.org/schemas/leds/leds-consumer.yaml#
>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>> +
>>>> +title: Common leds consumer
>>>> +
>>>> +maintainers:
>>>> +  - Aleksandrs Vinarskis <alex@vinarskis.com>
>>>> +
>>>> +description:
>>>> +  Some LED defined in DT are required by other DT consumers, for example
>>>> +  v4l2 subnode may require privacy or flash LED.
>>>> +
>>>> +  Document LED properties that its consumers may define.
>>>
>>> We already have the trigger-source binding for "attaching" LEDs to 
>>> devices. Why does that not work here?
>>
>> I have not actually considered this, as the existing privacy-led solution
>> from the original series is not trigger based. At least one of the reasons
>> for that is that trigger source can be rather easily altered from user
>> space, which would've been bad for this use case. If v4l2 acquires control
>> over the LED it actually removes triggers and disables sysfs on that LED.
> 
> So does that mean that v4l2 solves the problem of "trigger source can be
> rather easily altered from user space"?

Yes, currently the v4l2-core already does:

	sd->privacy_led = led_get(sd->dev, "privacy-led")
	led_sysfs_disable(sd->privacy_led);

Which disallows changing the LED state or trigger from
userspace. This is similar to how flash-LEDs are handled
which also involves directly controlling the LED rather
then using triggers and which also calls led_sysfs_disable().

led_get() already works for this on x86 and is already used
for this there which is why this code is already there.

I guess the difference with triggers is that triggers are
more of a soft binding between LED and controller of
the LED and where here we need more of a hard binding.

Regards,

Hans