Re: [PATCH v2 3/6] mtd: m25p80: add support to parse the SPI flash's partitions

[Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>]

On Tue, Aug 10, 2010 at 11:23 AM, Joakim Tjernlund
<joakim.tjernlund-SNLAxHN9vbcOP4wsBPIw7w@public.gmane.org> wrote:
> glikely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org wrote on 2010/08/10 16:56:42:
>>
>> On Tue, Aug 10, 2010 at 2:29 AM, Joakim Tjernlund
>> <joakim.tjernlund-SNLAxHN9vbcOP4wsBPIw7w@public.gmane.org> wrote:
>> >> On Tue, Aug 10, 2010 at 1:14 AM, David Brownell <david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org> wrote:
>> >> >
>> >> >
>> >> > --- On Mon, 8/9/10, Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org> wrote:
>> >> >
>> >> >
>> >> >> > +       nr_parts =
>> >> >> of_mtd_parse_partitions(&spi->dev, np, &parts);
>> >> >
>> >> > Let's keep OF-specific logic out of drivers like
>> >> > this one ...  intended to work without OF.
>> >> >
>> >> > NAK on adding dependencies like OF to drivers
>> >> > and other infrastructure that starts generic.
>> >
>> > Agreed.
>> >
>> >>
>> >> The OF hooks compile to no-ops when CONFIG_OF is disabled.  I've been
>> >> very careful about that.
>> >
>> > OF should not be in the drivers, one should be able use some other method
>> > than OF.
>>
>> If a device is described in the device tree, then the code has to live
>> *somewhere* to translate the data from the device tree into a form the
>> driver can use.  If not the driver, then where should that code live?
>>
>> If it is in the machine support code, then that requires foreknowledge
>> about all the device specific data that needs to be translated out of
>> the device tree at device registration time, which also means that the
>> translation code cannot be a module and it nullifies some of the
>> advantages of the device tree.  Not to mention the fact that it is
>> just plain ugly!  :-)
>>
>> Some of it can go into the infrastructure code, but only for parsing
>> common properties like irqs, address ranges, and some common bindings
>> like registering spi and i2c bus child nodes as devices.  (This
>> *particular* case may fall into this category if add_mtd_device() was
>> able to call the OF partition parsing hook if a device node pointer
>> was present; which does make a certain amount of sense, but I defer to
>> dwmw2 in this case).  However, device-specific properties cannot be
>> parsed in the infrastructure code because the infrastructure has no
>> knowledge of device specific bits.
>
> I am no expert at this at all, but I can give you an example what I don't
> want. Look at spi_mpc8xxx.c, earlier it was possible to define your own
> CS functions and register them from board code. After OF:ication one can
> not and the current OF methods aren't expressive enough to do what I need.
> The spi subsystem has a general framework for dealing with SPI devices
> and I think OF should be built on top of the native methods, at the
> very least not disable those methods like spi_mpc8xxx.c does.

You're right and it's a problem.  There wasn't a good way to handle
this when powerpc first moved over to use OF.  I think we've got a
good solution now.  See the use of a bus notifier in
arch/powerpc/platforms/512x/pdm360ng.c in Linus' current tree.  Most
of the time that won't be needed, but when a platform specific
callback is required, this seems to be a clean solution.

g.

------------------------------------------------------------------------------
This SF.net email is sponsored by 

Make an app they can't live without
Enter the BlackBerry Developer Challenge
http://p.sf.net/sfu/RIM-dev2dev