From: Hans de Goede <hansg@kernel.org>
Add 'name' argument to of_led_get() such that it can lookup LEDs in
devicetree by either name or index.
And use this modified function to add devicetree support to the generic
(non devicetree specific) [devm_]led_get() function.
This uses the standard devicetree pattern of adding a -names string array
to map names to the indexes for an array of resources.
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Reviewed-by: Lee Jones <lee@kernel.org>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Hans de Goede <hansg@kernel.org>
Signed-off-by: Aleksandrs Vinarskis <alex@vinarskis.com>
---
drivers/leds/led-class.c | 17 +++++++++++++++--
1 file changed, 15 insertions(+), 2 deletions(-)
diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
index 15633fbf3c166aa4f521774d245f6399a642bced..f3faf37f9a08ac762ed87b91cb3cab5faa8eacb0 100644
--- a/drivers/leds/led-class.c
+++ b/drivers/leds/led-class.c
@@ -252,15 +252,23 @@ static const struct class leds_class = {
* of_led_get() - request a LED device via the LED framework
* @np: device node to get the LED device from
* @index: the index of the LED
+ * @name: the name of the LED used to map it to its function, if present
*
* Returns the LED device parsed from the phandle specified in the "leds"
* property of a device tree node or a negative error-code on failure.
*/
-static struct led_classdev *of_led_get(struct device_node *np, int index)
+static struct led_classdev *of_led_get(struct device_node *np, int index,
+ const char *name)
{
struct device *led_dev;
struct device_node *led_node;
+ /*
+ * For named LEDs, first look up the name in the "led-names" property.
+ * If it cannot be found, then of_parse_phandle() will propagate the error.
+ */
+ if (name)
+ index = of_property_match_string(np, "led-names", name);
led_node = of_parse_phandle(np, "leds", index);
if (!led_node)
return ERR_PTR(-ENOENT);
@@ -324,7 +332,7 @@ struct led_classdev *__must_check devm_of_led_get(struct device *dev,
if (!dev)
return ERR_PTR(-EINVAL);
- led = of_led_get(dev->of_node, index);
+ led = of_led_get(dev->of_node, index, NULL);
if (IS_ERR(led))
return led;
@@ -342,9 +350,14 @@ EXPORT_SYMBOL_GPL(devm_of_led_get);
struct led_classdev *led_get(struct device *dev, char *con_id)
{
struct led_lookup_data *lookup;
+ struct led_classdev *led_cdev;
const char *provider = NULL;
struct device *led_dev;
+ led_cdev = of_led_get(dev->of_node, -1, con_id);
+ if (!IS_ERR(led_cdev) || PTR_ERR(led_cdev) != -ENOENT)
+ return led_cdev;
+
mutex_lock(&leds_lookup_lock);
list_for_each_entry(lookup, &leds_lookup_list, list) {
if (!strcmp(lookup->dev_id, dev_name(dev)) &&
--
2.48.1On Wed, 10 Sep 2025, Aleksandrs Vinarskis wrote:
> From: Hans de Goede <hansg@kernel.org>
>
> Add 'name' argument to of_led_get() such that it can lookup LEDs in
> devicetree by either name or index.
>
> And use this modified function to add devicetree support to the generic
> (non devicetree specific) [devm_]led_get() function.
>
> This uses the standard devicetree pattern of adding a -names string array
> to map names to the indexes for an array of resources.
>
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> Reviewed-by: Lee Jones <lee@kernel.org>
Remind me why this can't go in through LED again?
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> Signed-off-by: Hans de Goede <hansg@kernel.org>
> Signed-off-by: Aleksandrs Vinarskis <alex@vinarskis.com>
> ---
> drivers/leds/led-class.c | 17 +++++++++++++++--
> 1 file changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
> index 15633fbf3c166aa4f521774d245f6399a642bced..f3faf37f9a08ac762ed87b91cb3cab5faa8eacb0 100644
> --- a/drivers/leds/led-class.c
> +++ b/drivers/leds/led-class.c
> @@ -252,15 +252,23 @@ static const struct class leds_class = {
> * of_led_get() - request a LED device via the LED framework
> * @np: device node to get the LED device from
> * @index: the index of the LED
> + * @name: the name of the LED used to map it to its function, if present
> *
> * Returns the LED device parsed from the phandle specified in the "leds"
> * property of a device tree node or a negative error-code on failure.
> */
> -static struct led_classdev *of_led_get(struct device_node *np, int index)
> +static struct led_classdev *of_led_get(struct device_node *np, int index,
> + const char *name)
> {
> struct device *led_dev;
> struct device_node *led_node;
>
> + /*
> + * For named LEDs, first look up the name in the "led-names" property.
> + * If it cannot be found, then of_parse_phandle() will propagate the error.
> + */
> + if (name)
> + index = of_property_match_string(np, "led-names", name);
> led_node = of_parse_phandle(np, "leds", index);
> if (!led_node)
> return ERR_PTR(-ENOENT);
> @@ -324,7 +332,7 @@ struct led_classdev *__must_check devm_of_led_get(struct device *dev,
> if (!dev)
> return ERR_PTR(-EINVAL);
>
> - led = of_led_get(dev->of_node, index);
> + led = of_led_get(dev->of_node, index, NULL);
> if (IS_ERR(led))
> return led;
>
> @@ -342,9 +350,14 @@ EXPORT_SYMBOL_GPL(devm_of_led_get);
> struct led_classdev *led_get(struct device *dev, char *con_id)
> {
> struct led_lookup_data *lookup;
> + struct led_classdev *led_cdev;
> const char *provider = NULL;
> struct device *led_dev;
>
> + led_cdev = of_led_get(dev->of_node, -1, con_id);
> + if (!IS_ERR(led_cdev) || PTR_ERR(led_cdev) != -ENOENT)
> + return led_cdev;
> +
> mutex_lock(&leds_lookup_lock);
> list_for_each_entry(lookup, &leds_lookup_list, list) {
> if (!strcmp(lookup->dev_id, dev_name(dev)) &&
>
> --
> 2.48.1
>
>
--
Lee Jones [李琼斯]
Hi Lee,
On 11-Sep-25 10:15 AM, Lee Jones wrote:
> On Wed, 10 Sep 2025, Aleksandrs Vinarskis wrote:
>
>> From: Hans de Goede <hansg@kernel.org>
>>
>> Add 'name' argument to of_led_get() such that it can lookup LEDs in
>> devicetree by either name or index.
>>
>> And use this modified function to add devicetree support to the generic
>> (non devicetree specific) [devm_]led_get() function.
>>
>> This uses the standard devicetree pattern of adding a -names string array
>> to map names to the indexes for an array of resources.
>>
>> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
>> Reviewed-by: Lee Jones <lee@kernel.org>
>
> Remind me why this can't go in through LED again?
I don't think anyone has discussed how to merge this yet.
I believe that the LED tree is the correct tree to merge this
entire series through, once the DT bits have been reviewed.
Regards,
Hans
>> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
>> Signed-off-by: Hans de Goede <hansg@kernel.org>
>> Signed-off-by: Aleksandrs Vinarskis <alex@vinarskis.com>
>> ---
>> drivers/leds/led-class.c | 17 +++++++++++++++--
>> 1 file changed, 15 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
>> index 15633fbf3c166aa4f521774d245f6399a642bced..f3faf37f9a08ac762ed87b91cb3cab5faa8eacb0 100644
>> --- a/drivers/leds/led-class.c
>> +++ b/drivers/leds/led-class.c
>> @@ -252,15 +252,23 @@ static const struct class leds_class = {
>> * of_led_get() - request a LED device via the LED framework
>> * @np: device node to get the LED device from
>> * @index: the index of the LED
>> + * @name: the name of the LED used to map it to its function, if present
>> *
>> * Returns the LED device parsed from the phandle specified in the "leds"
>> * property of a device tree node or a negative error-code on failure.
>> */
>> -static struct led_classdev *of_led_get(struct device_node *np, int index)
>> +static struct led_classdev *of_led_get(struct device_node *np, int index,
>> + const char *name)
>> {
>> struct device *led_dev;
>> struct device_node *led_node;
>>
>> + /*
>> + * For named LEDs, first look up the name in the "led-names" property.
>> + * If it cannot be found, then of_parse_phandle() will propagate the error.
>> + */
>> + if (name)
>> + index = of_property_match_string(np, "led-names", name);
>> led_node = of_parse_phandle(np, "leds", index);
>> if (!led_node)
>> return ERR_PTR(-ENOENT);
>> @@ -324,7 +332,7 @@ struct led_classdev *__must_check devm_of_led_get(struct device *dev,
>> if (!dev)
>> return ERR_PTR(-EINVAL);
>>
>> - led = of_led_get(dev->of_node, index);
>> + led = of_led_get(dev->of_node, index, NULL);
>> if (IS_ERR(led))
>> return led;
>>
>> @@ -342,9 +350,14 @@ EXPORT_SYMBOL_GPL(devm_of_led_get);
>> struct led_classdev *led_get(struct device *dev, char *con_id)
>> {
>> struct led_lookup_data *lookup;
>> + struct led_classdev *led_cdev;
>> const char *provider = NULL;
>> struct device *led_dev;
>>
>> + led_cdev = of_led_get(dev->of_node, -1, con_id);
>> + if (!IS_ERR(led_cdev) || PTR_ERR(led_cdev) != -ENOENT)
>> + return led_cdev;
>> +
>> mutex_lock(&leds_lookup_lock);
>> list_for_each_entry(lookup, &leds_lookup_list, list) {
>> if (!strcmp(lookup->dev_id, dev_name(dev)) &&
>>
>> --
>> 2.48.1
>>
>>
>
On Thu, Sep 11, 2025 at 11:01:00AM +0200, Hans de Goede wrote:
> Hi Lee,
>
> On 11-Sep-25 10:15 AM, Lee Jones wrote:
> > On Wed, 10 Sep 2025, Aleksandrs Vinarskis wrote:
> >
> >> From: Hans de Goede <hansg@kernel.org>
> >>
> >> Add 'name' argument to of_led_get() such that it can lookup LEDs in
> >> devicetree by either name or index.
> >>
> >> And use this modified function to add devicetree support to the generic
> >> (non devicetree specific) [devm_]led_get() function.
> >>
> >> This uses the standard devicetree pattern of adding a -names string array
> >> to map names to the indexes for an array of resources.
> >>
> >> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> >> Reviewed-by: Lee Jones <lee@kernel.org>
> >
> > Remind me why this can't go in through LED again?
>
> I don't think anyone has discussed how to merge this yet.
>
> I believe that the LED tree is the correct tree to merge this
> entire series through, once the DT bits have been reviewed.
>
Unless there are some strong reasons (that I'm failing to spot), we
should merge the DeviceTree binding and implementation through the LED
tree. Then I merge the DTS change through the Qualcomm DT tree once the
bindings are available in linux-next.
Regards,
Bjorn
> Regards,
>
> Hans
>
>
>
>
> >> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> >> Signed-off-by: Hans de Goede <hansg@kernel.org>
> >> Signed-off-by: Aleksandrs Vinarskis <alex@vinarskis.com>
> >> ---
> >> drivers/leds/led-class.c | 17 +++++++++++++++--
> >> 1 file changed, 15 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
> >> index 15633fbf3c166aa4f521774d245f6399a642bced..f3faf37f9a08ac762ed87b91cb3cab5faa8eacb0 100644
> >> --- a/drivers/leds/led-class.c
> >> +++ b/drivers/leds/led-class.c
> >> @@ -252,15 +252,23 @@ static const struct class leds_class = {
> >> * of_led_get() - request a LED device via the LED framework
> >> * @np: device node to get the LED device from
> >> * @index: the index of the LED
> >> + * @name: the name of the LED used to map it to its function, if present
> >> *
> >> * Returns the LED device parsed from the phandle specified in the "leds"
> >> * property of a device tree node or a negative error-code on failure.
> >> */
> >> -static struct led_classdev *of_led_get(struct device_node *np, int index)
> >> +static struct led_classdev *of_led_get(struct device_node *np, int index,
> >> + const char *name)
> >> {
> >> struct device *led_dev;
> >> struct device_node *led_node;
> >>
> >> + /*
> >> + * For named LEDs, first look up the name in the "led-names" property.
> >> + * If it cannot be found, then of_parse_phandle() will propagate the error.
> >> + */
> >> + if (name)
> >> + index = of_property_match_string(np, "led-names", name);
> >> led_node = of_parse_phandle(np, "leds", index);
> >> if (!led_node)
> >> return ERR_PTR(-ENOENT);
> >> @@ -324,7 +332,7 @@ struct led_classdev *__must_check devm_of_led_get(struct device *dev,
> >> if (!dev)
> >> return ERR_PTR(-EINVAL);
> >>
> >> - led = of_led_get(dev->of_node, index);
> >> + led = of_led_get(dev->of_node, index, NULL);
> >> if (IS_ERR(led))
> >> return led;
> >>
> >> @@ -342,9 +350,14 @@ EXPORT_SYMBOL_GPL(devm_of_led_get);
> >> struct led_classdev *led_get(struct device *dev, char *con_id)
> >> {
> >> struct led_lookup_data *lookup;
> >> + struct led_classdev *led_cdev;
> >> const char *provider = NULL;
> >> struct device *led_dev;
> >>
> >> + led_cdev = of_led_get(dev->of_node, -1, con_id);
> >> + if (!IS_ERR(led_cdev) || PTR_ERR(led_cdev) != -ENOENT)
> >> + return led_cdev;
> >> +
> >> mutex_lock(&leds_lookup_lock);
> >> list_for_each_entry(lookup, &leds_lookup_list, list) {
> >> if (!strcmp(lookup->dev_id, dev_name(dev)) &&
> >>
> >> --
> >> 2.48.1
> >>
> >>
> >
>
On Tue, 16 Sep 2025, Bjorn Andersson wrote: > On Thu, Sep 11, 2025 at 11:01:00AM +0200, Hans de Goede wrote: > > Hi Lee, > > > > On 11-Sep-25 10:15 AM, Lee Jones wrote: > > > On Wed, 10 Sep 2025, Aleksandrs Vinarskis wrote: > > > > > >> From: Hans de Goede <hansg@kernel.org> > > >> > > >> Add 'name' argument to of_led_get() such that it can lookup LEDs in > > >> devicetree by either name or index. > > >> > > >> And use this modified function to add devicetree support to the generic > > >> (non devicetree specific) [devm_]led_get() function. > > >> > > >> This uses the standard devicetree pattern of adding a -names string array > > >> to map names to the indexes for an array of resources. > > >> > > >> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com> > > >> Reviewed-by: Lee Jones <lee@kernel.org> > > > > > > Remind me why this can't go in through LED again? > > > > I don't think anyone has discussed how to merge this yet. > > > > I believe that the LED tree is the correct tree to merge this > > entire series through, once the DT bits have been reviewed. > > > > Unless there are some strong reasons (that I'm failing to spot), we > should merge the DeviceTree binding and implementation through the LED > tree. Then I merge the DTS change through the Qualcomm DT tree once the > bindings are available in linux-next. 1-3 have been applied to the LED tree already. -- Lee Jones [李琼斯]
On Tuesday, September 16th, 2025 at 18:07, Lee Jones <lee@kernel.org> wrote: > > > On Tue, 16 Sep 2025, Bjorn Andersson wrote: > > > On Thu, Sep 11, 2025 at 11:01:00AM +0200, Hans de Goede wrote: > > > > > Hi Lee, > > > > > > On 11-Sep-25 10:15 AM, Lee Jones wrote: > > > > > > > On Wed, 10 Sep 2025, Aleksandrs Vinarskis wrote: > > > > > > > > > From: Hans de Goede hansg@kernel.org > > > > > > > > > > Add 'name' argument to of_led_get() such that it can lookup LEDs in > > > > > devicetree by either name or index. > > > > > > > > > > And use this modified function to add devicetree support to the generic > > > > > (non devicetree specific) [devm_]led_get() function. > > > > > > > > > > This uses the standard devicetree pattern of adding a -names string array > > > > > to map names to the indexes for an array of resources. > > > > > > > > > > Reviewed-by: Andy Shevchenko andy.shevchenko@gmail.com > > > > > Reviewed-by: Lee Jones lee@kernel.org > > > > > > > > Remind me why this can't go in through LED again? > > > > > > I don't think anyone has discussed how to merge this yet. > > > > > > I believe that the LED tree is the correct tree to merge this > > > entire series through, once the DT bits have been reviewed. > > > > Unless there are some strong reasons (that I'm failing to spot), we > > should merge the DeviceTree binding and implementation through the LED > > tree. Then I merge the DTS change through the Qualcomm DT tree once the > > bindings are available in linux-next. Hi Bjorn, The bindings are now in linux-next. Could you please pick the DTS change? Thanks, Alex > > > 1-3 have been applied to the LED tree already. > > -- > Lee Jones [李琼斯]
On 9/10/25 2:01 PM, Aleksandrs Vinarskis wrote: > From: Hans de Goede <hansg@kernel.org> > > Add 'name' argument to of_led_get() such that it can lookup LEDs in > devicetree by either name or index. > > And use this modified function to add devicetree support to the generic > (non devicetree specific) [devm_]led_get() function. > > This uses the standard devicetree pattern of adding a -names string array > to map names to the indexes for an array of resources. > > Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com> > Reviewed-by: Lee Jones <lee@kernel.org> > Reviewed-by: Linus Walleij <linus.walleij@linaro.org> > Signed-off-by: Hans de Goede <hansg@kernel.org> > Signed-off-by: Aleksandrs Vinarskis <alex@vinarskis.com> > --- I was thinking, perhaps we should introduce some sort of an exclusive access mechanism, so that the e.g. user (or malware) can't listen to uevents and immediately shut down the LED over sysfs Konrad
On Wednesday, September 10th, 2025 at 14:22, Konrad Dybcio <konrad.dybcio@oss.qualcomm.com> wrote:
>
>
> On 9/10/25 2:01 PM, Aleksandrs Vinarskis wrote:
>
> > From: Hans de Goede hansg@kernel.org
> >
> > Add 'name' argument to of_led_get() such that it can lookup LEDs in
> > devicetree by either name or index.
> >
> > And use this modified function to add devicetree support to the generic
> > (non devicetree specific) [devm_]led_get() function.
> >
> > This uses the standard devicetree pattern of adding a -names string array
> > to map names to the indexes for an array of resources.
> >
> > Reviewed-by: Andy Shevchenko andy.shevchenko@gmail.com
> > Reviewed-by: Lee Jones lee@kernel.org
> > Reviewed-by: Linus Walleij linus.walleij@linaro.org
> > Signed-off-by: Hans de Goede hansg@kernel.org
> > Signed-off-by: Aleksandrs Vinarskis alex@vinarskis.com
> > ---
>
>
> I was thinking, perhaps we should introduce some sort of an exclusive
> access mechanism, so that the e.g. user (or malware) can't listen to
> uevents and immediately shut down the LED over sysfs
It is already done by the original series from Hans (linked in cover),
which was merged few years back. It is also the reason why this
approach is used instead of typically used trigger-source - that
would've indeed allowed anyone with access to sysfs to disable the
indicator.
As per Hans [1], v4l2-core would disable sysfs of privacy indicator:
sd->privacy_led = led_get(sd->dev, "privacy-led")
led_sysfs_disable(sd->privacy_led);
Of course, this security only holds if one has secure boot enforced,
kernel, modules, _and_ device-tree blobs are signed.
Regards,
Alex
[1] https://lore.kernel.org/all/daf442a6-b4d6-4213-8ec0-10397d682cc4@kernel.org/
>
> Konrad
On 9/10/25 2:54 PM, Aleksandrs Vinarskis wrote: > > > > > > On Wednesday, September 10th, 2025 at 14:22, Konrad Dybcio <konrad.dybcio@oss.qualcomm.com> wrote: > >> >> >> On 9/10/25 2:01 PM, Aleksandrs Vinarskis wrote: >> >>> From: Hans de Goede hansg@kernel.org >>> >>> Add 'name' argument to of_led_get() such that it can lookup LEDs in >>> devicetree by either name or index. >>> >>> And use this modified function to add devicetree support to the generic >>> (non devicetree specific) [devm_]led_get() function. >>> >>> This uses the standard devicetree pattern of adding a -names string array >>> to map names to the indexes for an array of resources. >>> >>> Reviewed-by: Andy Shevchenko andy.shevchenko@gmail.com >>> Reviewed-by: Lee Jones lee@kernel.org >>> Reviewed-by: Linus Walleij linus.walleij@linaro.org >>> Signed-off-by: Hans de Goede hansg@kernel.org >>> Signed-off-by: Aleksandrs Vinarskis alex@vinarskis.com >>> --- >> >> >> I was thinking, perhaps we should introduce some sort of an exclusive >> access mechanism, so that the e.g. user (or malware) can't listen to >> uevents and immediately shut down the LED over sysfs > > It is already done by the original series from Hans (linked in cover), > which was merged few years back. It is also the reason why this > approach is used instead of typically used trigger-source - that > would've indeed allowed anyone with access to sysfs to disable the > indicator. > > As per Hans [1], v4l2-core would disable sysfs of privacy indicator: > > sd->privacy_led = led_get(sd->dev, "privacy-led") > led_sysfs_disable(sd->privacy_led); > > > Of course, this security only holds if one has secure boot enforced, > kernel, modules, _and_ device-tree blobs are signed. Great, thank you for this context Konrad
© 2016 - 2025 Red Hat, Inc.