On 07/05/2013 03:49 AM, Christian Ruppert wrote:

On Wed, Jun 26, 2013 at 11:40:42AM -0600, Stephen Warren wrote:

quoted

On 06/26/2013 05:50 AM, Christian Ruppert wrote:

quoted

On Wed, Jun 19, 2013 at 04:35:14PM -0600, Stephen Warren wrote:

quoted

On 06/18/2013 03:29 AM, Christian Ruppert wrote:

[...]

quoted

quoted

quoted

+Example
+-------
+
+iomux: iomux@FF10601c {
+	compatible = "abilis,tb10x-iomux";
+	reg = <0xFF10601c 0x4>;
+	pctl_gpio_a: pctl-gpio-a {
+		pingrp = "gpioa_pins";
+	};
+	pctl_uart0: pctl-uart0 {
+		pingrp = "uart0_pins";
+	};
+};

The two nodes pctl-gpio-a and pctl-uart0 seem to be missing data. The
idea here is that you define nodes that says:

* This node applies to these pin(s)/group(s).
* Select mux function X on those pins/groups and/or apply these pin
configuration options to those pins/groups.

The examples above don't include any mux/config options, nor does the
binding say how to do specify them.

The set of pin groups defined by this binding should correspond directly
to the set of pin groups that actually exist in HW. So, if you have 3
pin groups (A, B, C) in HW each of which has two mux functions (X, Y),
your DT binding should define just 3 pin groups (A, B, C), not 6 (A_X,
A_Y, B_X, B_Y, C_X, C_Y). In other words, the pin group name shouldn't
imply the mux function.

Can we consider it as agreed now that this implementation is acceptable
for the TB10x pin controller?

There are two issues here:

1) What is a pin group:

1a) Must it solely represent a group of pins that actually exists in HW
(e.g. it's an RTL port, or a set of pins all controlled at once by a
single bit/field in a register)

1b) A SW-defined group of pins, simply because it's convenient to talk
about that set of pins at once, even though HW doesn't impose that those
pins are in a group in any way.

Defining groups for either of those reasons is fine, although this is
the area where my preference and LinusW's differ.

2) Can groups represent just a set of pins, or can it also imply that a
particular mux function is selected on that group?

I believe that both LinusW and I are in agreement that a group is simply
a list/set/group of pins. You select mux functions onto groups. A
groups's definition can't imply that a particular mux function is
selected onto it.

If we don't follow this rule, then you end up with a combinatorial
explosion of groups; the cross-product of all possible groups of pins
vs. the mux function to select on them, rather than simply having a list
of groups of pins, which is a much smaller set/list.

So, in the DT example above, I still believe that you need an extra
property that defines which mux function to select onto the specified
group. The group name can't imply this, so there needs to be some way of
specifying it.

In your opinion, would something in the lines of

pctl_spi1: pctl-spi1 {
	abilis,pingrp = "spi1";

So that defines a list of pins.

	abilis,ioport = <4>;   /* spi1 is routed to port4 inside the
				pin controller */

I assume that defines the mux function value; the value that's
programmed into the HW register to select which HW module's signals are
routed out to the pins specified by abilis,pingrp.

	abilis,ioconf = <1>;   /* spi1 is available in configuration 1
				of that port. */

But I don't understand what that is. ...

...

In future, this could even be extended to allow several alternative
configurations for a given function, e.g.

pctl_spi3: pctl-spi3 {
	abilis,pingrp = "spi3";
	abilis,ioport = <6>;
	abilis,ioconf = <0 3>; /* spi3 is available in both
				configurations 0 and 3. Depending on
				what other functions are requested, the
				pinctrl driver can choose either of the
				two. */

... especially if you're talking about "spi3 being available in multiple
configurations". What's a configuration?