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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <8246b761-d83d-de59-fa30-a82d6bbc9533@redhat.com>
Date:   Sat, 3 Dec 2022 13:09:44 +0100
From:   Hans de Goede <hdegoede@...hat.com>
To:     Linus Walleij <linus.walleij@...aro.org>,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        Kent Gibson <warthog618@...il.com>
Cc:     Marc Zyngier <maz@...nel.org>, linux-gpio@...r.kernel.org,
        linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org,
        Bartosz Golaszewski <brgl@...ev.pl>,
        Jonathan Corbet <corbet@....net>
Subject: Re: [PATCH v1 2/3] Documentation: gpio: Add a section on what to
 return in ->get() callback

Hi Linus,

On 12/3/22 10:38, Linus Walleij wrote:
> On Wed, Nov 30, 2022 at 4:55 PM Andy Shevchenko
> <andriy.shevchenko@...ux.intel.com> 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