[v5,1/8] MTD: Add integrator-flash feature to physmap

In the process of moving platforms away from integrator-flash
(aka armflash), add to physmap the few features that make
armflash unique:

- optionnal probing for the AFS partition type
- init() and exit() methods, used by Integrator to control
  write access to the various onboard programmable components

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
Cc: David Woodhouse <dwmw2@infradead.org>
---
 drivers/mtd/maps/physmap.c  |   16 +++++++++++++++-
 include/linux/mtd/physmap.h |    2 ++
 2 files changed, 17 insertions(+), 1 deletions(-)

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