Date:   Mon, 13 Mar 2023 16:11:40 +0200
From:   Andy Shevchenko <andriy.shevchenko@intel.com>
To:     "Leonard, Niall" <Niall.Leonard@ncr.com>
Cc:     Linus Walleij <linus.walleij@linaro.org>,
        Bartosz Golaszewski <brgl@bgdev.pl>,
        Rob Herring <robh+dt@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        "linux-gpio@vger.kernel.org" <linux-gpio@vger.kernel.org>,
        "devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v2 2/2] gpio: mmio: Use new flag BGPIOF_NO_INPUT.
Message-ID: <ZA8vHLozhRrYRrER@smile.fi.intel.com>
References: <20230126-gpio-mmio-fix-v2-0-38397aace340@ncr.com>
 <20230126-gpio-mmio-fix-v2-2-38397aace340@ncr.com>
 <Y+jds9U1PgYpXBHT@smile.fi.intel.com>
 <ace51cc4-aa77-7ac0-b83f-12057a9bbae6@ncr.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <ace51cc4-aa77-7ac0-b83f-12057a9bbae6@ncr.com>
Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo
Precedence: bulk

On Mon, Mar 13, 2023 at 01:56:24PM +0000, Leonard, Niall wrote:
> On 12/02/2023 12:38, Andy Shevchenko wrote:
> > On Tue, Jan 31, 2023 at 01:49:38PM +0000, Niall Leonard wrote:
> >> Add new flag BGPIOF_NO_INPUT to header file.
> >> Use the existing shadow data register 'bgpio_data' to allow
> >> the last written value to be returned by the read operation
> >> when BGPIOF_NO_INPUT flag is set.
> >> Ensure this change only applies to the specific binding "wd,mbl-gpio".
> > 
> > I'm wondering why do we need that.
> > 
> > I mean the reading back the (possible cached) output value is the right thing
> > to do by default for GPIO (in output mode) or GPO. So, instead you can simply
> > check the current direction of the pin and return (cached) value.
> > 
> > Or did I miss something?
> > 
> My thinking here was that I don't want to break any existing code which 
> relies on the read always reading the physical port.
> I'm going to rethink my approach here as I'm starting to think the 
> better approach would be to modify the gpio-74xx-mmio.c driver to cater 
> for this hardware.

That's why we need a greatest denominator here, right?

Currently we have a full inconsistency on how drivers are implementing
all this. What I'm suggesting is to always have the following:
1) if pin is input or OS/OD/OE/OC -- read input buffer;
2) if pin is output, always read (cached) value.

Yes, there is an opinion to find a short circuit by reading input in
the output mode, but I consider that impractical complication.

Note, that above will also satisfy all (common) hardware limitations.

-- 
With Best Regards,
Andy Shevchenko