Hi Michal,

On Wed, 17 Aug 2022 at 02:28, Michal Suchanek msuchanek@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 msuchanek@suse.de

---

v2: Fix up tests

Note: there is seemingly bogus repeated device_remove(parent,

DM_REMOVE_NORMAL);

but I have no idea what the intent was, and fixing that is out of the scope of this patch anyway.

This is to remove child devices that have been probed, so that we get back to the original state.

drivers/core/uclass.c | 30 +++++++++++++++++++++--------- test/dm/test-fdt.c | 20 ++++++++++++++++---- 2 files changed, 37 insertions(+), 13 deletions(-)

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)

Can you avoid __ as this is reserved for compiler. Perhaps use a single underscore?

Please check 80cols

+{

•   if (!dev) {
  

•           *devp = dev;
  

•           return 0;
  

•   }
  

Is this for people that call next after they shouldn't?

•   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;
  

Can you drop the NULL assignment? uclass_find_first_device() sets dev to NULL anyway, as a first step.

    int ret;

•   *devp = NULL;
  

Is this safe to remove? If there is nothing, then

    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);
  

}

This is a major change in behaviour, so please do update the API docs at dm/uclass.h

int uclass_next_device_err(struct udevice **devp) diff --git a/test/dm/test-fdt.c b/test/dm/test-fdt.c index 6118ad42ca..165b4f5554 100644 --- a/test/dm/test-fdt.c +++ b/test/dm/test-fdt.c @@ -417,16 +417,28 @@ static int dm_test_first_next_device(struct

unit_test_state *uts)

    pdata = dev_get_plat(dev);
    pdata->probe_err = -ENOMEM;
    device_remove(parent, DM_REMOVE_NORMAL);

•   ut_assertok(uclass_first_device(UCLASS_TEST_PROBE, &dev));
  

•   ut_asserteq(-ENOMEM, uclass_next_device(&dev));
  

•   ut_asserteq_ptr(dev, NULL);
  

•   for (ret = uclass_first_device(UCLASS_TEST_PROBE, &dev), count =
  

0;

•        dev;
  

•        ret = uclass_next_device(&dev)) {
  

•           count++;
  

•           parent = dev_get_parent(dev);
  

•           }
  

•   ut_assertok(ret);
  

•   ut_asserteq(3, count);
  
    /* Now an error on the first one */
    ut_assertok(uclass_get_device(UCLASS_TEST_PROBE, 0, &dev));
    pdata = dev_get_plat(dev);
    pdata->probe_err = -ENOENT;
    device_remove(parent, DM_REMOVE_NORMAL);
  

•   ut_asserteq(-ENOENT, uclass_first_device(UCLASS_TEST_PROBE,
  

&dev));

•   for (ret = uclass_first_device(UCLASS_TEST_PROBE, &dev), count =
  

0;

•        dev;
  

•        ret = uclass_next_device(&dev)) {
  

•           count++;
  

•           parent = dev_get_parent(dev);
  

•           }
  

•   ut_assertok(ret);
  

•   ut_asserteq(2, count);
  
    return 0;
  

}

2.37.1

Regards, Simon

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 msuchanek@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(-)

diff --git a/drivers/core/uclass.c b/drivers/core/uclass.c index 08d9ed82de..8f4ecad26c 100644 --- a/drivers/core/uclass.c +++ b/drivers/core/uclass.c @@ -574,16 +574,35 @@ 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 ret; + } + while ((ret = uclass_get_device_tail(dev, ret, devp))) { + ret = uclass_find_next_device(&dev); + if (!dev) { + *devp = dev; + return 0; + } + } + + return 0; +} + int uclass_first_device(enum uclass_id id, struct udevice **devp) { struct udevice *dev; 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 +623,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) diff --git a/include/dm/uclass.h b/include/dm/uclass.h index f6c0110b06..76caa0d2b9 100644 --- a/include/dm/uclass.h +++ b/include/dm/uclass.h @@ -320,14 +320,14 @@ int uclass_get_device_by_driver(enum uclass_id id, const struct driver *drv, * uclass_first_device() - Get the first device in a uclass * * The device returned is probed if necessary, and ready for use + * Devices that fail to probe are skipped * * This function is useful to start iterating through a list of devices which * are functioning correctly and can be probed. * * @id: Uclass ID to look up * @devp: Returns pointer to the first device in that uclass if no error - * occurred, or NULL if there is no first device, or an error occurred with - * that device. + * occurred, or NULL if there is no usable device * Return: 0 if OK (found or not found), other -ve on error */ int uclass_first_device(enum uclass_id id, struct udevice **devp); @@ -336,6 +336,7 @@ int uclass_first_device(enum uclass_id id, struct udevice **devp); * uclass_first_device_err() - Get the first device in a uclass * * The device returned is probed if necessary, and ready for use + * Devices that fail to probe are skipped * * @id: Uclass ID to look up * @devp: Returns pointer to the first device in that uclass, or NULL if none @@ -347,14 +348,15 @@ int uclass_first_device_err(enum uclass_id id, struct udevice **devp); * uclass_next_device() - Get the next device in a uclass * * The device returned is probed if necessary, and ready for use + * Devices that fail to probe are skipped * * This function is useful to iterate through a list of devices which * are functioning correctly and can be probed. * * @devp: On entry, pointer to device to lookup. On exit, returns pointer * to the next device in the uclass if no error occurred, or NULL if there is - * no next device, or an error occurred with that next device. - * Return: 0 if OK (found or not found), other -ve on error + * no next device + * Return: 0 if OK (found or not found) */ int uclass_next_device(struct udevice **devp);

@@ -362,11 +364,12 @@ int uclass_next_device(struct udevice **devp); * uclass_next_device_err() - Get the next device in a uclass * * The device returned is probed if necessary, and ready for use + * Devices that fail to probe are skipped * * @devp: On entry, pointer to device to lookup. On exit, returns pointer * to the next device in the uclass if no error occurred, or NULL if * there is no next device. - * Return: 0 if found, -ENODEV if not found, other -ve on error + * Return: 0 if found, -ENODEV if not found */ int uclass_next_device_err(struct udevice **devp);

diff --git a/test/dm/test-fdt.c b/test/dm/test-fdt.c index 6118ad42ca..165b4f5554 100644 --- a/test/dm/test-fdt.c +++ b/test/dm/test-fdt.c @@ -417,16 +417,28 @@ static int dm_test_first_next_device(struct unit_test_state *uts) pdata = dev_get_plat(dev); pdata->probe_err = -ENOMEM; device_remove(parent, DM_REMOVE_NORMAL); - ut_assertok(uclass_first_device(UCLASS_TEST_PROBE, &dev)); - ut_asserteq(-ENOMEM, uclass_next_device(&dev)); - ut_asserteq_ptr(dev, NULL); + for (ret = uclass_first_device(UCLASS_TEST_PROBE, &dev), count = 0; + dev; + ret = uclass_next_device(&dev)) { + count++; + parent = dev_get_parent(dev); + } + ut_assertok(ret); + ut_asserteq(3, count);

/* Now an error on the first one */ ut_assertok(uclass_get_device(UCLASS_TEST_PROBE, 0, &dev)); pdata = dev_get_plat(dev); pdata->probe_err = -ENOENT; device_remove(parent, DM_REMOVE_NORMAL); - ut_asserteq(-ENOENT, uclass_first_device(UCLASS_TEST_PROBE, &dev)); + for (ret = uclass_first_device(UCLASS_TEST_PROBE, &dev), count = 0; + dev; + ret = uclass_next_device(&dev)) { + count++; + parent = dev_get_parent(dev); + } + ut_assertok(ret); + ut_asserteq(2, count);

return 0; }

pci_find_first_device description says it can be used for iteration with itself but it should really be with pci_find_next_device

Signed-off-by: Michal Suchanek msuchanek@suse.de --- include/pci.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/pci.h b/include/pci.h index d7ed35dd52..c55d6107a4 100644 --- a/include/pci.h +++ b/include/pci.h @@ -957,7 +957,7 @@ int pci_bus_find_devfn(const struct udevice *bus, pci_dev_t find_devfn, /** * pci_find_first_device() - return the first available PCI device * - * This function and pci_find_first_device() allow iteration through all + * This function and pci_find_next_device() allow iteration through all * available PCI devices on all buses. Assuming there are any, this will * return the first one. *

Hi Michal,

On Sun, 25 Sept 2022 at 02:28, Michal Suchanek msuchanek@suse.de wrote:

The description claims that the device is probed but it isn't.

Add the device_probe() call.

Also consolidate the iteration into one function.

Fixes: 8a5cbc065d ("dm: blk: Use uclass_find_first/next_device() in blk_first/next_device()") Signed-off-by: Michal Suchanek msuchanek@suse.de

---

drivers/block/blk-uclass.c | 46 ++++++++++++++++++-------------------- 1 file changed, 22 insertions(+), 24 deletions(-)

diff --git a/drivers/block/blk-uclass.c b/drivers/block/blk-uclass.c index 21c5209bb6..992f8ad3da 100644 --- a/drivers/block/blk-uclass.c +++ b/drivers/block/blk-uclass.c @@ -361,45 +361,43 @@ int blk_dselect_hwpart(struct blk_desc *desc, int hwpart) return blk_select_hwpart(desc->bdev, hwpart); }

-int blk_first_device(int if_type, struct udevice **devp) +static int _blk_next_device(int if_type, struct udevice **devp) { struct blk_desc *desc;

•   int ret;
  

•   int ret = 0;
  

•   for (; *devp; uclass_find_next_device(devp)) {
  

•           desc = dev_get_uclass_plat(*devp);
  

•           if (desc->if_type == if_type) {
  

•                   ret = device_probe(*devp);
  

•                   if (!ret)
  

•                           return 0;
  

•           }
  

•   }
  
  

•   ret = uclass_find_first_device(UCLASS_BLK, devp);
    if (ret)
            return ret;
  

•   if (!*devp)
  

•           return -ENODEV;
  

•   do {
  

•           desc = dev_get_uclass_plat(*devp);
  

•           if (desc->if_type == if_type)
  

•                   return 0;
  

•           ret = uclass_find_next_device(devp);
  

•           if (ret)
  

•                   return ret;
  

•   } while (*devp);
  

This looks wrong since a media device may have other devices under it, e.g. UCLASS_BOOTDEV so I think you should keep the existing code and just call uclass_probe() at the end.

You could add a test for this by checking that only the BLK device is probed.

    return -ENODEV;

}

+int blk_first_device(int if_type, struct udevice **devp) +{

•   uclass_find_first_device(UCLASS_BLK, devp);
  

•   return _blk_next_device(if_type, devp);
  

+}

int blk_next_device(struct udevice **devp) { struct blk_desc *desc;

•   int ret, if_type;
  

•   int if_type;
  
    desc = dev_get_uclass_plat(*devp);
    if_type = desc->if_type;
  

•   do {
  

•           ret = uclass_find_next_device(devp);
  

•           if (ret)
  

•                   return ret;
  

•           if (!*devp)
  

•                   return -ENODEV;
  

•           desc = dev_get_uclass_plat(*devp);
  

•           if (desc->if_type == if_type)
  

•                   return 0;
  

•   } while (1);
  

•   uclass_find_next_device(devp);
  

•   return _blk_next_device(if_type, devp);
  

}

int blk_find_device(int if_type, int devnum, struct udevice **devp)

2.37.3

Regards, Simon

Hi Michal,

On Sun, 2 Oct 2022 at 13:34, Michal Suchánek msuchanek@suse.de wrote:

On Thu, Sep 29, 2022 at 04:00:26AM -0600, Simon Glass wrote:

...

Hi Michal,

On Sun, 25 Sept 2022 at 02:28, Michal Suchanek msuchanek@suse.de wrote:

...

The description claims that the device is probed but it isn't.

Add the device_probe() call.

Also consolidate the iteration into one function.

Fixes: 8a5cbc065d ("dm: blk: Use uclass_find_first/next_device() in blk_first/next_device()") Signed-off-by: Michal Suchanek msuchanek@suse.de

---

drivers/block/blk-uclass.c | 46 ++++++++++++++++++-------------------- 1 file changed, 22 insertions(+), 24 deletions(-)

diff --git a/drivers/block/blk-uclass.c b/drivers/block/blk-uclass.c index 21c5209bb6..992f8ad3da 100644 --- a/drivers/block/blk-uclass.c +++ b/drivers/block/blk-uclass.c @@ -361,45 +361,43 @@ int blk_dselect_hwpart(struct blk_desc *desc, int hwpart) return blk_select_hwpart(desc->bdev, hwpart); }

-int blk_first_device(int if_type, struct udevice **devp) +static int _blk_next_device(int if_type, struct udevice **devp) { struct blk_desc *desc;

•   int ret;
  

•   int ret = 0;
  

•   for (; *devp; uclass_find_next_device(devp)) {
  

•           desc = dev_get_uclass_plat(*devp);
  

•           if (desc->if_type == if_type) {
  

•                   ret = device_probe(*devp);
  

•                   if (!ret)
  

•                           return 0;
  

•           }
  

•   }
  
  

•   ret = uclass_find_first_device(UCLASS_BLK, devp);
    if (ret)
            return ret;
  

•   if (!*devp)
  

•           return -ENODEV;
  

•   do {
  

•           desc = dev_get_uclass_plat(*devp);
  

•           if (desc->if_type == if_type)
  

•                   return 0;
  

•           ret = uclass_find_next_device(devp);
  

•           if (ret)
  

•                   return ret;
  

•   } while (*devp);
  

This looks wrong since a media device may have other devices under it, e.g. UCLASS_BOOTDEV so I think you should keep the existing code and just call uclass_probe() at the end.

You could add a test for this by checking that only the BLK device is probed.

The description says that it returns ready to use device, and that's not possible when the device is only probed at the end when it is to be returned.

Why is that?

There are some tests of this function but very few users so it may be OK to change the semantic again to resemble the _check variant uclass iterator and retorn broken devices but I don't think that was the intent here with using uclass_first_device/uclass_next_device originally.

I agree.

Also this change only makes a difference to the amount of devices probed for callers that only call the blk_first_device and never move on to the next. Callers that use the functions for iteration will move on to the next device and probe it anyway.

OK, perhaps I understand this. But don't you need to update the comment in the header file to say that devices that don't probe are silently skipped?

Also it really does need a test.

Regards, Simon

On Mon, Oct 10, 2022 at 09:49:20PM +0200, Michal Suchánek wrote:

On Sun, Oct 02, 2022 at 07:10:40PM -0600, Simon Glass wrote:

...

Hi Michal,

On Sun, 2 Oct 2022 at 13:34, Michal Suchánek msuchanek@suse.de wrote:

...

On Thu, Sep 29, 2022 at 04:00:26AM -0600, Simon Glass wrote:

...

Hi Michal,

On Sun, 25 Sept 2022 at 02:28, Michal Suchanek msuchanek@suse.de wrote:

...

The description claims that the device is probed but it isn't.

Add the device_probe() call.

Also consolidate the iteration into one function.

Fixes: 8a5cbc065d ("dm: blk: Use uclass_find_first/next_device() in blk_first/next_device()") Signed-off-by: Michal Suchanek msuchanek@suse.de

---

drivers/block/blk-uclass.c | 46 ++++++++++++++++++-------------------- 1 file changed, 22 insertions(+), 24 deletions(-)

diff --git a/drivers/block/blk-uclass.c b/drivers/block/blk-uclass.c index 21c5209bb6..992f8ad3da 100644 --- a/drivers/block/blk-uclass.c +++ b/drivers/block/blk-uclass.c @@ -361,45 +361,43 @@ int blk_dselect_hwpart(struct blk_desc *desc, int hwpart) return blk_select_hwpart(desc->bdev, hwpart); }

-int blk_first_device(int if_type, struct udevice **devp) +static int _blk_next_device(int if_type, struct udevice **devp) { struct blk_desc *desc;

•   int ret;
  

•   int ret = 0;
  

•   for (; *devp; uclass_find_next_device(devp)) {
  

•           desc = dev_get_uclass_plat(*devp);
  

•           if (desc->if_type == if_type) {
  

•                   ret = device_probe(*devp);
  

•                   if (!ret)
  

•                           return 0;
  

•           }
  

•   }
  
  

•   ret = uclass_find_first_device(UCLASS_BLK, devp);
    if (ret)
            return ret;
  

•   if (!*devp)
  

•           return -ENODEV;
  

•   do {
  

•           desc = dev_get_uclass_plat(*devp);
  

•           if (desc->if_type == if_type)
  

•                   return 0;
  

•           ret = uclass_find_next_device(devp);
  

•           if (ret)
  

•                   return ret;
  

•   } while (*devp);
  

This looks wrong since a media device may have other devices under it, e.g. UCLASS_BOOTDEV so I think you should keep the existing code and just call uclass_probe() at the end.

You could add a test for this by checking that only the BLK device is probed.

The description says that it returns ready to use device, and that's not possible when the device is only probed at the end when it is to be returned.

Why is that?

There are two options:

• probe the device, and skip it if it fails, potentially probing multiple devices before returning one
• decide what device to return, probe it, and if it fails return non-activated device

...

...

There are some tests of this function but very few users so it may be OK to change the semantic again to resemble the _check variant uclass iterator and retorn broken devices but I don't think that was the intent here with using uclass_first_device/uclass_next_device originally.

I agree.

...

Also this change only makes a difference to the amount of devices probed for callers that only call the blk_first_device and never move on to the next. Callers that use the functions for iteration will move on to the next device and probe it anyway.

OK, perhaps I understand this. But don't you need to update the comment in the header file to say that devices that don't probe are silently skipped?

They are not ready to use so they cannot be returned by the current description?

...

Also it really does need a test.

Right, tests are good to prevent similar regression in the future.

But we don't have the boilerplate for testing failure in block devices, only in the special probe test class.

Or do we?

Thanks

Michal

On Mon, 10 Oct 2022 at 16:33, Simon Glass sjg@chromium.org wrote:

Hi Michal,

On Mon, 10 Oct 2022 at 15:33, Michal Suchánek msuchanek@suse.de wrote:

...

On Mon, Oct 10, 2022 at 09:49:20PM +0200, Michal Suchánek wrote:

...

On Sun, Oct 02, 2022 at 07:10:40PM -0600, Simon Glass wrote:

...

Hi Michal,

On Sun, 2 Oct 2022 at 13:34, Michal Suchánek msuchanek@suse.de wrote:

...

On Thu, Sep 29, 2022 at 04:00:26AM -0600, Simon Glass wrote:

...

Hi Michal,

On Sun, 25 Sept 2022 at 02:28, Michal Suchanek msuchanek@suse.de wrote: > > The description claims that the device is probed but it isn't. > > Add the device_probe() call. > > Also consolidate the iteration into one function. > > Fixes: 8a5cbc065d ("dm: blk: Use uclass_find_first/next_device() in blk_first/next_device()") > Signed-off-by: Michal Suchanek msuchanek@suse.de > --- > drivers/block/blk-uclass.c | 46 ++++++++++++++++++-------------------- > 1 file changed, 22 insertions(+), 24 deletions(-) > > diff --git a/drivers/block/blk-uclass.c b/drivers/block/blk-uclass.c > index 21c5209bb6..992f8ad3da 100644 > --- a/drivers/block/blk-uclass.c > +++ b/drivers/block/blk-uclass.c > @@ -361,45 +361,43 @@ int blk_dselect_hwpart(struct blk_desc *desc, int hwpart) > return blk_select_hwpart(desc->bdev, hwpart); > } > > -int blk_first_device(int if_type, struct udevice **devp) > +static int _blk_next_device(int if_type, struct udevice **devp) > { > struct blk_desc *desc; > - int ret; > + int ret = 0; > + > + for (; *devp; uclass_find_next_device(devp)) { > + desc = dev_get_uclass_plat(*devp); > + if (desc->if_type == if_type) { > + ret = device_probe(*devp); > + if (!ret) > + return 0; > + } > + } > > - ret = uclass_find_first_device(UCLASS_BLK, devp); > if (ret) > return ret; > - if (!*devp) > - return -ENODEV; > - do { > - desc = dev_get_uclass_plat(*devp); > - if (desc->if_type == if_type) > - return 0; > - ret = uclass_find_next_device(devp); > - if (ret) > - return ret; > - } while (*devp);

This looks wrong since a media device may have other devices under it, e.g. UCLASS_BOOTDEV so I think you should keep the existing code and just call uclass_probe() at the end.

You could add a test for this by checking that only the BLK device is probed.

The description says that it returns ready to use device, and that's not possible when the device is only probed at the end when it is to be returned.

Why is that?

There are two options:

• probe the device, and skip it if it fails, potentially probing multiple devices before returning one
• decide what device to return, probe it, and if it fails return non-activated device

...

...

There are some tests of this function but very few users so it may be OK to change the semantic again to resemble the _check variant uclass iterator and retorn broken devices but I don't think that was the intent here with using uclass_first_device/uclass_next_device originally.

I agree.

...

Also this change only makes a difference to the amount of devices probed for callers that only call the blk_first_device and never move on to the next. Callers that use the functions for iteration will move on to the next device and probe it anyway.

OK, perhaps I understand this. But don't you need to update the comment in the header file to say that devices that don't probe are silently skipped?

They are not ready to use so they cannot be returned by the current description?

...

Also it really does need a test.

Right, tests are good to prevent similar regression in the future.

But we don't have the boilerplate for testing failure in block devices, only in the special probe test class.

Or do we?

Well you can add a new driver and a device associated with it, to test that.

Applied to u-boot-dm (please check)

We already have a function for probing all devices of a specific class, use it.

Signed-off-by: Michal Suchanek msuchanek@suse.de --- drivers/cpu/cpu-uclass.c | 20 ++++---------------- drivers/virtio/virtio-uclass.c | 15 +-------------- test/dm/core.c | 17 +++-------------- test/test-main.c | 11 +---------- 4 files changed, 9 insertions(+), 54 deletions(-)

diff --git a/drivers/cpu/cpu-uclass.c b/drivers/cpu/cpu-uclass.c index 71e5900d70..a754832526 100644 --- a/drivers/cpu/cpu-uclass.c +++ b/drivers/cpu/cpu-uclass.c @@ -20,25 +20,13 @@ DECLARE_GLOBAL_DATA_PTR;

int cpu_probe_all(void) { - struct udevice *cpu; - int ret; + int ret = uclass_probe_all(UCLASS_CPU);

- ret = uclass_first_device(UCLASS_CPU, &cpu); if (ret) { - debug("%s: No CPU found (err = %d)\n", __func__, ret); - return ret; - } - - while (cpu) { - ret = uclass_next_device(&cpu); - if (ret) { - debug("%s: Error while probing CPU (err = %d)\n", - __func__, ret); - return ret; - } + debug("%s: Error while probing CPUs (err = %d %s)\n", + __func__, ret, errno_str(ret)); } - - return 0; + return ret; }

int cpu_is_current(struct udevice *cpu) diff --git a/drivers/virtio/virtio-uclass.c b/drivers/virtio/virtio-uclass.c index 9e2d0e06a1..da4f2f26a6 100644 --- a/drivers/virtio/virtio-uclass.c +++ b/drivers/virtio/virtio-uclass.c @@ -183,21 +183,8 @@ void virtio_driver_features_init(struct virtio_dev_priv *priv,

int virtio_init(void) { - struct udevice *bus; - int ret; - /* Enumerate all known virtio devices */ - ret = uclass_first_device(UCLASS_VIRTIO, &bus); - if (ret) - return ret; - - while (bus) { - ret = uclass_next_device(&bus); - if (ret) - break; - } - - return ret; + return uclass_probe_all(UCLASS_VIRTIO); }

static int virtio_uclass_pre_probe(struct udevice *udev) diff --git a/test/dm/core.c b/test/dm/core.c index fd4d756972..84eb76ed5f 100644 --- a/test/dm/core.c +++ b/test/dm/core.c @@ -512,23 +512,15 @@ static int dm_test_leak(struct unit_test_state *uts) int i;

for (i = 0; i < 2; i++) { - struct udevice *dev; int ret; - int id;

dm_leak_check_start(uts);

ut_assertok(dm_scan_plat(false)); ut_assertok(dm_scan_fdt(false));

- /* Scanning the uclass is enough to probe all the devices */ - for (id = UCLASS_ROOT; id < UCLASS_COUNT; id++) { - for (ret = uclass_first_device(UCLASS_TEST, &dev); - dev; - ret = uclass_next_device(&dev)) - ; - ut_assertok(ret); - } + ret = uclass_probe_all(UCLASS_TEST); + ut_assertok(ret);

ut_assertok(dm_leak_check_end(uts)); } @@ -653,10 +645,7 @@ static int dm_test_children(struct unit_test_state *uts) ut_asserteq(2 + NODE_COUNT, dm_testdrv_op_count[DM_TEST_OP_PROBE]);

/* Probe everything */ - for (ret = uclass_first_device(UCLASS_TEST, &dev); - dev; - ret = uclass_next_device(&dev)) - ; + ret = uclass_probe_all(UCLASS_TEST); ut_assertok(ret);

ut_asserteq(total, dm_testdrv_op_count[DM_TEST_OP_PROBE]); diff --git a/test/test-main.c b/test/test-main.c index 31837e57a8..be0bf9d52c 100644 --- a/test/test-main.c +++ b/test/test-main.c @@ -89,16 +89,7 @@ static int dm_test_post_run(struct unit_test_state *uts) /* Ensure all the test devices are probed */ static int do_autoprobe(struct unit_test_state *uts) { - struct udevice *dev; - int ret; - - /* Scanning the uclass is enough to probe all the devices */ - for (ret = uclass_first_device(UCLASS_TEST, &dev); - dev; - ret = uclass_next_device(&dev)) - ; - - return ret; + return uclass_probe_all(UCLASS_TEST); }

/*

bootdev_list() uses uclass_*_device_err() to iterate devices. However, the only value _err adds is returning an error when the device pointer is null, and that's checked anyway.

Also there is some intent to report errors, and that's what uclass_*_device_check() is for, use it.

Also print the symbolic error.

Signed-off-by: Michal Suchanek msuchanek@suse.de --- boot/bootdev-uclass.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/boot/bootdev-uclass.c b/boot/bootdev-uclass.c index 13ac69eb39..e7115c5750 100644 --- a/boot/bootdev-uclass.c +++ b/boot/bootdev-uclass.c @@ -13,6 +13,7 @@ #include <bootmeth.h> #include <bootstd.h> #include <env.h> +#include <errno.h> #include <fs.h> #include <log.h> #include <malloc.h> @@ -195,16 +196,16 @@ void bootdev_list(bool probe) printf("Seq Probed Status Uclass Name\n"); printf("--- ------ ------ -------- ------------------\n"); if (probe) - ret = uclass_first_device_err(UCLASS_BOOTDEV, &dev); + ret = uclass_first_device_check(UCLASS_BOOTDEV, &dev); else ret = uclass_find_first_device(UCLASS_BOOTDEV, &dev); for (i = 0; dev; i++) { printf("%3x [ %c ] %6s %-9.9s %s\n", dev_seq(dev), device_active(dev) ? '+' : ' ', - ret ? simple_itoa(ret) : "OK", + ret ? errno_str(ret) : "OK", dev_get_uclass_name(dev_get_parent(dev)), dev->name); if (probe) - ret = uclass_next_device_err(&dev); + ret = uclass_next_device_check(&dev); else ret = uclass_find_next_device(&dev); }

The code checks that uclass_first_device returned a device but the returned value that is assigned is never used. Use uclass_first_device_err instead, and move the error return outside of the if block.

Fixes: f4ec1ae08e ("mxc_ipuv3_fb.c: call display_enable") Signed-off-by: Michal Suchanek msuchanek@suse.de --- drivers/video/imx/mxc_ipuv3_fb.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/video/imx/mxc_ipuv3_fb.c b/drivers/video/imx/mxc_ipuv3_fb.c index 49bbeefdd8..8b01a1be11 100644 --- a/drivers/video/imx/mxc_ipuv3_fb.c +++ b/drivers/video/imx/mxc_ipuv3_fb.c @@ -609,12 +609,11 @@ static int ipuv3_video_probe(struct udevice *dev) return ret;

#if defined(CONFIG_DISPLAY) - ret = uclass_first_device(UCLASS_DISPLAY, &disp_dev); - if (disp_dev) { + ret = uclass_first_device_err(UCLASS_DISPLAY, &disp_dev); + if (!ret) ret = display_enable(disp_dev, 16, NULL); - if (ret < 0) - return ret; - } + if (ret < 0) + return ret; #endif if (CONFIG_IS_ENABLED(PANEL)) { struct udevice *panel_dev;

The cycle is guarded by !ret && dev, code in if (ret || ! dev) is never executed. dev implies !ret, and ret is not even checked when getting next device, just drop the ret variable completely.

Signed-off-by: Michal Suchanek msuchanek@suse.de --- drivers/w1/w1-uclass.c | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/w1/w1-uclass.c b/drivers/w1/w1-uclass.c index de4f25bcf9..a4247ecd62 100644 --- a/drivers/w1/w1-uclass.c +++ b/drivers/w1/w1-uclass.c @@ -36,15 +36,10 @@ int w1_bus_find_dev(const struct udevice *bus, u64 id, struct udevice { struct udevice *dev; u8 family = id & 0xff; - int ret;

- for (ret = uclass_first_device(UCLASS_W1_EEPROM, &dev); - !ret && dev; + for (uclass_first_device(UCLASS_W1_EEPROM, &dev); + dev; uclass_next_device(&dev)) { - if (ret || !dev) { - debug("cannot find w1 eeprom dev\n"); - return -ENODEV; - }

if (dev_get_driver_data(dev) == family) { *devp = dev;

The ret value is never used. It is only set when no device is returned, and that is handled before the value of ret would be used.

Signed-off-by: Michal Suchanek msuchanek@suse.de --- drivers/dma/dma-uclass.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/dma/dma-uclass.c b/drivers/dma/dma-uclass.c index 012609bb53..952cbc2170 100644 --- a/drivers/dma/dma-uclass.c +++ b/drivers/dma/dma-uclass.c @@ -209,10 +209,9 @@ int dma_get_cfg(struct dma *dma, u32 cfg_id, void **cfg_data) int dma_get_device(u32 transfer_type, struct udevice **devp) { struct udevice *dev; - int ret;

- for (ret = uclass_first_device(UCLASS_DMA, &dev); dev && !ret; - ret = uclass_next_device(&dev)) { + for (uclass_first_device(UCLASS_DMA, &dev); dev; + uclass_next_device(&dev)) { struct dma_dev_priv *uc_priv;

uc_priv = dev_get_uclass_priv(dev); @@ -228,7 +227,7 @@ int dma_get_device(u32 transfer_type, struct udevice **devp)

*devp = dev;

- return ret; + return 0; }

int dma_memcpy(void *dst, void *src, size_t len)

There are a few commands that iterate uclass with uclass_first_device/uclass_next_device or the _err variant.

Use the _check class iterator variant to get devices that fail to probe as well, and print the status.

Signed-off-by: Michal Suchanek msuchanek@suse.de --- cmd/adc.c | 22 ++++++++++------------ cmd/demo.c | 16 ++++++++++------ cmd/gpio.c | 15 +++++++++++---- cmd/pmic.c | 15 ++++++++------- cmd/regulator.c | 13 +++++++------ 5 files changed, 46 insertions(+), 35 deletions(-)

diff --git a/cmd/adc.c b/cmd/adc.c index 1c5d3e10a3..7dcb44eb61 100644 --- a/cmd/adc.c +++ b/cmd/adc.c @@ -6,29 +6,27 @@ #include <common.h> #include <command.h> #include <dm.h> +#include <errno.h> #include <adc.h>

static int do_adc_list(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) { struct udevice *dev; - int ret; + int ret, err;

- ret = uclass_first_device_err(UCLASS_ADC, &dev); - if (ret) { - printf("No available ADC device\n"); - return CMD_RET_FAILURE; - } + ret = err = uclass_first_device_check(UCLASS_ADC, &dev);

- do { - printf("- %s\n", dev->name); + while (dev) { + printf("- %s status: %s\n", dev->name, + ret ? errno_str(ret) : "OK");

- ret = uclass_next_device(&dev); + ret = uclass_next_device_check(&dev); if (ret) - return CMD_RET_FAILURE; - } while (dev); + err = ret; + }

- return CMD_RET_SUCCESS; + return err ? CMD_RET_FAILURE : CMD_RET_SUCCESS; }

static int do_adc_info(struct cmd_tbl *cmdtp, int flag, int argc, diff --git a/cmd/demo.c b/cmd/demo.c index 571f562ec6..56551f269d 100644 --- a/cmd/demo.c +++ b/cmd/demo.c @@ -10,6 +10,7 @@ #include <command.h> #include <dm.h> #include <dm-demo.h> +#include <errno.h> #include <mapmem.h> #include <asm/io.h>

@@ -64,20 +65,23 @@ static int do_demo_light(struct cmd_tbl *cmdtp, int flag, int argc, int do_demo_list(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) { struct udevice *dev; - int i, ret; + int i, ret, err = 0;

puts("Demo uclass entries:\n");

- for (i = 0, ret = uclass_first_device(UCLASS_DEMO, &dev); + for (i = 0, ret = uclass_first_device_check(UCLASS_DEMO, &dev); dev; - ret = uclass_next_device(&dev)) { - printf("entry %d - instance %08x, ops %08x, plat %08x\n", + ret = uclass_next_device_check(&dev)) { + printf("entry %d - instance %08x, ops %08x, plat %08x, probe %i (%s)\n", i++, (uint)map_to_sysmem(dev), (uint)map_to_sysmem(dev->driver->ops), - (uint)map_to_sysmem(dev_get_plat(dev))); + (uint)map_to_sysmem(dev_get_plat(dev)), + ret, errno_str(ret)); + if (ret) + err = ret; }

- return cmd_process_error(cmdtp, ret); + return cmd_process_error(cmdtp, err); }

static struct cmd_tbl demo_commands[] = { diff --git a/cmd/gpio.c b/cmd/gpio.c index 53e9ce666f..4bf410a9e7 100644 --- a/cmd/gpio.c +++ b/cmd/gpio.c @@ -77,17 +77,24 @@ static int do_gpio_status(bool all, const char *gpio_name) struct udevice *dev; int banklen; int flags; - int ret; + int ret, err = 0;

flags = 0; if (gpio_name && !*gpio_name) gpio_name = NULL; - for (ret = uclass_first_device(UCLASS_GPIO, &dev); + for (ret = uclass_first_device_check(UCLASS_GPIO, &dev); dev; - ret = uclass_next_device(&dev)) { + ret = uclass_next_device_check(&dev)) { const char *bank_name; int num_bits;

+ if (ret) { + printf("GPIO device %s probe error %i (%s)\n", + dev->name, ret, errno_str(ret)); + err = ret; + continue; + } + flags |= FLAG_SHOW_BANK; if (all) flags |= FLAG_SHOW_ALL; @@ -120,7 +127,7 @@ static int do_gpio_status(bool all, const char *gpio_name) flags |= FLAG_SHOW_NEWLINE; }

- return ret; + return err; } #endif

diff --git a/cmd/pmic.c b/cmd/pmic.c index 0cb44d0740..d5624241e6 100644 --- a/cmd/pmic.c +++ b/cmd/pmic.c @@ -51,25 +51,26 @@ static int do_list(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) { struct udevice *dev; - int ret; + int ret, err = 0;

printf("| %-*.*s| %-*.*s| %s @ %s\n", LIMIT_DEV, LIMIT_DEV, "Name", LIMIT_PARENT, LIMIT_PARENT, "Parent name", "Parent uclass", "seq");

- for (ret = uclass_first_device(UCLASS_PMIC, &dev); dev; - ret = uclass_next_device(&dev)) { + for (ret = uclass_first_device_check(UCLASS_PMIC, &dev); dev; + ret = uclass_next_device_check(&dev)) { if (ret) - continue; + err = ret;

- printf("| %-*.*s| %-*.*s| %s @ %d\n", + printf("| %-*.*s| %-*.*s| %s @ %d | probe: %i (%s)\n", LIMIT_DEV, LIMIT_DEV, dev->name, LIMIT_PARENT, LIMIT_PARENT, dev->parent->name, - dev_get_uclass_name(dev->parent), dev_seq(dev->parent)); + dev_get_uclass_name(dev->parent), dev_seq(dev->parent), + ret, errno_str(ret)); }

- if (ret) + if (err) return CMD_RET_FAILURE;

return CMD_RET_SUCCESS; diff --git a/cmd/regulator.c b/cmd/regulator.c index 60a70036d6..129c9fec20 100644 --- a/cmd/regulator.c +++ b/cmd/regulator.c @@ -205,7 +205,7 @@ static void do_status_detail(struct udevice *dev, constraint(" * mode id:", mode, mode_name); }

-static void do_status_line(struct udevice *dev) +static void do_status_line(struct udevice *dev, int status) { struct dm_regulator_uclass_plat *pdata; int current, value, mode; @@ -231,6 +231,7 @@ static void do_status_line(struct udevice *dev) printf("%-10s", mode_name); else printf("%-10s", "-"); + printf(" %s", status ? errno_str(status) : "OK"); printf("\n"); }

@@ -250,11 +251,11 @@ static int do_status(struct cmd_tbl *cmdtp, int flag, int argc, }

/* Show all of them in a list, probing them as needed */ - printf("%-20s %-10s %10s %10s %-10s\n", "Name", "Enabled", "uV", "mA", - "Mode"); - for (ret = uclass_first_device(UCLASS_REGULATOR, &dev); dev; - ret = uclass_next_device(&dev)) - do_status_line(dev); + printf("%-20s %-10s %10s %10s %-10s %s\n", "Name", "Enabled", "uV", "mA", + "Mode", "Status"); + for (ret = uclass_first_device_check(UCLASS_REGULATOR, &dev); dev; + ret = uclass_next_device_check(&dev)) + do_status_line(dev, ret);

return CMD_RET_SUCCESS; }

There are a couple users of uclass_next_device return value that get the first device by other means and use uclass_next_device assuming the following device in the uclass is related to the first one.

Use uclass_next_device_err because the return value from uclass_next_device will be removed in a later patch.

Signed-off-by: Michal Suchanek msuchanek@suse.de --- arch/arm/mach-k3/j721s2_init.c | 2 +- board/atmel/common/mac_eeprom.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-k3/j721s2_init.c b/arch/arm/mach-k3/j721s2_init.c index 12da8136f9..dd0c7ba18f 100644 --- a/arch/arm/mach-k3/j721s2_init.c +++ b/arch/arm/mach-k3/j721s2_init.c @@ -164,7 +164,7 @@ void board_init_f(ulong dummy) if (ret) panic("DRAM 0 init failed: %d\n", ret);

- ret = uclass_next_device(&dev); + ret = uclass_next_device_err(&dev); if (ret) panic("DRAM 1 init failed: %d\n", ret); } diff --git a/board/atmel/common/mac_eeprom.c b/board/atmel/common/mac_eeprom.c index a723ba723c..4606008c69 100644 --- a/board/atmel/common/mac_eeprom.c +++ b/board/atmel/common/mac_eeprom.c @@ -56,7 +56,7 @@ int at91_set_eth1addr(int offset) return ret;

/* attempt to obtain a second eeprom device */ - ret = uclass_next_device(&dev); + ret = uclass_next_device_err(&dev); if (ret) return ret;

uclass_first_device/uclass_next_device return value will be removed, don't use it.

Signed-off-by: Michal Suchanek msuchanek@suse.de --- cmd/virtio.c | 9 +++------ drivers/gpio/gpio-uclass.c | 14 ++++++-------- 2 files changed, 9 insertions(+), 14 deletions(-)

diff --git a/cmd/virtio.c b/cmd/virtio.c index ea3ed2e631..f7a2ef4313 100644 --- a/cmd/virtio.c +++ b/cmd/virtio.c @@ -23,18 +23,15 @@ static int do_virtio(struct cmd_tbl *cmdtp, int flag, int argc, * device_probe() for children (i.e. virtio devices) */ struct udevice *bus, *child; - int ret;

- ret = uclass_first_device(UCLASS_VIRTIO, &bus); - if (ret) + uclass_first_device(UCLASS_VIRTIO, &bus); + if (!bus) return CMD_RET_FAILURE;

while (bus) { device_foreach_child_probe(child, bus) ; - ret = uclass_next_device(&bus); - if (ret) - break; + uclass_next_device(&bus); }

return CMD_RET_SUCCESS; diff --git a/drivers/gpio/gpio-uclass.c b/drivers/gpio/gpio-uclass.c index 0ed32b7217..3a35f1e30a 100644 --- a/drivers/gpio/gpio-uclass.c +++ b/drivers/gpio/gpio-uclass.c @@ -59,11 +59,10 @@ static int gpio_to_device(unsigned int gpio, struct gpio_desc *desc) { struct gpio_dev_priv *uc_priv; struct udevice *dev; - int ret;

- for (ret = uclass_first_device(UCLASS_GPIO, &dev); + for (uclass_first_device(UCLASS_GPIO, &dev); dev; - ret = uclass_next_device(&dev)) { + uclass_next_device(&dev)) { uc_priv = dev_get_uclass_priv(dev); if (gpio >= uc_priv->gpio_base && gpio < uc_priv->gpio_base + uc_priv->gpio_count) { @@ -73,7 +72,7 @@ static int gpio_to_device(unsigned int gpio, struct gpio_desc *desc) }

/* No such GPIO */ - return ret ? ret : -ENOENT; + return -ENOENT; }

#if CONFIG_IS_ENABLED(DM_GPIO_LOOKUP_LABEL) @@ -121,12 +120,11 @@ int dm_gpio_lookup_name(const char *name, struct gpio_desc *desc) struct udevice *dev; ulong offset; int numeric; - int ret;

numeric = isdigit(*name) ? dectoul(name, NULL) : -1; - for (ret = uclass_first_device(UCLASS_GPIO, &dev); + for (uclass_first_device(UCLASS_GPIO, &dev); dev; - ret = uclass_next_device(&dev)) { + uclass_next_device(&dev)) { int len;

uc_priv = dev_get_uclass_priv(dev); @@ -154,7 +152,7 @@ int dm_gpio_lookup_name(const char *name, struct gpio_desc *desc) }

if (!dev) - return ret ? ret : -EINVAL; + return -EINVAL;

gpio_desc_init(desc, dev, offset);

Clarify documentation, fix a few more cases that could be broken by the change.

Signed-off-by: Michal Suchanek msuchanek@suse.de --- drivers/pci/pci-uclass.c | 7 +++---- drivers/sysinfo/sysinfo-uclass.c | 10 +++++++++- include/dm/uclass.h | 15 +++++++++------ 3 files changed, 21 insertions(+), 11 deletions(-)

diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c index 00e3828d95..2aa1043604 100644 --- a/drivers/pci/pci-uclass.c +++ b/drivers/pci/pci-uclass.c @@ -1768,10 +1768,9 @@ int pci_sriov_init(struct udevice *pdev, int vf_en)

bdf = dm_pci_get_bdf(pdev);

- pci_get_bus(PCI_BUS(bdf), &bus); - - if (!bus) - return -ENODEV; + ret = pci_get_bus(PCI_BUS(bdf), &bus); + if (ret) + return ret;

bdf += PCI_BDF(0, 0, vf_offset);

diff --git a/drivers/sysinfo/sysinfo-uclass.c b/drivers/sysinfo/sysinfo-uclass.c index c5cc3cb959..10194d0e14 100644 --- a/drivers/sysinfo/sysinfo-uclass.c +++ b/drivers/sysinfo/sysinfo-uclass.c @@ -16,7 +16,15 @@ struct sysinfo_priv {

int sysinfo_get(struct udevice **devp) { - return uclass_first_device_err(UCLASS_SYSINFO, devp); + int ret = uclass_first_device_err(UCLASS_SYSINFO, devp); + + /* + * There is some very dodgy error handling in gazerbeam, + * do not return a device on error. + */ + if (ret) + *devp = NULL; + return ret; }

int sysinfo_detect(struct udevice *dev) diff --git a/include/dm/uclass.h b/include/dm/uclass.h index 40df0b34b1..888adbd0cb 100644 --- a/include/dm/uclass.h +++ b/include/dm/uclass.h @@ -349,7 +349,7 @@ void uclass_next_device(struct udevice **devp); /** * uclass_first_device_err() - Get the first device in a uclass * - * The device returned is probed if necessary, and ready for use + * The device returned is probed if necessary, and ready for use if no error is returned * * @id: Uclass ID to look up * @devp: Returns pointer to the first device in that uclass, or NULL if none @@ -360,7 +360,8 @@ int uclass_first_device_err(enum uclass_id id, struct udevice **devp); /** * uclass_next_device_err() - Get the next device in a uclass * - * The device returned is probed if necessary, and ready for use + * The device returned is probed if necessary, and ready for use if no error is + * returned * * @devp: On entry, pointer to device to lookup. On exit, returns pointer * to the next device in the uclass if no error occurred, or NULL if @@ -372,7 +373,8 @@ int uclass_next_device_err(struct udevice **devp); /** * uclass_first_device_check() - Get the first device in a uclass * - * The device returned is probed if necessary, and ready for use + * The device returned is probed if necessary, and ready for use if no error is + * returned * * This function is useful to start iterating through a list of devices which * are functioning correctly and can be probed. @@ -388,7 +390,8 @@ int uclass_first_device_check(enum uclass_id id, struct udevice **devp); /** * uclass_next_device_check() - Get the next device in a uclass * - * The device returned is probed if necessary, and ready for use + * The device returned is probed if necessary, and ready for use if no error is + * returned * * This function is useful to start iterating through a list of devices which * are functioning correctly and can be probed. @@ -490,7 +493,7 @@ int uclass_id_count(enum uclass_id id); * are no more devices. */ #define uclass_foreach_dev_probe(id, dev) \ - for (int _ret = uclass_first_device_err(id, &dev); !_ret && dev; \ - _ret = uclass_next_device_err(&dev)) + for (uclass_first_device(id, &dev); dev; \ + uclass_next_device(&dev))

#endif

When probing a device fails NULL pointer is returned, and following devices in uclass list cannot be iterated. Skip to next device on error instead.

With that the only condition under which these simple iteration functions return error is when the dm is not initialized at uclass_get time. This is not all that interesting, change return type to void.

Fixes: 6494d708bf ("dm: Add base driver model support") Signed-off-by: Michal Suchanek msuchanek@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 v4: - change return value to void - further simplify iteration --- drivers/core/uclass.c | 30 ++++++++++++++++++------------ include/dm/uclass.h | 13 ++++++------- test/dm/core.c | 10 ++++------ test/dm/test-fdt.c | 27 ++++++++++++++++++++------- 4 files changed, 48 insertions(+), 32 deletions(-)

diff --git a/drivers/core/uclass.c b/drivers/core/uclass.c index b7d11bdd23..6dec6a3973 100644 --- a/drivers/core/uclass.c +++ b/drivers/core/uclass.c @@ -574,28 +574,34 @@ int uclass_get_device_by_phandle(enum uclass_id id, struct udevice *parent, } #endif

-int uclass_first_device(enum uclass_id id, struct udevice **devp) +/* + * Starting from the given device return first device in the uclass that + * probes successfully. + */ +static void _uclass_next_device(struct udevice *dev, struct udevice **devp) +{ + for (; dev; uclass_find_next_device(&dev)) { + if (!device_probe(dev)) + break; + } + *devp = dev; +} + +void uclass_first_device(enum uclass_id id, struct udevice **devp) { struct udevice *dev; int ret;

- *devp = NULL; ret = uclass_find_first_device(id, &dev); - if (!dev) - return 0; - return uclass_get_device_tail(dev, ret, devp); + _uclass_next_device(dev, devp); }

-int uclass_next_device(struct udevice **devp) +void 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); + uclass_find_next_device(&dev); + _uclass_next_device(dev, devp); }

int uclass_first_device_err(enum uclass_id id, struct udevice **devp) diff --git a/include/dm/uclass.h b/include/dm/uclass.h index 5e9da2b106..40df0b34b1 100644 --- a/include/dm/uclass.h +++ b/include/dm/uclass.h @@ -320,32 +320,31 @@ int uclass_get_device_by_driver(enum uclass_id id, const struct driver *drv, * uclass_first_device() - Get the first device in a uclass * * The device returned is probed if necessary, and ready for use + * Devices that fail to probe are skipped * * This function is useful to start iterating through a list of devices which * are functioning correctly and can be probed. * * @id: Uclass ID to look up * @devp: Returns pointer to the first device in that uclass if no error - * occurred, or NULL if there is no first device, or an error occurred with - * that device. - * Return: 0 if OK (found or not found), other -ve on error + * occurred, or NULL if there is no usable device */ -int uclass_first_device(enum uclass_id id, struct udevice **devp); +void uclass_first_device(enum uclass_id id, struct udevice **devp);

/** * uclass_next_device() - Get the next device in a uclass * * The device returned is probed if necessary, and ready for use + * Devices that fail to probe are skipped * * This function is useful to iterate through a list of devices which * are functioning correctly and can be probed. * * @devp: On entry, pointer to device to lookup. On exit, returns pointer * to the next device in the uclass if no error occurred, or NULL if there is - * no next device, or an error occurred with that next device. - * Return: 0 if OK (found or not found), other -ve on error + * no next device */ -int uclass_next_device(struct udevice **devp); +void uclass_next_device(struct udevice **devp);

/** * uclass_first_device_err() - Get the first device in a uclass diff --git a/test/dm/core.c b/test/dm/core.c index 84eb76ed5f..7f3f8d183b 100644 --- a/test/dm/core.c +++ b/test/dm/core.c @@ -1078,11 +1078,10 @@ static int dm_test_uclass_devices_get(struct unit_test_state *uts) struct udevice *dev; int ret;

- for (ret = uclass_first_device(UCLASS_TEST, &dev); + for (ret = uclass_first_device_check(UCLASS_TEST, &dev); dev; - ret = uclass_next_device(&dev)) { + ret = uclass_next_device_check(&dev)) { ut_assert(!ret); - ut_assert(dev); ut_assert(device_active(dev)); }

@@ -1112,11 +1111,10 @@ static int dm_test_uclass_devices_get_by_name(struct unit_test_state *uts) * this will fail on checking condition: testdev == finddev, since the * uclass_get_device_by_name(), returns the first device by given name. */ - for (ret = uclass_first_device(UCLASS_TEST_FDT, &testdev); + for (ret = uclass_first_device_check(UCLASS_TEST_FDT, &testdev); testdev; - ret = uclass_next_device(&testdev)) { + ret = uclass_next_device_check(&testdev)) { ut_assertok(ret); - ut_assert(testdev); ut_assert(device_active(testdev));

findret = uclass_get_device_by_name(UCLASS_TEST_FDT, diff --git a/test/dm/test-fdt.c b/test/dm/test-fdt.c index 6118ad42ca..7cbc595c51 100644 --- a/test/dm/test-fdt.c +++ b/test/dm/test-fdt.c @@ -403,13 +403,12 @@ static int dm_test_first_next_device(struct unit_test_state *uts) int ret;

/* There should be 4 devices */ - for (ret = uclass_first_device(UCLASS_TEST_PROBE, &dev), count = 0; + for (uclass_first_device(UCLASS_TEST_PROBE, &dev), count = 0; dev; - ret = uclass_next_device(&dev)) { + uclass_next_device(&dev)) { count++; parent = dev_get_parent(dev); } - ut_assertok(ret); ut_asserteq(4, count);

/* Remove them and try again, with an error on the second one */ @@ -417,16 +416,30 @@ static int dm_test_first_next_device(struct unit_test_state *uts) pdata = dev_get_plat(dev); pdata->probe_err = -ENOMEM; device_remove(parent, DM_REMOVE_NORMAL); - ut_assertok(uclass_first_device(UCLASS_TEST_PROBE, &dev)); - ut_asserteq(-ENOMEM, uclass_next_device(&dev)); - ut_asserteq_ptr(dev, NULL); + for (ret = uclass_first_device_check(UCLASS_TEST_PROBE, &dev), + count = 0; + dev; + ret = uclass_next_device_check(&dev)) { + if (!ret) + count++; + else + ut_asserteq(-ENOMEM, ret); + parent = dev_get_parent(dev); + } + ut_asserteq(3, count);

/* Now an error on the first one */ ut_assertok(uclass_get_device(UCLASS_TEST_PROBE, 0, &dev)); pdata = dev_get_plat(dev); pdata->probe_err = -ENOENT; device_remove(parent, DM_REMOVE_NORMAL); - ut_asserteq(-ENOENT, uclass_first_device(UCLASS_TEST_PROBE, &dev)); + for (uclass_first_device(UCLASS_TEST_PROBE, &dev), count = 0; + dev; + uclass_next_device(&dev)) { + count++; + parent = dev_get_parent(dev); + } + ut_asserteq(2, count);

return 0; }

uclass_probe_all uses uclass_first_device/uclass_next_device assigning the return value.

The interface for getting meaningful error is uclass_first_device_check/uclass_next_device_check, use it.

Also do not stop iteration when an error is encountered. Probing all devices includes those that happen to be after a failing device in the uclass order.

Fixes: a59153dfeb ("dm: core: add function uclass_probe_all() to probe all devices") Signed-off-by: Michal Suchanek msuchanek@suse.de --- drivers/core/uclass.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/core/uclass.c b/drivers/core/uclass.c index 08d9ed82de..a591e22403 100644 --- a/drivers/core/uclass.c +++ b/drivers/core/uclass.c @@ -799,20 +799,18 @@ int uclass_pre_remove_device(struct udevice *dev) int uclass_probe_all(enum uclass_id id) { struct udevice *dev; - int ret; + int ret, err;

- ret = uclass_first_device(id, &dev); - if (ret || !dev) - return ret; + err = uclass_first_device_check(id, &dev);

/* Scanning uclass to probe all devices */ while (dev) { - ret = uclass_next_device(&dev); + ret = uclass_next_device_check(&dev); if (ret) - return ret; + err = ret; }

- return 0; + return err; }

int uclass_id_count(enum uclass_id id)

We already have a function for probing all devices of a specific class, use it.

Signed-off-by: Michal Suchanek msuchanek@suse.de --- drivers/cpu/cpu-uclass.c | 20 ++++---------------- drivers/virtio/virtio-uclass.c | 15 +-------------- test/dm/core.c | 17 +++-------------- test/test-main.c | 11 +---------- 4 files changed, 9 insertions(+), 54 deletions(-)

diff --git a/drivers/cpu/cpu-uclass.c b/drivers/cpu/cpu-uclass.c index 71e5900d70..a754832526 100644 --- a/drivers/cpu/cpu-uclass.c +++ b/drivers/cpu/cpu-uclass.c @@ -20,25 +20,13 @@ DECLARE_GLOBAL_DATA_PTR;

int cpu_probe_all(void) { - struct udevice *cpu; - int ret; + int ret = uclass_probe_all(UCLASS_CPU);

- ret = uclass_first_device(UCLASS_CPU, &cpu); if (ret) { - debug("%s: No CPU found (err = %d)\n", __func__, ret); - return ret; - } - - while (cpu) { - ret = uclass_next_device(&cpu); - if (ret) { - debug("%s: Error while probing CPU (err = %d)\n", - __func__, ret); - return ret; - } + debug("%s: Error while probing CPUs (err = %d %s)\n", + __func__, ret, errno_str(ret)); } - - return 0; + return ret; }

int cpu_is_current(struct udevice *cpu) diff --git a/drivers/virtio/virtio-uclass.c b/drivers/virtio/virtio-uclass.c index 9e2d0e06a1..da4f2f26a6 100644 --- a/drivers/virtio/virtio-uclass.c +++ b/drivers/virtio/virtio-uclass.c @@ -183,21 +183,8 @@ void virtio_driver_features_init(struct virtio_dev_priv *priv,

int virtio_init(void) { - struct udevice *bus; - int ret; - /* Enumerate all known virtio devices */ - ret = uclass_first_device(UCLASS_VIRTIO, &bus); - if (ret) - return ret; - - while (bus) { - ret = uclass_next_device(&bus); - if (ret) - break; - } - - return ret; + return uclass_probe_all(UCLASS_VIRTIO); }

static int virtio_uclass_pre_probe(struct udevice *udev) diff --git a/test/dm/core.c b/test/dm/core.c index fd4d756972..84eb76ed5f 100644 --- a/test/dm/core.c +++ b/test/dm/core.c @@ -512,23 +512,15 @@ static int dm_test_leak(struct unit_test_state *uts) int i;

for (i = 0; i < 2; i++) { - struct udevice *dev; int ret; - int id;

dm_leak_check_start(uts);

ut_assertok(dm_scan_plat(false)); ut_assertok(dm_scan_fdt(false));

- /* Scanning the uclass is enough to probe all the devices */ - for (id = UCLASS_ROOT; id < UCLASS_COUNT; id++) { - for (ret = uclass_first_device(UCLASS_TEST, &dev); - dev; - ret = uclass_next_device(&dev)) - ; - ut_assertok(ret); - } + ret = uclass_probe_all(UCLASS_TEST); + ut_assertok(ret);

ut_assertok(dm_leak_check_end(uts)); } @@ -653,10 +645,7 @@ static int dm_test_children(struct unit_test_state *uts) ut_asserteq(2 + NODE_COUNT, dm_testdrv_op_count[DM_TEST_OP_PROBE]);

/* Probe everything */ - for (ret = uclass_first_device(UCLASS_TEST, &dev); - dev; - ret = uclass_next_device(&dev)) - ; + ret = uclass_probe_all(UCLASS_TEST); ut_assertok(ret);

ut_asserteq(total, dm_testdrv_op_count[DM_TEST_OP_PROBE]); diff --git a/test/test-main.c b/test/test-main.c index 31837e57a8..be0bf9d52c 100644 --- a/test/test-main.c +++ b/test/test-main.c @@ -89,16 +89,7 @@ static int dm_test_post_run(struct unit_test_state *uts) /* Ensure all the test devices are probed */ static int do_autoprobe(struct unit_test_state *uts) { - struct udevice *dev; - int ret; - - /* Scanning the uclass is enough to probe all the devices */ - for (ret = uclass_first_device(UCLASS_TEST, &dev); - dev; - ret = uclass_next_device(&dev)) - ; - - return ret; + return uclass_probe_all(UCLASS_TEST); }

/*

When there is no PCI bus uclass_first_device will return no bus and no error which will result in pci_find_first_device calling skip_to_next_device with no bus, and the bus is only checked at the end of the while cycle, not the beginning.

Also stop dealing with the return value of uclass_first_device/uclass_next_device - once the iteration is fixed to continue after an error there is nothing meaningful to get anymore.

Fixes: 76c3fbcd3d ("dm: pci: Add a way to iterate through all PCI devices") Signed-off-by: Michal Suchanek msuchanek@suse.de --- drivers/pci/pci-uclass.c | 19 +++++-------------- 1 file changed, 5 insertions(+), 14 deletions(-)

diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c index 16a6a699f9..00e3828d95 100644 --- a/drivers/pci/pci-uclass.c +++ b/drivers/pci/pci-uclass.c @@ -1211,22 +1211,19 @@ static int pci_bridge_write_config(struct udevice *bus, pci_dev_t bdf, static int skip_to_next_device(struct udevice *bus, struct udevice **devp) { struct udevice *dev; - int ret = 0;

/* * Scan through all the PCI controllers. On x86 there will only be one * but that is not necessarily true on other hardware. */ - do { + while (bus) { device_find_first_child(bus, &dev); if (dev) { *devp = dev; return 0; } - ret = uclass_next_device(&bus); - if (ret) - return ret; - } while (bus); + uclass_next_device(&bus); + }

return 0; } @@ -1235,7 +1232,6 @@ int pci_find_next_device(struct udevice **devp) { struct udevice *child = *devp; struct udevice *bus = child->parent; - int ret;

/* First try all the siblings */ *devp = NULL; @@ -1248,9 +1244,7 @@ int pci_find_next_device(struct udevice **devp) }

/* We ran out of siblings. Try the next bus */ - ret = uclass_next_device(&bus); - if (ret) - return ret; + uclass_next_device(&bus);

return bus ? skip_to_next_device(bus, devp) : 0; } @@ -1258,12 +1252,9 @@ int pci_find_next_device(struct udevice **devp) int pci_find_first_device(struct udevice **devp) { struct udevice *bus; - int ret;

*devp = NULL; - ret = uclass_first_device(UCLASS_PCI, &bus); - if (ret) - return ret; + uclass_first_device(UCLASS_PCI, &bus);

return skip_to_next_device(bus, devp); }

bootdev_list() uses uclass_*_device_err() to iterate devices. However, the only value _err adds is returning an error when the device pointer is null, and that's checked anyway.

Also there is some intent to report errors, and that's what uclass_*_device_check() is for, use it.

Also print the symbolic error.

Signed-off-by: Michal Suchanek msuchanek@suse.de --- boot/bootdev-uclass.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/boot/bootdev-uclass.c b/boot/bootdev-uclass.c index 13ac69eb39..e7115c5750 100644 --- a/boot/bootdev-uclass.c +++ b/boot/bootdev-uclass.c @@ -13,6 +13,7 @@ #include <bootmeth.h> #include <bootstd.h> #include <env.h> +#include <errno.h> #include <fs.h> #include <log.h> #include <malloc.h> @@ -195,16 +196,16 @@ void bootdev_list(bool probe) printf("Seq Probed Status Uclass Name\n"); printf("--- ------ ------ -------- ------------------\n"); if (probe) - ret = uclass_first_device_err(UCLASS_BOOTDEV, &dev); + ret = uclass_first_device_check(UCLASS_BOOTDEV, &dev); else ret = uclass_find_first_device(UCLASS_BOOTDEV, &dev); for (i = 0; dev; i++) { printf("%3x [ %c ] %6s %-9.9s %s\n", dev_seq(dev), device_active(dev) ? '+' : ' ', - ret ? simple_itoa(ret) : "OK", + ret ? errno_str(ret) : "OK", dev_get_uclass_name(dev_get_parent(dev)), dev->name); if (probe) - ret = uclass_next_device_err(&dev); + ret = uclass_next_device_check(&dev); else ret = uclass_find_next_device(&dev); }

There is a complaint in the code that iterates keyboards that we don't have the _check variant of class iterator but we in fact do, use it.

In the code that iterates video devices there is an attempt to print errors but the simple iterator does not return a device when there is an error. Use the _check variant of the iterator as well.

Also print the symbolic error.

Fixes: b206cd7372 ("dm: stdio: Plumb in the new keyboard uclass") Fixes: e3b81c1c0d ("dm: stdio: video: Plumb the video uclass into stdio") Signed-off-by: Michal Suchanek msuchanek@suse.de --- common/stdio.c | 33 ++++++++++++++------------------- 1 file changed, 14 insertions(+), 19 deletions(-)

diff --git a/common/stdio.c b/common/stdio.c index 92161a0df8..17f0aef76d 100644 --- a/common/stdio.c +++ b/common/stdio.c @@ -306,7 +306,6 @@ int stdio_init_tables(void) int stdio_add_devices(void) { struct udevice *dev; - struct uclass *uc; int ret;

if (IS_ENABLED(CONFIG_DM_KEYBOARD)) { @@ -316,24 +315,18 @@ int stdio_add_devices(void) * have a list of input devices to start up in the stdin * environment variable. That work probably makes more sense * when stdio itself is converted to driver model. - * - * TODO(sjg@chromium.org): Convert changing - * uclass_first_device() etc. to return the device even on - * error. Then we could use that here. */ - ret = uclass_get(UCLASS_KEYBOARD, &uc); - if (ret) - return ret;

/* * Don't report errors to the caller - assume that they are * non-fatal */ - uclass_foreach_dev(dev, uc) { - ret = device_probe(dev); + for (ret = uclass_first_device_check(UCLASS_KEYBOARD, &dev); + dev; + ret = uclass_next_device_check(&dev)) { if (ret) - printf("Failed to probe keyboard '%s'\n", - dev->name); + printf("%s: Failed to probe keyboard '%s' (ret=%d %s)\n", + __func__, dev->name, ret, errno_str(ret)); } } #if CONFIG_IS_ENABLED(SYS_I2C_LEGACY) @@ -353,13 +346,15 @@ int stdio_add_devices(void) int ret;

if (!IS_ENABLED(CONFIG_SYS_CONSOLE_IS_IN_ENV)) { - for (ret = uclass_first_device(UCLASS_VIDEO, &vdev); - vdev; - ret = uclass_next_device(&vdev)) - ; - if (ret) - printf("%s: Video device failed (ret=%d)\n", - __func__, ret); + for (ret = uclass_first_device_check(UCLASS_VIDEO, + &vdev); + vdev; + ret = uclass_next_device_check(&vdev)) { + if (ret) + printf("%s: Failed to probe video device '%s' (ret=%d %s)\n", + __func__, dev->name, + ret, errno_str(ret)); + } } if (IS_ENABLED(CONFIG_SPLASH_SCREEN) && IS_ENABLED(CONFIG_CMD_BMP))

The code checks that uclass_first_device returned a device but the returned value that is assigned is never used. Use uclass_first_device_err instead, and move the error return outside of the if block.

Fixes: f4ec1ae08e ("mxc_ipuv3_fb.c: call display_enable") Signed-off-by: Michal Suchanek msuchanek@suse.de --- drivers/video/imx/mxc_ipuv3_fb.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/video/imx/mxc_ipuv3_fb.c b/drivers/video/imx/mxc_ipuv3_fb.c index 49bbeefdd8..8b01a1be11 100644 --- a/drivers/video/imx/mxc_ipuv3_fb.c +++ b/drivers/video/imx/mxc_ipuv3_fb.c @@ -609,12 +609,11 @@ static int ipuv3_video_probe(struct udevice *dev) return ret;

#if defined(CONFIG_DISPLAY) - ret = uclass_first_device(UCLASS_DISPLAY, &disp_dev); - if (disp_dev) { + ret = uclass_first_device_err(UCLASS_DISPLAY, &disp_dev); + if (!ret) ret = display_enable(disp_dev, 16, NULL); - if (ret < 0) - return ret; - } + if (ret < 0) + return ret; #endif if (CONFIG_IS_ENABLED(PANEL)) { struct udevice *panel_dev;

Use uclass_first_device_check/uclass_next_device_check to correctly count buses that fail to probe.

Fixes: d3e19cf919 ("w1: Add 1-Wire uclass") Signed-off-by: Michal Suchanek msuchanek@suse.de --- drivers/w1/w1-uclass.c | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/drivers/w1/w1-uclass.c b/drivers/w1/w1-uclass.c index 52b519c21d..de4f25bcf9 100644 --- a/drivers/w1/w1-uclass.c +++ b/drivers/w1/w1-uclass.c @@ -16,6 +16,7 @@

#include <common.h> #include <dm.h> +#include <errno.h> #include <log.h> #include <w1.h> #include <w1-eeprom.h> @@ -182,24 +183,25 @@ static int w1_enumerate(struct udevice *bus) int w1_get_bus(int busnum, struct udevice **busp) { int ret, i = 0; - struct udevice *dev;

- for (ret = uclass_first_device(UCLASS_W1, &dev); - dev && !ret; - ret = uclass_next_device(&dev), i++) { + for (ret = uclass_first_device_check(UCLASS_W1, &dev); + dev; + ret = uclass_next_device_check(&dev), i++) { if (i == busnum) { + if (ret) { + debug("Cannot probe w1 bus %d: %d (%s)\n", + busnum, ret, errno_str(ret)); + return ret; + } *busp = dev; return 0; } }

- if (!ret) { - debug("Cannot find w1 bus %d\n", busnum); - ret = -ENODEV; - } + debug("Cannot find w1 bus %d\n", busnum);

- return ret; + return -ENODEV; }

u8 w1_get_device_family(struct udevice *dev)

There are a few commands that iterate uclass with uclass_first_device/uclass_next_device or the _err variant.

Use the _check class iterator variant to get devices that fail to probe as well, and print the status.

Signed-off-by: Michal Suchanek msuchanek@suse.de --- cmd/adc.c | 22 ++++++++++------------ cmd/demo.c | 16 ++++++++++------ cmd/gpio.c | 15 +++++++++++---- cmd/pmic.c | 15 ++++++++------- cmd/regulator.c | 13 +++++++------ 5 files changed, 46 insertions(+), 35 deletions(-)

diff --git a/cmd/adc.c b/cmd/adc.c index 1c5d3e10a3..7dcb44eb61 100644 --- a/cmd/adc.c +++ b/cmd/adc.c @@ -6,29 +6,27 @@ #include <common.h> #include <command.h> #include <dm.h> +#include <errno.h> #include <adc.h>

static int do_adc_list(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) { struct udevice *dev; - int ret; + int ret, err;

- ret = uclass_first_device_err(UCLASS_ADC, &dev); - if (ret) { - printf("No available ADC device\n"); - return CMD_RET_FAILURE; - } + ret = err = uclass_first_device_check(UCLASS_ADC, &dev);

- do { - printf("- %s\n", dev->name); + while (dev) { + printf("- %s status: %s\n", dev->name, + ret ? errno_str(ret) : "OK");

- ret = uclass_next_device(&dev); + ret = uclass_next_device_check(&dev); if (ret) - return CMD_RET_FAILURE; - } while (dev); + err = ret; + }

- return CMD_RET_SUCCESS; + return err ? CMD_RET_FAILURE : CMD_RET_SUCCESS; }

static int do_adc_info(struct cmd_tbl *cmdtp, int flag, int argc, diff --git a/cmd/demo.c b/cmd/demo.c index 571f562ec6..56551f269d 100644 --- a/cmd/demo.c +++ b/cmd/demo.c @@ -10,6 +10,7 @@ #include <command.h> #include <dm.h> #include <dm-demo.h> +#include <errno.h> #include <mapmem.h> #include <asm/io.h>

@@ -64,20 +65,23 @@ static int do_demo_light(struct cmd_tbl *cmdtp, int flag, int argc, int do_demo_list(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) { struct udevice *dev; - int i, ret; + int i, ret, err = 0;

puts("Demo uclass entries:\n");

- for (i = 0, ret = uclass_first_device(UCLASS_DEMO, &dev); + for (i = 0, ret = uclass_first_device_check(UCLASS_DEMO, &dev); dev; - ret = uclass_next_device(&dev)) { - printf("entry %d - instance %08x, ops %08x, plat %08x\n", + ret = uclass_next_device_check(&dev)) { + printf("entry %d - instance %08x, ops %08x, plat %08x, probe %i (%s)\n", i++, (uint)map_to_sysmem(dev), (uint)map_to_sysmem(dev->driver->ops), - (uint)map_to_sysmem(dev_get_plat(dev))); + (uint)map_to_sysmem(dev_get_plat(dev)), + ret, errno_str(ret)); + if (ret) + err = ret; }

- return cmd_process_error(cmdtp, ret); + return cmd_process_error(cmdtp, err); }

static struct cmd_tbl demo_commands[] = { diff --git a/cmd/gpio.c b/cmd/gpio.c index 53e9ce666f..4bf410a9e7 100644 --- a/cmd/gpio.c +++ b/cmd/gpio.c @@ -77,17 +77,24 @@ static int do_gpio_status(bool all, const char *gpio_name) struct udevice *dev; int banklen; int flags; - int ret; + int ret, err = 0;

flags = 0; if (gpio_name && !*gpio_name) gpio_name = NULL; - for (ret = uclass_first_device(UCLASS_GPIO, &dev); + for (ret = uclass_first_device_check(UCLASS_GPIO, &dev); dev; - ret = uclass_next_device(&dev)) { + ret = uclass_next_device_check(&dev)) { const char *bank_name; int num_bits;

+ if (ret) { + printf("GPIO device %s probe error %i (%s)\n", + dev->name, ret, errno_str(ret)); + err = ret; + continue; + } + flags |= FLAG_SHOW_BANK; if (all) flags |= FLAG_SHOW_ALL; @@ -120,7 +127,7 @@ static int do_gpio_status(bool all, const char *gpio_name) flags |= FLAG_SHOW_NEWLINE; }

- return ret; + return err; } #endif

diff --git a/cmd/pmic.c b/cmd/pmic.c index 0cb44d0740..d5624241e6 100644 --- a/cmd/pmic.c +++ b/cmd/pmic.c @@ -51,25 +51,26 @@ static int do_list(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) { struct udevice *dev; - int ret; + int ret, err = 0;

printf("| %-*.*s| %-*.*s| %s @ %s\n", LIMIT_DEV, LIMIT_DEV, "Name", LIMIT_PARENT, LIMIT_PARENT, "Parent name", "Parent uclass", "seq");

- for (ret = uclass_first_device(UCLASS_PMIC, &dev); dev; - ret = uclass_next_device(&dev)) { + for (ret = uclass_first_device_check(UCLASS_PMIC, &dev); dev; + ret = uclass_next_device_check(&dev)) { if (ret) - continue; + err = ret;

- printf("| %-*.*s| %-*.*s| %s @ %d\n", + printf("| %-*.*s| %-*.*s| %s @ %d | probe: %i (%s)\n", LIMIT_DEV, LIMIT_DEV, dev->name, LIMIT_PARENT, LIMIT_PARENT, dev->parent->name, - dev_get_uclass_name(dev->parent), dev_seq(dev->parent)); + dev_get_uclass_name(dev->parent), dev_seq(dev->parent), + ret, errno_str(ret)); }

- if (ret) + if (err) return CMD_RET_FAILURE;

return CMD_RET_SUCCESS; diff --git a/cmd/regulator.c b/cmd/regulator.c index 60a70036d6..129c9fec20 100644 --- a/cmd/regulator.c +++ b/cmd/regulator.c @@ -205,7 +205,7 @@ static void do_status_detail(struct udevice *dev, constraint(" * mode id:", mode, mode_name); }

-static void do_status_line(struct udevice *dev) +static void do_status_line(struct udevice *dev, int status) { struct dm_regulator_uclass_plat *pdata; int current, value, mode; @@ -231,6 +231,7 @@ static void do_status_line(struct udevice *dev) printf("%-10s", mode_name); else printf("%-10s", "-"); + printf(" %s", status ? errno_str(status) : "OK"); printf("\n"); }

@@ -250,11 +251,11 @@ static int do_status(struct cmd_tbl *cmdtp, int flag, int argc, }

/* Show all of them in a list, probing them as needed */ - printf("%-20s %-10s %10s %10s %-10s\n", "Name", "Enabled", "uV", "mA", - "Mode"); - for (ret = uclass_first_device(UCLASS_REGULATOR, &dev); dev; - ret = uclass_next_device(&dev)) - do_status_line(dev); + printf("%-20s %-10s %10s %10s %-10s %s\n", "Name", "Enabled", "uV", "mA", + "Mode", "Status"); + for (ret = uclass_first_device_check(UCLASS_REGULATOR, &dev); dev; + ret = uclass_next_device_check(&dev)) + do_status_line(dev, ret);

return CMD_RET_SUCCESS; }

There is a number of users that use uclass_first_device to access the first and (assumed) only device in uclass.

Some check the return value of uclass_first_device and also that a device was returned which is exactly what uclass_first_device_err does.

Some are not checking that a device was returned and can potentially crash if no device exists in the uclass. Finally there is one that returns NULL on error either way.

Convert all of these to use uclass_first_device_err instead, the return value will be removed from uclass_first_device in a later patch.

Also print the symbolic error when the return value is printed.

Signed-off-by: Michal Suchanek msuchanek@suse.de --- arch/arm/mach-omap2/am33xx/board.c | 4 ++-- arch/x86/cpu/broadwell/cpu.c | 4 +--- arch/x86/cpu/intel_common/cpu.c | 4 +--- arch/x86/lib/pinctrl_ich6.c | 4 +--- board/intel/cougarcanyon2/cougarcanyon2.c | 4 +--- drivers/mmc/omap_hsmmc.c | 2 +- drivers/serial/serial-uclass.c | 2 +- drivers/serial/serial_bcm283x_mu.c | 2 +- drivers/serial/serial_bcm283x_pl011.c | 2 +- drivers/sysreset/sysreset_ast.c | 2 +- drivers/video/exynos/exynos_fb.c | 24 ++++++++++------------- drivers/video/mali_dp.c | 2 +- drivers/video/stm32/stm32_dsi.c | 6 ++++-- drivers/video/tegra124/dp.c | 7 ++++--- lib/acpi/acpi_table.c | 2 +- lib/efi_loader/efi_gop.c | 2 +- net/eth-uclass.c | 4 ++-- test/boot/bootmeth.c | 2 +- test/dm/acpi.c | 14 ++++++------- test/dm/devres.c | 4 ++-- test/dm/i2c.c | 8 ++++---- test/dm/virtio_device.c | 8 ++++---- test/dm/virtio_rng.c | 2 +- test/fuzz/cmd_fuzz.c | 2 +- test/fuzz/virtio.c | 2 +- 25 files changed, 55 insertions(+), 64 deletions(-)

diff --git a/arch/arm/mach-omap2/am33xx/board.c b/arch/arm/mach-omap2/am33xx/board.c index 7f1b84e466..f393ff9144 100644 --- a/arch/arm/mach-omap2/am33xx/board.c +++ b/arch/arm/mach-omap2/am33xx/board.c @@ -265,8 +265,8 @@ int arch_misc_init(void) struct udevice *dev; int ret;

- ret = uclass_first_device(UCLASS_MISC, &dev); - if (ret || !dev) + ret = uclass_first_device_err(UCLASS_MISC, &dev); + if (ret) return ret;

#if defined(CONFIG_DM_ETH) && defined(CONFIG_USB_ETHER) diff --git a/arch/x86/cpu/broadwell/cpu.c b/arch/x86/cpu/broadwell/cpu.c index 2adcf4b242..7877961451 100644 --- a/arch/x86/cpu/broadwell/cpu.c +++ b/arch/x86/cpu/broadwell/cpu.c @@ -31,11 +31,9 @@ static int broadwell_init_cpu(void *ctx, struct event *event) int ret;

/* Start up the LPC so we have serial */ - ret = uclass_first_device(UCLASS_LPC, &dev); + ret = uclass_first_device_err(UCLASS_LPC, &dev); if (ret) return ret; - if (!dev) - return -ENODEV; ret = cpu_set_flex_ratio_to_tdp_nominal(); if (ret) return ret; diff --git a/arch/x86/cpu/intel_common/cpu.c b/arch/x86/cpu/intel_common/cpu.c index 96d05e2eb3..8f489e6c65 100644 --- a/arch/x86/cpu/intel_common/cpu.c +++ b/arch/x86/cpu/intel_common/cpu.c @@ -61,11 +61,9 @@ int cpu_common_init(void) /* Early chipset init required before RAM init can work */ uclass_first_device(UCLASS_NORTHBRIDGE, &dev);

- ret = uclass_first_device(UCLASS_LPC, &lpc); + ret = uclass_first_device_err(UCLASS_LPC, &lpc); if (ret) return ret; - if (!lpc) - return -ENODEV;

/* Cause the SATA device to do its early init */ uclass_first_device(UCLASS_AHCI, &dev); diff --git a/arch/x86/lib/pinctrl_ich6.c b/arch/x86/lib/pinctrl_ich6.c index fd5e311b29..c93f245845 100644 --- a/arch/x86/lib/pinctrl_ich6.c +++ b/arch/x86/lib/pinctrl_ich6.c @@ -160,11 +160,9 @@ static int ich6_pinctrl_probe(struct udevice *dev) u32 iobase = -1;

debug("%s: start\n", __func__); - ret = uclass_first_device(UCLASS_PCH, &pch); + ret = uclass_first_device_err(UCLASS_PCH, &pch); if (ret) return ret; - if (!pch) - return -ENODEV;

/* * Get the memory/io base address to configure every pins. diff --git a/board/intel/cougarcanyon2/cougarcanyon2.c b/board/intel/cougarcanyon2/cougarcanyon2.c index ce11eae59d..7f61ef8b36 100644 --- a/board/intel/cougarcanyon2/cougarcanyon2.c +++ b/board/intel/cougarcanyon2/cougarcanyon2.c @@ -21,11 +21,9 @@ int board_early_init_f(void) struct udevice *pch; int ret;

- ret = uclass_first_device(UCLASS_PCH, &pch); + ret = uclass_first_device_err(UCLASS_PCH, &pch); if (ret) return ret; - if (!pch) - return -ENODEV;

/* Initialize LPC interface to turn on superio chipset decode range */ dm_pci_write_config16(pch, LPC_IO_DEC, COMA_DEC_RANGE | COMB_DEC_RANGE); diff --git a/drivers/mmc/omap_hsmmc.c b/drivers/mmc/omap_hsmmc.c index b2f4a4e721..a2595d19e7 100644 --- a/drivers/mmc/omap_hsmmc.c +++ b/drivers/mmc/omap_hsmmc.c @@ -644,7 +644,7 @@ static int omap_hsmmc_execute_tuning(struct udevice *dev, uint opcode) ((mmc->selected_mode == UHS_SDR50) && (val & CAPA2_TSDR50)))) return 0;

- ret = uclass_first_device(UCLASS_THERMAL, &thermal_dev); + ret = uclass_first_device_err(UCLASS_THERMAL, &thermal_dev); if (ret) { printf("Couldn't get thermal device for tuning\n"); return ret; diff --git a/drivers/serial/serial-uclass.c b/drivers/serial/serial-uclass.c index 30650e37b0..012b349511 100644 --- a/drivers/serial/serial-uclass.c +++ b/drivers/serial/serial-uclass.c @@ -143,7 +143,7 @@ static void serial_find_console_or_panic(void) #else if (!uclass_get_device_by_seq(UCLASS_SERIAL, INDEX, &dev) || !uclass_get_device(UCLASS_SERIAL, INDEX, &dev) || - (!uclass_first_device(UCLASS_SERIAL, &dev) && dev)) { + !uclass_first_device_err(UCLASS_SERIAL, &dev)) { gd->cur_serial_dev = dev; return; } diff --git a/drivers/serial/serial_bcm283x_mu.c b/drivers/serial/serial_bcm283x_mu.c index f0756c37c8..32c6e9ffc1 100644 --- a/drivers/serial/serial_bcm283x_mu.c +++ b/drivers/serial/serial_bcm283x_mu.c @@ -147,7 +147,7 @@ static bool bcm283x_is_serial_muxed(void) int serial_gpio = 15; struct udevice *dev;

- if (uclass_first_device(UCLASS_PINCTRL, &dev) || !dev) + if (uclass_first_device_err(UCLASS_PINCTRL, &dev)) return false;

if (pinctrl_get_gpio_mux(dev, 0, serial_gpio) != BCM2835_GPIO_ALT5) diff --git a/drivers/serial/serial_bcm283x_pl011.c b/drivers/serial/serial_bcm283x_pl011.c index fe746294cd..7d172cdac0 100644 --- a/drivers/serial/serial_bcm283x_pl011.c +++ b/drivers/serial/serial_bcm283x_pl011.c @@ -24,7 +24,7 @@ static bool bcm283x_is_serial_muxed(void) int serial_gpio = 15; struct udevice *dev;

- if (uclass_first_device(UCLASS_PINCTRL, &dev) || !dev) + if (uclass_first_device_err(UCLASS_PINCTRL, &dev)) return false;

if (pinctrl_get_gpio_mux(dev, 0, serial_gpio) != BCM2835_GPIO_ALT0) diff --git a/drivers/sysreset/sysreset_ast.c b/drivers/sysreset/sysreset_ast.c index d747ed00a7..92fad96871 100644 --- a/drivers/sysreset/sysreset_ast.c +++ b/drivers/sysreset/sysreset_ast.c @@ -18,7 +18,7 @@ static int ast_sysreset_request(struct udevice *dev, enum sysreset_t type) { struct udevice *wdt; u32 reset_mode; - int ret = uclass_first_device(UCLASS_WDT, &wdt); + int ret = uclass_first_device_err(UCLASS_WDT, &wdt);

if (ret) return ret; diff --git a/drivers/video/exynos/exynos_fb.c b/drivers/video/exynos/exynos_fb.c index 69992b3c2b..13f5317d11 100644 --- a/drivers/video/exynos/exynos_fb.c +++ b/drivers/video/exynos/exynos_fb.c @@ -28,7 +28,7 @@ #include <asm/arch/pinmux.h> #include <asm/arch/system.h> #include <asm/gpio.h> -#include <linux/errno.h> +#include <errno.h>

DECLARE_GLOBAL_DATA_PTR;

@@ -640,28 +640,24 @@ static int exynos_fb_probe(struct udevice *dev) #endif exynos_fimd_lcd_init(dev);

- ret = uclass_first_device(UCLASS_PANEL, &panel); + ret = uclass_first_device_err(UCLASS_PANEL, &panel); if (ret) { - printf("LCD panel failed to probe\n"); + printf("%s: LCD panel failed to probe %d (%s)\n", + __func__, ret, errno_str(ret)); return ret; } - if (!panel) { - printf("LCD panel not found\n"); - return -ENODEV; - }

- ret = uclass_first_device(UCLASS_DISPLAY, &dp); + ret = uclass_first_device_err(UCLASS_DISPLAY, &dp); if (ret) { - debug("%s: Display device error %d\n", __func__, ret); + debug("%s: Display device error %d (%s)\n", + __func__, ret, errno_str(ret)); return ret; } - if (!dev) { - debug("%s: Display device missing\n", __func__); - return -ENODEV; - } + ret = display_enable(dp, 18, NULL); if (ret) { - debug("%s: Display enable error %d\n", __func__, ret); + debug("%s: Display enable error %d (%s)\n", + __func__, ret, errno_str(ret)); return ret; }

diff --git a/drivers/video/mali_dp.c b/drivers/video/mali_dp.c index ba1ddd64e0..cbcdb99e1f 100644 --- a/drivers/video/mali_dp.c +++ b/drivers/video/mali_dp.c @@ -244,7 +244,7 @@ static int malidp_update_timings_from_edid(struct udevice *dev, struct udevice *disp_dev; int err;

- err = uclass_first_device(UCLASS_DISPLAY, &disp_dev); + err = uclass_first_device_err(UCLASS_DISPLAY, &disp_dev); if (err) return err;

diff --git a/drivers/video/stm32/stm32_dsi.c b/drivers/video/stm32/stm32_dsi.c index 5871ac7c4f..0924789f5e 100644 --- a/drivers/video/stm32/stm32_dsi.c +++ b/drivers/video/stm32/stm32_dsi.c @@ -14,6 +14,7 @@ #include <clk.h> #include <dm.h> #include <dsi_host.h> +#include <errno.h> #include <log.h> #include <mipi_dsi.h> #include <panel.h> @@ -346,9 +347,10 @@ static int stm32_dsi_attach(struct udevice *dev) struct display_timing timings; int ret;

- ret = uclass_first_device(UCLASS_PANEL, &priv->panel); + ret = uclass_first_device_err(UCLASS_PANEL, &priv->panel); if (ret) { - dev_err(dev, "panel device error %d\n", ret); + dev_err(dev, "panel device error %d (%s)\n", + ret, errno_str(ret)); return ret; }

diff --git a/drivers/video/tegra124/dp.c b/drivers/video/tegra124/dp.c index ee4f09a0c4..d4c0ba148e 100644 --- a/drivers/video/tegra124/dp.c +++ b/drivers/video/tegra124/dp.c @@ -1494,9 +1494,10 @@ int tegra_dp_enable(struct udevice *dev, int panel_bpp, return -ENOLINK; }

- ret = uclass_first_device(UCLASS_VIDEO_BRIDGE, &sor); - if (ret || !sor) { - debug("dp: failed to find SOR device: ret=%d\n", ret); + ret = uclass_first_device_err(UCLASS_VIDEO_BRIDGE, &sor); + if (ret) { + debug("dp: failed to find SOR device: ret=%d (%s)\n", + ret, errno_str(ret)); return ret; } priv->sor = sor; diff --git a/lib/acpi/acpi_table.c b/lib/acpi/acpi_table.c index f8642f9942..7c4189e243 100644 --- a/lib/acpi/acpi_table.c +++ b/lib/acpi/acpi_table.c @@ -40,7 +40,7 @@ int acpi_create_dmar(struct acpi_dmar *dmar, enum dmar_flags flags) struct udevice *cpu; int ret;

- ret = uclass_first_device(UCLASS_CPU, &cpu); + ret = uclass_first_device_err(UCLASS_CPU, &cpu); if (ret) return log_msg_ret("cpu", ret); ret = cpu_get_info(cpu, &info); diff --git a/lib/efi_loader/efi_gop.c b/lib/efi_loader/efi_gop.c index 5908b5c646..20bd7fff08 100644 --- a/lib/efi_loader/efi_gop.c +++ b/lib/efi_loader/efi_gop.c @@ -482,7 +482,7 @@ efi_status_t efi_gop_register(void) struct video_priv *priv;

/* We only support a single video output device for now */ - if (uclass_first_device(UCLASS_VIDEO, &vdev) || !vdev) { + if (uclass_first_device_err(UCLASS_VIDEO, &vdev)) { debug("WARNING: No video device\n"); return EFI_SUCCESS; } diff --git a/net/eth-uclass.c b/net/eth-uclass.c index 0f6b45b002..8c3f9cc31b 100644 --- a/net/eth-uclass.c +++ b/net/eth-uclass.c @@ -91,8 +91,8 @@ struct udevice *eth_get_dev(void) eth_errno = uclass_get_device_by_seq(UCLASS_ETH, 0, &uc_priv->current); if (eth_errno) - eth_errno = uclass_first_device(UCLASS_ETH, - &uc_priv->current); + eth_errno = uclass_first_device_err(UCLASS_ETH, + &uc_priv->current); } return uc_priv->current; } diff --git a/test/boot/bootmeth.c b/test/boot/bootmeth.c index fb62731339..f0b5ab9adb 100644 --- a/test/boot/bootmeth.c +++ b/test/boot/bootmeth.c @@ -156,7 +156,7 @@ static int bootmeth_state(struct unit_test_state *uts) struct udevice *dev; char buf[50];

- ut_assertok(uclass_first_device(UCLASS_BOOTMETH, &dev)); + ut_assertok(uclass_first_device_err(UCLASS_BOOTMETH, &dev)); ut_assertnonnull(dev);

ut_assertok(bootmeth_get_state_desc(dev, buf, sizeof(buf))); diff --git a/test/dm/acpi.c b/test/dm/acpi.c index edad91329f..9634fc2e90 100644 --- a/test/dm/acpi.c +++ b/test/dm/acpi.c @@ -169,28 +169,28 @@ static int dm_test_acpi_get_name(struct unit_test_state *uts) ut_asserteq_str("GHIJ", name);

/* Test getting the name from acpi_device_get_name() */ - ut_assertok(uclass_first_device(UCLASS_I2C, &i2c)); + ut_assertok(uclass_first_device_err(UCLASS_I2C, &i2c)); ut_assertok(acpi_get_name(i2c, name)); ut_asserteq_str("I2C0", name);

- ut_assertok(uclass_first_device(UCLASS_SPI, &spi)); + ut_assertok(uclass_first_device_err(UCLASS_SPI, &spi)); ut_assertok(acpi_get_name(spi, name)); ut_asserteq_str("SPI0", name);

/* ACPI doesn't know about the timer */ - ut_assertok(uclass_first_device(UCLASS_TIMER, &timer)); + ut_assertok(uclass_first_device_err(UCLASS_TIMER, &timer)); ut_asserteq(-ENOENT, acpi_get_name(timer, name));

/* May as well test the rest of the cases */ - ut_assertok(uclass_first_device(UCLASS_SOUND, &sound)); + ut_assertok(uclass_first_device_err(UCLASS_SOUND, &sound)); ut_assertok(acpi_get_name(sound, name)); ut_asserteq_str("HDAS", name);

- ut_assertok(uclass_first_device(UCLASS_PCI, &pci)); + ut_assertok(uclass_first_device_err(UCLASS_PCI, &pci)); ut_assertok(acpi_get_name(pci, name)); ut_asserteq_str("PCI0", name);

- ut_assertok(uclass_first_device(UCLASS_ROOT, &root)); + ut_assertok(uclass_first_device_err(UCLASS_ROOT, &root)); ut_assertok(acpi_get_name(root, name)); ut_asserteq_str("\_SB", name);

@@ -219,7 +219,7 @@ static int dm_test_acpi_create_dmar(struct unit_test_state *uts) struct acpi_dmar dmar; struct udevice *cpu;

- ut_assertok(uclass_first_device(UCLASS_CPU, &cpu)); + ut_assertok(uclass_first_device_err(UCLASS_CPU, &cpu)); ut_assertnonnull(cpu); ut_assertok(acpi_create_dmar(&dmar, DMAR_INTR_REMAP)); ut_asserteq(DMAR_INTR_REMAP, dmar.flags); diff --git a/test/dm/devres.c b/test/dm/devres.c index 524114c833..3df0f64362 100644 --- a/test/dm/devres.c +++ b/test/dm/devres.c @@ -165,8 +165,8 @@ static int dm_test_devres_phase(struct unit_test_state *uts) ut_asserteq(TEST_DEVRES_SIZE + TEST_DEVRES_SIZE3, stats.total_size);

/* Probing the device should add one allocation */ - ut_assertok(uclass_first_device(UCLASS_TEST_DEVRES, &dev)); - ut_assert(dev != NULL); + ut_assertok(uclass_first_device_err(UCLASS_TEST_DEVRES, &dev)); + ut_assertnonnull(dev); devres_get_stats(dev, &stats); ut_asserteq(3, stats.allocs); ut_asserteq(TEST_DEVRES_SIZE + TEST_DEVRES_SIZE2 + TEST_DEVRES_SIZE3, diff --git a/test/dm/i2c.c b/test/dm/i2c.c index 74b2097195..b46a22e79b 100644 --- a/test/dm/i2c.c +++ b/test/dm/i2c.c @@ -124,7 +124,7 @@ static int dm_test_i2c_bytewise(struct unit_test_state *uts) ut_asserteq_mem(buf, "\0\0\0\0\0", sizeof(buf));

/* Tell the EEPROM to only read/write one register at a time */ - ut_assertok(uclass_first_device(UCLASS_I2C_EMUL, &eeprom)); + ut_assertok(uclass_first_device_err(UCLASS_I2C_EMUL, &eeprom)); ut_assertnonnull(eeprom); sandbox_i2c_eeprom_set_test_mode(eeprom, SIE_TEST_MODE_SINGLE_BYTE);

@@ -177,7 +177,7 @@ static int dm_test_i2c_offset(struct unit_test_state *uts)

/* Do a transfer so we can find the emulator */ ut_assertok(dm_i2c_read(dev, 0, buf, 5)); - ut_assertok(uclass_first_device(UCLASS_I2C_EMUL, &eeprom)); + ut_assertok(uclass_first_device_err(UCLASS_I2C_EMUL, &eeprom));

/* Offset length 0 */ sandbox_i2c_eeprom_set_offset_len(eeprom, 0); @@ -250,7 +250,7 @@ static int dm_test_i2c_addr_offset(struct unit_test_state *uts)

/* Do a transfer so we can find the emulator */ ut_assertok(dm_i2c_read(dev, 0, buf, 5)); - ut_assertok(uclass_first_device(UCLASS_I2C_EMUL, &eeprom)); + ut_assertok(uclass_first_device_err(UCLASS_I2C_EMUL, &eeprom));

/* Offset length 0 */ sandbox_i2c_eeprom_set_offset_len(eeprom, 0); @@ -315,7 +315,7 @@ static int dm_test_i2c_reg_clrset(struct unit_test_state *uts)

/* Do a transfer so we can find the emulator */ ut_assertok(dm_i2c_read(dev, 0, buf, 5)); - ut_assertok(uclass_first_device(UCLASS_I2C_EMUL, &eeprom)); + ut_assertok(uclass_first_device_err(UCLASS_I2C_EMUL, &eeprom));

/* Dummy data for the test */ ut_assertok(dm_i2c_write(dev, 0, "\xff\x00\xff\x00\x10", 5)); diff --git a/test/dm/virtio_device.c b/test/dm/virtio_device.c index d0195e6bf0..b5c4523a02 100644 --- a/test/dm/virtio_device.c +++ b/test/dm/virtio_device.c @@ -22,7 +22,7 @@ static int dm_test_virtio_base(struct unit_test_state *uts) u8 status;

/* check probe success */ - ut_assertok(uclass_first_device(UCLASS_VIRTIO, &bus)); + ut_assertok(uclass_first_device_err(UCLASS_VIRTIO, &bus)); ut_assertnonnull(bus);

/* check the child virtio-rng device is bound */ @@ -60,7 +60,7 @@ static int dm_test_virtio_all_ops(struct unit_test_state *uts) struct virtqueue *vqs[2];

/* check probe success */ - ut_assertok(uclass_first_device(UCLASS_VIRTIO, &bus)); + ut_assertok(uclass_first_device_err(UCLASS_VIRTIO, &bus)); ut_assertnonnull(bus);

/* check the child virtio-rng device is bound */ @@ -102,7 +102,7 @@ static int dm_test_virtio_remove(struct unit_test_state *uts) struct udevice *bus, *dev;

/* check probe success */ - ut_assertok(uclass_first_device(UCLASS_VIRTIO, &bus)); + ut_assertok(uclass_first_device_err(UCLASS_VIRTIO, &bus)); ut_assertnonnull(bus);

/* check the child virtio-rng device is bound */ @@ -134,7 +134,7 @@ static int dm_test_virtio_ring(struct unit_test_state *uts) u8 buffer[2][32];

/* check probe success */ - ut_assertok(uclass_first_device(UCLASS_VIRTIO, &bus)); + ut_assertok(uclass_first_device_err(UCLASS_VIRTIO, &bus)); ut_assertnonnull(bus);

/* check the child virtio-blk device is bound */ diff --git a/test/dm/virtio_rng.c b/test/dm/virtio_rng.c index ff5646b4e1..8b9a04b1fd 100644 --- a/test/dm/virtio_rng.c +++ b/test/dm/virtio_rng.c @@ -28,7 +28,7 @@ static int dm_test_virtio_rng_check_len(struct unit_test_state *uts) u8 buffer[16];

/* check probe success */ - ut_assertok(uclass_first_device(UCLASS_VIRTIO, &bus)); + ut_assertok(uclass_first_device_err(UCLASS_VIRTIO, &bus)); ut_assertnonnull(bus);

/* check the child virtio-rng device is bound */ diff --git a/test/fuzz/cmd_fuzz.c b/test/fuzz/cmd_fuzz.c index 0cc01dc199..e2f44f3ecb 100644 --- a/test/fuzz/cmd_fuzz.c +++ b/test/fuzz/cmd_fuzz.c @@ -29,7 +29,7 @@ static struct udevice *find_fuzzing_engine(void) { struct udevice *dev;

- if (uclass_first_device(UCLASS_FUZZING_ENGINE, &dev)) + if (uclass_first_device_err(UCLASS_FUZZING_ENGINE, &dev)) return NULL;

return dev; diff --git a/test/fuzz/virtio.c b/test/fuzz/virtio.c index e5363d5638..8a47667e77 100644 --- a/test/fuzz/virtio.c +++ b/test/fuzz/virtio.c @@ -30,7 +30,7 @@ static int fuzz_vring(const uint8_t *data, size_t size) return 0;

/* check probe success */ - if (uclass_first_device(UCLASS_VIRTIO, &bus) || !bus) + if (uclass_first_device_err(UCLASS_VIRTIO, &bus)) panic("Could not find virtio bus\n");

/* check the child virtio-rng device is bound */

There are a couple users of uclass_next_device return value that get the first device by other means and use uclass_next_device assuming the following device in the uclass is related to the first one.

Use uclass_next_device_err because the return value from uclass_next_device will be removed in a later patch.

Signed-off-by: Michal Suchanek msuchanek@suse.de --- arch/arm/mach-k3/j721s2_init.c | 2 +- board/atmel/common/mac_eeprom.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-k3/j721s2_init.c b/arch/arm/mach-k3/j721s2_init.c index 12da8136f9..dd0c7ba18f 100644 --- a/arch/arm/mach-k3/j721s2_init.c +++ b/arch/arm/mach-k3/j721s2_init.c @@ -164,7 +164,7 @@ void board_init_f(ulong dummy) if (ret) panic("DRAM 0 init failed: %d\n", ret);

- ret = uclass_next_device(&dev); + ret = uclass_next_device_err(&dev); if (ret) panic("DRAM 1 init failed: %d\n", ret); } diff --git a/board/atmel/common/mac_eeprom.c b/board/atmel/common/mac_eeprom.c index a723ba723c..4606008c69 100644 --- a/board/atmel/common/mac_eeprom.c +++ b/board/atmel/common/mac_eeprom.c @@ -56,7 +56,7 @@ int at91_set_eth1addr(int offset) return ret;

/* attempt to obtain a second eeprom device */ - ret = uclass_next_device(&dev); + ret = uclass_next_device_err(&dev); if (ret) return ret;

blk_first_device_err/blk_next_device_err uses uclass_first_device_err/uclass_next_device_err for device iteration.

Although the function names superficially match the return value from uclass_first_device_err/uclass_next_device_err is never used meaningfully, and uclass_first_device/uclass_next_device works equally well for this purpose.

In the following patch the semantic of uclass_first_device_err/uclass_next_device_err will be changed to be based on uclass_first_device_check/uclass_next_device_check breaking this sole user that uses uclass_next_device_err for iteration.

Signed-off-by: Michal Suchanek msuchanek@suse.de --- drivers/block/blk-uclass.c | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-)

diff --git a/drivers/block/blk-uclass.c b/drivers/block/blk-uclass.c index 21c5209bb6..676f2444bf 100644 --- a/drivers/block/blk-uclass.c +++ b/drivers/block/blk-uclass.c @@ -585,11 +585,9 @@ int blk_find_next(enum blk_flag_t flags, struct udevice **devp)

int blk_first_device_err(enum blk_flag_t flags, struct udevice **devp) { - int ret; - - for (ret = uclass_first_device_err(UCLASS_BLK, devp); - !ret; - ret = uclass_next_device_err(devp)) { + for (uclass_first_device(UCLASS_BLK, devp); + *devp; + uclass_next_device(devp)) { if (!blk_flags_check(*devp, flags)) return 0; } @@ -599,11 +597,9 @@ int blk_first_device_err(enum blk_flag_t flags, struct udevice **devp)

int blk_next_device_err(enum blk_flag_t flags, struct udevice **devp) { - int ret; - - for (ret = uclass_next_device_err(devp); - !ret; - ret = uclass_next_device_err(devp)) { + for (uclass_next_device(devp); + *devp; + uclass_next_device(devp)) { if (!blk_flags_check(*devp, flags)) return 0; }

The _err variant iterators use the simple iterators without suffix as basis.

However, there is no user that uclass_next_device_err for iteration, many users of uclass_first_device_err use it to get the first and (assumed) only device of an uclass, and a couple that use uclass_next_device_err to get the device following a known device in the uclass list.

While there are some truly singleton device classes in which more than one device cannot exist these are quite rare, and most classes can have multiple devices even if it is not the case on the SoC's EVB.

In a later patch the simple iterators will be updated to not stop on error and return next device instead. With this in many cases the code that expects the first device or an error if it fails to probe may get the next device instead. Use the _check iterators as the basis of _err iterators to preserve the old behavior.

This is problematic for eth_get_dev: it relies on the broken behavior that returns an error but not the device on which the error happened which gives the caller no reasonable way to report or handle the error. With this change the device is returned but eth_get_dev stores the returned device pointer directly in a global state without checking the return value. Unset the pointer again in the error case.

Do the same for sysinfo_get because it is not clear how exactly the sysinfo is used by some callers, and fix up a call to pci_get_bus that checks the returned device and not the return value.

Switch uclass_foreach_dev_probe to the simple iterator - it does not use the returned value, and did not give out the failing devices previously.

Note in documentation that a non-activated device can be returned on error.

Signed-off-by: Michal Suchanek msuchanek@suse.de --- v5: - udate documentation - fix up a few more cases where device returned on error may cause problem --- drivers/core/uclass.c | 28 ++++++++++++------------- drivers/pci/pci-uclass.c | 7 +++---- drivers/sysinfo/sysinfo-uclass.c | 10 ++++++++- include/dm/uclass.h | 36 ++++++++++++++++++-------------- net/eth-uclass.c | 2 ++ 5 files changed, 48 insertions(+), 35 deletions(-)

diff --git a/drivers/core/uclass.c b/drivers/core/uclass.c index a591e22403..b7d11bdd23 100644 --- a/drivers/core/uclass.c +++ b/drivers/core/uclass.c @@ -586,19 +586,6 @@ int uclass_first_device(enum uclass_id id, struct udevice **devp) return uclass_get_device_tail(dev, ret, devp); }

-int uclass_first_device_err(enum uclass_id id, struct udevice **devp) -{ - int ret; - - ret = uclass_first_device(id, devp); - if (ret) - return ret; - else if (!*devp) - return -ENODEV; - - return 0; -} - int uclass_next_device(struct udevice **devp) { struct udevice *dev = *devp; @@ -611,11 +598,24 @@ int uclass_next_device(struct udevice **devp) return uclass_get_device_tail(dev, ret, devp); }

+int uclass_first_device_err(enum uclass_id id, struct udevice **devp) +{ + int ret; + + ret = uclass_first_device_check(id, devp); + if (ret) + return ret; + else if (!*devp) + return -ENODEV; + + return 0; +} + int uclass_next_device_err(struct udevice **devp) { int ret;

- ret = uclass_next_device(devp); + ret = uclass_next_device_check(devp); if (ret) return ret; else if (!*devp) diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c index 00e3828d95..2aa1043604 100644 --- a/drivers/pci/pci-uclass.c +++ b/drivers/pci/pci-uclass.c @@ -1768,10 +1768,9 @@ int pci_sriov_init(struct udevice *pdev, int vf_en)

bdf = dm_pci_get_bdf(pdev);

- pci_get_bus(PCI_BUS(bdf), &bus); - - if (!bus) - return -ENODEV; + ret = pci_get_bus(PCI_BUS(bdf), &bus); + if (ret) + return ret;

bdf += PCI_BDF(0, 0, vf_offset);

diff --git a/drivers/sysinfo/sysinfo-uclass.c b/drivers/sysinfo/sysinfo-uclass.c index c5cc3cb959..10194d0e14 100644 --- a/drivers/sysinfo/sysinfo-uclass.c +++ b/drivers/sysinfo/sysinfo-uclass.c @@ -16,7 +16,15 @@ struct sysinfo_priv {

int sysinfo_get(struct udevice **devp) { - return uclass_first_device_err(UCLASS_SYSINFO, devp); + int ret = uclass_first_device_err(UCLASS_SYSINFO, devp); + + /* + * There is some very dodgy error handling in gazerbeam, + * do not return a device on error. + */ + if (ret) + *devp = NULL; + return ret; }

int sysinfo_detect(struct udevice *dev) diff --git a/include/dm/uclass.h b/include/dm/uclass.h index f6c0110b06..b1c016ef9f 100644 --- a/include/dm/uclass.h +++ b/include/dm/uclass.h @@ -332,17 +332,6 @@ int uclass_get_device_by_driver(enum uclass_id id, const struct driver *drv, */ int uclass_first_device(enum uclass_id id, struct udevice **devp);

-/** - * uclass_first_device_err() - Get the first device in a uclass - * - * The device returned is probed if necessary, and ready for use - * - * @id: Uclass ID to look up - * @devp: Returns pointer to the first device in that uclass, or NULL if none - * Return: 0 if found, -ENODEV if not found, other -ve on error - */ -int uclass_first_device_err(enum uclass_id id, struct udevice **devp); - /** * uclass_next_device() - Get the next device in a uclass * @@ -358,10 +347,23 @@ int uclass_first_device_err(enum uclass_id id, struct udevice **devp); */ int uclass_next_device(struct udevice **devp);

+/** + * uclass_first_device_err() - Get the first device in a uclass + * + * The device returned is probed if necessary, and ready for use if no error is + * returned + * + * @id: Uclass ID to look up + * @devp: Returns pointer to the first device in that uclass, or NULL if none + * Return: 0 if found, -ENODEV if not found, other -ve on error + */ +int uclass_first_device_err(enum uclass_id id, struct udevice **devp); + /** * uclass_next_device_err() - Get the next device in a uclass * - * The device returned is probed if necessary, and ready for use + * The device returned is probed if necessary, and ready for use if no error is + * returned * * @devp: On entry, pointer to device to lookup. On exit, returns pointer * to the next device in the uclass if no error occurred, or NULL if @@ -373,7 +375,8 @@ int uclass_next_device_err(struct udevice **devp); /** * uclass_first_device_check() - Get the first device in a uclass * - * The device returned is probed if necessary, and ready for use + * The device returned is probed if necessary, and ready for use if no error is + * returned * * This function is useful to start iterating through a list of devices which * are functioning correctly and can be probed. @@ -389,7 +392,8 @@ int uclass_first_device_check(enum uclass_id id, struct udevice **devp); /** * uclass_next_device_check() - Get the next device in a uclass * - * The device returned is probed if necessary, and ready for use + * The device returned is probed if necessary, and ready for use if no error is + * returned * * This function is useful to start iterating through a list of devices which * are functioning correctly and can be probed. @@ -491,7 +495,7 @@ int uclass_id_count(enum uclass_id id); * are no more devices. */ #define uclass_foreach_dev_probe(id, dev) \ - for (int _ret = uclass_first_device_err(id, &dev); !_ret && dev; \ - _ret = uclass_next_device_err(&dev)) + for (uclass_first_device(id, &dev); dev; \ + uclass_next_device(&dev))

#endif diff --git a/net/eth-uclass.c b/net/eth-uclass.c index 8c3f9cc31b..f41da4b37b 100644 --- a/net/eth-uclass.c +++ b/net/eth-uclass.c @@ -93,6 +93,8 @@ struct udevice *eth_get_dev(void) if (eth_errno) eth_errno = uclass_first_device_err(UCLASS_ETH, &uc_priv->current); + if (eth_errno) + uc_priv->current = NULL; } return uc_priv->current; }

On Tue, 27 Sept 2022 at 15:38, Michal Suchanek msuchanek@suse.de wrote:

When probing a device fails NULL pointer is returned, and following devices in uclass list cannot be iterated. Skip to next device on error instead.

With that the only condition under which these simple iteration functions return error is when the dm is not initialized at uclass_get time. This is not all that interesting, change return type to void.

Fixes: 6494d708bf ("dm: Add base driver model support") Signed-off-by: Michal Suchanek msuchanek@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 v4: - change return value to void - further simplify iteration

---

drivers/core/uclass.c | 30 ++++++++++++++++++------------ include/dm/uclass.h | 13 ++++++------- test/dm/core.c | 10 ++++------ test/dm/test-fdt.c | 27 ++++++++++++++++++++------- 4 files changed, 48 insertions(+), 32 deletions(-)

Reviewed-by: Simon Glass sjg@chromium.org

diff --git a/drivers/core/uclass.c b/drivers/core/uclass.c index b7d11bdd23..6dec6a3973 100644 --- a/drivers/core/uclass.c +++ b/drivers/core/uclass.c @@ -574,28 +574,34 @@ int uclass_get_device_by_phandle(enum uclass_id id, struct udevice *parent, } #endif

-int uclass_first_device(enum uclass_id id, struct udevice **devp) +/*

• • Starting from the given device return first device in the uclass that

device, return the first

• • probes successfully.

Please describe the args here.

• */

+static void _uclass_next_device(struct udevice *dev, struct udevice **devp) +{

•   for (; dev; uclass_find_next_device(&dev)) {
  

•           if (!device_probe(dev))
  

•                   break;
  

•   }
  

•   *devp = dev;
  

+}

+void uclass_first_device(enum uclass_id id, struct udevice **devp) { struct udevice *dev; int ret;

•   *devp = NULL;
    ret = uclass_find_first_device(id, &dev);
  

•   if (!dev)
  

•           return 0;
  

•   return uclass_get_device_tail(dev, ret, devp);
  

•   _uclass_next_device(dev, devp);
  

}

-int uclass_next_device(struct udevice **devp) +void 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);
  

•   uclass_find_next_device(&dev);
  

•   _uclass_next_device(dev, devp);
  

}

int uclass_first_device_err(enum uclass_id id, struct udevice **devp) diff --git a/include/dm/uclass.h b/include/dm/uclass.h index b1c016ef9f..ee15c92063 100644 --- a/include/dm/uclass.h +++ b/include/dm/uclass.h @@ -320,32 +320,31 @@ int uclass_get_device_by_driver(enum uclass_id id, const struct driver *drv,

• uclass_first_device() - Get the first device in a uclass
• The device returned is probed if necessary, and ready for use

• • Devices that fail to probe are skipped
  • This function is useful to start iterating through a list of devices which
  • are functioning correctly and can be probed.
  • @id: Uclass ID to look up
  • @devp: Returns pointer to the first device in that uclass if no error

• • occurred, or NULL if there is no first device, or an error occurred with
• • that device.
• • Return: 0 if OK (found or not found), other -ve on error

• • occurred, or NULL if there is no usable device
  */

-int uclass_first_device(enum uclass_id id, struct udevice **devp); +void uclass_first_device(enum uclass_id id, struct udevice **devp);

/**

• uclass_next_device() - Get the next device in a uclass
• The device returned is probed if necessary, and ready for use

• • Devices that fail to probe are skipped
  • This function is useful to iterate through a list of devices which
  • are functioning correctly and can be probed.
  • @devp: On entry, pointer to device to lookup. On exit, returns pointer
  • to the next device in the uclass if no error occurred, or NULL if there is

• • no next device, or an error occurred with that next device.
• • Return: 0 if OK (found or not found), other -ve on error

• • no next device
  */

-int uclass_next_device(struct udevice **devp); +void uclass_next_device(struct udevice **devp);

/**

• uclass_first_device_err() - Get the first device in a uclass

diff --git a/test/dm/core.c b/test/dm/core.c index 84eb76ed5f..7f3f8d183b 100644 --- a/test/dm/core.c +++ b/test/dm/core.c @@ -1078,11 +1078,10 @@ static int dm_test_uclass_devices_get(struct unit_test_state *uts) struct udevice *dev; int ret;

•   for (ret = uclass_first_device(UCLASS_TEST, &dev);
  

•   for (ret = uclass_first_device_check(UCLASS_TEST, &dev);
         dev;
  

•        ret = uclass_next_device(&dev)) {
  

•        ret = uclass_next_device_check(&dev)) {
            ut_assert(!ret);
  

•           ut_assert(dev);
            ut_assert(device_active(dev));
    }
  
  

@@ -1112,11 +1111,10 @@ static int dm_test_uclass_devices_get_by_name(struct unit_test_state *uts) * this will fail on checking condition: testdev == finddev, since the * uclass_get_device_by_name(), returns the first device by given name. */

•   for (ret = uclass_first_device(UCLASS_TEST_FDT, &testdev);
  

•   for (ret = uclass_first_device_check(UCLASS_TEST_FDT, &testdev);
         testdev;
  

•        ret = uclass_next_device(&testdev)) {
  

•        ret = uclass_next_device_check(&testdev)) {
            ut_assertok(ret);
  

•           ut_assert(testdev);
            ut_assert(device_active(testdev));
  
            findret = uclass_get_device_by_name(UCLASS_TEST_FDT,
  

diff --git a/test/dm/test-fdt.c b/test/dm/test-fdt.c index 6118ad42ca..7cbc595c51 100644 --- a/test/dm/test-fdt.c +++ b/test/dm/test-fdt.c @@ -403,13 +403,12 @@ static int dm_test_first_next_device(struct unit_test_state *uts) int ret;

    /* There should be 4 devices */

•   for (ret = uclass_first_device(UCLASS_TEST_PROBE, &dev), count = 0;
  

•   for (uclass_first_device(UCLASS_TEST_PROBE, &dev), count = 0;
         dev;
  

•        ret = uclass_next_device(&dev)) {
  

•        uclass_next_device(&dev)) {
            count++;
            parent = dev_get_parent(dev);
            }
  

•   ut_assertok(ret);
    ut_asserteq(4, count);
  
    /* Remove them and try again, with an error on the second one */
  

@@ -417,16 +416,30 @@ static int dm_test_first_next_device(struct unit_test_state *uts) pdata = dev_get_plat(dev); pdata->probe_err = -ENOMEM; device_remove(parent, DM_REMOVE_NORMAL);

•   ut_assertok(uclass_first_device(UCLASS_TEST_PROBE, &dev));
  

•   ut_asserteq(-ENOMEM, uclass_next_device(&dev));
  

•   ut_asserteq_ptr(dev, NULL);
  

•   for (ret = uclass_first_device_check(UCLASS_TEST_PROBE, &dev),
  

•           count = 0;
  

•        dev;
  

•        ret = uclass_next_device_check(&dev)) {
  

•           if (!ret)
  

•                   count++;
  

•           else
  

•                   ut_asserteq(-ENOMEM, ret);
  

•           parent = dev_get_parent(dev);
  

•           }
  

•   ut_asserteq(3, count);
  
    /* Now an error on the first one */
    ut_assertok(uclass_get_device(UCLASS_TEST_PROBE, 0, &dev));
    pdata = dev_get_plat(dev);
    pdata->probe_err = -ENOENT;
    device_remove(parent, DM_REMOVE_NORMAL);
  

•   ut_asserteq(-ENOENT, uclass_first_device(UCLASS_TEST_PROBE, &dev));
  

•   for (uclass_first_device(UCLASS_TEST_PROBE, &dev), count = 0;
  

•        dev;
  

•        uclass_next_device(&dev)) {
  

•           count++;
  

•           parent = dev_get_parent(dev);
  

•           }
  

•   ut_asserteq(2, count);
  
    return 0;
  

}

2.37.3

We already have a function for probing all devices of a specific class, use it.

Signed-off-by: Michal Suchanek msuchanek@suse.de Reviewed-by: Simon Glass sjg@chromium.org --- drivers/cpu/cpu-uclass.c | 20 ++++---------------- drivers/virtio/virtio-uclass.c | 15 +-------------- test/dm/core.c | 17 +++-------------- test/test-main.c | 11 +---------- 4 files changed, 9 insertions(+), 54 deletions(-)

diff --git a/drivers/cpu/cpu-uclass.c b/drivers/cpu/cpu-uclass.c index 71e5900d70..a754832526 100644 --- a/drivers/cpu/cpu-uclass.c +++ b/drivers/cpu/cpu-uclass.c @@ -20,25 +20,13 @@ DECLARE_GLOBAL_DATA_PTR;

int cpu_probe_all(void) { - struct udevice *cpu; - int ret; + int ret = uclass_probe_all(UCLASS_CPU);

- ret = uclass_first_device(UCLASS_CPU, &cpu); if (ret) { - debug("%s: No CPU found (err = %d)\n", __func__, ret); - return ret; - } - - while (cpu) { - ret = uclass_next_device(&cpu); - if (ret) { - debug("%s: Error while probing CPU (err = %d)\n", - __func__, ret); - return ret; - } + debug("%s: Error while probing CPUs (err = %d %s)\n", + __func__, ret, errno_str(ret)); } - - return 0; + return ret; }

int cpu_is_current(struct udevice *cpu) diff --git a/drivers/virtio/virtio-uclass.c b/drivers/virtio/virtio-uclass.c index 9e2d0e06a1..da4f2f26a6 100644 --- a/drivers/virtio/virtio-uclass.c +++ b/drivers/virtio/virtio-uclass.c @@ -183,21 +183,8 @@ void virtio_driver_features_init(struct virtio_dev_priv *priv,

int virtio_init(void) { - struct udevice *bus; - int ret; - /* Enumerate all known virtio devices */ - ret = uclass_first_device(UCLASS_VIRTIO, &bus); - if (ret) - return ret; - - while (bus) { - ret = uclass_next_device(&bus); - if (ret) - break; - } - - return ret; + return uclass_probe_all(UCLASS_VIRTIO); }

static int virtio_uclass_pre_probe(struct udevice *udev) diff --git a/test/dm/core.c b/test/dm/core.c index fd4d756972..84eb76ed5f 100644 --- a/test/dm/core.c +++ b/test/dm/core.c @@ -512,23 +512,15 @@ static int dm_test_leak(struct unit_test_state *uts) int i;

for (i = 0; i < 2; i++) { - struct udevice *dev; int ret; - int id;

dm_leak_check_start(uts);

ut_assertok(dm_scan_plat(false)); ut_assertok(dm_scan_fdt(false));

- /* Scanning the uclass is enough to probe all the devices */ - for (id = UCLASS_ROOT; id < UCLASS_COUNT; id++) { - for (ret = uclass_first_device(UCLASS_TEST, &dev); - dev; - ret = uclass_next_device(&dev)) - ; - ut_assertok(ret); - } + ret = uclass_probe_all(UCLASS_TEST); + ut_assertok(ret);

ut_assertok(dm_leak_check_end(uts)); } @@ -653,10 +645,7 @@ static int dm_test_children(struct unit_test_state *uts) ut_asserteq(2 + NODE_COUNT, dm_testdrv_op_count[DM_TEST_OP_PROBE]);

/* Probe everything */ - for (ret = uclass_first_device(UCLASS_TEST, &dev); - dev; - ret = uclass_next_device(&dev)) - ; + ret = uclass_probe_all(UCLASS_TEST); ut_assertok(ret);

ut_asserteq(total, dm_testdrv_op_count[DM_TEST_OP_PROBE]); diff --git a/test/test-main.c b/test/test-main.c index d74df297c4..a98a77d68f 100644 --- a/test/test-main.c +++ b/test/test-main.c @@ -165,16 +165,7 @@ static int dm_test_post_run(struct unit_test_state *uts) /* Ensure all the test devices are probed */ static int do_autoprobe(struct unit_test_state *uts) { - struct udevice *dev; - int ret; - - /* Scanning the uclass is enough to probe all the devices */ - for (ret = uclass_first_device(UCLASS_TEST, &dev); - dev; - ret = uclass_next_device(&dev)) - ; - - return ret; + return uclass_probe_all(UCLASS_TEST); }

/*

When there is no PCI bus uclass_first_device will return no bus and no error which will result in pci_find_first_device calling skip_to_next_device with no bus, and the bus is only checked at the end of the while cycle, not the beginning.

Fixes: 76c3fbcd3d ("dm: pci: Add a way to iterate through all PCI devices") Signed-off-by: Michal Suchanek msuchanek@suse.de Reviewed-by: Simon Glass sjg@chromium.org --- v6: Move dropping the use of uclass iterator return value to later patch --- drivers/pci/pci-uclass.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c index 058b2f6359..5cff81ac44 100644 --- a/drivers/pci/pci-uclass.c +++ b/drivers/pci/pci-uclass.c @@ -1217,7 +1217,7 @@ static int skip_to_next_device(struct udevice *bus, struct udevice **devp) * Scan through all the PCI controllers. On x86 there will only be one * but that is not necessarily true on other hardware. */ - do { + while (bus) { device_find_first_child(bus, &dev); if (dev) { *devp = dev; @@ -1226,7 +1226,7 @@ static int skip_to_next_device(struct udevice *bus, struct udevice **devp) ret = uclass_next_device(&bus); if (ret) return ret; - } while (bus); + }

return 0; }

bootdev_list() uses uclass_*_device_err() to iterate devices. However, the only value _err adds is returning an error when the device pointer is null, and that's checked anyway.

Also there is some intent to report errors, and that's what uclass_*_device_check() is for, use it.

Signed-off-by: Michal Suchanek msuchanek@suse.de Reviewed-by: Simon Glass sjg@chromium.org --- v6: Drop errno_str --- boot/bootdev-uclass.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/boot/bootdev-uclass.c b/boot/bootdev-uclass.c index 13ac69eb39..9d98bee454 100644 --- a/boot/bootdev-uclass.c +++ b/boot/bootdev-uclass.c @@ -195,7 +195,7 @@ void bootdev_list(bool probe) printf("Seq Probed Status Uclass Name\n"); printf("--- ------ ------ -------- ------------------\n"); if (probe) - ret = uclass_first_device_err(UCLASS_BOOTDEV, &dev); + ret = uclass_first_device_check(UCLASS_BOOTDEV, &dev); else ret = uclass_find_first_device(UCLASS_BOOTDEV, &dev); for (i = 0; dev; i++) { @@ -204,7 +204,7 @@ void bootdev_list(bool probe) ret ? simple_itoa(ret) : "OK", dev_get_uclass_name(dev_get_parent(dev)), dev->name); if (probe) - ret = uclass_next_device_err(&dev); + ret = uclass_next_device_check(&dev); else ret = uclass_find_next_device(&dev); }

The code checks the return value from uclass_first_device as well as that the device exists but it passes on the return value which may be zero if there are no gadget devices. Just check that a device was returned and return -ENODEV otherwise.

Also remove the dev variable which is not really used for anything.

Signed-off-by: Michal Suchanek msuchanek@suse.de Reviewed-by: Simon Glass sjg@chromium.org --- drivers/usb/gadget/ether.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/gadget/ether.c b/drivers/usb/gadget/ether.c index 6ce389de9f..43aec7ffa7 100644 --- a/drivers/usb/gadget/ether.c +++ b/drivers/usb/gadget/ether.c @@ -2636,18 +2636,17 @@ static const struct eth_ops usb_eth_ops = {

int usb_ether_init(void) { - struct udevice *dev; struct udevice *usb_dev; int ret;

- ret = uclass_first_device(UCLASS_USB_GADGET_GENERIC, &usb_dev); - if (!usb_dev || ret) { + uclass_first_device(UCLASS_USB_GADGET_GENERIC, &usb_dev); + if (!usb_dev) { pr_err("No USB device found\n"); - return ret; + return -ENODEV; }

- ret = device_bind_driver(usb_dev, "usb_ether", "usb_ether", &dev); - if (!dev || ret) { + ret = device_bind_driver(usb_dev, "usb_ether", "usb_ether", NULL); + if (ret) { pr_err("usb - not able to bind usb_ether device\n"); return ret; }

There is a complaint in the code that iterates keyboards that we don't have the _check variant of class iterator but we in fact do, use it.

In the code that iterates video devices there is an attempt to print errors but the simple iterator does not return a device when there is an error. Use the _check variant of the iterator as well.

Also format error messages consistently.

Signed-off-by: Michal Suchanek msuchanek@suse.de Reviewed-by: Simon Glass sjg@chromium.org --- v6: - drop errno_str - fix dev vs vdev in video error print --- common/stdio.c | 32 +++++++++++++------------------- 1 file changed, 13 insertions(+), 19 deletions(-)

diff --git a/common/stdio.c b/common/stdio.c index 13083842cb..e316a355fa 100644 --- a/common/stdio.c +++ b/common/stdio.c @@ -314,7 +314,6 @@ int stdio_init_tables(void) int stdio_add_devices(void) { struct udevice *dev; - struct uclass *uc; int ret;

if (IS_ENABLED(CONFIG_DM_KEYBOARD)) { @@ -324,24 +323,18 @@ int stdio_add_devices(void) * have a list of input devices to start up in the stdin * environment variable. That work probably makes more sense * when stdio itself is converted to driver model. - * - * TODO(sjg@chromium.org): Convert changing - * uclass_first_device() etc. to return the device even on - * error. Then we could use that here. */ - ret = uclass_get(UCLASS_KEYBOARD, &uc); - if (ret) - return ret;

/* * Don't report errors to the caller - assume that they are * non-fatal */ - uclass_foreach_dev(dev, uc) { - ret = device_probe(dev); + for (ret = uclass_first_device_check(UCLASS_KEYBOARD, &dev); + dev; + ret = uclass_next_device_check(&dev)) { if (ret) - printf("Failed to probe keyboard '%s'\n", - dev->name); + printf("%s: Failed to probe keyboard '%s' (ret=%d)\n", + __func__, dev->name, ret); } } #if CONFIG_IS_ENABLED(SYS_I2C_LEGACY) @@ -361,13 +354,14 @@ int stdio_add_devices(void) int ret;

if (!IS_ENABLED(CONFIG_SYS_CONSOLE_IS_IN_ENV)) { - for (ret = uclass_first_device(UCLASS_VIDEO, &vdev); - vdev; - ret = uclass_next_device(&vdev)) - ; - if (ret) - printf("%s: Video device failed (ret=%d)\n", - __func__, ret); + for (ret = uclass_first_device_check(UCLASS_VIDEO, + &vdev); + vdev; + ret = uclass_next_device_check(&vdev)) { + if (ret) + printf("%s: Failed to probe video device '%s' (ret=%d)\n", + __func__, vdev->name, ret); + } } if (IS_ENABLED(CONFIG_SPLASH_SCREEN) && IS_ENABLED(CONFIG_CMD_BMP))

The code checks that uclass_first_device returned a device but the returned value that is assigned is never used. Use uclass_first_device_err instead, and move the error return outside of the if block.

Fixes: f4ec1ae08e ("mxc_ipuv3_fb.c: call display_enable") Signed-off-by: Michal Suchanek msuchanek@suse.de Reviewed-by: Simon Glass sjg@chromium.org --- drivers/video/imx/mxc_ipuv3_fb.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/video/imx/mxc_ipuv3_fb.c b/drivers/video/imx/mxc_ipuv3_fb.c index 49bbeefdd8..8b01a1be11 100644 --- a/drivers/video/imx/mxc_ipuv3_fb.c +++ b/drivers/video/imx/mxc_ipuv3_fb.c @@ -609,12 +609,11 @@ static int ipuv3_video_probe(struct udevice *dev) return ret;

#if defined(CONFIG_DISPLAY) - ret = uclass_first_device(UCLASS_DISPLAY, &disp_dev); - if (disp_dev) { + ret = uclass_first_device_err(UCLASS_DISPLAY, &disp_dev); + if (!ret) ret = display_enable(disp_dev, 16, NULL); - if (ret < 0) - return ret; - } + if (ret < 0) + return ret; #endif if (CONFIG_IS_ENABLED(PANEL)) { struct udevice *panel_dev;

Use uclass_first_device_check/uclass_next_device_check to correctly count buses that fail to probe.

Fixes: d3e19cf919 ("w1: Add 1-Wire uclass") Signed-off-by: Michal Suchanek msuchanek@suse.de Reviewed-by: Simon Glass sjg@chromium.org --- drivers/w1/w1-uclass.c | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/drivers/w1/w1-uclass.c b/drivers/w1/w1-uclass.c index 52b519c21d..de4f25bcf9 100644 --- a/drivers/w1/w1-uclass.c +++ b/drivers/w1/w1-uclass.c @@ -16,6 +16,7 @@

#include <common.h> #include <dm.h> +#include <errno.h> #include <log.h> #include <w1.h> #include <w1-eeprom.h> @@ -182,24 +183,25 @@ static int w1_enumerate(struct udevice *bus) int w1_get_bus(int busnum, struct udevice **busp) { int ret, i = 0; - struct udevice *dev;

- for (ret = uclass_first_device(UCLASS_W1, &dev); - dev && !ret; - ret = uclass_next_device(&dev), i++) { + for (ret = uclass_first_device_check(UCLASS_W1, &dev); + dev; + ret = uclass_next_device_check(&dev), i++) { if (i == busnum) { + if (ret) { + debug("Cannot probe w1 bus %d: %d (%s)\n", + busnum, ret, errno_str(ret)); + return ret; + } *busp = dev; return 0; } }

- if (!ret) { - debug("Cannot find w1 bus %d\n", busnum); - ret = -ENODEV; - } + debug("Cannot find w1 bus %d\n", busnum);

- return ret; + return -ENODEV; }

u8 w1_get_device_family(struct udevice *dev)

There are a few commands that iterate uclass with uclass_first_device/uclass_next_device or the _err variant.

Use the _check class iterator variant to get devices that fail to probe as well, and print the status.

Signed-off-by: Michal Suchanek msuchanek@suse.de Reviewed-by: Simon Glass sjg@chromium.org --- v6: Drop errno_str --- cmd/adc.c | 20 ++++++++------------ cmd/demo.c | 15 +++++++++------ cmd/gpio.c | 15 +++++++++++---- cmd/pmic.c | 15 ++++++++------- cmd/regulator.c | 13 +++++++------ 5 files changed, 43 insertions(+), 35 deletions(-)

diff --git a/cmd/adc.c b/cmd/adc.c index 1c5d3e10a3..a739d9e464 100644 --- a/cmd/adc.c +++ b/cmd/adc.c @@ -12,23 +12,19 @@ static int do_adc_list(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) { struct udevice *dev; - int ret; + int ret, err;

- ret = uclass_first_device_err(UCLASS_ADC, &dev); - if (ret) { - printf("No available ADC device\n"); - return CMD_RET_FAILURE; - } + ret = err = uclass_first_device_check(UCLASS_ADC, &dev);

- do { - printf("- %s\n", dev->name); + while (dev) { + printf("- %s status: %i\n", dev->name, ret);

- ret = uclass_next_device(&dev); + ret = uclass_next_device_check(&dev); if (ret) - return CMD_RET_FAILURE; - } while (dev); + err = ret; + }

- return CMD_RET_SUCCESS; + return err ? CMD_RET_FAILURE : CMD_RET_SUCCESS; }

static int do_adc_info(struct cmd_tbl *cmdtp, int flag, int argc, diff --git a/cmd/demo.c b/cmd/demo.c index 571f562ec6..ebd5a241c3 100644 --- a/cmd/demo.c +++ b/cmd/demo.c @@ -64,20 +64,23 @@ static int do_demo_light(struct cmd_tbl *cmdtp, int flag, int argc, int do_demo_list(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) { struct udevice *dev; - int i, ret; + int i, ret, err = 0;

puts("Demo uclass entries:\n");

- for (i = 0, ret = uclass_first_device(UCLASS_DEMO, &dev); + for (i = 0, ret = uclass_first_device_check(UCLASS_DEMO, &dev); dev; - ret = uclass_next_device(&dev)) { - printf("entry %d - instance %08x, ops %08x, plat %08x\n", + ret = uclass_next_device_check(&dev)) { + printf("entry %d - instance %08x, ops %08x, plat %08x, status %i\n", i++, (uint)map_to_sysmem(dev), (uint)map_to_sysmem(dev->driver->ops), - (uint)map_to_sysmem(dev_get_plat(dev))); + (uint)map_to_sysmem(dev_get_plat(dev)), + ret); + if (ret) + err = ret; }

- return cmd_process_error(cmdtp, ret); + return cmd_process_error(cmdtp, err); }

static struct cmd_tbl demo_commands[] = { diff --git a/cmd/gpio.c b/cmd/gpio.c index 53e9ce666f..f4565982ec 100644 --- a/cmd/gpio.c +++ b/cmd/gpio.c @@ -77,17 +77,24 @@ static int do_gpio_status(bool all, const char *gpio_name) struct udevice *dev; int banklen; int flags; - int ret; + int ret, err = 0;

flags = 0; if (gpio_name && !*gpio_name) gpio_name = NULL; - for (ret = uclass_first_device(UCLASS_GPIO, &dev); + for (ret = uclass_first_device_check(UCLASS_GPIO, &dev); dev; - ret = uclass_next_device(&dev)) { + ret = uclass_next_device_check(&dev)) { const char *bank_name; int num_bits;

+ if (ret) { + printf("GPIO device %s probe error %i\n", + dev->name, ret); + err = ret; + continue; + } + flags |= FLAG_SHOW_BANK; if (all) flags |= FLAG_SHOW_ALL; @@ -120,7 +127,7 @@ static int do_gpio_status(bool all, const char *gpio_name) flags |= FLAG_SHOW_NEWLINE; }

- return ret; + return err; } #endif

diff --git a/cmd/pmic.c b/cmd/pmic.c index 0cb44d0740..49a405fa29 100644 --- a/cmd/pmic.c +++ b/cmd/pmic.c @@ -51,25 +51,26 @@ static int do_list(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) { struct udevice *dev; - int ret; + int ret, err = 0;

printf("| %-*.*s| %-*.*s| %s @ %s\n", LIMIT_DEV, LIMIT_DEV, "Name", LIMIT_PARENT, LIMIT_PARENT, "Parent name", "Parent uclass", "seq");

- for (ret = uclass_first_device(UCLASS_PMIC, &dev); dev; - ret = uclass_next_device(&dev)) { + for (ret = uclass_first_device_check(UCLASS_PMIC, &dev); dev; + ret = uclass_next_device_check(&dev)) { if (ret) - continue; + err = ret;

- printf("| %-*.*s| %-*.*s| %s @ %d\n", + printf("| %-*.*s| %-*.*s| %s @ %d | status: %i\n", LIMIT_DEV, LIMIT_DEV, dev->name, LIMIT_PARENT, LIMIT_PARENT, dev->parent->name, - dev_get_uclass_name(dev->parent), dev_seq(dev->parent)); + dev_get_uclass_name(dev->parent), dev_seq(dev->parent), + ret); }

- if (ret) + if (err) return CMD_RET_FAILURE;

return CMD_RET_SUCCESS; diff --git a/cmd/regulator.c b/cmd/regulator.c index 60a70036d6..ed4996dbd2 100644 --- a/cmd/regulator.c +++ b/cmd/regulator.c @@ -205,7 +205,7 @@ static void do_status_detail(struct udevice *dev, constraint(" * mode id:", mode, mode_name); }

-static void do_status_line(struct udevice *dev) +static void do_status_line(struct udevice *dev, int status) { struct dm_regulator_uclass_plat *pdata; int current, value, mode; @@ -231,6 +231,7 @@ static void do_status_line(struct udevice *dev) printf("%-10s", mode_name); else printf("%-10s", "-"); + printf(" %i", status); printf("\n"); }

@@ -250,11 +251,11 @@ static int do_status(struct cmd_tbl *cmdtp, int flag, int argc, }

/* Show all of them in a list, probing them as needed */ - printf("%-20s %-10s %10s %10s %-10s\n", "Name", "Enabled", "uV", "mA", - "Mode"); - for (ret = uclass_first_device(UCLASS_REGULATOR, &dev); dev; - ret = uclass_next_device(&dev)) - do_status_line(dev); + printf("%-20s %-10s %10s %10s %-10s %s\n", "Name", "Enabled", "uV", "mA", + "Mode", "Status"); + for (ret = uclass_first_device_check(UCLASS_REGULATOR, &dev); dev; + ret = uclass_next_device_check(&dev)) + do_status_line(dev, ret);

return CMD_RET_SUCCESS; }

There is a number of users that use uclass_first_device to access the first and (assumed) only device in uclass.

Some check the return value of uclass_first_device and also that a device was returned which is exactly what uclass_first_device_err does.

Some are not checking that a device was returned and can potentially crash if no device exists in the uclass. Finally there is one that returns NULL on error either way.

Convert all of these to use uclass_first_device_err instead, the return value will be removed from uclass_first_device in a later patch.

Signed-off-by: Michal Suchanek msuchanek@suse.de Reviewed-by: Simon Glass sjg@chromium.org --- v6: drop errno_str --- arch/arm/mach-omap2/am33xx/board.c | 4 ++-- arch/x86/cpu/broadwell/cpu.c | 4 +--- arch/x86/cpu/intel_common/cpu.c | 4 +--- arch/x86/lib/pinctrl_ich6.c | 4 +--- board/intel/cougarcanyon2/cougarcanyon2.c | 4 +--- drivers/mmc/omap_hsmmc.c | 2 +- drivers/serial/serial-uclass.c | 2 +- drivers/serial/serial_bcm283x_mu.c | 2 +- drivers/serial/serial_bcm283x_pl011.c | 2 +- drivers/sysreset/sysreset_ast.c | 2 +- drivers/video/exynos/exynos_fb.c | 14 +++----------- drivers/video/mali_dp.c | 2 +- drivers/video/stm32/stm32_dsi.c | 2 +- drivers/video/tegra124/dp.c | 4 ++-- lib/acpi/acpi_table.c | 2 +- lib/efi_loader/efi_gop.c | 2 +- net/eth-uclass.c | 4 ++-- test/boot/bootmeth.c | 2 +- test/dm/acpi.c | 14 +++++++------- test/dm/devres.c | 4 ++-- test/dm/i2c.c | 8 ++++---- test/dm/virtio_device.c | 8 ++++---- test/dm/virtio_rng.c | 2 +- test/fuzz/cmd_fuzz.c | 2 +- test/fuzz/virtio.c | 2 +- 25 files changed, 43 insertions(+), 59 deletions(-)

diff --git a/arch/arm/mach-omap2/am33xx/board.c b/arch/arm/mach-omap2/am33xx/board.c index 7f1b84e466..f393ff9144 100644 --- a/arch/arm/mach-omap2/am33xx/board.c +++ b/arch/arm/mach-omap2/am33xx/board.c @@ -265,8 +265,8 @@ int arch_misc_init(void) struct udevice *dev; int ret;

- ret = uclass_first_device(UCLASS_MISC, &dev); - if (ret || !dev) + ret = uclass_first_device_err(UCLASS_MISC, &dev); + if (ret) return ret;

#if defined(CONFIG_DM_ETH) && defined(CONFIG_USB_ETHER) diff --git a/arch/x86/cpu/broadwell/cpu.c b/arch/x86/cpu/broadwell/cpu.c index 2adcf4b242..7877961451 100644 --- a/arch/x86/cpu/broadwell/cpu.c +++ b/arch/x86/cpu/broadwell/cpu.c @@ -31,11 +31,9 @@ static int broadwell_init_cpu(void *ctx, struct event *event) int ret;

/* Start up the LPC so we have serial */ - ret = uclass_first_device(UCLASS_LPC, &dev); + ret = uclass_first_device_err(UCLASS_LPC, &dev); if (ret) return ret; - if (!dev) - return -ENODEV; ret = cpu_set_flex_ratio_to_tdp_nominal(); if (ret) return ret; diff --git a/arch/x86/cpu/intel_common/cpu.c b/arch/x86/cpu/intel_common/cpu.c index 96d05e2eb3..8f489e6c65 100644 --- a/arch/x86/cpu/intel_common/cpu.c +++ b/arch/x86/cpu/intel_common/cpu.c @@ -61,11 +61,9 @@ int cpu_common_init(void) /* Early chipset init required before RAM init can work */ uclass_first_device(UCLASS_NORTHBRIDGE, &dev);

- ret = uclass_first_device(UCLASS_LPC, &lpc); + ret = uclass_first_device_err(UCLASS_LPC, &lpc); if (ret) return ret; - if (!lpc) - return -ENODEV;

/* Cause the SATA device to do its early init */ uclass_first_device(UCLASS_AHCI, &dev); diff --git a/arch/x86/lib/pinctrl_ich6.c b/arch/x86/lib/pinctrl_ich6.c index fd5e311b29..c93f245845 100644 --- a/arch/x86/lib/pinctrl_ich6.c +++ b/arch/x86/lib/pinctrl_ich6.c @@ -160,11 +160,9 @@ static int ich6_pinctrl_probe(struct udevice *dev) u32 iobase = -1;

debug("%s: start\n", __func__); - ret = uclass_first_device(UCLASS_PCH, &pch); + ret = uclass_first_device_err(UCLASS_PCH, &pch); if (ret) return ret; - if (!pch) - return -ENODEV;

/* * Get the memory/io base address to configure every pins. diff --git a/board/intel/cougarcanyon2/cougarcanyon2.c b/board/intel/cougarcanyon2/cougarcanyon2.c index ce11eae59d..7f61ef8b36 100644 --- a/board/intel/cougarcanyon2/cougarcanyon2.c +++ b/board/intel/cougarcanyon2/cougarcanyon2.c @@ -21,11 +21,9 @@ int board_early_init_f(void) struct udevice *pch; int ret;

- ret = uclass_first_device(UCLASS_PCH, &pch); + ret = uclass_first_device_err(UCLASS_PCH, &pch); if (ret) return ret; - if (!pch) - return -ENODEV;

/* Initialize LPC interface to turn on superio chipset decode range */ dm_pci_write_config16(pch, LPC_IO_DEC, COMA_DEC_RANGE | COMB_DEC_RANGE); diff --git a/drivers/mmc/omap_hsmmc.c b/drivers/mmc/omap_hsmmc.c index b2f4a4e721..a2595d19e7 100644 --- a/drivers/mmc/omap_hsmmc.c +++ b/drivers/mmc/omap_hsmmc.c @@ -644,7 +644,7 @@ static int omap_hsmmc_execute_tuning(struct udevice *dev, uint opcode) ((mmc->selected_mode == UHS_SDR50) && (val & CAPA2_TSDR50)))) return 0;

- ret = uclass_first_device(UCLASS_THERMAL, &thermal_dev); + ret = uclass_first_device_err(UCLASS_THERMAL, &thermal_dev); if (ret) { printf("Couldn't get thermal device for tuning\n"); return ret; diff --git a/drivers/serial/serial-uclass.c b/drivers/serial/serial-uclass.c index da3e1eb3ab..83cda1f204 100644 --- a/drivers/serial/serial-uclass.c +++ b/drivers/serial/serial-uclass.c @@ -143,7 +143,7 @@ static void serial_find_console_or_panic(void) #else if (!uclass_get_device_by_seq(UCLASS_SERIAL, INDEX, &dev) || !uclass_get_device(UCLASS_SERIAL, INDEX, &dev) || - (!uclass_first_device(UCLASS_SERIAL, &dev) && dev)) { + !uclass_first_device_err(UCLASS_SERIAL, &dev)) { gd->cur_serial_dev = dev; return; } diff --git a/drivers/serial/serial_bcm283x_mu.c b/drivers/serial/serial_bcm283x_mu.c index 493a42b4cc..12cbcb9858 100644 --- a/drivers/serial/serial_bcm283x_mu.c +++ b/drivers/serial/serial_bcm283x_mu.c @@ -147,7 +147,7 @@ static bool bcm283x_is_serial_muxed(void) int serial_gpio = 15; struct udevice *dev;

- if (uclass_first_device(UCLASS_PINCTRL, &dev) || !dev) + if (uclass_first_device_err(UCLASS_PINCTRL, &dev)) return false;

if (pinctrl_get_gpio_mux(dev, 0, serial_gpio) != BCM2835_GPIO_ALT5) diff --git a/drivers/serial/serial_bcm283x_pl011.c b/drivers/serial/serial_bcm283x_pl011.c index fe746294cd..7d172cdac0 100644 --- a/drivers/serial/serial_bcm283x_pl011.c +++ b/drivers/serial/serial_bcm283x_pl011.c @@ -24,7 +24,7 @@ static bool bcm283x_is_serial_muxed(void) int serial_gpio = 15; struct udevice *dev;

- if (uclass_first_device(UCLASS_PINCTRL, &dev) || !dev) + if (uclass_first_device_err(UCLASS_PINCTRL, &dev)) return false;

if (pinctrl_get_gpio_mux(dev, 0, serial_gpio) != BCM2835_GPIO_ALT0) diff --git a/drivers/sysreset/sysreset_ast.c b/drivers/sysreset/sysreset_ast.c index d747ed00a7..92fad96871 100644 --- a/drivers/sysreset/sysreset_ast.c +++ b/drivers/sysreset/sysreset_ast.c @@ -18,7 +18,7 @@ static int ast_sysreset_request(struct udevice *dev, enum sysreset_t type) { struct udevice *wdt; u32 reset_mode; - int ret = uclass_first_device(UCLASS_WDT, &wdt); + int ret = uclass_first_device_err(UCLASS_WDT, &wdt);

if (ret) return ret; diff --git a/drivers/video/exynos/exynos_fb.c b/drivers/video/exynos/exynos_fb.c index 69992b3c2b..86970a6d5d 100644 --- a/drivers/video/exynos/exynos_fb.c +++ b/drivers/video/exynos/exynos_fb.c @@ -640,25 +640,17 @@ static int exynos_fb_probe(struct udevice *dev) #endif exynos_fimd_lcd_init(dev);

- ret = uclass_first_device(UCLASS_PANEL, &panel); + ret = uclass_first_device_err(UCLASS_PANEL, &panel); if (ret) { - printf("LCD panel failed to probe\n"); + printf("%s: LCD panel failed to probe %d\n", __func__, ret); return ret; } - if (!panel) { - printf("LCD panel not found\n"); - return -ENODEV; - }

- ret = uclass_first_device(UCLASS_DISPLAY, &dp); + ret = uclass_first_device_err(UCLASS_DISPLAY, &dp); if (ret) { debug("%s: Display device error %d\n", __func__, ret); return ret; } - if (!dev) { - debug("%s: Display device missing\n", __func__); - return -ENODEV; - } ret = display_enable(dp, 18, NULL); if (ret) { debug("%s: Display enable error %d\n", __func__, ret); diff --git a/drivers/video/mali_dp.c b/drivers/video/mali_dp.c index ba1ddd64e0..cbcdb99e1f 100644 --- a/drivers/video/mali_dp.c +++ b/drivers/video/mali_dp.c @@ -244,7 +244,7 @@ static int malidp_update_timings_from_edid(struct udevice *dev, struct udevice *disp_dev; int err;

- err = uclass_first_device(UCLASS_DISPLAY, &disp_dev); + err = uclass_first_device_err(UCLASS_DISPLAY, &disp_dev); if (err) return err;

diff --git a/drivers/video/stm32/stm32_dsi.c b/drivers/video/stm32/stm32_dsi.c index 5871ac7c4f..e6347bb8da 100644 --- a/drivers/video/stm32/stm32_dsi.c +++ b/drivers/video/stm32/stm32_dsi.c @@ -346,7 +346,7 @@ static int stm32_dsi_attach(struct udevice *dev) struct display_timing timings; int ret;

- ret = uclass_first_device(UCLASS_PANEL, &priv->panel); + ret = uclass_first_device_err(UCLASS_PANEL, &priv->panel); if (ret) { dev_err(dev, "panel device error %d\n", ret); return ret; diff --git a/drivers/video/tegra124/dp.c b/drivers/video/tegra124/dp.c index ee4f09a0c4..b27b1633ba 100644 --- a/drivers/video/tegra124/dp.c +++ b/drivers/video/tegra124/dp.c @@ -1494,8 +1494,8 @@ int tegra_dp_enable(struct udevice *dev, int panel_bpp, return -ENOLINK; }

- ret = uclass_first_device(UCLASS_VIDEO_BRIDGE, &sor); - if (ret || !sor) { + ret = uclass_first_device_err(UCLASS_VIDEO_BRIDGE, &sor); + if (ret) { debug("dp: failed to find SOR device: ret=%d\n", ret); return ret; } diff --git a/lib/acpi/acpi_table.c b/lib/acpi/acpi_table.c index f8642f9942..7c4189e243 100644 --- a/lib/acpi/acpi_table.c +++ b/lib/acpi/acpi_table.c @@ -40,7 +40,7 @@ int acpi_create_dmar(struct acpi_dmar *dmar, enum dmar_flags flags) struct udevice *cpu; int ret;

- ret = uclass_first_device(UCLASS_CPU, &cpu); + ret = uclass_first_device_err(UCLASS_CPU, &cpu); if (ret) return log_msg_ret("cpu", ret); ret = cpu_get_info(cpu, &info); diff --git a/lib/efi_loader/efi_gop.c b/lib/efi_loader/efi_gop.c index 5908b5c646..20bd7fff08 100644 --- a/lib/efi_loader/efi_gop.c +++ b/lib/efi_loader/efi_gop.c @@ -482,7 +482,7 @@ efi_status_t efi_gop_register(void) struct video_priv *priv;

/* We only support a single video output device for now */ - if (uclass_first_device(UCLASS_VIDEO, &vdev) || !vdev) { + if (uclass_first_device_err(UCLASS_VIDEO, &vdev)) { debug("WARNING: No video device\n"); return EFI_SUCCESS; } diff --git a/net/eth-uclass.c b/net/eth-uclass.c index 0f6b45b002..8c3f9cc31b 100644 --- a/net/eth-uclass.c +++ b/net/eth-uclass.c @@ -91,8 +91,8 @@ struct udevice *eth_get_dev(void) eth_errno = uclass_get_device_by_seq(UCLASS_ETH, 0, &uc_priv->current); if (eth_errno) - eth_errno = uclass_first_device(UCLASS_ETH, - &uc_priv->current); + eth_errno = uclass_first_device_err(UCLASS_ETH, + &uc_priv->current); } return uc_priv->current; } diff --git a/test/boot/bootmeth.c b/test/boot/bootmeth.c index fb62731339..f0b5ab9adb 100644 --- a/test/boot/bootmeth.c +++ b/test/boot/bootmeth.c @@ -156,7 +156,7 @@ static int bootmeth_state(struct unit_test_state *uts) struct udevice *dev; char buf[50];

- ut_assertok(uclass_first_device(UCLASS_BOOTMETH, &dev)); + ut_assertok(uclass_first_device_err(UCLASS_BOOTMETH, &dev)); ut_assertnonnull(dev);

ut_assertok(bootmeth_get_state_desc(dev, buf, sizeof(buf))); diff --git a/test/dm/acpi.c b/test/dm/acpi.c index edad91329f..9634fc2e90 100644 --- a/test/dm/acpi.c +++ b/test/dm/acpi.c @@ -169,28 +169,28 @@ static int dm_test_acpi_get_name(struct unit_test_state *uts) ut_asserteq_str("GHIJ", name);

/* Test getting the name from acpi_device_get_name() */ - ut_assertok(uclass_first_device(UCLASS_I2C, &i2c)); + ut_assertok(uclass_first_device_err(UCLASS_I2C, &i2c)); ut_assertok(acpi_get_name(i2c, name)); ut_asserteq_str("I2C0", name);

- ut_assertok(uclass_first_device(UCLASS_SPI, &spi)); + ut_assertok(uclass_first_device_err(UCLASS_SPI, &spi)); ut_assertok(acpi_get_name(spi, name)); ut_asserteq_str("SPI0", name);

/* ACPI doesn't know about the timer */ - ut_assertok(uclass_first_device(UCLASS_TIMER, &timer)); + ut_assertok(uclass_first_device_err(UCLASS_TIMER, &timer)); ut_asserteq(-ENOENT, acpi_get_name(timer, name));

/* May as well test the rest of the cases */ - ut_assertok(uclass_first_device(UCLASS_SOUND, &sound)); + ut_assertok(uclass_first_device_err(UCLASS_SOUND, &sound)); ut_assertok(acpi_get_name(sound, name)); ut_asserteq_str("HDAS", name);

- ut_assertok(uclass_first_device(UCLASS_PCI, &pci)); + ut_assertok(uclass_first_device_err(UCLASS_PCI, &pci)); ut_assertok(acpi_get_name(pci, name)); ut_asserteq_str("PCI0", name);

- ut_assertok(uclass_first_device(UCLASS_ROOT, &root)); + ut_assertok(uclass_first_device_err(UCLASS_ROOT, &root)); ut_assertok(acpi_get_name(root, name)); ut_asserteq_str("\_SB", name);

@@ -219,7 +219,7 @@ static int dm_test_acpi_create_dmar(struct unit_test_state *uts) struct acpi_dmar dmar; struct udevice *cpu;

- ut_assertok(uclass_first_device(UCLASS_CPU, &cpu)); + ut_assertok(uclass_first_device_err(UCLASS_CPU, &cpu)); ut_assertnonnull(cpu); ut_assertok(acpi_create_dmar(&dmar, DMAR_INTR_REMAP)); ut_asserteq(DMAR_INTR_REMAP, dmar.flags); diff --git a/test/dm/devres.c b/test/dm/devres.c index 524114c833..3df0f64362 100644 --- a/test/dm/devres.c +++ b/test/dm/devres.c @@ -165,8 +165,8 @@ static int dm_test_devres_phase(struct unit_test_state *uts) ut_asserteq(TEST_DEVRES_SIZE + TEST_DEVRES_SIZE3, stats.total_size);

/* Probing the device should add one allocation */ - ut_assertok(uclass_first_device(UCLASS_TEST_DEVRES, &dev)); - ut_assert(dev != NULL); + ut_assertok(uclass_first_device_err(UCLASS_TEST_DEVRES, &dev)); + ut_assertnonnull(dev); devres_get_stats(dev, &stats); ut_asserteq(3, stats.allocs); ut_asserteq(TEST_DEVRES_SIZE + TEST_DEVRES_SIZE2 + TEST_DEVRES_SIZE3, diff --git a/test/dm/i2c.c b/test/dm/i2c.c index 74b2097195..b46a22e79b 100644 --- a/test/dm/i2c.c +++ b/test/dm/i2c.c @@ -124,7 +124,7 @@ static int dm_test_i2c_bytewise(struct unit_test_state *uts) ut_asserteq_mem(buf, "\0\0\0\0\0", sizeof(buf));

/* Tell the EEPROM to only read/write one register at a time */ - ut_assertok(uclass_first_device(UCLASS_I2C_EMUL, &eeprom)); + ut_assertok(uclass_first_device_err(UCLASS_I2C_EMUL, &eeprom)); ut_assertnonnull(eeprom); sandbox_i2c_eeprom_set_test_mode(eeprom, SIE_TEST_MODE_SINGLE_BYTE);

@@ -177,7 +177,7 @@ static int dm_test_i2c_offset(struct unit_test_state *uts)

/* Do a transfer so we can find the emulator */ ut_assertok(dm_i2c_read(dev, 0, buf, 5)); - ut_assertok(uclass_first_device(UCLASS_I2C_EMUL, &eeprom)); + ut_assertok(uclass_first_device_err(UCLASS_I2C_EMUL, &eeprom));

/* Offset length 0 */ sandbox_i2c_eeprom_set_offset_len(eeprom, 0); @@ -250,7 +250,7 @@ static int dm_test_i2c_addr_offset(struct unit_test_state *uts)

/* Do a transfer so we can find the emulator */ ut_assertok(dm_i2c_read(dev, 0, buf, 5)); - ut_assertok(uclass_first_device(UCLASS_I2C_EMUL, &eeprom)); + ut_assertok(uclass_first_device_err(UCLASS_I2C_EMUL, &eeprom));

/* Offset length 0 */ sandbox_i2c_eeprom_set_offset_len(eeprom, 0); @@ -315,7 +315,7 @@ static int dm_test_i2c_reg_clrset(struct unit_test_state *uts)

/* Do a transfer so we can find the emulator */ ut_assertok(dm_i2c_read(dev, 0, buf, 5)); - ut_assertok(uclass_first_device(UCLASS_I2C_EMUL, &eeprom)); + ut_assertok(uclass_first_device_err(UCLASS_I2C_EMUL, &eeprom));

/* Dummy data for the test */ ut_assertok(dm_i2c_write(dev, 0, "\xff\x00\xff\x00\x10", 5)); diff --git a/test/dm/virtio_device.c b/test/dm/virtio_device.c index d0195e6bf0..b5c4523a02 100644 --- a/test/dm/virtio_device.c +++ b/test/dm/virtio_device.c @@ -22,7 +22,7 @@ static int dm_test_virtio_base(struct unit_test_state *uts) u8 status;

/* check probe success */ - ut_assertok(uclass_first_device(UCLASS_VIRTIO, &bus)); + ut_assertok(uclass_first_device_err(UCLASS_VIRTIO, &bus)); ut_assertnonnull(bus);

/* check the child virtio-rng device is bound */ @@ -60,7 +60,7 @@ static int dm_test_virtio_all_ops(struct unit_test_state *uts) struct virtqueue *vqs[2];

/* check probe success */ - ut_assertok(uclass_first_device(UCLASS_VIRTIO, &bus)); + ut_assertok(uclass_first_device_err(UCLASS_VIRTIO, &bus)); ut_assertnonnull(bus);

/* check the child virtio-rng device is bound */ @@ -102,7 +102,7 @@ static int dm_test_virtio_remove(struct unit_test_state *uts) struct udevice *bus, *dev;

/* check probe success */ - ut_assertok(uclass_first_device(UCLASS_VIRTIO, &bus)); + ut_assertok(uclass_first_device_err(UCLASS_VIRTIO, &bus)); ut_assertnonnull(bus);

/* check the child virtio-rng device is bound */ @@ -134,7 +134,7 @@ static int dm_test_virtio_ring(struct unit_test_state *uts) u8 buffer[2][32];

/* check probe success */ - ut_assertok(uclass_first_device(UCLASS_VIRTIO, &bus)); + ut_assertok(uclass_first_device_err(UCLASS_VIRTIO, &bus)); ut_assertnonnull(bus);

/* check the child virtio-blk device is bound */ diff --git a/test/dm/virtio_rng.c b/test/dm/virtio_rng.c index ff5646b4e1..8b9a04b1fd 100644 --- a/test/dm/virtio_rng.c +++ b/test/dm/virtio_rng.c @@ -28,7 +28,7 @@ static int dm_test_virtio_rng_check_len(struct unit_test_state *uts) u8 buffer[16];

/* check probe success */ - ut_assertok(uclass_first_device(UCLASS_VIRTIO, &bus)); + ut_assertok(uclass_first_device_err(UCLASS_VIRTIO, &bus)); ut_assertnonnull(bus);

/* check the child virtio-rng device is bound */ diff --git a/test/fuzz/cmd_fuzz.c b/test/fuzz/cmd_fuzz.c index 0cc01dc199..e2f44f3ecb 100644 --- a/test/fuzz/cmd_fuzz.c +++ b/test/fuzz/cmd_fuzz.c @@ -29,7 +29,7 @@ static struct udevice *find_fuzzing_engine(void) { struct udevice *dev;

- if (uclass_first_device(UCLASS_FUZZING_ENGINE, &dev)) + if (uclass_first_device_err(UCLASS_FUZZING_ENGINE, &dev)) return NULL;

return dev; diff --git a/test/fuzz/virtio.c b/test/fuzz/virtio.c index e5363d5638..8a47667e77 100644 --- a/test/fuzz/virtio.c +++ b/test/fuzz/virtio.c @@ -30,7 +30,7 @@ static int fuzz_vring(const uint8_t *data, size_t size) return 0;

/* check probe success */ - if (uclass_first_device(UCLASS_VIRTIO, &bus) || !bus) + if (uclass_first_device_err(UCLASS_VIRTIO, &bus)) panic("Could not find virtio bus\n");

/* check the child virtio-rng device is bound */

There are a couple users of uclass_next_device return value that get the first device by other means and use uclass_next_device assuming the following device in the uclass is related to the first one.

Use uclass_next_device_err because the return value from uclass_next_device will be removed in a later patch.

Signed-off-by: Michal Suchanek msuchanek@suse.de Reviewed-by: Simon Glass sjg@chromium.org --- arch/arm/mach-k3/j721s2_init.c | 2 +- board/atmel/common/mac_eeprom.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-k3/j721s2_init.c b/arch/arm/mach-k3/j721s2_init.c index 12da8136f9..dd0c7ba18f 100644 --- a/arch/arm/mach-k3/j721s2_init.c +++ b/arch/arm/mach-k3/j721s2_init.c @@ -164,7 +164,7 @@ void board_init_f(ulong dummy) if (ret) panic("DRAM 0 init failed: %d\n", ret);

- ret = uclass_next_device(&dev); + ret = uclass_next_device_err(&dev); if (ret) panic("DRAM 1 init failed: %d\n", ret); } diff --git a/board/atmel/common/mac_eeprom.c b/board/atmel/common/mac_eeprom.c index a723ba723c..4606008c69 100644 --- a/board/atmel/common/mac_eeprom.c +++ b/board/atmel/common/mac_eeprom.c @@ -56,7 +56,7 @@ int at91_set_eth1addr(int offset) return ret;

/* attempt to obtain a second eeprom device */ - ret = uclass_next_device(&dev); + ret = uclass_next_device_err(&dev); if (ret) return ret;

blk_first_device_err/blk_next_device_err uses uclass_first_device_err/uclass_next_device_err for device iteration.

Although the function names superficially match the return value from uclass_first_device_err/uclass_next_device_err is never used meaningfully, and uclass_first_device/uclass_next_device works equally well for this purpose.

In the following patch the semantic of uclass_first_device_err/uclass_next_device_err will be changed to be based on uclass_first_device_check/uclass_next_device_check breaking this sole user that uses uclass_next_device_err for iteration.

Signed-off-by: Michal Suchanek msuchanek@suse.de Reviewed-by: Simon Glass sjg@chromium.org --- drivers/block/blk-uclass.c | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-)

diff --git a/drivers/block/blk-uclass.c b/drivers/block/blk-uclass.c index 7d12d5413f..bcc14a684b 100644 --- a/drivers/block/blk-uclass.c +++ b/drivers/block/blk-uclass.c @@ -593,11 +593,9 @@ int blk_find_next(enum blk_flag_t flags, struct udevice **devp)

int blk_first_device_err(enum blk_flag_t flags, struct udevice **devp) { - int ret; - - for (ret = uclass_first_device_err(UCLASS_BLK, devp); - !ret; - ret = uclass_next_device_err(devp)) { + for (uclass_first_device(UCLASS_BLK, devp); + *devp; + uclass_next_device(devp)) { if (!blk_flags_check(*devp, flags)) return 0; } @@ -607,11 +605,9 @@ int blk_first_device_err(enum blk_flag_t flags, struct udevice **devp)

int blk_next_device_err(enum blk_flag_t flags, struct udevice **devp) { - int ret; - - for (ret = uclass_next_device_err(devp); - !ret; - ret = uclass_next_device_err(devp)) { + for (uclass_next_device(devp); + *devp; + uclass_next_device(devp)) { if (!blk_flags_check(*devp, flags)) return 0; }

eth_get_dev relies on the broken behavior that returns an error but not the device on which the error happened which gives the caller no reasonable way to report or handle the error.

In a later patch uclass_first_device_err will be changed to return the device on error but eth_get_dev stores the returned device pointer directly in a global state without checking the return value. Unset the pointer again in the error case.

Signed-off-by: Michal Suchanek msuchanek@suse.de Reviewed-by: Simon Glass sjg@chromium.org --- v6: split off as separate patch --- net/eth-uclass.c | 2 ++ 1 file changed, 2 insertions(+)

diff --git a/net/eth-uclass.c b/net/eth-uclass.c index 8c3f9cc31b..f41da4b37b 100644 --- a/net/eth-uclass.c +++ b/net/eth-uclass.c @@ -93,6 +93,8 @@ struct udevice *eth_get_dev(void) if (eth_errno) eth_errno = uclass_first_device_err(UCLASS_ETH, &uc_priv->current); + if (eth_errno) + uc_priv->current = NULL; } return uc_priv->current; }

Check the returned value from pci_get_bus. After a later patch uclass_first_device_err may return a device on error and then checking that a device is returned will not suffice.

Signed-off-by: Michal Suchanek msuchanek@suse.de --- v6: - split off as separate patch --- drivers/pci/pci-uclass.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c index 5cff81ac44..6dd19650f9 100644 --- a/drivers/pci/pci-uclass.c +++ b/drivers/pci/pci-uclass.c @@ -1777,10 +1777,9 @@ int pci_sriov_init(struct udevice *pdev, int vf_en)

bdf = dm_pci_get_bdf(pdev);

- pci_get_bus(PCI_BUS(bdf), &bus); - - if (!bus) - return -ENODEV; + ret = pci_get_bus(PCI_BUS(bdf), &bus); + if (ret) + return ret;

bdf += PCI_BDF(0, 0, vf_offset);

In a later patch sysinfo_get will be changed to return the device in cae of an error. Set sysinfo to NULL on error to preserve previous behavior.

Signed-off-by: Michal Suchanek msuchanek@suse.de --- board/gdsys/mpc8308/gazerbeam.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/board/gdsys/mpc8308/gazerbeam.c b/board/gdsys/mpc8308/gazerbeam.c index 3d4a7e57fe..ba88401f13 100644 --- a/board/gdsys/mpc8308/gazerbeam.c +++ b/board/gdsys/mpc8308/gazerbeam.c @@ -49,8 +49,10 @@ int board_early_init_r(void) int mc = 0; int con = 0;

- if (sysinfo_get(&sysinfo)) + if (sysinfo_get(&sysinfo)) { puts("Could not find sysinfo information device.\n"); + sysinfo = NULL; + }

/* Initialize serdes */ uclass_get_device_by_phandle(UCLASS_MISC, sysinfo, "serdes", &serdes); @@ -92,8 +94,10 @@ int checksysinfo(void) int mc = 0; int con = 0;

- if (sysinfo_get(&sysinfo)) + if (sysinfo_get(&sysinfo)) { puts("Could not find sysinfo information device.\n"); + sysinfo = NULL; + }

sysinfo_get_int(sysinfo, BOARD_MULTICHANNEL, &mc); sysinfo_get_int(sysinfo, BOARD_VARIANT, &con); @@ -130,8 +134,10 @@ int last_stage_init(void) struct udevice *tpm; int ret;

- if (sysinfo_get(&sysinfo)) + if (sysinfo_get(&sysinfo)) { puts("Could not find sysinfo information device.\n"); + sysinfo = NULL; + }

if (sysinfo) { int res = sysinfo_get_int(sysinfo, BOARD_HWVERSION,