Re: [PATCH v1 2/3] Documentation: gpio: Add a section on what to return in ->get() callback

From: Hans de Goede
Date: Sat Dec 03 2022 - 07:10:52 EST


Hi Linus,

On 12/3/22 10:38, Linus Walleij wrote:
> On Wed, Nov 30, 2022 at 4:55 PM Andy Shevchenko
> <andriy.shevchenko@xxxxxxxxxxxxxxx> wrote:
>
>> +The below table gathered the most used cases.
>> +
>> +========== ========== =============== =======================
>> + Input Output State What value to return?
>> +========== ========== =============== =======================
>> + Disabled Disabled Hi-Z input buffer
>> + Disabled OS/OD/etc Single ended [cached] output buffer
>> + x Push-Pull Out [cached] output buffer
>> + Enabled Disabled In input buffer
>> + Enabled OS/OD/etc Bidirectional input buffer
>> +========== ========== =============== =======================
>
> This looks about right to me, but we need more input, Kent?

As I already mentioned in earlier replies to me this
seems to make things needlessly complicated for GPIO chips
where there are separate registers for reading the input-buffer vs
setting the output-buffer.

To implement the above drivers for these would need to check if
the pin is in push/pull mode and then read the register setting
the output-buffer in get() while reading the register reading
from the input-buffer in other cases in get().

I fail to see any downsides to just always reading
the register reading the input-buffer on GPIO chips like this,
when the pin in in push/pull output mode that should simply
give us the right value and when it does not this could
help detect short-circuits to Gnd/Vdd.

Where as I fear that implementing 2 different strategies in
get() for these kinda GPIO chips, will most likely be a
source of bug. Esp. since testing all the permutations
from the above table is going to be tricky in many cases.

If we go this route and demand that drivers for GPIO chips
with a separate (read-only) register for the input-buffer
sometimes read the register for the output-buffer on get()
can we then add a helper to the core which returns which
of the 2 registers should be used so that drivers don't
have to duplicate the logic for checking this ?

Regards,

Hans