Re: [PATCH v3 2/4] dt-bindings: leds: commonize leds property

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

[-- Attachment #1: Type: text/plain, Size: 2312 bytes --]

Hi All,

On 9-Sep-25 12:22 AM, Rob Herring wrote:
> On Mon, Sep 08, 2025 at 09:36:39AM +0200, Konrad Dybcio wrote:
>> On 9/8/25 9:33 AM, Hans de Goede wrote:
>>> Hi,
>>>
>>> On 8-Sep-25 09:20, Konrad Dybcio wrote:
>>>> On 9/8/25 1:18 AM, Aleksandrs Vinarskis wrote:
>>>>> A number of existing schemas use 'leds' property to provide
>>>>> phandle-array of LED(s) to the consumer. Additionally, with the
>>>>> upcoming privacy-led support in device-tree, v4l2 subnode could be a
>>>>> LED consumer, meaning that all camera sensors should support 'leds'
>>>>> and 'led-names' property via common 'video-interface-devices.yaml'.
>>>>>
>>>>> To avoid dublication, commonize 'leds' property from existing schemas
>>>>> to newly introduced 'led-consumer.yaml'.
>>>>>
>>>>> Signed-off-by: Aleksandrs Vinarskis <alex@vinarskis.com>
>>>>> ---
>>>>
>>>> [...]
>>>>
>>>>>  
>>>>> +  leds:
>>>>> +    minItems: 1
>>>>> +    maxItems: 1
>>>>
>>>> My brain compiler suggests this will throw a warning (minItems should
>>>> be redundant in this case)
>>>>> +
>>>>> +  led-names:
>>>>> +    enum:
>>>>> +      - privacy-led
>>>>
>>>> Nit: "privacy" makes more sense without the suffix, as we inherently
>>>> know this is supposed to be an LED
>>>
>>> Note "privacy-led" as name is already used on the x86/ACPI side and
>>> the code consuming this will be shared.
>>>
>>> With that said if there is a strong preference for going with just
>>> "privacy" the x86 side can be adjusted since the provider-info is
>>> generated through a LED lookup table on the x86/ACPI side. So we can
>>> just modify both the lookup table generation as well as the already
>>> existing led_get(dev, "privacy-led") call to use just "privacy"
>>> without problems.
>>
>> In that case, it may be cleaner to just go with what we have today
>> (unless the dt maintainers have stronger opinions)
> 
> Well, I do, but I guess it's fine. Please don't add the suffix on the 
> rest and add a comment for why it's there.

As mentioned dropping the "-led" suffix is no big deal for the ACPI
side and if we don't want the suffix then IMHO we should just drop
it rather then making an exception here.

Attached are 2 patches which drop the suffix on the ACPI side.

If people agree with dropping the suffix I'll officially submit these
upstream.

Regards,

Hans

[-- Attachment #2: 0001-platform-x86-int3472-Use-privacy-as-con_id-for-the-L.patch --]
[-- Type: text/x-patch, Size: 1446 bytes --]

From d98d60b0b8c13fc3ea65159265c21c7bb359e605 Mon Sep 17 00:00:00 2001
From: Hans de Goede <hansg@kernel.org>
Date: Tue, 9 Sep 2025 11:09:37 +0200
Subject: [PATCH 1/2] platform/x86: int3472: Use "privacy" as con_id for the
 LED lookup

During DT-binding review for extending the V4L2 camera sensor privacy LED
support to systems using devicetree, it has come up that having a "-led"
suffix for the LED name / con_id is undesirable since it already is clear
that it is a LED.

Drop the "-led" suffix from the privacy LED's lookup table con_id.

Signed-off-by: Hans de Goede <hansg@kernel.org>
---
Note this needs to be merged in the same kernel cycle as the matching
"media: v4l2-subdev: Use "privacy" as con_id when getting the privacy LED"
patch.
---
 drivers/platform/x86/intel/int3472/led.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/platform/x86/intel/int3472/led.c b/drivers/platform/x86/intel/int3472/led.c
index f1d6d7b0cb75..b1d84b968112 100644
--- a/drivers/platform/x86/intel/int3472/led.c
+++ b/drivers/platform/x86/intel/int3472/led.c
@@ -43,7 +43,7 @@ int skl_int3472_register_pled(struct int3472_discrete_device *int3472, struct gp
 
 	int3472->pled.lookup.provider = int3472->pled.name;
 	int3472->pled.lookup.dev_id = int3472->sensor_name;
-	int3472->pled.lookup.con_id = "privacy-led";
+	int3472->pled.lookup.con_id = "privacy";
 	led_add_lookup(&int3472->pled.lookup);
 
 	return 0;
-- 
2.51.0


[-- Attachment #3: 0002-media-v4l2-subdev-Use-privacy-as-con_id-when-getting.patch --]
[-- Type: text/x-patch, Size: 1503 bytes --]

From 02dce3eec34cd8b894fc5fef009a6e302f58c2f3 Mon Sep 17 00:00:00 2001
From: Hans de Goede <hansg@kernel.org>
Date: Tue, 9 Sep 2025 11:14:34 +0200
Subject: [PATCH 2/2] media: v4l2-subdev: Use "privacy" as con_id when getting
 the privacy LED

During DT-binding review for extending the V4L2 camera sensor privacy LED
support to systems using devicetree, it has come up that having a "-led"
suffix for the LED name / con_id is undesirable since it already is clear
that it is a LED.

Drop the "-led" suffix from the con_id argument to led_get().

Signed-off-by: Hans de Goede <hansg@kernel.org>
---
Note this needs to be merged in the same kernel cycle as the matching
"platform/x86: int3472: Use "privacy" as con_id for the LED lookup"
patch.
---
 drivers/media/v4l2-core/v4l2-subdev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
index 113eb74eb7c1..babcc1120354 100644
--- a/drivers/media/v4l2-core/v4l2-subdev.c
+++ b/drivers/media/v4l2-core/v4l2-subdev.c
@@ -2602,7 +2602,7 @@ EXPORT_SYMBOL_GPL(v4l2_subdev_is_streaming);
 int v4l2_subdev_get_privacy_led(struct v4l2_subdev *sd)
 {
 #if IS_REACHABLE(CONFIG_LEDS_CLASS)
-	sd->privacy_led = led_get(sd->dev, "privacy-led");
+	sd->privacy_led = led_get(sd->dev, "privacy");
 	if (IS_ERR(sd->privacy_led) && PTR_ERR(sd->privacy_led) != -ENOENT)
 		return dev_err_probe(sd->dev, PTR_ERR(sd->privacy_led),
 				     "getting privacy LED\n");
-- 
2.51.0