On Thu, 2022-07-14 at 17:58 +0300, Andy Shevchenko wrote: > On Wed, Jul 13, 2022 at 03:14:17PM +0200, Nuno Sá wrote: > > The gpio core looks at 'FLAG_BIAS_DISABLE' in preparation of > > calling the > > gpiochip 'set_config()' hook. However, AFAICT, there's no way that > > this > > flag is set because there's no support for it in firwmare code. > > Moreover, > > in 'gpiod_configure_flags()', only pull-ups and pull-downs are > > being > > handled. > > > > On top of this, there are some users that are looking at > > 'PIN_CONFIG_BIAS_DISABLE' in the 'set_config()' hook. So, unless > > I'm > > missing something, it looks like this was never working for these > > chips. > > > > Note that the ACPI case is only compiled tested. At first glance, > > it seems > > the current patch is enough but i'm not really sure... > > So, I looked closer to the issue you are trying to describe here. > > As far as I understand we have 4 state of BIAS in the hardware: > 1/ AS IS (defined by firnware) > 2/ Disabled (neither PU, not PD) > 3/ PU > 4/ PD > > The case when the default of bias is not disabled (for example > specific, and I > think very special, hardware may reset it to PD or PU), it's a > hardware driver > responsibility to inform the framework about the real state of the > lines and > synchronize it. > > Another case is when the firmware sets the line in non-disabled state > and > by some reason you need to disable it. The question is, why? > Not getting this point... > > As a side note, this came to my attention during this patchset [1] > > (and, ofr OF, was tested with it). > > > > [1]: > > https://lore.kernel.org/linux-input/20220708093448.42617-5-nuno.sa@xxxxxxxxxx/ > > Since this provides a GPIO chip, correct?, it's responsibility of the > driver to > synchronize it, no? Basically if you really don't trust firmware, you > may What do you mean by synchronize? > go via all GPIO lines and switch them to the known (in software) > state. This > approach on the other hand is error prone, because firmware should > know better > which pin is used for which purpose, no? If you don't trust firwmare > (in some > cases), then it's a matter of buggy platform that has to be quirked > out. > I'm not getting what you mean by "firmware should know better"? So, basically, and let's take OF as example, you can request a GPIO in OF by doing something like: foo-gpios = <&gpio 1 GPIO_PULL_UP>; In this way, when the consumer driver gets the gpio, all the flags will be properly set so that when we set a direction (for example) the gpiochip's 'set_config()' will be called and the driver does what it needs to setup the pull-up. If we want BIAS_DISABLED on the pin, there's no way to the same as the above. So basically, this can ever happen: https://elixir.bootlin.com/linux/latest/source/drivers/gpio/gpiolib.c#L2227 (only possible from the gpiochip cdev interface) So, what I'm proposing is to be possible to do from OF: foo-gpios = <&gpio 1 GPIO_PULL_DISABLE>; And then we will get into the gpiochip's 'set_config()' to disable the pull-up or pull-down. As I said, my device is an input keymap that can export pins as GPIOs (to be consumed by gpio_keys). The pins by default have pull-ups that can be disabled by doing a device i2c write. I'm just trying to use the infrastructure that already exists in gpiolib (for pull-up|down) to accomplish this. There's no pinctrl driver controlling the pins. The device itself controls them and having this device as a pinctrl one is not really applicable. - Nuno Sá
