[v2] gpio: bcm281xx: Fix return value of bcm_kona_gpio_get()

Message ID 1383941945-27569-1-git-send-email-markus.mayer@linaro.org
State New
Headers show

Commit Message

Markus Mayer Nov. 8, 2013, 8:19 p.m.
We need to return the corresponding bit for a particular GPIO. This bit
contains shift not mask.

Signed-off-by: Markus Mayer <markus.mayer@linaro.org>
Reviewed-by: Tim Kryger <tim.kryger@linaro.org>
Reviewed-by: Matt Porter <matt.porter@linaro.org>
---

Changes since v1:

- Clarified commit message

 drivers/gpio/gpio-bcm-kona.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Linus Walleij Nov. 18, 2013, 9:55 a.m. | #1
On Fri, Nov 8, 2013 at 9:19 PM, Markus Mayer <markus.mayer@linaro.org> wrote:

> We need to return the corresponding bit for a particular GPIO. This bit
> contains shift not mask.
>
> Signed-off-by: Markus Mayer <markus.mayer@linaro.org>
> Reviewed-by: Tim Kryger <tim.kryger@linaro.org>
> Reviewed-by: Matt Porter <matt.porter@linaro.org>

OK...

>         /* return the specified bit status */
> -       return !!(val & bit);
> +       return (val >> bit) & 1;

What about doing it like this instead:

return !!(val & BIT(bit));

?

Yours,
Linus Walleij
Markus Mayer Nov. 18, 2013, 11:56 p.m. | #2
On 18 November 2013 01:55, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Fri, Nov 8, 2013 at 9:19 PM, Markus Mayer <markus.mayer@linaro.org> wrote:
>
>> We need to return the corresponding bit for a particular GPIO. This bit
>> contains shift not mask.
>>
>> Signed-off-by: Markus Mayer <markus.mayer@linaro.org>
>> Reviewed-by: Tim Kryger <tim.kryger@linaro.org>
>> Reviewed-by: Matt Porter <matt.porter@linaro.org>
>
> OK...
>
>>         /* return the specified bit status */
>> -       return !!(val & bit);
>> +       return (val >> bit) & 1;
>
> What about doing it like this instead:
>
> return !!(val & BIT(bit));

I'll have to run that change through some tests to ensure everything
is working as intended.

Regards,
-Markus
Linus Walleij Nov. 19, 2013, 9:44 a.m. | #3
On Tue, Nov 19, 2013 at 12:56 AM, Markus Mayer <markus.mayer@linaro.org> wrote:
> On 18 November 2013 01:55, Linus Walleij <linus.walleij@linaro.org> wrote:
>> On Fri, Nov 8, 2013 at 9:19 PM, Markus Mayer <markus.mayer@linaro.org> wrote:
>>
>>> We need to return the corresponding bit for a particular GPIO. This bit
>>> contains shift not mask.
>>>
>>> Signed-off-by: Markus Mayer <markus.mayer@linaro.org>
>>> Reviewed-by: Tim Kryger <tim.kryger@linaro.org>
>>> Reviewed-by: Matt Porter <matt.porter@linaro.org>
>>
>> OK...
>>
>>>         /* return the specified bit status */
>>> -       return !!(val & bit);
>>> +       return (val >> bit) & 1;
>>
>> What about doing it like this instead:
>>
>> return !!(val & BIT(bit));
>
> I'll have to run that change through some tests to ensure everything
> is working as intended.

OK please do, because that style is used elsewhere in the driver
so I'd like to keep it consistent.

Yours,
Linus Walleij

Patch

diff --git a/drivers/gpio/gpio-bcm-kona.c b/drivers/gpio/gpio-bcm-kona.c
index 72c927d..db473f1 100644
--- a/drivers/gpio/gpio-bcm-kona.c
+++ b/drivers/gpio/gpio-bcm-kona.c
@@ -158,7 +158,7 @@  static int bcm_kona_gpio_get(struct gpio_chip *chip, unsigned gpio)
 	spin_unlock_irqrestore(&kona_gpio->lock, flags);
 
 	/* return the specified bit status */
-	return !!(val & bit);
+	return (val >> bit) & 1;
 }
 
 static int bcm_kona_gpio_direction_input(struct gpio_chip *chip, unsigned gpio)