Skip to contentSkip to conversation

b43: Fix rxheader channel parsing

PATCH In review Michael Buesch 3 participants 6 messages Jan 2, 2008 Networking drivers (wireless)

lore
Share & tools
lore

Michael Buesch

author
#0 lore raw

PATCH 1/1 b43: Fix rxheader channel parsing

This patch fixes the parsing of the RX data header channel field.

The current code parses the header incorrectly and passes a wrong channel number and frequency for each frame to mac80211. The FIXMEs added by this patch don't matter for now as the code where they live won't get executed anyway. They will be fixed later.

Signed-off-by Michael Buesch <[email protected]>

---

John, as this is a bugfix, it should go into 2.6.24 if still possible.

drivers/net/wireless/b43/xmit.c +19 -8
Index: wireless-2.6/drivers/net/wireless/b43/xmit.c
===================================================================
@@ -549,21 +549,32 @@ void b43_rx(struct b43_wldev *dev, struc
 	switch (chanstat & B43_RX_CHAN_PHYTYPE) {
 	case B43_PHYTYPE_A:
 		status.phymode = MODE_IEEE80211A;
-		status.freq = chanid;
-		status.channel = b43_freq_to_channel_a(chanid);
-		break;
-	case B43_PHYTYPE_B:
-		status.phymode = MODE_IEEE80211B;
-		status.freq = chanid + 2400;
-		status.channel = b43_freq_to_channel_bg(chanid + 2400);
+		B43_WARN_ON(1);
+		/* FIXME: We don't really know which value the "chanid" contains.
+		 *        So the following assignment might be wrong. */
+		status.channel = chanid;
+		status.freq = b43_channel_to_freq_5ghz(status.channel);
 		break;
 	case B43_PHYTYPE_G:
 		status.phymode = MODE_IEEE80211G;
+		/* chanid is the radio channel cookie value as used
+		 * to tune the radio. */
 		status.freq = chanid + 2400;
-		status.channel = b43_freq_to_channel_bg(chanid + 2400);
+		status.channel = b43_freq_to_channel_2ghz(status.freq);
+		break;
+	case B43_PHYTYPE_N:
+		status.phymode = 0xDEAD /*FIXME MODE_IEEE80211N*/;
+		/* chanid is the SHM channel cookie. Which is the plain
+		 * channel number in b43. */
+		status.channel = chanid;
+		if (chanstat & B43_RX_CHAN_5GHZ)
+			status.freq = b43_freq_to_channel_5ghz(status.freq);
+		else
+			status.freq = b43_freq_to_channel_2ghz(status.freq);
 		break;
 	default:
 		B43_WARN_ON(1);
+		goto drop;
 	}
 
 	dev->stats.last_rx = jiffies;
drivers/net/wireless/b43/xmit.h +36 -29

85 lines not on this page · raw message

drivers/net/wireless/b43/b43.h +2 -0
Index: wireless-2.6/drivers/net/wireless/b43/b43.h
===================================================================
@@ -277,6 +277,8 @@ enum {
 #define B43_PHYTYPE_A			0x00
 #define B43_PHYTYPE_B			0x01
 #define B43_PHYTYPE_G			0x02
+#define B43_PHYTYPE_N			0x04
+#define B43_PHYTYPE_LP			0x05
 
 /* PHYRegisters */
 #define B43_PHY_ILT_A_CTRL		0x0072
drivers/net/wireless/b43/main.h +4 -16
Index: wireless-2.6/drivers/net/wireless/b43/main.h
===================================================================
@@ -39,11 +39,11 @@
 #define PAD_BYTES(nr_bytes)		P4D_BYTES( __LINE__ , (nr_bytes))
 
 /* Lightweight function to convert a frequency (in Mhz) to a channel number. */
-static inline u8 b43_freq_to_channel_a(int freq)
+static inline u8 b43_freq_to_channel_5ghz(int freq)
 {
 	return ((freq - 5000) / 5);
 }
-static inline u8 b43_freq_to_channel_bg(int freq)
+static inline u8 b43_freq_to_channel_2ghz(int freq)
 {
 	u8 channel;
 

@@ -54,19 +54,13 @@ static inline u8 b43_freq_to_channel_bg(
 
 	return channel;
 }
-static inline u8 b43_freq_to_channel(struct b43_wldev *dev, int freq)
-{
-	if (dev->phy.type == B43_PHYTYPE_A)
-		return b43_freq_to_channel_a(freq);
-	return b43_freq_to_channel_bg(freq);
-}
 
 /* Lightweight function to convert a channel number to a frequency (in Mhz). */
-static inline int b43_channel_to_freq_a(u8 channel)
+static inline int b43_channel_to_freq_5ghz(u8 channel)
 {
 	return (5000 + (5 * channel));
 }
-static inline int b43_channel_to_freq_bg(u8 channel)
+static inline int b43_channel_to_freq_2ghz(u8 channel)
 {
 	int freq;
 

@@ -77,12 +71,6 @@ static inline int b43_channel_to_freq_bg
 
 	return freq;
 }
-static inline int b43_channel_to_freq(struct b43_wldev *dev, u8 channel)
-{
-	if (dev->phy.type == B43_PHYTYPE_A)
-		return b43_channel_to_freq_a(channel);
-	return b43_channel_to_freq_bg(channel);
-}
 
 static inline int b43_is_cck_rate(int rate)
 {
#1 lore raw
Quoted 24 lines · Michael Buesch

This patch fixes the parsing of the RX data header channel field.

The current code parses the header incorrectly and passes a wrong channel number and frequency for each frame to mac80211. The FIXMEs added by this patch don't matter for now as the code where they live won't get executed anyway. They will be fixed later.

Signed-off-by Michael Buesch <[email protected]>

---

John, as this is a bugfix, it should go into 2.6.24 if still possible.

Index: wireless-2.6/drivers/net/wireless/b43/xmit.c
===================================================================

drivers/net/wireless/b43/xmit.c +0 -3
@@ -549,21 +549,32 @@ void b43_rx(struct b43_wldev *dev, struc
 	switch (chanstat & B43_RX_CHAN_PHYTYPE) {
 	case B43_PHYTYPE_A:
 		status.phymode = MODE_IEEE80211A;
-		status.freq = chanid;
-		status.channel = b43_freq_to_channel_a(chanid);
-		break;

drivers/net/wireless/b43/xmit.c void b43_rx(struct b43_wldev *dev, struc · line 557

Quoted 10 lines
drivers/net/wireless/b43/xmit.c +5 -4
-	case B43_PHYTYPE_B:
-		status.phymode = MODE_IEEE80211B;
-		status.freq = chanid + 2400;
-		status.channel = b43_freq_to_channel_bg(chanid + 2400);
+		B43_WARN_ON(1);
+		/* FIXME: We don't really know which value the "chanid" contains.
+		 *        So the following assignment might be wrong. */
+		status.channel = chanid;
+		status.freq = b43_channel_to_freq_5ghz(status.channel);
 		break;

Shouldn't you just drop this case? No B PHY devices will ever use b43 and the default branch will issue the WARN_ON anyway.

Larry

author
#2 lore raw
Quoted 30 lines · On Wednesday 02 January 2008 19:52:08 Larry Finger wrote:
Quoted 29 lines · Michael Buesch

This patch fixes the parsing of the RX data header channel field.

The current code parses the header incorrectly and passes a wrong channel number and frequency for each frame to mac80211. The FIXMEs added by this patch don't matter for now as the code where they live won't get executed anyway. They will be fixed later.

Signed-off-by Michael Buesch <[email protected]>

---

John, as this is a bugfix, it should go into 2.6.24 if still possible.

Index: wireless-2.6/drivers/net/wireless/b43/xmit.c
===================================================================

drivers/net/wireless/b43/xmit.c +1 -7
@@ -549,21 +549,32 @@ void b43_rx(struct b43_wldev *dev, struc
 	switch (chanstat & B43_RX_CHAN_PHYTYPE) {
 	case B43_PHYTYPE_A:
 		status.phymode = MODE_IEEE80211A;
-		status.freq = chanid;
-		status.channel = b43_freq_to_channel_a(chanid);
-		break;
-	case B43_PHYTYPE_B:
-		status.phymode = MODE_IEEE80211B;
-		status.freq = chanid + 2400;
-		status.channel = b43_freq_to_channel_bg(chanid + 2400);
+		B43_WARN_ON(1);
Quoted 8 lines

drivers/net/wireless/b43/xmit.c void b43_rx(struct b43_wldev *dev, struc · line 557

Quoted 5 lines
drivers/net/wireless/b43/xmit.c +4 -0
+		/* FIXME: We don't really know which value the "chanid" contains.
+		 *        So the following assignment might be wrong. */
+		status.channel = chanid;
+		status.freq = b43_channel_to_freq_5ghz(status.channel);
 		break;

Shouldn't you just drop this case? No B PHY devices will ever use b43 and the default branch will issue the WARN_ON anyway.

I guess you misread the patch.

Signature · 1 line
Greetings Michael.
#3 lore raw

Happy New Year, Michael!

:)

Ehud

Quoted 46 lines · Michael Buesch
Quoted 31 lines · On Wednesday 02 January 2008 19:52:08 Larry Finger wrote:
Quoted 29 lines · Michael Buesch

This patch fixes the parsing of the RX data header channel field.

The current code parses the header incorrectly and passes a wrong channel number and frequency for each frame to mac80211. The FIXMEs added by this patch don't matter for now as the code where they live won't get executed anyway. They will be fixed later.

Signed-off-by Michael Buesch <[email protected]>

---

John, as this is a bugfix, it should go into 2.6.24 if still possible.

Index: wireless-2.6/drivers/net/wireless/b43/xmit.c
===================================================================

drivers/net/wireless/b43/xmit.c +1 -7
@@ -549,21 +549,32 @@ void b43_rx(struct b43_wldev *dev, struc
 	switch (chanstat & B43_RX_CHAN_PHYTYPE) {
 	case B43_PHYTYPE_A:
 		status.phymode = MODE_IEEE80211A;
-		status.freq = chanid;
-		status.channel = b43_freq_to_channel_a(chanid);
-		break;
-	case B43_PHYTYPE_B:
-		status.phymode = MODE_IEEE80211B;
-		status.freq = chanid + 2400;
-		status.channel = b43_freq_to_channel_bg(chanid + 2400);
+		B43_WARN_ON(1);
Quoted 9 lines

drivers/net/wireless/b43/xmit.c void b43_rx(struct b43_wldev *dev, struc · line 579

Quoted 6 lines
drivers/net/wireless/b43/xmit.c +4 -0
+		/* FIXME: We don't really know which value the "chanid" contains.
+		 *        So the following assignment might be wrong. */
+		status.channel = chanid;
+		status.freq = b43_channel_to_freq_5ghz(status.channel);
 		break;

Shouldn't you just drop this case? No B PHY devices will ever use b43 and the default branch will issue the WARN_ON anyway.

I guess you misread the patch.

author
#4 lore raw
Quoted 3 lines · On Wednesday 02 January 2008 20:37:41 Ehud Gavron wrote:

Happy New Year, Michael!

:)

Yeah, thanks. :)

Happy New Year to everyone on the list.
Let's work on making The Linux Wireless Year (tm) out of it.

Signature · 1 line
Greetings Michael.
#5 lore raw
In reply to Michael Buesch
Quoted 5 lines · Michael Buesch
Quoted 2 lines

Shouldn't you just drop this case? No B PHY devices will ever use b43 and the default branch will issue the WARN_ON anyway.

I guess you misread the patch.

I did indeed. Sorry.

Larry