Message-ID: <20150327163108.GA5089@earth>
Date:	Fri, 27 Mar 2015 17:31:08 +0100
From:	Sebastian Reichel <sre@...nel.org>
To:	"Dr. H. Nikolaus Schaller" <hns@...delico.com>
Cc:	NeilBrown <neilb@...e.de>,
	List for communicating with real GTA04 owners 
	<gta04-owner@...delico.com>, Mark Rutland <mark.rutland@....com>,
	One Thousand Gnomes <gnomes@...rguk.ukuu.org.uk>,
	Peter Hurley <peter@...leysoftware.com>,
	Arnd Bergmann <arnd@...db.de>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Pavel Machek <pavel@....cz>,
	Grant Likely <grant.likely@...aro.org>,
	Jiri Slaby <jslaby@...e.cz>, devicetree@...r.kernel.org,
	lkml <linux-kernel@...r.kernel.org>
Subject: Re: [Gta04-owner] [PATCH 3/3] tty/slaves: add a driver to power
 on/off UART attached devices.

Hi,

On Fri, Mar 27, 2015 at 10:22:11AM +0100, Dr. H. Nikolaus Schaller wrote:
> > Coming back at my sentence: I asked about a non-trivial, non-bus
> > connected HW, which has a DT binding.
> 
> from omap3-beagle-xm.dts:
> hsusb2_phy (connected through ULPI)
> gpio-keys (it is not a trivial on/off)
> tfp410 (DVI encoder only part of the video pipeline)

These just need a couple of gpios. For me that's trivial compared to
an interface as complex as UART. Since they do not have any complex
interface they could be a child of, they must land in /.

> sound/codec (connected through McBSPs)

The audio codec is a child device of the TWL. The node you are
talking about is a ***virtual*** device. See also

http://devicetree.org/SoC_Audio

Note that the audio binding of many devices is not that "clean",
since quite some relationships are hidden away behind kernel quirks
and very specific compatible strings.

> and there are components on the board which are not described at
> all but work out of the box: audio connectors, usb/ethernet hub

Sure. Everything that can be identified automatically (USB/Ethernet
stuff) is not encoded into DT. DT has been created to provide a
description for devices, which can *not* be identified automatically.

And passive components like audio connectors are normally not
encoded into DT, since no configuration is needed and no information
is gained. Before you argument with video connectors: Yes they are
different, but IMHO for good reasons:

1. The user can see "DVI" label for DVI output and "HDMI" label
   for HDMI output. This is not known by the display controller.
2. The display data channel is i2c and the connector may use a
   system i2c controller together with the data wires provided by
   the display controller. The display subsystem must know which
   i2c controller takes care of the port.

> In the meantime, I have proposed an IMHO even better approach.
> 
> It is not to make the gps a regulator or a subnode of some “primary interface”,
> but give it a reference to the uart it is connected to.
> 
> And make it an independent node on DT root level.
> 
> Then we are completely independent on how the gps driver presents
> data to any user space on any OS.

IMHO that approach is not better per se. The child variant is more
generic it contains information without the kernel even knowing
about uart specific stuff.

Note, that we could always use references in DT. The tree structure
is not needed at all, since references make it possible to describe
a graph. Obviously that would defeat the purpose of the tree
structure. In my opinion making something a node in / is always the
last resort and in my perspective it has been handled in such a way
so far.

> What is bad with having a net? Why must we squeeze everything into
> a tree structure just because it is called such? DT language
> provides the syntax for references and that allows to make nets.
> And it is heavily used.

I think so far references were mainly used when tree structure was
too limited to describe something.

> >> You said NAK about referencing a regulator from the UART node and
> >> now?
> > 
> > No. I said NAK about referencing *the* regulator from the BT/GPS
> > node. My NAK was about referencing a regulator in the UART node
> > (local side), which is actually needed by the GPS/BT module (remote
> > side).
> 
> Ah, now I understand your argument, and agree to it.
> 
> My comment was on a slightly different topic that the DTR line from the
> UART (node) can be used to signal the remote regulator to be turned on.
> 
> But I learned yesterday that we can even hide this explicit signalling
> mechanism (being regulator, gpio or whatever) by:
> 
> bt {
> 	compatible = “vendor,btchip”;
> 	regulator = <&regulator to power the btchip>;
> 	uart = <&uart1>;
> 	gpio = <&gpio to enable the chip>;
> };
> 
> uart1 {
> 	/* no reference to bt */
> 	regulator = <&optional regulator to make the local side work>;
> };
> 
> You have two nodes. One with all properties for the local uart side.
> And one for the remote side. With a link so that the kernel code can
> find the uart the device is connected to.
> 
> And then kernel A can use one signalling mechanism between the uart
> driver and the bt driver and kernel B another. This is the level of abstracion
> we should get to.

Since this part of the argumentation is about the regulator VS
enable gpio let's leave out the child VS uart-reference here.

So first note about the regulators: they are normally referenced
using something like "vcc-supply = <&regulator>;". This makes it
possible to request more than one regulator. It also shows, what
they are normally used for (you have a module, which has a vcc-pin
and you need some regulator (which can be a fixed-regulator) to
drive it.

So now if the module has an enable-gpio one has two options:

1) add "enable-gpio" to the module's node

2) break up the module into subcomponents, which expose its
   internal structure

Normally in DT option 1) is used, if 2) is not needed. If
some *other* component also makes use of the regulator enabled
by the enable-gpio you would choose option 2), otherwise you
would choose option 1) to keep everything simpler.

> > From my POV putting it below the UART node is more logical and from
> > your POV putting it below / is more logical.
> 
> Another aspect just came to my mind:
> 
> IMHO / is the better solution because it also allows to override the
> uart property in an overlay. For example board variant 1 connects to
> uart1 and a hardware changeconnects the bt chip to uart2 in variant 2.
> 
> You can easily rearrange this link by including the dts of variant 1 and 
> just overwriting uart = <&uart2>.
> 
> But you can’t do that with a child node. I.e. you have to overwrite the whole
> uart1 and uart2 nodes to “remove” the relation between uart1 and the bt/gps chip.
> 
> This is probably the reason why gpio controllers are referenced and
> not the parents of all connected chips.

The main problem with making gpio controllers parents is, that you
can have only one parent. Since most chips already have a parent
gpios are referenced. Then in opposite to high-level interfaces
many devices need multiple GPIOs. So it will be hard to find a
single parent.

So let's assume there is 20% of devices, which need only one GPIO
and have no other "more important" interface. Surely you could add
them as a child of a gpio-controller, but adding another binding
method for the system subsystem is obviously not a good idea.

> > From my POV the page is simply focused on standard bindings, which
> > have already been established.
> 
> Well, new bindings should have very good reasons not to follow these
> principles.

Sure, but the standard bindings are very focused on bus connected
devices (esp. since the page is from 2010). So obviously the page is
heavily influenced. As I already mentioned I don't think we have
something comparable to a UART attached device with DT binding so far.

(except of mentioned HSI)

> > If I understand you right, your POV does not allow child nodes
> > (which are not directly under /) without reg property / address.
> 
> Yes, I read it like that.
> 
> Slides 22 and 27 of
> 
> http://events.linuxfoundation.org/sites/events/files/slides/petazzoni-device-tree-dummies.pdf
> 
> also appear to suggest to put busses and devices on root level.
> 
> Well, he says “typically”, because he just describes the status quo and is
> not wanting to set a standard.

The text merely tells you, that these kind of nodes can be found
below the / node. It does not tell you, that all of them are there.
Simple Counterexample:

/ -> ocp -> i2c@...x -> i2c-dev

The i2c bus is not at /, so not all busses are at /.

> > Just have a look at the DT binding
> > documentation directory in the kernel - we already have them.
> 
> Maybe, todays Linux bidings and DTs are not general enough in such cases
> and should be revisited? But that goes far beyond our discussion and problem.

Well, I'm against moving all MFD submodules to / and that is what
you suggest by disallowing child nodes without an address. It will
just clutter the / node with lots of stuff.

> > We obviously disagree here :)
> 
> Exactly. This is why I am trying to find a concise definition how
> it should be.
> 
> And to be honest I am quite worried that it does not appear to
> exist but DT should be stable and well defined and even OS
> agnostic. How can that work?

By having people, who review new subsystem bindings. I guess
once the UART binding has been done we have a reference case
anyway.

> >> * if a driver needs access to another component, a reference property is used
> > 
> > Here we also disagree. The kernel also makes use of parent > child
> > connections (e.g. the driver for an i2c client node will request
> > data via its parent)
> 
> That is in my view strictly following because i2c is a bus and therefore
> there is a parent>child relation and it is ok to use it to request the parent.
> 
> But in my words the bus is not “another component”. Another component
> in my example means the “other" between BT chip and SoC with UART.

The same argumentation can be used with BT node below UART node.

> > and sometimes even sibling relationships (in
> > MFD devices).
> 
> I am not at all against using the information that is present in
> the DT :) I just want to have the DT the information encoded in
> the right way.

Needless to point out, that's why we are having this discussion.

> > So let me summarize:
> > 
> > * Option A: Do not encode remote side information
> >   Rationale: UART port can be opened in userspace without that info
> >   Counterargument: This information is needed if the OS should be
> >   able to hide away the tty and provide support for the connected
> >   remote device (e.g. provide a bluetooth device instead of a tty)
> 
> Agreed. I think we can drop this option since there are good use cases.
> 
> A second one for the records: if we want to provide data from an UART
> connected GPS chip through an iio interface.

All examples can be summarized as "device abstraction".

The UART interface is no different from an SPI bus in this regard.
We could just provide the SPI bus node and let the user cope with
the rest. Instead most users do not even need to know, that their
fancy wifi module is connected via SPI. The same is true for UART
attached devices.

Note, that in my opinion UART is mostly the same as an SPI bus with
only one device (yes, I'm aware, that there is an addtional clock
line etc.). The only difference from DT perspective is the
addressability, which is not important from my POV.

> > * Option B: Put UART remote side node to / and add a reference to the UART port
> >   Rationale: DT wiki page indicates, that child nodes must be addressable
> and UART is not per se a “bus”.
> 
> > 
> > * Option C: Put UART remote side node below the UART node
> >   Rationale: Remote device is accessed through the UART
> and treat serial connections as single-client busses

Note, that for me child device does not imply a bus, since other
devices certainly do not use it that way - e.g. most (all?) MFD
devices use child nodes. One can argue of course, that there are
two rules for child devices:

 1. addressable bus clients
 2. device subfunctionalities

> > Since this is about a subsystem binding DT maintainers ACK is
> > definitely needed, so maybe we should just wait for their decision.
> 
> Good summary! 
> 
> So let’s wait for questions / clarifications / comments /
> decisions by DT maintainers.

Since DT maintainers are typically heavily overloaded with work it's
probably a good idea to write up a summarized text in a separate
thread with a obvious hint, that DT binding maintainer feedback is
requested.

-- Sebastian

Download attachment "signature.asc" of type "application/pgp-signature" (820 bytes)

Powered by blists - more mailing lists