Re: [PATCH v6] pinctrl: add a pin config interface

[Linus Walleij <linus.walleij@linaro.org>]

On Wed, Dec 14, 2011 at 8:04 AM, Chanho Park <chanho61.park@samsung.com> wrote:

>> ChangeLog v5->v6:
>> - Change to using a pin name as parameter for the
>>   [get|set]_config() functions, as suggested by Stephen Warren.
>>   This is more natural as names will be what a developer has
>>   access to in written documentation etc.
>
>
> I don't agree with it.
> Someone doesn't like to assign a pin name individually because a
> pin number is sufficient to represent each pin.
> We also provide a macro to define a anonymous pin descriptor.
>
> /* Convenience macro to define a single named or anonymous pin descriptor */
> #define PINCTRL_PIN(a, b) { .number = a, .name = b }
> #define PINCTRL_PIN_ANON(a) { .number = a }

That is true, anonymous pins won't be reachable from the
outside. But to some extent that's to be expected.

What turned me around to see things Stephen's way was the fact
that if someone reads this code and finds:

pin_config_set(dev, 42, MY_FUNNY_PULLUP_MACRO);

It is harder to understand than:

pin_config_set(dev, "I2C0 SCL", MY_FUNNY_PULLUP_MACRO);

The second gives and idea about what's going on - you can
look up the pin name "I2C0 SCL" in the datasheet and if
you know something about I2C electronic requirements you
immediately see that "aha, it's pulling up I2C0 SCL line, yeah
that's required of course".

> If we use only the pin name for [get|set]_config(), we must assign the name
> of all pins.

No, you only need to assign a name for the pins you want to
send control commands to. You can still use the pins for
say pinmuxing without naming them.

> IMHO, we also provide pin numbers to control anonymous pins.

Also or instead of? I think the interface needs to be
either names or pin numbers, supporting both will
be confusing.

Is this a blocker for the Samsung way of doing things?

I'm not entirely sure about this ... what do others say?

Yours.
Linus Walleij