Hello,
On Thu, Aug 04, 2022 at 01:22:57PM -0600, Simon Glass wrote:
> Hi Michal,
>
> On Thu, 4 Aug 2022 at 11:59, Michal Suchanek <msucha...@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")
> > Signed-off-by: Michal Suchanek <msucha...@suse.de>
> > ---
> > drivers/core/uclass.c | 30 +++++++++++++++++++++---------
> > 1 file changed, 21 insertions(+), 9 deletions(-)
>
> Are you able to create a test for this, e.g. in test/dm/core.c or test-fdt.c ?
>
> Some sandbox devices can be made to give an error, so the test can
> check the logic.
>
> Strangely enough, this is actually a very big change, so we need to be
> careful.
Right, I can have a look at doing some tests.
Knowing what the problem is designig a test should not be too difficult.
It does not make the change any smaller but it can at least prevent
regressions.
Not sure when I get to it, though.
Thanks
Michal
>
> >
> > diff --git a/drivers/core/uclass.c b/drivers/core/uclass.c
> > index 08d9ed82de..ccf7d59141 100644
> > --- a/drivers/core/uclass.c
> > +++ b/drivers/core/uclass.c
> > @@ -574,16 +574,31 @@ int uclass_get_device_by_phandle(enum uclass_id id,
> > struct udevice *parent,
> > }
> > #endif
> >
> > +/* Starting from the given device return first device in the uclass that
> > probes successfully */
> > +static int __uclass_next_device(struct udevice *dev, int ret, struct
> > udevice **devp)
> > +{
> > + if (!dev) {
> > + *devp = dev;
> > + return 0;
> > + }
> > + while ((ret = uclass_get_device_tail(dev, ret, devp))) {
> > + ret = uclass_find_next_device(&dev);
> > + if (!dev) {
> > + *devp = dev;
> > + return 0;
> > + }
> > + }
> > +
> > + return ret;
> > +}
> > +
> > int uclass_first_device(enum uclass_id id, struct udevice **devp)
> > {
> > - struct udevice *dev;
> > + struct udevice *dev = NULL;
> > int ret;
> >
> > - *devp = NULL;
> > ret = uclass_find_first_device(id, &dev);
> > - if (!dev)
> > - return 0;
> > - return uclass_get_device_tail(dev, ret, devp);
> > + return __uclass_next_device(dev, ret, devp);
> > }
> >
> > int uclass_first_device_err(enum uclass_id id, struct udevice **devp)
> > @@ -604,11 +619,8 @@ int uclass_next_device(struct udevice **devp)
> > struct udevice *dev = *devp;
> > int ret;
> >
> > - *devp = NULL;
> > ret = uclass_find_next_device(&dev);
> > - if (!dev)
> > - return 0;
> > - return uclass_get_device_tail(dev, ret, devp);
> > + return __uclass_next_device(dev, ret, devp);
> > }
> >
> > int uclass_next_device_err(struct udevice **devp)
> > --
> > 2.37.1
> >
>
> Regards,
> Simon
