[PATCH v2] leds: class: Use firmware nodes for device lookup

[PATCH v2] leds: class: Use firmware nodes for device lookup

[Alban Bedel]

Replace the OF based lookup with the fwnode equivalent to get support
for ACPI and software nodes.

Signed-off-by: Alban Bedel <alban.bedel@lht.dlh.de>
---
v2: * Keep the doc string
    * Update comment to reference the function now used
---
 drivers/leds/led-class.c | 30 ++++++++++++++++--------------
 1 file changed, 16 insertions(+), 14 deletions(-)

diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
index 9e14ae588f78..a17db3d6644f 100644
--- a/drivers/leds/led-class.c
+++ b/drivers/leds/led-class.c
@@ -249,32 +249,34 @@ 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
+ * fwnode_led_get() - request a LED device via the LED framework
+ * @fwnode: firmware 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,
-				       const char *name)
+static struct led_classdev *fwnode_led_get(struct fwnode_handle *fwnode,
+					   int index, const char *name)
 {
+	struct fwnode_handle *led_node;
 	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 it cannot be found, then fwnode_find_reference() 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);
+		index = fwnode_property_match_string(fwnode, "led-names",
+						     name);
+	led_node = fwnode_find_reference(fwnode, "leds", index);
+	if (IS_ERR(led_node))
+		return ERR_CAST(led_node);
 
-	led_dev = class_find_device_by_fwnode(&leds_class, of_fwnode_handle(led_node));
-	of_node_put(led_node);
+	led_dev = class_find_device_by_fwnode(&leds_class, led_node);
+	fwnode_handle_put(led_node);
 
 	return led_module_get(led_dev);
 }
@@ -332,7 +334,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, NULL);
+	led = fwnode_led_get(dev_fwnode(dev), index, NULL);
 	if (IS_ERR(led))
 		return led;
 
@@ -354,7 +356,7 @@ struct led_classdev *led_get(struct device *dev, char *con_id)
 	const char *provider = NULL;
 	struct device *led_dev;
 
-	led_cdev = of_led_get(dev->of_node, -1, con_id);
+	led_cdev = fwnode_led_get(dev_fwnode(dev), -1, con_id);
 	if (!IS_ERR(led_cdev) || PTR_ERR(led_cdev) != -ENOENT)
 		return led_cdev;
 
-- 
2.39.5

Re: [PATCH v2] leds: class: Use firmware nodes for device lookup

[Lee Jones]

On Wed, 13 May 2026, Alban Bedel wrote:

> Replace the OF based lookup with the fwnode equivalent to get support
> for ACPI and software nodes.
> 
> Signed-off-by: Alban Bedel <alban.bedel@lht.dlh.de>
> ---
> v2: * Keep the doc string
>     * Update comment to reference the function now used
> ---
>  drivers/leds/led-class.c | 30 ++++++++++++++++--------------
>  1 file changed, 16 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
> index 9e14ae588f78..a17db3d6644f 100644
> --- a/drivers/leds/led-class.c
> +++ b/drivers/leds/led-class.c
> @@ -249,32 +249,34 @@ 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
> + * fwnode_led_get() - request a LED device via the LED framework
> + * @fwnode: firmware 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,
> -				       const char *name)
> +static struct led_classdev *fwnode_led_get(struct fwnode_handle *fwnode,
> +					   int index, const char *name)
>  {
> +	struct fwnode_handle *led_node;
>  	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 it cannot be found, then fwnode_find_reference() 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);
> +		index = fwnode_property_match_string(fwnode, "led-names",
> +						     name);
> +	led_node = fwnode_find_reference(fwnode, "leds", index);

What happens if fwnode_property_match_string() returns an error?

> +	if (IS_ERR(led_node))
> +		return ERR_CAST(led_node);
>  
> -	led_dev = class_find_device_by_fwnode(&leds_class, of_fwnode_handle(led_node));
> -	of_node_put(led_node);
> +	led_dev = class_find_device_by_fwnode(&leds_class, led_node);
> +	fwnode_handle_put(led_node);
>  
>  	return led_module_get(led_dev);
>  }
> @@ -332,7 +334,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, NULL);
> +	led = fwnode_led_get(dev_fwnode(dev), index, NULL);
>  	if (IS_ERR(led))
>  		return led;
>  
> @@ -354,7 +356,7 @@ struct led_classdev *led_get(struct device *dev, char *con_id)
>  	const char *provider = NULL;
>  	struct device *led_dev;
>  
> -	led_cdev = of_led_get(dev->of_node, -1, con_id);
> +	led_cdev = fwnode_led_get(dev_fwnode(dev), -1, con_id);
>  	if (!IS_ERR(led_cdev) || PTR_ERR(led_cdev) != -ENOENT)
>  		return led_cdev;
>  
> -- 
> 2.39.5
> 

-- 
Lee Jones

Re: (subset) [PATCH v2] leds: class: Use firmware nodes for device lookup

[Lee Jones]

On Wed, 13 May 2026 13:58:53 +0200, Alban Bedel wrote:
> Replace the OF based lookup with the fwnode equivalent to get support
> for ACPI and software nodes.

Applied, thanks!

[1/1] leds: class: Use firmware nodes for device lookup
      commit: 38cb54cada737fd9511dcb5d1a7cca728b81571c

--
Lee Jones [李琼斯]

Re: [PATCH v2] leds: class: Use firmware nodes for device lookup

[Tommaso Merciai]

Hi Lee,
Thanks for your patch.

On Wed, May 20, 2026 at 04:22:25PM +0100, Lee Jones wrote:
> On Wed, 13 May 2026, Alban Bedel wrote:
> 
> > Replace the OF based lookup with the fwnode equivalent to get support
> > for ACPI and software nodes.
> > 
> > Signed-off-by: Alban Bedel <alban.bedel@lht.dlh.de>
> > ---
> > v2: * Keep the doc string
> >     * Update comment to reference the function now used
> > ---
> >  drivers/leds/led-class.c | 30 ++++++++++++++++--------------
> >  1 file changed, 16 insertions(+), 14 deletions(-)
> > 
> > diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
> > index 9e14ae588f78..a17db3d6644f 100644
> > --- a/drivers/leds/led-class.c
> > +++ b/drivers/leds/led-class.c
> > @@ -249,32 +249,34 @@ 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
> > + * fwnode_led_get() - request a LED device via the LED framework
> > + * @fwnode: firmware 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,
> > -				       const char *name)
> > +static struct led_classdev *fwnode_led_get(struct fwnode_handle *fwnode,
> > +					   int index, const char *name)
> >  {
> > +	struct fwnode_handle *led_node;
> >  	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 it cannot be found, then fwnode_find_reference() 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);
> > +		index = fwnode_property_match_string(fwnode, "led-names",
> > +						     name);
> > +	led_node = fwnode_find_reference(fwnode, "leds", index);
> 
> What happens if fwnode_property_match_string() returns an error?

Agree.

I think we need to check index:
		if (index < 0)
			return ERR_PTR(-ENOENT);

If not I'm getting:

	[   19.008923] ov5645 0-003c: OV5645 detected at address 0x3c
	[   19.014903] ov5645 0-003c: error -EINVAL: getting privacy LED
	[   19.020725] ov5645 0-003c: error -EINVAL: could not register v4l2 device
	[   19.028611] ov5645 0-003c: probe with driver ov5645 failed with error -22

While testing:

OV5645 image sensor with Renesas RZ/G3E:

	- arch/arm64/boot/dts/renesas/r9a09g047e57-smarc-cru-csi-ov5645.dtso

Kind Regards,
Tommaso

> 
> > +	if (IS_ERR(led_node))
> > +		return ERR_CAST(led_node);
> >  
> > -	led_dev = class_find_device_by_fwnode(&leds_class, of_fwnode_handle(led_node));
> > -	of_node_put(led_node);
> > +	led_dev = class_find_device_by_fwnode(&leds_class, led_node);
> > +	fwnode_handle_put(led_node);
> >  
> >  	return led_module_get(led_dev);
> >  }
> > @@ -332,7 +334,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, NULL);
> > +	led = fwnode_led_get(dev_fwnode(dev), index, NULL);
> >  	if (IS_ERR(led))
> >  		return led;
> >  
> > @@ -354,7 +356,7 @@ struct led_classdev *led_get(struct device *dev, char *con_id)
> >  	const char *provider = NULL;
> >  	struct device *led_dev;
> >  
> > -	led_cdev = of_led_get(dev->of_node, -1, con_id);
> > +	led_cdev = fwnode_led_get(dev_fwnode(dev), -1, con_id);
> >  	if (!IS_ERR(led_cdev) || PTR_ERR(led_cdev) != -ENOENT)
> >  		return led_cdev;
> >  
> > -- 
> > 2.39.5
> > 
> 
> -- 
> Lee Jones

Re: [PATCH v2] leds: class: Use firmware nodes for device lookup

[Alban Bedel]

On Tue, 9 Jun 2026 16:46:36 +0200
Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com> wrote:

> Hi Lee,
> Thanks for your patch.
> 
> On Wed, May 20, 2026 at 04:22:25PM +0100, Lee Jones wrote:
> [...]
> > What happens if fwnode_property_match_string() returns an error?  
> 
> Agree.
> 
> I think we need to check index:
> 		if (index < 0)
> 			return ERR_PTR(-ENOENT);

I don't think that's the right solution. The documentation of
fwnode_property_get_reference_args() says that it return -ENOENT
when the index is out range. So it looks like the OF implementation
has a bug.

Looking at of_fwnode_get_reference_args() it directly pass the return
value of __of_parse_phandle_with_args(), which return -EINVAL when the
index is out of range. We should rather fix the OF implementation of
fwnode_property_get_reference_args() to respect the documented
interface.

Alban

Re: [PATCH v2] leds: class: Use firmware nodes for device lookup

[Tommaso Merciai]

Hi Alban,
Thanks for your comments.

On Tue, Jun 09, 2026 at 06:54:29PM +0200, Alban Bedel wrote:
> On Tue, 9 Jun 2026 16:46:36 +0200
> Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com> wrote:
> 
> > Hi Lee,
> > Thanks for your patch.
> > 
> > On Wed, May 20, 2026 at 04:22:25PM +0100, Lee Jones wrote:
> > [...]
> > > What happens if fwnode_property_match_string() returns an error?  
> > 
> > Agree.
> > 
> > I think we need to check index:
> > 		if (index < 0)
> > 			return ERR_PTR(-ENOENT);
> 
> I don't think that's the right solution. The documentation of
> fwnode_property_get_reference_args() says that it return -ENOENT
> when the index is out range. So it looks like the OF implementation
> has a bug.
> 
> Looking at of_fwnode_get_reference_args() it directly pass the return
> value of __of_parse_phandle_with_args(), which return -EINVAL when the
> index is out of range. We should rather fix the OF implementation of
> fwnode_property_get_reference_args() to respect the documented
> interface.

Maybe into of_fwnode_get_reference_args() first thing to do is to check
index with:

	if (index > INT_MAX)
		return -ENOENT;

fwnode_led_get()
    fwnode_property_match_string() -> "led-names" not exist -> index = -EINVAL (-22, int)
    fwnode_find_reference(fwnode, "leds", index)
        /* int(-22) -> unsigned int( index > INT_MAX), fwnode_find_reference takes unsigned int */
        fwnode_property_get_reference_args(fwnode, name, NULL, 0, index, &args)
            of_fwnode_get_reference_args()
                /* index is already unsigned int( index > INT_MAX) */
                of_parse_phandle_with_fixed_args(, index,)
                    /* unsigned int(index > INT_MAX) -> int(-22), takes int */
                    __of_parse_phandle_with_args()
                        if (index < 0)
                            return -EINVAL;


Not sure. What do you think?

Thanks, Tommaso

> 
> Alban
> 

Re: [PATCH v2] leds: class: Use firmware nodes for device lookup

[Alban Bedel]

On Wed, 10 Jun 2026 16:08:49 +0200
Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com> wrote:

> Hi Alban,
> Thanks for your comments.
> 
> On Tue, Jun 09, 2026 at 06:54:29PM +0200, Alban Bedel wrote:
> > On Tue, 9 Jun 2026 16:46:36 +0200
> > Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com> wrote:
> >   
> > > Hi Lee,
> > > Thanks for your patch.
> > > 
> > > On Wed, May 20, 2026 at 04:22:25PM +0100, Lee Jones wrote:
> > > [...]  
> > > > What happens if fwnode_property_match_string() returns an
> > > > error?    
> > > 
> > > Agree.
> > > 
> > > I think we need to check index:
> > > 		if (index < 0)
> > > 			return ERR_PTR(-ENOENT);  
> > 
> > I don't think that's the right solution. The documentation of
> > fwnode_property_get_reference_args() says that it return -ENOENT
> > when the index is out range. So it looks like the OF implementation
> > has a bug.
> > 
> > Looking at of_fwnode_get_reference_args() it directly pass the
> > return value of __of_parse_phandle_with_args(), which return
> > -EINVAL when the index is out of range. We should rather fix the OF
> > implementation of fwnode_property_get_reference_args() to respect
> > the documented interface.  
> 
> Maybe into of_fwnode_get_reference_args() first thing to do is to
> check index with:
> 
> 	if (index > INT_MAX)
> 		return -ENOENT;

Either that or fix it in __of_parse_phandle_with_args() which currently
return -EINVAL for negative indexes. It is used for all variations of
of_parse_phandle_with[_fixed|_optional|]_args() but none of these
documented their error values.

A quick search showed that the vast majority of users either pass a
constant index or a value from a loop under their control, so those
would not be affected by such a change. From the one left all the one I
checked overwrote the returned value with either -ENODEV or -ENOENT,
but I haven't checked them all.

I would tend to do the fix in __of_parse_phandle_with_args() as I don't
really see the value in differentiating negative index from too large
ones in this API.

Alban

Re: [PATCH v2] leds: class: Use firmware nodes for device lookup

[Tommaso Merciai]

On Wed, Jun 10, 2026 at 05:05:41PM +0200, Alban Bedel wrote:
> On Wed, 10 Jun 2026 16:08:49 +0200
> Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com> wrote:
> 
> > Hi Alban,
> > Thanks for your comments.
> > 
> > On Tue, Jun 09, 2026 at 06:54:29PM +0200, Alban Bedel wrote:
> > > On Tue, 9 Jun 2026 16:46:36 +0200
> > > Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com> wrote:
> > >   
> > > > Hi Lee,
> > > > Thanks for your patch.
> > > > 
> > > > On Wed, May 20, 2026 at 04:22:25PM +0100, Lee Jones wrote:
> > > > [...]  
> > > > > What happens if fwnode_property_match_string() returns an
> > > > > error?    
> > > > 
> > > > Agree.
> > > > 
> > > > I think we need to check index:
> > > > 		if (index < 0)
> > > > 			return ERR_PTR(-ENOENT);  
> > > 
> > > I don't think that's the right solution. The documentation of
> > > fwnode_property_get_reference_args() says that it return -ENOENT
> > > when the index is out range. So it looks like the OF implementation
> > > has a bug.

On second thought, given the following call stack:

fwnode_led_get()
    fwnode_property_match_string() -> "led-names" not exist -> index = -EINVAL (-22, int)
    fwnode_find_reference(fwnode, "leds", index)
        /* int(-22) -> unsigned int( index > INT_MAX), fwnode_find_reference takes unsigned int */
        fwnode_property_get_reference_args(fwnode, name, NULL, 0, index, &args)
            of_fwnode_get_reference_args()
                /* index is already unsigned int( index > INT_MAX) */
                of_parse_phandle_with_fixed_args(, index,)
                    /* unsigned int(index > INT_MAX) -> int(-22), takes int */
                    __of_parse_phandle_with_args()
                        if (index < 0)
                            return -EINVAL;

The caller shouldn't pass a signed error as an unsigned int.
I think index should be check into fwnode_led_get().

Thanks, Tommaso

> > > 
> > > Looking at of_fwnode_get_reference_args() it directly pass the
> > > return value of __of_parse_phandle_with_args(), which return
> > > -EINVAL when the index is out of range. We should rather fix the OF
> > > implementation of fwnode_property_get_reference_args() to respect
> > > the documented interface.  
> > 
> > Maybe into of_fwnode_get_reference_args() first thing to do is to
> > check index with:
> > 
> > 	if (index > INT_MAX)
> > 		return -ENOENT;
> 
> Either that or fix it in __of_parse_phandle_with_args() which currently
> return -EINVAL for negative indexes. It is used for all variations of
> of_parse_phandle_with[_fixed|_optional|]_args() but none of these
> documented their error values.
> 
> A quick search showed that the vast majority of users either pass a
> constant index or a value from a loop under their control, so those
> would not be affected by such a change. From the one left all the one I
> checked overwrote the returned value with either -ENODEV or -ENOENT,
> but I haven't checked them all.
> 
> I would tend to do the fix in __of_parse_phandle_with_args() as I don't
> really see the value in differentiating negative index from too large
> ones in this API.
> 
> Alban
>