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: <Y44P3811P1KB5pEl@sol>
Date:   Mon, 5 Dec 2022 23:35:59 +0800
From:   Kent Gibson <warthog618@...il.com>
To:     Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
Cc:     Linus Walleij <linus.walleij@...aro.org>,
        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>,
        Hans de Goede <hdegoede@...hat.com>
Subject: Re: [PATCH v1 2/3] Documentation: gpio: Add a section on what to
 return in ->get() callback

On Mon, Dec 05, 2022 at 02:08:32PM +0200, Andy Shevchenko wrote:
> On Mon, Dec 05, 2022 at 09:43:32AM +0800, Kent Gibson wrote:
> > On Sat, Dec 03, 2022 at 10:38:45AM +0100, 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?
> > > 
> > 
> > Firstly, I'm all for tightening up the driver contract, and hope that
> > whatever is decided will also be updated in driver.h itself.
> > 
> > I can also understand Andy wanting to add support for Bidirectional
> > using the existing API.
> > 
> > But, and please correct me if I'm wrong, the user has no control over
> > whether an open drain output is single ended or bidirectional, and
> > no visibility as to which the driver supports or chooses.
> > So the contract is still vague.
> > 
> > My preference would be for the driver API to be extended with a new
> > callback for the output buffer, say get_output(), and have the existing
> > get() always return the input buffer.  Both would return an error if the
> > buffer is unavailable or disconnected, e.g. in the Hi-Z case.
> > As per Hans' suggestions, this would keep the drivers simple.
> 
> That's not about keeping driver simple, it's about how from hardware
> (electrical) point of view we should recognize the GPIO signal value.
> And I disagree on the input buffer to be always involved (in particular,
> not all hardware may support that anyway). That said, I will send an answer
> to all you guys, but just to make sure that we are on the different pages
> here I state yet another time that this is not about solely software p.o.v.
> And yes, there is no simple answer to the question.
> 

To be clear, my suggestion is focussed on providing visibility to allow
the user to determine if their hardware supports their use case - without
them having to get out a scope to check.
And it doesn't care what those use cases are.

The fact that it also keeps the driver logic simple is a happy
coincidence, but I agree with Hans that that is a huge benefit and so
reiterated it above.  My bad if that gave the impression that was my
primary focus.

> > Then cdev could determine the approriate buffer to return, depending
> > on the mode.  Or, better yet, we extend that through the uAPI and
> > handball that decision to the user.
> 
> TL;DR: I don't like this idea.
> 

And yours paints us into a corner.

Cheers,
Kent.