[PATCH v3] dm: core: Do not stop uclass iteration on error

[Michal Suchánek]

On Sat, Aug 27, 2022 at 07:52:27PM -0600, Simon Glass wrote:
> Hi Michal,
> 
> On Fri, 19 Aug 2022 at 14:23, Michal Suchanek <msuchanek at suse.de> wrote:
> >
> > When probing a device fails NULL pointer is returned, and other devices
> > cannot be iterated. Skip to next device on error instead.
> >
> > Fixes: 6494d708bf ("dm: Add base driver model support")
> 
> I think you should drop this as you are doing a change of behaviour,
> not fixing a bug!

You can hardly fix a bug without a change in behavior.

These functions are used for iterating devices, and are not iterating
devices. That's clearly a bug.

> > Signed-off-by: Michal Suchanek <msuchanek at suse.de>
> > ---
> > v2: - Fix up tests
> > v3: - Fix up API doc
> >     - Correctly forward error from uclass_get
> >     - Do not return an error when last device fails to probe
> >     - Drop redundant initialization
> >     - Wrap at 80 columns
> > ---
> >  drivers/core/uclass.c | 32 ++++++++++++++++++++++++--------
> >  include/dm/uclass.h   | 13 ++++++++-----
> >  test/dm/test-fdt.c    | 20 ++++++++++++++++----
> >  3 files changed, 48 insertions(+), 17 deletions(-)
> 
> Unfortunately this still fails one test. Try 'make qcheck' to see it -
> it is ethernet.

I will look at that.

> I actually think you should create new functions for this feature,
> e.g.uclass_first_device_ok(), since it makes it impossible to see what
> when wrong with a device in the middle.
> 
> I have long had all this in my mind. One idea for a future change is
> to return the error, but set dev, so that the caller knows there is a
> device, which failed. When we are at the end, dev is set to NULL.

We already have uclass_first_device_check() and
uclass_next_device_check() to iterate all devices, including broken
ones, and getting the errors as well.

That's for the case you want all the details, and these are for the case
you just want to get devices and don't care about the details.

That's AFAICT as much as this iteration interface can provide, and we
have both cases covered.

Thanks

Michal