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

[Bartosz Golaszewski <bgolaszewski@xxxxxxxxxxxx>]

wt., 28 kwi 2020 o 16:58 Paul Thomas <pthomas8589@xxxxxxxxx> napisał(a):
>
> On Tue, Apr 28, 2020 at 10:18 AM Andy Shevchenko
> <andriy.shevchenko@xxxxxxxxxxxxxxx> wrote:
> >
> > 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.
> OK, sounds good.
>

I applied the whole series to my for-next branch. I'll soon send a PR to Linus.

Bart