Messages in this thread | | | From | Linus Walleij <> | Date | Mon, 24 Apr 2023 08:56:52 +0200 | Subject | Re: [PATCH v1] gpio: mmio: restroe get multiple gpio mask |
| |
Hi Yan,
thanks for your patch!
On Sun, Apr 23, 2023 at 11:07 AM Yan Wang <rk.code@outlook.com> wrote:
> Simplify the code,should not modify its logic.
I don't see how it simplifies the code, it seems to me that it is making it more complex?
> Fixes: 761b5c30c206 ("gpio: mmio: replace open-coded for_each_set_bit()") > Signed-off-by: Yan Wang <rk.code@outlook.com> > --- > drivers/gpio/gpio-mmio.c | 12 ++++++++---- > 1 file changed, 8 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpio/gpio-mmio.c b/drivers/gpio/gpio-mmio.c > index d9dff3dc92ae..c2347ef3a4df 100644 > --- a/drivers/gpio/gpio-mmio.c > +++ b/drivers/gpio/gpio-mmio.c > @@ -271,10 +271,14 @@ static void bgpio_multiple_get_masks(struct gpio_chip *gc, > *clear_mask = 0; > > for_each_set_bit(i, mask, gc->bgpio_bits) { > - if (test_bit(i, bits)) > - *set_mask |= bgpio_line2mask(gc, i); > - else > - *clear_mask |= bgpio_line2mask(gc, i); > + if (*mask == 0) > + break; > + if (__test_and_clear_bit(i, mask)) { > + if (test_bit(i, bits)) > + *set_mask |= bgpio_line2mask(gc, i); > + else > + *clear_mask |= bgpio_line2mask(gc, i); > + }
The intention of the change seems to be to break out of the loop when all set bits are handled, by successively masking off one bit at a time from mask. So this is intended as an optimization, not a simplification.
I think for_each_set_bit() is already skipping over every bit that is zero, see include/linux/find.h.
So this optimization gains us nothing.
Yours, Linus Walleij
| |