| Message ID | 20180929122022.6825-1-jmkrzyszt@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | gpiolib: Fix incorrect use of find_next_zero_bit() | expand |
Hi Janusz, On 2018-09-29 14:20, Janusz Krzysztofik wrote: > Commit b17566a6b08b ("gpiolib: Implement fast processing path in > get/set array"), already fixed to some extent with commit 5d581d7e8cdc > ("gpiolib: Fix missing updates of bitmap index"), introduced a new mode > of processing bitmaps where bits applicable for fast bitmap processing > path are supposed to be skipped while iterating bits which don't apply. > Unfortunately, find_next_zero_bit() function supposed to skip over > those fast bits is always called with a 'start' argument equal to an > index of last zero bit found and returns that index value again an > again, causing an infinite loop. > > Fix it by incrementing the index uncoditionally before > find_next_zero_bit() is optionally called. > > Reported-by: Marek Szyprowski <m.szyprowski@samsung.com> > Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com> Tested-by: Marek Szyprowski <m.szyprowski@samsung.com> > --- > Marek, > > Could you please test it on top of next-20180920 with "gpiolib: Fix > missing updates of bitmap index" and optionally "mmc: pwrseq_simple: > Fix incorrect handling of GPIO bitmap" also applied? This patch finally fixes the boot issue on Samsung Chromebook Snow. Thanks! > > Thanks, > Janusz > > > drivers/gpio/gpiolib.c | 9 +++------ > 1 file changed, 3 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c > index 6ae13e3e05f1..940b543e966d 100644 > --- a/drivers/gpio/gpiolib.c > +++ b/drivers/gpio/gpiolib.c > @@ -2878,12 +2878,11 @@ int gpiod_get_array_value_complex(bool raw, bool can_sleep, > int hwgpio = gpio_chip_hwgpio(desc); > > __set_bit(hwgpio, mask); > + i++; > > if (array_info) > i = find_next_zero_bit(array_info->get_mask, > array_size, i); > - else > - i++; > } while ((i < array_size) && > (desc_array[i]->gdev->chip == chip)); > > @@ -2903,12 +2902,11 @@ int gpiod_get_array_value_complex(bool raw, bool can_sleep, > value = !value; > __assign_bit(j, value_bitmap, value); > trace_gpio_value(desc_to_gpio(desc), 1, value); > + j++; > > if (array_info) > j = find_next_zero_bit(array_info->get_mask, i, > j); > - else > - j++; > } > > if (mask != fastpath) > @@ -3191,12 +3189,11 @@ int gpiod_set_array_value_complex(bool raw, bool can_sleep, > __clear_bit(hwgpio, bits); > count++; > } > + i++; > > if (array_info) > i = find_next_zero_bit(array_info->set_mask, > array_size, i); > - else > - i++; > } while ((i < array_size) && > (desc_array[i]->gdev->chip == chip)); > /* push collected bits to outputs */ Best regards
On Sat, Sep 29, 2018 at 2:19 PM Janusz Krzysztofik <jmkrzyszt@gmail.com> wrote: > Commit b17566a6b08b ("gpiolib: Implement fast processing path in > get/set array"), already fixed to some extent with commit 5d581d7e8cdc > ("gpiolib: Fix missing updates of bitmap index"), introduced a new mode > of processing bitmaps where bits applicable for fast bitmap processing > path are supposed to be skipped while iterating bits which don't apply. > Unfortunately, find_next_zero_bit() function supposed to skip over > those fast bits is always called with a 'start' argument equal to an > index of last zero bit found and returns that index value again an > again, causing an infinite loop. > > Fix it by incrementing the index uncoditionally before > find_next_zero_bit() is optionally called. > > Reported-by: Marek Szyprowski <m.szyprowski@samsung.com> > Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com> Patch applied with Marek's Tested-by. Thanks to both of you for digging in and fixing this up! Now we are in good shape for the v4.20 cycle :) Yours, Linus Walleij
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index 6ae13e3e05f1..940b543e966d 100644 --- a/drivers/gpio/gpiolib.c +++ b/drivers/gpio/gpiolib.c @@ -2878,12 +2878,11 @@ int gpiod_get_array_value_complex(bool raw, bool can_sleep, int hwgpio = gpio_chip_hwgpio(desc); __set_bit(hwgpio, mask); + i++; if (array_info) i = find_next_zero_bit(array_info->get_mask, array_size, i); - else - i++; } while ((i < array_size) && (desc_array[i]->gdev->chip == chip)); @@ -2903,12 +2902,11 @@ int gpiod_get_array_value_complex(bool raw, bool can_sleep, value = !value; __assign_bit(j, value_bitmap, value); trace_gpio_value(desc_to_gpio(desc), 1, value); + j++; if (array_info) j = find_next_zero_bit(array_info->get_mask, i, j); - else - j++; } if (mask != fastpath) @@ -3191,12 +3189,11 @@ int gpiod_set_array_value_complex(bool raw, bool can_sleep, __clear_bit(hwgpio, bits); count++; } + i++; if (array_info) i = find_next_zero_bit(array_info->set_mask, array_size, i); - else - i++; } while ((i < array_size) && (desc_array[i]->gdev->chip == chip)); /* push collected bits to outputs */
Commit b17566a6b08b ("gpiolib: Implement fast processing path in get/set array"), already fixed to some extent with commit 5d581d7e8cdc ("gpiolib: Fix missing updates of bitmap index"), introduced a new mode of processing bitmaps where bits applicable for fast bitmap processing path are supposed to be skipped while iterating bits which don't apply. Unfortunately, find_next_zero_bit() function supposed to skip over those fast bits is always called with a 'start' argument equal to an index of last zero bit found and returns that index value again an again, causing an infinite loop. Fix it by incrementing the index uncoditionally before find_next_zero_bit() is optionally called. Reported-by: Marek Szyprowski <m.szyprowski@samsung.com> Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com> --- Marek, Could you please test it on top of next-20180920 with "gpiolib: Fix missing updates of bitmap index" and optionally "mmc: pwrseq_simple: Fix incorrect handling of GPIO bitmap" also applied? Thanks, Janusz drivers/gpio/gpiolib.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-)