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

[Miquel Raynal <miquel.raynal@bootlin.com>]

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>][,<label>]]\"");
>  
>  static int __init block2mtd_init(void)
>  {


Thanks,
Miquèl

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/