Re: [PATCH] of: property: Fix of_fwnode_get_reference_args() with negative index

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

On Mon, 15 Jun 2026 11:54:01 +0200
Krzysztof Kozlowski <krzk@kernel.org> wrote:

> On 15/06/2026 11:33, Alban Bedel wrote:
> > On Mon, 15 Jun 2026 08:33:32 +0200
> > Krzysztof Kozlowski <krzk@kernel.org> wrote:
> >   
> >> On Thu, Jun 11, 2026 at 12:28:06PM +0200, Alban Bedel wrote:  
> >>> fwnode_property_get_reference_args() should return -ENOENT when an out
> >>> of bound index is passed. An issue arised with the OF backend because
> >>> the OF API use signed indexes while the fwnode API use unsigned ones.
> >>> When an index value greater the INT_MAX was passed to the OF backend
> >>> it got casted to a negative value and it returned -EINVAL instead of    
> >>
> >> INT_MAX is not out of bound for this function. It is invalid value,
> >> because OF code expects signed.  
> > 
> > But this is fwnode code, it use the OF API but it should implement the
> > fwnode API which, unlike the OF API, use unsigned index.  
> >>> -ENOENT. This patch add a check to of_fwnode_get_reference_args() to
> >>> catch negative index before they are passed to the OF API and return
> >>> -ENOENT right away.    
> >>
> >> I do not understand why are you fixing this issue that way. For this
> >> API, the INT_MAX is correct value, but you claim that it is wrong and
> >> should be ENOENT (even if there is entry).
> >>
> >> Fine, if this is not a correct value, then EINVAL.  
> > 
> > Indices larger than INT_MAX are valid in the fwnode API, so returning
> > -EINVAL is not appropriate here.
> >   
> 
> Then neither ENOENT are.
>
> But really, EINVAL is correct here. This is OF implementation, so this
> implementation decides what is EINVAL and what is right. Not fwnode API.

I think there is a missunderstanding here. The function we are talking
about, of_fwnode_get_reference_args(), is the OF backed implementation
of fwnode_property_get_reference_args(). As such it must follow the API
documented by fwnode_property_get_reference_args() which list the
following return values:

 * Return: %0 on success
 *	    %-ENOENT when the index is out of bounds, the index has an empty
 *		     reference or the property was not found
 *	    %-EINVAL on parse error
 *	    %-ENOTCONN when the remote firmware node exists but has not been
 *		       registered yet

It is not explicitly documented what should be returned if the index is
not representable in the backend, but considering it out of bound seems
to be the most sensible thing to do.

Alban