michael/u-boot
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

block: Remove blk_find_first/next

Browse Source 
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>
Reviewed-by: Andrew Goodbody <andrew.goodbody@linaro.org>
This commit is contained in:
Greg Malysa
2025-07-17 06:19:01 -04:00
committed by Tom Rini
parent cf13f33d3a
commit 3532f1f5ed
3 changed files with 3 additions and 112 deletions
Download Patch File Download Diff File Expand all files Collapse all files

24
drivers/block/blk-uclass.c
View File

@@ -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);

45
include/blk.h
View File

@@ -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
*

46
test/dm/blk.c
View File

@@ -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);