Message-ID: <20150506171802.GE2974@leverpostej>
Date:	Wed, 6 May 2015 18:18:02 +0100
From:	Mark Rutland <mark.rutland@....com>
To:	"Dr. H. Nikolaus Schaller" <hns@...delico.com>
Cc:	Peter Hurley <peter@...leysoftware.com>,
	Pavel Machek <pavel@....cz>,
	List for communicating with real GTA04 owners 
	<gta04-owner@...delico.com>, NeilBrown <neil@...wn.name>,
	One Thousand Gnomes <gnomes@...rguk.ukuu.org.uk>,
	Arnd Bergmann <arnd@...db.de>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Sebastian Reichel <sre@...nel.org>,
	"grant.likely@...aro.org" <grant.likely@...aro.org>,
	Jiri Slaby <jslaby@...e.cz>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [Gta04-owner] [PATCH 0/3] tty slave device support - version 3.

On Wed, May 06, 2015 at 05:09:20PM +0100, Dr. H. Nikolaus Schaller wrote:
> Hi Mark,
> 
> Am 06.05.2015 um 16:15 schrieb Mark Rutland <mark.rutland@....com>:
> 
> >>>>>> No, I am not playing devil’s advocate (which would imply that I am doing this
> >>>>>> for fun to tease the dog), but I feel I have to be the advocate of future board
> >>>>>> designers who want to easily import an existing board DT and overwrite device
> >>>>>> tree nodes to describe design changes, i.e. what slave device is connected to
> >>>>>> which uart.
> > 
> > [...]
> > 
> >>> If this happens, you can move the slave device into a fragment that you
> >>> can include under the correct node. That's trivial.
> >> 
> >> But less readable. And that is important as well.
> > 
> > I disagree. The manipulation you have to perform to override properties
> > is at least as bad as including a file.
> 
> What about:
> 
> #include "omap3-beagle-xm.dts"
> 
> / {
> 	/* HS USB Port 2 Power enable was inverted with the xM C */
> 	hsusb2_power: hsusb2_power_reg {
> 		enable-active-high;
> 	};
> };
> 
> compared to 
> 
> #include “board1.dts”
> 
> / {
> 	/* slave was reconnected to uart4 */
> 	slave {
> 		uart = <&uart4>;
> 	};
> };

As I mentioned, you can easily carve up your DTS to make that work with
includes if you really must:

/* UART0 board variant */
#include "board.dtsi"
&uart0 {
	#include "some-uart-slave.dtsi"
};

/* UART1 board variant */
#include "board.dtsi"
&uart1 {
	#include "some-uart-slave.dtsi"
};

If you happen to find includes ugly then you can say it's ugly, but it's
functionally equivalent, and also means you can avoid having
disabled/partial nodes all over the place.

If you really wanted you could macro the label instead and have the
slaved node in full.

[...]

> > As Neil mentioned earlier, ignore "bus". Here we're caring about a 1-1
> > connection between master (UART) and slave (device), and it happens that
> > in most other cases the master is actually a bus, so that's how things
> > happen to be named.
> 
> So you also mean that all master-slave connections must be represented
> by DT subnodes and everything else is not acceptable?
> 
> What about:
> 
> 	sound {
> 		compatible = "ti,omap-twl4030";
> 		ti,model = "omap3beagle";
> 
> 		ti,mcbsp = <&mcbsp2>;
> 	};
> 
> Isn’t McBSP a “bus” with your definition as well? Like a “master”. And the “twl4030”
> is the slave (codec)?

I'm nowhere near familiar enough with the audio hardware nor their
bindings to comment on that, I'm afraid. My rough understanding was that
the twl4030 node here was referring to the logical subsystem as a whole,
with the mcbsp being a physical component, but that's almost certainly
somewhat wrong.

> &usb_otg_hs {
> 	interface-type = <0>;
> 	usb-phy = <&usb2_phy>;
> 	phys = <&usb2_phy>;
> 	phy-names = "usb2-phy";
> 	mode = <3>;
> 	power = <50>;
> };
> 
> Isn’t a PHY interface (e.g. ULPI-PHY) a “slave” connected to a 12 wire ULPI “bus” as well?

Not necessarily. If you took a look at the bindings you'd notice that
many PHYs have MMIO interfaces for configuration which we have to poke.
Those MMIO interfaces live on an MMIO bus so we describe those and link
to the phy by phandle reference.

A given device could operate as a PHY to multiple controllers, hence
phy-cells, and hence in general a PHY cannot be considered a slave
device.

> At least in this two areas everything done so far appears to contradict your argument.

Not really. You've found bindings with a different idiom, but those seem
to be organised as they are for reasons which don't appear to apply to
UART slave devices as you describe them.

> This is for me the blueprint how it should be done for UART slaves like any point-to-point
> multi-wire interfaces (question not even discussed: does UART-to-UART have clear master
> and slave roles? Isn’t the connected device the “master”? but I don’t want to broaden the
> discussion again).
> 
> This is basically why I keep this discussion open, because it is not that obvious
> that Neil’s proposal is right and mine is wrong.
> 
> > 
> >>> for other
> >>> interfaces like SPI and I2C. I do not see a compelling reason to do
> >>> otherwise for devices

[...]

> >>> hanging off of UARTs -- doing so would make things
> >>> less straightforward because you have a fundamentally different idiom.
> >> 
> >> Yes, my proposal is fundamentally different from I2C and SPI practice, but
> >> it is the same that is heavily used for e.g. GPIOs and Regulators.
> > 
> > Well, those cases are somewhat distinct, and I'd say that UART slaves
> > are much closer to SPI/I2C devices than GPIOs or regulators.
> 
> As shown above they are IMHO closer to McBSP and ULPI-PHY (and some
> other interfaces).

As above, I disagree.

> > Let's say I have a GPIO described via a phandle. That GPIO is actually
> > owned by some GPIO controller whose control interface is sat on an MMIO
> > bus. What we're describing with the phandle is use of the GPIO, but not
> > the main interface for interactive with the GPIO, which is the MMIO
> > interface of the controller.
> 
> Right. For the UART we do the same: the UART controller is connected
> to the MMIO and (in my proposal) the phandle describes the use of the UART
> (to connect to some slave device). Exactly the same situation.

Except that your fundamental interface to the device is via the UART,
which is not typically the case for things like regulators and/or GPIOs.
If I want a regulator to do something, I ask it to do so via the
controller's MMIO interface. If you want the device to do something, you
ask it to do so via the UART. 

> > In the case of UART slave devices the control interface is attached to
> > the UART, and effectively the slave sits on the UART's "bus". We could
> > refer to it from elsewhere by phandle, but its canonical parent should
> > be the UART, as that’s what its main interface is attached to.
> 
> What is the “main interface” of some device? Why should it have a special
> role in DT? I have doubts that it is useful to declare some interface as “main”,
> unless it is a MMIO bus connection.
> 
> There are e.g. chips with multi-interfaces like a twl4030. They have 2 I2C busses, 2 PCM
> “busses”, an ULIP-PHY and some GPIOs.

I think that would already be covered as an I2C device with two I2C IDs,
much like we would cater for an MMIO-interfaced PHY with two MMIO
control register regions

> Or some 3G/4G modems which have
> USB, UART (both useable for AT commands in parallel!), PCM and some GPIOs.

Of these I would expect that the USB or UART inerfaces would be the main
ones, though I would expect that we'd require two separate nodes which
we would have to link up.

> Which interface is “main”? For the twl4030 it happens that one of the I2C interfaces
> is chosen (because it allows to access the registers inside the chip).

Because it happens to be the "main" interface ;)

> Now you might say: yes, the registers of some uart connected device can
> be accessed through the uart as well. But usually there is a higher level
> protocol (AT commands) that give some sort of “register addressing”. But
> that is a different level than using I2C to access e.g. the gpio controller in the twl4030.
> 
> I just want to say that requiring and focussing on a “main” interface of a peripheral
> chip may lead to troubles.

I don't disagree that it falls apart in some cases. However, that's not
the general case, and it applies to other interface types too, so I
don't think it should matter in the context of this discussions.

> >> From my DT designer PoV I would say the UART exists in some SoC
> >> (independently of a device being connected) and then, a device is connected
> >> to the UART. Hence the proposal of adding this connection link to the device’s
> >> node and not making the device a subnode. And having the device driver do
> >> power management and only ask the uart/tty driver to be notified about open()
> >> and close() events. How power is managed in detail is then not any part of the
> >> tty/serial drivers.
> > 
> > The way that the power management interfaces are organised within Linux
> > is orthogonal to the way we describe things in the DT.
> 
> Agreed. And therefore power management registration, notifications etc.
> are to be hidden by the drivers and should not be an argument to say “with
> subnodes it is easier to implement and probing is done in the right order”.

While generally we shouldn't treat OS internals as an argument for DT
organisation, laying things out in a sensible manner for
discoverabilitiy is somewhat different from stating that the run-time
use of a device is fundamentally tied to its description in the DT.

We _could_ list all nodes in a flat list with cross references instead
of having a tree. But it would be horrible for _any_ OS to deal with, so
we don't do that.

Thanks,
Mark.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists