2016-11-15 17:54:32

by Matthias Schiffer

[permalink] [raw]
Subject: [PATCH] ath9k: fix ath9k_hw_gpio_get() to return 0 or 1 on success

Commit b2d70d4944c1 ("ath9k: make GPIO API to support both of WMAC and
SOC") refactored ath9k_hw_gpio_get() to support both WMAC and SOC GPIOs,
changing the return on success from 1 to BIT(gpio). This broke some callers
like ath_is_rfkill_set().

Instead of fixing all callers, change ath9k_hw_gpio_get() back to only
return 0 or 1.

Fixes: b2d70d4944c1 ("ath9k: make GPIO API to support both of WMAC and SOC")
Cc: <[email protected]> # v4.7+
Signed-off-by: Matthias Schiffer <[email protected]>
---
drivers/net/wireless/ath/ath9k/hw.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath9k/hw.c b/drivers/net/wireless/ath/ath9k/hw.c
index 14b13f0..a35f78b 100644
--- a/drivers/net/wireless/ath/ath9k/hw.c
+++ b/drivers/net/wireless/ath/ath9k/hw.c
@@ -2792,7 +2792,7 @@ u32 ath9k_hw_gpio_get(struct ath_hw *ah, u32 gpio)
WARN_ON(1);
}

- return val;
+ return !!val;
}
EXPORT_SYMBOL(ath9k_hw_gpio_get);

--
2.10.2


2016-11-16 15:59:27

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH] ath9k: fix ath9k_hw_gpio_get() to return 0 or 1 on success

Matthias Schiffer <[email protected]> writes:

> On 11/16/2016 03:08 PM, Kalle Valo wrote:
>> Matthias Schiffer <[email protected]> writes:
>>
>>> Commit b2d70d4944c1 ("ath9k: make GPIO API to support both of WMAC and
>>> SOC") refactored ath9k_hw_gpio_get() to support both WMAC and SOC GPIOs,
>>> changing the return on success from 1 to BIT(gpio). This broke some callers
>>> like ath_is_rfkill_set().
>>>
>>> Instead of fixing all callers, change ath9k_hw_gpio_get() back to only
>>> return 0 or 1.
>>>
>>> Fixes: b2d70d4944c1 ("ath9k: make GPIO API to support both of WMAC and SOC")
>>> Cc: <[email protected]> # v4.7+
>>> Signed-off-by: Matthias Schiffer <[email protected]>
>>
>> Can you describe more about the symptoms, how did this break from user's
>> point of view? I can add that to the commit log.
>>
>
> I did actually not experience an issue myself with a mainline kernel.
>
> Looking at the functions ath_is_rfkill_set() and ath9k_rfkill_poll_state()
> in gpio.c, this issue causes wiphy_rfkill_set_hw_state() always to be
> passed false when ah->rfkill_polarity==1, breaking rfkill. I don't know how
> common devices with ah->rfkill_polarity==1 are.
>
> I became aware of this issue when rebasing an out-of-tree patch in LEDE
> which uses the WMAC GPIOs to configure some kind of bandpass filter found
> in Ubiquiti hardware. (I hope to find time to get this patch upsteam at
> some point...)

Thanks, this kind of information is good to have in the commit log. That
way maintainers, like me or stable tree folks, can prioritise the fix to
various releases. And I guess Linux distros also like to know this as
well as some of the normal users when trying to solve problems.

I'll mention in the commit log that this doesn't fix any known bug. And
I'll push this to 4.10, I don't think this is important enough for 4.9.

--
Kalle Valo

2016-11-16 14:09:04

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH] ath9k: fix ath9k_hw_gpio_get() to return 0 or 1 on success

Matthias Schiffer <[email protected]> writes:

> Commit b2d70d4944c1 ("ath9k: make GPIO API to support both of WMAC and
> SOC") refactored ath9k_hw_gpio_get() to support both WMAC and SOC GPIOs,
> changing the return on success from 1 to BIT(gpio). This broke some callers
> like ath_is_rfkill_set().
>
> Instead of fixing all callers, change ath9k_hw_gpio_get() back to only
> return 0 or 1.
>
> Fixes: b2d70d4944c1 ("ath9k: make GPIO API to support both of WMAC and SOC")
> Cc: <[email protected]> # v4.7+
> Signed-off-by: Matthias Schiffer <[email protected]>

Can you describe more about the symptoms, how did this break from user's
point of view? I can add that to the commit log.

--
Kalle Valo

2016-11-23 13:59:42

by Kalle Valo

[permalink] [raw]
Subject: Re: ath9k: fix ath9k_hw_gpio_get() to return 0 or 1 on success

Matthias Schiffer <[email protected]> wrote:
> Commit b2d70d4944c1 ("ath9k: make GPIO API to support both of WMAC and
> SOC") refactored ath9k_hw_gpio_get() to support both WMAC and SOC GPIOs,
> changing the return on success from 1 to BIT(gpio). This broke some callers
> like ath_is_rfkill_set().
>
> Instead of fixing all callers, change ath9k_hw_gpio_get() back to only
> return 0 or 1.
>
> Fixes: b2d70d4944c1 ("ath9k: make GPIO API to support both of WMAC and SOC")
> Cc: <[email protected]> # v4.7+
> Signed-off-by: Matthias Schiffer <[email protected]>

Patch applied to ath-next branch of ath.git, thanks.

91851cc7a939 ath9k: fix ath9k_hw_gpio_get() to return 0 or 1 on success

--
https://patchwork.kernel.org/patch/9430247/

Documentation about submitting wireless patches and checking status
from patchwork:

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

2016-11-16 14:40:07

by Matthias Schiffer

[permalink] [raw]
Subject: Re: [PATCH] ath9k: fix ath9k_hw_gpio_get() to return 0 or 1 on success

On 11/16/2016 03:08 PM, Kalle Valo wrote:
> Matthias Schiffer <[email protected]> writes:
>
>> Commit b2d70d4944c1 ("ath9k: make GPIO API to support both of WMAC and
>> SOC") refactored ath9k_hw_gpio_get() to support both WMAC and SOC GPIOs,
>> changing the return on success from 1 to BIT(gpio). This broke some callers
>> like ath_is_rfkill_set().
>>
>> Instead of fixing all callers, change ath9k_hw_gpio_get() back to only
>> return 0 or 1.
>>
>> Fixes: b2d70d4944c1 ("ath9k: make GPIO API to support both of WMAC and SOC")
>> Cc: <[email protected]> # v4.7+
>> Signed-off-by: Matthias Schiffer <[email protected]>
>
> Can you describe more about the symptoms, how did this break from user's
> point of view? I can add that to the commit log.
>

I did actually not experience an issue myself with a mainline kernel.

Looking at the functions ath_is_rfkill_set() and ath9k_rfkill_poll_state()
in gpio.c, this issue causes wiphy_rfkill_set_hw_state() always to be
passed false when ah->rfkill_polarity==1, breaking rfkill. I don't know how
common devices with ah->rfkill_polarity==1 are.

I became aware of this issue when rebasing an out-of-tree patch in LEDE
which uses the WMAC GPIOs to configure some kind of bandpass filter found
in Ubiquiti hardware. (I hope to find time to get this patch upsteam at
some point...)

Matthias


Attachments:
signature.asc (801.00 B)
OpenPGP digital signature