Hi Laurent,
Thanks for the review.
On Wed, Feb 23, 2022 at 12:15:27PM +0200, Laurent Pinchart wrote:
> Hi Sakari,
>
> Thank you for the patch.
>
> On Wed, Feb 23, 2022 at 11:47:20AM +0200, Sakari Ailus wrote:
> > v4l2_fwnode_parse_reference() relied on counting the number of references
> > for async array memory allocation. The array is long gone so remove
> > counting the references now.
> >
> > This also changes how the function arrives in different unsuccessful
> > return values but the functionality remains unchanged.
> >
> > Also the check for -ENODATA is removed, it was made redundant by commit
> > c343bc2ce2c6 ("ACPI: properties: Align return codes of
> > __acpi_node_get_property_reference()").
>
> I would have done this first in a separate patch.
I'll split this.
>
> > Signed-off-by: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx>
> > ---
> > drivers/media/v4l2-core/v4l2-fwnode.c | 28 ++++++++-------------------
> > 1 file changed, 8 insertions(+), 20 deletions(-)
> >
> > diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c
> > index 71dcc9a96535..fe2dfc8f9d56 100644
> > --- a/drivers/media/v4l2-core/v4l2-fwnode.c
> > +++ b/drivers/media/v4l2-core/v4l2-fwnode.c
> > @@ -894,25 +894,8 @@ static int v4l2_fwnode_reference_parse(struct device *dev,
> > int ret;
> >
> > for (index = 0;
> > - !(ret = fwnode_property_get_reference_args(dev_fwnode(dev),
> > - prop, NULL, 0,
> > - index, &args));
> > - index++)
> > - fwnode_handle_put(args.fwnode);
> > -
> > - if (!index)
> > - return -ENOENT;
> > -
> > - /*
> > - * Note that right now both -ENODATA and -ENOENT may signal
> > - * out-of-bounds access. Return the error in cases other than that.
> > - */
> > - if (ret != -ENOENT && ret != -ENODATA)
> > - return ret;
> > -
> > - for (index = 0;
> > - !fwnode_property_get_reference_args(dev_fwnode(dev), prop, NULL,
> > - 0, index, &args);
> > + !(ret = fwnode_property_get_reference_args(dev_fwnode(dev), prop,
> > + NULL, 0, index, &args));
> > index++) {
> > struct v4l2_async_subdev *asd;
> >
> > @@ -928,7 +911,12 @@ static int v4l2_fwnode_reference_parse(struct device *dev,
> > }
> > }
> >
> > - return 0;
> > + /* -ENOENT here means successful parsing */
> > + if (ret != -ENOENT)
> > + return ret;
> > +
> > + /* Return -ENOENT if no references were found */
> > + return index ?: -ENOENT;
>
> The function now returns index on success, while it used to return 0. Is
> it intentional ?
No, I'll fix that for v2.
--
Regards,
Sakari Ailus
