On 06/19/2013 12:10 PM, Stephen Warren wrote:

On 06/14/2013 03:12 AM, Christian Ruppert wrote:

quoted

On Thu, Jun 13, 2013 at 03:38:09PM -0600, Stephen Warren wrote:

quoted

On 06/13/2013 06:55 AM, Christian Ruppert wrote:

quoted

This patch adds the infrastructure required to register non-linear gpio
ranges through gpiolib and the standard GPIO device tree bindings.

That's not exactly true. The existing gpio-ranges property already
allows non-linear ranges to be represented quite easily; each entry in
the gpio-ranges list is <gpio-base> <pinctrl-base> <count>, so you can
piece together any mapping you want.

You're right, my description is somewhat imprecise here.

quoted

The potential advantage of this patch is that the pinctrl-side of the
mapping can be a group name rather than pin IDs, which might reduce the
size of the mapping list if you have an extremely sparse or non-linear
mapping /and/ parts of that mapping just happen to align with the pin
groups in the pin controller HW, since each entry in the gpio-ranges
property can be sparse/non-linear, rather than being a small linear
chunk of the mapping.

Pin controller authors have the freedom to define pin groups just for
the purpose of "predefining" the pinctrl side of GPIO ranges.

Hmm. I suppose that's true. I'm not sure how enthusiastic I am about
doing this though... The reason I'm unsure is because it starts using
pin groups from something other than groups of pins in HW that are all
affected by the same mux or config bits in a register, and starts using
pin groups for something else; GPIO<->pinmux pins mapping. Perhaps it's
OK though, considering the other abuses of pin groups that are already
present, such as using pin groups to represent default/common uses of
groups of pins that don't actually exist in HW.

I've realized what I don't like about this.

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.

However, the connectivity between GPIO HW module "pins" (i.e. the GPIOs)
and pinctrl HW module "pins" (inputs to mux functions) is something that
only exists at the top-level of the SoC; outside the GPIO HW module
itself, and outside the pinctrl HW module itself.

In other words, you could have the exact same GPIO and pinctrl HW
modules instantiated into two different SoCs, but with completely
different mapping of GPIO IDs to pinctrl pin IDs.

As such, it isn't even generally possible for the pinctrl HW module to
define pin groups that describe the mapping, because the mapping is not
a property of the pinctrl HW module, and hence should not be defined,
even partially, by the pinctrl HW module's driver. In a similar fashion,
the DT binding for the pinctrl HW module should describe only the HW
module itself, and not the mapping/interaction with the outside world.
In other words, the DT binding for the pinctrl HW module also can't
define the names of any pin groups used in the GPIO<->pinctrl mapping,
for the same reasons.

As such, I'm not sure that I conceptually agree with this patch series.

Sure, it may make the gpio-ranges property more compact in some
(unusual?) non-linear cases. However, it's representing things
semantically incorrectly.

So, I'd like to question the motivation for using names here again.
Presumably the SoC vendor will write the gpio-ranges property for each
SoC, and put that into the SoC's .dtsi file. As such, no customer is
ever going to have to care about the property or its contents. So, I
don't really see how this helps you with your issue re: wanting to hide
details of multiple different ball-out options on similar SoCs, since
even with a manually-written purely numeric gpio-ranges property, all
that information is already essentially hidden; it's something that will
be written once, and then never looked at.