Hi Mark,

Am 13.01.2016 um 20:15 schrieb Mark Rutland <[email protected]>:

> On Tue, Jan 12, 2016 at 02:28:00PM +0100, H. Nikolaus Schaller wrote:
>> Hi Tomeu,
>> 
>> Am 12.01.2016 um 14:06 schrieb Tomeu Vizoso <[email protected]>:
>> 
>>> On 11 May 2015 at 03:56, NeilBrown <[email protected]> wrote:
>>>> Hi all,
>>>> here is version 4 of my "UART slave device" patch set, previously
>>>> known as "tty slave devices".
>>> 
>>> Hi Neil,
>>> 
>>> do you (or someone else) have plans to continue this work in the short
>>> or medium term?
>> 
>> yes, there is something in our upstreaming pipeline. This one works for us 
>> on top of 4.4.0:
>> 
>> <http://git.goldelico.com/?p=gta04-kernel.git;a=shortlog;h=refs/heads/work/hns/misc/w2sg-tty-slave2-v4>
>> 
>> There is one point still to be solved: the exact style of the DT bindings.
>> 
>> We have an idea how a driver can implement two different styles (child node 
>> AND phandle)
>> so that it is up to the DTS developer to use the one that best fits into the 
>> existing DTS.
> 
> From my perspective as a binding maintainer, and as I stated before, the
> child node approach made the most sense and was most consistent with the

> way we handle other devices.

I simply don't see that this is the most common way other devices are handled.

I find many counter-examples which use phandles:
* gpios
* regulators
* iio channels used by other drivers (e.g. iio-hwmon)
* phy devices
* timers
* pwms
* interrupts
* dma
* mcbsp (see e.g. 
http://lxr.free-electrons.com/source/arch/arm/boot/dts/omap3-n900.dts#L127)
* mmc-pwr-seq-simple (which does not even describe a physical piece of hardware)

All of them define the provider in one node. And refer to it by a phandle in 
another node
where they are used.

So I see a lot of provider-consumer relationships modeled by phandles but not 
by child nodes.

Next, if I look up real world DT sources, child nodes have in a majority of 
cases a
reg = <...> or ranges = <...> entry to define specific addresses of each child 
node and
to distinguish between them.

This is not always the case (e.g. children of the root node) but often. 
Therefore I assume
the child-node pattern is mainly intended for distinguishing between multiple 
*addressable*
subdevices connected to a single provider, i.e. some sort of "shared bus".

In the specific problem I (and Neil) want to solve (GTA04 devices and more to 
come),
the UART is simply a provider of serial data lines and power control events (or 
whatever
the driver implementations want to do with the knowledge about this connection).

Although we have multiple such uart-device connections, they are all individual 
point-to-point.

Not a bus structure with multiple clients. So there are several simple 
provider-consumer relations.
Hence there is no urgent need for addresses of multiple child nodes of a single 
UART and
no reg/ranges property.

Of course, with the child node approach it would give the flexibility to 
introduce such
a feature easily in the future - but I don't see a use case. Not even at the 
horizon.

And I wonder how I should implement a driver if a child node provides a reg 
property.
Should I invent and implement a protocol layer to make the UART an addressable 
bus?

But the chip I connect to an UART does not understand that and I can't change 
it.
So it is probably not expected by the uart-slaves story - and I have no need 
for addressability
of multiple subnodes.

So I conclude: the single chip is the consumer of a simple UART provider and 
should therefore
be described as a connection through a phandle. Like in all the other DT 
examples listed
above. The best description is IMHO:

https://www.kernel.org/doc/Documentation/devicetree/bindings/iio/iio-bindings.txt

At least this is how I see the DT world when going through some device tree 
files
and trying to deduce what the common style is.

This appears to be opposite to what you say: "most consistent with the way we
handle other devices". I only find that other devices which understand some 
addressing
scheme are handled that way.

> 
> I don't understand what the benefit of supporting two styles of
> description would be, relative to the maintenance cost.

Supporting both styles is a proposal to make both of us happy.

And there isn't much to be maintained. It is just a notice in the bindings 
document
of uart-slaves that the phandle is optional, if the node is the single child 
node of an
UART. If it isn't a subnode of an UART, or not at index 0, the phandle is needed
to describe the cross-reference. So it can be seen as a simple extension to move
the node outside but keep the link.

A rough estimate is that it requires just ~20 lines to implement in our driver 
(unless
we need locks, error handling etc.).

Then, the DT developers (like me) can decide which style better fits into the 
DTS
structure that already exists, when adding a salve device to some UART.

My experience from almost daily work with device trees is that phandles give
more flexibility in expressing the hardware structure in DT language. And they
allow to better group properties. In this case: "I am connected to interface 
...".

And the allow to easily modify it by includes and overlays to describe small 
hardware
variants ("I am now connected to a different interface ..."). Moving a subnode 
between
parents is difficult without multiple well designed include files, while for 
phandle
there is a simple idiom:

        #include <existing.dts(i)>
        &child { link = <&new-parent>; };

IMHO this is easy to read and understand. And I have used that pattern several
times, e.g. for "adding" hardware to some evaluation board without touching the
original DTS. So I don't want to miss it in this case.

> Nor do I
> understand your fixation with the phandle approach,

Well, because I don't understand your fixation on the child node approach for 
this
non-addressable point-to-point connection. Why prepare for a feature that nobody
really needs and has asked for?

To be more specific:

* I find that the phandle approach better (more flexible) suits the problem I 
want to have solved.
* there is no need for multiple child nodes for a point-to-point connection, 
because UART is rarely used as a bus.
* I see a lot of examples where phandles are intensively used and there it 
appears to be right to do so.

I just know that you conclude "child nodes made the most sense and was most 
consistent".

But I still wonder why. It does not appear to match what I observe in 
arch/arm/boot/dts
and the problem I want to solve.

> given it has been
> repeatedly disagreed with by binding maintainers.

Binding maintainers may sometimes be as wrong as I may be here. This needs a 
discussion
but not a circular argument, that it already has been disagreed repeatedly.

I may have missed it, but I am also not aware that there was a technical 
analysis of both
approaches, comparing the pro's and con's. I had received requests to show code 
for the
phandle approach and we provided it.

Coming to different conclusions can happen, if requirements are weighted 
differently. Or
the problem to be solved is not completely understood. But then, the 
requirements and
assumptions should be discussed (which is difficult on a patch-review-based 
discussion list).

On a more general level, the key problem is that *I* have to write and maintain 
a
multitude of board specific DTS files (not all of them in mainline) using the 
style
*you* decide.

A style which I don't feel to be the "right" one, because it is less flexible 
(e.g. swapping
child nodes between parents in board variants).

Summary: your decision gives flexibility for future expansion that I do not 
need (and
probably nobody else) and does not provide the flexibility I need today (and 
others
might appreciate).

So what should I do? Except being fixed on the phandle approach, repeating my 
arguments
and describe requirements. And submitting our code and bindings document 
proposal
every now and then?

Thanks for giving me another chance to explain my PoV,
Nikolaus

_______________________________________________
Gta04-owner mailing list
[email protected]
http://lists.goldelico.com/mailman/listinfo.cgi/gta04-owner

Reply via email to