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

[Sander Vanheule]

Hi Andy,

On Tue, 2021-06-01 at 18:24 +0300, Andy Shevchenko wrote:
> On Tue, Jun 1, 2021 at 2:49 PM Michael Walle <michael@xxxxxxxx> wrote:
> > Am 2021-05-31 17:48, schrieb Andy Shevchenko:
> > > On Mon, May 31, 2021 at 6:33 PM Sander Vanheule <sander@xxxxxxxxxxxxx>
> > > wrote:
> > > > On Mon, 2021-05-31 at 14:16 +0300, Andy Shevchenko wrote:
> > > > > On Monday, May 31, 2021, Michael Walle <michael@xxxxxxxx> 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.

Thank you for clarifying. Essentialy this is then the same solution as an extra
cache in gpio-regmap for the output values, except the cacheing is handled by
the regmap layer.

I think this was the last issue standing, so after I implement this, I'll spin a
v4.

Best,
Sander