[RFC] misc/at24: add experimental OF support for the generic eeprom driver

[Grant Likely]

On Fri, Oct 9, 2009 at 8:01 AM, Nate Case <ncase at xes-inc.com> wrote:
> On Thu, 2009-10-08 at 23:40 -0600, Grant Likely wrote:
>> For your future reference, patches that look at the device tree should
>> also cc: devicetree-discuss at lists.ozlabs.org so that new bindings can
>> be reviewed and common mistakes can be avoided.  It is expected that
>> new device tree bindings are accompanied with documentation describing
>> what the binding is for and how it should be used (see
>> Documentation/powerpc/dts-bindings).
>>
>> I know this change is already in mainline, but can you please post the
>> device tree fragment that you're using to describe this chip?  I want
>> to make sure we don't get stuck with things in the kernel that will be
>> hard to maintain in the long term.
>
> Hi Grant,
>
> Sorry for neglecting to include devicetree-discuss on that one.  I was
> in fact aware of this list, and subscribe to it.  I really just forgot
> in this case.

No worries, it happens.

> I also have a documentation patch for it that went along with it, but it
> wasn't ready in time and so it's been sitting in our local patch queue.
> I can submit that soon,  but it probably makes sense for Wolfram to
> voice whatever his concerns were about "questionable" properties before
> I document what's there.

Yes, please post it as soon as you can.

> Here's an example device tree node for this case:
>
>               gpio1: gpio at 18 {
>                        compatible = "nxp,pca9557";
>                        reg = <0x18>;
>                        #gpio-cells = <2>;
>                        gpio-controller;
>                        polarity = <0x00>;
>               };
>
> In this case, the linux,gpio-base property wasn't specified.  But, the
> use case is identical to the pdata->gpio_base field.  "polarity" is used
> for specifying polarity inversion for each line, and is in the same
> format of the 'polarity inversion' register on the chip.  My reasoning
> in the property naming was as follows:
>
>    linux,gpio-base: Linux-specific as it relates to internal GPIO
>                     numbering.  So, it's prefixed with "linux,"

This would be the 'questionable' property.  :-)  The device tree is
supposed to be OS agnostic, so I get the heebee geebees when I see new
'linux,<blah>' properties defined because it means Linux internal
implementation details are getting leaked out into the data blob.  The
problem leakage is that the device tree should not be impacted by
internal Linux code changes.

In this particular case, specifying the exact GPIO base number doesn't
really belong in the device tree because Linux is able to assign and
manage GPIO numbers on its own just like all other OF GPIO controller
drivers currently do.  In fact, that goes for pretty much all
enumeration, not just GPIO.  In general, a particular device instance
(uart, gpio, phy, whatever) should be resolved from its node in the
device tree, and not by some arbitrary number assigned by the .dts
author.  The problem with arbitrary numbers is they don't expose the
linkage between nodes in the same way a 'phandle' does (A phandle can
be searched for.  An arbitrary number assignment, good luck?), and
they don't expose intended usage (its just a meaningless number, and
it only works because userspace just happens to 'agree' to use the
same number).

>    polarity: Dictated by how hardware is wired up, so it's needed
>              regardless of the OS.

Typically GPIO drivers have been handling this by using #gpio-cells
set to 2, and using the 2nd cell for flags.  The priority can be
encoded as a flag.

g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.