| Message ID | 20190222123205.20015-1-daniel@dd-wrt.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Richard Weinberger |
| Headers | show |
| Series | mtdram: Add module parameter name. | expand |
Hi Daniel, Please Cc: the MTD maintainers as advised by get_maintainers.pl. Daniel Danzberger <daniel@dd-wrt.com> wrote on Fri, 22 Feb 2019 13:32:05 +0100: > This parameter will overwrite the default name "mtdram test device", when set. Do you have a use case? > > Signed-off-by: Daniel Danzberger <daniel@dd-wrt.com> > --- > drivers/mtd/devices/mtdram.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/mtd/devices/mtdram.c b/drivers/mtd/devices/mtdram.c > index 46238796145f..1a8ec3756a1d 100644 > --- a/drivers/mtd/devices/mtdram.c > +++ b/drivers/mtd/devices/mtdram.c > @@ -18,6 +18,7 @@ > #include <linux/mtd/mtd.h> > #include <linux/mtd/mtdram.h> > > +static char name[32] = "mtdram test device"; > static unsigned long total_size = CONFIG_MTDRAM_TOTAL_SIZE; > static unsigned long erase_size = CONFIG_MTDRAM_ERASE_SIZE; > static unsigned long writebuf_size = 64; > @@ -31,6 +32,8 @@ module_param(erase_size, ulong, 0); > MODULE_PARM_DESC(erase_size, "Device erase block size in KiB"); > module_param(writebuf_size, ulong, 0); > MODULE_PARM_DESC(writebuf_size, "Device write buf size in Bytes (Default: 64)"); > +module_param_string(name, name, sizeof(name) - 1, 0); The module_param_string kernel doc says "@len: the maximum length of the string, incl. terminator" and later "@len is usually just sizeof(string)." So I suppose sizeof(name) will be enough. > +MODULE_PARM_DESC(name, "Device name"); > #endif > > // We could store these in the mtd structure, but we only support 1 device.. > @@ -170,7 +173,7 @@ static int __init init_mtdram(void) > mtd_info = NULL; > return -ENOMEM; > } > - err = mtdram_init_device(mtd_info, addr, MTDRAM_TOTAL_SIZE, "mtdram test device"); > + err = mtdram_init_device(mtd_info, addr, MTDRAM_TOTAL_SIZE, name); > if (err) { > vfree(addr); > kfree(mtd_info); Thanks, Miquèl
Hi Daniel, Also, "mtd: mtdram:" would be a better prefix, and you can drop the period at the end of the commit title. Daniel Danzberger <daniel@dd-wrt.com> wrote on Fri, 22 Feb 2019 13:32:05 +0100: > This parameter will overwrite the default name "mtdram test device", when set. > > Signed-off-by: Daniel Danzberger <daniel@dd-wrt.com> > --- > drivers/mtd/devices/mtdram.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/mtd/devices/mtdram.c b/drivers/mtd/devices/mtdram.c > index 46238796145f..1a8ec3756a1d 100644 > --- a/drivers/mtd/devices/mtdram.c > +++ b/drivers/mtd/devices/mtdram.c > @@ -18,6 +18,7 @@ > #include <linux/mtd/mtd.h> > #include <linux/mtd/mtdram.h> > > +static char name[32] = "mtdram test device"; > static unsigned long total_size = CONFIG_MTDRAM_TOTAL_SIZE; > static unsigned long erase_size = CONFIG_MTDRAM_ERASE_SIZE; > static unsigned long writebuf_size = 64; > @@ -31,6 +32,8 @@ module_param(erase_size, ulong, 0); > MODULE_PARM_DESC(erase_size, "Device erase block size in KiB"); > module_param(writebuf_size, ulong, 0); > MODULE_PARM_DESC(writebuf_size, "Device write buf size in Bytes (Default: 64)"); > +module_param_string(name, name, sizeof(name) - 1, 0); > +MODULE_PARM_DESC(name, "Device name"); > #endif > > // We could store these in the mtd structure, but we only support 1 device.. > @@ -170,7 +173,7 @@ static int __init init_mtdram(void) > mtd_info = NULL; > return -ENOMEM; > } > - err = mtdram_init_device(mtd_info, addr, MTDRAM_TOTAL_SIZE, "mtdram test device"); > + err = mtdram_init_device(mtd_info, addr, MTDRAM_TOTAL_SIZE, name); > if (err) { > vfree(addr); > kfree(mtd_info); Thanks, Miquèl
On Thu, Mar 7, 2019 at 2:21 PM Miquel Raynal <miquel.raynal@bootlin.com> wrote: > > Hi Daniel, > > Please Cc: the MTD maintainers as advised by get_maintainers.pl. > > Daniel Danzberger <daniel@dd-wrt.com> wrote on Fri, 22 Feb 2019 > 13:32:05 +0100: > > > This parameter will overwrite the default name "mtdram test device", when set. > > Do you have a use case? Me best guess is dealing with existing mtdparts. ...which is a sane use case IMHO. If we add this feature to mtdram, we should consider it for nandsim too.
On 3/7/19 2:21 PM, Miquel Raynal wrote: > Hi Daniel, > > Please Cc: the MTD maintainers as advised by get_maintainers.pl. > > Daniel Danzberger <daniel@dd-wrt.com> wrote on Fri, 22 Feb 2019 > 13:32:05 +0100: > >> This parameter will overwrite the default name "mtdram test device", when set. > > Do you have a use case ? I was testing some userspace mtd code that searches for a specific mtd partition by name. I didn't want to rename the partition name in the code every time I test with mtdram. I was thinking someone else might run into a similar situation, so I pushed it upstream. > >> >> Signed-off-by: Daniel Danzberger <daniel@dd-wrt.com> >> --- >> drivers/mtd/devices/mtdram.c | 5 ++++- >> 1 file changed, 4 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/mtd/devices/mtdram.c b/drivers/mtd/devices/mtdram.c >> index 46238796145f..1a8ec3756a1d 100644 >> --- a/drivers/mtd/devices/mtdram.c >> +++ b/drivers/mtd/devices/mtdram.c >> @@ -18,6 +18,7 @@ >> #include <linux/mtd/mtd.h> >> #include <linux/mtd/mtdram.h> >> >> +static char name[32] = "mtdram test device"; >> static unsigned long total_size = CONFIG_MTDRAM_TOTAL_SIZE; >> static unsigned long erase_size = CONFIG_MTDRAM_ERASE_SIZE; >> static unsigned long writebuf_size = 64; >> @@ -31,6 +32,8 @@ module_param(erase_size, ulong, 0); >> MODULE_PARM_DESC(erase_size, "Device erase block size in KiB"); >> module_param(writebuf_size, ulong, 0); >> MODULE_PARM_DESC(writebuf_size, "Device write buf size in Bytes (Default: 64)"); >> +module_param_string(name, name, sizeof(name) - 1, 0); > > The module_param_string kernel doc says > > "@len: the maximum length of the string, incl. terminator" > > and later > > "@len is usually just sizeof(string)." > > So I suppose sizeof(name) will be enough.Agree. > >> +MODULE_PARM_DESC(name, "Device name"); >> #endif >> >> // We could store these in the mtd structure, but we only support 1 device.. >> @@ -170,7 +173,7 @@ static int __init init_mtdram(void) >> mtd_info = NULL; >> return -ENOMEM; >> } >> - err = mtdram_init_device(mtd_info, addr, MTDRAM_TOTAL_SIZE, "mtdram test device"); >> + err = mtdram_init_device(mtd_info, addr, MTDRAM_TOTAL_SIZE, name); >> if (err) { >> vfree(addr); >> kfree(mtd_info); > > > Thanks, > Miquèl >
diff --git a/drivers/mtd/devices/mtdram.c b/drivers/mtd/devices/mtdram.c index 46238796145f..1a8ec3756a1d 100644 --- a/drivers/mtd/devices/mtdram.c +++ b/drivers/mtd/devices/mtdram.c @@ -18,6 +18,7 @@ #include <linux/mtd/mtd.h> #include <linux/mtd/mtdram.h> +static char name[32] = "mtdram test device"; static unsigned long total_size = CONFIG_MTDRAM_TOTAL_SIZE; static unsigned long erase_size = CONFIG_MTDRAM_ERASE_SIZE; static unsigned long writebuf_size = 64; @@ -31,6 +32,8 @@ module_param(erase_size, ulong, 0); MODULE_PARM_DESC(erase_size, "Device erase block size in KiB"); module_param(writebuf_size, ulong, 0); MODULE_PARM_DESC(writebuf_size, "Device write buf size in Bytes (Default: 64)"); +module_param_string(name, name, sizeof(name) - 1, 0); +MODULE_PARM_DESC(name, "Device name"); #endif // We could store these in the mtd structure, but we only support 1 device.. @@ -170,7 +173,7 @@ static int __init init_mtdram(void) mtd_info = NULL; return -ENOMEM; } - err = mtdram_init_device(mtd_info, addr, MTDRAM_TOTAL_SIZE, "mtdram test device"); + err = mtdram_init_device(mtd_info, addr, MTDRAM_TOTAL_SIZE, name); if (err) { vfree(addr); kfree(mtd_info);
This parameter will overwrite the default name "mtdram test device", when set. Signed-off-by: Daniel Danzberger <daniel@dd-wrt.com> --- drivers/mtd/devices/mtdram.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)