Re: [PATCH v3 3/4] powerpc: NAND: FSL UPM: document new bindings

[Grant Likely <grant.likely@secretlab.ca>]

On Thu, Mar 26, 2009 at 4:14 PM, Wolfgang Grandegger <wg@grandegger.com> wrote:
> Anton Vorontsov wrote:
>> On Thu, Mar 26, 2009 at 11:02:06AM -0600, Grant Likely wrote:
>>> In other words, this device is not register-level compatible with the
>>> fsl,upm-nand device.  Give the node a new compatible value
>>> (tqc,tqm8548-upm-nand) and add another entry to the of_fun_match table
>>> for the new device.  Use the .data element in the match table to
>>> supply an alternate fun_cmd_ctrl() function for this board (instead of
>>> using a property value do decide which fun_cmd_ctrl() behaviour to
>>> use).  New boards that *do* use the same addressing scheme can claim
>>> compatibility with tqc,tqm8548-upm-nand.
>>
>> I don't like this. :-/
>>
>> UPM is an universal thing, so there are thousands of ways we can
>> connect NAND to the UPM. Of which only ~10 would be sane (others are
>> insane, and nobody would do this. If they do, _then_ we'll fall back
>> to <board>-upm-nand scheme for a particular board).
>
> Yep.
>
>> I don't see any problem with fsl,upm-addr-line-cs-offsets. It can
>> describe any scheme in "addr lines are cs" connection, it's a common
>> setup for multi-chip memory, we shouldn't treat it is as something
>> extraordinary.
>
> I fully agree. I'm going to provide a patch on monday.

Well, I still don't think it is the wisest choice.  My position is
that it is better to be conservative and pedantic now because it is
easy to relax the rules from that point.  If it turns out after some
experience with "fsl,upm-addr-line-cs-offset" that the scheme has a
serious flaw, then the impact is contained.  On the other side, if it
is confirmed and useful and correct, it is a trivial change to make it
available to everything that claims compatibility with fsl,upm-nand.

That said, I won't oppose it if you go this route.  However at the
very least, please change the nand node's compatible list to be:

compatible = "tqc,tqm8548-upm-nand", "fsl,upm-nand";

The custom glue logic makes it something unique, so "tqc,..." should
be at the start of the list to describe it as such, even if the driver
only ever uses "fsl,upm-nand".

g.

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