Re: [Gta04-owner] [PATCH 3/3] tty/slaves: add a driver to power on/off UART attached devices.

Dr. H. Nikolaus Schaller
2015-03-27T17:11:42Z

Am 27.03.2015 um 17:31 schrieb Sebastian Reichel <sre@kernel.org>:

> 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 /.

ULPI-PHY is not at all “just a couple of GPIOs”. It is a 60 MHz 8 bit wide
parallel protocol with handshake… Sort of 8 UARTs or SPI with common clock.

But still no bus. The bus driven by the ULPI-PHY is “USB2” which is known
not to be a physical bus but a tree. It just becomes a bus by higher level
protocols.

I must admit that I lost the goal that you want to demonstrate with this
question/examples.

>
>> 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.

Ah interesting that they exist! A virtual gpio was not acceptable…

> 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.

Exactly.

>
> 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.

Yes. Indeed.

>
>> 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.

Right.

> Obviously that would defeat the purpose of the tree
> structure.

Which might be there for completeness if something wants to be
better described in a tree structure. Multiple clients of a single bus controller
that can be selected (addressed) are obviously better described by
a parent>child relationship (which includes some sort of “address”
to describe how this addressing works).

> 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.

But that contradicts some documents I have found and linked. Please
show me a document about DT that supports your view.

I agree that both views can be valid, but in lack of some “official” guideline
we can’t decide ourselves.

>
>> 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>;".

s/regulator/vcc-supply/ myexample.dts

> 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
>

s/gpio/enable-gpio/ myexample.dts

> 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.

Sorry, but what do you want to explain here which helps to answer
where to locate the bt node?

>
>>> 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.

Yes, this is exactly why I assume that gpio controllers are referenced.

Now, replace gpio with one uart for a mfd. And the same argument
leads that a mfd can’t have multiple parents. Therefore it should not
have the uart as parent as well.

>
>>> 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)

McBSP?

>
>>> 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@xxxx -> i2c-dev
>
> The i2c bus is not at /, so not all busses are at /.

ocp is itself a bus. So i2c is just a second level bus. Both are
adressable (@xxxx for the controller’s address and the i2c dev
has a reg property).

>
>>> 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.

Clutter? IMHO not.

>
>>> 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

Any interface is mostly the same as an SPI bus. It may just have faster
or slower clocks, have more or less parallel data lines sharing the same
clock. Different logic levels (binary / ternatry / self-clocking etc.) as well.

The question is how far we want to strech these similarities into the DT.

> (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.

Well, by declaring something as not important you can argue for everything :)

>
>>> * 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

Yes!

I think I had written that before but it might have been lost in the growing
discussion.

>
>>> 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.

Agreed. Should be limited to Option B and C since we agree that A
does not provide all information that could be useful for future kernels.

BR,
Nikolaus