Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Mon, 5 Dec 2022 23:35:59 +0800
From:   Kent Gibson <warthog618@gmail.com>
To:     Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc:     Linus Walleij <linus.walleij@linaro.org>,
        Marc Zyngier <maz@kernel.org>, linux-gpio@vger.kernel.org,
        linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,
        Bartosz Golaszewski <brgl@bgdev.pl>,
        Jonathan Corbet <corbet@lwn.net>,
        Hans de Goede <hdegoede@redhat.com>
Subject: Re: [PATCH v1 2/3] Documentation: gpio: Add a section on what to
 return in ->get() callback
Message-ID: <Y44P3811P1KB5pEl@sol>
References: <20221130155519.20362-1-andriy.shevchenko@linux.intel.com>
 <20221130155519.20362-2-andriy.shevchenko@linux.intel.com>
 <CACRpkdaQWZE6=BNEh5hSH9=jBK=TcLoD1uUb=JyNYmHFvaSAfg@mail.gmail.com>
 <Y41MxPthLjitvzEl@sol>
 <Y43fQFBcPgKtuKRZ@smile.fi.intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <Y43fQFBcPgKtuKRZ@smile.fi.intel.com>
Precedence: bulk

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