From: Greg Malysa <malysagreg@gmail.com>
To: u-boot@lists.denx.de
Cc: Andrew Goodbody <andrew.goodbody@linaro.org>,
Greg Malysa <malysagreg@gmail.com>,
Alexey Romanov <avromanov@salutedevices.com>,
Heiko Schocher <hs@denx.de>,
Heinrich Schuchardt <xypron.glpk@gmx.de>,
Ilias Apalodimas <ilias.apalodimas@linaro.org>,
Michael Trimarchi <michael@amarulasolutions.com>,
Quentin Schulz <quentin.schulz@cherry.de>,
Sean Anderson <seanga2@gmail.com>, Simon Glass <sjg@chromium.org>,
Tom Rini <trini@konsulko.com>
Subject: [PATCH] block: Remove blk_find_first/next
Date: Thu, 17 Jul 2025 06:19:01 -0400 [thread overview]
Message-ID: <20250717102012.10018-1-malysagreg@gmail.com> (raw)
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
next reply other threads:[~2025-07-17 10:23 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-07-17 10:19 Greg Malysa [this message]
2025-07-17 11:15 ` [PATCH] block: Remove blk_find_first/next Andrew Goodbody
2025-07-24 1:37 ` Tom Rini
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20250717102012.10018-1-malysagreg@gmail.com \
--to=malysagreg@gmail.com \
--cc=andrew.goodbody@linaro.org \
--cc=avromanov@salutedevices.com \
--cc=hs@denx.de \
--cc=ilias.apalodimas@linaro.org \
--cc=michael@amarulasolutions.com \
--cc=quentin.schulz@cherry.de \
--cc=seanga2@gmail.com \
--cc=sjg@chromium.org \
--cc=trini@konsulko.com \
--cc=u-boot@lists.denx.de \
--cc=xypron.glpk@gmx.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.