Re: [PATCH] pinctrl: indicate GPIO direction on single GPIO request

[Linus Walleij]

On Mon, Nov 21, 2011 at 10:54 PM, Stephen Warren <swarren@xxxxxxxxxx> wrote:
> Linus Walleij wrote at Monday, November 21, 2011 2:44 PM:
>> On Mon, Nov 21, 2011 at 8:00 PM, Stephen Warren <swarren@xxxxxxxxxx> wrote:
>> >
>> > int gpio_request_at(unsigned gpio, const char *label, unsigned at);
>>
>> This has a nice look to it, I must admit.
>>
>> Since the pinctrl number space is per-controller this signature
>> will not work. It would have to pass in a pin controller reference
>> too, preferably as struct device * or a string. Else "at" is just
>> a number without meaning. "At *which* controller?"
>
> I'm assuming that each (physical) chip will have one pinctrl driver.
> Therefore, for any GPIO driver (and hence any GPIO number) on that chip,
> there is a single pinctrl driver that's relevant, and hence pinctrl
> driver can be determined directly and internally from the GPIO ID.

Yep that's the assumption behind the current range concept
as well. (And the ranges are used to look up pin controllers for
a certain GPIO.)

> Now that I say it though, I wonder if that assumption is totally valid;
> after all, we're talking about having multiple GPIO controller drivers
> within a (physical) chip, so perhaps having multiple pinctrl drivers
> for different subsets of the pins wouldn't be impossible, albeit pretty
> unlikely and bizarre. Should we just ignore that case?

No but we could postpone dealing with it until there is a
piece of hardware that needs the feature. :-)

> We could deal
> with it by internally creating an aggregate driver if it ever happened.

That's one way, the agregate driver will be more beautiful
if we spread it out across a few separate files in that case.
But let's take that when it hits us.

>> Yes letting gpio_chip hold a reference to the pin controller
>> is equivalent to the pin controller GPIO range holding a
>> reference to the struct gpio_chip * as it can do today.
>
> Yes, those are pretty much the same thing.
>
> Assuming my assumption above is correct, then each GPIO (and GPIO chip)
> maps to a single pinctrl, which makes storing the data easy and static,
> but each pinctrl range might map to different GPIO chips and IDs which
> makes things harder, so I still see some value pushing the data storage
> into GPIO.

I think it's possible to make a patch for that and refactor
the current range concept into that pretty cleanly while
moving along with the current codebase. It would take some time
though, not due to writing code but due to unavoidable discussions
about gpiolib, since people are full of opinions regarding that...

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/