Thread (41 messages) 41 messages, 8 authors, 2022-07-19

Re: [PATCH 1/4] gpiolib: add support for bias pull disable

From: Nuno Sá <hidden>
Date: 2022-07-14 07:13:24
Also in: linux-acpi, linux-gpio

On Thu, 2022-07-14 at 12:20 +0800, Kent Gibson wrote:
On Wed, Jul 13, 2022 at 08:36:38PM +0300, Andy Shevchenko wrote:
quoted
On Wed, Jul 13, 2022 at 03:14:18PM +0200, Nuno Sá wrote:
quoted
This change prepares the gpio core to look at firmware flags and
set
'FLAG_BIAS_DISABLE' if necessary. It works in similar way to
'GPIO_PULL_DOWN' and 'GPIO_PULL_UP'.
...
quoted
        GPIO_PULL_UP                    = (1 << 4),
        GPIO_PULL_DOWN                  = (1 << 5),
+       GPIO_PULL_DISABLE               = (1 << 6),
To me it seems superfluous. You have already two flags:
PUp
PDown
When none is set --> Pdisable
Agree with Andy on this.  The FLAG_BIAS_DISABLE was added, by me, to
allow the cdev interface to support bias.  cdev requires a "don't
care"
state, distinct from an explicit BIAS_DISABLE.
The FLAG_BIAS_DISABLE allows gpiolib-cdev to communicate that to
gpiolib, without altering the interpretation of the existing PULL_UP
and
PULL_DOWN flags.
That is not an issue on the machine interface, where the two
GPIO_PULL
flags suffice.
I see, but this means we can only disable the pin BIAS through
userspace. I might be wrong but I don't see a reason why it wouldn't be
valid to do it from an in kernel path as we do for PULL-UPS and PULL-
DOWNS 
If you are looking for the place where FLAG_BIAS_DISABLE is set it is
in
gpio_v2_line_config_flags_to_desc_flags() in gpiolib-cdev.c.

Referring to gpio_set_bias(), the only place in gpiolib the
FLAG_BIAS_DISABLE is used, if neither FLAG_PULL_UP, FLAG_PULL_DOWN,
nor FLAG_BIAS_DISABLE are set then the bias configuration remains
unchanged (the don't care case) - no change is passed to the driver.
Otherwise the corresponding PIN_CONFIG_BIAS flag is passed to the
driver.
Exactly, but note FLAG_BIAS_DISABLE can only be set from userspace at
this point (IIUTC). If everyone agrees that should be case, so be it.
But as I said, I just don't see why it's wrong to do it within the
kernel.
If there are cases of drivers not fully or correctly supporting those
PIN_CONFIG_BIAS flags, then that is an issue with those drivers.
Look at my reply to Andy in the cover for more details

- Nuno Sá
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help