* [PATCH 1/1] mtd: block2mtd: add support for an optional custom MTD label
@ 2021-10-05 8:03 Joachim Wiberg
2021-10-08 7:27 ` Miquel Raynal
0 siblings, 1 reply; 4+ messages in thread
From: Joachim Wiberg @ 2021-10-05 8:03 UTC (permalink / raw)
To: linux-mtd; +Cc: Joern Engel, Miquel Raynal, 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..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 :
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>][,<label>]]\"");
static int __init block2mtd_init(void)
{
--
2.25.1
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 1/1] mtd: block2mtd: add support for an optional custom MTD label
2021-10-05 8:03 [PATCH 1/1] mtd: block2mtd: add support for an optional custom MTD label Joachim Wiberg
@ 2021-10-08 7:27 ` Miquel Raynal
2021-10-08 8:26 ` Joachim Wiberg
0 siblings, 1 reply; 4+ messages in thread
From: Miquel Raynal @ 2021-10-08 7:27 UTC (permalink / raw)
To: Joachim Wiberg; +Cc: linux-mtd, Joern Engel
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/
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 1/1] mtd: block2mtd: add support for an optional custom MTD label
2021-10-08 7:27 ` Miquel Raynal
@ 2021-10-08 8:26 ` Joachim Wiberg
2021-10-08 8:34 ` Miquel Raynal
0 siblings, 1 reply; 4+ messages in thread
From: Joachim Wiberg @ 2021-10-08 8:26 UTC (permalink / raw)
To: Miquel Raynal; +Cc: linux-mtd, Joern Engel
Hi Miquèl!
On Fri, Oct 08, 2021 at 09:27, Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> troglobit@gmail.com wrote on Tue, 5 Oct 2021 10:03:59 +0200:
>> [snip]
>> @@ -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?
Sure thing!
>> @@ -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.
OK, I'll see what I can do, it's for the positional module arguments,
so a FOO_MAX_ARGS or sth, I guess ...
>> @@ -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++)
used here as well ...
>> token[i] = strsep(&str, ",");
But what about each separate arg, is there any convention? I'll see
what I can find in other modules.
>> 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.
Well, it's a logical consequence of adding another module argument, but
you're right, this could be hit before as well if someone added multiple
commas without argument. (That breaks erase_size default value.)
Maybe this and the defines fit in the same separate "cleanup" patch?
>> - 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?
OK! (Just tried to follow the same log level used for erase_size.)
Thank you for the review, I'll post a v2 later today, or during the
weekend :)
Cheers
/Joachim
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 1/1] mtd: block2mtd: add support for an optional custom MTD label
2021-10-08 8:26 ` Joachim Wiberg
@ 2021-10-08 8:34 ` Miquel Raynal
0 siblings, 0 replies; 4+ messages in thread
From: Miquel Raynal @ 2021-10-08 8:34 UTC (permalink / raw)
To: Joachim Wiberg; +Cc: linux-mtd, Joern Engel
Hi Joachim,
troglobit@gmail.com wrote on Fri, 08 Oct 2021 10:26:46 +0200:
> Hi Miquèl!
>
> On Fri, Oct 08, 2021 at 09:27, Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> > troglobit@gmail.com wrote on Tue, 5 Oct 2021 10:03:59 +0200:
> >> [snip]
> >> @@ -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?
>
> Sure thing!
>
> >> @@ -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.
>
> OK, I'll see what I can do, it's for the positional module arguments,
> so a FOO_MAX_ARGS or sth, I guess ...
>
> >> @@ -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++)
>
> used here as well ...
>
> >> token[i] = strsep(&str, ",");
>
> But what about each separate arg, is there any convention? I'll see
> what I can find in other modules.
>
> >> 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.
>
> Well, it's a logical consequence of adding another module argument, but
> you're right, this could be hit before as well if someone added multiple
> commas without argument. (That breaks erase_size default value.)
In this case you can keep this change in the commit but you need to
explain it in the commit message.
> Maybe this and the defines fit in the same separate "cleanup" patch?
>
> >> - 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?
>
> OK! (Just tried to follow the same log level used for erase_size.)
Ok then keep the driver consistent, it's not a bit deal and we can
lower the level later if it's too noisy.
> Thank you for the review, I'll post a v2 later today, or during the
> weekend :)
>
> Cheers
> /Joachim
>
Thanks,
Miquèl
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-10-08 8:35 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-05 8:03 [PATCH 1/1] mtd: block2mtd: add support for an optional custom MTD label Joachim Wiberg
2021-10-08 7:27 ` Miquel Raynal
2021-10-08 8:26 ` Joachim Wiberg
2021-10-08 8:34 ` Miquel Raynal
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.