Re: [PATCH 0/5] RTL8231 GPIO expander support

From: Andy Shevchenko <andy.shevchenko@gmail.com>
To: Michael Walle <michael@walle.cc>
Cc: Sander Vanheule <sander@svanheule.net>,
	Hans de Goede <hdegoede@redhat.com>, Andrew Lunn <andrew@lunn.ch>,
	Pavel Machek <pavel@ucw.cz>, Rob Herring <robh+dt@kernel.org>,
	Lee Jones <lee.jones@linaro.org>, Mark Brown <broonie@kernel.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	"Rafael J . Wysocki" <rafael@kernel.org>,
	Linus Walleij <linus.walleij@linaro.org>,
	Bartosz Golaszewski <bgolaszewski@baylibre.com>,
	Linux LED Subsystem <linux-leds@vger.kernel.org>,
	devicetree <devicetree@vger.kernel.org>,
	"open list:GPIO SUBSYSTEM" <linux-gpio@vger.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 0/5] RTL8231 GPIO expander support
Date: Tue, 1 Jun 2021 18:24:09 +0300	[thread overview]
Message-ID: <CAHp75VdaPS0YPvNHVgeDOuz=9ACn7okHxhyNEZuyQR8=kSdNdg@mail.gmail.com> (raw)
In-Reply-To: <8df77f619730b9e7b5cdd7ddefb60a03@walle.cc>

On Tue, Jun 1, 2021 at 2:49 PM Michael Walle <michael@walle.cc> wrote:
> Am 2021-05-31 17:48, schrieb Andy Shevchenko:
> > On Mon, May 31, 2021 at 6:33 PM Sander Vanheule <sander@svanheule.net>
> > wrote:
> >> On Mon, 2021-05-31 at 14:16 +0300, Andy Shevchenko wrote:
> >> > On Monday, May 31, 2021, Michael Walle <michael@walle.cc> wrote:
> >> > > Am 2021-05-31 10:36, schrieb Sander Vanheule:
> >
> >> Am I missing something here? It seems to me like the regmap interface
> >> can't
> >> really accommodate what's required, unless maybe the rtl8231 regmap
> >> users
> >> perform some manual locking. This all seems terribly complicated
> >> compared to
> >> using an internal output-value cache inside regmap-gpio.
> >
> > Have you had a chance to look into the PCA953x driver?
> > Sounds to me that you are missing the APIs that regmap provides.
>
> What would that be? The register cache? We need to cache the
> value somehow, because (still assuming it is write only) we cannot
> read it back. Thus the read of the RMW, would need get the
> value from the cache. Thus the user of gpio-regmap would need
> to make sure, to (a) use a cache for the regmap supplied to
> gpio-regmap and (b) populate its initial values correctly. Is
> that what you are suggesting? And hopefully, no other user
> of the regmap will call regcache_mark_dirty() or something
> like that.
>
> I had a quick look at the PCA953x driver but it all its
> registers are readable according to the comment on the top
> of the file.

Since regmap doesn't have a facility to support the registers _at the
same offset_ with different meaning (depending on data direction), the
only way to handle this (without redesign regmap internals) is to add
specific "pages" via additional bits in the address space. So, let's
say 0 = RW, 1 = RO, 2 = WO. In this case see the following offset
mapping of the hypothetical hardware registers:

REG1 (RW)   0x00 -> 0x000
REG2 (RW)   0x04 -> 0x004
REG3 (RO)   0x08 -> 0x108
REG3 (RW)   0x08 -> 0x208

Then these bits should be masked out. Something similar is done in the
PCA953x driver for extended addresses and autoincrement.

This is what I propose to look at as the starter.

-- 
With Best Regards,
Andy Shevchenko