[1/4] Revert "gpio: generic: clamp retured value to [0,1]"
diff mbox

Message ID 1430321701-13758-2-git-send-email-vladimir_zapolskiy@mentor.com
State New
Headers show

Commit Message

Vladimir Zapolskiy April 29, 2015, 3:34 p.m. UTC
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(-)

Comments

Linus Walleij May 11, 2015, 9:46 a.m. UTC | #1
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
Vladimir Zapolskiy May 11, 2015, 1:12 p.m. UTC | #2
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
Alexandre Courbot May 12, 2015, 7:45 a.m. UTC | #3
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
Vladimir Zapolskiy May 12, 2015, 7:54 a.m. UTC | #4
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
Linus Walleij May 12, 2015, 11:52 a.m. UTC | #5
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

Patch
diff mbox

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)