Re: [PATCH 1/1] mtd: block2mtd: add support for an optional custom MTD label

Miquel Raynal
2021-10-08T07:28:37Z

Hi Joachim,

troglobit@gmail.com wrote on Tue, 5 Oct 2021 10:03:59 +0200:

> This patch adds support for an optional MTD label for mtd2block emulated
> MTD devices. Useful when, e.g. testing device images using Qemu. The
> following /etc/fstab line in can then be used to mount a file system
> regardless of the actual MTD partition number:
>
> mtd:Config /mnt jffs2 noatime,nodiratime 0 0
>
> Kernel command line syntax:
>
> block2mtd.block2mtd=/dev/sda,,Config
>
> The ',,' is the optional erase_size, which like before this patch,
> defaults to PAGE_SIZE when omitted.
>
> Signed-off-by: Joachim Wiberg <troglobit@gmail.com>
> ---
> drivers/mtd/devices/block2mtd.c | 26 +++++++++++++++++---------
> 1 file changed, 17 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/mtd/devices/block2mtd.c b/drivers/mtd/devices/block2mtd.c
> index c08721b11642..9465b8ff48f6 100644
> --- a/drivers/mtd/devices/block2mtd.c
> +++ b/drivers/mtd/devices/block2mtd.c
> @@ -214,7 +214,7 @@ static void block2mtd_free_device(struct block2mtd_dev *dev)
>
>
> static struct block2mtd_dev *add_device(char *devname, int erase_size,
> - int timeout)
> + char *label, int timeout)
> {
> #ifndef MODULE
> int i;
> @@ -278,7 +278,10 @@ static struct block2mtd_dev *add_device(char *devname, int erase_size,
>
> /* Setup the MTD structure */
> /* make the name contain the block device in */
> - name = kasprintf(GFP_KERNEL, "block2mtd: %s", devname);
> + if (!label)
> + name = kasprintf(GFP_KERNEL, "block2mtd: %s", devname);
> + else
> + name = kstrdup(label, GFP_KERNEL);
> if (!name)
> goto err_destroy_mutex;
>
> @@ -304,7 +307,7 @@ static struct block2mtd_dev *add_device(char *devname, int erase_size,
>
> list_add(&dev->list, &blkmtd_device_list);
> pr_info("mtd%d: [%s] erase_size = %dKiB [%d]\n",
> - dev->mtd.index,
> + dev->mtd.index, label ? label :

Can you put this on the next line?

> dev->mtd.name + strlen("block2mtd: "),
> dev->mtd.erasesize >> 10, dev->mtd.erasesize);
> return dev;
> @@ -381,8 +384,8 @@ static int block2mtd_setup2(const char *val)
> /* 80 for device, 12 for erase size, 80 for name, 8 for timeout */
> char buf[80 + 12 + 80 + 8];
> char *str = buf;
> - char *token[2];
> - char *name;
> + char *token[3];

This number deserves a define and should be used later on as well.

> + char *name, *label = NULL;
> size_t erase_size = PAGE_SIZE;
> unsigned long timeout = MTD_DEFAULT_TIMEOUT;
> int i, ret;
> @@ -395,7 +398,7 @@ static int block2mtd_setup2(const char *val)
> strcpy(str, val);
> kill_final_newline(str);
>
> - for (i = 0; i < 2; i++)
> + for (i = 0; i < 3; i++)
> token[i] = strsep(&str, ",");
>
> if (str) {
> @@ -414,7 +417,7 @@ static int block2mtd_setup2(const char *val)
> return 0;
> }
>
> - if (token[1]) {
> + if (token[1] && strlen(token[1])) {

This change is not related to your commit I believe, please split if
my assumption is right.

> ret = parse_num(&erase_size, token[1]);
> if (ret) {
> pr_err("illegal erase size\n");
> @@ -422,7 +425,12 @@ static int block2mtd_setup2(const char *val)
> }
> }
>
> - add_device(name, erase_size, timeout);
> + if (token[2]) {
> + label = token[2];
> + pr_info("Using custom MTD label '%s' for dev %s\n", label, name);

Not sure this qualifies as an info log message, perhaps a debug one
would be more appropriate?

> + }
> +
> + add_device(name, erase_size, label, timeout);
>
> return 0;
> }
> @@ -456,7 +464,7 @@ static int block2mtd_setup(const char *val, const struct kernel_param *kp)
>
>
> module_param_call(block2mtd, block2mtd_setup, NULL, NULL, 0200);
> -MODULE_PARM_DESC(block2mtd, "Device to use. \"block2mtd=<dev>[,<erasesize>]\"");
> +MODULE_PARM_DESC(block2mtd, "Device to use. \"block2mtd=<dev>[,[<erasesize>][, ]]\"");
>
> static int __init block2mtd_init(void)
> {

Thanks,
Miquèl