2008-11-12 23:41:53

by Mike Frysinger

[permalink] [raw]
Subject: [PATCH] mtd: unify mtd partition/device registration

Rather than having every map/mtd driver doing the same sequence of
registering partitions and/or devices, implement common parse_mtd().

Signed-off-by: Mike Frysinger <[email protected]>
---
drivers/mtd/mtdcore.c | 37 +++++++++++++++++++++++++++++++++++++
include/linux/mtd/mtd.h | 3 +++
2 files changed, 40 insertions(+), 0 deletions(-)

diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
index a9d2469..654ec1b 100644
--- a/drivers/mtd/mtdcore.c
+++ b/drivers/mtd/mtdcore.c
@@ -292,6 +292,43 @@ void put_mtd_device(struct mtd_info *mtd)
module_put(mtd->owner);
}

+#include <linux/mtd/partitions.h>
+
+/**
+ * parse_mtd - add partitions / devices
+ *
+ * If partitioning support is enabled, attempt to call parse_mtd_partitions()
+ * and add_mtd_partitions() with all available parsers. Otherwise just add
+ * the MTD device.
+ */
+
+int parse_mtd(struct mtd_info *mtd, const char **probe_types,
+ struct mtd_partition *parts, int nr_parts)
+{
+#ifdef CONFIG_MTD_PARTITIONS
+ const char *default_part_probe_types[] = {
+ "cmdlinepart",
+ "RedBoot",
+ NULL
+ };
+ int ret;
+
+ if (!probe_types)
+ probe_types = default_part_probe_types;
+
+ ret = parse_mtd_partitions(mtd, probe_types, &parts, 0);
+ if (ret > 0) {
+ ret = add_mtd_partitions(mtd, parts, ret);
+ kfree(parts);
+ return ret;
+ } else if (nr_parts)
+ return add_mtd_partitions(mtd, parts, nr_parts);
+#endif
+
+ return add_mtd_device(mtd);
+}
+EXPORT_SYMBOL(parse_mtd);
+
/* default_mtd_writev - default mtd writev method for MTD devices that
* don't implement their own
*/
diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h
index eae26bb..dec3456 100644
--- a/include/linux/mtd/mtd.h
+++ b/include/linux/mtd/mtd.h
@@ -232,6 +232,9 @@ extern struct mtd_info *get_mtd_device_nm(const char *name);

extern void put_mtd_device(struct mtd_info *mtd);

+struct mtd_partition;
+int parse_mtd(struct mtd_info *mtd, const char **probe_types,
+ struct mtd_partition *parts, int nr_parts);

struct mtd_notifier {
void (*add)(struct mtd_info *mtd);
--
1.6.0.3


2008-11-12 23:42:36

by Mike Frysinger

[permalink] [raw]
Subject: [PATCH] mtd/physmap: use parse_mtd()

Call parse_mtd() to handle partition/device registration rather than doing
it all ourself.

Signed-off-by: Mike Frysinger <[email protected]>
---
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);
--
1.6.0.3

2008-11-12 23:42:54

by Mike Frysinger

[permalink] [raw]
Subject: [PATCH] mtd/bfin-async-flash: use parse_mtd()

Call parse_mtd() to handle partition/device registration rather than doing
it all ourself.

Signed-off-by: Mike Frysinger <[email protected]>
---
drivers/mtd/maps/bfin-async-flash.c | 28 +++-------------------------
1 files changed, 3 insertions(+), 25 deletions(-)

diff --git a/drivers/mtd/maps/bfin-async-flash.c b/drivers/mtd/maps/bfin-async-flash.c
index 6fec86a..01c9e7a 100644
--- a/drivers/mtd/maps/bfin-async-flash.c
+++ b/drivers/mtd/maps/bfin-async-flash.c
@@ -120,13 +120,8 @@ static void bfin_copy_to(struct map_info *map, unsigned long to, const void *fro
switch_back(state);
}

-#ifdef CONFIG_MTD_PARTITIONS
-static const char *part_probe_types[] = { "cmdlinepart", "RedBoot", NULL };
-#endif
-
static int __devinit bfin_flash_probe(struct platform_device *pdev)
{
- int ret;
struct physmap_flash_data *pdata = pdev->dev.platform_data;
struct resource *memory = platform_get_resource(pdev, IORESOURCE_MEM, 0);
struct resource *flash_ambctl = platform_get_resource(pdev, IORESOURCE_MEM, 1);
@@ -150,6 +145,8 @@ static int __devinit bfin_flash_probe(struct platform_device *pdev)
state->flash_ambctl0 = flash_ambctl->start;
state->flash_ambctl1 = flash_ambctl->end;

+ platform_set_drvdata(pdev, state);
+
if (gpio_request(state->enet_flash_pin, DRIVER_NAME)) {
pr_devinit(KERN_ERR DRIVER_NAME ": Failed to request gpio %d\n", state->enet_flash_pin);
return -EBUSY;
@@ -161,26 +158,7 @@ static int __devinit bfin_flash_probe(struct platform_device *pdev)
if (!state->mtd)
return -ENXIO;

-#ifdef CONFIG_MTD_PARTITIONS
- ret = parse_mtd_partitions(state->mtd, part_probe_types, &pdata->parts, 0);
- if (ret > 0) {
- pr_devinit(KERN_NOTICE DRIVER_NAME ": Using commandline partition definition\n");
- add_mtd_partitions(state->mtd, pdata->parts, ret);
-
- } else if (pdata->nr_parts) {
- pr_devinit(KERN_NOTICE DRIVER_NAME ": Using board partition definition\n");
- add_mtd_partitions(state->mtd, pdata->parts, pdata->nr_parts);
-
- } else
-#endif
- {
- pr_devinit(KERN_NOTICE DRIVER_NAME ": no partition info available, registering whole flash at once\n");
- add_mtd_device(state->mtd);
- }
-
- platform_set_drvdata(pdev, state);
-
- return 0;
+ return parse_mtd(state->mtd, NULL, pdata->parts, pdata->nr_parts);
}

static int __devexit bfin_flash_remove(struct platform_device *pdev)
--
1.6.0.3

2008-11-13 13:43:54

by Atsushi Nemoto

[permalink] [raw]
Subject: Re: [PATCH] mtd: unify mtd partition/device registration

On Wed, 12 Nov 2008 18:38:53 -0500, Mike Frysinger <[email protected]> wrote:
> Rather than having every map/mtd driver doing the same sequence of
> registering partitions and/or devices, implement common parse_mtd().
>
> Signed-off-by: Mike Frysinger <[email protected]>
> ---
> drivers/mtd/mtdcore.c | 37 +++++++++++++++++++++++++++++++++++++
> include/linux/mtd/mtd.h | 3 +++
> 2 files changed, 40 insertions(+), 0 deletions(-)

I like this idea.

> +int parse_mtd(struct mtd_info *mtd, const char **probe_types,
> + struct mtd_partition *parts, int nr_parts)
> +{
> +#ifdef CONFIG_MTD_PARTITIONS
> + const char *default_part_probe_types[] = {
> + "cmdlinepart",
> + "RedBoot",
> + NULL
> + };

But I'm not sure this default_part_probe_types is appropriate.

How about enclose each parser with #ifdefs?

const char *default_part_probe_types[] = {
#ifdef CONFIG_MTD_CMDLINE_PARTS
"cmdlinepart",
#endif
#ifdef CONFIG_MTD_REDBOOT_PARTS
"RedBoot",
#endif
NULL
};

This get rid of "RedBoot partition parsing not available" noise in
boot message when you use default_part_probe_types without
MTD_REDBOOT_PARTS.

---
Atsushi Nemoto

2008-11-13 13:51:08

by Atsushi Nemoto

[permalink] [raw]
Subject: Re: [PATCH] mtd/physmap: use parse_mtd()

On Wed, 12 Nov 2008 18:39:48 -0500, Mike Frysinger <[email protected]> wrote:
> Call parse_mtd() to handle partition/device registration rather than doing
> it all ourself.
>
> Signed-off-by: Mike Frysinger <[email protected]>
> ---
> 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

2008-11-13 14:28:29

by Atsushi Nemoto

[permalink] [raw]
Subject: Re: [PATCH] mtd: unify mtd partition/device registration

On Thu, 13 Nov 2008 22:43:50 +0900 (JST), Atsushi Nemoto <[email protected]> wrote:
> On Wed, 12 Nov 2008 18:38:53 -0500, Mike Frysinger <[email protected]> wrote:
> > Rather than having every map/mtd driver doing the same sequence of
> > registering partitions and/or devices, implement common parse_mtd().
> >
> > Signed-off-by: Mike Frysinger <[email protected]>
> > ---
> > drivers/mtd/mtdcore.c | 37 +++++++++++++++++++++++++++++++++++++
> > include/linux/mtd/mtd.h | 3 +++
> > 2 files changed, 40 insertions(+), 0 deletions(-)
>
> I like this idea.

Some drivers call both add_mtd_device() and add_mtd_partitions(). The
mtd_device is used to access whole flash area (like /dev/hda). How do
you think of these usage patterns?

maps/edb7312.c
maps/mbx860.c
maps/plat-ram.c
nand/cafe_nand.c
nand/diskonchip.c
nand/ndfc.c

Automatic fallback to mtd_device in parse_mtd() is convenient but
somewhat unflexible...

---
Atsushi Nemoto

2008-11-13 14:44:35

by Mike Frysinger

[permalink] [raw]
Subject: Re: [PATCH] mtd/physmap: use parse_mtd()

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 <[email protected]>
>> ---
>> 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

2008-11-13 14:51:20

by Mike Frysinger

[permalink] [raw]
Subject: Re: [PATCH] mtd: unify mtd partition/device registration

On Thu, Nov 13, 2008 at 09:28, Atsushi Nemoto wrote:
> On Thu, 13 Nov 2008 22:43:50 +0900 (JST), Atsushi Nemoto wrote:
>> On Wed, 12 Nov 2008 18:38:53 -0500, Mike Frysinger wrote:
>> > Rather than having every map/mtd driver doing the same sequence of
>> > registering partitions and/or devices, implement common parse_mtd().
>> >
>> > Signed-off-by: Mike Frysinger <[email protected]>
>> > ---
>> > drivers/mtd/mtdcore.c | 37 +++++++++++++++++++++++++++++++++++++
>> > include/linux/mtd/mtd.h | 3 +++
>> > 2 files changed, 40 insertions(+), 0 deletions(-)
>>
>> I like this idea.
>
> Some drivers call both add_mtd_device() and add_mtd_partitions(). The
> mtd_device is used to access whole flash area (like /dev/hda). How do
> you think of these usage patterns?
>
> maps/edb7312.c
> maps/mbx860.c
> maps/plat-ram.c
> nand/cafe_nand.c
> nand/diskonchip.c
> nand/ndfc.c
>
> Automatic fallback to mtd_device in parse_mtd() is convenient but
> somewhat unflexible...

we could just have it do it all the time. i dont see a problem with
exposing the entire block device the whole time ? i know for the
driver or two of mine, i'm fine with it.

...
ret = parse_mtd_partitions(mtd, probe_types, &parts, 0);
if (ret > 0) {
ret = add_mtd_partitions(mtd, parts, ret);
kfree(parts);
} else if (nr_parts)
ret = add_mtd_partitions(mtd, parts, nr_parts);
if (ret)
return ret;
#endif

return add_mtd_device(mtd);
-mike

2008-11-13 14:53:42

by Mike Frysinger

[permalink] [raw]
Subject: Re: [PATCH] mtd: unify mtd partition/device registration

On Thu, Nov 13, 2008 at 08:43, Atsushi Nemoto wrote:
> On Wed, 12 Nov 2008 18:38:53 -0500, Mike Frysinger wrote:
>> +int parse_mtd(struct mtd_info *mtd, const char **probe_types,
>> + struct mtd_partition *parts, int nr_parts)
>> +{
>> +#ifdef CONFIG_MTD_PARTITIONS
>> + const char *default_part_probe_types[] = {
>> + "cmdlinepart",
>> + "RedBoot",
>> + NULL
>> + };
>
> But I'm not sure this default_part_probe_types is appropriate.
>
> How about enclose each parser with #ifdefs?
>
> const char *default_part_probe_types[] = {
> #ifdef CONFIG_MTD_CMDLINE_PARTS
> "cmdlinepart",
> #endif
> #ifdef CONFIG_MTD_REDBOOT_PARTS
> "RedBoot",
> #endif
> NULL
> };
>
> This get rid of "RedBoot partition parsing not available" noise in
> boot message when you use default_part_probe_types without
> MTD_REDBOOT_PARTS.

yeah, that parsing thing is annoying, but i didnt think enough so to
add ifdef's. i'm fine with it either way.
-mike

2008-11-13 15:51:36

by Atsushi Nemoto

[permalink] [raw]
Subject: Re: [PATCH] mtd: unify mtd partition/device registration

On Thu, 13 Nov 2008 09:51:01 -0500, "Mike Frysinger" <[email protected]> wrote:
> > Automatic fallback to mtd_device in parse_mtd() is convenient but
> > somewhat unflexible...
>
> we could just have it do it all the time. i dont see a problem with
> exposing the entire block device the whole time ? i know for the
> driver or two of mine, i'm fine with it.
>
> ...
> ret = parse_mtd_partitions(mtd, probe_types, &parts, 0);
> if (ret > 0) {
> ret = add_mtd_partitions(mtd, parts, ret);
> kfree(parts);
> } else if (nr_parts)
> ret = add_mtd_partitions(mtd, parts, nr_parts);
> if (ret)
> return ret;
> #endif
>
> return add_mtd_device(mtd);

I'm OK with this. But not sure all current mtd partition users.

Are you going to convert all parse_mtd_partitions() call? Maybe some
people do not want to change /dev/mtd numbers...

---
Atsushi Nemoto

2008-11-13 15:56:00

by Mike Frysinger

[permalink] [raw]
Subject: Re: [PATCH] mtd: unify mtd partition/device registration

On Thu, Nov 13, 2008 at 10:51, Atsushi Nemoto wrote:
> On Thu, 13 Nov 2008 09:51:01 -0500, "Mike Frysinger" wrote:
>> > Automatic fallback to mtd_device in parse_mtd() is convenient but
>> > somewhat unflexible...
>>
>> we could just have it do it all the time. i dont see a problem with
>> exposing the entire block device the whole time ? i know for the
>> driver or two of mine, i'm fine with it.
>>
>> ...
>> ret = parse_mtd_partitions(mtd, probe_types, &parts, 0);
>> if (ret > 0) {
>> ret = add_mtd_partitions(mtd, parts, ret);
>> kfree(parts);
>> } else if (nr_parts)
>> ret = add_mtd_partitions(mtd, parts, nr_parts);
>> if (ret)
>> return ret;
>> #endif
>>
>> return add_mtd_device(mtd);
>
> I'm OK with this. But not sure all current mtd partition users.
>
> Are you going to convert all parse_mtd_partitions() call? Maybe some
> people do not want to change /dev/mtd numbers...

if it's a real concern, we can make it optional. i'll post patches to
convert physmap and my drivers as those are the only ones i can
actually test.
-mike

2008-11-14 21:40:54

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH] mtd: unify mtd partition/device registration

On Wed, 12 Nov 2008 18:38:53 -0500
Mike Frysinger <[email protected]> wrote:

> +int parse_mtd(struct mtd_info *mtd, const char **probe_types,
> + struct mtd_partition *parts, int nr_parts)
> +{
> +#ifdef CONFIG_MTD_PARTITIONS
> + const char *default_part_probe_types[] = {
> + "cmdlinepart",
> + "RedBoot",
> + NULL
> + };
> + int ret;
> +
> + if (!probe_types)
> + probe_types = default_part_probe_types;
> +
> + ret = parse_mtd_partitions(mtd, probe_types, &parts, 0);
> + if (ret > 0) {
> + ret = add_mtd_partitions(mtd, parts, ret);
> + kfree(parts);
> + return ret;
> + } else if (nr_parts)
> + return add_mtd_partitions(mtd, parts, nr_parts);
> +#endif
> +
> + return add_mtd_device(mtd);
> +}

look:

From: Andrew Morton <[email protected]>

text data bss dec hex filename
before: 2488 88 132 2708 a94 drivers/mtd/mtdcore.o
after: 2456 100 132 2688 a80 drivers/mtd/mtdcore.o

Cc: Atsushi Nemoto <[email protected]>
Cc: David Woodhouse <[email protected]>
Cc: Mike Frysinger <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>
---

drivers/mtd/mtdcore.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff -puN drivers/mtd/mtdcore.c~mtd-unify-mtd-partition-device-registration-fix drivers/mtd/mtdcore.c
--- a/drivers/mtd/mtdcore.c~mtd-unify-mtd-partition-device-registration-fix
+++ a/drivers/mtd/mtdcore.c
@@ -306,7 +306,7 @@ int parse_mtd(struct mtd_info *mtd, cons
struct mtd_partition *parts, int nr_parts)
{
#ifdef CONFIG_MTD_PARTITIONS
- const char *default_part_probe_types[] = {
+ static const char *default_part_probe_types[] = {
"cmdlinepart",
"RedBoot",
NULL
_

2008-11-14 21:41:48

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH] mtd/physmap: use parse_mtd()

On Wed, 12 Nov 2008 18:39:48 -0500
Mike Frysinger <[email protected]> wrote:

> Call parse_mtd() to handle partition/device registration rather than doing
> it all ourself.
>
> Signed-off-by: Mike Frysinger <[email protected]>
> ---
> 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?

2008-11-14 22:07:48

by Mike Frysinger

[permalink] [raw]
Subject: Re: [PATCH] mtd/physmap: use parse_mtd()

On Fri, Nov 14, 2008 at 16:41, Andrew Morton wrote:
> On Wed, 12 Nov 2008 18:39:48 -0500
> Mike Frysinger <[email protected]> wrote:
>> Call parse_mtd() to handle partition/device registration rather than doing
>> it all ourself.
>>
>> Signed-off-by: Mike Frysinger <[email protected]>
>> ---
>> 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

2008-11-16 17:35:34

by Jörn Engel

[permalink] [raw]
Subject: Re: [PATCH] mtd: unify mtd partition/device registration

On Wed, 12 November 2008 18:38:53 -0500, Mike Frysinger wrote:
>
> Rather than having every map/mtd driver doing the same sequence of
> registering partitions and/or devices, implement common parse_mtd().

I just added partitioning support to block2mtd. Looks like I might
rethink the patch. :)

> @@ -292,6 +292,43 @@ void put_mtd_device(struct mtd_info *mtd)
> module_put(mtd->owner);
> }
>
> +#include <linux/mtd/partitions.h>

#include in the middle of the code?

Jörn

--
Happiness isn't having what you want, it's wanting what you have.
-- unknown

2008-11-16 17:55:28

by Mike Frysinger

[permalink] [raw]
Subject: Re: [PATCH] mtd: unify mtd partition/device registration

On Sunday 16 November 2008 12:34:54 Jörn Engel wrote:
> On Wed, 12 November 2008 18:38:53 -0500, Mike Frysinger wrote:
> > Rather than having every map/mtd driver doing the same sequence of
> > registering partitions and/or devices, implement common parse_mtd().
>
> I just added partitioning support to block2mtd. Looks like I might
> rethink the patch. :)
>
> > @@ -292,6 +292,43 @@ void put_mtd_device(struct mtd_info *mtd)
> > module_put(mtd->owner);
> > }
> >
> > +#include <linux/mtd/partitions.h>
>
> #include in the middle of the code?

maybe i'll stick a comment above it ... the reason was to be proactive in
preventing mtd partitioning code bleeding into the core. maybe i'm just
paranoid.
-mike

2008-11-16 18:02:49

by Jörn Engel

[permalink] [raw]
Subject: Re: [PATCH] mtd: unify mtd partition/device registration

On Sun, 16 November 2008 12:55:17 -0500, Mike Frysinger wrote:
>
> maybe i'll stick a comment above it ... the reason was to be proactive in
> preventing mtd partitioning code bleeding into the core. maybe i'm just
> paranoid.

Sounds a bit silly to me. *shrugs*

Jörn

--
It is the mark of an educated mind to be able to entertain a thought
without accepting it.
-- Aristotle