* [PATCH] wl12xx: set the actual tid instead of the ac
@ 2011-03-16 21:03 Eliad Peller
2011-03-17 5:38 ` Juuso Oikarinen
2011-03-21 14:24 ` Luciano Coelho
0 siblings, 2 replies; 6+ messages in thread
From: Eliad Peller @ 2011-03-16 21:03 UTC (permalink / raw)
To: Luciano Coelho; +Cc: linux-wireless
When passing a tx frame, the driver incorrectly set desc->tid
with the ac instead of the actual tid.
It has some serious implications when using 802.11n + QoS,
as the fw starts a BlockAck with the wrong tid (which finally
cause beacon loss and disconnection / some fw crash)
Fix it by using the actual tid stored in skb->priority.
Reported-by: Shahar Levi <shahar_levi@ti.com>
Signed-off-by: Eliad Peller <eliad@wizery.com>
---
drivers/net/wireless/wl12xx/tx.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/wireless/wl12xx/tx.c b/drivers/net/wireless/wl12xx/tx.c
index 5e9ef7d..4bb3d99 100644
--- a/drivers/net/wireless/wl12xx/tx.c
+++ b/drivers/net/wireless/wl12xx/tx.c
@@ -205,9 +205,9 @@ static void wl1271_tx_fill_hdr(struct wl1271 *wl, struct sk_buff *skb,
/* configure the tx attributes */
tx_attr = wl->session_counter << TX_HW_ATTR_OFST_SESSION_COUNTER;
- /* queue (we use same identifiers for tid's and ac's */
+ /* queue */
ac = wl1271_tx_get_queue(skb_get_queue_mapping(skb));
- desc->tid = ac;
+ desc->tid = skb->priority;
if (wl->bss_type != BSS_TYPE_AP_BSS) {
desc->aid = hlid;
--
1.7.0.4
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] wl12xx: set the actual tid instead of the ac
2011-03-16 21:03 [PATCH] wl12xx: set the actual tid instead of the ac Eliad Peller
@ 2011-03-17 5:38 ` Juuso Oikarinen
2011-03-21 7:54 ` Eliad Peller
2011-03-21 14:24 ` Luciano Coelho
1 sibling, 1 reply; 6+ messages in thread
From: Juuso Oikarinen @ 2011-03-17 5:38 UTC (permalink / raw)
To: ext Eliad Peller; +Cc: Luciano Coelho, linux-wireless
On Wed, 2011-03-16 at 23:03 +0200, ext Eliad Peller wrote:
> When passing a tx frame, the driver incorrectly set desc->tid
> with the ac instead of the actual tid.
>
> It has some serious implications when using 802.11n + QoS,
> as the fw starts a BlockAck with the wrong tid (which finally
> cause beacon loss and disconnection / some fw crash)
>
> Fix it by using the actual tid stored in skb->priority.
How will the firmware handle the TIDs larger than 3, as currently to the
firmware it appears only TID's 0-3 are configured, and the rest are
whatever values happen to be there default?
-Juuso
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] wl12xx: set the actual tid instead of the ac
2011-03-17 5:38 ` Juuso Oikarinen
@ 2011-03-21 7:54 ` Eliad Peller
2011-03-21 7:59 ` Juuso Oikarinen
0 siblings, 1 reply; 6+ messages in thread
From: Eliad Peller @ 2011-03-21 7:54 UTC (permalink / raw)
To: Juuso Oikarinen; +Cc: Luciano Coelho, linux-wireless
On Thu, Mar 17, 2011 at 7:38 AM, Juuso Oikarinen
<juuso.oikarinen@nokia.com> wrote:
> On Wed, 2011-03-16 at 23:03 +0200, ext Eliad Peller wrote:
>> When passing a tx frame, the driver incorrectly set desc->tid
>> with the ac instead of the actual tid.
>>
>> It has some serious implications when using 802.11n + QoS,
>> as the fw starts a BlockAck with the wrong tid (which finally
>> cause beacon loss and disconnection / some fw crash)
>>
>> Fix it by using the actual tid stored in skb->priority.
>
> How will the firmware handle the TIDs larger than 3, as currently to the
> firmware it appears only TID's 0-3 are configured, and the rest are
> whatever values happen to be there default?
>
that's a good question.
according to the fw guys, the fw auto-maps the tid to the correct ac.
the confusing thing is that ACX_TID_CFG actually gets AC as its input
(in the queue_id param) rather than TID.
thus, only 0-3 (ACs, not TIDs) are configured.
Eliad.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] wl12xx: set the actual tid instead of the ac
2011-03-21 7:54 ` Eliad Peller
@ 2011-03-21 7:59 ` Juuso Oikarinen
2011-03-21 8:49 ` Juuso Oikarinen
0 siblings, 1 reply; 6+ messages in thread
From: Juuso Oikarinen @ 2011-03-21 7:59 UTC (permalink / raw)
To: ext Eliad Peller; +Cc: Luciano Coelho, linux-wireless
On Mon, 2011-03-21 at 09:54 +0200, ext Eliad Peller wrote:
> On Thu, Mar 17, 2011 at 7:38 AM, Juuso Oikarinen
> <juuso.oikarinen@nokia.com> wrote:
> > On Wed, 2011-03-16 at 23:03 +0200, ext Eliad Peller wrote:
> >> When passing a tx frame, the driver incorrectly set desc->tid
> >> with the ac instead of the actual tid.
> >>
> >> It has some serious implications when using 802.11n + QoS,
> >> as the fw starts a BlockAck with the wrong tid (which finally
> >> cause beacon loss and disconnection / some fw crash)
> >>
> >> Fix it by using the actual tid stored in skb->priority.
> >
> > How will the firmware handle the TIDs larger than 3, as currently to the
> > firmware it appears only TID's 0-3 are configured, and the rest are
> > whatever values happen to be there default?
> >
> that's a good question.
> according to the fw guys, the fw auto-maps the tid to the correct ac.
> the confusing thing is that ACX_TID_CFG actually gets AC as its input
> (in the queue_id param) rather than TID.
> thus, only 0-3 (ACs, not TIDs) are configured.
>
Yes, so the TID's are most likely not configured in a way that the
firmware will be able to "auto-map". I think the at minimum the
ACX_TID_CFG must be revisited before this change can work.
-Juuso
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] wl12xx: set the actual tid instead of the ac
2011-03-21 7:59 ` Juuso Oikarinen
@ 2011-03-21 8:49 ` Juuso Oikarinen
0 siblings, 0 replies; 6+ messages in thread
From: Juuso Oikarinen @ 2011-03-21 8:49 UTC (permalink / raw)
To: ext Eliad Peller; +Cc: Luciano Coelho, linux-wireless
On Mon, 2011-03-21 at 09:59 +0200, ext Juuso Oikarinen wrote:
> On Mon, 2011-03-21 at 09:54 +0200, ext Eliad Peller wrote:
> > On Thu, Mar 17, 2011 at 7:38 AM, Juuso Oikarinen
> > <juuso.oikarinen@nokia.com> wrote:
> > > On Wed, 2011-03-16 at 23:03 +0200, ext Eliad Peller wrote:
> > >> When passing a tx frame, the driver incorrectly set desc->tid
> > >> with the ac instead of the actual tid.
> > >>
> > >> It has some serious implications when using 802.11n + QoS,
> > >> as the fw starts a BlockAck with the wrong tid (which finally
> > >> cause beacon loss and disconnection / some fw crash)
> > >>
> > >> Fix it by using the actual tid stored in skb->priority.
> > >
> > > How will the firmware handle the TIDs larger than 3, as currently to the
> > > firmware it appears only TID's 0-3 are configured, and the rest are
> > > whatever values happen to be there default?
> > >
> > that's a good question.
> > according to the fw guys, the fw auto-maps the tid to the correct ac.
> > the confusing thing is that ACX_TID_CFG actually gets AC as its input
> > (in the queue_id param) rather than TID.
> > thus, only 0-3 (ACs, not TIDs) are configured.
> >
>
> Yes, so the TID's are most likely not configured in a way that the
> firmware will be able to "auto-map". I think the at minimum the
> ACX_TID_CFG must be revisited before this change can work.
>
As discussed on IRC. So there is no dependency between the index of the
ACX_TID_CFG configurations and the tid passed to the TX descriptor.
Instead the FW independently maps the tids to corresponding AC's,
regardless of the tsid's configured in ACX_TID_CFG.
My confusion here was that I assumed the indexes given to ACX_TID_CFG
must match the actual priority values.
Based on this and the IRC chat (which was enlightening):
Reviewed-by: Juuso Oikarinen <juuso.oikarinen@nokia.com>
-Juuso
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] wl12xx: set the actual tid instead of the ac
2011-03-16 21:03 [PATCH] wl12xx: set the actual tid instead of the ac Eliad Peller
2011-03-17 5:38 ` Juuso Oikarinen
@ 2011-03-21 14:24 ` Luciano Coelho
1 sibling, 0 replies; 6+ messages in thread
From: Luciano Coelho @ 2011-03-21 14:24 UTC (permalink / raw)
To: Eliad Peller; +Cc: linux-wireless
On Wed, 2011-03-16 at 23:03 +0200, Eliad Peller wrote:
> When passing a tx frame, the driver incorrectly set desc->tid
> with the ac instead of the actual tid.
>
> It has some serious implications when using 802.11n + QoS,
> as the fw starts a BlockAck with the wrong tid (which finally
> cause beacon loss and disconnection / some fw crash)
>
> Fix it by using the actual tid stored in skb->priority.
>
> Reported-by: Shahar Levi <shahar_levi@ti.com>
> Signed-off-by: Eliad Peller <eliad@wizery.com>
> ---
Applied, thank you! And thanks Juuso for the review!
--
Cheers,
Luca.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2011-03-21 14:25 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-03-16 21:03 [PATCH] wl12xx: set the actual tid instead of the ac Eliad Peller
2011-03-17 5:38 ` Juuso Oikarinen
2011-03-21 7:54 ` Eliad Peller
2011-03-21 7:59 ` Juuso Oikarinen
2011-03-21 8:49 ` Juuso Oikarinen
2011-03-21 14:24 ` Luciano Coelho
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox