drivers/leds/led-class.c | 30 ++++++++++++++++-------------- 1 file changed, 16 insertions(+), 14 deletions(-)
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.5On 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 [李琼斯]
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
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
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
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
>
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
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
>
© 2016 - 2026 Red Hat, Inc.