[PATCH v2 2/2] uclass: Cleanup uclass_find_next_device

[Quentin Schulz]

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 <andrew.goodbody at linaro.org>
> ---
>   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