[PATCH] gpio: generic: clamp values from bgpio_get_set()

[PATCH] gpio: generic: clamp values from bgpio_get_set()

[Linus Walleij]

The bgpio_get_set() call should return a value clamped to [0,1],
the current code will return a negative value if reading
bit 31, which turns the value negative as this is a signed value
and thus gets interpreted as an error by the gpiolib core.
Found on the gpio-mxc but applies to any MMIO driver.

Fixes: b19e7f51a55f "gpio: gpio-generic: add flag to read out output value from reg_set"
Cc: kernel@pengutronix.de
Cc: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>
Reported-by: Clemens Gruber <clemens.gruber@pqgruber.com>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/gpio/gpio-generic.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpio/gpio-generic.c b/drivers/gpio/gpio-generic.c
index 72088028d7a9..ea581dc23d44 100644
--- a/drivers/gpio/gpio-generic.c
+++ b/drivers/gpio/gpio-generic.c
@@ -141,9 +141,9 @@ static int bgpio_get_set(struct gpio_chip *gc, unsigned int gpio)
 	unsigned long pinmask = bgc->pin2mask(bgc, gpio);
 
 	if (bgc->dir & pinmask)
-		return bgc->read_reg(bgc->reg_set) & pinmask;
+		return !!(bgc->read_reg(bgc->reg_set) & pinmask);
 	else
-		return bgc->read_reg(bgc->reg_dat) & pinmask;
+		return !!(bgc->read_reg(bgc->reg_dat) & pinmask);
 }
 
 static int bgpio_get(struct gpio_chip *gc, unsigned int gpio)
-- 
2.4.3

Re: [PATCH] gpio: generic: clamp values from bgpio_get_set()

[Fabio Estevam]

Hi Linus,

On Fri, Dec 11, 2015 at 7:36 PM, Linus Walleij <linus.walleij@linaro.org> wrote:
> The bgpio_get_set() call should return a value clamped to [0,1],
> the current code will return a negative value if reading
> bit 31, which turns the value negative as this is a signed value
> and thus gets interpreted as an error by the gpiolib core.
> Found on the gpio-mxc but applies to any MMIO driver.
>
> Fixes: b19e7f51a55f "gpio: gpio-generic: add flag to read out output value from reg_set"

This commit appeared in 4.2, so it would be nice to add a stable tag:

Cc: <stable@vger.kernel.org> # 4.2+

Regards,

Fabio Estevam

Re: [PATCH] gpio: generic: clamp values from bgpio_get_set()

[Linus Walleij]

On Fri, Dec 11, 2015 at 10:47 PM, Fabio Estevam <festevam@gmail.com> wrote:
> On Fri, Dec 11, 2015 at 7:36 PM, Linus Walleij <linus.walleij@linaro.org> wrote:
>> The bgpio_get_set() call should return a value clamped to [0,1],
>> the current code will return a negative value if reading
>> bit 31, which turns the value negative as this is a signed value
>> and thus gets interpreted as an error by the gpiolib core.
>> Found on the gpio-mxc but applies to any MMIO driver.
>>
>> Fixes: b19e7f51a55f "gpio: gpio-generic: add flag to read out output value from reg_set"
>
> This commit appeared in 4.2, so it would be nice to add a stable tag:
>
> Cc: <stable@vger.kernel.org> # 4.2+

IIUC Fixes: has exactly this effect: it will be applied to any kernel where
that commit is present. The beauty of git commit hashes.

Yours,
Linus Walleij

Re: [PATCH] gpio: generic: clamp values from bgpio_get_set()

[Fabio Estevam]

On Fri, Dec 11, 2015 at 8:10 PM, Linus Walleij <linus.walleij@linaro.org> wrote:

>>> Fixes: b19e7f51a55f "gpio: gpio-generic: add flag to read out output value from reg_set"
>>
>> This commit appeared in 4.2, so it would be nice to add a stable tag:
>>
>> Cc: <stable@vger.kernel.org> # 4.2+
>
> IIUC Fixes: has exactly this effect: it will be applied to any kernel where
> that commit is present. The beauty of git commit hashes.

Not sure if this has the same effect as it is not mentioned at
Documentation/stable_kernel_rules.txt

Regards,

Fabio Estevam

Re: [PATCH] gpio: generic: clamp values from bgpio_get_set()

[Linus Walleij]

On Fri, Dec 11, 2015 at 11:23 PM, Fabio Estevam <festevam@gmail.com> wrote:
> On Fri, Dec 11, 2015 at 8:10 PM, Linus Walleij <linus.walleij@linaro.org> wrote:
>
>>>> Fixes: b19e7f51a55f "gpio: gpio-generic: add flag to read out output value from reg_set"
>>>
>>> This commit appeared in 4.2, so it would be nice to add a stable tag:
>>>
>>> Cc: <stable@vger.kernel.org> # 4.2+
>>
>> IIUC Fixes: has exactly this effect: it will be applied to any kernel where
>> that commit is present. The beauty of git commit hashes.
>
> Not sure if this has the same effect as it is not mentioned at
> Documentation/stable_kernel_rules.txt

I have surely noticed it having that effect in practice, maybe it is time
to send a patch to that document.

Yours,
Linus Walleij

Re: [PATCH] gpio: generic: clamp values from bgpio_get_set()

[Greg KH]

On Tue, Dec 15, 2015 at 09:37:53AM +0100, Linus Walleij wrote:
> On Fri, Dec 11, 2015 at 11:23 PM, Fabio Estevam <festevam@gmail.com> wrote:
> > On Fri, Dec 11, 2015 at 8:10 PM, Linus Walleij <linus.walleij@linaro.org> wrote:
> >
> >>>> Fixes: b19e7f51a55f "gpio: gpio-generic: add flag to read out output value from reg_set"
> >>>
> >>> This commit appeared in 4.2, so it would be nice to add a stable tag:
> >>>
> >>> Cc: <stable@vger.kernel.org> # 4.2+
> >>
> >> IIUC Fixes: has exactly this effect: it will be applied to any kernel where
> >> that commit is present. The beauty of git commit hashes.
> >
> > Not sure if this has the same effect as it is not mentioned at
> > Documentation/stable_kernel_rules.txt
> 
> I have surely noticed it having that effect in practice, maybe it is time
> to send a patch to that document.

No, please don't.  I do pick up _some_ patches that have "Fixes:" tags
in them, if they look relevant, and I'm bored at the moment.  Otherwise,
if you want something in a stable release, put the needed
"stable@vger.kernel.org" tag in it, that way you know it will go there
as I know to look for that.

thanks,

greg k-h

Re: [PATCH] gpio: generic: clamp values from bgpio_get_set()

[Linus Walleij]

On Tue, Dec 15, 2015 at 6:00 PM, Greg KH <gregkh@linuxfoundation.org> wrote:

> No, please don't.  I do pick up _some_ patches that have "Fixes:" tags
> in them, if they look relevant, and I'm bored at the moment.  Otherwise,
> if you want something in a stable release, put the needed
> "stable@vger.kernel.org" tag in it, that way you know it will go there
> as I know to look for that.

OK! I will be a better person from this day :D

Yours,
Linus Walleij

Re: [PATCH] gpio: generic: clamp values from bgpio_get_set()

[Vladimir Zapolskiy]

On 11.12.2015 23:47, Fabio Estevam wrote:
> Hi Linus,
> 
> On Fri, Dec 11, 2015 at 7:36 PM, Linus Walleij <linus.walleij@linaro.org> wrote:
>> The bgpio_get_set() call should return a value clamped to [0,1],
>> the current code will return a negative value if reading
>> bit 31, which turns the value negative as this is a signed value
>> and thus gets interpreted as an error by the gpiolib core.
>> Found on the gpio-mxc but applies to any MMIO driver.
>>
>> Fixes: b19e7f51a55f "gpio: gpio-generic: add flag to read out output value from reg_set"
> 
> This commit appeared in 4.2, so it would be nice to add a stable tag:
> 
> Cc: <stable@vger.kernel.org> # 4.2+

As it was discussed yesterday in another thread, the specified commit is
correct, and v4.2 works good.

The problem is in commit e20538b82f1f ("gpio: Propagate errors from
chip->get()"), which is present in v4.3.

So, Fixes: tag should be removed or corrected IMHO.

--
With best wishes,
Vladimir

Re: [PATCH] gpio: generic: clamp values from bgpio_get_set()

[Linus Walleij]

On Thu, Dec 17, 2015 at 2:49 PM, Vladimir Zapolskiy
<vladimir_zapolskiy@mentor.com> wrote:
> On 11.12.2015 23:47, Fabio Estevam wrote:
>> Hi Linus,
>>
>> On Fri, Dec 11, 2015 at 7:36 PM, Linus Walleij <linus.walleij@linaro.org> wrote:
>>> The bgpio_get_set() call should return a value clamped to [0,1],
>>> the current code will return a negative value if reading
>>> bit 31, which turns the value negative as this is a signed value
>>> and thus gets interpreted as an error by the gpiolib core.
>>> Found on the gpio-mxc but applies to any MMIO driver.
>>>
>>> Fixes: b19e7f51a55f "gpio: gpio-generic: add flag to read out output value from reg_set"
>>
>> This commit appeared in 4.2, so it would be nice to add a stable tag:
>>
>> Cc: <stable@vger.kernel.org> # 4.2+
>
> As it was discussed yesterday in another thread, the specified commit is
> correct, and v4.2 works good.
>
> The problem is in commit e20538b82f1f ("gpio: Propagate errors from
> chip->get()"), which is present in v4.3.
>
> So, Fixes: tag should be removed or corrected IMHO.

OK fixed it, also tagged for stable # 4.3+

Yours,
Linus Walleij