2011-05-17 11:11:59

by Felix Fietkau

[permalink] [raw]
Subject: [PATCH] ath9k: implement .tx_last_beacon()

Signed-off-by: Felix Fietkau <[email protected]>
---
drivers/net/wireless/ath/ath9k/ath9k.h | 3 ++
drivers/net/wireless/ath/ath9k/beacon.c | 12 +++++++++
drivers/net/wireless/ath/ath9k/main.c | 38 +++++++++++++++++++++++++++++++
3 files changed, 53 insertions(+), 0 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h b/drivers/net/wireless/ath/ath9k/ath9k.h
index 03b37d7..d6e7825 100644
--- a/drivers/net/wireless/ath/ath9k/ath9k.h
+++ b/drivers/net/wireless/ath/ath9k/ath9k.h
@@ -397,6 +397,9 @@ struct ath_beacon {
struct ath_descdma bdma;
struct ath_txq *cabq;
struct list_head bbuf;
+
+ bool tx_processed;
+ bool tx_last;
};

void ath_beacon_tasklet(unsigned long data);
diff --git a/drivers/net/wireless/ath/ath9k/beacon.c b/drivers/net/wireless/ath/ath9k/beacon.c
index 0199af0..e8756c5 100644
--- a/drivers/net/wireless/ath/ath9k/beacon.c
+++ b/drivers/net/wireless/ath/ath9k/beacon.c
@@ -18,6 +18,12 @@

#define FUDGE 2

+static void ath9k_reset_beacon_status(struct ath_softc *sc)
+{
+ sc->beacon.tx_processed = false;
+ sc->beacon.tx_last = false;
+}
+
/*
* This function will modify certain transmit queue properties depending on
* the operating mode of the station (AP or AdHoc). Parameters are AIFS
@@ -72,6 +78,8 @@ static void ath_beacon_setup(struct ath_softc *sc, struct ath_vif *avp,
struct ieee80211_supported_band *sband;
u8 rate = 0;

+ ath9k_reset_beacon_status(sc);
+
ds = bf->bf_desc;
flags = ATH9K_TXDESC_NOACK;

@@ -134,6 +142,8 @@ static struct ath_buf *ath_beacon_generate(struct ieee80211_hw *hw,
struct ieee80211_tx_info *info;
int cabq_depth;

+ ath9k_reset_beacon_status(sc);
+
avp = (void *)vif->drv_priv;
cabq = sc->beacon.cabq;

@@ -644,6 +654,8 @@ static void ath_beacon_config_adhoc(struct ath_softc *sc,
struct ath_common *common = ath9k_hw_common(ah);
u32 tsf, delta, intval, nexttbtt;

+ ath9k_reset_beacon_status(sc);
+
tsf = ath9k_hw_gettsf32(ah) + TU_TO_USEC(FUDGE);
intval = TU_TO_USEC(conf->beacon_interval & ATH9K_BEACON_PERIOD);

diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
index 17ebdf1..576b87d 100644
--- a/drivers/net/wireless/ath/ath9k/main.c
+++ b/drivers/net/wireless/ath/ath9k/main.c
@@ -2332,6 +2332,43 @@ static bool ath9k_tx_frames_pending(struct ieee80211_hw *hw)
return false;
}

+int ath9k_tx_last_beacon(struct ieee80211_hw *hw)
+{
+ struct ath_softc *sc = hw->priv;
+ struct ath_hw *ah = sc->sc_ah;
+ struct ieee80211_vif *vif;
+ struct ath_vif *avp;
+ struct ath_buf *bf;
+ struct ath_tx_status ts;
+ int status;
+
+ if (ah->opmode != NL80211_IFTYPE_ADHOC)
+ return 0;
+
+ vif = sc->beacon.bslot[0];
+ if (!vif)
+ return 0;
+
+ avp = (void *)vif->drv_priv;
+ if (!avp->is_bslot_active)
+ return 0;
+
+ if (!sc->beacon.tx_processed) {
+ bf = avp->av_bcbuf;
+ if (!bf || !bf->bf_mpdu)
+ return 0;
+
+ status = ath9k_hw_txprocdesc(ah, bf->bf_desc, &ts);
+ if (status == -EINPROGRESS)
+ return 0;
+
+ sc->beacon.tx_processed = true;
+ sc->beacon.tx_last = !(ts.ts_status & ATH9K_TXERR_FILT);
+ }
+
+ return sc->beacon.tx_last;
+}
+
struct ieee80211_ops ath9k_ops = {
.tx = ath9k_tx,
.start = ath9k_start,
@@ -2356,4 +2393,5 @@ struct ieee80211_ops ath9k_ops = {
.set_coverage_class = ath9k_set_coverage_class,
.flush = ath9k_flush,
.tx_frames_pending = ath9k_tx_frames_pending,
+ .tx_last_beacon = ath9k_tx_last_beacon,
};
--
1.7.3.2



2011-05-17 13:00:31

by Rajkumar Manoharan

[permalink] [raw]
Subject: Re: [PATCH] ath9k: implement .tx_last_beacon()

On Tue, May 17, 2011 at 04:41:51PM +0530, Felix Fietkau wrote:
> Signed-off-by: Felix Fietkau <[email protected]>
> ---
> drivers/net/wireless/ath/ath9k/ath9k.h | 3 ++
> drivers/net/wireless/ath/ath9k/beacon.c | 12 +++++++++
> drivers/net/wireless/ath/ath9k/main.c | 38 +++++++++++++++++++++++++++++++
> 3 files changed, 53 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h b/drivers/net/wireless/ath/ath9k/ath9k.h
> index 03b37d7..d6e7825 100644
> --- a/drivers/net/wireless/ath/ath9k/ath9k.h
> +++ b/drivers/net/wireless/ath/ath9k/ath9k.h
> @@ -397,6 +397,9 @@ struct ath_beacon {
> struct ath_descdma bdma;
> struct ath_txq *cabq;
> struct list_head bbuf;
> +
> + bool tx_processed;
> + bool tx_last;
> };
>
> void ath_beacon_tasklet(unsigned long data);
> diff --git a/drivers/net/wireless/ath/ath9k/beacon.c b/drivers/net/wireless/ath/ath9k/beacon.c
> index 0199af0..e8756c5 100644
> --- a/drivers/net/wireless/ath/ath9k/beacon.c
> +++ b/drivers/net/wireless/ath/ath9k/beacon.c
> @@ -18,6 +18,12 @@
>
> #define FUDGE 2
>
> +static void ath9k_reset_beacon_status(struct ath_softc *sc)
> +{
> + sc->beacon.tx_processed = false;
> + sc->beacon.tx_last = false;
> +}
> +
> /*
> * This function will modify certain transmit queue properties depending on
> * the operating mode of the station (AP or AdHoc). Parameters are AIFS
> @@ -72,6 +78,8 @@ static void ath_beacon_setup(struct ath_softc *sc, struct ath_vif *avp,
> struct ieee80211_supported_band *sband;
> u8 rate = 0;
>
> + ath9k_reset_beacon_status(sc);
> +
> ds = bf->bf_desc;
> flags = ATH9K_TXDESC_NOACK;
>
> @@ -134,6 +142,8 @@ static struct ath_buf *ath_beacon_generate(struct ieee80211_hw *hw,
> struct ieee80211_tx_info *info;
> int cabq_depth;
>
> + ath9k_reset_beacon_status(sc);
> +
> avp = (void *)vif->drv_priv;
> cabq = sc->beacon.cabq;
>
> @@ -644,6 +654,8 @@ static void ath_beacon_config_adhoc(struct ath_softc *sc,
> struct ath_common *common = ath9k_hw_common(ah);
> u32 tsf, delta, intval, nexttbtt;
>
> + ath9k_reset_beacon_status(sc);
> +
> tsf = ath9k_hw_gettsf32(ah) + TU_TO_USEC(FUDGE);
> intval = TU_TO_USEC(conf->beacon_interval & ATH9K_BEACON_PERIOD);
>
> diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
> index 17ebdf1..576b87d 100644
> --- a/drivers/net/wireless/ath/ath9k/main.c
> +++ b/drivers/net/wireless/ath/ath9k/main.c
> @@ -2332,6 +2332,43 @@ static bool ath9k_tx_frames_pending(struct ieee80211_hw *hw)
> return false;
> }
>
> +int ath9k_tx_last_beacon(struct ieee80211_hw *hw)
> +{
> + struct ath_softc *sc = hw->priv;
> + struct ath_hw *ah = sc->sc_ah;
> + struct ieee80211_vif *vif;
> + struct ath_vif *avp;
> + struct ath_buf *bf;
> + struct ath_tx_status ts;
> + int status;
> +
> + if (ah->opmode != NL80211_IFTYPE_ADHOC)
> + return 0;
> +
opmode check is not needed. tx_last_beacon is called only on ibss mode.

> + vif = sc->beacon.bslot[0];
> + if (!vif)
> + return 0;
> +

> + avp = (void *)vif->drv_priv;
> + if (!avp->is_bslot_active)
> + return 0;
> +
> + if (!sc->beacon.tx_processed) {
> + bf = avp->av_bcbuf;
> + if (!bf || !bf->bf_mpdu)
> + return 0;
> +
> + status = ath9k_hw_txprocdesc(ah, bf->bf_desc, &ts);
> + if (status == -EINPROGRESS)
> + return 0;
> +
This could be racy with beacon tasklet. Process beacon descriptor
and update tx_last for every beacon in the beacon tasklet. In such case,
reset tx_processed alone in ath9k_reset_beacon_status.

> + sc->beacon.tx_processed = true;
> + sc->beacon.tx_last = !(ts.ts_status & ATH9K_TXERR_FILT);
> + }
What about other TXERR_* status?
> +
> + return sc->beacon.tx_last;
> +}
> +
--
Rajkumar

2011-05-17 13:07:22

by Felix Fietkau

[permalink] [raw]
Subject: Re: [PATCH] ath9k: implement .tx_last_beacon()

On 2011-05-17 3:00 PM, Rajkumar Manoharan wrote:
> On Tue, May 17, 2011 at 04:41:51PM +0530, Felix Fietkau wrote:
>> Signed-off-by: Felix Fietkau<[email protected]>
>> ---
>> drivers/net/wireless/ath/ath9k/ath9k.h | 3 ++
>> drivers/net/wireless/ath/ath9k/beacon.c | 12 +++++++++
>> drivers/net/wireless/ath/ath9k/main.c | 38 +++++++++++++++++++++++++++++++
>> 3 files changed, 53 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h b/drivers/net/wireless/ath/ath9k/ath9k.h
>> index 03b37d7..d6e7825 100644
>> --- a/drivers/net/wireless/ath/ath9k/ath9k.h
>> +++ b/drivers/net/wireless/ath/ath9k/ath9k.h
>> @@ -397,6 +397,9 @@ struct ath_beacon {
>> struct ath_descdma bdma;
>> struct ath_txq *cabq;
>> struct list_head bbuf;
>> +
>> + bool tx_processed;
>> + bool tx_last;
>> };
>>
>> void ath_beacon_tasklet(unsigned long data);
>> diff --git a/drivers/net/wireless/ath/ath9k/beacon.c b/drivers/net/wireless/ath/ath9k/beacon.c
>> index 0199af0..e8756c5 100644
>> --- a/drivers/net/wireless/ath/ath9k/beacon.c
>> +++ b/drivers/net/wireless/ath/ath9k/beacon.c
>> @@ -18,6 +18,12 @@
>>
>> #define FUDGE 2
>>
>> +static void ath9k_reset_beacon_status(struct ath_softc *sc)
>> +{
>> + sc->beacon.tx_processed = false;
>> + sc->beacon.tx_last = false;
>> +}
>> +
>> /*
>> * This function will modify certain transmit queue properties depending on
>> * the operating mode of the station (AP or AdHoc). Parameters are AIFS
>> @@ -72,6 +78,8 @@ static void ath_beacon_setup(struct ath_softc *sc, struct ath_vif *avp,
>> struct ieee80211_supported_band *sband;
>> u8 rate = 0;
>>
>> + ath9k_reset_beacon_status(sc);
>> +
>> ds = bf->bf_desc;
>> flags = ATH9K_TXDESC_NOACK;
>>
>> @@ -134,6 +142,8 @@ static struct ath_buf *ath_beacon_generate(struct ieee80211_hw *hw,
>> struct ieee80211_tx_info *info;
>> int cabq_depth;
>>
>> + ath9k_reset_beacon_status(sc);
>> +
>> avp = (void *)vif->drv_priv;
>> cabq = sc->beacon.cabq;
>>
>> @@ -644,6 +654,8 @@ static void ath_beacon_config_adhoc(struct ath_softc *sc,
>> struct ath_common *common = ath9k_hw_common(ah);
>> u32 tsf, delta, intval, nexttbtt;
>>
>> + ath9k_reset_beacon_status(sc);
>> +
>> tsf = ath9k_hw_gettsf32(ah) + TU_TO_USEC(FUDGE);
>> intval = TU_TO_USEC(conf->beacon_interval& ATH9K_BEACON_PERIOD);
>>
>> diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
>> index 17ebdf1..576b87d 100644
>> --- a/drivers/net/wireless/ath/ath9k/main.c
>> +++ b/drivers/net/wireless/ath/ath9k/main.c
>> @@ -2332,6 +2332,43 @@ static bool ath9k_tx_frames_pending(struct ieee80211_hw *hw)
>> return false;
>> }
>>
>> +int ath9k_tx_last_beacon(struct ieee80211_hw *hw)
>> +{
>> + struct ath_softc *sc = hw->priv;
>> + struct ath_hw *ah = sc->sc_ah;
>> + struct ieee80211_vif *vif;
>> + struct ath_vif *avp;
>> + struct ath_buf *bf;
>> + struct ath_tx_status ts;
>> + int status;
>> +
>> + if (ah->opmode != NL80211_IFTYPE_ADHOC)
>> + return 0;
>> +
> opmode check is not needed. tx_last_beacon is called only on ibss mode.
I mostly added that because we have a patch in OpenWrt that allows
ad-hoc + AP, because I was getting a constant stream of emails asking
about this. I guess I can remove that here and move it to a local patch.

>> + vif = sc->beacon.bslot[0];
>> + if (!vif)
>> + return 0;
>> +
>
>> + avp = (void *)vif->drv_priv;
>> + if (!avp->is_bslot_active)
>> + return 0;
>> +
>> + if (!sc->beacon.tx_processed) {
>> + bf = avp->av_bcbuf;
>> + if (!bf || !bf->bf_mpdu)
>> + return 0;
>> +
>> + status = ath9k_hw_txprocdesc(ah, bf->bf_desc,&ts);
>> + if (status == -EINPROGRESS)
>> + return 0;
>> +
> This could be racy with beacon tasklet. Process beacon descriptor
> and update tx_last for every beacon in the beacon tasklet. In such case,
> reset tx_processed alone in ath9k_reset_beacon_status.
If I process it in the beacon tasklet, it's either too late or too
early, but never at the right point in time. I guess I could add some
locking instead...

>> + sc->beacon.tx_processed = true;
>> + sc->beacon.tx_last = !(ts.ts_status& ATH9K_TXERR_FILT);
>> + }
> What about other TXERR_* status?
The other ones shouldn't be relevant for beacons at all, but I guess I
could use TXERR_MASK instead.

Thanks,

- Felix