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

[Alban Bedel <alban.bedel@lht.dlh.de>]

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