Hi Sughosh
On Mon, 20 Jan 2025 at 12:51, Sughosh Ganu <sughosh.g...@linaro.org> wrote:
>
> Add information about the type of blkmap device in the blkmap
> structure. Currently, the blkmap device is used for mapping to either
> a memory based block device, or another block device (linear
> mapping). Put information in the blkmap structure to identify if it is
> associated with a memory or linear mapped device. Which can then be
> used to take specific action based on the type of blkmap device.
I haven't followed up all the discussions yet, but I do have a question.
Are blkmap devices now unconditionally added in a pmem node? (if they
are backed by memory)
Thanks
/Ilias
>
> Signed-off-by: Sughosh Ganu <sughosh.g...@linaro.org>
> ---
> Changes since V2: New patch
>
> cmd/blkmap.c | 16 ++++++++++++----
> drivers/block/blkmap.c | 10 +++++++++-
> drivers/block/blkmap_helper.c | 2 +-
> include/blkmap.h | 12 +++++++++++-
> test/dm/blkmap.c | 16 ++++++++--------
> 5 files changed, 41 insertions(+), 15 deletions(-)
>
> diff --git a/cmd/blkmap.c b/cmd/blkmap.c
> index 164f80f1387..1bf0747ab16 100644
> --- a/cmd/blkmap.c
> +++ b/cmd/blkmap.c
> @@ -119,15 +119,23 @@ static int do_blkmap_map(struct cmd_tbl *cmdtp, int
> flag,
> static int do_blkmap_create(struct cmd_tbl *cmdtp, int flag,
> int argc, char *const argv[])
> {
> + enum blkmap_type type;
> const char *label;
> int err;
>
> - if (argc != 2)
> + if (argc != 3)
> return CMD_RET_USAGE;
>
> label = argv[1];
>
> - err = blkmap_create(label, NULL);
> + if (!strcmp(argv[2], "linear"))
> + type = BLKMAP_LINEAR;
> + else if (!strcmp(argv[2], "mem"))
> + type = BLKMAP_MEM;
> + else
> + return CMD_RET_USAGE;
> +
> + err = blkmap_create(label, NULL, type);
> if (err) {
> printf("Unable to create \"%s\": %d\n", label, err);
> return CMD_RET_FAILURE;
> @@ -218,7 +226,7 @@ U_BOOT_CMD_WITH_SUBCMDS(
> "blkmap read <addr> <blk#> <cnt>\n"
> "blkmap write <addr> <blk#> <cnt>\n"
> "blkmap get <label> dev [<var>] - store device number in variable\n"
> - "blkmap create <label> - create device\n"
> + "blkmap create <label> <type> - create device(linear/mem)\n"
> "blkmap destroy <label> - destroy device\n"
> "blkmap map <label> <blk#> <cnt> linear <interface> <dev> <blk#> -
> device mapping\n"
> "blkmap map <label> <blk#> <cnt> mem <addr> - memory mapping\n",
> @@ -228,6 +236,6 @@ U_BOOT_CMD_WITH_SUBCMDS(
> U_BOOT_SUBCMD_MKENT(read, 5, 1, do_blkmap_common),
> U_BOOT_SUBCMD_MKENT(write, 5, 1, do_blkmap_common),
> U_BOOT_SUBCMD_MKENT(get, 5, 1, do_blkmap_get),
> - U_BOOT_SUBCMD_MKENT(create, 2, 1, do_blkmap_create),
> + U_BOOT_SUBCMD_MKENT(create, 3, 1, do_blkmap_create),
> U_BOOT_SUBCMD_MKENT(destroy, 2, 1, do_blkmap_destroy),
> U_BOOT_SUBCMD_MKENT(map, 32, 1, do_blkmap_map));
> diff --git a/drivers/block/blkmap.c b/drivers/block/blkmap.c
> index 34eed1380dc..a817345b6bc 100644
> --- a/drivers/block/blkmap.c
> +++ b/drivers/block/blkmap.c
> @@ -153,6 +153,9 @@ int blkmap_map_linear(struct udevice *dev, lbaint_t
> blknr, lbaint_t blkcnt,
> struct blk_desc *bd, *lbd;
> int err;
>
> + if (bm->type != BLKMAP_LINEAR)
> + return log_msg_ret("Invalid blkmap type", -EINVAL);
> +
> bd = dev_get_uclass_plat(bm->blk);
> lbd = dev_get_uclass_plat(lblk);
> if (lbd->blksz != bd->blksz) {
> @@ -240,6 +243,9 @@ int __blkmap_map_mem(struct udevice *dev, lbaint_t blknr,
> lbaint_t blkcnt,
> struct blkmap_mem *bmm;
> int err;
>
> + if (bm->type != BLKMAP_MEM)
> + return log_msg_ret("Invalid blkmap type", -EINVAL);
> +
> bmm = malloc(sizeof(*bmm));
> if (!bmm)
> return -ENOMEM;
> @@ -435,7 +441,8 @@ struct udevice *blkmap_from_label(const char *label)
> return NULL;
> }
>
> -int blkmap_create(const char *label, struct udevice **devp)
> +int blkmap_create(const char *label, struct udevice **devp,
> + enum blkmap_type type)
> {
> char *hname, *hlabel;
> struct udevice *dev;
> @@ -472,6 +479,7 @@ int blkmap_create(const char *label, struct udevice
> **devp)
> device_set_name_alloced(dev);
> bm = dev_get_plat(dev);
> bm->label = hlabel;
> + bm->type = type;
>
> if (devp)
> *devp = dev;
> diff --git a/drivers/block/blkmap_helper.c b/drivers/block/blkmap_helper.c
> index bfba14110d2..56cbe57d4aa 100644
> --- a/drivers/block/blkmap_helper.c
> +++ b/drivers/block/blkmap_helper.c
> @@ -19,7 +19,7 @@ int blkmap_create_ramdisk(const char *label, ulong
> image_addr, ulong image_size,
> struct blk_desc *desc;
> struct udevice *bm_dev;
>
> - ret = blkmap_create(label, &bm_dev);
> + ret = blkmap_create(label, &bm_dev, BLKMAP_MEM);
> if (ret) {
> log_err("failed to create blkmap\n");
> return ret;
> diff --git a/include/blkmap.h b/include/blkmap.h
> index d53095437fa..21169c30af1 100644
> --- a/include/blkmap.h
> +++ b/include/blkmap.h
> @@ -9,6 +9,12 @@
>
> #include <dm/lists.h>
>
> +/* Type of blkmap device, Linear or Memory */
> +enum blkmap_type {
> + BLKMAP_LINEAR = 1,
> + BLKMAP_MEM,
> +};
> +
> /**
> * struct blkmap - Block map
> *
> @@ -16,11 +22,13 @@
> *
> * @label: Human readable name of this blkmap
> * @blk: Underlying block device
> + * @type: Type of blkmap device
> * @slices: List of slices associated with this blkmap
> */
> struct blkmap {
> char *label;
> struct udevice *blk;
> + enum blkmap_type type;
> struct list_head slices;
> };
>
> @@ -78,9 +86,11 @@ struct udevice *blkmap_from_label(const char *label);
> *
> * @label: Label of the new blkmap
> * @devp: If not NULL, updated with the address of the resulting device
> + * @type: Type of blkmap device to create
> * Returns: 0 on success, negative error code on failure
> */
> -int blkmap_create(const char *label, struct udevice **devp);
> +int blkmap_create(const char *label, struct udevice **devp,
> + enum blkmap_type type);
>
> /**
> * blkmap_destroy() - Destroy blkmap
> diff --git a/test/dm/blkmap.c b/test/dm/blkmap.c
> index a6a0b4d4e20..06816cb4b54 100644
> --- a/test/dm/blkmap.c
> +++ b/test/dm/blkmap.c
> @@ -56,7 +56,7 @@ static int dm_test_blkmap_read(struct unit_test_state *uts)
> struct udevice *dev, *blk;
> const struct mapping *m;
>
> - ut_assertok(blkmap_create("rdtest", &dev));
> + ut_assertok(blkmap_create("rdtest", &dev, BLKMAP_MEM));
> ut_assertok(blk_get_from_parent(dev, &blk));
>
> /* Generate an ordered and an unordered pattern in memory */
> @@ -85,7 +85,7 @@ static int dm_test_blkmap_write(struct unit_test_state *uts)
> struct udevice *dev, *blk;
> const struct mapping *m;
>
> - ut_assertok(blkmap_create("wrtest", &dev));
> + ut_assertok(blkmap_create("wrtest", &dev, BLKMAP_MEM));
> ut_assertok(blk_get_from_parent(dev, &blk));
>
> /* Generate an ordered and an unordered pattern in memory */
> @@ -114,7 +114,7 @@ static int dm_test_blkmap_slicing(struct unit_test_state
> *uts)
> {
> struct udevice *dev;
>
> - ut_assertok(blkmap_create("slicetest", &dev));
> + ut_assertok(blkmap_create("slicetest", &dev, BLKMAP_MEM));
>
> ut_assertok(blkmap_map_mem(dev, 8, 8, NULL));
>
> @@ -140,19 +140,19 @@ static int dm_test_blkmap_creation(struct
> unit_test_state *uts)
> {
> struct udevice *first, *second;
>
> - ut_assertok(blkmap_create("first", &first));
> + ut_assertok(blkmap_create("first", &first, BLKMAP_LINEAR));
>
> /* Can't have two "first"s */
> - ut_asserteq(-EBUSY, blkmap_create("first", &second));
> + ut_asserteq(-EBUSY, blkmap_create("first", &second, BLKMAP_LINEAR));
>
> /* But "second" should be fine */
> - ut_assertok(blkmap_create("second", &second));
> + ut_assertok(blkmap_create("second", &second, BLKMAP_LINEAR));
>
> /* Once "first" is destroyed, we should be able to create it
> * again
> */
> ut_assertok(blkmap_destroy(first));
> - ut_assertok(blkmap_create("first", &first));
> + ut_assertok(blkmap_create("first", &first, BLKMAP_LINEAR));
>
> ut_assertok(blkmap_destroy(first));
> ut_assertok(blkmap_destroy(second));
> @@ -168,7 +168,7 @@ static int dm_test_cmd_blkmap(struct unit_test_state *uts)
> ut_assertok(run_command("blkmap info", 0));
> ut_assert_console_end();
>
> - ut_assertok(run_command("blkmap create ramdisk", 0));
> + ut_assertok(run_command("blkmap create ramdisk mem", 0));
> ut_assert_nextline("Created \"ramdisk\"");
> ut_assert_console_end();
>
> --
> 2.34.1
>
