RE: [PATCH v3 1/1] device property: Allow error pointer to be passed to fwnode APIs

From: Sa, Nuno
Date: Tue Mar 08 2022 - 05:32:44 EST


> From: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
> Sent: Tuesday, March 8, 2022 11:10 AM
> To: Sa, Nuno <Nuno.Sa@xxxxxxxxxx>
> Cc: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>; linux-
> acpi@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; Daniel Scally
> <djrscally@xxxxxxxxx>; Heikki Krogerus
> <heikki.krogerus@xxxxxxxxxxxxxxx>; Sakari Ailus
> <sakari.ailus@xxxxxxxxxxxxxxx>; Greg Kroah-Hartman
> <gregkh@xxxxxxxxxxxxxxxxxxx>; Rafael J. Wysocki
> <rafael@xxxxxxxxxx>; Len Brown <lenb@xxxxxxxxxx>
> Subject: Re: [PATCH v3 1/1] device property: Allow error pointer to be
> passed to fwnode APIs
>
> [External]
>
> On Tue, Mar 08, 2022 at 09:25:07AM +0000, Sa, Nuno wrote:
> > > -----Original Message-----
> > > From: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
> > > Sent: Monday, March 7, 2022 9:30 PM
>
> ...
>
> > > v3: dropped test of secondary fwnode (Nuno), added tag (Nuno),
> > > amended commit message
> > > v2: adjusted the entire fwnode API (Sakari)
> > >
> > > Nuno, can you re-test this with the ltc2983 series to be sure it is still
> > > okay?
> >
> > Still works!
>
> Thanks for confirming!
>
> ...
>
> > > @@ -988,14 +998,14 @@
> fwnode_graph_get_next_endpoint(const
> > > struct fwnode_handle *fwnode,
> > > parent = fwnode_graph_get_port_parent(prev);
> > > else
> > > parent = fwnode;
> > > + if (IS_ERR_OR_NULL(parent))
> > > + return NULL;
>
> (1)
>
> > > ep = fwnode_call_ptr_op(parent, graph_get_next_endpoint,
> > > prev);
> > > + if (ep)
> > > + return ep;
> >
> > I might be missing something but before the check being done was
> > 'if (IS_ERR_OR_NULL(ep)'. Is there anyway for ep to be an error
> > pointer? Looking at OF, It seems that only NULL or a valid pointer
> > is being returned. Did not looked at others implementations of
> > though...
>
> Yes, the IS_ERR() part is redundant there. I was quite confused with
> that code while working on this change. So, now it looks much clearer
> what's going on and what kind of values are being expected. This also
> justifies the choice of returned value in (1).
>

Makes sense to me.

Acked-by: Nuno Sá <nuno.sa@xxxxxxxxxx>

> --
> With Best Regards,
> Andy Shevchenko
>