Messages in this thread | | | Subject | Re: [PATCH v1] gpio: mmio: restroe get multiple gpio mask | From | Yan Wang <> | Date | Mon, 24 Apr 2023 22:21:54 +0800 |
| |
> On Apr 24, 2023, at 21:31, Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > > On Sun, Apr 23, 2023 at 05:06:48PM +0800, Yan Wang wrote: >> Simplify the code,should not modify its logic. > >> Fixes: 761b5c30c206 ("gpio: mmio: replace open-coded for_each_set_bit()") > > What does it fix? > > ... > >> 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; > > Huh?! > > We never enter here if mask is 0. So, do not add a dead code, please. > > Moreover, in principle mask can be longer than 1 long, this code simply wrong.
You are right.
Because I use gpiod_set_array_value_cansleep() to set multiple gpios occur wrong . I restored logic of the original code and found it to be effective. So,I try to modify it.
I recheck logic of this position that it’s correct. I think there should be a error in Gpiolib.
> NAK > >> + 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); >> + } >> } > > -- > With Best Regards, > Andy Shevchenko > >
| |