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

[Linus Walleij]

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)

Etc.

> Anyway, I still fail to see why those configuration flags, only
> affecting the way the pin controller hardware enables/disables
> its internal buffers and its internal operations have to be
> described in term of their externally visible electrically characteristics.

To me internal vs external is not what matters. What matters is
if this is likely to pop up in more platforms, and then the property
should be generic.

The generic pin config definitions are likely to be picked up by other
standards and even be inspiration to hardware engineers so that
is why it matters so much.

> To me, what already exists are pin configuration properties generic to
> the whole pin controller subsystem, and I understand you don't want to
> see duplication there.
>
> At the same time, to me, those flags are settings the pin controller
> wants to have specified by software to overcome its hw design flaws,
> and are intended to configure its internal buffers in a way it cannot
> do by itself for some very specific operation modes (they are listed
> in the hw reference manual, it's not something you can chose to
> configure or not, if you want a pin working in i2c mode, you HAVE to
> pass those flags to pin controller).

Sounds like a case for

renesas,bi-directional;
renesas,output-enable;

following the Qualcomm pattern in that case.

But let's see if something else comes out of this discussion.

Yours,
Linus Walleij