On Thu, Aug 22, 2013 at 02:10:55PM -0600, Stephen Warren wrote:

On 08/21/2013 09:57 AM, Christian Ruppert wrote:

quoted

On Wed, Aug 14, 2013 at 06:53:56PM +0200, Linus Walleij wrote:

quoted

On Mon, Aug 5, 2013 at 1:51 PM, Christian Ruppert
[off-list ref] wrote:

quoted

[Me]

quoted

I don't see any of the port concept creeping into the device tree
in this version and that is how I think it should be kept:
the "port" particulars is a thing for the driver and not the
device tree.

...

quoted

quoted

quoted

In the driver under discussion, pin groups are defined for every
"interface" to make sure that interfaces can be requested in an
orthogonal way by different modules and modules don't have to be "aware"
of which interfaces are grouped into which port (and which other modules
request which other interfaces). A request either succeeds or fails.
Resource management (which interfaces can be mapped simultaneously) is
done inside the pinctrl driver.

OK

This actually looks 100% coherent with Documentation/pinctrl.txt. But
then I don't understand Stephen's request to introduce the concept of
"ports" in the device tree. IMHO ports are a hardware limitation which
should be managed inside the pinctrl driver and if possible not leak
out of it. Also (as stated above), the concept of "ports" does not even
exist in the pinmux framework so why introduce it for DT?

I might have thoroughly misunderstood you here, Stephen. Please be
patient with me and explain once more.

I don't think I asked for ports to be represented in DT. Do you have
more context?

Well, you never asked about ports but you asked specifically about pin
groups. The most concise definition of pin groups I have found in your
mails is from https://lkml.org/lkml/2013/6/21/462:

When I pushed for the concept of groups, I intended it to mean precisely
one single thing. The points below describe this.

1) A pin is a single pin/ball/pad on the package.

2) Some register fields affect just a single pin. For example, there may
be a register field that affects pin A8's mux setting only.

3) Some register fields affect multiple pins at once. For example,
perhaps one register field affects both pin A8's an pin A7's mux setting
at once.

4) Depending on HW design, all register fields might be of type
described at (2) above, or all of the type described at (3) above, or a
mixture of both. Tegra is a mixture.

5) I expect the concept of a pin group to solely represent the various
groups of pins affected by each register field; in (2) above one pin per
group, in (3) above many pins per group.

Thus, to my mind, a pin group is purely a HW concept, and dictated
purely by HW design.

Point 5 clearly defines the "concept of a pin group to solely represent
the various groups of pins affected by each register field". The TB10x
definition of a "port" (concept which doesn't seem to exist in the pinctrl
framework) is the set of pins controlled by the same register field
(i.e. the same mux). In my understanding this matches point 5 but as I
said this is probably a misunderstanding and I would be grateful for
clarification. There are other comments which fuel that
misunderstanding, e.g. https://lkml.org/lkml/2013/6/19/589:

Pin groups are supposed to be something that represents some property of
the pinctrl HW itself. So, if you have register "X" bits 3-0 that define
the mux function for pins 8, 9, 10, and 11, then there really is a pin
group that exists in HW, and that pin group will still exist with that
same definition no matter what SoC you put the pinctrl HW into. If this
changes, it's not the same pinctrl HW module.

In TB10x, every function can be activated on exactly one pin group, and
Documentation/pinctrl.txt says "If only one possible group of pins is
available for the function, no group name need to be supplied.".

Maybe the answer to our concrete question of the tb10x driver is thus
renaming the pingrp device tree property of the original patch into
something like function (by which a pin group can be implied)?

For example:
iomux: iomux@FF10601c {
        compatible = "abilis,tb10x-iomux";
        reg = <0xFF10601c 0x4>;
        pctl_gpio_a: pctl-gpio-a {
                abilis,function = "gpioa";
        };
        pctl_uart0: pctl-uart0 {
                abilis,function = "uart0";
        };
};

What do you think?
  Christian