Patchwork [01/44] mtd: add new API for handling MTD registration

login
register
mail settings
Submitter Dmitry Eremin-Solenikov
Date June 7, 2011, 1:36 p.m.
Message ID <1307453803-31950-2-git-send-email-dbaryshkov@gmail.com>
Download mbox | patch
Permalink /patch/99163/
State New
Headers show

Comments

Dmitry Eremin-Solenikov - June 7, 2011, 1:36 p.m.
Lots (nearly all) mtd drivers contain nearly the similar code that
calls parse_mtd_partitions, provides some platform-default values, if
parsing fails, and registers  mtd device.

This is an aim to provide single implementation of this scenario:
mtd_device_parse_register() which will handle all this parsing and
defaults.

Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
---
 drivers/mtd/mtdcore.c   |   30 ++++++++++++++++++++++++++++++
 include/linux/mtd/mtd.h |    5 +++++
 2 files changed, 35 insertions(+), 0 deletions(-)
Jamie Iles - June 7, 2011, 1:44 p.m.
Hi Dmitry,

This sounds like a good idea.  Nitpick inline.

Jamie

On Tue, Jun 07, 2011 at 05:36:00PM +0400, Dmitry Eremin-Solenikov wrote:
> Lots (nearly all) mtd drivers contain nearly the similar code that
> calls parse_mtd_partitions, provides some platform-default values, if
> parsing fails, and registers  mtd device.
> 
> This is an aim to provide single implementation of this scenario:
> mtd_device_parse_register() which will handle all this parsing and
> defaults.
> 
> Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
> ---
>  drivers/mtd/mtdcore.c   |   30 ++++++++++++++++++++++++++++++
>  include/linux/mtd/mtd.h |    5 +++++
>  2 files changed, 35 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
> index c510aff..ac871ad 100644
> --- a/drivers/mtd/mtdcore.c
> +++ b/drivers/mtd/mtdcore.c
> @@ -451,6 +451,36 @@ int mtd_device_register(struct mtd_info *master,
>  }
>  EXPORT_SYMBOL_GPL(mtd_device_register);
>  
> +int mtd_device_parse_register(struct mtd_info *mtd,
> +			      const char **part_probe_types,
> +			      unsigned long origin,
> +			      const struct mtd_partition *defparts,
> +			      int defnr_parts)
> +{
> +	int err;
> +	struct mtd_partition *parts;
> +
> +	err = parse_mtd_partitions(mtd, part_probe_types, &parts, origin);
> +	if (err <= 0 && defnr_parts) {
> +		unsigned long size = sizeof(*parts) * defnr_parts;
> +		err = defnr_parts;
> +		parts = kzalloc(size, GFP_KERNEL);
> +		memcpy(parts, defparts, size);

Shouldn't this check the return of kzalloc()?  How about using kmemdup() 
instead of kzalloc() + memcpy()?  Also some kernel-doc to describe the 
search order would be nice.

Jamie
Dmitry Eremin-Solenikov - June 7, 2011, 2:33 p.m.
On 07.06.2011 17:44, Jamie Iles wrote:
> Hi Dmitry,
>
> This sounds like a good idea.  Nitpick inline.
>
> Jamie
>
> On Tue, Jun 07, 2011 at 05:36:00PM +0400, Dmitry Eremin-Solenikov wrote:
>> Lots (nearly all) mtd drivers contain nearly the similar code that
>> calls parse_mtd_partitions, provides some platform-default values, if
>> parsing fails, and registers  mtd device.
>>
>> This is an aim to provide single implementation of this scenario:
>> mtd_device_parse_register() which will handle all this parsing and
>> defaults.
>>
>> Signed-off-by: Dmitry Eremin-Solenikov<dbaryshkov@gmail.com>
>> ---
>>   drivers/mtd/mtdcore.c   |   30 ++++++++++++++++++++++++++++++
>>   include/linux/mtd/mtd.h |    5 +++++
>>   2 files changed, 35 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
>> index c510aff..ac871ad 100644
>> --- a/drivers/mtd/mtdcore.c
>> +++ b/drivers/mtd/mtdcore.c
>> @@ -451,6 +451,36 @@ int mtd_device_register(struct mtd_info *master,
>>   }
>>   EXPORT_SYMBOL_GPL(mtd_device_register);
>>
>> +int mtd_device_parse_register(struct mtd_info *mtd,
>> +			      const char **part_probe_types,
>> +			      unsigned long origin,
>> +			      const struct mtd_partition *defparts,
>> +			      int defnr_parts)
>> +{
>> +	int err;
>> +	struct mtd_partition *parts;
>> +
>> +	err = parse_mtd_partitions(mtd, part_probe_types,&parts, origin);
>> +	if (err<= 0&&  defnr_parts) {
>> +		unsigned long size = sizeof(*parts) * defnr_parts;
>> +		err = defnr_parts;
>> +		parts = kzalloc(size, GFP_KERNEL);
>> +		memcpy(parts, defparts, size);
>
> Shouldn't this check the return of kzalloc()?  How about using kmemdup()
> instead of kzalloc() + memcpy()?

Good catch! Thank you.

>  Also some kernel-doc to describe the search order would be nice.

Search order is mainly detailed either by defaults specified in 
mtdpart.c or by probe types which user specifies.

Patch

diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
index c510aff..ac871ad 100644
--- a/drivers/mtd/mtdcore.c
+++ b/drivers/mtd/mtdcore.c
@@ -451,6 +451,36 @@  int mtd_device_register(struct mtd_info *master,
 }
 EXPORT_SYMBOL_GPL(mtd_device_register);
 
+int mtd_device_parse_register(struct mtd_info *mtd,
+			      const char **part_probe_types,
+			      unsigned long origin,
+			      const struct mtd_partition *defparts,
+			      int defnr_parts)
+{
+	int err;
+	struct mtd_partition *parts;
+
+	err = parse_mtd_partitions(mtd, part_probe_types, &parts, origin);
+	if (err <= 0 && defnr_parts) {
+		unsigned long size = sizeof(*parts) * defnr_parts;
+		err = defnr_parts;
+		parts = kzalloc(size, GFP_KERNEL);
+		memcpy(parts, defparts, size);
+	}
+
+	if (err > 0) {
+		err = add_mtd_partitions(mtd, parts, err);
+		kfree(parts);
+	} else {
+		err = add_mtd_device(mtd);
+		if (err == 1)
+			err = -ENODEV;
+	}
+
+	return err;
+}
+EXPORT_SYMBOL_GPL(mtd_device_parse_register);
+
 /**
  * mtd_device_unregister - unregister an existing MTD device.
  *
diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h
index 2541fb8..d28a241 100644
--- a/include/linux/mtd/mtd.h
+++ b/include/linux/mtd/mtd.h
@@ -327,6 +327,11 @@  struct mtd_partition;
 extern int mtd_device_register(struct mtd_info *master,
 			       const struct mtd_partition *parts,
 			       int nr_parts);
+extern int mtd_device_parse_register(struct mtd_info *mtd,
+			      const char **part_probe_types,
+			      unsigned long origin,
+			      const struct mtd_partition *defparts,
+			      int defnr_parts);
 extern int mtd_device_unregister(struct mtd_info *master);
 extern struct mtd_info *get_mtd_device(struct mtd_info *mtd, int num);
 extern int __get_mtd_device(struct mtd_info *mtd);