2014-01-10 20:21:24

by Alex Gal

[permalink] [raw]
Subject: [PATCH v5] wl12xx: fix tx power setting

The driver ignores BSS_CHANGED_TXPOWER changes.
Fix this by calling ACX_TX_POWER when appropriate.

Signed-off-by: Alex Gal <[email protected]>
---
drivers/net/wireless/ti/wlcore/main.c | 10 ++++++++++
1 file changed, 10 insertions(+)

diff --git a/drivers/net/wireless/ti/wlcore/main.c b/drivers/net/wireless/ti/wlcore/main.c
index e9da47c..89d2310 100644
--- a/drivers/net/wireless/ti/wlcore/main.c
+++ b/drivers/net/wireless/ti/wlcore/main.c
@@ -4457,6 +4457,16 @@ static void wl1271_op_bss_info_changed(struct ieee80211_hw *hw,
if (ret < 0)
goto out;

+ if ((changed & BSS_CHANGED_TXPOWER) &&
+ bss_conf->txpower != wlvif->power_level) {
+
+ ret = wl1271_acx_tx_power(wl, wlvif, bss_conf->txpower);
+ if (ret < 0)
+ goto out;
+
+ wlvif->power_level = bss_conf->txpower;
+ }
+
if (is_ap)
wl1271_bss_info_changed_ap(wl, vif, bss_conf, changed);
else
--
1.8.5.2



2014-01-13 13:35:47

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH v5] wl12xx: fix tx power setting

On Fri, 2014-01-10 at 15:21 -0500, Alex Gal wrote:
> The driver ignores BSS_CHANGED_TXPOWER changes.
> Fix this by calling ACX_TX_POWER when appropriate.

This doesn't really make sense since the driver handles
IEEE80211_CONF_CHANGE_POWER. If there's something wrong with that, maybe
that should be fixed?

You should be able to see this with the debugging stuff.

johannes


2014-01-13 19:07:59

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH v5] wl12xx: fix tx power setting

On Mon, 2014-01-13 at 14:04 -0500, Alex Gal wrote:
> > Oh, really? Ok. I didn't think that driver supported channel contexts.
> > In that case your patch is right but incomplete - you should remove the
> > CONF_CHANGE_POWER handling.
>
> I think wlcore is used by both wl12xx and wl18xx.
>
> It looks like wl18xx supports multiple channels and perhaps channel
> contexts (?) while the wl12xx does not.

So maybe you would still need both.

johannes


2014-01-13 17:23:32

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH v5] wl12xx: fix tx power setting

On Mon, 2014-01-13 at 12:14 -0500, Alex Gal wrote:

> > This doesn't really make sense since the driver handles
> > IEEE80211_CONF_CHANGE_POWER. If there's something wrong with that, maybe
> > that should be fixed?

> The IEEE80211_CONF_CHANGE_POWER never reaches the driver since in
> /net/mac80211/main.c:ieee80211_hw_config:
>
> if (!local->use_chanctx)
> changed |= ieee80211_hw_conf_chan(local);
> else
> changed &= ~(IEEE80211_CONF_CHANGE_CHANNEL |
> IEEE80211_CONF_CHANGE_POWER);
>
> only the else statement is executed.

Oh, really? Ok. I didn't think that driver supported channel contexts.
In that case your patch is right but incomplete - you should remove the
CONF_CHANGE_POWER handling.

johannes


2014-01-13 19:04:53

by Alex Gal

[permalink] [raw]
Subject: Re: [PATCH v5] wl12xx: fix tx power setting

> Oh, really? Ok. I didn't think that driver supported channel contexts.
> In that case your patch is right but incomplete - you should remove the
> CONF_CHANGE_POWER handling.

I think wlcore is used by both wl12xx and wl18xx.

It looks like wl18xx supports multiple channels and perhaps channel
contexts (?) while the wl12xx does not.

Luca, who is no longer the driver maintainer, might shed some light on this :)

I will recreate the patch afterwards by removing the CONF_CHANGE_POWER handling.

Thank you

2014-01-13 17:14:32

by Alex Gal

[permalink] [raw]
Subject: Re: [PATCH v5] wl12xx: fix tx power setting

Hello,

On Mon, Jan 13, 2014 at 8:35 AM, Johannes Berg
<[email protected]> wrote:
> On Fri, 2014-01-10 at 15:21 -0500, Alex Gal wrote:
>> The driver ignores BSS_CHANGED_TXPOWER changes.
>> Fix this by calling ACX_TX_POWER when appropriate.
>
> This doesn't really make sense since the driver handles
> IEEE80211_CONF_CHANGE_POWER. If there's something wrong with that, maybe
> that should be fixed?
>
> You should be able to see this with the debugging stuff.

I tried debugging a bit more.

The IEEE80211_CONF_CHANGE_POWER never reaches the driver since in
/net/mac80211/main.c:ieee80211_hw_config:

if (!local->use_chanctx)
changed |= ieee80211_hw_conf_chan(local);
else
changed &= ~(IEEE80211_CONF_CHANGE_CHANNEL |
IEEE80211_CONF_CHANGE_POWER);

only the else statement is executed.

However, the BSS_CHANGED_TXPOWER gets sent to the driver so I thought,
incorrectly,
that would be the the fix.

This behavior happens in kernel 3.10 but I suspect the same to happen
in mainline, unfortunately, I cannot test it.

Please let me know how I can debug this further since I am not
familiar with this code.

Cheers,

Alex