mtdram: Add module parameter name.
diff mbox series

Message ID 20190222123205.20015-1-daniel@dd-wrt.com
State Changes Requested
Delegated to: Richard Weinberger
Headers show
Series
  • mtdram: Add module parameter name.
Related show

Commit Message

Daniel Danzberger Feb. 22, 2019, 12:32 p.m. UTC
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(-)

Comments

Miquel Raynal March 7, 2019, 1:21 p.m. UTC | #1
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
Miquel Raynal March 7, 2019, 1:24 p.m. UTC | #2
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
Richard Weinberger March 7, 2019, 3:01 p.m. UTC | #3
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.
Daniel Danzberger March 7, 2019, 3:11 p.m. UTC | #4
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
>

Patch
diff mbox series

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);