Re: [PATCH 2/3] [POWERPC] Xilinx: of_serial support for Xilinx uart 16550.

[Sergei Shtylyov <sshtylyov@ru.mvista.com>]

Hello.

Segher Boessenkool wrote:

>>> Personally, I'm not fond of this approach.  There is already some
>>> traction to using the reg-shift property to specify spacing, and I
>>> think it would be appropriate to also define a reg-offset property to
>>> handle the +3 offset and then let the xilinx 16550 nodes use those.

>>    That's making things only worse than the mere "reg-shift" idea. I 
>> think that both are totally wrong. Everything about the programming 
>> interface should be said in the "compatible" and possibly "model" 
>> properties.

> No.  In effect, you are saying here that no device binding should define
> any binding-specific properties.  This will just lead to combinatorial
> explosion of "compatible" values.

> That said, "reg-spacing"/"reg-shift"/"reg-offset" should *not* be
> considered something generic; they are part of specific device
> bindings.  Of course it is nice if various bindings use the same
> names for the same concepts, but that's an orthogonal issue.

    The proposed use clearly would treat them as generic, since in the context 
of the Xilinx UART they're just not needed -- it's known beforehand and most 
probably fixed how/where the registers are mapped. There's just no need for 
such info in the device tree -- unless you're going to teach the *generic* 
driver to handle this specific (and possibly others alike) kind of a device.

>>  of_serial driver should recognize them and pass the necessary details 
>> to 8250.c. As for me, I'm strongly against plaguing the device tree 
>> with the *Linux driver implementation specifics*

> "reg-*" has nothing to do with Linux device driver implementation
> issues: it describes how a device is physically wired up!

    Hm... wasn't that you who were telling that use of "range" properties 
guarantees 1:1 correspondence of the upstream/downstream bus addresses (in 
their LSB part of course -- meaning that the device registers 0..x are seen by 
the CPU at addresses base+0..base+X?

>> (despite I was trying this with MTD -- there it seemed somewhat more 
>> grounded :-).

> Quite the opposite, but let's not rehash that discussion.

    I might be mixing with the February thread about this UART -- have 
alsready forgotten about it.

>>> In support of my argument; the fact that you need a table of data says
>>> to me that this data should really be encoded in the device tree.  :-)

>>    Not at all.

> Not _necessarily_.  I agree with Grant here: for many of these devices
> with byte-size registers, it is very common to find them with their
> register banks wired up differently, and that is often the *only*
> difference to the "normal" device.  In this situation, it makes a lot
> of sense to describe that difference with "reg-*" properties.

    Note that "compicated" mapping is not (necessarily) a property of the 
device itself but generally a property of the chip select circuit, i.e. 
external entity.

> In some other situations, it is better to create a new binding for
> the device.

> Segher

WBR, Sergei