2011-01-09 16:52:28

by Rajkumar Manoharan

[permalink] [raw]
Subject: [PATCH] ath9k: set hw opmode while changing interface type

The commit "ath9k: Add change_interface callback" is
failed to set hw opmode while changing interface type
on runtime. Not setting opmode fails to generate
beacons on changing to AP mode.

Cc: [email protected]
Cc: Jouni Malinen <[email protected]>
Signed-off-by: Rajkumar Manoharan <[email protected]>
---
drivers/net/wireless/ath/ath9k/beacon.c | 1 +
drivers/net/wireless/ath/ath9k/main.c | 4 ++++
2 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/beacon.c b/drivers/net/wireless/ath/ath9k/beacon.c
index 385ba03..efa573b 100644
--- a/drivers/net/wireless/ath/ath9k/beacon.c
+++ b/drivers/net/wireless/ath/ath9k/beacon.c
@@ -338,6 +338,7 @@ void ath_beacon_return(struct ath_softc *sc, struct ath_vif *avp)
sc->beacon.bslot[avp->av_bslot] = NULL;
sc->beacon.bslot_aphy[avp->av_bslot] = NULL;
sc->nbcnvifs--;
+ avp->av_bslot = -1;
}

bf = avp->av_bcbuf;
diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
index f90a6ca..648a761 100644
--- a/drivers/net/wireless/ath/ath9k/main.c
+++ b/drivers/net/wireless/ath/ath9k/main.c
@@ -1456,6 +1456,8 @@ static int ath9k_change_interface(struct ieee80211_hw *hw,
{
struct ath_wiphy *aphy = hw->priv;
struct ath_softc *sc = aphy->sc;
+ struct ath_hw *ah = sc->sc_ah;
+ struct ath_vif *avp = (void *)vif->drv_priv;
struct ath_common *common = ath9k_hw_common(sc->sc_ah);
int ret = 0;

@@ -1470,6 +1472,7 @@ static int ath9k_change_interface(struct ieee80211_hw *hw,
ret = -ENOBUFS;
goto out;
}
+ sc->sc_flags |= SC_OP_TSF_RESET;
break;
case NL80211_IFTYPE_STATION:
/* Stop ANI */
@@ -1488,6 +1491,7 @@ static int ath9k_change_interface(struct ieee80211_hw *hw,
vif->type = new_type;
vif->p2p = p2p;

+ ah->opmode = avp->av_opmode = new_type;
out:
mutex_unlock(&sc->mutex);
return ret;
--
1.7.3.5



2011-01-12 14:21:01

by Rajkumar Manoharan

[permalink] [raw]
Subject: Re: [PATCH] ath9k: set hw opmode while changing interface type

On Sun, Jan 09, 2011 at 11:38:21PM +0530, Felix Fietkau wrote:
> On 2011-01-09 9:51 AM, Rajkumar Manoharan wrote:
> > The commit "ath9k: Add change_interface callback" is
> > failed to set hw opmode while changing interface type
> > on runtime. Not setting opmode fails to generate
> > beacons on changing to AP mode.
> >
> I think since not only ah->opmode matters, but also various other things
> that conditionally enable/disable ANI, various flags in ah->imask, etc.
> it would probably be best to merge most of the code of .add_interface,
> .remove_interface and .change_interface into one function that iterates
> over all active interfaces and calculates all the mode dependent
> parameters. The way things are done right now is just too fragile.

Sorry for delayed response. Will send a RFC patch. Thanks for the review.

--
Rajkumar

2011-01-09 18:08:30

by Felix Fietkau

[permalink] [raw]
Subject: Re: [PATCH] ath9k: set hw opmode while changing interface type

On 2011-01-09 9:51 AM, Rajkumar Manoharan wrote:
> The commit "ath9k: Add change_interface callback" is
> failed to set hw opmode while changing interface type
> on runtime. Not setting opmode fails to generate
> beacons on changing to AP mode.
>
> Cc: [email protected]
> Cc: Jouni Malinen<[email protected]>
> Signed-off-by: Rajkumar Manoharan<[email protected]>
> ---
> drivers/net/wireless/ath/ath9k/beacon.c | 1 +
> drivers/net/wireless/ath/ath9k/main.c | 4 ++++
> 2 files changed, 5 insertions(+), 0 deletions(-)
>
I'm really not convinced that this is enough. How about implementing
proper code for recalculating ah->opmode and doing hw resets when
necessary, instead of adding more band-aids that can only deal with
having only one interface. This patch really only fixes a few visible
parts of the problem, but in the details the ath9k_change_interface
callback still somewhat broken.

I think since not only ah->opmode matters, but also various other things
that conditionally enable/disable ANI, various flags in ah->imask, etc.
it would probably be best to merge most of the code of .add_interface,
.remove_interface and .change_interface into one function that iterates
over all active interfaces and calculates all the mode dependent
parameters. The way things are done right now is just too fragile.

- Felix