Skip to contentSkip to conversation

Fix the bcm43xx driver breakage in 2.6.24/25.

PATCH In review Alexey Zaytsev 6 participants 11 messages Feb 23, 2008 – Feb 29, 2008

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 Fix the bcm43xx driver breakage in 2.6.24/25.

[resent with lkml added to cc, sorry]

Hello, gentlemen.

Theese two patches fix a problem instroduced in Linux 2.6.24, and still present in the current Linus' 2.6.25 tree.

The bcm43xx Wifi driver won't work any more, if the b44 Ethernet driver is enabled. This happens because the b44 driver needlessly enables the b43_pci_bridge code, which claims the same pci ids as the bcm43xx driver. The b43_pci_bridge code is needed for the b43{legacy} drivers, but for the b44, only the "ssb pci core" is needed.

The first patch separates the ssb b43 pci bridge and the ssb pci core config options and enables only the needed ones.

A similar patch was once sent to Michael Buesch, but he rejected it, fearing that changing the config autoselects cold introduce some unexpected bugs.
If you are intrested, a rather emotional thread may be found e.g. here:
http://thread.gmane.org/gmane.linux.kernel/644266

That said, I believe the patch may introduce no more bugs, but fixes a critical regression, which left some users without a working wifi driver on the 2.6.24 kernel.

If you doubt its correctness, I may write a longer description on the changes introduced, but I hope they should be obvoius.

This patch is againts the current Linus' tree. I'd like it to be added to both 2.6.25 and the 2.6.24-stable trees.

The second patch disables the bcm43xx driver if it can't work due to the b43/b43legacy drivers inclusion, so no users should be confused.

PS: Sorry, I'm a bit tierd of trying to get git-send-patch to work with the gmail smtp server, so the patches were went with the thunderbird. If they end up being correpted, I've added them as attachments.

-----

From: Alexey Zaytsev <[email protected]>
Date: Sat, 23 Feb 2008 12:59:26 +0300
Subject: [PATCH] Use a separate config option for the b43 pci to ssb bridge.

The bridge code was unnecessary enabled by the b44 driver, but it prevents the bcm43xx driver from being loaded, as the bridge claims the same pci ids.

Now we enable the birdge only if the b43{legacy} drivers are selected.

---

drivers/net/wireless/b43/Kconfig       |    1 +
drivers/net/wireless/b43legacy/Kconfig |    1 +
drivers/ssb/Kconfig                    |    5 +++++
drivers/ssb/Makefile                   |    2 +-
drivers/ssb/ssb_private.h              |    4 ++--
5 files changed, 10 insertions(+), 3 deletions(-)
drivers/net/wireless/b43/Kconfig +0 -0
index 1a2141d..8bc4bc4 100644
@@ -32,6 +32,7 @@ config B43_PCI_AUTOSELECT
	bool
	depends on B43 && SSB_PCIHOST_POSSIBLE
	select SSB_PCIHOST

+ select SSB_B43_PCI_BRIDGE
default y

# Auto-select SSB PCICORE driver, if possible

drivers/net/wireless/b43legacy/Kconfig +0 -0
index 6745579..13c65fa 100644
@@ -25,6 +25,7 @@ config B43LEGACY_PCI_AUTOSELECT
	bool
	depends on B43LEGACY && SSB_PCIHOST_POSSIBLE
	select SSB_PCIHOST

+ select SSB_B43_PCI_BRIDGE
default y

# Auto-select SSB PCICORE driver, if possible

drivers/ssb/Kconfig +0 -0
index d976660..6f627f3 100644
@@ -35,6 +35,11 @@ config SSB_PCIHOST
 

If unsure, say Y

+config SSB_B43_PCI_BRIDGE
+ bool
+ depends on SSB_PCIHOST
+ default n
+
config SSB_PCMCIAHOST_POSSIBLE

	bool
	depends on SSB && (PCMCIA = y || PCMCIA = SSB) && EXPERIMENTAL
drivers/ssb/Makefile +0 -0
index 7be3975..1bf35f0 100644
@@ -13,6 +13,6 @@ ssb-$(CONFIG_SSB_DRIVER_PCICORE)	+= driver_pcicore.o
 
# b43 pci-ssb-bridge driver
# Not strictly a part of SSB, but kept here for convenience
-ssb-$(CONFIG_SSB_PCIHOST)		+= b43_pci_bridge.o
+ssb-$(CONFIG_SSB_B43_PCI_BRIDGE)	+= b43_pci_bridge.o

obj-$(CONFIG_SSB) += ssb.o

drivers/ssb/ssb_private.h +0 -0
index a789364..21eca2b 100644
@@ -120,10 +120,10 @@ extern int ssb_devices_thaw(struct ssb_bus *bus);

extern struct ssb_bus *ssb_pci_dev_to_bus(struct pci_dev *pdev);

/* b43_pci_bridge.c */
-#ifdef CONFIG_SSB_PCIHOST
+#ifdef CONFIG_SSB_B43_PCI_BRIDGE
extern int __init b43_pci_ssb_bridge_init(void);
extern void __exit b43_pci_ssb_bridge_exit(void);
-#else /* CONFIG_SSB_PCIHOST */
+#else /* CONFIG_SSB_B43_PCI_BRIDGR */
static inline int b43_pci_ssb_bridge_init(void)

{
	return 0;
Signature · 15 lines
1.5.2.GIT


From: Alexey Zaytsev <[email protected]>
Date: Sat, 23 Feb 2008 13:33:07 +0300
Subject: [PATCH] Don't build the bcm43xx driver if SSB is static and the b43 PCI-SSB bridge code is enabled.

This may happen e.g. when the ssb is statically enables by the b44 driver,
and the b43 pci-ssb bridge is enbled by the b43/b43legacy drivers, or the
b43/b43legacy drivers abe built statically.

Signed-off-by: Alexey Zaytsev <[email protected]>
---
drivers/net/wireless/bcm43xx/Kconfig |    2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
drivers/net/wireless/bcm43xx/Kconfig +0 -0
index 0159701..afb8f43 100644
@@ -1,6 +1,6 @@

config BCM43XX
tristate "Broadcom BCM43xx wireless support (DEPRECATED)"
- depends on PCI && IEEE80211 && IEEE80211_SOFTMAC && WLAN_80211 && EXPERIMENTAL
+ depends on PCI && IEEE80211 && IEEE80211_SOFTMAC && WLAN_80211 && (!SSB_B43_PCI_BRIDGE || SSB != y) && EXPERIMENTAL
select WIRELESS_EXT
select FW_LOADER
select HW_RANDOM

Signature · 1 line
1.5.2.GIT
Quoted 1 line

From edddbb1a3fdd60eba64869a2866031817d183b7d Mon Sep 17 00:00:00 2001

From: Alexey Zaytsev <[email protected]>
Date: Sat, 23 Feb 2008 12:59:26 +0300
Subject: [PATCH] Use a separate config option for the b43 pci to ssb bridge.

The bridge code was unnecessary enabled by the b44 driver, but it prevents the bcm43xx driver from being loaded, as the bridge claims the same pci ids.

Now we enable the birdge only if the b43{legacy} drivers are selected.

---
 drivers/net/wireless/b43/Kconfig       |    1 +
 drivers/net/wireless/b43legacy/Kconfig |    1 +
 drivers/ssb/Kconfig                    |    5 +++++
 drivers/ssb/Makefile                   |    2 +-
 drivers/ssb/ssb_private.h              |    4 ++--
 5 files changed, 10 insertions(+), 3 deletions(-)
drivers/net/wireless/b43/Kconfig +1 -0
index 1a2141d..8bc4bc4 100644
@@ -32,6 +32,7 @@ config B43_PCI_AUTOSELECT
 	bool
 	depends on B43 && SSB_PCIHOST_POSSIBLE
 	select SSB_PCIHOST
+	select SSB_B43_PCI_BRIDGE
 	default y
 
 # Auto-select SSB PCICORE driver, if possible
drivers/net/wireless/b43legacy/Kconfig +1 -0
index 6745579..13c65fa 100644
@@ -25,6 +25,7 @@ config B43LEGACY_PCI_AUTOSELECT
 	bool
 	depends on B43LEGACY && SSB_PCIHOST_POSSIBLE
 	select SSB_PCIHOST
+	select SSB_B43_PCI_BRIDGE
 	default y
 
 # Auto-select SSB PCICORE driver, if possible
drivers/ssb/Kconfig +5 -0
index d976660..6f627f3 100644
@@ -35,6 +35,11 @@ config SSB_PCIHOST
 
 	  If unsure, say Y
 
+config SSB_B43_PCI_BRIDGE
+	bool
+	depends on SSB_PCIHOST
+	default n
+
 config SSB_PCMCIAHOST_POSSIBLE
 	bool
 	depends on SSB && (PCMCIA = y || PCMCIA = SSB) && EXPERIMENTAL
drivers/ssb/Makefile +1 -1
index 7be3975..1bf35f0 100644
@@ -13,6 +13,6 @@ ssb-$(CONFIG_SSB_DRIVER_PCICORE)	+= driver_pcicore.o
 
 # b43 pci-ssb-bridge driver
 # Not strictly a part of SSB, but kept here for convenience
-ssb-$(CONFIG_SSB_PCIHOST)		+= b43_pci_bridge.o
+ssb-$(CONFIG_SSB_B43_PCI_BRIDGE)	+= b43_pci_bridge.o
 
 obj-$(CONFIG_SSB)			+= ssb.o
drivers/ssb/ssb_private.h +2 -2
index a789364..21eca2b 100644
@@ -120,10 +120,10 @@ extern int ssb_devices_thaw(struct ssb_bus *bus);
 extern struct ssb_bus *ssb_pci_dev_to_bus(struct pci_dev *pdev);
 
 /* b43_pci_bridge.c */
-#ifdef CONFIG_SSB_PCIHOST
+#ifdef CONFIG_SSB_B43_PCI_BRIDGE
 extern int __init b43_pci_ssb_bridge_init(void);
 extern void __exit b43_pci_ssb_bridge_exit(void);
-#else /* CONFIG_SSB_PCIHOST */
+#else /* CONFIG_SSB_B43_PCI_BRIDGR */
 static inline int b43_pci_ssb_bridge_init(void)
 {
 	return 0;
Signature · 1 line
1.5.2.GIT
Quoted 1 line

From c62c389961166fc0813a54ee5ae0e2cc0d61cd0e Mon Sep 17 00:00:00 2001

From: Alexey Zaytsev <[email protected]>
Date: Sat, 23 Feb 2008 13:33:07 +0300
Subject: [PATCH] Don't build the bcm43xx driver if SSB is static and the b43 PCI-SSB bridge code is enabled.

This may happen e.g. when the ssb is statically enables by the b44 driver, and the b43 pci-ssb bridge is enbled by the b43/b43legacy drivers, or the b43/b43legacy drivers abe built statically.

---
 drivers/net/wireless/bcm43xx/Kconfig |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
drivers/net/wireless/bcm43xx/Kconfig +1 -1
index 0159701..afb8f43 100644
@@ -1,6 +1,6 @@
 config BCM43XX
 	tristate "Broadcom BCM43xx wireless support (DEPRECATED)"
-	depends on PCI && IEEE80211 && IEEE80211_SOFTMAC && WLAN_80211 && EXPERIMENTAL
+	depends on PCI && IEEE80211 && IEEE80211_SOFTMAC && WLAN_80211 && (!SSB_B43_PCI_BRIDGE || SSB != y) && EXPERIMENTAL
 	select WIRELESS_EXT
 	select FW_LOADER
 	select HW_RANDOM
Signature · 1 line
1.5.2.GIT
#1 lore raw

drivers/net/wireless/bcm43xx/Kconfig line 3

Quoted 9 lines · On Saturday 23 February 2008 14:17:55 Alexey Zaytsev wrote:
drivers/net/wireless/bcm43xx/Kconfig +1 -1
index 0159701..afb8f43 100644
@@ -1,6 +1,6 @@
 config BCM43XX
 	tristate "Broadcom BCM43xx wireless support (DEPRECATED)"
-	depends on PCI && IEEE80211 && IEEE80211_SOFTMAC && WLAN_80211 && EXPERIMENTAL
+	depends on PCI && IEEE80211 && IEEE80211_SOFTMAC && WLAN_80211 && (!SSB_B43_PCI_BRIDGE || SSB != y) && EXPERIMENTAL

so if SSB is m it will break module auto-loading, right?

Signature · 1 line
Greetings Michael.
#2 lore raw
Quoted 13 lines · Michael Buesch

drivers/net/wireless/bcm43xx/Kconfig line 3

Quoted 9 lines · On Saturday 23 February 2008 14:17:55 Alexey Zaytsev wrote:
drivers/net/wireless/bcm43xx/Kconfig +1 -1
index 0159701..afb8f43 100644
@@ -1,6 +1,6 @@
 config BCM43XX
      tristate "Broadcom BCM43xx wireless support (DEPRECATED)"
-     depends on PCI && IEEE80211 && IEEE80211_SOFTMAC && WLAN_80211 && EXPERIMENTAL
+     depends on PCI && IEEE80211 && IEEE80211_SOFTMAC && WLAN_80211 && (!SSB_B43_PCI_BRIDGE || SSB != y) && EXPERIMENTAL

so if SSB is m it will break module auto-loading, right?

Right.
If ssb == m, you can at least unload the ssb module manually, or specify the right
module to load. In the first patch I sent to you, it was "!SSB_B43_PCI_BRIDGE" instead of
"(!SSB_B43_PCI_BRIDGE || SSB != y)", but I thought that there is no reason to completely hide the bcm43xx option if ssb is compiled as module.

Acked-by on v1 1/1
#3 lore raw
In reply to Alexey Zaytsev
Quoted 32 lines · Alexey Zaytsev

[resent with lkml added to cc, sorry]

Hello, gentlemen.

Theese two patches fix a problem instroduced in Linux 2.6.24, and still present in the current Linus' 2.6.25 tree.

The bcm43xx Wifi driver won't work any more, if the b44 Ethernet driver is enabled. This happens because the b44 driver needlessly enables the b43_pci_bridge code, which claims the same pci ids as the bcm43xx driver. The b43_pci_bridge code is needed for the b43{legacy} drivers, but for the b44, only the "ssb pci core" is needed.

The first patch separates the ssb b43 pci bridge and the ssb pci core config options and enables only the needed ones.

A similar patch was once sent to Michael Buesch, but he rejected it, fearing that changing the config autoselects cold introduce some unexpected bugs.
If you are intrested, a rather emotional thread may be found e.g. here:
http://thread.gmane.org/gmane.linux.kernel/644266

That said, I believe the patch may introduce no more bugs, but fixes a critical regression, which left some users without a working wifi driver on the 2.6.24 kernel.

If you doubt its correctness, I may write a longer description on the changes introduced, but I hope they should be obvoius.

Quoted 6 lines

This patch is againts the current Linus' tree. I'd like it to be added to both 2.6.25 and the 2.6.24-stable trees.

The second patch disables the bcm43xx driver if it can't work due to the b43/b43legacy drivers inclusion, so no users should be confused.

I have reviewed the above patches and tested them with what I believe to be most combinations. Everything worked and corrected the unintended consequence of breaking bcm43xx when b44 is included. I agree that these patches belong in 2.6.25 and stable (2.6.24). Of course, the problem goes away in 2.6.26.

Larry

#4 lore raw
Quoted 5 lines · Larry Finger

I have reviewed the above patches and tested them with what I believe to be most combinations. Everything worked and corrected the unintended consequence of breaking bcm43xx when b44 is included. I agree that these patches belong in 2.6.25 and stable (2.6.24). Of course, the problem goes away in 2.6.26.

I can't say I really like Alexey's patches, but I guess they are OK. I think I like the "b43_pci_bridge as a module" option a little better, but as Michael illustrated there are problems with that approach too.

I'll apply the patches.

John

Signature · 2 lines
John W. Linville
[email protected]
#5 lore raw
Quoted 11 lines · On Wednesday 27 February 2008 21:15:50 John W. Linville wrote:
Quoted 5 lines · Larry Finger

I have reviewed the above patches and tested them with what I believe to be most combinations. Everything worked and corrected the unintended consequence of breaking bcm43xx when b44 is included. I agree that these patches belong in 2.6.25 and stable (2.6.24). Of course, the problem goes away in 2.6.26.

I can't say I really like Alexey's patches, but I guess they are OK. I think I like the "b43_pci_bridge as a module" option a little better, but as Michael illustrated there are problems with that approach too.

Well, I do like "b43_pci_bridge as a module" more than this inter-dependency crap. Yeah, I don't like "b43_pci_bridge as a module" a lot, but certainly more than Alexey's patch.

Signature · 1 line
Greetings Michael.
#6 lore raw
Quoted 9 lines · Michael Buesch
Quoted 8 lines · On Wednesday 27 February 2008 21:15:50 John W. Linville wrote:
Quoted 5 lines · Larry Finger

I have reviewed the above patches and tested them with what I believe to be most combinations. Everything worked and corrected the unintended consequence of breaking bcm43xx when b44 is included. I agree that these patches belong in 2.6.25 and stable (2.6.24). Of course, the problem goes away in 2.6.26.

Quoted 7 lines
Quoted 3 lines

I can't say I really like Alexey's patches, but I guess they are OK. I think I like the "b43_pci_bridge as a module" option a little better, but as Michael illustrated there are problems with that approach too.

Well, I do like "b43_pci_bridge as a module" more than this inter-dependency crap. Yeah, I don't like "b43_pci_bridge as a module" a lot, but certainly more than Alexey's patch.

Patches welcome... :-)

Signature · 2 lines
John W. Linville
[email protected]
#7 lore raw
In reply to Alexey Zaytsev
Quoted 9 lines · On Sat, 23 Feb 2008 16:17:55 +0300 Alexey Zaytsev <[email protected]>…

Theese two patches fix a problem instroduced in Linux 2.6.24, and still present in the current Linus' 2.6.25 tree.

The bcm43xx Wifi driver won't work any more, if the b44 Ethernet driver is enabled. This happens because the b44 driver needlessly enables the b43_pci_bridge code, which claims the same pci ids as the bcm43xx driver. The b43_pci_bridge code is needed for the b43{legacy} drivers, but for the b44, only the "ssb pci core" is needed.

Hate to stick my head into this beehive, but...

I have a patch queued (the-scheduled-bcm43xx-removal.patch) which will remove this driver from 2.6.26.

Is that bad?

#8 lore raw
Quoted 12 lines · On Friday 29 February 2008 09:21:19 Andrew Morton wrote:
Quoted 9 lines · On Sat, 23 Feb 2008 16:17:55 +0300 Alexey Zaytsev <[email protected]>…

Theese two patches fix a problem instroduced in Linux 2.6.24, and still present in the current Linus' 2.6.25 tree.

The bcm43xx Wifi driver won't work any more, if the b44 Ethernet driver is enabled. This happens because the b44 driver needlessly enables the b43_pci_bridge code, which claims the same pci ids as the bcm43xx driver. The b43_pci_bridge code is needed for the b43{legacy} drivers, but for the b44, only the "ssb pci core" is needed.

Quoted 6 lines

Hate to stick my head into this beehive, but...

I have a patch queued (the-scheduled-bcm43xx-removal.patch) which will remove this driver from 2.6.26.

Is that bad?

Not at all. The b43 driver from 2.6.26 will work. So there's no reason to keep bcm43xx.
Alexey just tested a patch that backports the fix to 2.6.24 and it works fine.

Signature · 1 line
Greetings Michael.
#9 lore raw
Quoted 15 lines · Michael Buesch
Quoted 14 lines · On Friday 29 February 2008 09:21:19 Andrew Morton wrote:
Quoted 9 lines · On Sat, 23 Feb 2008 16:17:55 +0300 Alexey Zaytsev <[email protected]>…

Theese two patches fix a problem instroduced in Linux 2.6.24, and still present in the current Linus' 2.6.25 tree.

The bcm43xx Wifi driver won't work any more, if the b44 Ethernet driver is enabled. This happens because the b44 driver needlessly enables the b43_pci_bridge code, which claims the same pci ids as the bcm43xx driver. The b43_pci_bridge code is needed for the b43{legacy} drivers, but for the b44, only the "ssb pci core" is needed.

Hate to stick my head into this beehive, but...

Quoted 8 lines
Quoted 4 lines

I have a patch queued (the-scheduled-bcm43xx-removal.patch) which will remove this driver from 2.6.26.

Is that bad?

Not at all. The b43 driver from 2.6.26 will work. So there's no reason to keep bcm43xx. Alexey just tested a patch that backports the fix to 2.6.24 and it works fine.

yeah, i'd support the removal of it too, if the transition is smooth and the regressions are resolved. And it should be smooth, as long as users are able to load the firmware blob, right? There's also an entry for it in Documentation/feature-removal-schedule.txt:

  What:   bcm43xx wireless network driver
  When:   2.6.26
  Files:  drivers/net/wireless/bcm43xx
  Why:    This driver's functionality has been replaced by the
          mac80211-based b43 and b43legacy drivers.

Who: John W. Linville <[email protected]>

Ingo

#10 lore raw
Quoted 23 lines · On Friday 29 February 2008 21:54:00 Ingo Molnar wrote:
Quoted 20 lines · Michael Buesch
Quoted 18 lines · On Friday 29 February 2008 09:21:19 Andrew Morton wrote:
Quoted 9 lines · On Sat, 23 Feb 2008 16:17:55 +0300 Alexey Zaytsev <[email protected]>…

Theese two patches fix a problem instroduced in Linux 2.6.24, and still present in the current Linus' 2.6.25 tree.

The bcm43xx Wifi driver won't work any more, if the b44 Ethernet driver is enabled. This happens because the b44 driver needlessly enables the b43_pci_bridge code, which claims the same pci ids as the bcm43xx driver. The b43_pci_bridge code is needed for the b43{legacy} drivers, but for the b44, only the "ssb pci core" is needed.

Hate to stick my head into this beehive, but...

I have a patch queued (the-scheduled-bcm43xx-removal.patch) which will remove this driver from 2.6.26.

Is that bad?

Quoted 7 lines
Quoted 3 lines

Not at all. The b43 driver from 2.6.26 will work. So there's no reason to keep bcm43xx. Alexey just tested a patch that backports the fix to 2.6.24 and it works fine.

yeah, i'd support the removal of it too, if the transition is smooth and the regressions are resolved. And it should be smooth, as long as users are able to load the firmware blob, right? There's also an entry for it

As far as I can see, yes. The b43 driver is pretty mature these days. It does base on the codebase of the bcm43xx driver and the most intrusive changes date back over a year ago. The driver got lots of testing in the meantime.
I do own the most common device flavours (except the 4311. So if somebody has one do donate.. :) ) and I don't see any regressions.

Most "regressions" come from people not installing the updated firmware. But that's an issue we simply cannot solve in the driver.

Of course, as I always said, if some last-minute regression appears, we will defer the bcm43xx removal. So I hope I'm not _really_ the worst maintainer ever, although I try hard to, when it comes to patches I really dislike. ;)

Signature · 1 line
Greetings Michael.