Patchwork mtd/physmap: use parse_mtd()

login
register
mail settings
Submitter Mike Frysinger
Date Nov. 12, 2008, 11:39 p.m.
Message ID <1226533189-8492-1-git-send-email-vapier@gentoo.org>
Download mbox | patch
Permalink /patch/8559/
State Superseded, archived
Headers show

Comments

Mike Frysinger - Nov. 12, 2008, 11:39 p.m.
Call parse_mtd() to handle partition/device registration rather than doing
it all ourself.

Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
 drivers/mtd/maps/physmap.c |   21 +--------------------
 1 files changed, 1 insertions(+), 20 deletions(-)
Atsushi Nemoto - Nov. 13, 2008, 1:51 p.m.
On Wed, 12 Nov 2008 18:39:48 -0500, Mike Frysinger <vapier@gentoo.org> wrote:
> Call parse_mtd() to handle partition/device registration rather than doing
> it all ourself.
> 
> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
> ---
>  drivers/mtd/maps/physmap.c |   21 +--------------------
>  1 files changed, 1 insertions(+), 20 deletions(-)

You should adjust physmap_flash_remove() too.

* remove kfree(info->parts)
* call del_mtd_partitions() or del_mtd_device()

Maybe del_parsed_mtd() or something is required?

---
Atsushi Nemoto
Mike Frysinger - Nov. 13, 2008, 2:44 p.m.
On Thu, Nov 13, 2008 at 08:51, Atsushi Nemoto wrote:
> On Wed, 12 Nov 2008 18:39:48 -0500, Mike Frysinger wrote:
>> Call parse_mtd() to handle partition/device registration rather than doing
>> it all ourself.
>>
>> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
>> ---
>>  drivers/mtd/maps/physmap.c |   21 +--------------------
>>  1 files changed, 1 insertions(+), 20 deletions(-)
>
> You should adjust physmap_flash_remove() too.
>
> * remove kfree(info->parts)
> * call del_mtd_partitions() or del_mtd_device()
>
> Maybe del_parsed_mtd() or something is required?

yeah, this last bit sounds good
-mike
Andrew Morton - Nov. 14, 2008, 9:41 p.m.
On Wed, 12 Nov 2008 18:39:48 -0500
Mike Frysinger <vapier@gentoo.org> wrote:

> Call parse_mtd() to handle partition/device registration rather than doing
> it all ourself.
> 
> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
> ---
>  drivers/mtd/maps/physmap.c |   21 +--------------------
>  1 files changed, 1 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/mtd/maps/physmap.c b/drivers/mtd/maps/physmap.c
> index 42d844f..9b87fd8 100644
> --- a/drivers/mtd/maps/physmap.c
> +++ b/drivers/mtd/maps/physmap.c
> @@ -84,9 +84,6 @@ static int physmap_flash_remove(struct platform_device *dev)
>  }
>  
>  static const char *rom_probe_types[] = { "cfi_probe", "jedec_probe", "map_rom", NULL };
> -#ifdef CONFIG_MTD_PARTITIONS
> -static const char *part_probe_types[] = { "cmdlinepart", "RedBoot", NULL };
> -#endif
>  
>  static int physmap_flash_probe(struct platform_device *dev)
>  {
> @@ -170,23 +167,7 @@ static int physmap_flash_probe(struct platform_device *dev)
>  	if (err)
>  		goto err_out;
>  
> -#ifdef CONFIG_MTD_PARTITIONS
> -	err = parse_mtd_partitions(info->cmtd, part_probe_types, &info->parts, 0);
> -	if (err > 0) {
> -		add_mtd_partitions(info->cmtd, info->parts, err);
> -		return 0;
> -	}
> -
> -	if (physmap_data->nr_parts) {
> -		printk(KERN_NOTICE "Using physmap partition information\n");
> -		add_mtd_partitions(info->cmtd, physmap_data->parts,
> -				   physmap_data->nr_parts);
> -		return 0;
> -	}
> -#endif
> -
> -	add_mtd_device(info->cmtd);
> -	return 0;
> +	return parse_mtd(info->cmtd, NULL, physmap_data->parts, physmap_data->nr_parts);
>  
>  err_out:
>  	physmap_flash_remove(dev);

This didn't apply due to
physmap-fix-leak-of-memory-returned-by-parse_mtd_partitions.patch.  I
just smashed it in anyway.  Should I drop
physmap-fix-leak-of-memory-returned-by-parse_mtd_partitions.patch
instead?  Your changelog mentioned nothing about leak-fixing?
Mike Frysinger - Nov. 14, 2008, 10:07 p.m.
On Fri, Nov 14, 2008 at 16:41, Andrew Morton wrote:
> On Wed, 12 Nov 2008 18:39:48 -0500
> Mike Frysinger <vapier@gentoo.org> wrote:
>> Call parse_mtd() to handle partition/device registration rather than doing
>> it all ourself.
>>
>> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
>> ---
>>  drivers/mtd/maps/physmap.c |   21 +--------------------
>>  1 files changed, 1 insertions(+), 20 deletions(-)
>>
>> diff --git a/drivers/mtd/maps/physmap.c b/drivers/mtd/maps/physmap.c
>> index 42d844f..9b87fd8 100644
>> --- a/drivers/mtd/maps/physmap.c
>> +++ b/drivers/mtd/maps/physmap.c
>> @@ -84,9 +84,6 @@ static int physmap_flash_remove(struct platform_device *dev)
>>  }
>>
>>  static const char *rom_probe_types[] = { "cfi_probe", "jedec_probe", "map_rom", NULL };
>> -#ifdef CONFIG_MTD_PARTITIONS
>> -static const char *part_probe_types[] = { "cmdlinepart", "RedBoot", NULL };
>> -#endif
>>
>>  static int physmap_flash_probe(struct platform_device *dev)
>>  {
>> @@ -170,23 +167,7 @@ static int physmap_flash_probe(struct platform_device *dev)
>>       if (err)
>>               goto err_out;
>>
>> -#ifdef CONFIG_MTD_PARTITIONS
>> -     err = parse_mtd_partitions(info->cmtd, part_probe_types, &info->parts, 0);
>> -     if (err > 0) {
>> -             add_mtd_partitions(info->cmtd, info->parts, err);
>> -             return 0;
>> -     }
>> -
>> -     if (physmap_data->nr_parts) {
>> -             printk(KERN_NOTICE "Using physmap partition information\n");
>> -             add_mtd_partitions(info->cmtd, physmap_data->parts,
>> -                                physmap_data->nr_parts);
>> -             return 0;
>> -     }
>> -#endif
>> -
>> -     add_mtd_device(info->cmtd);
>> -     return 0;
>> +     return parse_mtd(info->cmtd, NULL, physmap_data->parts, physmap_data->nr_parts);
>>
>>  err_out:
>>       physmap_flash_remove(dev);
>
> This didn't apply due to
> physmap-fix-leak-of-memory-returned-by-parse_mtd_partitions.patch.  I
> just smashed it in anyway.  Should I drop
> physmap-fix-leak-of-memory-returned-by-parse_mtd_partitions.patch
> instead?  Your changelog mentioned nothing about leak-fixing?

i'd like feedback from the main mtd guy(s) first, but if you're going
to queue things, it should be with v2 that i posted rather than this
set.

while mine doesnt explicitly mention leak fixing, it does fix it in
the process of moving code about.  so if you do add v2, you can safely
drop the leak fix.
-mike

Patch

diff --git a/drivers/mtd/maps/physmap.c b/drivers/mtd/maps/physmap.c
index 42d844f..9b87fd8 100644
--- a/drivers/mtd/maps/physmap.c
+++ b/drivers/mtd/maps/physmap.c
@@ -84,9 +84,6 @@  static int physmap_flash_remove(struct platform_device *dev)
 }
 
 static const char *rom_probe_types[] = { "cfi_probe", "jedec_probe", "map_rom", NULL };
-#ifdef CONFIG_MTD_PARTITIONS
-static const char *part_probe_types[] = { "cmdlinepart", "RedBoot", NULL };
-#endif
 
 static int physmap_flash_probe(struct platform_device *dev)
 {
@@ -170,23 +167,7 @@  static int physmap_flash_probe(struct platform_device *dev)
 	if (err)
 		goto err_out;
 
-#ifdef CONFIG_MTD_PARTITIONS
-	err = parse_mtd_partitions(info->cmtd, part_probe_types, &info->parts, 0);
-	if (err > 0) {
-		add_mtd_partitions(info->cmtd, info->parts, err);
-		return 0;
-	}
-
-	if (physmap_data->nr_parts) {
-		printk(KERN_NOTICE "Using physmap partition information\n");
-		add_mtd_partitions(info->cmtd, physmap_data->parts,
-				   physmap_data->nr_parts);
-		return 0;
-	}
-#endif
-
-	add_mtd_device(info->cmtd);
-	return 0;
+	return parse_mtd(info->cmtd, NULL, physmap_data->parts, physmap_data->nr_parts);
 
 err_out:
 	physmap_flash_remove(dev);