On 15/07/2025 13:06, Quentin Schulz wrote:
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?
Yes, will fix.
}
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.
OK, will change as suggested.
[...]
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.
The for loops in test/dm/core.c mostly reduce to being empty as the only
test left inside is that dev is not NULL, but in that case the for loop
terminates so the test can never fail. So the for loops can mostly be
removed as they do nothing.
Thanks,
Andrew
Cheers,
Quentin
