Re: [PATCH V3 1/2] pinctrl: bcm2835: Implement bcm2835_pinconf_get

[Linus Walleij <linus.walleij@xxxxxxxxxx>]

On Sun, Mar 3, 2024 at 3:19 PM Chen-Yu Tsai <wens@xxxxxxxxxx> wrote:

> Hi Linus,
(...)
> I'd like to take this opportunity to ask about INPUT_ENABLE and
> OUTPUT_ENABLE.
>
> AFAICT from existing comments in include/linux/pinctrl/pinconf-generic.h ,
> these two options refer to input and output buffers or connections within
> the general electric path, i.e. it allows the signal to pass through in
> a certain direction. It does not refer to the current selected direction
> of the GPIO function, which is covered by the PIN_CONFIG_OUTPUT option,
> and by gpiolib, if and only if the pin has been allocated for gpiolib
> use. But it seems multiple existing drivers do this.
>
> What's the correct thing to do here?

It's really up to the driver author: the text in pinconf-generic.h does its best
to describe the intended semantics, but sometimes hardware will not fully
match what is said in the documentation.

I guess in this case the PIN_CONFIG_OUTPUT_ENABLE and
PIN_CONFIG_OUTPUT is not two distinctly different things for this
hardware so a reasonable semantic is to implement both in the same
case and do the same for both of them.

+       case PIN_CONFIG_OUTPUT_ENABLE:
+       case PIN_CONFIG_OUTPUT:
+               if (fsel != BCM2835_FSEL_GPIO_OUT)
+                       return -EINVAL;
+
+               val = bcm2835_gpio_get_bit(pc, GPLEV0, pin);
+               *config = pinconf_to_config_packed(param, val);
+               break;

Does it seem reasonable?

Yours,
Linus Walleij