Re: [PATCH v5 01/10] pinctrl: generic: Add bi-directional and output-enable

[Geert Uytterhoeven]

Hi Linus et al,

On Mon, May 8, 2017 at 11:19 PM, Linus Walleij <linus.walleij@xxxxxxxxxx> wrote:
> On Mon, May 8, 2017 at 7:25 PM, jmondi <jacopo@xxxxxxxxxx> wrote:
>> From my perspective these flags are configurations internal to the pin
>> controller hardware used to enable/disable input buffers when a pin needs to
>> perform in both direction.
>> The level of detail I can provide on this is the logical diagram we have pointed
>> you to already.
>>
>> As I assume you are trying to get this answer from us in order to
>> avoid duplicating things in pin controller sub-system, and I
>> understand this, but my question here was "can we have those flags as part
>> of the pinmux property argument list, as that property description
>> seems to allow us to do that, instead of making them generic pin
>> configuration properties and upset other developers?"
>
> Pinmux with all it's magic flags baked into one is not any better
> or any more readable. The solution is already very pretty except
> for these two flags which I am sure we can agree on a way forward
> for.
>
> What we choose between is not this or another less transparent
> pin configuration mechanism, the mechanism (whether magic bits
> to pinmux or reasonable properties) does not matter.
>
> There is a strong preference to use the generic bindings.
>
> So the discussion is whether to use:
>
> bi-directional;
> output-enable;
>
> Or some already defined config flags.
>
> If these are too idiomatic to be used by others, they should anyways
> look similar, like:
>
> renesas,bi-directional;
> renesas,output-enable;
>
> Like the Qualcomm weirdness found in drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
>
> qcom,pull-up-strength = <..>;
>
> Check how they use
> #define PMIC_GPIO_CONF_PULL_UP                 (PIN_CONFIG_END + 1)

The question I'm asking myself is: are these settings related to pin
configuration (i.e. depending on the use of the pin, and several settings
are valid, depending on the use case), or are they related to pinmux
(i.e. defined by the function)?

Configuring drive strength or pull-up are clearly related to pin
configuration.  Depending on what you connect, or on how you connect it,
you want a different drive strength, or choose a different output buffer
type (e.g. totem pole vs. open-collector). All of these are valid
configurations, depending on the use case.

But the settings RZ/A1H needs are different.  Some (e.g. "input-enable")
may sound like related to pin configuration. However, the big discerning
factor is that these settings are implied by the pinmux function. Their
presence is purely dictated by the chosen pinmux function. There's no use
case for doing otherwise (i.e. adding them when not needed, or removing
them when needed, according to the datasheet).

Note that e.g. the existing "input-enable" property is clearly a pin
configuration property. This is even reflected in DT, as the property is
not specified as part of the "pinmux" property, but in the surrounding pin
subnode of the pin-controller.

Hence I think we should not use generic pin properties, but consider these
settings to be part of pinmux configuration.
As having large tables in the driver is undesirable, I think storing the
settings in the "pinmux" property (by encoding them as flags passed to the
RZA1_PINMUX() macro) is our best option.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds