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

[Linus Walleij]

On Wed, Mar 6, 2024 at 4:10 AM Chen-Yu Tsai <wens at kernel.org> wrote:
> On Wed, Mar 6, 2024 at 6:38 AM Linus Walleij <linus.walleij at linaro.org> wrote:
> > On Sun, Mar 3, 2024 at 3:19 PM Chen-Yu Tsai <wens at kernel.org> 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.
>
> But that doesn't really match the intended semantics. Maybe the driver
> should just ignore PIN_CONFIG_OUTPUT / PIN_CONFIG_INPUT if the hardware
> doesn't have matching toggles?
>
> On MediaTek hardware, they have input enable controls, but not for output
> enable. So mapping directly to GPIO direction doesn't quite make sense.

I think you should look what will work in practice, given the state
definitions in the device tree file and the runtime requirements from
GPIO.

If these things add up and work in practice it's fine.

Rough consensus and running code.

Yours,
Linus Walleij