Re: [PATCH 1/2] pinmux: Add TB10x pinmux driver

From: Linus Walleij
Date: Tue May 14 2013 - 08:29:52 EST


On Fri, May 10, 2013 at 10:25 AM, Christian Ruppert
<christian.ruppert@xxxxxxxxxx> wrote:

>> > What do you think about the following modification to the pinctrl/GPIO
>> > frameworks instead (not yet a formal patch, more a request for comment
>> > to illustrate what I mean. If you agree, I will clean it up and submit a
>> > proper patch after discussion).
>> >
>> > It adds a dt_gpiorange_xlate function to the pinctrl callbacks which
>> > defaults to the conventional behaviour using kernel logical pin numbers.
>> > However, pin controllers which provide more complex mechanisms can
>> > define #gpio-range-cells and provide this callback in order to keep
>> > Linux pin numbering inside the kernel.
(...)
> The patch does not change the default behaviour of the kernel: In case
> no dt_gpiorange_xlate callback is defined for a given driver (e.g. for
> pre-existing drivers), the default function simply interprets the first
> argument as Linux pin number and the second as pin count, same as now.
> New drivers can use the callback to translate device specific pin
> references to Linux pin numbers (in the idea of of_xlate in the GPIO
> framework or xlate in the irqchip framework).

I like the concept.

However I am totally opposed to the idea of making this something
device tree-exclusive.

Look at for example
drivers/pinctrl/pinctrl-abx500.c:

static u8 abx500_get_mode(struct pinctrl_dev *pctldev, struct gpio_chip *chip,
unsigned gpio)
{
(...)
/* on ABx5xx, there is no GPIO0, so adjust the offset */
unsigned offset = gpio - 1;

As you can see, this driver, which does not use device tree,
is working around the same problem. Here the problem is that
the pins are numbered starting at 1 instead of 0, a very trivial
numberspace shuffleing.

I'd be open to this approach if you:

- Make it generic for all pinctrl drivers, i.e. add the translation
to the core so it does not just apply to devices using device tree.

- Augment the pinctrl-abx500.c driver to show how this simplifies
that driver. (Does not need to be perfect, I'll help out finalizing it
for sure.)

- Then add DT-specific wrapper using this core feature.

This way the problem will be solved for everybody, including
ACPI when they sooner or later come back with the same issue.

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