[ath9k-devel] [PATCH 0/1] ath9k: from TODO: tackle merging of ath9k_txq_info and ath9k_tx_queue_info structures

[ath9k-devel] [PATCH 0/1] ath9k: from TODO: tackle merging of ath9k_txq_info and ath9k_tx_queue_info structures

[Alistair John Strachan]

Luis posted a TODO list for the ath9k driver to the ath9k-devel list. This
patch addresses one of the "Cleanup" TODO items, namely the merging of
the ath9k_txq_info and ath9k_tx_queue_info structures.

Please apply.

 drivers/net/wireless/ath9k/ath9k.h  |   23 +++--------------------
 drivers/net/wireless/ath9k/beacon.c |    4 ++--
 drivers/net/wireless/ath9k/core.h   |    4 ++--
 drivers/net/wireless/ath9k/hw.c     |   12 ++++++------
 drivers/net/wireless/ath9k/main.c   |    2 +-
 drivers/net/wireless/ath9k/xmit.c   |   10 +++++-----
 6 files changed, 19 insertions(+), 36 deletions(-)

--
Cheers,
Alistair.

[ath9k-devel] [PATCH 1/1] ath9k: merge ath9k_txq_info and ath9k_tx_queue_info structures

[Alistair John Strachan]

Merge struct ath9k_txq_info into struct ath9k_tx_queue_info. This
change was mostly mechanical with the exception of the field tqi_compBuf
which is renamed as tqi_physCompBuf and fixed up where it was used.

Merging the structures the other way would also have been possible,
but the ath9k_tx_queue_info name is more consistent with other similarly
named structures declared above.

Patch is compile time and runtime tested.

Changes-licensed-under: ISC
Signed-off-by: Alistair John Strachan <alistair@devzero.co.uk>
Cc: Luis R. Rodriguez <lrodriguez@atheros.com>
Cc: Jouni Malinen <jouni.malinen@atheros.com>

---

 drivers/net/wireless/ath9k/ath9k.h  |   23 +++--------------------
 drivers/net/wireless/ath9k/beacon.c |    4 ++--
 drivers/net/wireless/ath9k/core.h   |    4 ++--
 drivers/net/wireless/ath9k/hw.c     |   12 ++++++------
 drivers/net/wireless/ath9k/main.c   |    2 +-
 drivers/net/wireless/ath9k/xmit.c   |   10 +++++-----
 6 files changed, 19 insertions(+), 36 deletions(-)

diff --git a/drivers/net/wireless/ath9k/ath9k.h b/drivers/net/wireless/ath9k/ath9k.h
index dc1da64..eb995ed 100644
--- a/drivers/net/wireless/ath9k/ath9k.h
+++ b/drivers/net/wireless/ath9k/ath9k.h
@@ -281,23 +281,6 @@ enum ath9k_tx_queue_flags {
 	TXQ_FLAG_FRAG_BURST_BACKOFF_ENABLE = 0x0080,
 };
 
-struct ath9k_txq_info {
-	u32 tqi_ver;
-	enum ath9k_tx_queue_subtype tqi_subtype;
-	enum ath9k_tx_queue_flags tqi_qflags;
-	u32 tqi_priority;
-	u32 tqi_aifs;
-	u32 tqi_cwmin;
-	u32 tqi_cwmax;
-	u16 tqi_shretry;
-	u16 tqi_lgretry;
-	u32 tqi_cbrPeriod;
-	u32 tqi_cbrOverflowLimit;
-	u32 tqi_burstTime;
-	u32 tqi_readyTime;
-	u32 tqi_compBuf;
-};
-
 #define ATH9K_TXQ_USEDEFAULT ((u32) -1)
 
 #define ATH9K_DECOMP_MASK_SIZE     128
@@ -999,9 +982,9 @@ u32 ath9k_regd_get_antenna_allowed(struct ath_hal *ah,
 				     struct ath9k_channel *chan);
 u32 ath9k_hw_mhz2ieee(struct ath_hal *ah, u32 freq, u32 flags);
 bool ath9k_hw_gettxqueueprops(struct ath_hal *ah, int q,
-			      struct ath9k_txq_info *qInfo);
+			      struct ath9k_tx_queue_info *qInfo);
 bool ath9k_hw_settxqueueprops(struct ath_hal *ah, int q,
-			      const struct ath9k_txq_info *qInfo);
+			      const struct ath9k_tx_queue_info *qInfo);
 struct ath9k_channel *ath9k_regd_check_channel(struct ath_hal *ah,
 					      const struct ath9k_channel *c);
 void ath9k_hw_set11n_txdesc(struct ath_hal *ah, struct ath_desc *ds,
@@ -1052,7 +1035,7 @@ void ath9k_hw_set11n_virtualmorefrag(struct ath_hal *ah,
 bool ath9k_hw_set_txpowerlimit(struct ath_hal *ah, u32 limit);
 bool ath9k_regd_is_public_safety_sku(struct ath_hal *ah);
 int ath9k_hw_setuptxqueue(struct ath_hal *ah, enum ath9k_tx_queue type,
-			  const struct ath9k_txq_info *qInfo);
+			  const struct ath9k_tx_queue_info *qInfo);
 u32 ath9k_hw_numtxpending(struct ath_hal *ah, u32 q);
 const char *ath9k_hw_probe(u16 vendorid, u16 devid);
 bool ath9k_hw_disable(struct ath_hal *ah);
diff --git a/drivers/net/wireless/ath9k/beacon.c b/drivers/net/wireless/ath9k/beacon.c
index 00993f8..ecdbf0a 100644
--- a/drivers/net/wireless/ath9k/beacon.c
+++ b/drivers/net/wireless/ath9k/beacon.c
@@ -30,7 +30,7 @@
 static int ath_beaconq_config(struct ath_softc *sc)
 {
 	struct ath_hal *ah = sc->sc_ah;
-	struct ath9k_txq_info qi;
+	struct ath9k_tx_queue_info qi;
 
 	ath9k_hw_gettxqueueprops(ah, sc->sc_bhalq, &qi);
 	if (sc->sc_opmode == ATH9K_M_HOSTAP) {
@@ -334,7 +334,7 @@ static void ath_beacon_start_adhoc(struct ath_softc *sc, int if_id)
 
 int ath_beaconq_setup(struct ath_hal *ah)
 {
-	struct ath9k_txq_info qi;
+	struct ath9k_tx_queue_info qi;
 
 	memzero(&qi, sizeof(qi));
 	qi.tqi_aifs = 1;
diff --git a/drivers/net/wireless/ath9k/core.h b/drivers/net/wireless/ath9k/core.h
index cf76b36..2866521 100644
--- a/drivers/net/wireless/ath9k/core.h
+++ b/drivers/net/wireless/ath9k/core.h
@@ -568,7 +568,7 @@ void ath_txq_schedule(struct ath_softc *sc, struct ath_txq *txq);
 int ath_tx_init(struct ath_softc *sc, int nbufs);
 int ath_tx_cleanup(struct ath_softc *sc);
 int ath_tx_get_qnum(struct ath_softc *sc, int qtype, int haltype);
-int ath_txq_update(struct ath_softc *sc, int qnum, struct ath9k_txq_info *q);
+int ath_txq_update(struct ath_softc *sc, int qnum, struct ath9k_tx_queue_info *q);
 int ath_tx_start(struct ath_softc *sc, struct sk_buff *skb);
 void ath_tx_tasklet(struct ath_softc *sc);
 u32 ath_txq_depth(struct ath_softc *sc, int qnum);
@@ -1015,7 +1015,7 @@ struct ath_softc {
 	u32               sc_ant_tx[8];   /* recent tx frames/antenna */
 
 	/* Beacon */
-	struct ath9k_txq_info     sc_beacon_qi;   /* adhoc only: beacon
+	struct ath9k_tx_queue_info     sc_beacon_qi;   /* adhoc only: beacon
 						queue parameters */
 	struct ath_descdma      sc_bdma;        /* beacon descriptors */
 	struct ath_txq          *sc_cabq;       /* tx q for cab frames */
diff --git a/drivers/net/wireless/ath9k/hw.c b/drivers/net/wireless/ath9k/hw.c
index 1f6f393..f063ef0 100644
--- a/drivers/net/wireless/ath9k/hw.c
+++ b/drivers/net/wireless/ath9k/hw.c
@@ -7389,7 +7389,7 @@ ath9k_hw_updatetxtriglevel(struct ath_hal *ah, bool bIncTrigLevel)
 
 static bool ath9k_hw_set_txq_props(struct ath_hal *ah,
 				   struct ath9k_tx_queue_info *qi,
-				   const struct ath9k_txq_info *qInfo)
+				   const struct ath9k_tx_queue_info *qInfo)
 {
 	u32 cw;
 
@@ -7449,7 +7449,7 @@ static bool ath9k_hw_set_txq_props(struct ath_hal *ah,
 }
 
 bool ath9k_hw_settxqueueprops(struct ath_hal *ah, int q,
-			      const struct ath9k_txq_info *qInfo)
+			      const struct ath9k_tx_queue_info *qInfo)
 {
 	struct ath_hal_5416 *ahp = AH5416(ah);
 	struct hal_capabilities *pCap = &ah->ah_caps;
@@ -7463,7 +7463,7 @@ bool ath9k_hw_settxqueueprops(struct ath_hal *ah, int q,
 }
 
 static bool ath9k_hw_get_txq_props(struct ath_hal *ah,
-				   struct ath9k_txq_info *qInfo,
+				   struct ath9k_tx_queue_info *qInfo,
 				   const struct ath9k_tx_queue_info *qi)
 {
 	if (qi->tqi_type == ATH9K_TX_QUEUE_INACTIVE) {
@@ -7492,7 +7492,7 @@ static bool ath9k_hw_get_txq_props(struct ath_hal *ah,
 
 bool
 ath9k_hw_gettxqueueprops(struct ath_hal *ah, int q,
-			 struct ath9k_txq_info *qInfo)
+			 struct ath9k_tx_queue_info *qInfo)
 {
 	struct ath_hal_5416 *ahp = AH5416(ah);
 	struct hal_capabilities *pCap = &ah->ah_caps;
@@ -7507,7 +7507,7 @@ ath9k_hw_gettxqueueprops(struct ath_hal *ah, int q,
 
 int
 ath9k_hw_setuptxqueue(struct ath_hal *ah, enum ath9k_tx_queue type,
-		      const struct ath9k_txq_info *qInfo)
+		      const struct ath9k_tx_queue_info *qInfo)
 {
 	struct ath_hal_5416 *ahp = AH5416(ah);
 	struct ath9k_tx_queue_info *qi;
@@ -7566,7 +7566,7 @@ ath9k_hw_setuptxqueue(struct ath_hal *ah, enum ath9k_tx_queue type,
 		qi->tqi_lgretry = INIT_LG_RETRY;
 		qi->tqi_physCompBuf = 0;
 	} else {
-		qi->tqi_physCompBuf = qInfo->tqi_compBuf;
+		qi->tqi_physCompBuf = qInfo->tqi_physCompBuf;
 		(void) ath9k_hw_settxqueueprops(ah, q, qInfo);
 	}
 
diff --git a/drivers/net/wireless/ath9k/main.c b/drivers/net/wireless/ath9k/main.c
index 9549524..c497733 100644
--- a/drivers/net/wireless/ath9k/main.c
+++ b/drivers/net/wireless/ath9k/main.c
@@ -700,7 +700,7 @@ static int ath9k_conf_tx(struct ieee80211_hw *hw,
 			 const struct ieee80211_tx_queue_params *params)
 {
 	struct ath_softc *sc = hw->priv;
-	struct ath9k_txq_info qi;
+	struct ath9k_tx_queue_info qi;
 	int ret = 0, qnum;
 
 	if (queue >= WME_NUM_AC)
diff --git a/drivers/net/wireless/ath9k/xmit.c b/drivers/net/wireless/ath9k/xmit.c
index f0297ee..8022137 100644
--- a/drivers/net/wireless/ath9k/xmit.c
+++ b/drivers/net/wireless/ath9k/xmit.c
@@ -2209,7 +2209,7 @@ int ath_tx_cleanup(struct ath_softc *sc)
 struct ath_txq *ath_txq_setup(struct ath_softc *sc, int qtype, int subtype)
 {
 	struct ath_hal *ah = sc->sc_ah;
-	struct ath9k_txq_info qi;
+	struct ath9k_tx_queue_info qi;
 	int qnum;
 
 	memzero(&qi, sizeof(qi));
@@ -2217,7 +2217,7 @@ struct ath_txq *ath_txq_setup(struct ath_softc *sc, int qtype, int subtype)
 	qi.tqi_aifs = ATH9K_TXQ_USEDEFAULT;
 	qi.tqi_cwmin = ATH9K_TXQ_USEDEFAULT;
 	qi.tqi_cwmax = ATH9K_TXQ_USEDEFAULT;
-	qi.tqi_compBuf = 0;
+	qi.tqi_physCompBuf = 0;
 
 	/*
 	 * Enable interrupts only for EOL and DESC conditions.
@@ -2337,11 +2337,11 @@ int ath_tx_get_qnum(struct ath_softc *sc, int qtype, int haltype)
 
 /* Update parameters for a transmit queue */
 
-int ath_txq_update(struct ath_softc *sc, int qnum, struct ath9k_txq_info *qi0)
+int ath_txq_update(struct ath_softc *sc, int qnum, struct ath9k_tx_queue_info *qi0)
 {
 	struct ath_hal *ah = sc->sc_ah;
 	int error = 0;
-	struct ath9k_txq_info qi;
+	struct ath9k_tx_queue_info qi;
 
 	if (qnum == sc->sc_bhalq) {
 		/*
@@ -2376,7 +2376,7 @@ int ath_txq_update(struct ath_softc *sc, int qnum, struct ath9k_txq_info *qi0)
 
 int ath_cabq_update(struct ath_softc *sc)
 {
-	struct ath9k_txq_info qi;
+	struct ath9k_tx_queue_info qi;
 	int qnum = sc->sc_cabq->axq_qnum;
 	struct ath_beacon_config conf;
 

[ath9k-devel] [PATCH 1/1] ath9k: merge ath9k_txq_info and ath9k_tx_queue_info structures

[Luis R. Rodriguez]

On Wed, Aug 6, 2008 at 2:52 PM, Alistair John Strachan
<alistair@devzero.co.uk> wrote:
> Merge struct ath9k_txq_info into struct ath9k_tx_queue_info. This
> change was mostly mechanical with the exception of the field tqi_compBuf
> which is renamed as tqi_physCompBuf and fixed up where it was used.
>
> Merging the structures the other way would also have been possible,
> but the ath9k_tx_queue_info name is more consistent with other similarly
> named structures declared above.

Thanks, I think Sujith was working on this too, also Jouni is working
on beaconing stuff so they can iron the integration out.

> Patch is compile time and runtime tested.

Thanks for the heads up, you should only mention something like this
if *you didn't* test the patch. Otherwise its assumed you did. Also
just a minor note, if you send only one patch there is no need to send
a patch 0/1.

> Changes-licensed-under: ISC

No need for this for us, if you tell us once you are keeping the
license for all subsequent patches for ath9k that's good enough for
our records. But thanks for your consideration on this.

  Luis

[ath9k-devel] [PATCH 1/1] ath9k: merge ath9k_txq_info and ath9k_tx_queue_info structures

[Alistair John Strachan]

On Thursday 07 August 2008 04:45:27 Luis R. Rodriguez wrote:
> On Wed, Aug 6, 2008 at 2:52 PM, Alistair John Strachan
>
> <alistair@devzero.co.uk> wrote:
> > Merge struct ath9k_txq_info into struct ath9k_tx_queue_info. This
> > change was mostly mechanical with the exception of the field tqi_compBuf
> > which is renamed as tqi_physCompBuf and fixed up where it was used.
> >
> > Merging the structures the other way would also have been possible,
> > but the ath9k_tx_queue_info name is more consistent with other similarly
> > named structures declared above.
>
> Thanks, I think Sujith was working on this too, also Jouni is working
> on beaconing stuff so they can iron the integration out.

It's a bit disappointing that this very same patch has appeared from you 
internally. Maybe some more communication is required to make sure there isn't 
duplicated work, like for example updates on what Atheros are actually doing 
themselves from the TODO list. Otherwise I don't see that community 
participation is easily possible.

> > Patch is compile time and runtime tested.
>
> Thanks for the heads up, you should only mention something like this
> if *you didn't* test the patch. Otherwise its assumed you did. Also
> just a minor note, if you send only one patch there is no need to send
> a patch 0/1.

If you know of a way of disabling the apparently mandatory --compose --subject 
feature of git-send-email I'd love to hear it. From what I can see, even 
though it might be optimized for sending patch series, an intro patch is still 
sometimes useful for information that should NOT end up in the commit log.

Also I have to disagree with you over the "implicitly tested" thing. I've been 
reading LKML for too long and know that the frequency of poorly or completely 
untested patches that people try to push can be high. On reflection this 
information should have appeared in the intro rather than the patch.

> > Changes-licensed-under: ISC
>
> No need for this for us, if you tell us once you are keeping the
> license for all subsequent patches for ath9k that's good enough for
> our records. But thanks for your consideration on this.

Yep, okay.

-- 
Cheers,
Alistair.

[ath9k-devel] [PATCH 1/1] ath9k: merge ath9k_txq_info and ath9k_tx_queue_info structures

[Sujith]

Alistair John Strachan wrote:
 > It's a bit disappointing that this very same patch has appeared from you 
 > internally. Maybe some more communication is required to make sure there isn't 
 > duplicated work, like for example updates on what Atheros are actually doing 
 > themselves from the TODO list. Otherwise I don't see that community 
 > participation is easily possible.

Sorry about that, you are right, it is duplicated effort.
I'll update http://wireless.kernel.org/en/users/Drivers/ath9k to indicate
what we are working on currently.

Sujith

[ath9k-devel] [PATCH 1/1] ath9k: merge ath9k_txq_info and ath9k_tx_queue_info structures

[Luis R. Rodriguez]

On Thu, Aug 7, 2008 at 2:29 AM, Alistair John Strachan
<alistair@devzero.co.uk> wrote:
> On Thursday 07 August 2008 04:45:27 Luis R. Rodriguez wrote:

>> > Patch is compile time and runtime tested.
>>
>> Thanks for the heads up, you should only mention something like this
>> if *you didn't* test the patch. Otherwise its assumed you did. Also
>> just a minor note, if you send only one patch there is no need to send
>> a patch 0/1.
>
> If you know of a way of disabling the apparently mandatory --compose --subject
> feature of git-send-email I'd love to hear it. From what I can see, even
> though it might be optimized for sending patch series, an intro patch is still
> sometimes useful for information that should NOT end up in the commit log.

Sure, feel free to send it, I'm just letting you know its not
necessary at least not something we ask for on linux-wireless.

> Also I have to disagree with you over the "implicitly tested" thing. I've been
> reading LKML for too long and know that the frequency of poorly or completely
> untested patches that people try to push can be high. On reflection this
> information should have appeared in the intro rather than the patch.

Sure, thanks for pointing this out.

  Luis