[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <KL1PR01MB54482B74FB1AC599EBDB5D1AE6679@KL1PR01MB5448.apcprd01.prod.exchangelabs.com>
Date: Mon, 24 Apr 2023 22:21:54 +0800
From: Yan Wang <rk.code@...look.com>
To: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
Cc: linus.walleij@...aro.org, brgl@...ev.pl,
"open list:GPIO SUBSYSTEM" <linux-gpio@...r.kernel.org>,
open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v1] gpio: mmio: restroe get multiple gpio mask
> On Apr 24, 2023, at 21:31, Andy Shevchenko <andriy.shevchenko@...ux.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
>
>
Powered by blists - more mailing lists