2014-09-16 12:52:33

by Loic Poulain

Subject: [PATCH] net: rfkill: gpio: Fix clock status

Clock is disabled when the device is blocked.
So, clock_enabled is the logical negation of "blocked".

Signed-off-by: Loic Poulain <[email protected]>
---
net/rfkill/rfkill-gpio.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/rfkill/rfkill-gpio.c b/net/rfkill/rfkill-gpio.c
index 14c98e4..408e51f 100644
--- a/net/rfkill/rfkill-gpio.c
+++ b/net/rfkill/rfkill-gpio.c
@@ -54,7 +54,7 @@ static int rfkill_gpio_set_power(void *data, bool blocked)
if (blocked && !IS_ERR(rfkill->clk) && rfkill->clk_enabled)
clk_disable(rfkill->clk);

- rfkill->clk_enabled = blocked;
+ rfkill->clk_enabled = !blocked;

return 0;
}
--
1.8.3.2

2014-09-17 15:15:12

by John W. Linville

Subject: Re: [PATCH] net: rfkill: gpio: Fix clock status

On Tue, Sep 16, 2014 at 02:53:58PM +0200, Loic Poulain wrote:
> Clock is disabled when the device is blocked.
> So, clock_enabled is the logical negation of "blocked".
>
> Signed-off-by: Loic Poulain <[email protected]>
> ---
> net/rfkill/rfkill-gpio.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/rfkill/rfkill-gpio.c b/net/rfkill/rfkill-gpio.c
> index 14c98e4..408e51f 100644
> --- a/net/rfkill/rfkill-gpio.c
> +++ b/net/rfkill/rfkill-gpio.c
> @@ -54,7 +54,7 @@ static int rfkill_gpio_set_power(void *data, bool blocked)
> if (blocked && !IS_ERR(rfkill->clk) && rfkill->clk_enabled)
> clk_disable(rfkill->clk);
>
> - rfkill->clk_enabled = blocked;
> + rfkill->clk_enabled = !blocked;
>
> return 0;
> }

This looks like the right fix, but...the code has been that way for
a long time. If this patch is correct, how has this gone undetected
for so long?

John
--
John W. Linville Someday the world will need a hero, and you
[email protected] might be all we have. Be ready.

2014-09-17 15:33:55

by Loic Poulain

Subject: Re: [PATCH] net: rfkill: gpio: Fix clock status

If rfkill block/unblock are balanced, the following conditions are never
true:
- if (!blocked && !rfkill->clk_enabled)
- if (blocked && rfkill->clk_enabled)

Then, clock is neither disabled nor enabled.
If clock is enabled by default, it does not cause any obvious issue.

Or maybe there's not much device with clock resource.

Regards,
Loic

On 17/09/2014 17:11, John W. Linville wrote:
> On Tue, Sep 16, 2014 at 02:53:58PM +0200, Loic Poulain wrote:
>> Clock is disabled when the device is blocked.
>> So, clock_enabled is the logical negation of "blocked".
>>
>> Signed-off-by: Loic Poulain <[email protected]>
>> ---
>> net/rfkill/rfkill-gpio.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/net/rfkill/rfkill-gpio.c b/net/rfkill/rfkill-gpio.c
>> index 14c98e4..408e51f 100644
>> --- a/net/rfkill/rfkill-gpio.c
>> +++ b/net/rfkill/rfkill-gpio.c
>> @@ -54,7 +54,7 @@ static int rfkill_gpio_set_power(void *data, bool blocked)
>> if (blocked && !IS_ERR(rfkill->clk) && rfkill->clk_enabled)
>> clk_disable(rfkill->clk);
>>
>> - rfkill->clk_enabled = blocked;
>> + rfkill->clk_enabled = !blocked;
>>
>> return 0;
>> }
> This looks like the right fix, but...the code has been that way for
> a long time. If this patch is correct, how has this gone undetected
> for so long?
>
> John

--
Intel Open Source Technology Center
http://oss.intel.com/