[PATCH] block: Remove blk_find_first/next

[PATCH] block: Remove blk_find_first/next

[Greg Malysa]

In [0], Andrew noted a code quality issue in the implementation of
blk_find_first and blk_find_next. This led to the observation that the
logic of these functions was also likely incorrect, and based on a quick
check it seemed the functions were unused outside of test code, which
did not exercise the potential failure case, so we felt they should be
removed. In [1], a test patch which illustrates the failure in sandbox
is provided for reference.

Because a more thorough check agrees that these functions are unused,
they are currently incorrect, and fixed/removable flags on block devices
prior to probe are unreliable, just remove these functions instead of
fixing them. All potential users should have used blk_first_device_err
instead anyway.

CI results at [2].

[0] https://patchwork.ozlabs.org/project/uboot/patch/20250714-blk-uclass-v1-1-d21428c5f762@linaro.org/
[1] https://gist.github.com/gmalysa/b05e73a5c14bc18c5741a0e0e06a2992
[2] https://gitlab.com/gmalysa/lnxdsp-u-boot/-/pipelines/1931210857

Signed-off-by: Greg Malysa <malysagreg@gmail.com>

---

 drivers/block/blk-uclass.c | 24 --------------------
 include/blk.h              | 45 -------------------------------------
 test/dm/blk.c              | 46 +++-----------------------------------
 3 files changed, 3 insertions(+), 112 deletions(-)

diff --git a/drivers/block/blk-uclass.c b/drivers/block/blk-uclass.c
index f3ac8db9464..73c24fd9176 100644
--- a/drivers/block/blk-uclass.c
+++ b/drivers/block/blk-uclass.c
@@ -611,30 +611,6 @@ static int blk_flags_check(struct udevice *dev, enum blk_flag_t req_flags)
 	return flags & req_flags ? 0 : 1;
 }
 
-int blk_find_first(enum blk_flag_t flags, struct udevice **devp)
-{
-	int ret;
-
-	for (ret = uclass_find_first_device(UCLASS_BLK, devp);
-	     *devp && !blk_flags_check(*devp, flags);
-	     ret = uclass_find_next_device(devp))
-		return 0;
-
-	return -ENODEV;
-}
-
-int blk_find_next(enum blk_flag_t flags, struct udevice **devp)
-{
-	int ret;
-
-	for (ret = uclass_find_next_device(devp);
-	     *devp && !blk_flags_check(*devp, flags);
-	     ret = uclass_find_next_device(devp))
-		return 0;
-
-	return -ENODEV;
-}
-
 int blk_first_device_err(enum blk_flag_t flags, struct udevice **devp)
 {
 	for (uclass_first_device(UCLASS_BLK, devp);
diff --git a/include/blk.h b/include/blk.h
index 488d04cf32a..8d1b70cabd3 100644
--- a/include/blk.h
+++ b/include/blk.h
@@ -781,51 +781,6 @@ 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);
 
-/**
- * blk_find_first() - Return the first matching block device
- * @flags: Indicates type of device to return
- * @devp:	Returns pointer to device, or NULL on error
- *
- * 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.
- *
- * Note that some devices are considered removable until they have been probed
- *
- * @return 0 if found, -ENODEV if not found
- */
-int blk_find_first(enum blk_flag_t flags, struct udevice **devp);
-
-/**
- * blk_find_next() - Return the next matching block device
- * @flags: Indicates type of device to return
- * @devp: On entry, pointer to device to lookup. On exit, returns pointer
- * to the next device in the same uclass, or NULL if none
- *
- * 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.
- *
- * Note that some devices are considered removable until they have been probed
- *
- * @return 0 if found, -ENODEV if not found
- */
-int blk_find_next(enum blk_flag_t flags, struct udevice **devp);
-
-/**
- * blk_foreach() - iterate through block devices
- *
- * This creates a for() loop which works through the available block devices in
- * order from start to end.
- *
- * If for some reason the uclass cannot be found, this does nothing.
- *
- * @_flags: Indicates type of device to return
- * @_pos: struct udevice * to hold the current device. Set to NULL when there
- * are no more devices.
- */
-#define blk_foreach(_flags, _pos) \
-	for (int _ret = blk_find_first(_flags, &_pos); !_ret && _pos; \
-	     _ret = blk_find_next(_flags, &_pos))
-
 /**
  * blk_foreach_probe() - Helper function to iteration through block devices
  *
diff --git a/test/dm/blk.c b/test/dm/blk.c
index aa5cbc63777..1b928b27d9c 100644
--- a/test/dm/blk.c
+++ b/test/dm/blk.c
@@ -229,30 +229,7 @@ static int dm_test_blk_flags(struct unit_test_state *uts)
 {
 	struct udevice *dev;
 
-	/* Iterate through devices without probing them */
-	ut_assertok(blk_find_first(BLKF_BOTH, &dev));
-	ut_assertnonnull(dev);
-	ut_asserteq_str("mmc2.blk", dev->name);
-
-	ut_assertok(blk_find_next(BLKF_BOTH, &dev));
-	ut_assertnonnull(dev);
-	ut_asserteq_str("mmc1.blk", dev->name);
-
-	ut_assertok(blk_find_next(BLKF_BOTH, &dev));
-	ut_assertnonnull(dev);
-	ut_asserteq_str("mmc0.blk", dev->name);
-
-	ut_asserteq(-ENODEV, blk_find_next(BLKF_BOTH, &dev));
-	ut_assertnull(dev);
-
-	/* All devices are removable until probed */
-	ut_asserteq(-ENODEV, blk_find_first(BLKF_FIXED, &dev));
-
-	ut_assertok(blk_find_first(BLKF_REMOVABLE, &dev));
-	ut_assertnonnull(dev);
-	ut_asserteq_str("mmc2.blk", dev->name);
-
-	/* Now probe them and iterate again */
+	/* Probe and look through block devices */
 	ut_assertok(blk_first_device_err(BLKF_BOTH, &dev));
 	ut_assertnonnull(dev);
 	ut_asserteq_str("mmc2.blk", dev->name);
@@ -289,30 +266,13 @@ static int dm_test_blk_flags(struct unit_test_state *uts)
 }
 DM_TEST(dm_test_blk_flags, UTF_SCAN_PDATA | UTF_SCAN_FDT);
 
-/* Test blk_foreach() and friend */
+/* Test blk_foreach_probe() */
 static int dm_test_blk_foreach(struct unit_test_state *uts)
 {
 	struct udevice *dev;
 	int found;
 
-	/* Test blk_foreach() - use the 3rd bytes of the name (0/1/2) */
-	found = 0;
-	blk_foreach(BLKF_BOTH, dev)
-		found |= 1 << dectoul(&dev->name[3], NULL);
-	ut_asserteq(7, found);
-
-	/* All devices are removable until probed */
-	found = 0;
-	blk_foreach(BLKF_FIXED, dev)
-		found |= 1 << dectoul(&dev->name[3], NULL);
-	ut_asserteq(0, found);
-
-	found = 0;
-	blk_foreach(BLKF_REMOVABLE, dev)
-		found |= 1 << dectoul(&dev->name[3], NULL);
-	ut_asserteq(7, found);
-
-	/* Now try again with the probing functions */
+	/* The test device tree has two fixed and one removable block device(s) */
 	found = 0;
 	blk_foreach_probe(BLKF_BOTH, dev)
 		found |= 1 << dectoul(&dev->name[3], NULL);
-- 
2.45.2

base-commit: 3b4604a40b9fd61b87e9d059fc56f04d36f1a380
branch: uboot-master

Re: [PATCH] block: Remove blk_find_first/next

[Andrew Goodbody]

On 17/07/2025 11:19, Greg Malysa wrote:
> In [0], Andrew noted a code quality issue in the implementation of
> blk_find_first and blk_find_next. This led to the observation that the
> logic of these functions was also likely incorrect, and based on a quick
> check it seemed the functions were unused outside of test code, which
> did not exercise the potential failure case, so we felt they should be
> removed. In [1], a test patch which illustrates the failure in sandbox
> is provided for reference.
> 
> Because a more thorough check agrees that these functions are unused,
> they are currently incorrect, and fixed/removable flags on block devices
> prior to probe are unreliable, just remove these functions instead of
> fixing them. All potential users should have used blk_first_device_err
> instead anyway.
> 
> CI results at [2].
> 
> [0] https://patchwork.ozlabs.org/project/uboot/patch/20250714-blk-uclass-v1-1-d21428c5f762@linaro.org/
> [1] https://gist.github.com/gmalysa/b05e73a5c14bc18c5741a0e0e06a2992
> [2] https://gitlab.com/gmalysa/lnxdsp-u-boot/-/pipelines/1931210857
> 
> Signed-off-by: Greg Malysa <malysagreg@gmail.com>
> 
> ---
> 
>   drivers/block/blk-uclass.c | 24 --------------------
>   include/blk.h              | 45 -------------------------------------
>   test/dm/blk.c              | 46 +++-----------------------------------
>   3 files changed, 3 insertions(+), 112 deletions(-)
> 
> diff --git a/drivers/block/blk-uclass.c b/drivers/block/blk-uclass.c
> index f3ac8db9464..73c24fd9176 100644
> --- a/drivers/block/blk-uclass.c
> +++ b/drivers/block/blk-uclass.c
> @@ -611,30 +611,6 @@ static int blk_flags_check(struct udevice *dev, enum blk_flag_t req_flags)
>   	return flags & req_flags ? 0 : 1;
>   }
>   
> -int blk_find_first(enum blk_flag_t flags, struct udevice **devp)
> -{
> -	int ret;
> -
> -	for (ret = uclass_find_first_device(UCLASS_BLK, devp);
> -	     *devp && !blk_flags_check(*devp, flags);
> -	     ret = uclass_find_next_device(devp))
> -		return 0;
> -
> -	return -ENODEV;
> -}
> -
> -int blk_find_next(enum blk_flag_t flags, struct udevice **devp)
> -{
> -	int ret;
> -
> -	for (ret = uclass_find_next_device(devp);
> -	     *devp && !blk_flags_check(*devp, flags);
> -	     ret = uclass_find_next_device(devp))
> -		return 0;
> -
> -	return -ENODEV;
> -}
> -
>   int blk_first_device_err(enum blk_flag_t flags, struct udevice **devp)
>   {
>   	for (uclass_first_device(UCLASS_BLK, devp);
> diff --git a/include/blk.h b/include/blk.h
> index 488d04cf32a..8d1b70cabd3 100644
> --- a/include/blk.h
> +++ b/include/blk.h
> @@ -781,51 +781,6 @@ 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);
>   
> -/**
> - * blk_find_first() - Return the first matching block device
> - * @flags: Indicates type of device to return
> - * @devp:	Returns pointer to device, or NULL on error
> - *
> - * 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.
> - *
> - * Note that some devices are considered removable until they have been probed
> - *
> - * @return 0 if found, -ENODEV if not found
> - */
> -int blk_find_first(enum blk_flag_t flags, struct udevice **devp);
> -
> -/**
> - * blk_find_next() - Return the next matching block device
> - * @flags: Indicates type of device to return
> - * @devp: On entry, pointer to device to lookup. On exit, returns pointer
> - * to the next device in the same uclass, or NULL if none
> - *
> - * 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.
> - *
> - * Note that some devices are considered removable until they have been probed
> - *
> - * @return 0 if found, -ENODEV if not found
> - */
> -int blk_find_next(enum blk_flag_t flags, struct udevice **devp);
> -
> -/**
> - * blk_foreach() - iterate through block devices
> - *
> - * This creates a for() loop which works through the available block devices in
> - * order from start to end.
> - *
> - * If for some reason the uclass cannot be found, this does nothing.
> - *
> - * @_flags: Indicates type of device to return
> - * @_pos: struct udevice * to hold the current device. Set to NULL when there
> - * are no more devices.
> - */
> -#define blk_foreach(_flags, _pos) \
> -	for (int _ret = blk_find_first(_flags, &_pos); !_ret && _pos; \
> -	     _ret = blk_find_next(_flags, &_pos))
> -
>   /**
>    * blk_foreach_probe() - Helper function to iteration through block devices
>    *
> diff --git a/test/dm/blk.c b/test/dm/blk.c
> index aa5cbc63777..1b928b27d9c 100644
> --- a/test/dm/blk.c
> +++ b/test/dm/blk.c
> @@ -229,30 +229,7 @@ static int dm_test_blk_flags(struct unit_test_state *uts)
>   {
>   	struct udevice *dev;
>   
> -	/* Iterate through devices without probing them */
> -	ut_assertok(blk_find_first(BLKF_BOTH, &dev));
> -	ut_assertnonnull(dev);
> -	ut_asserteq_str("mmc2.blk", dev->name);
> -
> -	ut_assertok(blk_find_next(BLKF_BOTH, &dev));
> -	ut_assertnonnull(dev);
> -	ut_asserteq_str("mmc1.blk", dev->name);
> -
> -	ut_assertok(blk_find_next(BLKF_BOTH, &dev));
> -	ut_assertnonnull(dev);
> -	ut_asserteq_str("mmc0.blk", dev->name);
> -
> -	ut_asserteq(-ENODEV, blk_find_next(BLKF_BOTH, &dev));
> -	ut_assertnull(dev);
> -
> -	/* All devices are removable until probed */
> -	ut_asserteq(-ENODEV, blk_find_first(BLKF_FIXED, &dev));
> -
> -	ut_assertok(blk_find_first(BLKF_REMOVABLE, &dev));
> -	ut_assertnonnull(dev);
> -	ut_asserteq_str("mmc2.blk", dev->name);
> -
> -	/* Now probe them and iterate again */
> +	/* Probe and look through block devices */
>   	ut_assertok(blk_first_device_err(BLKF_BOTH, &dev));
>   	ut_assertnonnull(dev);
>   	ut_asserteq_str("mmc2.blk", dev->name);
> @@ -289,30 +266,13 @@ static int dm_test_blk_flags(struct unit_test_state *uts)
>   }
>   DM_TEST(dm_test_blk_flags, UTF_SCAN_PDATA | UTF_SCAN_FDT);
>   
> -/* Test blk_foreach() and friend */
> +/* Test blk_foreach_probe() */
>   static int dm_test_blk_foreach(struct unit_test_state *uts)
>   {
>   	struct udevice *dev;
>   	int found;
>   
> -	/* Test blk_foreach() - use the 3rd bytes of the name (0/1/2) */
> -	found = 0;
> -	blk_foreach(BLKF_BOTH, dev)
> -		found |= 1 << dectoul(&dev->name[3], NULL);
> -	ut_asserteq(7, found);
> -
> -	/* All devices are removable until probed */
> -	found = 0;
> -	blk_foreach(BLKF_FIXED, dev)
> -		found |= 1 << dectoul(&dev->name[3], NULL);
> -	ut_asserteq(0, found);
> -
> -	found = 0;
> -	blk_foreach(BLKF_REMOVABLE, dev)
> -		found |= 1 << dectoul(&dev->name[3], NULL);
> -	ut_asserteq(7, found);
> -
> -	/* Now try again with the probing functions */
> +	/* The test device tree has two fixed and one removable block device(s) */
>   	found = 0;
>   	blk_foreach_probe(BLKF_BOTH, dev)
>   		found |= 1 << dectoul(&dev->name[3], NULL);

Reviewed-by: Andrew Goodbody <andrew.goodbody@linaro.org>

Re: [PATCH] block: Remove blk_find_first/next

[Tom Rini]

On Thu, 17 Jul 2025 06:19:01 -0400, Greg Malysa wrote:

> In [0], Andrew noted a code quality issue in the implementation of
> blk_find_first and blk_find_next. This led to the observation that the
> logic of these functions was also likely incorrect, and based on a quick
> check it seemed the functions were unused outside of test code, which
> did not exercise the potential failure case, so we felt they should be
> removed. In [1], a test patch which illustrates the failure in sandbox
> is provided for reference.
> 
> [...]

Applied to u-boot/master, thanks!

[1/1] block: Remove blk_find_first/next
      commit: 3532f1f5edfc97c9dcea723cdeb732eda44bc669
-- 
Tom