Re: [PATCH v3 1/3] gpio: pca953x: Rewrite ->get_multiple() function

[Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>]

On Tue, Apr 28, 2020 at 09:09:03AM -0400, Paul Thomas wrote:
> On Tue, Apr 28, 2020 at 8:41 AM Andy Shevchenko
> <andriy.shevchenko@xxxxxxxxxxxxxxx> wrote:
> >
> > On Tue, Apr 28, 2020 at 02:13:21PM +0200, Linus Walleij wrote:
> > > On Tue, Apr 21, 2020 at 5:42 PM Bartosz Golaszewski
> > > <bgolaszewski@xxxxxxxxxxxx> wrote:
> > > > wt., 21 kwi 2020 o 15:03 Andy Shevchenko
> > > > <andriy.shevchenko@xxxxxxxxxxxxxxx> napisał(a):
> > > > > On Mon, Apr 20, 2020 at 08:27:50PM +0300, Andy Shevchenko wrote:
> >
> > ...
> >
> > > I don't mind trying to put in code to optimize use cases
> > > when accessing single bytes here either. But I'd like there
> > > to be explicit comments in the code saying why these
> > > optimizations are there. Can we do those on top of
> > > this (hopefully) known working base?
> For the record my original get_multiple patch was a known working base.
> 
> >
> > As I pointed out to Paul, the optimization like he proposed is not bad thing
> > per se, the implementation is. On top of that I suggested to take a look to IRQ
> > status bits, which presumable will leverage from this optimization as well.
> >
> > So, After applying this series it would be matter of change one line in the
> > ->get_multiple() to replace read_regs() with optimized version or so along with
> > IRQ bits changes.
> 
> This new function would then at least need to be called with mask as
> an additional argument right? Then the bitmap_replace() will set
> everything regardless of if it was read, this is fine I suppose
> because it doesn't matter if it's setting bits outside of mask. You
> just have two loops one in the new function and one in
> bitmap_replace().

Note, on 64-bit architectures there is no loop in bitmap ops (since 40 <= 64 in
this case). On 32-bit it might be (only for 40 case, which I think less present
in the wild than the rest). And bitmap ops are optimized over longs, so, it's
pretty much fast (esp. in comparison to I²C IO).

> If this is what people would like to see I can work
> on it. I did look into the sparse reads and it seems as though regmap
> has a gather_write, but not a gather_read and gather_write isn't in
> regmap-i2c.c anyway.

Yes, I think this is the right way to go.

-- 
With Best Regards,
Andy Shevchenko