Message ID | 1305557977-16871-2-git-send-email-marc.zyngier@arm.com |
---|---|
State | Accepted |
Commit | b7281ca2a4b00044c60c25059f467d05772cdbe3 |
Headers | show |
Hi, On Mon, 2011-05-16 at 15:59 +0100, Marc Zyngier wrote: > #ifdef CONFIG_MTD_PARTITIONS > -static const char *part_probe_types[] = { "cmdlinepart", "RedBoot", NULL }; > +static const char *part_probe_types[] = { "cmdlinepart", "RedBoot", > +#ifdef CONFIG_MTD_AFS_PARTS > + "afs", > +#endif > + NULL }; We want to kill CONFIG_MTD_PARTITIONS: http://www.linux-mtd.infradead.org/doc/general.html#L_partitions_ban and Jamie Iles is doing this ATM. The reason for this is that MTD partitions are used always everywhere anyway, and current amount of ifdefs is scary. I see this CONFIG_MTD_AFS_PARTS you add and I do not think it is the right way to go. I cannot tell you now what would be exactly the right way, but something which does not require ifdef, something where drivers do not contain any information about partition types like "afs" or "cmdlinepart" or whatever, something where partition types are registered within an infrastructure and most of the stuff is hidden from the drivers. I think we should ban stuff like the above as well and force people to create saner MTD partitions support. Sorry if this sounds like an attack, it is not. I just think that we carry crap for too long and should start forcing people to clean it up by not accepting changes :-)
On Tue, 2011-05-17 at 08:37 +0300, Artem Bityutskiy wrote: > Sorry if this sounds like an attack, it is not. I just think that we > carry crap for too long and should start forcing people to clean it up > by not accepting changes :-) Sorry, I forgot to note that I do not insist that you have to re-work MTD partitions support - you already do a very good thing by killing a redundant driver, and delaying this would be counter-productive. But I anyway wanted to express my thoughts.
On Tue, May 17, 2011 at 08:46:18AM +0300, Artem Bityutskiy wrote: > On Tue, 2011-05-17 at 08:37 +0300, Artem Bityutskiy wrote: > > Sorry if this sounds like an attack, it is not. I just think that we > > carry crap for too long and should start forcing people to clean it up > > by not accepting changes :-) > > Sorry, I forgot to note that I do not insist that you have to re-work > MTD partitions support - you already do a very good thing by killing a > redundant driver, and delaying this would be counter-productive. But I > anyway wanted to express my thoughts. So, what's happening with this patch set? I'd like to have an ack from the MTD people for the set.
On Mon, May 16, 2011 at 10:59, Marc Zyngier wrote: > --- a/include/linux/mtd/physmap.h > +++ b/include/linux/mtd/physmap.h > @@ -22,6 +22,8 @@ struct map_info; > > struct physmap_flash_data { > unsigned int width; > + int (*init)(struct platform_device *); > + void (*exit)(struct platform_device *); > void (*set_vpp)(struct map_info *, int); > unsigned int nr_parts; > unsigned int pfow_base; you use the platform_device structure but never include any header for it. please add "struct platform_device;" to the top of this file after the #includes. include/linux/mtd/physmap.h:25: warning: ‘struct platform_device’ declared inside parameter list include/linux/mtd/physmap.h:25: warning: its scope is only this definition or declaration, which is probably not what you want include/linux/mtd/physmap.h:26: warning: ‘struct platform_device’ declared inside parameter list include/linux/mtd/physmap.h:27: warning: ‘struct platform_device’ declared inside parameter list -mike
diff --git a/drivers/mtd/maps/physmap.c b/drivers/mtd/maps/physmap.c index 7522df4..49676b7 100644 --- a/drivers/mtd/maps/physmap.c +++ b/drivers/mtd/maps/physmap.c @@ -67,6 +67,10 @@ static int physmap_flash_remove(struct platform_device *dev) if (info->mtd[i] != NULL) map_destroy(info->mtd[i]); } + + if (physmap_data->exit) + physmap_data->exit(dev); + return 0; } @@ -77,7 +81,11 @@ static const char *rom_probe_types[] = { "map_rom", NULL }; #ifdef CONFIG_MTD_PARTITIONS -static const char *part_probe_types[] = { "cmdlinepart", "RedBoot", NULL }; +static const char *part_probe_types[] = { "cmdlinepart", "RedBoot", +#ifdef CONFIG_MTD_AFS_PARTS + "afs", +#endif + NULL }; #endif static int physmap_flash_probe(struct platform_device *dev) @@ -100,6 +108,12 @@ static int physmap_flash_probe(struct platform_device *dev) goto err_out; } + if (physmap_data->init) { + err = physmap_data->init(dev); + if (err) + goto err_out; + } + platform_set_drvdata(dev, info); for (i = 0; i < dev->num_resources; i++) { diff --git a/include/linux/mtd/physmap.h b/include/linux/mtd/physmap.h index bcfd9f7..d37cca0 100644 --- a/include/linux/mtd/physmap.h +++ b/include/linux/mtd/physmap.h @@ -22,6 +22,8 @@ struct map_info; struct physmap_flash_data { unsigned int width; + int (*init)(struct platform_device *); + void (*exit)(struct platform_device *); void (*set_vpp)(struct map_info *, int); unsigned int nr_parts; unsigned int pfow_base;