| Message ID | 1430321701-13758-2-git-send-email-vladimir_zapolskiy@mentor.com |
|---|---|
| State | New |
| Headers | show |
On Wed, Apr 29, 2015 at 5:34 PM, Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com> wrote: > This reverts commit 25b35da7f4cce82271859f1b6eabd9f3bd41a2bb. > > The original change is a fast workaround in GPIO generic driver, > which is properly fixed by Alexandre's 23600969ff centralized handling > of return values from GPIO chip drivers. > > To avoid a redundant check and copy/paste confusion, it is better to > revert the change done in a particular driver. > > Signed-off-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com> I'm gonna drop this patch, because the code is helpful like this, clear to see what is going on. In the long run I want to change the signature of the get/set functions to take bools because that is what they actually handle. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Linus, On 11.05.2015 12:46, Linus Walleij wrote: > On Wed, Apr 29, 2015 at 5:34 PM, Vladimir Zapolskiy > <vladimir_zapolskiy@mentor.com> wrote: > >> This reverts commit 25b35da7f4cce82271859f1b6eabd9f3bd41a2bb. >> >> The original change is a fast workaround in GPIO generic driver, >> which is properly fixed by Alexandre's 23600969ff centralized handling >> of return values from GPIO chip drivers. >> >> To avoid a redundant check and copy/paste confusion, it is better to >> revert the change done in a particular driver. >> >> Signed-off-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com> > > I'm gonna drop this patch, because the code is helpful like this, > clear to see what is going on. > > In the long run I want to change the signature of the get/set functions > to take bools because that is what they actually handle. thank you for review. In my opinion (and according to my tests of course) it is safe to apply this revert commit, also the commit is anyway needed, when you switch to bool return values. I understand your wish to keep this code as a hint for the planned updates in the future, if price of the dead code plus small deviation between bgpio_get() and bgpio_get_set() is less than price of the hint in your opinion, I have no objections to skip this patch. -- With best wishes, Vladimir -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Vladimir, On Mon, May 11, 2015 at 10:12 PM, Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com> wrote: > Hi Linus, > > On 11.05.2015 12:46, Linus Walleij wrote: >> On Wed, Apr 29, 2015 at 5:34 PM, Vladimir Zapolskiy >> <vladimir_zapolskiy@mentor.com> wrote: >> >>> This reverts commit 25b35da7f4cce82271859f1b6eabd9f3bd41a2bb. >>> >>> The original change is a fast workaround in GPIO generic driver, >>> which is properly fixed by Alexandre's 23600969ff centralized handling >>> of return values from GPIO chip drivers. >>> >>> To avoid a redundant check and copy/paste confusion, it is better to >>> revert the change done in a particular driver. >>> >>> Signed-off-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com> >> >> I'm gonna drop this patch, because the code is helpful like this, >> clear to see what is going on. >> >> In the long run I want to change the signature of the get/set functions >> to take bools because that is what they actually handle. > > thank you for review. > > In my opinion (and according to my tests of course) it is safe to apply > this revert commit, also the commit is anyway needed, when you switch to > bool return values. > > I understand your wish to keep this code as a hint for the planned > updates in the future, if price of the dead code plus small deviation > between bgpio_get() and bgpio_get_set() is less than price of the hint > in your opinion, I have no objections to skip this patch. I'd also agree with Linus that we could live without this patch until we switch the functions to return bools. Actually, with implicit type conversion doing most of the job, I wonder if it would be that hard to do - maybe you will want to give it a quick try? -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Alexandre, On 12.05.2015 10:45, Alexandre Courbot wrote: > Hi Vladimir, > > On Mon, May 11, 2015 at 10:12 PM, Vladimir Zapolskiy > <vladimir_zapolskiy@mentor.com> wrote: >> Hi Linus, >> >> On 11.05.2015 12:46, Linus Walleij wrote: >>> On Wed, Apr 29, 2015 at 5:34 PM, Vladimir Zapolskiy >>> <vladimir_zapolskiy@mentor.com> wrote: >>> >>>> This reverts commit 25b35da7f4cce82271859f1b6eabd9f3bd41a2bb. >>>> >>>> The original change is a fast workaround in GPIO generic driver, >>>> which is properly fixed by Alexandre's 23600969ff centralized handling >>>> of return values from GPIO chip drivers. >>>> >>>> To avoid a redundant check and copy/paste confusion, it is better to >>>> revert the change done in a particular driver. >>>> >>>> Signed-off-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com> >>> >>> I'm gonna drop this patch, because the code is helpful like this, >>> clear to see what is going on. >>> >>> In the long run I want to change the signature of the get/set functions >>> to take bools because that is what they actually handle. >> >> thank you for review. >> >> In my opinion (and according to my tests of course) it is safe to apply >> this revert commit, also the commit is anyway needed, when you switch to >> bool return values. >> >> I understand your wish to keep this code as a hint for the planned >> updates in the future, if price of the dead code plus small deviation >> between bgpio_get() and bgpio_get_set() is less than price of the hint >> in your opinion, I have no objections to skip this patch. > > I'd also agree with Linus that we could live without this patch until > we switch the functions to return bools. > > Actually, with implicit type conversion doing most of the job, I > wonder if it would be that hard to do - maybe you will want to give it > a quick try? > it should be quite a simple and straightforward change, I'll prepare patches for review, thank you for offering the task. -- With best wishes, Vladimir -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, May 12, 2015 at 9:54 AM, Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com> wrote: > On 12.05.2015 10:45, Alexandre Courbot wrote: >> Actually, with implicit type conversion doing most of the job, I >> wonder if it would be that hard to do - maybe you will want to give it >> a quick try? > > it should be quite a simple and straightforward change, I'll prepare > patches for review, thank you for offering the task. Awesome, thanks for proposing this to be done guys. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/gpio/gpio-generic.c b/drivers/gpio/gpio-generic.c index b92a690..3f90be8 100644 --- a/drivers/gpio/gpio-generic.c +++ b/drivers/gpio/gpio-generic.c @@ -139,7 +139,7 @@ static int bgpio_get(struct gpio_chip *gc, unsigned int gpio) { struct bgpio_chip *bgc = to_bgpio_chip(gc); - return !!(bgc->read_reg(bgc->reg_dat) & bgc->pin2mask(bgc, gpio)); + return bgc->read_reg(bgc->reg_dat) & bgc->pin2mask(bgc, gpio); } static void bgpio_set(struct gpio_chip *gc, unsigned int gpio, int val)
This reverts commit 25b35da7f4cce82271859f1b6eabd9f3bd41a2bb. The original change is a fast workaround in GPIO generic driver, which is properly fixed by Alexandre's 23600969ff centralized handling of return values from GPIO chip drivers. To avoid a redundant check and copy/paste confusion, it is better to revert the change done in a particular driver. Signed-off-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com> --- drivers/gpio/gpio-generic.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)