Hi Andrew,
On 7/15/25 11:23 AM, Andrew Goodbody wrote:
uclass_find_next_device always returns 0, so instead make it a void and
update calling sites.
Signed-off-by: Andrew Goodbody <[email protected]>
---
board/emulation/qemu-ppce500/qemu-ppce500.c | 4 ++--
boot/bootdev-uclass.c | 2 +-
cmd/regulator.c | 4 ++--
drivers/core/uclass.c | 16 +++++-----------
drivers/power/regulator/regulator-uclass.c | 10 ++++++----
drivers/remoteproc/rproc-uclass.c | 6 ++++--
include/dm/uclass-internal.h | 4 +---
test/dm/core.c | 28 ++++++++++++++--------------
8 files changed, 35 insertions(+), 39 deletions(-)
diff --git a/board/emulation/qemu-ppce500/qemu-ppce500.c
b/board/emulation/qemu-ppce500/qemu-ppce500.c
index 40d295dbf06..58de4a05296 100644
--- a/board/emulation/qemu-ppce500/qemu-ppce500.c
+++ b/board/emulation/qemu-ppce500/qemu-ppce500.c
@@ -170,9 +170,9 @@ int misc_init_r(void)
* Detect the presence of the platform bus node, and
* create a virtual memory mapping for it.
*/
- for (ret = uclass_find_first_device(UCLASS_SIMPLE_BUS, &dev);
+ for (uclass_find_first_device(UCLASS_SIMPLE_BUS, &dev);
dev;
- ret = uclass_find_next_device(&dev)) {
+ uclass_find_next_device(&dev)) {
if (device_is_compatible(dev, "qemu,platform")) {
struct simple_bus_plat *plat = dev_get_uclass_plat(dev);
diff --git a/boot/bootdev-uclass.c b/boot/bootdev-uclass.c
index 3791ebfcb42..6718c482e52 100644
--- a/boot/bootdev-uclass.c
+++ b/boot/bootdev-uclass.c
@@ -216,7 +216,7 @@ void bootdev_list(bool probe)
if (probe)
ret = uclass_next_device_check(&dev);
else
- ret = uclass_find_next_device(&dev);
+ uclass_find_next_device(&dev);
I think we need to ret = 0 here as well? Otherwise the printf at the top
of the for-loop will return whatever was the last return value?
}
printf("--- ------ ------ -------- ------------------\n");
printf("(%d bootdev%s)\n", i, i != 1 ? "s" : "");
diff --git a/cmd/regulator.c b/cmd/regulator.c
index da298090bb7..db843283662 100644
--- a/cmd/regulator.c
+++ b/cmd/regulator.c
@@ -97,9 +97,9 @@ static int do_list(struct cmd_tbl *cmdtp, int flag, int argc,
"Parent");
for (ret = uclass_find_first_device(UCLASS_REGULATOR, &dev); dev;
- ret = uclass_find_next_device(&dev)) {
+ uclass_find_next_device(&dev)) {
if (ret)
- continue;
+ break;
I think it would make this more readable by having
ret = uclass_find_first_device(UCLASS_REGULATOR, &dev);
if (ret)
return ret;
for (; dev; uclass_find_next_device(&dev)) {
uc_pdata...
}
return ret;
What do you think? This also avoids checking for ret in every loop
considering it won't ever change.
[...]
diff --git a/drivers/power/regulator/regulator-uclass.c
b/drivers/power/regulator/regulator-uclass.c
index 09567eb9dbb..eac025c9c02 100644
--- a/drivers/power/regulator/regulator-uclass.c
+++ b/drivers/power/regulator/regulator-uclass.c
@@ -261,10 +261,10 @@ int regulator_get_by_platname(const char *plat_name,
struct udevice **devp)
*devp = NULL;
for (ret = uclass_find_first_device(UCLASS_REGULATOR, &dev); dev;
- ret = uclass_find_next_device(&dev)) {
+ uclass_find_next_device(&dev)) {
if (ret) {
dev_dbg(dev, "ret=%d\n", ret);
- continue;
+ break;
}
Same here.
uc_pdata = dev_get_uclass_plat(dev);
@@ -411,8 +411,10 @@ static bool regulator_name_is_unique(struct udevice
*check_dev,
int len;
for (ret = uclass_find_first_device(UCLASS_REGULATOR, &dev); dev;
- ret = uclass_find_next_device(&dev)) {
- if (ret || dev == check_dev)
+ uclass_find_next_device(&dev)) {
+ if (ret)
+ break;
Ditto.
+ if (dev == check_dev)
continue;
uc_pdata = dev_get_uclass_plat(dev);
diff --git a/drivers/remoteproc/rproc-uclass.c
b/drivers/remoteproc/rproc-uclass.c
index 3233ff80419..7916cb27e3d 100644
--- a/drivers/remoteproc/rproc-uclass.c
+++ b/drivers/remoteproc/rproc-uclass.c
@@ -56,8 +56,10 @@ static int for_each_remoteproc_device(int (*fn) (struct
udevice *dev,
int ret;
for (ret = uclass_find_first_device(UCLASS_REMOTEPROC, &dev); dev;
- ret = uclass_find_next_device(&dev)) {
- if (ret || dev == skip_dev)
+ uclass_find_next_device(&dev)) {
+ if (ret)
+ return ret;
Ditto.
+ if (dev == skip_dev)
continue;
uc_pdata = dev_get_uclass_plat(dev);
ret = fn(dev, uc_pdata, data);
diff --git a/include/dm/uclass-internal.h b/include/dm/uclass-internal.h
index 3ddcdd21439..9cb3f090271 100644
--- a/include/dm/uclass-internal.h
+++ b/include/dm/uclass-internal.h
@@ -149,10 +149,8 @@ int uclass_find_first_device(enum uclass_id id, struct
udevice **devp);
*
* The device is not prepared for use - this is an internal function.
* The function uclass_get_device_tail() can be used to probe the device.
- *
- * Return: 0 if OK (found or not found), -ve on error
*/
-int uclass_find_next_device(struct udevice **devp);
+void uclass_find_next_device(struct udevice **devp);
/**
* uclass_find_device_by_namelen() - Find uclass device based on ID and name
diff --git a/test/dm/core.c b/test/dm/core.c
index 959b834576f..201daee6836 100644
--- a/test/dm/core.c
+++ b/test/dm/core.c
@@ -737,7 +737,7 @@ static int dm_test_device_reparent(struct unit_test_state
*uts)
/* try to get devices */
for (ret = uclass_find_first_device(UCLASS_TEST, &dev);
dev;
- ret = uclass_find_next_device(&dev)) {
+ uclass_find_next_device(&dev)) {
ut_assert(!ret);
Ditto for all the tests in test/dm/core.c.
Cheers,
Quentin
