Skip to contentSkip to conversation

ath5k: Remove fill_tx_desc

PATCH In review Nick Kossifidis 4 participants 5 messages Oct 27, 2007 – Oct 31, 2007 Networking drivers (wireless)

lore
Share & tools
lore

Nick Kossifidis

author
#0 lore raw

PATCH 1/1 ath5k: Remove fill_tx_desc

fill_tx_desc is used for fast frames operation, since we don't support fast frames (and since fill_tx_desc had a bug -thanx to Ulrich Meis for finding that out-) these functions are not needed (+ they are misleading because they don't "fill" any tx descriptor).

I couldn't test this patch much so plz someone ACK it...

It applies on top of my previous patches (i just thought that [PATCH 8/7] won't look nice ;-) ).

Changes-licensed-under: ISC

Signed-off-by Ulrich Meis <[email protected]>
Signed-off-by Nick Kossifidis <[email protected]>

---

drivers/net/wireless/ath5k/ath5k.h +0 -2
index ef76f1c..4122466 100644
@@ -1031,8 +1031,6 @@ struct ath5k_hw {
 	bool (*ah_setup_xtx_desc)(struct ath5k_hw *, struct ath5k_desc *,
 		unsigned int, unsigned int, unsigned int, unsigned int,
 		unsigned int, unsigned int);
-	int (*ah_fill_tx_desc)(struct ath5k_hw *, struct ath5k_desc *,
-		unsigned int, bool, bool);
 	int (*ah_proc_tx_desc)(struct ath5k_hw *, struct ath5k_desc *);
 	int (*ah_proc_rx_desc)(struct ath5k_hw *, struct ath5k_desc *);
 };
drivers/net/wireless/ath5k/base.c +0 -8
index e6a3155..4b4ddf4 100644
@@ -1390,10 +1390,6 @@ ath5k_txbuf_setup(struct ath5k_softc *sc, struct ath5k_buf *bf,
 	ds->ds_link = 0;
 	ds->ds_data = bf->skbaddr;
 
-	ret = ah->ah_fill_tx_desc(ah, ds, skb->len, true, true);
-	if (ret)
-		goto err_unmap;
-
 	spin_lock_bh(&txq->lock);
 	list_add_tail(&bf->list, &txq->q);
 	sc->tx_stats.data[txq->qnum].len++;

@@ -1966,10 +1962,6 @@ ath5k_beacon_setup(struct ath5k_softc *sc, struct ath5k_buf *bf,
 			AR5K_TXKEYIX_INVALID, antenna, flags, 0, 0);
 	if (ret)
 		goto err_unmap;
-	/* NB: beacon's BufLen must be a multiple of 4 bytes */
-	ret = ah->ah_fill_tx_desc(ah, ds, roundup(skb->len, 4), true, true);
-	if (ret)
-		goto err_unmap;
 
 	return 0;
 err_unmap:
drivers/net/wireless/ath5k/hw.c +54 -81

237 lines not on this page · raw message

#1 lore raw
Quoted 8 lines · Nick Kossifidis

fill_tx_desc is used for fast frames operation, since we don't support fast frames (and since fill_tx_desc had a bug -thanx to Ulrich Meis for finding that out-) these functions are not needed (+ they are misleading because they don't "fill" any tx descriptor).

I couldn't test this patch much so plz someone ACK it...

Quoted 7 lines

It applies on top of my previous patches (i just thought that [PATCH 8/7] won't look nice ;-) ).

Changes-licensed-under: ISC

Signed-off-by Ulrich Meis <[email protected]>
Signed-off-by Nick Kossifidis <[email protected]>

Very nice, I've been testing this on 2.6.24-rc1 for a day without problems. I'll soon post results of of all your patches and your 1-7 series on big endian. Compiling takes a while there, heh.. I guess I should be cross compiling huh.

Luis

#2 lore raw
Quoted 13 lines · Luis R. Rodriguez
Quoted 12 lines · Nick Kossifidis

fill_tx_desc is used for fast frames operation, since we don't support fast frames (and since fill_tx_desc had a bug -thanx to Ulrich Meis for finding that out-) these functions are not needed (+ they are misleading because they don't "fill" any tx descriptor).

I couldn't test this patch much so plz someone ACK it...

It applies on top of my previous patches (i just thought that [PATCH 8/7] won't look nice ;-) ).

Quoted 8 lines
Quoted 3 lines

Changes-licensed-under: ISC

Signed-off-by Ulrich Meis <[email protected]>
Signed-off-by Nick Kossifidis <[email protected]>

Very nice, I've been testing this on 2.6.24-rc1 for a day without problems. I'll soon post results of of all your patches and your 1-7 series on big endian. Compiling takes a while there, heh.. I guess I should be cross compiling huh.

People using the 5212 need the line below though if they wanna be able to sent out frames :) It's missing in the 4word version. Besides that the patches also seem to run fine here.

Uli

P.S.: Sry Nick, posted this to you yesterday but forgot to include the list.

drivers/net/wireless/ath5k/hw.c +2 -0
index 7b9920c..dc9b881 100644
@@ -3641,6 +3641,8 @@ static int ath5k_hw_setup_4word_tx_desc(struct ath5k_hw *ah,
        if (buff_len & ~AR5K_4W_TX_DESC_CTL1_BUF_LEN)
                return -EINVAL;
 
+       tx_desc->tx_control_1 = buff_len & AR5K_2W_TX_DESC_CTL1_BUF_LEN;
+
        tx_desc->tx_control_0 |=
                AR5K_REG_SM(tx_power, AR5K_4W_TX_DESC_CTL0_XMIT_POWER) |
                AR5K_REG_SM(antenna_mode, AR5K_4W_TX_DESC_CTL0_ANT_MODE_XMIT);
#3 lore raw

2007/10/30, Ulrich Meis <[email protected]>:

Quoted 23 lines · Luis R. Rodriguez
Quoted 13 lines · Nick Kossifidis
Quoted 12 lines

fill_tx_desc is used for fast frames operation, since we don't support fast frames (and since fill_tx_desc had a bug -thanx to Ulrich Meis for finding that out-) these functions are not needed (+ they are misleading because they don't "fill" any tx descriptor).

I couldn't test this patch much so plz someone ACK it...

It applies on top of my previous patches (i just thought that [PATCH 8/7] won't look nice ;-) ).

Quoted 8 lines
Quoted 3 lines

Changes-licensed-under: ISC

Signed-off-by Ulrich Meis <[email protected]>
Signed-off-by Nick Kossifidis <[email protected]>

Very nice, I've been testing this on 2.6.24-rc1 for a day without problems. I'll soon post results of of all your patches and your 1-7 series on big endian. Compiling takes a while there, heh.. I guess I should be cross compiling huh.

Quoted 6 lines

People using the 5212 need the line below though if they wanna be able to sent out frames :) It's missing in the 4word version. Besides that the patches also seem to run fine here.

Uli

Ouch, i've missed this, thanx for finding out ;-)

drivers/net/wireless/ath5k/hw.c static int ath5k_hw_setup_4word_tx_desc(struct ath5k_hw *ah, · line 3645

Quoted 2 lines
drivers/net/wireless/ath5k/hw.c +2 -0
+       tx_desc->tx_control_1 = buff_len & AR5K_2W_TX_DESC_CTL1_BUF_LEN;
+

2W_TX_DESC -> define for 2word descriptor (check out hw.h) just to maintain naming
scheme, i suggest we use the 4W_TX_DESC define (they are the same values but it's better for code readability).

tx_desc->tx_control_1 = buff_len & AR5K_4W_TX_DESC_CTL1_BUF_LEN;

can you plz repost the patch as a separate thread (check out
http://www.linuxwireless.org/en/developers/SubmittingPatches) ?

Signature · 3 lines
GPG ID: 0xD21DB2DB
As you read this post global entropy rises. Have Fun ;-)
Nick
#4 lore raw
Quoted 41 lines · Nick Kossifidis

2007/10/30, Ulrich Meis <[email protected]>:

Quoted 23 lines · Luis R. Rodriguez
Quoted 13 lines · Nick Kossifidis
Quoted 12 lines

fill_tx_desc is used for fast frames operation, since we don't support fast frames (and since fill_tx_desc had a bug -thanx to Ulrich Meis for finding that out-) these functions are not needed (+ they are misleading because they don't "fill" any tx descriptor).

I couldn't test this patch much so plz someone ACK it...

It applies on top of my previous patches (i just thought that [PATCH 8/7] won't look nice ;-) ).

Quoted 8 lines
Quoted 3 lines

Changes-licensed-under: ISC

Signed-off-by Ulrich Meis <[email protected]>
Signed-off-by Nick Kossifidis <[email protected]>

Very nice, I've been testing this on 2.6.24-rc1 for a day without problems. I'll soon post results of of all your patches and your 1-7 series on big endian. Compiling takes a while there, heh.. I guess I should be cross compiling huh.

Quoted 6 lines

People using the 5212 need the line below though if they wanna be able to sent out frames :) It's missing in the 4word version. Besides that the patches also seem to run fine here.

Uli

Ouch, i've missed this, thanx for finding out ;-)

drivers/net/wireless/ath5k/hw.c static int ath5k_hw_setup_4word_tx_desc(struct ath5k_hw *ah, · line 3645

Quoted 2 lines
drivers/net/wireless/ath5k/hw.c +2 -0
+       tx_desc->tx_control_1 = buff_len & AR5K_2W_TX_DESC_CTL1_BUF_LEN;
+

2W_TX_DESC -> define for 2word descriptor (check out hw.h) just to maintain naming
scheme, i suggest we use the 4W_TX_DESC define (they are the same values but it's better for code readability).

Quoted 4 lines

tx_desc->tx_control_1 = buff_len & AR5K_4W_TX_DESC_CTL1_BUF_LEN;

can you plz repost the patch as a separate thread (check out
http://www.linuxwireless.org/en/developers/SubmittingPatches) ?

IMO you should have just sent a patch as your patch really does break AR5212 TX but whatever works with John. Anyway, just a pointer to John -- hunk 3 on hw.c will fail here but its not important its just a redundant hunk.

Luis