* [PATCH 01/15] blk: Use a macro for the typical block size
2023-09-26 8:43 [PATCH 00/15] blk: sandbox: Support binding a device with a given logical block size Bin Meng
@ 2023-09-26 8:43 ` Bin Meng
2023-10-02 1:16 ` Simon Glass
2023-09-26 8:43 ` [PATCH 02/15] cmd: host: Mandate the filename parameter in the 'bind' command Bin Meng
` (15 subsequent siblings)
16 siblings, 1 reply; 38+ messages in thread
From: Bin Meng @ 2023-09-26 8:43 UTC (permalink / raw)
To: Simon Glass, U-Boot Mailing List
Cc: Bin Meng, Heinrich Schuchardt, Jaehoon Chung, Johan Jonker,
Marek Vasut, Mattijs Korpershoek, Peng Fan, Tobias Waldekranz
Avoid using the magic number 512 directly.
Signed-off-by: Bin Meng <bmeng@tinylab.org>
---
common/usb_storage.c | 4 ++--
drivers/ata/dwc_ahsata.c | 3 ++-
drivers/ata/fsl_sata.c | 3 ++-
drivers/ata/sata_mv.c | 3 ++-
drivers/ata/sata_sil.c | 3 ++-
drivers/block/blkmap.c | 2 +-
drivers/block/host_dev.c | 2 +-
drivers/mmc/mmc-uclass.c | 2 +-
drivers/nvme/nvme.c | 2 +-
include/blk.h | 2 ++
10 files changed, 16 insertions(+), 10 deletions(-)
diff --git a/common/usb_storage.c b/common/usb_storage.c
index 85774220ef..35c656db0d 100644
--- a/common/usb_storage.c
+++ b/common/usb_storage.c
@@ -219,8 +219,8 @@ static int usb_stor_probe_device(struct usb_device *udev)
snprintf(str, sizeof(str), "lun%d", lun);
ret = blk_create_devicef(udev->dev, "usb_storage_blk", str,
- UCLASS_USB, usb_max_devs, 512, 0,
- &dev);
+ UCLASS_USB, usb_max_devs,
+ DEFAULT_BLKSZ, 0, &dev);
if (ret) {
debug("Cannot bind driver\n");
return ret;
diff --git a/drivers/ata/dwc_ahsata.c b/drivers/ata/dwc_ahsata.c
index 6a4d861bf1..b4d4e39c9b 100644
--- a/drivers/ata/dwc_ahsata.c
+++ b/drivers/ata/dwc_ahsata.c
@@ -880,7 +880,8 @@ int dwc_ahsata_scan(struct udevice *dev)
device_find_first_child(dev, &blk);
if (!blk) {
ret = blk_create_devicef(dev, "dwc_ahsata_blk", "blk",
- UCLASS_AHCI, -1, 512, 0, &blk);
+ UCLASS_AHCI, -1, DEFAULT_BLKSZ,
+ 0, &blk);
if (ret) {
debug("Can't create device\n");
return ret;
diff --git a/drivers/ata/fsl_sata.c b/drivers/ata/fsl_sata.c
index 972101b29c..969bc191f8 100644
--- a/drivers/ata/fsl_sata.c
+++ b/drivers/ata/fsl_sata.c
@@ -888,7 +888,8 @@ static int fsl_ata_probe(struct udevice *dev)
for (i = 0; i < nr_ports; i++) {
snprintf(sata_name, sizeof(sata_name), "fsl_sata%d", i);
ret = blk_create_devicef(dev, "sata_fsl_blk", sata_name,
- UCLASS_AHCI, -1, 512, 0, &blk);
+ UCLASS_AHCI, -1, DEFAULT_BLKSZ,
+ 0, &blk);
if (ret) {
debug("Can't create device\n");
return ret;
diff --git a/drivers/ata/sata_mv.c b/drivers/ata/sata_mv.c
index 18c7a66db1..1abea0b309 100644
--- a/drivers/ata/sata_mv.c
+++ b/drivers/ata/sata_mv.c
@@ -1076,7 +1076,8 @@ static int sata_mv_probe(struct udevice *dev)
for (i = 0; i < nr_ports; i++) {
ret = blk_create_devicef(dev, "sata_mv_blk", "blk",
- UCLASS_AHCI, -1, 512, 0, &blk);
+ UCLASS_AHCI, -1, DEFAULT_BLKSZ,
+ 0, &blk);
if (ret) {
debug("Can't create device\n");
continue;
diff --git a/drivers/ata/sata_sil.c b/drivers/ata/sata_sil.c
index b5e150d568..43a91a7912 100644
--- a/drivers/ata/sata_sil.c
+++ b/drivers/ata/sata_sil.c
@@ -730,7 +730,8 @@ static int sil_pci_probe(struct udevice *dev)
for (i = sata_info.portbase; i < sata_info.maxport; i++) {
snprintf(sata_name, sizeof(sata_name), "sil_sata%d", i);
ret = blk_create_devicef(dev, "sata_sil_blk", sata_name,
- UCLASS_AHCI, -1, 512, 0, &blk);
+ UCLASS_AHCI, -1, DEFAULT_BLKSZ,
+ 0, &blk);
if (ret) {
debug("Can't create device\n");
return ret;
diff --git a/drivers/block/blkmap.c b/drivers/block/blkmap.c
index 2bb0acc20f..409aa46de2 100644
--- a/drivers/block/blkmap.c
+++ b/drivers/block/blkmap.c
@@ -390,7 +390,7 @@ int blkmap_dev_bind(struct udevice *dev)
int err;
err = blk_create_devicef(dev, "blkmap_blk", "blk", UCLASS_BLKMAP,
- dev_seq(dev), 512, 0, &bm->blk);
+ dev_seq(dev), DEFAULT_BLKSZ, 0, &bm->blk);
if (err)
return log_msg_ret("blk", err);
diff --git a/drivers/block/host_dev.c b/drivers/block/host_dev.c
index 64422417b7..31c7814054 100644
--- a/drivers/block/host_dev.c
+++ b/drivers/block/host_dev.c
@@ -105,7 +105,7 @@ static int host_sb_bind(struct udevice *dev)
int ret;
ret = blk_create_devicef(dev, "sandbox_host_blk", "blk", UCLASS_HOST,
- dev_seq(dev), 512, 0, &blk);
+ dev_seq(dev), DEFAULT_BLKSZ, 0, &blk);
if (ret)
return log_msg_ret("blk", ret);
diff --git a/drivers/mmc/mmc-uclass.c b/drivers/mmc/mmc-uclass.c
index 0e157672ea..328456831d 100644
--- a/drivers/mmc/mmc-uclass.c
+++ b/drivers/mmc/mmc-uclass.c
@@ -412,7 +412,7 @@ int mmc_bind(struct udevice *dev, struct mmc *mmc, const struct mmc_config *cfg)
debug("%s: alias devnum=%d\n", __func__, dev_seq(dev));
ret = blk_create_devicef(dev, "mmc_blk", "blk", UCLASS_MMC,
- dev_seq(dev), 512, 0, &bdev);
+ dev_seq(dev), DEFAULT_BLKSZ, 0, &bdev);
if (ret) {
debug("Cannot create block device\n");
return ret;
diff --git a/drivers/nvme/nvme.c b/drivers/nvme/nvme.c
index 20dc910d8a..c39cd41aa3 100644
--- a/drivers/nvme/nvme.c
+++ b/drivers/nvme/nvme.c
@@ -906,7 +906,7 @@ int nvme_init(struct udevice *udev)
/* The real blksz and size will be set by nvme_blk_probe() */
ret = blk_create_devicef(udev, "nvme-blk", name, UCLASS_NVME,
- -1, 512, 0, &ns_udev);
+ -1, DEFAULT_BLKSZ, 0, &ns_udev);
if (ret)
goto free_id;
diff --git a/include/blk.h b/include/blk.h
index 2c9c7985a8..4a4365fbbf 100644
--- a/include/blk.h
+++ b/include/blk.h
@@ -20,6 +20,8 @@ typedef ulong lbaint_t;
#define LBAF "%" LBAFlength "x"
#define LBAFU "%" LBAFlength "u"
+#define DEFAULT_BLKSZ 512
+
struct udevice;
static inline bool blk_enabled(void)
--
2.25.1
^ permalink raw reply related [flat|nested] 38+ messages in thread* Re: [PATCH 01/15] blk: Use a macro for the typical block size
2023-09-26 8:43 ` [PATCH 01/15] blk: Use a macro for the typical " Bin Meng
@ 2023-10-02 1:16 ` Simon Glass
0 siblings, 0 replies; 38+ messages in thread
From: Simon Glass @ 2023-10-02 1:16 UTC (permalink / raw)
To: Bin Meng
Cc: U-Boot Mailing List, Bin Meng, Heinrich Schuchardt, Jaehoon Chung,
Johan Jonker, Marek Vasut, Mattijs Korpershoek, Peng Fan,
Tobias Waldekranz
On Tue, 26 Sept 2023 at 02:45, Bin Meng <bmeng@tinylab.org> wrote:
>
> Avoid using the magic number 512 directly.
>
> Signed-off-by: Bin Meng <bmeng@tinylab.org>
> ---
>
> common/usb_storage.c | 4 ++--
> drivers/ata/dwc_ahsata.c | 3 ++-
> drivers/ata/fsl_sata.c | 3 ++-
> drivers/ata/sata_mv.c | 3 ++-
> drivers/ata/sata_sil.c | 3 ++-
> drivers/block/blkmap.c | 2 +-
> drivers/block/host_dev.c | 2 +-
> drivers/mmc/mmc-uclass.c | 2 +-
> drivers/nvme/nvme.c | 2 +-
> include/blk.h | 2 ++
> 10 files changed, 16 insertions(+), 10 deletions(-)
Reviewed-by: Simon Glass <sjg@chromium.org>
^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH 02/15] cmd: host: Mandate the filename parameter in the 'bind' command
2023-09-26 8:43 [PATCH 00/15] blk: sandbox: Support binding a device with a given logical block size Bin Meng
2023-09-26 8:43 ` [PATCH 01/15] blk: Use a macro for the typical " Bin Meng
@ 2023-09-26 8:43 ` Bin Meng
2023-10-02 1:17 ` Simon Glass
2023-09-26 8:43 ` [PATCH 03/15] blk: sandbox: Support binding a device with a given logical block size Bin Meng
` (14 subsequent siblings)
16 siblings, 1 reply; 38+ messages in thread
From: Bin Meng @ 2023-09-26 8:43 UTC (permalink / raw)
To: Simon Glass, U-Boot Mailing List
At present the host bind command does not require filename to be
provided. When it is not given NULL is passed to the host device
driver, which ends up failure afterwards.
Change to mandate the filename so that it is useful.
Signed-off-by: Bin Meng <bmeng@tinylab.org>
---
cmd/host.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/cmd/host.c b/cmd/host.c
index fb1cb1fdd1..b924940ffb 100644
--- a/cmd/host.c
+++ b/cmd/host.c
@@ -59,10 +59,10 @@ static int do_host_bind(struct cmd_tbl *cmdtp, int flag, int argc,
argv++;
}
- if (argc > 2)
+ if (argc != 2)
return CMD_RET_USAGE;
label = argv[0];
- file = argc > 1 ? argv[1] : NULL;
+ file = argv[1];
ret = host_create_attach_file(label, file, removable, &dev);
if (ret) {
@@ -253,7 +253,7 @@ U_BOOT_CMD(
"host save hostfs - <addr> <filename> <bytes> [<offset>] - "
"save a file to host\n"
"host size hostfs - <filename> - determine size of file on host\n"
- "host bind [-r] <label> [<filename>] - bind \"host\" device to file\n"
+ "host bind [-r] <label> <filename> - bind \"host\" device to file\n"
" -r = mark as removable\n"
"host unbind <label> - unbind file from \"host\" device\n"
"host info [<label>] - show device binding & info\n"
--
2.25.1
^ permalink raw reply related [flat|nested] 38+ messages in thread* [PATCH 03/15] blk: sandbox: Support binding a device with a given logical block size
2023-09-26 8:43 [PATCH 00/15] blk: sandbox: Support binding a device with a given logical block size Bin Meng
2023-09-26 8:43 ` [PATCH 01/15] blk: Use a macro for the typical " Bin Meng
2023-09-26 8:43 ` [PATCH 02/15] cmd: host: Mandate the filename parameter in the 'bind' command Bin Meng
@ 2023-09-26 8:43 ` Bin Meng
2023-09-26 8:43 ` [PATCH 04/15] blk: host_dev: Make host_sb_detach_file() and host_sb_ops static Bin Meng
` (13 subsequent siblings)
16 siblings, 0 replies; 38+ messages in thread
From: Bin Meng @ 2023-09-26 8:43 UTC (permalink / raw)
To: Simon Glass, U-Boot Mailing List; +Cc: Heinrich Schuchardt
Allow optionally set the logical block size of the host device to
bind in the "host bind" command. If not given, defaults to 512.
Signed-off-by: Bin Meng <bmeng@tinylab.org>
---
cmd/host.c | 16 +++++++++++++---
drivers/block/host-uclass.c | 15 ++++++++++++---
include/sandbox_host.h | 7 +++++--
test/dm/blk.c | 7 ++++---
test/dm/host.c | 6 +++---
5 files changed, 37 insertions(+), 14 deletions(-)
diff --git a/cmd/host.c b/cmd/host.c
index b924940ffb..2334ccd9bc 100644
--- a/cmd/host.c
+++ b/cmd/host.c
@@ -13,6 +13,7 @@
#include <dm/device-internal.h>
#include <dm/uclass-internal.h>
#include <linux/errno.h>
+#include <linux/log2.h>
static int do_host_load(struct cmd_tbl *cmdtp, int flag, int argc,
char *const argv[])
@@ -45,6 +46,7 @@ static int do_host_bind(struct cmd_tbl *cmdtp, int flag, int argc,
struct udevice *dev;
const char *label;
char *file;
+ unsigned long blksz = DEFAULT_BLKSZ;
int ret;
/* Skip 'bind' */
@@ -59,12 +61,19 @@ static int do_host_bind(struct cmd_tbl *cmdtp, int flag, int argc,
argv++;
}
- if (argc != 2)
+ if (argc < 2 || argc > 3)
return CMD_RET_USAGE;
label = argv[0];
file = argv[1];
+ if (argc > 2) {
+ blksz = dectoul(argv[2], NULL);
+ if (blksz < DEFAULT_BLKSZ || !is_power_of_2(blksz)) {
+ printf("blksz must be >= 512 and power of 2\n");
+ return CMD_RET_FAILURE;
+ }
+ }
- ret = host_create_attach_file(label, file, removable, &dev);
+ ret = host_create_attach_file(label, file, removable, blksz, &dev);
if (ret) {
printf("Cannot create device / bind file\n");
return CMD_RET_FAILURE;
@@ -253,7 +262,8 @@ U_BOOT_CMD(
"host save hostfs - <addr> <filename> <bytes> [<offset>] - "
"save a file to host\n"
"host size hostfs - <filename> - determine size of file on host\n"
- "host bind [-r] <label> <filename> - bind \"host\" device to file\n"
+ "host bind [-r] <label> <filename> [<blksz>] - bind \"host\" device to file,\n"
+ " and optionally set the device's logical block size\n"
" -r = mark as removable\n"
"host unbind <label> - unbind file from \"host\" device\n"
"host info [<label>] - show device binding & info\n"
diff --git a/drivers/block/host-uclass.c b/drivers/block/host-uclass.c
index 6460d968c2..b3647e3ce3 100644
--- a/drivers/block/host-uclass.c
+++ b/drivers/block/host-uclass.c
@@ -13,6 +13,7 @@
#include <blk.h>
#include <dm.h>
#include <malloc.h>
+#include <part.h>
#include <sandbox_host.h>
#include <dm/device-internal.h>
#include <dm/lists.h>
@@ -29,7 +30,8 @@ struct host_priv {
struct udevice *cur_dev;
};
-int host_create_device(const char *label, bool removable, struct udevice **devp)
+int host_create_device(const char *label, bool removable, unsigned long blksz,
+ struct udevice **devp)
{
char dev_name[30], *str, *label_new;
struct host_sb_plat *plat;
@@ -68,6 +70,12 @@ int host_create_device(const char *label, bool removable, struct udevice **devp)
struct blk_desc *desc = dev_get_uclass_plat(blk);
desc->removable = removable;
+
+ /* update blk device's block size with the provided one */
+ if (blksz != desc->blksz) {
+ desc->blksz = blksz;
+ desc->log2blksz = LOG2(desc->blksz);
+ }
}
plat = dev_get_plat(dev);
@@ -95,12 +103,13 @@ int host_attach_file(struct udevice *dev, const char *filename)
}
int host_create_attach_file(const char *label, const char *filename,
- bool removable, struct udevice **devp)
+ bool removable, unsigned long blksz,
+ struct udevice **devp)
{
struct udevice *dev;
int ret;
- ret = host_create_device(label, removable, &dev);
+ ret = host_create_device(label, removable, blksz, &dev);
if (ret)
return log_msg_ret("cre", ret);
diff --git a/include/sandbox_host.h b/include/sandbox_host.h
index ebd7d99b47..f7a5fc6723 100644
--- a/include/sandbox_host.h
+++ b/include/sandbox_host.h
@@ -74,10 +74,11 @@ int host_detach_file(struct udevice *dev);
* @label: Label of the attachment, e.g. "test1"
* @removable: true if the device should be marked as removable, false
* if it is fixed. See enum blk_flag_t
+ * @blksz: logical block size of the device
* @devp: Returns the device created, on success
* Returns: 0 if OK, -ve on error
*/
-int host_create_device(const char *label, bool removable,
+int host_create_device(const char *label, bool removable, unsigned long blksz,
struct udevice **devp);
/**
@@ -87,11 +88,13 @@ int host_create_device(const char *label, bool removable,
* @filename: Name of the file, e.g. "/path/to/disk.img"
* @removable: true if the device should be marked as removable, false
* if it is fixed. See enum blk_flag_t
+ * @blksz: logical block size of the device
* @devp: Returns the device created, on success
* Returns: 0 if OK, -ve on error
*/
int host_create_attach_file(const char *label, const char *filename,
- bool removable, struct udevice **devp);
+ bool removable, unsigned long blksz,
+ struct udevice **devp);
/**
* host_find_by_label() - Find a host by label
diff --git a/test/dm/blk.c b/test/dm/blk.c
index 446c4423e6..799f1e4dc7 100644
--- a/test/dm/blk.c
+++ b/test/dm/blk.c
@@ -4,6 +4,7 @@
*/
#include <common.h>
+#include <blk.h>
#include <dm.h>
#include <part.h>
#include <sandbox_host.h>
@@ -22,8 +23,8 @@ static int dm_test_blk_base(struct unit_test_state *uts)
struct udevice *blk0, *blk1, *dev0, *dev1, *dev, *chk0, *chk1;
/* Create two, one the parent of the other */
- ut_assertok(host_create_device("test0", false, &dev0));
- ut_assertok(host_create_device("test1", false, &dev1));
+ ut_assertok(host_create_device("test0", false, DEFAULT_BLKSZ, &dev0));
+ ut_assertok(host_create_device("test1", false, DEFAULT_BLKSZ, &dev1));
/* Check we can find them */
ut_assertok(blk_get_device(UCLASS_HOST, 0, &blk0));
@@ -99,7 +100,7 @@ static int dm_test_blk_find(struct unit_test_state *uts)
{
struct udevice *blk, *chk, *dev;
- ut_assertok(host_create_device("test0", false, &dev));
+ ut_assertok(host_create_device("test0", false, DEFAULT_BLKSZ, &dev));
ut_assertok(blk_find_device(UCLASS_HOST, 0, &chk));
ut_assertok(device_find_first_child_by_uclass(dev, UCLASS_BLK, &blk));
diff --git a/test/dm/host.c b/test/dm/host.c
index 355ba7770a..19f6c67462 100644
--- a/test/dm/host.c
+++ b/test/dm/host.c
@@ -32,7 +32,7 @@ static int dm_test_host(struct unit_test_state *uts)
ut_asserteq(-ENODEV, uclass_first_device_err(UCLASS_PARTITION, &part));
mem_start = ut_check_delta(0);
- ut_assertok(host_create_device(label, true, &dev));
+ ut_assertok(host_create_device(label, true, DEFAULT_BLKSZ, &dev));
/* Check that the plat data has been allocated */
plat = dev_get_plat(dev);
@@ -81,7 +81,7 @@ static int dm_test_host_dup(struct unit_test_state *uts)
struct udevice *dev, *chk;
ut_asserteq(0, uclass_id_count(UCLASS_HOST));
- ut_assertok(host_create_device(label, true, &dev));
+ ut_assertok(host_create_device(label, true, DEFAULT_BLKSZ, &dev));
/* Attach a file created in test_host.py */
ut_assertok(host_attach_file(dev, filename));
@@ -90,7 +90,7 @@ static int dm_test_host_dup(struct unit_test_state *uts)
ut_asserteq(1, uclass_id_count(UCLASS_HOST));
/* Create another device with the same label (should remove old one) */
- ut_assertok(host_create_device(label, true, &dev));
+ ut_assertok(host_create_device(label, true, DEFAULT_BLKSZ, &dev));
/* Attach a different file created in test_host.py */
ut_assertok(host_attach_file(dev, filename2));
--
2.25.1
^ permalink raw reply related [flat|nested] 38+ messages in thread* [PATCH 04/15] blk: host_dev: Make host_sb_detach_file() and host_sb_ops static
2023-09-26 8:43 [PATCH 00/15] blk: sandbox: Support binding a device with a given logical block size Bin Meng
` (2 preceding siblings ...)
2023-09-26 8:43 ` [PATCH 03/15] blk: sandbox: Support binding a device with a given logical block size Bin Meng
@ 2023-09-26 8:43 ` Bin Meng
2023-10-02 1:16 ` Simon Glass
2023-09-26 8:43 ` [PATCH 05/15] blk: host_dev: Sanity check on the size of host backing file Bin Meng
` (12 subsequent siblings)
16 siblings, 1 reply; 38+ messages in thread
From: Bin Meng @ 2023-09-26 8:43 UTC (permalink / raw)
To: Simon Glass, U-Boot Mailing List; +Cc: Heinrich Schuchardt
They are only used in drivers/block/host_dev.c.
Signed-off-by: Bin Meng <bmeng@tinylab.org>
---
drivers/block/host_dev.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/block/host_dev.c b/drivers/block/host_dev.c
index 31c7814054..0b43f80a86 100644
--- a/drivers/block/host_dev.c
+++ b/drivers/block/host_dev.c
@@ -73,7 +73,7 @@ err_fname:
return ret;
}
-int host_sb_detach_file(struct udevice *dev)
+static int host_sb_detach_file(struct udevice *dev)
{
struct host_sb_plat *plat = dev_get_plat(dev);
int ret;
@@ -123,7 +123,7 @@ static int host_sb_bind(struct udevice *dev)
return 0;
}
-struct host_ops host_sb_ops = {
+static struct host_ops host_sb_ops = {
.attach_file = host_sb_attach_file,
.detach_file = host_sb_detach_file,
};
--
2.25.1
^ permalink raw reply related [flat|nested] 38+ messages in thread* [PATCH 05/15] blk: host_dev: Sanity check on the size of host backing file
2023-09-26 8:43 [PATCH 00/15] blk: sandbox: Support binding a device with a given logical block size Bin Meng
` (3 preceding siblings ...)
2023-09-26 8:43 ` [PATCH 04/15] blk: host_dev: Make host_sb_detach_file() and host_sb_ops static Bin Meng
@ 2023-09-26 8:43 ` Bin Meng
2023-10-02 1:16 ` Simon Glass
2023-09-26 8:43 ` [PATCH 06/15] cmd: host: Print out the block size of the host device Bin Meng
` (11 subsequent siblings)
16 siblings, 1 reply; 38+ messages in thread
From: Bin Meng @ 2023-09-26 8:43 UTC (permalink / raw)
To: Simon Glass, U-Boot Mailing List; +Cc: Heinrich Schuchardt
Since we are emulating a block device, its size should be multiple
of the configured block size.
Signed-off-by: Bin Meng <bmeng@tinylab.org>
---
drivers/block/host_dev.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/block/host_dev.c b/drivers/block/host_dev.c
index 0b43f80a86..30c7415793 100644
--- a/drivers/block/host_dev.c
+++ b/drivers/block/host_dev.c
@@ -58,6 +58,11 @@ static int host_sb_attach_file(struct udevice *dev, const char *filename)
size = os_filesize(fd);
desc = dev_get_uclass_plat(blk);
+ if (size % desc->blksz) {
+ printf("The size of host backing file '%s' is not multiple of "
+ "the device block size\n", filename);
+ goto err_fname;
+ }
desc->lba = size / desc->blksz;
/* write this in last, when nothing can go wrong */
--
2.25.1
^ permalink raw reply related [flat|nested] 38+ messages in thread* [PATCH 06/15] cmd: host: Print out the block size of the host device
2023-09-26 8:43 [PATCH 00/15] blk: sandbox: Support binding a device with a given logical block size Bin Meng
` (4 preceding siblings ...)
2023-09-26 8:43 ` [PATCH 05/15] blk: host_dev: Sanity check on the size of host backing file Bin Meng
@ 2023-09-26 8:43 ` Bin Meng
2023-10-02 1:17 ` Simon Glass
2023-09-26 8:43 ` [PATCH 07/15] blk: blkmap: Make bind/unbind routines static Bin Meng
` (10 subsequent siblings)
16 siblings, 1 reply; 38+ messages in thread
From: Bin Meng @ 2023-09-26 8:43 UTC (permalink / raw)
To: Simon Glass, U-Boot Mailing List
It's useful if we can print out the block size of the host device
in the "host info" command.
Signed-off-by: Bin Meng <bmeng@tinylab.org>
---
cmd/host.c | 7 ++++---
test/dm/host.c | 20 ++++++++++----------
2 files changed, 14 insertions(+), 13 deletions(-)
diff --git a/cmd/host.c b/cmd/host.c
index 2334ccd9bc..c33c2a9787 100644
--- a/cmd/host.c
+++ b/cmd/host.c
@@ -160,8 +160,8 @@ static void show_host_dev(struct udevice *dev)
return;
desc = dev_get_uclass_plat(blk);
- printf("%12lu %-15s %s\n", (unsigned long)desc->lba, plat->label,
- plat->filename);
+ printf("%12lu %6lu %-15s %s\n", (unsigned long)desc->lba, desc->blksz,
+ plat->label, plat->filename);
}
static int do_host_info(struct cmd_tbl *cmdtp, int flag, int argc,
@@ -179,7 +179,8 @@ static int do_host_info(struct cmd_tbl *cmdtp, int flag, int argc,
return CMD_RET_FAILURE;
}
- printf("%3s %12s %-15s %s\n", "dev", "blocks", "label", "path");
+ printf("%3s %12s %6s %-15s %s\n",
+ "dev", "blocks", "blksz", "label", "path");
if (dev) {
show_host_dev(dev);
} else {
diff --git a/test/dm/host.c b/test/dm/host.c
index 19f6c67462..580b14cf79 100644
--- a/test/dm/host.c
+++ b/test/dm/host.c
@@ -114,7 +114,7 @@ static int dm_test_cmd_host(struct unit_test_state *uts)
/* first check 'host info' with binding */
ut_assertok(run_command("host info", 0));
- ut_assert_nextline("dev blocks label path");
+ ut_assert_nextline("dev blocks blksz label path");
ut_assert_console_end();
ut_assertok(run_commandf("host bind -r test2 %s", filename));
@@ -126,8 +126,8 @@ static int dm_test_cmd_host(struct unit_test_state *uts)
ut_asserteq(true, desc->removable);
ut_assertok(run_command("host info", 0));
- ut_assert_nextline("dev blocks label path");
- ut_assert_nextline(" 0 4096 test2 2MB.ext2.img");
+ ut_assert_nextline("dev blocks blksz label path");
+ ut_assert_nextline(" 0 4096 512 test2 2MB.ext2.img");
ut_assert_console_end();
ut_assertok(run_commandf("host bind fat %s", filename2));
@@ -139,9 +139,9 @@ static int dm_test_cmd_host(struct unit_test_state *uts)
ut_asserteq(false, desc->removable);
ut_assertok(run_command("host info", 0));
- ut_assert_nextline("dev blocks label path");
- ut_assert_nextline(" 0 4096 test2 2MB.ext2.img");
- ut_assert_nextline(" 1 2048 fat 1MB.fat32.img");
+ ut_assert_nextline("dev blocks blksz label path");
+ ut_assert_nextline(" 0 4096 512 test2 2MB.ext2.img");
+ ut_assert_nextline(" 1 2048 512 fat 1MB.fat32.img");
ut_assert_console_end();
ut_asserteq(1, run_command("host info test", 0));
@@ -149,8 +149,8 @@ static int dm_test_cmd_host(struct unit_test_state *uts)
ut_assert_console_end();
ut_assertok(run_command("host info fat", 0));
- ut_assert_nextline("dev blocks label path");
- ut_assert_nextline(" 1 2048 fat 1MB.fat32.img");
+ ut_assert_nextline("dev blocks blksz label path");
+ ut_assert_nextline(" 1 2048 512 fat 1MB.fat32.img");
ut_assert_console_end();
/* check 'host dev' */
@@ -186,8 +186,8 @@ static int dm_test_cmd_host(struct unit_test_state *uts)
ut_assert_console_end();
ut_assertok(run_command("host info", 0));
- ut_assert_nextline("dev blocks label path");
- ut_assert_nextline(" 1 2048 fat 1MB.fat32.img");
+ ut_assert_nextline("dev blocks blksz label path");
+ ut_assert_nextline(" 1 2048 512 fat 1MB.fat32.img");
ut_assert_console_end();
return 0;
--
2.25.1
^ permalink raw reply related [flat|nested] 38+ messages in thread* [PATCH 07/15] blk: blkmap: Make bind/unbind routines static
2023-09-26 8:43 [PATCH 00/15] blk: sandbox: Support binding a device with a given logical block size Bin Meng
` (5 preceding siblings ...)
2023-09-26 8:43 ` [PATCH 06/15] cmd: host: Print out the block size of the host device Bin Meng
@ 2023-09-26 8:43 ` Bin Meng
2023-10-02 1:16 ` Simon Glass
2023-09-26 8:43 ` [PATCH 08/15] cmd: blkmap: Make map_handlers[] and its .fn static Bin Meng
` (9 subsequent siblings)
16 siblings, 1 reply; 38+ messages in thread
From: Bin Meng @ 2023-09-26 8:43 UTC (permalink / raw)
To: Simon Glass, U-Boot Mailing List; +Cc: Tobias Waldekranz
These 2 are only used in drivers/block/blkmap.c.
Signed-off-by: Bin Meng <bmeng@tinylab.org>
---
drivers/block/blkmap.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/block/blkmap.c b/drivers/block/blkmap.c
index 409aa46de2..f6acfa8927 100644
--- a/drivers/block/blkmap.c
+++ b/drivers/block/blkmap.c
@@ -383,7 +383,7 @@ U_BOOT_DRIVER(blkmap_blk) = {
.ops = &blkmap_blk_ops,
};
-int blkmap_dev_bind(struct udevice *dev)
+static int blkmap_dev_bind(struct udevice *dev)
{
struct blkmap *bm = dev_get_plat(dev);
struct blk_desc *bd;
@@ -410,7 +410,7 @@ int blkmap_dev_bind(struct udevice *dev)
return 0;
}
-int blkmap_dev_unbind(struct udevice *dev)
+static int blkmap_dev_unbind(struct udevice *dev)
{
struct blkmap *bm = dev_get_plat(dev);
struct blkmap_slice *bms, *tmp;
--
2.25.1
^ permalink raw reply related [flat|nested] 38+ messages in thread* [PATCH 08/15] cmd: blkmap: Make map_handlers[] and its .fn static
2023-09-26 8:43 [PATCH 00/15] blk: sandbox: Support binding a device with a given logical block size Bin Meng
` (6 preceding siblings ...)
2023-09-26 8:43 ` [PATCH 07/15] blk: blkmap: Make bind/unbind routines static Bin Meng
@ 2023-09-26 8:43 ` Bin Meng
2023-10-02 1:17 ` Simon Glass
2023-09-26 8:43 ` [PATCH 09/15] blk: blkmap: Support mapping to device of any block size Bin Meng
` (8 subsequent siblings)
16 siblings, 1 reply; 38+ messages in thread
From: Bin Meng @ 2023-09-26 8:43 UTC (permalink / raw)
To: Simon Glass, U-Boot Mailing List; +Cc: Tobias Waldekranz
These are only used in cmd/blkmap.c.
Signed-off-by: Bin Meng <bmeng@tinylab.org>
---
cmd/blkmap.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/cmd/blkmap.c b/cmd/blkmap.c
index b34c013072..ef74ebc003 100644
--- a/cmd/blkmap.c
+++ b/cmd/blkmap.c
@@ -25,7 +25,8 @@ struct map_handler {
map_parser_fn fn;
};
-int do_blkmap_map_linear(struct map_ctx *ctx, int argc, char *const argv[])
+static int do_blkmap_map_linear(struct map_ctx *ctx, int argc,
+ char *const argv[])
{
struct blk_desc *lbd;
int err, ldevnum;
@@ -58,7 +59,7 @@ int do_blkmap_map_linear(struct map_ctx *ctx, int argc, char *const argv[])
return CMD_RET_SUCCESS;
}
-int do_blkmap_map_mem(struct map_ctx *ctx, int argc, char *const argv[])
+static int do_blkmap_map_mem(struct map_ctx *ctx, int argc, char *const argv[])
{
phys_addr_t addr;
int err;
@@ -80,7 +81,7 @@ int do_blkmap_map_mem(struct map_ctx *ctx, int argc, char *const argv[])
return CMD_RET_SUCCESS;
}
-struct map_handler map_handlers[] = {
+static struct map_handler map_handlers[] = {
{ .name = "linear", .fn = do_blkmap_map_linear },
{ .name = "mem", .fn = do_blkmap_map_mem },
--
2.25.1
^ permalink raw reply related [flat|nested] 38+ messages in thread* [PATCH 09/15] blk: blkmap: Support mapping to device of any block size
2023-09-26 8:43 [PATCH 00/15] blk: sandbox: Support binding a device with a given logical block size Bin Meng
` (7 preceding siblings ...)
2023-09-26 8:43 ` [PATCH 08/15] cmd: blkmap: Make map_handlers[] and its .fn static Bin Meng
@ 2023-09-26 8:43 ` Bin Meng
2023-09-26 19:29 ` Tobias Waldekranz
2023-10-02 1:16 ` Simon Glass
2023-09-26 8:43 ` [PATCH 10/15] cmd: blk_common: Use macros for the return values Bin Meng
` (7 subsequent siblings)
16 siblings, 2 replies; 38+ messages in thread
From: Bin Meng @ 2023-09-26 8:43 UTC (permalink / raw)
To: Simon Glass, U-Boot Mailing List; +Cc: Tobias Waldekranz
At present if a device to map has a block size other than 512,
the blkmap map process just fails. There is no reason why we
can't just use the block size of the mapped device.
Signed-off-by: Bin Meng <bmeng@tinylab.org>
---
drivers/block/blkmap.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/block/blkmap.c b/drivers/block/blkmap.c
index f6acfa8927..149a4cac3e 100644
--- a/drivers/block/blkmap.c
+++ b/drivers/block/blkmap.c
@@ -171,11 +171,11 @@ int blkmap_map_linear(struct udevice *dev, lbaint_t blknr, lbaint_t blkcnt,
bd = dev_get_uclass_plat(bm->blk);
lbd = dev_get_uclass_plat(lblk);
- if (lbd->blksz != bd->blksz)
- /* We could support block size translation, but we
- * don't yet.
- */
- return -EINVAL;
+ if (lbd->blksz != bd->blksz) {
+ /* update to match the mapped device */
+ bd->blksz = lbd->blksz;
+ bd->log2blksz = LOG2(bd->blksz);
+ }
linear = malloc(sizeof(*linear));
if (!linear)
--
2.25.1
^ permalink raw reply related [flat|nested] 38+ messages in thread* Re: [PATCH 09/15] blk: blkmap: Support mapping to device of any block size
2023-09-26 8:43 ` [PATCH 09/15] blk: blkmap: Support mapping to device of any block size Bin Meng
@ 2023-09-26 19:29 ` Tobias Waldekranz
2023-09-26 22:44 ` Bin Meng
2023-10-02 1:16 ` Simon Glass
1 sibling, 1 reply; 38+ messages in thread
From: Tobias Waldekranz @ 2023-09-26 19:29 UTC (permalink / raw)
To: Bin Meng, Simon Glass, U-Boot Mailing List
On tis, sep 26, 2023 at 16:43, Bin Meng <bmeng@tinylab.org> wrote:
> At present if a device to map has a block size other than 512,
> the blkmap map process just fails. There is no reason why we
> can't just use the block size of the mapped device.
Won't this be very confusing to the user?
The blkmap device uses a fixed block size of 512:
https://source.denx.de/u-boot/u-boot/-/blob/master/drivers/block/blkmap.c?ref_type=heads#L393
So if I map a slice of a 4k device into a blkmap, then
blkmap read 0x80000000 0 1
would copy 4k instead of 512 bytes from the lower device to 0x80000000,
even though the blkmap reports a block size of 512.
It seems to me that the expected behavior would be that only the first
512 bytes would be copied in the command above.
>
> Signed-off-by: Bin Meng <bmeng@tinylab.org>
> ---
>
> drivers/block/blkmap.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/block/blkmap.c b/drivers/block/blkmap.c
> index f6acfa8927..149a4cac3e 100644
> --- a/drivers/block/blkmap.c
> +++ b/drivers/block/blkmap.c
> @@ -171,11 +171,11 @@ int blkmap_map_linear(struct udevice *dev, lbaint_t blknr, lbaint_t blkcnt,
>
> bd = dev_get_uclass_plat(bm->blk);
> lbd = dev_get_uclass_plat(lblk);
> - if (lbd->blksz != bd->blksz)
> - /* We could support block size translation, but we
> - * don't yet.
> - */
Hence this comment ^
> - return -EINVAL;
> + if (lbd->blksz != bd->blksz) {
> + /* update to match the mapped device */
> + bd->blksz = lbd->blksz;
> + bd->log2blksz = LOG2(bd->blksz);
> + }
>
> linear = malloc(sizeof(*linear));
> if (!linear)
> --
> 2.25.1
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: [PATCH 09/15] blk: blkmap: Support mapping to device of any block size
2023-09-26 19:29 ` Tobias Waldekranz
@ 2023-09-26 22:44 ` Bin Meng
0 siblings, 0 replies; 38+ messages in thread
From: Bin Meng @ 2023-09-26 22:44 UTC (permalink / raw)
To: Tobias Waldekranz; +Cc: Bin Meng, Simon Glass, U-Boot Mailing List
Hi Tobias,
On Wed, Sep 27, 2023 at 3:29 AM Tobias Waldekranz <tobias@waldekranz.com> wrote:
>
> On tis, sep 26, 2023 at 16:43, Bin Meng <bmeng@tinylab.org> wrote:
> > At present if a device to map has a block size other than 512,
> > the blkmap map process just fails. There is no reason why we
> > can't just use the block size of the mapped device.
>
> Won't this be very confusing to the user?
I don't see any confusion.
>
> The blkmap device uses a fixed block size of 512:
>
> https://source.denx.de/u-boot/u-boot/-/blob/master/drivers/block/blkmap.c?ref_type=heads#L393
Yes, the blkmap device was originally created with a fixed block size
of 512, and that's fine.
>
> So if I map a slice of a 4k device into a blkmap, then
>
> blkmap read 0x80000000 0 1
>
> would copy 4k instead of 512 bytes from the lower device to 0x80000000,
> even though the blkmap reports a block size of 512.
>
> It seems to me that the expected behavior would be that only the first
> 512 bytes would be copied in the command above.
No, the blkmap block size was later updated to match the real
underlying device parameter during the map process. So it will copy
all 4k to 0x80000000.
>
> >
> > Signed-off-by: Bin Meng <bmeng@tinylab.org>
> > ---
> >
> > drivers/block/blkmap.c | 10 +++++-----
> > 1 file changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/block/blkmap.c b/drivers/block/blkmap.c
> > index f6acfa8927..149a4cac3e 100644
> > --- a/drivers/block/blkmap.c
> > +++ b/drivers/block/blkmap.c
> > @@ -171,11 +171,11 @@ int blkmap_map_linear(struct udevice *dev, lbaint_t blknr, lbaint_t blkcnt,
> >
> > bd = dev_get_uclass_plat(bm->blk);
> > lbd = dev_get_uclass_plat(lblk);
> > - if (lbd->blksz != bd->blksz)
> > - /* We could support block size translation, but we
> > - * don't yet.
> > - */
>
> Hence this comment ^
This comment was completely removed with the new updates. There is no
need to do any block size translation. We could just use whatever
block size the lower device is using, hence this patch.
>
> > - return -EINVAL;
> > + if (lbd->blksz != bd->blksz) {
> > + /* update to match the mapped device */
> > + bd->blksz = lbd->blksz;
> > + bd->log2blksz = LOG2(bd->blksz);
> > + }
> >
> > linear = malloc(sizeof(*linear));
> > if (!linear)
> > --
Regards,
Bin
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH 09/15] blk: blkmap: Support mapping to device of any block size
2023-09-26 8:43 ` [PATCH 09/15] blk: blkmap: Support mapping to device of any block size Bin Meng
2023-09-26 19:29 ` Tobias Waldekranz
@ 2023-10-02 1:16 ` Simon Glass
1 sibling, 0 replies; 38+ messages in thread
From: Simon Glass @ 2023-10-02 1:16 UTC (permalink / raw)
To: Bin Meng; +Cc: U-Boot Mailing List, Tobias Waldekranz
On Tue, 26 Sept 2023 at 02:53, Bin Meng <bmeng@tinylab.org> wrote:
>
> At present if a device to map has a block size other than 512,
> the blkmap map process just fails. There is no reason why we
> can't just use the block size of the mapped device.
>
> Signed-off-by: Bin Meng <bmeng@tinylab.org>
> ---
>
> drivers/block/blkmap.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
Reviewed-by: Simon Glass <sjg@chromium.org>
^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH 10/15] cmd: blk_common: Use macros for the return values
2023-09-26 8:43 [PATCH 00/15] blk: sandbox: Support binding a device with a given logical block size Bin Meng
` (8 preceding siblings ...)
2023-09-26 8:43 ` [PATCH 09/15] blk: blkmap: Support mapping to device of any block size Bin Meng
@ 2023-09-26 8:43 ` Bin Meng
2023-10-02 1:17 ` Simon Glass
2023-09-26 8:43 ` [PATCH 11/15] dm: blk: Rename get_desc() and make it externally visible Bin Meng
` (6 subsequent siblings)
16 siblings, 1 reply; 38+ messages in thread
From: Bin Meng @ 2023-09-26 8:43 UTC (permalink / raw)
To: Simon Glass, U-Boot Mailing List; +Cc: Tobias Waldekranz
Avoid using magic number 0/1 for the command result.
Signed-off-by: Bin Meng <bmeng@tinylab.org>
---
cmd/blk_common.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/cmd/blk_common.c b/cmd/blk_common.c
index 9f9d4327a9..ad9b16dc09 100644
--- a/cmd/blk_common.c
+++ b/cmd/blk_common.c
@@ -25,18 +25,18 @@ int blk_common_cmd(int argc, char *const argv[], enum uclass_id uclass_id,
case 2:
if (strncmp(argv[1], "inf", 3) == 0) {
blk_list_devices(uclass_id);
- return 0;
+ return CMD_RET_SUCCESS;
} else if (strncmp(argv[1], "dev", 3) == 0) {
if (blk_print_device_num(uclass_id, *cur_devnump)) {
printf("\nno %s devices available\n", if_name);
return CMD_RET_FAILURE;
}
- return 0;
+ return CMD_RET_SUCCESS;
} else if (strncmp(argv[1], "part", 4) == 0) {
if (blk_list_part(uclass_id))
printf("\nno %s partition table available\n",
if_name);
- return 0;
+ return CMD_RET_SUCCESS;
}
return CMD_RET_USAGE;
case 3:
@@ -49,7 +49,7 @@ int blk_common_cmd(int argc, char *const argv[], enum uclass_id uclass_id,
} else {
return CMD_RET_FAILURE;
}
- return 0;
+ return CMD_RET_SUCCESS;
} else if (strncmp(argv[1], "part", 4) == 0) {
int dev = (int)dectoul(argv[2], NULL);
@@ -58,7 +58,7 @@ int blk_common_cmd(int argc, char *const argv[], enum uclass_id uclass_id,
if_name, dev);
return CMD_RET_FAILURE;
}
- return 0;
+ return CMD_RET_SUCCESS;
}
return CMD_RET_USAGE;
@@ -80,7 +80,7 @@ int blk_common_cmd(int argc, char *const argv[], enum uclass_id uclass_id,
printf("%ld blocks read: %s\n", n,
n == cnt ? "OK" : "ERROR");
- return n == cnt ? 0 : 1;
+ return n == cnt ? CMD_RET_SUCCESS : CMD_RET_FAILURE;
} else if (strcmp(argv[1], "write") == 0) {
phys_addr_t paddr = hextoul(argv[2], NULL);
lbaint_t blk = hextoul(argv[3], NULL);
@@ -98,7 +98,7 @@ int blk_common_cmd(int argc, char *const argv[], enum uclass_id uclass_id,
printf("%ld blocks written: %s\n", n,
n == cnt ? "OK" : "ERROR");
- return n == cnt ? 0 : 1;
+ return n == cnt ? CMD_RET_SUCCESS : CMD_RET_FAILURE;
} else {
return CMD_RET_USAGE;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 38+ messages in thread* Re: [PATCH 10/15] cmd: blk_common: Use macros for the return values
2023-09-26 8:43 ` [PATCH 10/15] cmd: blk_common: Use macros for the return values Bin Meng
@ 2023-10-02 1:17 ` Simon Glass
2023-10-10 9:05 ` Bin Meng
0 siblings, 1 reply; 38+ messages in thread
From: Simon Glass @ 2023-10-02 1:17 UTC (permalink / raw)
To: Bin Meng; +Cc: U-Boot Mailing List, Tobias Waldekranz
Hi Bin,
On Tue, 26 Sept 2023 at 02:54, Bin Meng <bmeng@tinylab.org> wrote:
>
> Avoid using magic number 0/1 for the command result.
>
> Signed-off-by: Bin Meng <bmeng@tinylab.org>
> ---
>
> cmd/blk_common.c | 14 +++++++-------
> 1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/cmd/blk_common.c b/cmd/blk_common.c
> index 9f9d4327a9..ad9b16dc09 100644
> --- a/cmd/blk_common.c
> +++ b/cmd/blk_common.c
> @@ -25,18 +25,18 @@ int blk_common_cmd(int argc, char *const argv[], enum uclass_id uclass_id,
> case 2:
> if (strncmp(argv[1], "inf", 3) == 0) {
> blk_list_devices(uclass_id);
> - return 0;
> + return CMD_RET_SUCCESS;
I really don't like this...0 is success.
> } else if (strncmp(argv[1], "dev", 3) == 0) {
> if (blk_print_device_num(uclass_id, *cur_devnump)) {
> printf("\nno %s devices available\n", if_name);
> return CMD_RET_FAILURE;
> }
> - return 0;
> + return CMD_RET_SUCCESS;
> } else if (strncmp(argv[1], "part", 4) == 0) {
> if (blk_list_part(uclass_id))
> printf("\nno %s partition table available\n",
> if_name);
> - return 0;
> + return CMD_RET_SUCCESS;
> }
> return CMD_RET_USAGE;
> case 3:
> @@ -49,7 +49,7 @@ int blk_common_cmd(int argc, char *const argv[], enum uclass_id uclass_id,
> } else {
> return CMD_RET_FAILURE;
> }
> - return 0;
> + return CMD_RET_SUCCESS;
> } else if (strncmp(argv[1], "part", 4) == 0) {
> int dev = (int)dectoul(argv[2], NULL);
>
> @@ -58,7 +58,7 @@ int blk_common_cmd(int argc, char *const argv[], enum uclass_id uclass_id,
> if_name, dev);
> return CMD_RET_FAILURE;
> }
> - return 0;
> + return CMD_RET_SUCCESS;
> }
> return CMD_RET_USAGE;
>
> @@ -80,7 +80,7 @@ int blk_common_cmd(int argc, char *const argv[], enum uclass_id uclass_id,
>
> printf("%ld blocks read: %s\n", n,
> n == cnt ? "OK" : "ERROR");
> - return n == cnt ? 0 : 1;
> + return n == cnt ? CMD_RET_SUCCESS : CMD_RET_FAILURE;
CMD_RET_FAILURE is OK, but I would prefer not to use CMD_RET_SUCCESS.
It is 0 and always will be.
It encourages people to do things like:
if (ret == CMD_RET_SUCCESS)
instead of
if (!ret)
It would eventually creep into everything, including our clean error handling.
> } else if (strcmp(argv[1], "write") == 0) {
> phys_addr_t paddr = hextoul(argv[2], NULL);
> lbaint_t blk = hextoul(argv[3], NULL);
> @@ -98,7 +98,7 @@ int blk_common_cmd(int argc, char *const argv[], enum uclass_id uclass_id,
>
> printf("%ld blocks written: %s\n", n,
> n == cnt ? "OK" : "ERROR");
> - return n == cnt ? 0 : 1;
> + return n == cnt ? CMD_RET_SUCCESS : CMD_RET_FAILURE;
> } else {
> return CMD_RET_USAGE;
> }
> --
> 2.25.1
>
Regards,
Simon
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: [PATCH 10/15] cmd: blk_common: Use macros for the return values
2023-10-02 1:17 ` Simon Glass
@ 2023-10-10 9:05 ` Bin Meng
2023-10-10 14:58 ` Simon Glass
0 siblings, 1 reply; 38+ messages in thread
From: Bin Meng @ 2023-10-10 9:05 UTC (permalink / raw)
To: Simon Glass; +Cc: Bin Meng, U-Boot Mailing List, Tobias Waldekranz
Hi Simon,
On Mon, Oct 2, 2023 at 9:42 AM Simon Glass <sjg@chromium.org> wrote:
>
> Hi Bin,
>
> On Tue, 26 Sept 2023 at 02:54, Bin Meng <bmeng@tinylab.org> wrote:
> >
> > Avoid using magic number 0/1 for the command result.
> >
> > Signed-off-by: Bin Meng <bmeng@tinylab.org>
> > ---
> >
> > cmd/blk_common.c | 14 +++++++-------
> > 1 file changed, 7 insertions(+), 7 deletions(-)
> >
> > diff --git a/cmd/blk_common.c b/cmd/blk_common.c
> > index 9f9d4327a9..ad9b16dc09 100644
> > --- a/cmd/blk_common.c
> > +++ b/cmd/blk_common.c
> > @@ -25,18 +25,18 @@ int blk_common_cmd(int argc, char *const argv[], enum uclass_id uclass_id,
> > case 2:
> > if (strncmp(argv[1], "inf", 3) == 0) {
> > blk_list_devices(uclass_id);
> > - return 0;
> > + return CMD_RET_SUCCESS;
>
> I really don't like this...0 is success.
>
> > } else if (strncmp(argv[1], "dev", 3) == 0) {
> > if (blk_print_device_num(uclass_id, *cur_devnump)) {
> > printf("\nno %s devices available\n", if_name);
> > return CMD_RET_FAILURE;
> > }
> > - return 0;
> > + return CMD_RET_SUCCESS;
> > } else if (strncmp(argv[1], "part", 4) == 0) {
> > if (blk_list_part(uclass_id))
> > printf("\nno %s partition table available\n",
> > if_name);
> > - return 0;
> > + return CMD_RET_SUCCESS;
> > }
> > return CMD_RET_USAGE;
> > case 3:
> > @@ -49,7 +49,7 @@ int blk_common_cmd(int argc, char *const argv[], enum uclass_id uclass_id,
> > } else {
> > return CMD_RET_FAILURE;
> > }
> > - return 0;
> > + return CMD_RET_SUCCESS;
> > } else if (strncmp(argv[1], "part", 4) == 0) {
> > int dev = (int)dectoul(argv[2], NULL);
> >
> > @@ -58,7 +58,7 @@ int blk_common_cmd(int argc, char *const argv[], enum uclass_id uclass_id,
> > if_name, dev);
> > return CMD_RET_FAILURE;
> > }
> > - return 0;
> > + return CMD_RET_SUCCESS;
> > }
> > return CMD_RET_USAGE;
> >
> > @@ -80,7 +80,7 @@ int blk_common_cmd(int argc, char *const argv[], enum uclass_id uclass_id,
> >
> > printf("%ld blocks read: %s\n", n,
> > n == cnt ? "OK" : "ERROR");
> > - return n == cnt ? 0 : 1;
> > + return n == cnt ? CMD_RET_SUCCESS : CMD_RET_FAILURE;
>
> CMD_RET_FAILURE is OK, but I would prefer not to use CMD_RET_SUCCESS.
> It is 0 and always will be.
>
> It encourages people to do things like:
>
> if (ret == CMD_RET_SUCCESS)
>
> instead of
>
> if (!ret)
I see your concern. However we don't change the return value type to
enum, so people can still use
if (!ret)
I would still defend that we should use CMD_RET_SUCCESS.
This is like EXIT_XXX defined in stdlib.h:
#define EXIT_FAILURE 1 /* Failing exit status. */
#define EXIT_SUCCESS 0 /* Successful exit status. */
One should use predefined macros whenever possible.
>
> It would eventually creep into everything, including our clean error handling.
>
> > } else if (strcmp(argv[1], "write") == 0) {
> > phys_addr_t paddr = hextoul(argv[2], NULL);
> > lbaint_t blk = hextoul(argv[3], NULL);
> > @@ -98,7 +98,7 @@ int blk_common_cmd(int argc, char *const argv[], enum uclass_id uclass_id,
> >
> > printf("%ld blocks written: %s\n", n,
> > n == cnt ? "OK" : "ERROR");
> > - return n == cnt ? 0 : 1;
> > + return n == cnt ? CMD_RET_SUCCESS : CMD_RET_FAILURE;
> > } else {
> > return CMD_RET_USAGE;
> > }
> > --
Regards,
Bin
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: [PATCH 10/15] cmd: blk_common: Use macros for the return values
2023-10-10 9:05 ` Bin Meng
@ 2023-10-10 14:58 ` Simon Glass
2023-10-10 20:17 ` Tom Rini
0 siblings, 1 reply; 38+ messages in thread
From: Simon Glass @ 2023-10-10 14:58 UTC (permalink / raw)
To: Bin Meng; +Cc: Bin Meng, U-Boot Mailing List, Tobias Waldekranz
Hi Bin,
On Tue, 10 Oct 2023 at 03:06, Bin Meng <bmeng.cn@gmail.com> wrote:
>
> Hi Simon,
>
> On Mon, Oct 2, 2023 at 9:42 AM Simon Glass <sjg@chromium.org> wrote:
> >
> > Hi Bin,
> >
> > On Tue, 26 Sept 2023 at 02:54, Bin Meng <bmeng@tinylab.org> wrote:
> > >
> > > Avoid using magic number 0/1 for the command result.
> > >
> > > Signed-off-by: Bin Meng <bmeng@tinylab.org>
> > > ---
> > >
> > > cmd/blk_common.c | 14 +++++++-------
> > > 1 file changed, 7 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/cmd/blk_common.c b/cmd/blk_common.c
> > > index 9f9d4327a9..ad9b16dc09 100644
> > > --- a/cmd/blk_common.c
> > > +++ b/cmd/blk_common.c
> > > @@ -25,18 +25,18 @@ int blk_common_cmd(int argc, char *const argv[],
enum uclass_id uclass_id,
> > > case 2:
> > > if (strncmp(argv[1], "inf", 3) == 0) {
> > > blk_list_devices(uclass_id);
> > > - return 0;
> > > + return CMD_RET_SUCCESS;
> >
> > I really don't like this...0 is success.
> >
> > > } else if (strncmp(argv[1], "dev", 3) == 0) {
> > > if (blk_print_device_num(uclass_id,
*cur_devnump)) {
> > > printf("\nno %s devices available\n",
if_name);
> > > return CMD_RET_FAILURE;
> > > }
> > > - return 0;
> > > + return CMD_RET_SUCCESS;
> > > } else if (strncmp(argv[1], "part", 4) == 0) {
> > > if (blk_list_part(uclass_id))
> > > printf("\nno %s partition table
available\n",
> > > if_name);
> > > - return 0;
> > > + return CMD_RET_SUCCESS;
> > > }
> > > return CMD_RET_USAGE;
> > > case 3:
> > > @@ -49,7 +49,7 @@ int blk_common_cmd(int argc, char *const argv[],
enum uclass_id uclass_id,
> > > } else {
> > > return CMD_RET_FAILURE;
> > > }
> > > - return 0;
> > > + return CMD_RET_SUCCESS;
> > > } else if (strncmp(argv[1], "part", 4) == 0) {
> > > int dev = (int)dectoul(argv[2], NULL);
> > >
> > > @@ -58,7 +58,7 @@ int blk_common_cmd(int argc, char *const argv[],
enum uclass_id uclass_id,
> > > if_name, dev);
> > > return CMD_RET_FAILURE;
> > > }
> > > - return 0;
> > > + return CMD_RET_SUCCESS;
> > > }
> > > return CMD_RET_USAGE;
> > >
> > > @@ -80,7 +80,7 @@ int blk_common_cmd(int argc, char *const argv[],
enum uclass_id uclass_id,
> > >
> > > printf("%ld blocks read: %s\n", n,
> > > n == cnt ? "OK" : "ERROR");
> > > - return n == cnt ? 0 : 1;
> > > + return n == cnt ? CMD_RET_SUCCESS :
CMD_RET_FAILURE;
> >
> > CMD_RET_FAILURE is OK, but I would prefer not to use CMD_RET_SUCCESS.
> > It is 0 and always will be.
> >
> > It encourages people to do things like:
> >
> > if (ret == CMD_RET_SUCCESS)
> >
> > instead of
> >
> > if (!ret)
>
> I see your concern. However we don't change the return value type to
> enum, so people can still use
>
> if (!ret)
>
> I would still defend that we should use CMD_RET_SUCCESS.
>
> This is like EXIT_XXX defined in stdlib.h:
>
> #define EXIT_FAILURE 1 /* Failing exit status. */
> #define EXIT_SUCCESS 0 /* Successful exit status. */
>
> One should use predefined macros whenever possible.
I agree except for success, where I don't, sorry. It should always be 0 in
U-Boot.
People then have to look up the value and also we get things like
if (ret != CMD_RET_SUCCESS)
It is a slippery and I would rather not start down it.
>
> >
> > It would eventually creep into everything, including our clean error
handling.
> >
> > > } else if (strcmp(argv[1], "write") == 0) {
> > > phys_addr_t paddr = hextoul(argv[2], NULL);
> > > lbaint_t blk = hextoul(argv[3], NULL);
> > > @@ -98,7 +98,7 @@ int blk_common_cmd(int argc, char *const argv[],
enum uclass_id uclass_id,
> > >
> > > printf("%ld blocks written: %s\n", n,
> > > n == cnt ? "OK" : "ERROR");
> > > - return n == cnt ? 0 : 1;
> > > + return n == cnt ? CMD_RET_SUCCESS :
CMD_RET_FAILURE;
> > > } else {
> > > return CMD_RET_USAGE;
> > > }
> > > --
Regards,
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: [PATCH 10/15] cmd: blk_common: Use macros for the return values
2023-10-10 14:58 ` Simon Glass
@ 2023-10-10 20:17 ` Tom Rini
0 siblings, 0 replies; 38+ messages in thread
From: Tom Rini @ 2023-10-10 20:17 UTC (permalink / raw)
To: Simon Glass; +Cc: Bin Meng, Bin Meng, U-Boot Mailing List, Tobias Waldekranz
[-- Attachment #1: Type: text/plain, Size: 4843 bytes --]
On Tue, Oct 10, 2023 at 08:58:04AM -0600, Simon Glass wrote:
> Hi Bin,
>
> On Tue, 10 Oct 2023 at 03:06, Bin Meng <bmeng.cn@gmail.com> wrote:
> >
> > Hi Simon,
> >
> > On Mon, Oct 2, 2023 at 9:42 AM Simon Glass <sjg@chromium.org> wrote:
> > >
> > > Hi Bin,
> > >
> > > On Tue, 26 Sept 2023 at 02:54, Bin Meng <bmeng@tinylab.org> wrote:
> > > >
> > > > Avoid using magic number 0/1 for the command result.
> > > >
> > > > Signed-off-by: Bin Meng <bmeng@tinylab.org>
> > > > ---
> > > >
> > > > cmd/blk_common.c | 14 +++++++-------
> > > > 1 file changed, 7 insertions(+), 7 deletions(-)
> > > >
> > > > diff --git a/cmd/blk_common.c b/cmd/blk_common.c
> > > > index 9f9d4327a9..ad9b16dc09 100644
> > > > --- a/cmd/blk_common.c
> > > > +++ b/cmd/blk_common.c
> > > > @@ -25,18 +25,18 @@ int blk_common_cmd(int argc, char *const argv[],
> enum uclass_id uclass_id,
> > > > case 2:
> > > > if (strncmp(argv[1], "inf", 3) == 0) {
> > > > blk_list_devices(uclass_id);
> > > > - return 0;
> > > > + return CMD_RET_SUCCESS;
> > >
> > > I really don't like this...0 is success.
> > >
> > > > } else if (strncmp(argv[1], "dev", 3) == 0) {
> > > > if (blk_print_device_num(uclass_id,
> *cur_devnump)) {
> > > > printf("\nno %s devices available\n",
> if_name);
> > > > return CMD_RET_FAILURE;
> > > > }
> > > > - return 0;
> > > > + return CMD_RET_SUCCESS;
> > > > } else if (strncmp(argv[1], "part", 4) == 0) {
> > > > if (blk_list_part(uclass_id))
> > > > printf("\nno %s partition table
> available\n",
> > > > if_name);
> > > > - return 0;
> > > > + return CMD_RET_SUCCESS;
> > > > }
> > > > return CMD_RET_USAGE;
> > > > case 3:
> > > > @@ -49,7 +49,7 @@ int blk_common_cmd(int argc, char *const argv[],
> enum uclass_id uclass_id,
> > > > } else {
> > > > return CMD_RET_FAILURE;
> > > > }
> > > > - return 0;
> > > > + return CMD_RET_SUCCESS;
> > > > } else if (strncmp(argv[1], "part", 4) == 0) {
> > > > int dev = (int)dectoul(argv[2], NULL);
> > > >
> > > > @@ -58,7 +58,7 @@ int blk_common_cmd(int argc, char *const argv[],
> enum uclass_id uclass_id,
> > > > if_name, dev);
> > > > return CMD_RET_FAILURE;
> > > > }
> > > > - return 0;
> > > > + return CMD_RET_SUCCESS;
> > > > }
> > > > return CMD_RET_USAGE;
> > > >
> > > > @@ -80,7 +80,7 @@ int blk_common_cmd(int argc, char *const argv[],
> enum uclass_id uclass_id,
> > > >
> > > > printf("%ld blocks read: %s\n", n,
> > > > n == cnt ? "OK" : "ERROR");
> > > > - return n == cnt ? 0 : 1;
> > > > + return n == cnt ? CMD_RET_SUCCESS :
> CMD_RET_FAILURE;
> > >
> > > CMD_RET_FAILURE is OK, but I would prefer not to use CMD_RET_SUCCESS.
> > > It is 0 and always will be.
> > >
> > > It encourages people to do things like:
> > >
> > > if (ret == CMD_RET_SUCCESS)
> > >
> > > instead of
> > >
> > > if (!ret)
> >
> > I see your concern. However we don't change the return value type to
> > enum, so people can still use
> >
> > if (!ret)
> >
> > I would still defend that we should use CMD_RET_SUCCESS.
> >
> > This is like EXIT_XXX defined in stdlib.h:
> >
> > #define EXIT_FAILURE 1 /* Failing exit status. */
> > #define EXIT_SUCCESS 0 /* Successful exit status. */
> >
> > One should use predefined macros whenever possible.
>
> I agree except for success, where I don't, sorry. It should always be 0 in
> U-Boot.
>
> People then have to look up the value and also we get things like
>
> if (ret != CMD_RET_SUCCESS)
>
> It is a slippery and I would rather not start down it.
To be clear, we have a large amount of code today that uses
CMD_RET_SUCCESS in the way this patch converts to. We do have a few
cases even of the kind of code you worry about seeing, but I think in
context clearer (it's in security or OTP fuse contexts) and the compiler
will do the right thing regardless.
Reviewed-by: Tom Rini <trini@konsulko.com>
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH 11/15] dm: blk: Rename get_desc() and make it externally visible
2023-09-26 8:43 [PATCH 00/15] blk: sandbox: Support binding a device with a given logical block size Bin Meng
` (9 preceding siblings ...)
2023-09-26 8:43 ` [PATCH 10/15] cmd: blk_common: Use macros for the return values Bin Meng
@ 2023-09-26 8:43 ` Bin Meng
2023-10-02 1:17 ` Simon Glass
2023-09-26 8:43 ` [PATCH 12/15] cmd: blk_common: Stop using hard-coded block size for Sandbox operations Bin Meng
` (5 subsequent siblings)
16 siblings, 1 reply; 38+ messages in thread
From: Bin Meng @ 2023-09-26 8:43 UTC (permalink / raw)
To: Simon Glass, U-Boot Mailing List
Cc: Abdellatif El Khlifi, Mattijs Korpershoek, Michal Suchanek,
Tobias Waldekranz
get_desc() can be useful outside blk-uclass.c. Let's change it to
an API and make it externally visible.
Signed-off-by: Bin Meng <bmeng@tinylab.org>
---
drivers/block/blk-uclass.c | 26 ++++++++------------------
include/blk.h | 12 ++++++++++++
2 files changed, 20 insertions(+), 18 deletions(-)
diff --git a/drivers/block/blk-uclass.c b/drivers/block/blk-uclass.c
index 614b975e25..9407621fb2 100644
--- a/drivers/block/blk-uclass.c
+++ b/drivers/block/blk-uclass.c
@@ -176,17 +176,7 @@ struct blk_desc *blk_get_by_device(struct udevice *dev)
return NULL;
}
-/**
- * get_desc() - Get the block device descriptor for the given device number
- *
- * @uclass_id: Interface type
- * @devnum: Device number (0 = first)
- * @descp: Returns block device descriptor on success
- * Return: 0 on success, -ENODEV if there is no such device and no device
- * with a higher device number, -ENOENT if there is no such device but there
- * is one with a higher number, or other -ve on other error.
- */
-static int get_desc(enum uclass_id uclass_id, int devnum, struct blk_desc **descp)
+int blk_get_desc(enum uclass_id uclass_id, int devnum, struct blk_desc **descp)
{
bool found_more = false;
struct udevice *dev;
@@ -238,7 +228,7 @@ int blk_list_part(enum uclass_id uclass_id)
int ret;
for (ok = 0, devnum = 0;; ++devnum) {
- ret = get_desc(uclass_id, devnum, &desc);
+ ret = blk_get_desc(uclass_id, devnum, &desc);
if (ret == -ENODEV)
break;
else if (ret)
@@ -261,7 +251,7 @@ int blk_print_part_devnum(enum uclass_id uclass_id, int devnum)
struct blk_desc *desc;
int ret;
- ret = get_desc(uclass_id, devnum, &desc);
+ ret = blk_get_desc(uclass_id, devnum, &desc);
if (ret)
return ret;
if (desc->type == DEV_TYPE_UNKNOWN)
@@ -278,7 +268,7 @@ void blk_list_devices(enum uclass_id uclass_id)
int i;
for (i = 0;; ++i) {
- ret = get_desc(uclass_id, i, &desc);
+ ret = blk_get_desc(uclass_id, i, &desc);
if (ret == -ENODEV)
break;
else if (ret)
@@ -295,7 +285,7 @@ int blk_print_device_num(enum uclass_id uclass_id, int devnum)
struct blk_desc *desc;
int ret;
- ret = get_desc(uclass_id, devnum, &desc);
+ ret = blk_get_desc(uclass_id, devnum, &desc);
if (ret)
return ret;
printf("\nIDE device %d: ", devnum);
@@ -310,7 +300,7 @@ int blk_show_device(enum uclass_id uclass_id, int devnum)
int ret;
printf("\nDevice %d: ", devnum);
- ret = get_desc(uclass_id, devnum, &desc);
+ ret = blk_get_desc(uclass_id, devnum, &desc);
if (ret == -ENODEV || ret == -ENOENT) {
printf("unknown device\n");
return -ENODEV;
@@ -332,7 +322,7 @@ ulong blk_read_devnum(enum uclass_id uclass_id, int devnum, lbaint_t start,
ulong n;
int ret;
- ret = get_desc(uclass_id, devnum, &desc);
+ ret = blk_get_desc(uclass_id, devnum, &desc);
if (ret)
return ret;
n = blk_dread(desc, start, blkcnt, buffer);
@@ -348,7 +338,7 @@ ulong blk_write_devnum(enum uclass_id uclass_id, int devnum, lbaint_t start,
struct blk_desc *desc;
int ret;
- ret = get_desc(uclass_id, devnum, &desc);
+ ret = blk_get_desc(uclass_id, devnum, &desc);
if (ret)
return ret;
return blk_dwrite(desc, start, blkcnt, buffer);
diff --git a/include/blk.h b/include/blk.h
index 4a4365fbbf..0cd758d6f7 100644
--- a/include/blk.h
+++ b/include/blk.h
@@ -504,6 +504,18 @@ const char *blk_get_devtype(struct udevice *dev);
*/
struct blk_desc *blk_get_by_device(struct udevice *dev);
+/**
+ * blk_get_desc() - Get the block device descriptor for the given device number
+ *
+ * @uclass_id: Interface type
+ * @devnum: Device number (0 = first)
+ * @descp: Returns block device descriptor on success
+ * Return: 0 on success, -ENODEV if there is no such device and no device
+ * with a higher device number, -ENOENT if there is no such device but there
+ * is one with a higher number, or other -ve on other error.
+ */
+int blk_get_desc(enum uclass_id uclass_id, int devnum, struct blk_desc **descp);
+
#else
#include <errno.h>
/*
--
2.25.1
^ permalink raw reply related [flat|nested] 38+ messages in thread* [PATCH 12/15] cmd: blk_common: Stop using hard-coded block size for Sandbox operations
2023-09-26 8:43 [PATCH 00/15] blk: sandbox: Support binding a device with a given logical block size Bin Meng
` (10 preceding siblings ...)
2023-09-26 8:43 ` [PATCH 11/15] dm: blk: Rename get_desc() and make it externally visible Bin Meng
@ 2023-09-26 8:43 ` Bin Meng
2023-10-02 1:17 ` Simon Glass
2023-09-26 8:43 ` [PATCH 13/15] dm: blk: Drop blk_{read,write}_devnum() Bin Meng
` (4 subsequent siblings)
16 siblings, 1 reply; 38+ messages in thread
From: Bin Meng @ 2023-09-26 8:43 UTC (permalink / raw)
To: Simon Glass, U-Boot Mailing List; +Cc: Tobias Waldekranz
commit 3d2fc7971454 ("cmd: blk: Allow generic read/write operations to work in sandbox")
used the hard-coded block size (512) for accessing the sandbox host
device. Now that we have added support for non-512 block size for both
Sandbox host device and blkmap driver, let's stop using the hard-coded
block size.
Signed-off-by: Bin Meng <bmeng@tinylab.org>
---
cmd/blk_common.c | 20 ++++++++++++++------
1 file changed, 14 insertions(+), 6 deletions(-)
diff --git a/cmd/blk_common.c b/cmd/blk_common.c
index ad9b16dc09..02ac92837b 100644
--- a/cmd/blk_common.c
+++ b/cmd/blk_common.c
@@ -67,15 +67,19 @@ int blk_common_cmd(int argc, char *const argv[], enum uclass_id uclass_id,
phys_addr_t paddr = hextoul(argv[2], NULL);
lbaint_t blk = hextoul(argv[3], NULL);
ulong cnt = hextoul(argv[4], NULL);
+ struct blk_desc *desc;
void *vaddr;
ulong n;
+ int ret;
printf("\n%s read: device %d block # "LBAFU", count %lu ... ",
if_name, *cur_devnump, blk, cnt);
- vaddr = map_sysmem(paddr, 512 * cnt);
- n = blk_read_devnum(uclass_id, *cur_devnump, blk, cnt,
- vaddr);
+ ret = blk_get_desc(uclass_id, *cur_devnump, &desc);
+ if (ret)
+ return CMD_RET_FAILURE;
+ vaddr = map_sysmem(paddr, desc->blksz * cnt);
+ n = blk_dread(desc, blk, cnt, vaddr);
unmap_sysmem(vaddr);
printf("%ld blocks read: %s\n", n,
@@ -85,15 +89,19 @@ int blk_common_cmd(int argc, char *const argv[], enum uclass_id uclass_id,
phys_addr_t paddr = hextoul(argv[2], NULL);
lbaint_t blk = hextoul(argv[3], NULL);
ulong cnt = hextoul(argv[4], NULL);
+ struct blk_desc *desc;
void *vaddr;
ulong n;
+ int ret;
printf("\n%s write: device %d block # "LBAFU", count %lu ... ",
if_name, *cur_devnump, blk, cnt);
- vaddr = map_sysmem(paddr, 512 * cnt);
- n = blk_write_devnum(uclass_id, *cur_devnump, blk, cnt,
- vaddr);
+ ret = blk_get_desc(uclass_id, *cur_devnump, &desc);
+ if (ret)
+ return CMD_RET_FAILURE;
+ vaddr = map_sysmem(paddr, desc->blksz * cnt);
+ n = blk_dwrite(desc, blk, cnt, vaddr);
unmap_sysmem(vaddr);
printf("%ld blocks written: %s\n", n,
--
2.25.1
^ permalink raw reply related [flat|nested] 38+ messages in thread* [PATCH 13/15] dm: blk: Drop blk_{read,write}_devnum()
2023-09-26 8:43 [PATCH 00/15] blk: sandbox: Support binding a device with a given logical block size Bin Meng
` (11 preceding siblings ...)
2023-09-26 8:43 ` [PATCH 12/15] cmd: blk_common: Stop using hard-coded block size for Sandbox operations Bin Meng
@ 2023-09-26 8:43 ` Bin Meng
2023-10-02 1:17 ` Simon Glass
2023-09-26 8:43 ` [PATCH 14/15] disk: part: Print out the unknown device uclass id Bin Meng
` (3 subsequent siblings)
16 siblings, 1 reply; 38+ messages in thread
From: Bin Meng @ 2023-09-26 8:43 UTC (permalink / raw)
To: Simon Glass, U-Boot Mailing List
Cc: Abdellatif El Khlifi, Mattijs Korpershoek, Michal Suchanek,
Tobias Waldekranz
blk_{read,write}_devnum() are no longer used by anywhere in the
source tree. Drop them.
Signed-off-by: Bin Meng <bmeng@tinylab.org>
---
drivers/block/blk-uclass.c | 29 -----------------------------
include/blk.h | 26 --------------------------
2 files changed, 55 deletions(-)
diff --git a/drivers/block/blk-uclass.c b/drivers/block/blk-uclass.c
index 9407621fb2..a4c6cf194c 100644
--- a/drivers/block/blk-uclass.c
+++ b/drivers/block/blk-uclass.c
@@ -315,35 +315,6 @@ int blk_show_device(enum uclass_id uclass_id, int devnum)
return 0;
}
-ulong blk_read_devnum(enum uclass_id uclass_id, int devnum, lbaint_t start,
- lbaint_t blkcnt, void *buffer)
-{
- struct blk_desc *desc;
- ulong n;
- int ret;
-
- ret = blk_get_desc(uclass_id, devnum, &desc);
- if (ret)
- return ret;
- n = blk_dread(desc, start, blkcnt, buffer);
- if (IS_ERR_VALUE(n))
- return n;
-
- return n;
-}
-
-ulong blk_write_devnum(enum uclass_id uclass_id, int devnum, lbaint_t start,
- lbaint_t blkcnt, const void *buffer)
-{
- struct blk_desc *desc;
- int ret;
-
- ret = blk_get_desc(uclass_id, devnum, &desc);
- if (ret)
- return ret;
- return blk_dwrite(desc, start, blkcnt, buffer);
-}
-
int blk_select_hwpart(struct udevice *dev, int hwpart)
{
const struct blk_ops *ops = blk_get_ops(dev);
diff --git a/include/blk.h b/include/blk.h
index 0cd758d6f7..e1477df3b6 100644
--- a/include/blk.h
+++ b/include/blk.h
@@ -718,32 +718,6 @@ int blk_print_device_num(enum uclass_id uclass_id, int devnum);
*/
int blk_print_part_devnum(enum uclass_id uclass_id, int devnum);
-/**
- * blk_read_devnum() - read blocks from a device
- *
- * @uclass_id: Block device type
- * @devnum: Device number
- * @start: Start block number to read (0=first)
- * @blkcnt: Number of blocks to read
- * @buffer: Address to write data to
- * Return: number of blocks read, or -ve error number on error
- */
-ulong blk_read_devnum(enum uclass_id uclass_id, int devnum, lbaint_t start,
- lbaint_t blkcnt, void *buffer);
-
-/**
- * blk_write_devnum() - write blocks to a device
- *
- * @uclass_id: Block device type
- * @devnum: Device number
- * @start: Start block number to write (0=first)
- * @blkcnt: Number of blocks to write
- * @buffer: Address to read data from
- * Return: number of blocks written, or -ve error number on error
- */
-ulong blk_write_devnum(enum uclass_id uclass_id, int devnum, lbaint_t start,
- lbaint_t blkcnt, const void *buffer);
-
/**
* blk_select_hwpart_devnum() - select a hardware partition
*
--
2.25.1
^ permalink raw reply related [flat|nested] 38+ messages in thread* Re: [PATCH 13/15] dm: blk: Drop blk_{read,write}_devnum()
2023-09-26 8:43 ` [PATCH 13/15] dm: blk: Drop blk_{read,write}_devnum() Bin Meng
@ 2023-10-02 1:17 ` Simon Glass
0 siblings, 0 replies; 38+ messages in thread
From: Simon Glass @ 2023-10-02 1:17 UTC (permalink / raw)
To: Bin Meng
Cc: U-Boot Mailing List, Abdellatif El Khlifi, Mattijs Korpershoek,
Michal Suchanek, Tobias Waldekranz
On Tue, 26 Sept 2023 at 04:09, Bin Meng <bmeng@tinylab.org> wrote:
>
> blk_{read,write}_devnum() are no longer used by anywhere in the
> source tree. Drop them.
>
> Signed-off-by: Bin Meng <bmeng@tinylab.org>
> ---
>
> drivers/block/blk-uclass.c | 29 -----------------------------
> include/blk.h | 26 --------------------------
> 2 files changed, 55 deletions(-)
>
Reviewed-by: Simon Glass <sjg@chromium.org>
^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH 14/15] disk: part: Print out the unknown device uclass id
2023-09-26 8:43 [PATCH 00/15] blk: sandbox: Support binding a device with a given logical block size Bin Meng
` (12 preceding siblings ...)
2023-09-26 8:43 ` [PATCH 13/15] dm: blk: Drop blk_{read,write}_devnum() Bin Meng
@ 2023-09-26 8:43 ` Bin Meng
2023-10-02 1:17 ` Simon Glass
2023-09-26 8:43 ` [PATCH 15/15] disk: part: Handle blkmap device in print_part_header() Bin Meng
` (2 subsequent siblings)
16 siblings, 1 reply; 38+ messages in thread
From: Bin Meng @ 2023-09-26 8:43 UTC (permalink / raw)
To: Simon Glass, U-Boot Mailing List
Cc: Heiko Schocher, Heinrich Schuchardt, Joshua Watt,
Tobias Waldekranz
It's helpful to output the device uclass id for unknown devices
during the debugging process.
Signed-off-by: Bin Meng <bmeng@tinylab.org>
---
disk/part.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/disk/part.c b/disk/part.c
index eec02f5898..493e04943b 100644
--- a/disk/part.c
+++ b/disk/part.c
@@ -310,7 +310,7 @@ static void print_part_header(const char *type, struct blk_desc *dev_desc)
puts("EFI");
break;
default:
- puts("UNKNOWN");
+ printf("UNKNOWN(%d)", dev_desc->uclass_id);
break;
}
printf (" device %d -- Partition Type: %s\n\n",
--
2.25.1
^ permalink raw reply related [flat|nested] 38+ messages in thread* [PATCH 15/15] disk: part: Handle blkmap device in print_part_header()
2023-09-26 8:43 [PATCH 00/15] blk: sandbox: Support binding a device with a given logical block size Bin Meng
` (13 preceding siblings ...)
2023-09-26 8:43 ` [PATCH 14/15] disk: part: Print out the unknown device uclass id Bin Meng
@ 2023-09-26 8:43 ` Bin Meng
2023-10-02 1:17 ` Simon Glass
2023-09-26 8:58 ` [PATCH 00/15] blk: sandbox: Support binding a device with a given logical block size Heinrich Schuchardt
2023-10-11 1:49 ` Tom Rini
16 siblings, 1 reply; 38+ messages in thread
From: Bin Meng @ 2023-09-26 8:43 UTC (permalink / raw)
To: Simon Glass, U-Boot Mailing List
Cc: Heiko Schocher, Heinrich Schuchardt, Joshua Watt,
Tobias Waldekranz
Print out the blkmap device type when showing partition header for
a blkmap device.
Signed-off-by: Bin Meng <bmeng@tinylab.org>
---
disk/part.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/disk/part.c b/disk/part.c
index 493e04943b..6997a89775 100644
--- a/disk/part.c
+++ b/disk/part.c
@@ -309,6 +309,9 @@ static void print_part_header(const char *type, struct blk_desc *dev_desc)
case UCLASS_EFI_MEDIA:
puts("EFI");
break;
+ case UCLASS_BLKMAP:
+ puts("BLKMAP");
+ break;
default:
printf("UNKNOWN(%d)", dev_desc->uclass_id);
break;
--
2.25.1
^ permalink raw reply related [flat|nested] 38+ messages in thread* Re: [PATCH 00/15] blk: sandbox: Support binding a device with a given logical block size
2023-09-26 8:43 [PATCH 00/15] blk: sandbox: Support binding a device with a given logical block size Bin Meng
` (14 preceding siblings ...)
2023-09-26 8:43 ` [PATCH 15/15] disk: part: Handle blkmap device in print_part_header() Bin Meng
@ 2023-09-26 8:58 ` Heinrich Schuchardt
2023-09-26 14:11 ` Bin Meng
2023-10-11 1:49 ` Tom Rini
16 siblings, 1 reply; 38+ messages in thread
From: Heinrich Schuchardt @ 2023-09-26 8:58 UTC (permalink / raw)
To: Bin Meng
Cc: Abdellatif El Khlifi, Bin Meng, Heiko Schocher, Jaehoon Chung,
Johan Jonker, Joshua Watt, Marek Vasut, Mattijs Korpershoek,
Michal Suchanek, Peng Fan, Tobias Waldekranz, Simon Glass,
U-Boot Mailing List
On 9/26/23 10:43, Bin Meng wrote:
> At present on Sandbox when binding to a host backing file, the host
> block device is created with a hard-coded 512 bytes block size.
>
> Such assumption works for most cases, but for situation that with a raw
> image file dump from a pre-formatted GPT partitioned disk image from a
> 4KiB block size device, when binding this file to a host device and mapping
> this device to a blkmap, "blkmap" command like "blkmap part" won't work
> correctly, due to block size mismatch during parsing the partition table.
>
> This series updates Sandbox block driver, as well as the blkmap driver,
> to get rid of the hard-coded 512 bytes block size assumption.
>
> This series is available at u-boot-x86/blk for testing.
It is really good to have an easy way to test other block sizes.
We can also use QEMU for alternative block sizes:
$ qemu-system-riscv64 -device nvme,help
logical_block_size=<size>
physical_block_size=<size>
In the commit messages, please, make it clear that you refer to logical
block sizes and not to the physical block size.
Best regards
Heinrich
>
> Test log (512 block size):
>
> => host bind 0 test.img
> => host info
> dev blocks blksz label path
> 0 262144 512 0 test.img
> => blkmap create 0
> Created "0"
> => blkmap map 0 0 40000 linear host 0 0
> Block 0x0+0x40000 mapped to block 0x0 of "host 0"
> => blkmap info
> Device 0: Vendor: U-Boot Rev: 1.0 Prod: blkmap
> Type: Hard Disk
> Capacity: 128.0 MB = 0.1 GB (262144 x 512)
> => blkmap part
>
> Partition Map for BLKMAP device 0 -- Partition Type: EFI
>
> Part Start LBA End LBA Name
> Attributes
> Type GUID
> Partition GUID
> 1 0x00000022 0x000000bd "u-boot-spl"
> attrs: 0x0000000000000000
> type: 5b193300-fc78-40cd-8002-e86c45580b47
> (5b193300-fc78-40cd-8002-e86c45580b47)
> guid: 0bb6bb6e-4aac-4c27-be03-016b01e7b941
> 2 0x00000822 0x00000c84 "u-boot"
> attrs: 0x0000000000000000
> type: 2e54b353-1271-4842-806f-e436d6af6985
> (2e54b353-1271-4842-806f-e436d6af6985)
> guid: 91d50814-8e31-4cc0-97dc-779e1dc59056
> 3 0x00000c85 0x0000cc84 "rootfs"
> attrs: 0x0000000000000004
> type: 0fc63daf-8483-4772-8e79-3d69d8477de4
> (linux)
> guid: 42799722-6e55-46e6-afa9-529e7af3f03b
>
> Test log (4096 block size):
>
> => host bind 0 test.img 4096
> => host info
> dev blocks blksz label path
> 0 32768 4096 0 test.img
> => blkmap create 0
> Created "0"
> => blkmap map 0 0 8000 linear host 0 0
> Block 0x0+0x8000 mapped to block 0x0 of "host 0"
> => blkmap info
> Device 0: Vendor: U-Boot Rev: 1.0 Prod: blkmap
> Type: Hard Disk
> Capacity: 128.0 MB = 0.1 GB (32768 x 4096)
> => blkmap part
>
> Partition Map for BLKMAP device 0 -- Partition Type: EFI
>
> Part Start LBA End LBA Name
> Attributes
> Type GUID
> Partition GUID
> 1 0x00000100 0x00001fff "primary"
> attrs: 0x0000000000000000
> type: 0fc63daf-8483-4772-8e79-3d69d8477de4
> (linux)
> guid: eba904d7-72c1-4dbd-bb4e-36be49cba5e3
> 2 0x00002000 0x00007ffa "primary"
> attrs: 0x0000000000000000
> type: 0fc63daf-8483-4772-8e79-3d69d8477de4
> (linux)
> guid: c48c360e-db47-46da-ab87-26416fad3cd3
>
>
> Bin Meng (15):
> blk: Use a macro for the typical block size
> cmd: host: Mandate the filename parameter in the 'bind' command
> blk: sandbox: Support binding a device with a given logical block size
> blk: host_dev: Make host_sb_detach_file() and host_sb_ops static
> blk: host_dev: Sanity check on the size of host backing file
> cmd: host: Print out the block size of the host device
> blk: blkmap: Make bind/unbind routines static
> cmd: blkmap: Make map_handlers[] and its .fn static
> blk: blkmap: Support mapping to device of any block size
> cmd: blk_common: Use macros for the return values
> dm: blk: Rename get_desc() and make it externally visible
> cmd: blk_common: Stop using hard-coded block size for Sandbox
> operations
> dm: blk: Drop blk_{read,write}_devnum()
> disk: part: Print out the unknown device uclass id
> disk: part: Handle blkmap device in print_part_header()
>
> cmd/blk_common.c | 34 +++++++++++++++----------
> cmd/blkmap.c | 7 ++---
> cmd/host.c | 25 +++++++++++++-----
> common/usb_storage.c | 4 +--
> disk/part.c | 5 +++-
> drivers/ata/dwc_ahsata.c | 3 ++-
> drivers/ata/fsl_sata.c | 3 ++-
> drivers/ata/sata_mv.c | 3 ++-
> drivers/ata/sata_sil.c | 3 ++-
> drivers/block/blk-uclass.c | 51 +++++--------------------------------
> drivers/block/blkmap.c | 16 ++++++------
> drivers/block/host-uclass.c | 15 ++++++++---
> drivers/block/host_dev.c | 11 +++++---
> drivers/mmc/mmc-uclass.c | 2 +-
> drivers/nvme/nvme.c | 2 +-
> include/blk.h | 40 ++++++++++-------------------
> include/sandbox_host.h | 7 +++--
> test/dm/blk.c | 7 ++---
> test/dm/host.c | 26 +++++++++----------
> 19 files changed, 129 insertions(+), 135 deletions(-)
>
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: [PATCH 00/15] blk: sandbox: Support binding a device with a given logical block size
2023-09-26 8:58 ` [PATCH 00/15] blk: sandbox: Support binding a device with a given logical block size Heinrich Schuchardt
@ 2023-09-26 14:11 ` Bin Meng
0 siblings, 0 replies; 38+ messages in thread
From: Bin Meng @ 2023-09-26 14:11 UTC (permalink / raw)
To: Heinrich Schuchardt
Cc: Bin Meng, Abdellatif El Khlifi, Heiko Schocher, Jaehoon Chung,
Johan Jonker, Joshua Watt, Marek Vasut, Mattijs Korpershoek,
Michal Suchanek, Peng Fan, Tobias Waldekranz, Simon Glass,
U-Boot Mailing List
Hi Heinrich,
On Tue, Sep 26, 2023 at 4:58 PM Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> On 9/26/23 10:43, Bin Meng wrote:
> > At present on Sandbox when binding to a host backing file, the host
> > block device is created with a hard-coded 512 bytes block size.
> >
> > Such assumption works for most cases, but for situation that with a raw
> > image file dump from a pre-formatted GPT partitioned disk image from a
> > 4KiB block size device, when binding this file to a host device and mapping
> > this device to a blkmap, "blkmap" command like "blkmap part" won't work
> > correctly, due to block size mismatch during parsing the partition table.
> >
> > This series updates Sandbox block driver, as well as the blkmap driver,
> > to get rid of the hard-coded 512 bytes block size assumption.
> >
> > This series is available at u-boot-x86/blk for testing.
>
> It is really good to have an easy way to test other block sizes.
>
> We can also use QEMU for alternative block sizes:
>
> $ qemu-system-riscv64 -device nvme,help
> logical_block_size=<size>
> physical_block_size=<size>
Yeah, please report any issue you find.
>
> In the commit messages, please, make it clear that you refer to logical
> block sizes and not to the physical block size.
For drivers we rarely care about physical_block_size as they always
operate on LBA. But I can write it down clearly that logical block
size is used.
Regards,
Bin
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH 00/15] blk: sandbox: Support binding a device with a given logical block size
2023-09-26 8:43 [PATCH 00/15] blk: sandbox: Support binding a device with a given logical block size Bin Meng
` (15 preceding siblings ...)
2023-09-26 8:58 ` [PATCH 00/15] blk: sandbox: Support binding a device with a given logical block size Heinrich Schuchardt
@ 2023-10-11 1:49 ` Tom Rini
16 siblings, 0 replies; 38+ messages in thread
From: Tom Rini @ 2023-10-11 1:49 UTC (permalink / raw)
To: Bin Meng
Cc: Simon Glass, U-Boot Mailing List, Abdellatif El Khlifi, Bin Meng,
Heiko Schocher, Heinrich Schuchardt, Jaehoon Chung, Johan Jonker,
Joshua Watt, Marek Vasut, Mattijs Korpershoek, Michal Suchanek,
Peng Fan, Tobias Waldekranz
[-- Attachment #1: Type: text/plain, Size: 3567 bytes --]
On Tue, Sep 26, 2023 at 04:43:30PM +0800, Bin Meng wrote:
> At present on Sandbox when binding to a host backing file, the host
> block device is created with a hard-coded 512 bytes block size.
>
> Such assumption works for most cases, but for situation that with a raw
> image file dump from a pre-formatted GPT partitioned disk image from a
> 4KiB block size device, when binding this file to a host device and mapping
> this device to a blkmap, "blkmap" command like "blkmap part" won't work
> correctly, due to block size mismatch during parsing the partition table.
>
> This series updates Sandbox block driver, as well as the blkmap driver,
> to get rid of the hard-coded 512 bytes block size assumption.
>
> This series is available at u-boot-x86/blk for testing.
>
> Test log (512 block size):
>
> => host bind 0 test.img
> => host info
> dev blocks blksz label path
> 0 262144 512 0 test.img
> => blkmap create 0
> Created "0"
> => blkmap map 0 0 40000 linear host 0 0
> Block 0x0+0x40000 mapped to block 0x0 of "host 0"
> => blkmap info
> Device 0: Vendor: U-Boot Rev: 1.0 Prod: blkmap
> Type: Hard Disk
> Capacity: 128.0 MB = 0.1 GB (262144 x 512)
> => blkmap part
>
> Partition Map for BLKMAP device 0 -- Partition Type: EFI
>
> Part Start LBA End LBA Name
> Attributes
> Type GUID
> Partition GUID
> 1 0x00000022 0x000000bd "u-boot-spl"
> attrs: 0x0000000000000000
> type: 5b193300-fc78-40cd-8002-e86c45580b47
> (5b193300-fc78-40cd-8002-e86c45580b47)
> guid: 0bb6bb6e-4aac-4c27-be03-016b01e7b941
> 2 0x00000822 0x00000c84 "u-boot"
> attrs: 0x0000000000000000
> type: 2e54b353-1271-4842-806f-e436d6af6985
> (2e54b353-1271-4842-806f-e436d6af6985)
> guid: 91d50814-8e31-4cc0-97dc-779e1dc59056
> 3 0x00000c85 0x0000cc84 "rootfs"
> attrs: 0x0000000000000004
> type: 0fc63daf-8483-4772-8e79-3d69d8477de4
> (linux)
> guid: 42799722-6e55-46e6-afa9-529e7af3f03b
>
> Test log (4096 block size):
>
> => host bind 0 test.img 4096
> => host info
> dev blocks blksz label path
> 0 32768 4096 0 test.img
> => blkmap create 0
> Created "0"
> => blkmap map 0 0 8000 linear host 0 0
> Block 0x0+0x8000 mapped to block 0x0 of "host 0"
> => blkmap info
> Device 0: Vendor: U-Boot Rev: 1.0 Prod: blkmap
> Type: Hard Disk
> Capacity: 128.0 MB = 0.1 GB (32768 x 4096)
> => blkmap part
>
> Partition Map for BLKMAP device 0 -- Partition Type: EFI
>
> Part Start LBA End LBA Name
> Attributes
> Type GUID
> Partition GUID
> 1 0x00000100 0x00001fff "primary"
> attrs: 0x0000000000000000
> type: 0fc63daf-8483-4772-8e79-3d69d8477de4
> (linux)
> guid: eba904d7-72c1-4dbd-bb4e-36be49cba5e3
> 2 0x00002000 0x00007ffa "primary"
> attrs: 0x0000000000000000
> type: 0fc63daf-8483-4772-8e79-3d69d8477de4
> (linux)
> guid: c48c360e-db47-46da-ab87-26416fad3cd3
With minor changes to apply again, applied to u-boot/master, thanks!
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 38+ messages in thread