Re: [PATCH v1 2/3] Documentation: gpio: Add a section on what to return in ->get() callback
From: Kent Gibson
Date: Sun Dec 04 2022 - 20:43:45 EST
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.
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.
Cheers,
Kent.