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

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

[Joachim Wiberg]

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..87cdf041a218 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 :
 		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];
+	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])) {
 		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);
+	}
+
+	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>][,<name>]]\"");
 
 static int __init block2mtd_init(void)
 {
-- 
2.25.1


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

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

[Miquel Raynal]

Hi Joachim,

troglobit@gmail.com wrote on Tue, 28 Sep 2021 17:49:38 +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.

We already have a label property that can be added in the DT, isn't
it enough?

Thanks,
Miquèl

> 
> 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..87cdf041a218 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 :
>  		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];
> +	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])) {
>  		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);
> +	}
> +
> +	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>][,<name>]]\"");
>  
>  static int __init block2mtd_init(void)
>  {


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

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

[Joachim Wiberg]

Hi Miquel!

On Tue, Sep 28, 2021 at 18:05, Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> Hi Joachim,
> troglobit@gmail.com wrote on Tue, 28 Sep 2021 17:49:38 +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.
> We already have a label property that can be added in the DT, isn't
> it enough?

Unless I'm missing something, no.  Sorry if if the description was a bit
unclear.  Let me try expanding on it a bit.

The block2mtd driver is used a lot when emulating proper MTDs, I'm sure
we are not alone in building system images that a flashed to an MTD
device, which we want to be able to test in Qemu before deploying.

So what we typically do is have a read-only rootfs (squashfs) on one
partition, and another for config data that uses overlayfs to get a
writable /etc, for instance.  This is a reduced example, and the order
of parititions sometimes differ, so we use parition labels. When we test
in Qemu there's only the automatic DTB from Qemu itself that's injected,
e.g.

  qemu-system-aarch64 -M virt -cpu cortex-a53 -nographic       \
	-kernel Image -initrd rootfs.img -append               \
          "root=/dev/ram block2mtd.block2mtd=/dev/vda"         \
        -drive file=config.img,if=none,format=raw,id=hd0       \
        -device virtio-blk-device,drive=hd0

With the default blockt2mtd args we convert the emulated disk to be
used as an MTD partition, but the name we get in Linux is not the
name we have in our /etc/fstab on a real target.

  # cat /proc/mtd 
  dev:    size   erasesize  name
  mtd0: 00f00000 00001000 "block2mtd: /dev/vda"

With the patch we can align the two cases; the embedded system and the
emulated one, by using the same paritition label.  I.e., with the patch
and the new argument to the kernel cmdline:

  qemu-system-aarch64 -M virt -cpu cortex-a53 -nographic       \
	-kernel Image -initrd rootfs.img -append               \
          "root=/dev/ram block2mtd.block2mtd=/dev/vda,,Config" \
        -drive file=config.img,if=none,format=raw,id=hd0       \
        -device virtio-blk-device,drive=hd0

Linux gives the emulated MTD the same partition name we use in the
real embedded system:

  # cat /proc/mtd 
  dev:    size   erasesize  name
  mtd0: 00f00000 00001000 "Config"


Best regards
 /Joachim

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

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

[Miquel Raynal]

Hi Joachim,

troglobit@gmail.com wrote on Wed, 29 Sep 2021 17:27:20 +0200:

> Hi Miquel!
> 
> On Tue, Sep 28, 2021 at 18:05, Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> > Hi Joachim,
> > troglobit@gmail.com wrote on Tue, 28 Sep 2021 17:49:38 +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.  
> > We already have a label property that can be added in the DT, isn't
> > it enough?  
> 
> Unless I'm missing something, no.  Sorry if if the description was a bit
> unclear.  Let me try expanding on it a bit.
> 
> The block2mtd driver is used a lot when emulating proper MTDs, I'm sure
> we are not alone in building system images that a flashed to an MTD
> device, which we want to be able to test in Qemu before deploying.
> 
> So what we typically do is have a read-only rootfs (squashfs) on one
> partition, and another for config data that uses overlayfs to get a
> writable /etc, for instance.  This is a reduced example, and the order
> of parititions sometimes differ, so we use parition labels. When we test
> in Qemu there's only the automatic DTB from Qemu itself that's injected,
> e.g.
> 
>   qemu-system-aarch64 -M virt -cpu cortex-a53 -nographic       \
> 	-kernel Image -initrd rootfs.img -append               \
>           "root=/dev/ram block2mtd.block2mtd=/dev/vda"         \
>         -drive file=config.img,if=none,format=raw,id=hd0       \
>         -device virtio-blk-device,drive=hd0
> 
> With the default blockt2mtd args we convert the emulated disk to be
> used as an MTD partition, but the name we get in Linux is not the
> name we have in our /etc/fstab on a real target.
> 
>   # cat /proc/mtd 
>   dev:    size   erasesize  name
>   mtd0: 00f00000 00001000 "block2mtd: /dev/vda"
> 
> With the patch we can align the two cases; the embedded system and the
> emulated one, by using the same paritition label.  I.e., with the patch
> and the new argument to the kernel cmdline:
> 
>   qemu-system-aarch64 -M virt -cpu cortex-a53 -nographic       \
> 	-kernel Image -initrd rootfs.img -append               \
>           "root=/dev/ram block2mtd.block2mtd=/dev/vda,,Config" \
>         -drive file=config.img,if=none,format=raw,id=hd0       \
>         -device virtio-blk-device,drive=hd0
> 
> Linux gives the emulated MTD the same partition name we use in the
> real embedded system:
> 
>   # cat /proc/mtd 
>   dev:    size   erasesize  name
>   mtd0: 00f00000 00001000 "Config"

I don't know this portion of the code very well, but indeed if this is
something only declared on the cmdline I understand the request.

Thanks,
Miquèl

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