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