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

[Kent Gibson]

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@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?
> > > 
> > 
> > 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.