bcm43xx regression in 2.6.24 (with patch)

bcm43xx regression in 2.6.24 (with patch)

[Alexey Zaytsev]

Hello.

The bcm43xx 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.

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


Greg, how do you think, should this do a -stable material?
If the line number limit really matters, I could remove the
comments in the b43{legacy} Kconfigs. ;)


PS:
Yes, the bcm43xx driver is deprecated, but the b43 driver did
not work for me right out of the box (firmware updated).
Please don't remove the bc43xx driver until there are really no
problems with the new one.

I'll try the new driver once again this weekend, and will send
a bug report when I'll be sure it is not a stupid mistake.

---

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.

Signed-off-by: Alexey Zaytsev <alexey.zaytsev@gmail.com>
---
 drivers/net/wireless/b43/Kconfig       |    4 ++++
 drivers/net/wireless/b43legacy/Kconfig |    4 ++++
 drivers/net/wireless/bcm43xx/Kconfig   |    2 +-
 drivers/ssb/Kconfig                    |    5 +++++
 drivers/ssb/Makefile                   |    2 +-
 drivers/ssb/ssb_private.h              |    4 ++--
 6 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/b43/Kconfig b/drivers/net/wireless/b43/Kconfig
index fdbc351..8b7043a 100644
--- a/drivers/net/wireless/b43/Kconfig
+++ b/drivers/net/wireless/b43/Kconfig
@@ -24,6 +24,9 @@ config B43
          This driver uses V4 firmware, which must be installed separately using
          b43-fwcutter.

+         Building this driver prevents the now deprecated bcm43xx driver from being
+         built.
+
          This driver can be built as a module (recommended) that will be called "b43".
          If unsure, say M.

@@ -32,6 +35,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
diff --git a/drivers/net/wireless/b43legacy/Kconfig b/drivers/net/wireless/b43legacy/Kconfig
index 7e23ec2..48f8e7d 100644
--- a/drivers/net/wireless/b43legacy/Kconfig
+++ b/drivers/net/wireless/b43legacy/Kconfig
@@ -17,6 +17,9 @@ config B43LEGACY
          This driver uses V3 firmware, which must be installed separately using
          b43-fwcutter.

+         Building this driver prevents the now deprecated bcm43xx driver from being
+         built.
+
          This driver can be built as a module (recommended) that will be
          called "b43legacy". If unsure, say M.

@@ -25,6 +28,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
diff --git a/drivers/net/wireless/bcm43xx/Kconfig b/drivers/net/wireless/bcm43xx/Kconfig
index ce397e4..32068d8 100644
--- a/drivers/net/wireless/bcm43xx/Kconfig
+++ b/drivers/net/wireless/bcm43xx/Kconfig
@@ -1,6 +1,6 @@
 config BCM43XX
        tristate "Broadcom BCM43xx wireless support"
-       depends on PCI && IEEE80211 && IEEE80211_SOFTMAC && WLAN_80211 && EXPERIMENTAL
+       depends on PCI && IEEE80211 && IEEE80211_SOFTMAC && WLAN_80211 && !SSB_B43_PCI_BRIDGE && EXPERIMENTAL
        select WIRELESS_EXT
        select FW_LOADER
        select HW_RANDOM
diff --git a/drivers/ssb/Kconfig b/drivers/ssb/Kconfig
index d976660..6f627f3 100644
--- a/drivers/ssb/Kconfig
+++ b/drivers/ssb/Kconfig
@@ -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
diff --git a/drivers/ssb/Makefile b/drivers/ssb/Makefile
index 7be3975..1bf35f0 100644
--- a/drivers/ssb/Makefile
+++ b/drivers/ssb/Makefile
@@ -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
diff --git a/drivers/ssb/ssb_private.h b/drivers/ssb/ssb_private.h
index a789364..21eca2b 100644
--- a/drivers/ssb/ssb_private.h
+++ b/drivers/ssb/ssb_private.h
@@ -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;

Re: bcm43xx regression in 2.6.24 (with patch)

[Michael Buesch]

On Friday 22 February 2008 12:17:15 Alexey Zaytsev wrote:
> 
> Hello.
> 
> The bcm43xx 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.
> 
> This patch separates the ssb b43 pci bridge and the ssb pci
> core config options and enables only the needed ones.

Nack. Switch to b43. bcm43xx is going to be removed anyway.
I'm not going to play these kconfig SELECT tricks anymore.
We had _lots_ of bugs there. People submitted patches that
obviously looked OK and still they turned out to break
some dependencies. KConfig SELECT is a feature from hell.

-- 
Greetings Michael.

Re: bcm43xx regression in 2.6.24 (with patch)

[Gabriel C]

Michael Buesch wrote:
> On Friday 22 February 2008 12:17:15 Alexey Zaytsev wrote:
>> Hello.
>>
>> The bcm43xx 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.
>>
>> This patch separates the ssb b43 pci bridge and the ssb pci
>> core config options and enables only the needed ones.
> 
> Nack. Switch to b43. bcm43xx is going to be removed anyway.

Since when this is a valid reason to NACK a patch which fixes 
a regression in the 'stable' kernel ? 

Probably you may want to ask first whatever 'b43' works for Alexey ?
and most probably the answer will be 'no' because that is the only reason 
one would want to fix the *old* one when *both* drivers are enabled.

Really 'Switch to foo' and 'foo_old will be removed sometimes' 
is not a 'valid' reason to NACK anything as long you 'have foo and foo_old' 
*supported* in that kernel.

> I'm not going to play these kconfig SELECT tricks anymore.

Fix it different then.

> We had _lots_ of bugs there. People submitted patches that
> obviously looked OK and still they turned out to break
> some dependencies. KConfig SELECT is a feature from hell.
> 

Yes it is but that is still not a valid reason to NACK that patch , I'm sorry.

Regards,

Gabriel

Re: bcm43xx regression in 2.6.24 (with patch)

[Michael Buesch]

On Friday 22 February 2008 18:51:32 Gabriel C wrote:
> > I'm not going to play these kconfig SELECT tricks anymore.
> 
> Fix it different then.

Please do so.

> Yes it is but that is still not a valid reason to NACK that patch , I'm sorry.

NACK means I (being the maintainer of the modified code) won't
sign-off this patch. Nothing more or less. If some upstream maintainer
decides to bypass me and apply the patch nevertheless is not of
my business. But I will not sign-off or ack this patch.

-- 
Greetings Michael.

Re: bcm43xx regression in 2.6.24 (with patch)

[Greg KH]

On Fri, Feb 22, 2008 at 06:56:14PM +0100, Michael Buesch wrote:
> On Friday 22 February 2008 18:51:32 Gabriel C wrote:
> > > I'm not going to play these kconfig SELECT tricks anymore.
> > 
> > Fix it different then.
> 
> Please do so.
> 
> > Yes it is but that is still not a valid reason to NACK that patch , I'm sorry.
> 
> NACK means I (being the maintainer of the modified code) won't
> sign-off this patch. Nothing more or less. If some upstream maintainer
> decides to bypass me and apply the patch nevertheless is not of
> my business. But I will not sign-off or ack this patch.

As the maintainer will not accept this, I'm not going to take it for the
-stable tree either, sorry.

thanks,

greg k-h

Re: bcm43xx regression in 2.6.24 (with patch)

[Alexey Zaytsev]

Michael Buesch wrote:
> On Friday 22 February 2008 12:17:15 Alexey Zaytsev wrote:
>> Hello.
>>
>> The bcm43xx 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.
>>
>> This patch separates the ssb b43 pci bridge and the ssb pci
>> core config options and enables only the needed ones.
> 
> Nack. Switch to b43. bcm43xx is going to be removed anyway.
> I'm not going to play these kconfig SELECT tricks anymore.
> We had _lots_ of bugs there. People submitted patches that
> obviously looked OK and still they turned out to break
> some dependencies. KConfig SELECT is a feature from hell.
> 

Sorry, I don't get it. You are going to remove the (somehow)
working driver, while there are known problems with the new
one? Why? To me it sounds like _breaking old but working code_
to get _more bug reports on the new code_. But, what was the
goal providing users with working drivers, or getting more
bug reports?

And please look at the problem from a user's view point. I'm
happily using the 2.6.23 kernel. The bcm43xx driver is the
only one available. And it works somehow (well, no s2ram).
Now I upgrade to the 2.6.24 kernel. I see there is a new 
b43 driver and try it.
For some reason it does not work, even after I follow the
firmware upgrading instructions. Not a big deal, I clearly
understand that you have to work with reverse-engineered
specs and even with good specs, bugs happen, no problem.
I'll try to debug it in the weekend and maybe will send a
bug report or patch.

But right now I have other business, sorry. The problem is,
now also the old (but not now marked as deprecated) driver
stops working.

If this is not a repgession, than I don't know what is.
And if it is a regression, it should be fixed at least in the
2.6.24.y series, do you agree?

I have provided a patch that I believe is trivial, that I have
tested with all possible config option combinations I thought
were possible, and that fixes the regression. If you have a
reason to believe it is wrong, please say it, I won't be
offended. If there is a problem with the patch, I'll gladly
fix and resend it. 

This Nack leaves me with one option - to switch back to 2.6.23.
Sorry, no testing, probably more bugs not found in 2.6.24. ;(

Re: bcm43xx regression in 2.6.24 (with patch)

[Michael Buesch]

On Friday 22 February 2008 19:10:39 Alexey Zaytsev wrote:
> Sorry, I don't get it. You are going to remove the (somehow)
> working driver, while there are known problems with the new
> one? Why?

Because the new driver works, if you just set it up right.
Until now every "breakage" was just a usage error.

> And please look at the problem from a user's view point. I'm
> happily using the 2.6.23 kernel. The bcm43xx driver is the
> only one available. And it works somehow (well, no s2ram).

b43 supports s2ram and s2disk.

> If this is not a repgession, than I don't know what is.
> And if it is a regression, it should be fixed at least in the
> 2.6.24.y series, do you agree?

No. Playing with kconfig SELECT is really _nothing_ for a -stable
series. I am _not_ going to be responsible for any
breakages. And using SELECT _does_ break things. We had dozens of bugs
until all the damn dependencies were right. Even "Obviously Correct (tm)"
changes to SELECT _do_ break on weird configurations.
See the bcm43xx mail archives for examples.

If you need that patch for some reason, please apply it locally.
The problem will automatically get fixed when bcm43xx goes away, soon.

> I have provided a patch that I believe is trivial, that I have
> tested with all possible config option combinations I thought
> were possible, and that fixes the regression. If you have a
> reason to believe it is wrong, please say it, I won't be
> offended. If there is a problem with the patch, I'll gladly
> fix and resend it. 
> 
> This Nack leaves me with one option - to switch back to 2.6.23.
> Sorry, no testing, probably more bugs not found in 2.6.24. ;(

It is not my problem, if you refuse to use b43.
You also still refuse to tell me details about your card and _what_
does not work. I do own lots of different card and they
all work fine with b43. There's one exception, the 4311 rev 3 (or something,
don't quite remember). But patches are available and will ship in 2.6.25.
bcm43xx won't get removed until that shipped.

-- 
Greetings Michael.

Re: bcm43xx regression in 2.6.24 (with patch)

[Alexey Zaytsev]

[replying from my non-work account]

On Fri, Feb 22, 2008 at 8:48 PM, Michael Buesch <mb@bu3sch.de> wrote:
> On Friday 22 February 2008 19:10:39 Alexey Zaytsev wrote:
>  > Sorry, I don't get it. You are going to remove the (somehow)
>  > working driver, while there are known problems with the new
>  > one? Why?
>
>  Because the new driver works, if you just set it up right.
>  Until now every "breakage" was just a usage error.
>
>
>  > And please look at the problem from a user's view point. I'm
>  > happily using the 2.6.23 kernel. The bcm43xx driver is the
>  > only one available. And it works somehow (well, no s2ram).
>
>  b43 supports s2ram and s2disk.
>
>
>  > If this is not a repgession, than I don't know what is.
>  > And if it is a regression, it should be fixed at least in the
>  > 2.6.24.y series, do you agree?
>
>  No. Playing with kconfig SELECT is really _nothing_ for a -stable
>  series. I am _not_ going to be responsible for any
>  breakages. And using SELECT _does_ break things. We had dozens of bugs
>  until all the damn dependencies were right. Even "Obviously Correct (tm)"
>  changes to SELECT _do_ break on weird configurations.
>  See the bcm43xx mail archives for examples.
>

Well, someone obviously played with the SELECTs, and broke the bcm43xx driver.
Now I've got a patch that I believe gets the SELECTs right and fixes
the driver, so
what is wrong?

>  If you need that patch for some reason, please apply it locally.
>  The problem will automatically get fixed when bcm43xx goes away, soon.
>
>
>  > I have provided a patch that I believe is trivial, that I have
>  > tested with all possible config option combinations I thought
>  > were possible, and that fixes the regression. If you have a
>  > reason to believe it is wrong, please say it, I won't be
>  > offended. If there is a problem with the patch, I'll gladly
>  > fix and resend it.
>  >
>  > This Nack leaves me with one option - to switch back to 2.6.23.
>  > Sorry, no testing, probably more bugs not found in 2.6.24. ;(
>
>  It is not my problem, if you refuse to use b43.
>  You also still refuse to tell me details about your card and _what_
>  does not work. I do own lots of different card and they
>  all work fine with b43. There's one exception, the 4311 rev 3 (or something,
>  don't quite remember). But patches are available and will ship in 2.6.25.
>  bcm43xx won't get removed until that shipped.

Yes, it's a 4311 rev 01, but I'm probably was just too lame to upgrade the
firmware or something. :E

I really don't get it, what is going on here? You state that the new b32 driver
has problems on some hardware, where the old bcm43xx driver just works.
And at the same time, you are surprised that I "refuse" to use the b43 driver
and push patches for the bcm43xx driver you broke... Oh, really, why?!

Re: bcm43xx regression in 2.6.24 (with patch)

[Michael Buesch]

On Friday 22 February 2008 21:06:00 Alexey Zaytsev wrote:
> >  It is not my problem, if you refuse to use b43.
> >  You also still refuse to tell me details about your card and _what_
> >  does not work. I do own lots of different card and they
> >  all work fine with b43. There's one exception, the 4311 rev 3 (or something,
> >  don't quite remember). But patches are available and will ship in 2.6.25.
> >  bcm43xx won't get removed until that shipped.
> 
> Yes, it's a 4311 rev 01, but I'm probably was just too lame to upgrade the
> firmware or something. :E
> 
> I really don't get it, what is going on here? You state that the new b32 driver
> has problems on some hardware, where the old bcm43xx driver just works.
> And at the same time, you are surprised that I "refuse" to use the b43 driver
> and push patches for the bcm43xx driver you broke... Oh, really, why?!

So, please find someone who will sign-off your patch. I won't.
What's so hard to understand about that? Do I _have_ to sign off all patches
random people send to me?
I do _not_ want to be made responsible for that patch by signing it off.
It is as simple as that.
And I officially do not care about bcm43xx since a year and a half anymore.
So why should I ACK it or sign it off?

-- 
Greetings Michael.

Re: bcm43xx regression in 2.6.24 (with patch)

[Alexey Zaytsev]

On Fri, Feb 22, 2008 at 11:12 PM, Michael Buesch <mb@bu3sch.de> wrote:
> On Friday 22 February 2008 21:06:00 Alexey Zaytsev wrote:
>  > >  It is not my problem, if you refuse to use b43.
>  > >  You also still refuse to tell me details about your card and _what_
>  > >  does not work. I do own lots of different card and they
>  > >  all work fine with b43. There's one exception, the 4311 rev 3 (or something,
>  > >  don't quite remember). But patches are available and will ship in 2.6.25.
>  > >  bcm43xx won't get removed until that shipped.
>  >
>  > Yes, it's a 4311 rev 01, but I'm probably was just too lame to upgrade the
>  > firmware or something. :E
>  >
>  > I really don't get it, what is going on here? You state that the new b32 driver
>  > has problems on some hardware, where the old bcm43xx driver just works.
>  > And at the same time, you are surprised that I "refuse" to use the b43 driver
>  > and push patches for the bcm43xx driver you broke... Oh, really, why?!
>
>  So, please find someone who will sign-off your patch. I won't.
>  What's so hard to understand about that? Do I _have_ to sign off all patches
>  random people send to me?
>  I do _not_ want to be made responsible for that patch by signing it off.
>  It is as simple as that.
>  And I officially do not care about bcm43xx since a year and a half anymore.
>  So why should I ACK it or sign it off?
>

I thought that there was a rule that if you break something in the kernel, you
normally would be the one who fixes things up. Sorry, it looks I was wrong.

I'll resend the patch directly to Greg KH and Jeff Garzik for -stable and 2.6.25
inclusion.

Re: bcm43xx regression in 2.6.24 (with patch)

[Greg KH]

On Fri, Feb 22, 2008 at 11:38:34PM +0300, Alexey Zaytsev wrote:
> On Fri, Feb 22, 2008 at 11:12 PM, Michael Buesch <mb@bu3sch.de> wrote:
> > On Friday 22 February 2008 21:06:00 Alexey Zaytsev wrote:
> >  > >  It is not my problem, if you refuse to use b43.
> >  > >  You also still refuse to tell me details about your card and _what_
> >  > >  does not work. I do own lots of different card and they
> >  > >  all work fine with b43. There's one exception, the 4311 rev 3 (or something,
> >  > >  don't quite remember). But patches are available and will ship in 2.6.25.
> >  > >  bcm43xx won't get removed until that shipped.
> >  >
> >  > Yes, it's a 4311 rev 01, but I'm probably was just too lame to upgrade the
> >  > firmware or something. :E
> >  >
> >  > I really don't get it, what is going on here? You state that the new b32 driver
> >  > has problems on some hardware, where the old bcm43xx driver just works.
> >  > And at the same time, you are surprised that I "refuse" to use the b43 driver
> >  > and push patches for the bcm43xx driver you broke... Oh, really, why?!
> >
> >  So, please find someone who will sign-off your patch. I won't.
> >  What's so hard to understand about that? Do I _have_ to sign off all patches
> >  random people send to me?
> >  I do _not_ want to be made responsible for that patch by signing it off.
> >  It is as simple as that.
> >  And I officially do not care about bcm43xx since a year and a half anymore.
> >  So why should I ACK it or sign it off?
> >
> 
> I thought that there was a rule that if you break something in the kernel, you
> normally would be the one who fixes things up. Sorry, it looks I was wrong.
> 
> I'll resend the patch directly to Greg KH and Jeff Garzik for -stable and 2.6.25
> inclusion.

One of the rules for the -stable tree is that the patch is accepted by
the maintainer and it is already in the upstream kernel version.  Both
of these requirements do not seem to be met here.

I suggest you work _with_ Michael instead of against him to try to
determine why your device doesn't work properly with the recommended
driver.

thanks,

greg k-h

Re: bcm43xx regression in 2.6.24 (with patch)

[Alexey Zaytsev]

On Sat, Feb 23, 2008 at 1:12 AM, Greg KH <greg@kroah.com> wrote:
>
> On Fri, Feb 22, 2008 at 11:38:34PM +0300, Alexey Zaytsev wrote:
>  > On Fri, Feb 22, 2008 at 11:12 PM, Michael Buesch <mb@bu3sch.de> wrote:
>  > > On Friday 22 February 2008 21:06:00 Alexey Zaytsev wrote:
>  > >  > >  It is not my problem, if you refuse to use b43.
>  > >  > >  You also still refuse to tell me details about your card and _what_
>  > >  > >  does not work. I do own lots of different card and they
>  > >  > >  all work fine with b43. There's one exception, the 4311 rev 3 (or something,
>  > >  > >  don't quite remember). But patches are available and will ship in 2.6.25.
>  > >  > >  bcm43xx won't get removed until that shipped.
>  > >  >
>  > >  > Yes, it's a 4311 rev 01, but I'm probably was just too lame to upgrade the
>  > >  > firmware or something. :E
>  > >  >
>  > >  > I really don't get it, what is going on here? You state that the new b32 driver
>  > >  > has problems on some hardware, where the old bcm43xx driver just works.
>  > >  > And at the same time, you are surprised that I "refuse" to use the b43 driver
>  > >  > and push patches for the bcm43xx driver you broke... Oh, really, why?!
>  > >
>  > >  So, please find someone who will sign-off your patch. I won't.
>  > >  What's so hard to understand about that? Do I _have_ to sign off all patches
>  > >  random people send to me?
>  > >  I do _not_ want to be made responsible for that patch by signing it off.
>  > >  It is as simple as that.
>  > >  And I officially do not care about bcm43xx since a year and a half anymore.
>  > >  So why should I ACK it or sign it off?
>  > >
>  >
>  > I thought that there was a rule that if you break something in the kernel, you
>  > normally would be the one who fixes things up. Sorry, it looks I was wrong.
>  >
>  > I'll resend the patch directly to Greg KH and Jeff Garzik for -stable and 2.6.25
>  > inclusion.
>
>  One of the rules for the -stable tree is that the patch is accepted by
>  the maintainer and it is already in the upstream kernel version.  Both
>  of these requirements do not seem to be met here.

Well, it looks like Michael is not the bcm43xx maintaner. I sent the
patch to him,
because it was his code that broke the driver, and I thought it would
be easy for him to review my patch, as it touches his code.

I'll resend the patch to the proper maintainers now.

>
>  I suggest you work _with_ Michael instead of against him to try to
>  determine why your device doesn't work properly with the recommended
>  driver.

Well, if you read my first email, that is exactly what I intended to
do, but even if
Michael would be able to fix the b43 driver to work with my hardware, the code
will only show up in 2.6.25, leaving some 2.6.24 users with broken
wifi. So I thought
it would be a good thing to add my simple fix that enabled the old driver to the
-stable tree, so that we could have working wifi soon.

This is still my intension, I'll resend to the proper maintainters.

>
>  thanks,
>
>  greg k-h
>

Re: bcm43xx regression in 2.6.24 (with patch)

[Michael Buesch]

On Friday 22 February 2008, Alexey Zaytsev wrote:
> Well, it looks like Michael is not the bcm43xx maintaner. I sent the
> patch to him,
> because it was his code that broke the driver, and I thought it would
> be easy for him to review my patch, as it touches his code.

See? I'm tired of this "how dare can you break my kernel!?" bullshit.
That's exactly the reason why I NACKed this patch.
I do _not_ understand the KConfig SELECT logic. And I do think almost
nobody does understand how that all works together.
In the past people came with similiar patches like yours that looked
obviously OK. They said sentences like "it is trivial to get the SELECT
logics right". But it turned out they were wrong and it introduced
other regressions that I was made responsible for.

SELECT is _extremely_ difficult to get right, as it completely ignores
dependencies. See all this FOOBAR_POSSIBLE select logic that we use
in SSB to get SELECT working correctly with dependencies.

So my solution for this particular breakage you are seeing here is
to wait for the bcm43xx removal, which will happen soon. That will
fix it and will have almost zero chance to introduce new bugs.

> Well, if you read my first email, that is exactly what I intended to
> do, but even if
> Michael would be able to fix the b43 driver to work with my hardware, the code
> will only show up in 2.6.25, leaving some 2.6.24 users with broken
> wifi. So I thought

Blah. The people with a bcm4311 revision 1 wireless card plus a bcm44xx ethernet card.
You can count those people on two fingers.

> it would be a good thing to add my simple fix that enabled the old driver to the
> -stable tree, so that we could have working wifi soon.
> 
> This is still my intension, I'll resend to the proper maintainters.

Ok thanks. We'll see if it really was a simple fix then.
If it turns out to break something you will get mail. :)
You can send this patch to a netdev maintainer or the wireless maintainer.
Maybe one of those will sign it off. Good luck.

Re: bcm43xx regression in 2.6.24 (with patch)

[Ingo Molnar]

* Michael Buesch <mb@bu3sch.de> wrote:

> > If this is not a repgession, than I don't know what is. And if it is 
> > a regression, it should be fixed at least in the 2.6.24.y series, do 
> > you agree?
> 
> No. Playing with kconfig SELECT is really _nothing_ for a -stable 
> series. I am _not_ going to be responsible for any breakages. [...]

well, i've reviewed this thread and it's pretty apparent to any outside 
observer that you as a maintainer are ignoring Alexey Zaytsev's pretty 
reasonable request for a fix.

Alexey had a problem, he analyzed it, he found a fix which he tested, 
and he even has offered to test anything you send his way:

|| I have provided a patch that I believe is trivial, that I have tested 
|| with all possible config option combinations I thought were possible, 
|| and that fixes the regression. If you have a reason to believe it is 
|| wrong, please say it, I won't be offended. If there is a problem with 
|| the patch, I'll gladly fix and resend it.

that's about the most friendly tester attitude that is imaginable.

but what were you able to make out of that positive attitude? The only 
things i've seen you send his way were insults and general handwaving 
about how his patch breaks stuff (without providing a _shred_ of 
evidence).

I have to say, after having observed multiple incidents around b43 in 
the past few months you are one of the worst driver maintainers i've 
ever seen on lkml: you are ignoring regressions, you are frequently 
insulting our testers and now you even have the gall to NAK a patch to 
_your own buggy driver code_ without providing an alternative fix. 
Kudos.

	Ingo

Re: bcm43xx regression in 2.6.24 (with patch)

[Pekka Enberg]

On Sat, Feb 23, 2008 at 1:07 PM, Ingo Molnar <mingo@elte.hu> wrote:
>  I have to say, after having observed multiple incidents around b43 in
>  the past few months you are one of the worst driver maintainers i've
>  ever seen on lkml: you are ignoring regressions, you are frequently
>  insulting our testers and now you even have the gall to NAK a patch to
>  _your own buggy driver code_ without providing an alternative fix.
>  Kudos.

Agreed. Alexey, did you identify a specific git commit that caused the
regression? Can we just revert that from 2.6.24? Michael, even if
_you're_ planning to remove bcm43xx we must not let it regress until
it's gone.

Re: bcm43xx regression in 2.6.24 (with patch)

[Ingo Molnar]

* Pekka Enberg <penberg@cs.helsinki.fi> wrote:

> Agreed. Alexey, did you identify a specific git commit that caused the 
> regression? Can we just revert that from 2.6.24? Michael, even if 
> _you're_ planning to remove bcm43xx we must not let it regress until 
> it's gone.

btw., if the best answer is: "do not enable b43 and bcm43xx at once in 
the .config because we messed up the dependencies and fixing it in 
2.6.24-stable is unfortunately too risky", then that's a perfectly fine 
answer for -stable. Then the fix would be to exclude each other in the 
Kconfig space (which is not risky). But just ignoring bugreports and 
NAK-ing a fix is not a valid answer.

	Ingo

Re: bcm43xx regression in 2.6.24 (with patch)

[Alexey Zaytsev]

On Sat, Feb 23, 2008 at 2:24 PM, Ingo Molnar <mingo@elte.hu> wrote:
>
>  * Pekka Enberg <penberg@cs.helsinki.fi> wrote:
>
>  > Agreed. Alexey, did you identify a specific git commit that caused the
>  > regression? Can we just revert that from 2.6.24? Michael, even if
>  > _you're_ planning to remove bcm43xx we must not let it regress until
>  > it's gone.
>
>  btw., if the best answer is: "do not enable b43 and bcm43xx at once in
>  the .config because we messed up the dependencies and fixing it in
>  2.6.24-stable is unfortunately too risky", then that's a perfectly fine
>  answer for -stable. Then the fix would be to exclude each other in the
>  Kconfig space (which is not risky). But just ignoring bugreports and
>  NAK-ing a fix is not a valid answer.
>

The problem is not with enabling both b43 and bcm43xx (will, whis won't work
anyway, and there is no chance fixing it). The problem is with enabling the
bcm43xx wifi driver and the b44 Ethernet driver. The ethernet driver then
wrongly autosolects some config options that prevent the bcm43xx
driver from loading, so I'm left with either no wifi or no ethenet.


>         Ingo
>
>
> --
>  To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>  the body of a message to majordomo@vger.kernel.org
>  More majordomo info at  http://vger.kernel.org/majordomo-info.html
>  Please read the FAQ at  http://www.tux.org/lkml/
>

Re: bcm43xx regression in 2.6.24 (with patch)

[Pekka Enberg]

On Sat, Feb 23, 2008 at 1:32 PM, Alexey Zaytsev
<alexey.zaytsev@gmail.com> wrote:
>  The problem is not with enabling both b43 and bcm43xx (will, whis won't work
>  anyway, and there is no chance fixing it). The problem is with enabling the
>  bcm43xx wifi driver and the b44 Ethernet driver. The ethernet driver then
>  wrongly autosolects some config options that prevent the bcm43xx
>  driver from loading, so I'm left with either no wifi or no ethenet.

So does reverting commit 753f492093da7a40141bfe083073400f518f4c68
("[B44]: port to native ssb support") fix the regression?

Re: bcm43xx regression in 2.6.24 (with patch)

[Alexey Zaytsev]

On Sat, Feb 23, 2008 at 2:37 PM, Pekka Enberg <penberg@cs.helsinki.fi> wrote:
> On Sat, Feb 23, 2008 at 1:32 PM, Alexey Zaytsev
>  <alexey.zaytsev@gmail.com> wrote:
>  >  The problem is not with enabling both b43 and bcm43xx (will, whis won't work
>  >  anyway, and there is no chance fixing it). The problem is with enabling the
>  >  bcm43xx wifi driver and the b44 Ethernet driver. The ethernet driver then
>  >  wrongly autosolects some config options that prevent the bcm43xx
>  >  driver from loading, so I'm left with either no wifi or no ethenet.
>
>  So does reverting commit 753f492093da7a40141bfe083073400f518f4c68
>  ("[B44]: port to native ssb support") fix the regression?
>

Compiling it right now.

I'm sure it fixes the problem, but I don't feel like we should revert
it. I can't
be sure it won't cause any other problems, and anyway, this does not look
right a solution. Any other driver (rightfully) requiring the SSB_HOST would
then also break the bcm43xx driver.

I'll send an simplified patch in a few minutes.

Re: bcm43xx regression in 2.6.24 (with patch)

[Pekka J Enberg]

Hi Alexey,

On Sat, Feb 23, 2008 at 2:37 PM, Pekka Enberg <penberg@cs.helsinki.fi> wrote:
> >  So does reverting commit 753f492093da7a40141bfe083073400f518f4c68
> >  ("[B44]: port to native ssb support") fix the regression?
> >
 
On Sat, 23 Feb 2008, Alexey Zaytsev wrote:
> Compiling it right now.
> 
> I'm sure it fixes the problem, but I don't feel like we should revert
> it. I can't
> be sure it won't cause any other problems, and anyway, this does not look
> right a solution. Any other driver (rightfully) requiring the SSB_HOST would
> then also break the bcm43xx driver.
> 
> I'll send an simplified patch in a few minutes.

The point here is that it's Michael's patch and I don't want to start 
blaming anyone until you've verified it actually broke your setup. And if 
the problem was actually caused by that patch, Michael needs to either ACK 
your patch or fix it himself.

			Pekka

Re: bcm43xx regression in 2.6.24 (with patch)

[Alexey Zaytsev]

On Sat, Feb 23, 2008 at 2:57 PM, Pekka J Enberg <penberg@cs.helsinki.fi> wrote:
> Hi Alexey,
>
>
>  On Sat, Feb 23, 2008 at 2:37 PM, Pekka Enberg <penberg@cs.helsinki.fi> wrote:
>
> > >  So does reverting commit 753f492093da7a40141bfe083073400f518f4c68
>  > >  ("[B44]: port to native ssb support") fix the regression?
>  > >
>
>
> On Sat, 23 Feb 2008, Alexey Zaytsev wrote:
>  > Compiling it right now.
>  >
>  > I'm sure it fixes the problem, but I don't feel like we should revert
>  > it. I can't
>  > be sure it won't cause any other problems, and anyway, this does not look
>  > right a solution. Any other driver (rightfully) requiring the SSB_HOST would
>  > then also break the bcm43xx driver.
>  >
>  > I'll send an simplified patch in a few minutes.
>
>  The point here is that it's Michael's patch and I don't want to start
>  blaming anyone until you've verified it actually broke your setup. And if
>  the problem was actually caused by that patch, Michael needs to either ACK
>  your patch or fix it himself.

Yes, the problem was caused by this patch. With it reverted, I've got
both ethernet
and wifi working.

>
>                         Pekka
>

Re: bcm43xx regression in 2.6.24 (with patch)

[Pekka Enberg]

Hi,

On Sat, Feb 23, 2008 at 2:37 PM, Pekka Enberg <penberg@cs.helsinki.fi> wrote:
> > > >  So does reverting commit 753f492093da7a40141bfe083073400f518f4c68
> > > >  ("[B44]: port to native ssb support") fix the regression?

On Sat, Feb 23, 2008 at 2:17 PM, Alexey Zaytsev
<alexey.zaytsev@gmail.com> wrote:
>  Yes, the problem was caused by this patch. With it reverted, I've got both ethernet
>  and wifi working.

Michael, I am always glad to help out but I would much appreciate if
you could show some effort on debugging your own bugs in the future.
Perhaps you could now work with Alexey to fix it?

Re: bcm43xx regression in 2.6.24 (with patch)

[Michael Buesch]

On Saturday 23 February 2008 12:32:50 Alexey Zaytsev wrote:
> The problem is not with enabling both b43 and bcm43xx (will, whis won't work
> anyway, and there is no chance fixing it). The problem is with enabling the
> bcm43xx wifi driver and the b44 Ethernet driver. The ethernet driver then
> wrongly autosolects some config options that prevent the bcm43xx
> driver from loading, so I'm left with either no wifi or no ethenet.

Ok, so I just tested this on a machine and it is _not_ true.
It works just fine to run b44/ssb with bcm43xx in parallel.

Can you please stop fooling me and spreading this FUD?

-- 
Greetings Michael.

Re: bcm43xx regression in 2.6.24 (with patch)

[Alexey Zaytsev]

On Sun, Feb 24, 2008 at 5:29 PM, Michael Buesch <mb@bu3sch.de> wrote:
> On Saturday 23 February 2008 12:32:50 Alexey Zaytsev wrote:
>  > The problem is not with enabling both b43 and bcm43xx (will, whis won't work
>  > anyway, and there is no chance fixing it). The problem is with enabling the
>  > bcm43xx wifi driver and the b44 Ethernet driver. The ethernet driver then
>  > wrongly autosolects some config options that prevent the bcm43xx
>  > driver from loading, so I'm left with either no wifi or no ethenet.
>
>  Ok, so I just tested this on a machine and it is _not_ true.
>  It works just fine to run b44/ssb with bcm43xx in parallel.
>
>  Can you please stop fooling me and spreading this FUD?
>

The ony way I see this was possible, you manually changed the
module loading order, so that the b43xx module was loaded prior
to the ssb and b44 modules. Right? Or the b43xx driver was built
statically and the b44 and the ssb as modules.

Looks like this, kind of, ruins my statement that is is impossible to get
working wifi in 2.6.24. Sorry, it was not intensinal. ;)

But there still are lots of valid combinations where the b43xx driver
silently won't work. E.g. when either the ssb or the b44 modules are
loaded prior to it, or when either of them are built statically (building
the b43xx driver staticaly won't help here). And my patch fixes them.

So, my statement goes, the bug stays. Agreed?

Re: bcm43xx regression in 2.6.24 (with patch)

[Michael Buesch]

On Sunday 24 February 2008, Alexey Zaytsev wrote:
> On Sun, Feb 24, 2008 at 5:29 PM, Michael Buesch <mb@bu3sch.de> wrote:
> > On Saturday 23 February 2008 12:32:50 Alexey Zaytsev wrote:
> >  > The problem is not with enabling both b43 and bcm43xx (will, whis won't work
> >  > anyway, and there is no chance fixing it). The problem is with enabling the
> >  > bcm43xx wifi driver and the b44 Ethernet driver. The ethernet driver then
> >  > wrongly autosolects some config options that prevent the bcm43xx
> >  > driver from loading, so I'm left with either no wifi or no ethenet.
> >
> >  Ok, so I just tested this on a machine and it is _not_ true.
> >  It works just fine to run b44/ssb with bcm43xx in parallel.
> >
> >  Can you please stop fooling me and spreading this FUD?
> >
> 
> The ony way I see this was possible, you manually changed the
> module loading order, so that the b43xx module was loaded prior
> to the ssb and b44 modules. Right?

Right. So "so I'm left with either no wifi or no ethenet" being wrong.

> Looks like this, kind of, ruins my statement that is is impossible to get
> working wifi in 2.6.24. Sorry, it was not intensinal. ;)

Ok, I see.

> But there still are lots of valid combinations where the b43xx driver
> silently won't work. E.g. when either the ssb or the b44 modules are
> loaded prior to it, or when either of them are built statically (building
> the b43xx driver staticaly won't help here). And my patch fixes them.

With your patch you can still artificially create situations that
something doesn't work.
My point being: It works just fine without the patch.
And as I'm not really sure about the implications of the patch, I
won't ack this. But it seems Larry acked it. So that's also fine, if
he takes over responsibility for that change then. So the next
"You broke my kernel!!11" round can go to his mail address and I won't
be bothered by this anymore. ;)

Re: bcm43xx regression in 2.6.24 (with patch)

[Pekka J Enberg]

Hi Michael,

On Sun, 24 Feb 2008, Michael Buesch wrote:
> > The ony way I see this was possible, you manually changed the
> > module loading order, so that the b43xx module was loaded prior
> > to the ssb and b44 modules. Right?
> 
> Right. So "so I'm left with either no wifi or no ethenet" being wrong.

Lets make this simple: it used to work before and now it doesn't. 
Therefore it's a regression that must be addressed. Period.

			Pekka

Re: bcm43xx regression in 2.6.24 (with patch)

[Greg KH]

On Mon, Feb 25, 2008 at 08:16:17AM +0200, Pekka J Enberg wrote:
> Hi Michael,
> 
> On Sun, 24 Feb 2008, Michael Buesch wrote:
> > > The ony way I see this was possible, you manually changed the
> > > module loading order, so that the b43xx module was loaded prior
> > > to the ssb and b44 modules. Right?
> > 
> > Right. So "so I'm left with either no wifi or no ethenet" being wrong.
> 
> Lets make this simple: it used to work before and now it doesn't. 
> Therefore it's a regression that must be addressed. Period.

Isn't the resolution Michael is suggesting is, "use the different driver"?

thanks,

greg k-h

Re: bcm43xx regression in 2.6.24 (with patch)

[Pekka J Enberg]

Hi Greg,

On Sun, 24 Feb 2008, Greg KH wrote:
> > Lets make this simple: it used to work before and now it doesn't. 
> > Therefore it's a regression that must be addressed. Period.
> 
> Isn't the resolution Michael is suggesting is, "use the different driver"?

Alexey said it didn't work but even if it did, we don't let old drivers 
_regress_ as long as they're in the tree. There are plenty of precedents 
here so I don't see why this particular Broadcom driver is any different.

			Pekka

Re: bcm43xx regression in 2.6.24 (with patch)

[Michael Buesch]

On Monday 25 February 2008 07:49:35 Greg KH wrote:
> On Mon, Feb 25, 2008 at 08:16:17AM +0200, Pekka J Enberg wrote:
> > Hi Michael,
> > 
> > On Sun, 24 Feb 2008, Michael Buesch wrote:
> > > > The ony way I see this was possible, you manually changed the
> > > > module loading order, so that the b43xx module was loaded prior
> > > > to the ssb and b44 modules. Right?
> > > 
> > > Right. So "so I'm left with either no wifi or no ethenet" being wrong.
> > 
> > Lets make this simple: it used to work before and now it doesn't. 
> > Therefore it's a regression that must be addressed. Period.
> 
> Isn't the resolution Michael is suggesting is, "use the different driver"?

I have two resolutions. One being:
rmmod b44
rmmod ssb
modprobe bcm43xx
modprobe b44

The other being: Wait for 2.6.25 and use the maintained b43 driver.

-- 
Greetings Michael.

Re: bcm43xx regression in 2.6.24 (with patch)

[Pekka Enberg]

On Mon, Feb 25, 2008 at 11:54 AM, Michael Buesch <mb@bu3sch.de> wrote:
>  > Isn't the resolution Michael is suggesting is, "use the different driver"?
>
>  I have two resolutions. One being:
>  rmmod b44
>  rmmod ssb
>  modprobe bcm43xx
>  modprobe b44
>
>  The other being: Wait for 2.6.25 and use the maintained b43 driver.

Neither of which seem like acceptable solutions for a 2.6.23 -> 2.6.24
_regression_. Or maybe I am just too naive to believe Linus' statement
on not letting the kernel regress...

Re: bcm43xx regression in 2.6.24 (with patch)

[Michael Buesch]

On Monday 25 February 2008 13:11:04 Pekka Enberg wrote:
> On Mon, Feb 25, 2008 at 11:54 AM, Michael Buesch <mb@bu3sch.de> wrote:
> >  > Isn't the resolution Michael is suggesting is, "use the different driver"?
> >
> >  I have two resolutions. One being:
> >  rmmod b44
> >  rmmod ssb
> >  modprobe bcm43xx
> >  modprobe b44
> >
> >  The other being: Wait for 2.6.25 and use the maintained b43 driver.
> 
> Neither of which seem like acceptable solutions for a 2.6.23 -> 2.6.24
> _regression_. Or maybe I am just too naive to believe Linus' statement
> on not letting the kernel regress...

So, please sign-off the patch that we have, if you think it's right
and doesn't cause more regressions.

-- 
Greetings Michael.

Re: bcm43xx regression in 2.6.24 (with patch)

[Pekka J Enberg]

On Mon, 25 Feb 2008, Michael Buesch wrote:
> > Neither of which seem like acceptable solutions for a 2.6.23 -> 2.6.24
> > _regression_. Or maybe I am just too naive to believe Linus' statement
> > on not letting the kernel regress...
> 
> So, please sign-off the patch that we have, if you think it's right
> and doesn't cause more regressions.

I did look at the patch and can gladly add a:

Reviewed-by: Pekka Enberg <penberg@cs.helsinki.fi>

But this seems backwards. It was _your_ commit that broke the setup and 
the patch touches a driver _you're_ maintaining.

So can we just revert commit 753f492093da7a40141bfe083073400f518f4c68 
("[B44]: port to native ssb support") from 2.6.24 and you can add it back 
to 2.6.25 if the problem indeed does go away?

			Pekka

Re: bcm43xx regression in 2.6.24 (with patch)

[Alexey Zaytsev]

On Mon, Feb 25, 2008 at 3:25 PM, Pekka J Enberg <penberg@cs.helsinki.fi> wrote:
> On Mon, 25 Feb 2008, Michael Buesch wrote:
>  > > Neither of which seem like acceptable solutions for a 2.6.23 -> 2.6.24
>  > > _regression_. Or maybe I am just too naive to believe Linus' statement
>  > > on not letting the kernel regress...
>  >
>  > So, please sign-off the patch that we have, if you think it's right
>  > and doesn't cause more regressions.
>
>  I did look at the patch and can gladly add a:
>
>  Reviewed-by: Pekka Enberg <penberg@cs.helsinki.fi>
>
Thanks for reviewing. Was it the patch I sent to Larry Finger
with the subject line "[PATCH] Fix the bcm43xx driver breakage in 2.6.24/25"
or the patch I sent with the first email in this thread? The patches are
generally the same, but the one sent to Larry was split into two pieces
and the condition on which the bcm43xx config option was hidden
changed a bit.

>  But this seems backwards. It was _your_ commit that broke the setup and
>  the patch touches a driver _you're_ maintaining.
>
>  So can we just revert commit 753f492093da7a40141bfe083073400f518f4c68
>  ("[B44]: port to native ssb support") from 2.6.24 and you can add it back
>  to 2.6.25 if the problem indeed does go away?

I'm quite sure my patch won't cause any problems, but if Greg
wants to be 100% sure there won't be any regressions introduced
in -stable, reverting the said commit should be the right thing.

Re: bcm43xx regression in 2.6.24 (with patch)

[Alexey Zaytsev]

On Mon, Feb 25, 2008 at 9:49 AM, Greg KH <greg@kroah.com> wrote:
>
> On Mon, Feb 25, 2008 at 08:16:17AM +0200, Pekka J Enberg wrote:
>  > Hi Michael,
>  >
>  > On Sun, 24 Feb 2008, Michael Buesch wrote:
>  > > > The ony way I see this was possible, you manually changed the
>  > > > module loading order, so that the b43xx module was loaded prior
>  > > > to the ssb and b44 modules. Right?
>  > >
>  > > Right. So "so I'm left with either no wifi or no ethenet" being wrong.
>  >
>  > Lets make this simple: it used to work before and now it doesn't.
>  > Therefore it's a regression that must be addressed. Period.
>
>  Isn't the resolution Michael is suggesting is, "use the different driver"?
>

The b43 driver from 2.6.24 does not work with my hardware. The one from
2.6.25 seems to work, but 2.6.25 is far from being ready yet.

The only way you can get the old driver working in 2.6.24 is to
compile certain (completely unrelated for an outsider) drivers as
modules and play with the module loading order. I think this is a
bug, and it should be fixed in -stable.

And for 2.6.25, I think the patch should also be included, as the
bcm43xx driver is still there.


Btw, what are we discussing? The real bcm43xx maintainer
(Larry Finger) reviewed the patch and agreet it should go
into 2.6.24-stable and 2.6.25 . Now we only have to wait for
John Linville and Jeff Garzik to pick it up, right? If someone
still thinks the patch should not go in, he should at least
add them to the CC list. ;)



>  thanks,
>
>  greg k-h
>

Re: bcm43xx regression in 2.6.24 (with patch)

[Michael Buesch]

On Monday 25 February 2008 11:23:02 Alexey Zaytsev wrote:
> On Mon, Feb 25, 2008 at 9:49 AM, Greg KH <greg@kroah.com> wrote:
> >
> > On Mon, Feb 25, 2008 at 08:16:17AM +0200, Pekka J Enberg wrote:
> >  > Hi Michael,
> >  >
> >  > On Sun, 24 Feb 2008, Michael Buesch wrote:
> >  > > > The ony way I see this was possible, you manually changed the
> >  > > > module loading order, so that the b43xx module was loaded prior
> >  > > > to the ssb and b44 modules. Right?
> >  > >
> >  > > Right. So "so I'm left with either no wifi or no ethenet" being wrong.
> >  >
> >  > Lets make this simple: it used to work before and now it doesn't.
> >  > Therefore it's a regression that must be addressed. Period.
> >
> >  Isn't the resolution Michael is suggesting is, "use the different driver"?
> >
> 
> The b43 driver from 2.6.24 does not work with my hardware. The one from
> 2.6.25 seems to work, but 2.6.25 is far from being ready yet.
> 
> The only way you can get the old driver working in 2.6.24 is to
> compile certain (completely unrelated for an outsider) drivers as
> modules and play with the module loading order. I think this is a
> bug, and it should be fixed in -stable.

It must first go into the 2.6.25 tree and then into -stable.
And the patch must be considered to do The Right Thing (tm).

> And for 2.6.25, I think the patch should also be included, as the
> bcm43xx driver is still there.
> 
> Btw, what are we discussing? The real bcm43xx maintainer
> (Larry Finger) reviewed the patch and agreet it should go
> into 2.6.24-stable and 2.6.25 . Now we only have to wait for
> John Linville and Jeff Garzik to pick it up, right? If someone
> still thinks the patch should not go in, he should at least
> add them to the CC list. ;)

Heh, wait. Larry is neither the bcm43xx maintainer [1], nor does this
code touch bcm43xx code. It does touch ssb and b43 code, which I
am the maintainer of.
Though, as I said, if somebody else is familiar with the code does really
understand the patch and does ACK it (which Larry did), I'm probably
OK with applying it. As long as I'm not bothered with any followup
regressions caused by this. So if the one signing off the patch will
handle all this, I'm fine with it.

[1] bcm43xx is unmaintained. Larry used to be the maintainer until
he dropped it a few months ago.

-- 
Greetings Michael.

Re: bcm43xx regression in 2.6.24 (with patch)

[Xavier Bestel]

On Mon, 2008-02-25 at 11:38 +0100, Michael Buesch wrote:
> [1] bcm43xx is unmaintained. Larry used to be the maintainer until
> he dropped it a few months ago. 

Doesn't that mean that Alexey gets to be the maintainer, as he's the one
patching it ?

	Xav

Re: bcm43xx regression in 2.6.24 (with patch)

[Michael Buesch]

On Monday 25 February 2008 11:49:34 Xavier Bestel wrote:
> On Mon, 2008-02-25 at 11:38 +0100, Michael Buesch wrote:
> > [1] bcm43xx is unmaintained. Larry used to be the maintainer until
> > he dropped it a few months ago. 
> 
> Doesn't that mean that Alexey gets to be the maintainer, as he's the one
> patching it ?

1) He is not patching it.

2) Yeah, if he likes to be the bcm43xx maintainer, please go for it.

-- 
Greetings Michael.

Re: bcm43xx regression in 2.6.24 (with patch)

[Michael Buesch]

On Saturday 23 February 2008 12:07:51 Ingo Molnar wrote:
> 
> * Michael Buesch <mb@bu3sch.de> wrote:
> 
> > > If this is not a repgession, than I don't know what is. And if it is 
> > > a regression, it should be fixed at least in the 2.6.24.y series, do 
> > > you agree?
> > 
> > No. Playing with kconfig SELECT is really _nothing_ for a -stable 
> > series. I am _not_ going to be responsible for any breakages. [...]
> 
> well, i've reviewed this thread and it's pretty apparent to any outside 
> observer that you as a maintainer are ignoring Alexey Zaytsev's pretty 
> reasonable request for a fix.
> 
> Alexey had a problem, he analyzed it, he found a fix which he tested, 
> and he even has offered to test anything you send his way:
> 
> || I have provided a patch that I believe is trivial, that I have tested 
> || with all possible config option combinations I thought were possible, 
> || and that fixes the regression. If you have a reason to believe it is 
> || wrong, please say it, I won't be offended. If there is a problem with 
> || the patch, I'll gladly fix and resend it.
> 
> that's about the most friendly tester attitude that is imaginable.
> 
> but what were you able to make out of that positive attitude? The only 
> things i've seen you send his way were insults and general handwaving 
> about how his patch breaks stuff (without providing a _shred_ of 
> evidence).

blah:

> I have to say, after having observed multiple incidents around b43 in 
> the past few months you are one of the worst driver maintainers i've 
> ever seen on lkml: you are ignoring regressions, you are frequently 
> insulting our testers and now you even have the gall to NAK a patch to 
> _your own buggy driver code_ without providing an alternative fix. 
> Kudos.

So I am forced to sign-off random patches people send to me?
I explained why I do not. If you do not like that, please do sign it off.
If you do think the patch is correct, please _do_ sign it off Ingo.

This problem will fix itself by switching to b43 and dropping bcm43xx.
_That_ is my way to fix the bug.
I don't understand all the SELECT implications, so I'm not going
to introduce more of them. Because if the next regression appears from
I SELECT that I signed off
goto blah;

-- 
Greetings Michael.

Re: bcm43xx regression in 2.6.24 (with patch)

[Michael Buesch]

On Saturday 23 February 2008 12:07:51 Ingo Molnar wrote:
> I have to say, after having observed multiple incidents around b43 in 
> the past few months you are one of the worst driver maintainers i've 
> ever seen on lkml: you are ignoring regressions, you are frequently 
> insulting our testers and now you even have the gall to NAK a patch to 

The insults being? A few quotes, please.

> _your own buggy driver code_ without providing an alternative fix. 
> Kudos.

How dare can you break my kernel, yeah... . We know that.
That my patches _fixed_ the kernel for thousands of people doesn't
matter. What matters is the one person for whom it broke. So let's
apply a questionable fix for it that we don't understand. Hopefully
that fixes it and doesn't break it again for somebody else... .

I provided an alternative fix. In my very first email.
Ingo, if you know that this fix is right, then please sign it off
so we can apply it.

-- 
Greetings Michael.

Re: bcm43xx regression in 2.6.24 (with patch)

[Ingo Molnar]

* Michael Buesch <mb@bu3sch.de> wrote:

> On Saturday 23 February 2008 12:07:51 Ingo Molnar wrote:
> > I have to say, after having observed multiple incidents around b43 in 
> > the past few months you are one of the worst driver maintainers i've 
> > ever seen on lkml: you are ignoring regressions, you are frequently 
> > insulting our testers and now you even have the gall to NAK a patch to 
> 
> The insults being? A few quotes, please.

  " I'm tired of this "how dare can you break my kernel!?" bullshit. "
  ...
  " Blah. The people with a bcm4311 revision 1 wireless card plus a 
    bcm44xx ethernet card. You can count those people on two fingers. "
  ...

    http://lkml.org/lkml/2008/2/23/3

	Ingo

Re: bcm43xx regression in 2.6.24 (with patch)

[Michael Buesch]

On Saturday 23 February 2008 17:44:33 Ingo Molnar wrote:
> 
> * Michael Buesch <mb@bu3sch.de> wrote:
> 
> > On Saturday 23 February 2008 12:07:51 Ingo Molnar wrote:
> > > I have to say, after having observed multiple incidents around b43 in 
> > > the past few months you are one of the worst driver maintainers i've 
> > > ever seen on lkml: you are ignoring regressions, you are frequently 
> > > insulting our testers and now you even have the gall to NAK a patch to 
> > 
> > The insults being? A few quotes, please.
> 
>   " I'm tired of this "how dare can you break my kernel!?" bullshit. "
>   ...
>   " Blah. The people with a bcm4311 revision 1 wireless card plus a 
>     bcm44xx ethernet card. You can count those people on two fingers. "
>   ...
> 
>     http://lkml.org/lkml/2008/2/23/3

Hm, maybe my fault and I don't know the english language enough, but these
do not count as personal insults here where I live in Germany. Especially
the second one is in no way an insult here. It merely tells you that the
number of people having this issue is very low.

I'm sorry if that sounded like insults. That wasn't my intention.

-- 
Greetings Michael.

Re: bcm43xx regression in 2.6.24 (with patch)

[Alexey Zaytsev]

On Sat, Feb 23, 2008 at 7:27 PM, Michael Buesch <mb@bu3sch.de> wrote:
> On Saturday 23 February 2008 12:07:51 Ingo Molnar wrote:
>
> > I have to say, after having observed multiple incidents around b43 in
>  > the past few months you are one of the worst driver maintainers i've
>  > ever seen on lkml: you are ignoring regressions, you are frequently
>  > insulting our testers and now you even have the gall to NAK a patch to
>
>  The insults being? A few quotes, please.
>

If you really want to know, the
"Because the new driver works, if you just set it up right."
for me was clearly a hint that I'm just an other imcompetent
user, who can't even follow the instructions from linuxwireless.org.
And you knew that the new driver did no work with the bcm4311
chips, which is the sad thing.

>
>  > _your own buggy driver code_ without providing an alternative fix.
>  > Kudos.
>
>  How dare can you break my kernel, yeah... . We know that.
>  That my patches _fixed_ the kernel for thousands of people doesn't
>  matter.

Not true at all. I  my first and second emails, there was not a single word
about who broke the driver. There were only the fix, and the explanation,
why the fix is needed. I really appreciate the work done on the b43 driver,
and I'd emmediately switch to it, if only it worked for me*.

>  What matters is the one person for whom it broke.

I'm the one person who investigated the problem, wrote a
patch and sent it to the lkml. After lurking in #bcm-users
for a day, I saw quite a few users asking why the b43 driver
did not work for them, and the best answer they got, was
got read the linuxwireless.org. Are you sure they all were
just unable to upgrade the firmware?

And by the way, the bcm4311 and b44 combination is not
something I assembled just to piss you off. It's what HP puts
in their HP Compaq nx7300 laptops. They were like the
cheapest Core Duo laptops a year ago, and I'm sure they were
not exclusively made.

>  So let's
>  apply a questionable fix for it that we don't understand. Hopefully
>  that fixes it and doesn't break it again for somebody else... .
>
>  I provided an alternative fix. In my very first email.

Sorry, your fix does not work for my hardware. And you know it.

>  Ingo, if you know that this fix is right, then please sign it off
>  so we can apply it.
>
>  --
>  Greetings Michael.
>
>
> --
>  To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>  the body of a message to majordomo@vger.kernel.org
>  More majordomo info at  http://vger.kernel.org/majordomo-info.html
>  Please read the FAQ at  http://www.tux.org/lkml/
>

Re: bcm43xx regression in 2.6.24 (with patch)

[Michael Buesch]

On Saturday 23 February 2008 22:32:46 Alexey Zaytsev wrote:
> >  The insults being? A few quotes, please.
> If you really want to know, the
> "Because the new driver works, if you just set it up right."
> for me was clearly a hint that I'm just an other imcompetent
> user, who can't even follow the instructions from linuxwireless.org.

Well. That's 99% of all bugreports I receive. You will understand
that I won't explain the same crap that's written on linuxwireless.org
over and over 20 times each day to random people.
And yes, you belong to the group of my "random people", as I don't
remember your name from the top of my head.
So in 99% of the cases the user did something wrong.

But I still don't see the insult.

> And you knew that the new driver did no work with the bcm4311
> chips, which is the sad thing.

That is not true. It doesn't work with exactly _one_ revision
of the bcm4311 card. And that is already fixed in 2.6.25.
I'd like to have that in .24-stable, too, but I guess it's too big.
It changes some parts of the DMA engine code.

> Not true at all. I  my first and second emails, there was not a single word
> about who broke the driver. There were only the fix, and the explanation,
> why the fix is needed. I really appreciate the work done on the b43 driver,
> and I'd emmediately switch to it, if only it worked for me*.

Yeah. But I don't like that patch. Then people came and said stuff like
I _have_ to accept it because it fixes stuff I broke. Excuse me, but
nobody can force me to sign off a patch that I am not completely
convinced of. _Even_ if it turns out that a commit made by me
broke something. Especially, if there's already another fix for the
situation queued up in the development kernel.

> I'm the one person who investigated the problem, wrote a
> patch and sent it to the lkml. After lurking in #bcm-users
> for a day, I saw quite a few users asking why the b43 driver
> did not work for them, and the best answer they got, was
> got read the linuxwireless.org. Are you sure they all were
> just unable to upgrade the firmware?

Yeah. Experience shows that. Read the archives.

> And by the way, the bcm4311 and b44 combination is not
> something I assembled just to piss you off. It's what HP puts
> in their HP Compaq nx7300 laptops. They were like the
> cheapest Core Duo laptops a year ago, and I'm sure they were
> not exclusively made.

So be it. And you guess what? We already sent a fix for those
people upstream a long time ago. It is in 2.6.25.

> >  So let's
> >  apply a questionable fix for it that we don't understand. Hopefully
> >  that fixes it and doesn't break it again for somebody else... .
> >
> >  I provided an alternative fix. In my very first email.
> 
> Sorry, your fix does not work for my hardware. And you know it.

Did you _ever_ even try it?
http://linuxwireless.org/en/users/Download

-- 
Greetings Michael.

Re: bcm43xx regression in 2.6.24 (with patch)

[John W. Linville]

On Sat, Feb 23, 2008 at 11:20:58PM +0100, Michael Buesch wrote:
> On Saturday 23 February 2008 22:32:46 Alexey Zaytsev wrote:

> > And you knew that the new driver did no work with the bcm4311
> > chips, which is the sad thing.
> 
> That is not true. It doesn't work with exactly _one_ revision
> of the bcm4311 card. And that is already fixed in 2.6.25.
> I'd like to have that in .24-stable, too, but I guess it's too big.
> It changes some parts of the DMA engine code.

Which patch is it?  Is it this one?  Or is there another one that touches 4311?

commit 013978b688d2a27af3ab55ca739e8c8ac7254870
Author: Larry Finger <Larry.Finger@lwfinger.net>
Date:   Mon Nov 26 10:29:47 2007 -0600

    b43: Changes to enable BCM4311 rev 02 with wireless core revision 13

    The BCM94311MCG rev 02 chip has an 802.11 core with revision 13 and
    has not been supported until now. The changes include the following:

<snip>

If so, perhaps we should just consider applying this to -stable?

I really do wish everyone would tone down their rhetoric on this
issue...

John
-- 
John W. Linville
linville@tuxdriver.com

Re: bcm43xx regression in 2.6.24 (with patch)

[Alexey Zaytsev]

On Tue, Feb 26, 2008 at 11:20 PM, John W. Linville
<linville@tuxdriver.com> wrote:
> On Sat, Feb 23, 2008 at 11:20:58PM +0100, Michael Buesch wrote:
>  > On Saturday 23 February 2008 22:32:46 Alexey Zaytsev wrote:
>
>
> > > And you knew that the new driver did no work with the bcm4311
>  > > chips, which is the sad thing.
>  >
>  > That is not true. It doesn't work with exactly _one_ revision
>  > of the bcm4311 card. And that is already fixed in 2.6.25.
>  > I'd like to have that in .24-stable, too, but I guess it's too big.
>  > It changes some parts of the DMA engine code.
>
>  Which patch is it?  Is it this one?  Or is there another one that touches 4311?
>
>  commit 013978b688d2a27af3ab55ca739e8c8ac7254870
>  Author: Larry Finger <Larry.Finger@lwfinger.net>
>  Date:   Mon Nov 26 10:29:47 2007 -0600
>
>     b43: Changes to enable BCM4311 rev 02 with wireless core revision 13
>
>     The BCM94311MCG rev 02 chip has an 802.11 core with revision 13 and
>     has not been supported until now. The changes include the following:
>
>  <snip>
>
>  If so, perhaps we should just consider applying this to -stable?
>

I see a few problems here:

This commit seems to depend on some commits prior to it, as it touches
drivers/net/wireless/b43/wa.c which is absent in 2.6.24.3. When I get home,
I'll test if it still works if you ignore wa.c

The patch may be too complex for -stable. I think there was a hard line
count limit, like no more than 100 lines. I'm not sure if this still stands.

This  does not fix the bcm43xx driver.

>  I really do wish everyone would tone down their rhetoric on this
>  issue...
>
>  John
>  --
>  John W. Linville
>  linville@tuxdriver.com
>

Re: bcm43xx regression in 2.6.24 (with patch)

[Michael Buesch]

On Tuesday 26 February 2008 22:38:56 Alexey Zaytsev wrote:
> On Tue, Feb 26, 2008 at 11:20 PM, John W. Linville
> <linville@tuxdriver.com> wrote:
> > On Sat, Feb 23, 2008 at 11:20:58PM +0100, Michael Buesch wrote:
> >  > On Saturday 23 February 2008 22:32:46 Alexey Zaytsev wrote:
> >
> >
> > > > And you knew that the new driver did no work with the bcm4311
> >  > > chips, which is the sad thing.
> >  >
> >  > That is not true. It doesn't work with exactly _one_ revision
> >  > of the bcm4311 card. And that is already fixed in 2.6.25.
> >  > I'd like to have that in .24-stable, too, but I guess it's too big.
> >  > It changes some parts of the DMA engine code.
> >
> >  Which patch is it?  Is it this one?  Or is there another one that touches 4311?

Yeah, I think that's it.

> >  commit 013978b688d2a27af3ab55ca739e8c8ac7254870
> >  Author: Larry Finger <Larry.Finger@lwfinger.net>
> >  Date:   Mon Nov 26 10:29:47 2007 -0600
> >
> >     b43: Changes to enable BCM4311 rev 02 with wireless core revision 13
> >
> >     The BCM94311MCG rev 02 chip has an 802.11 core with revision 13 and
> >     has not been supported until now. The changes include the following:
> >
> >  <snip>
> >
> >  If so, perhaps we should just consider applying this to -stable?
> >
> 
> I see a few problems here:
> 
> This commit seems to depend on some commits prior to it, as it touches
> drivers/net/wireless/b43/wa.c which is absent in 2.6.24.3. When I get home,
> I'll test if it still works if you ignore wa.c

Yeah, seems so. But if you are willing to test an adjusted version, I can
probably do a backported version of this patch.
This patch is well tested and does the right thing, so I'm willing to sign
off a backported version of this.
First need to find the place in the old phy.c code that matches the new wa.c code.

> The patch may be too complex for -stable. I think there was a hard line
> count limit, like no more than 100 lines. I'm not sure if this still stands.
> 
> This  does not fix the bcm43xx driver.

It does not even touch the bcm43xx driver.
Besides that the bcm43xx driver is not broken. That's the whole reason
this damn thread started at all. So it can't be broken.

-- 
Greetings Michael.

Re: bcm43xx regression in 2.6.24 (with patch)

[Alexey Zaytsev]

On Wed, Feb 27, 2008 at 1:04 AM, Michael Buesch <mb@bu3sch.de> wrote:
> On Tuesday 26 February 2008 22:38:56 Alexey Zaytsev wrote:
>  > On Tue, Feb 26, 2008 at 11:20 PM, John W. Linville
>  > <linville@tuxdriver.com> wrote:
>  > > On Sat, Feb 23, 2008 at 11:20:58PM +0100, Michael Buesch wrote:
>  > >  > On Saturday 23 February 2008 22:32:46 Alexey Zaytsev wrote:
>  > >
>  > >
>  > > > > And you knew that the new driver did no work with the bcm4311
>  > >  > > chips, which is the sad thing.
>  > >  >
>  > >  > That is not true. It doesn't work with exactly _one_ revision
>  > >  > of the bcm4311 card. And that is already fixed in 2.6.25.
>  > >  > I'd like to have that in .24-stable, too, but I guess it's too big.
>  > >  > It changes some parts of the DMA engine code.
>  > >
>  > >  Which patch is it?  Is it this one?  Or is there another one that touches 4311?
>
>  Yeah, I think that's it.
>
>
>  > >  commit 013978b688d2a27af3ab55ca739e8c8ac7254870
>  > >  Author: Larry Finger <Larry.Finger@lwfinger.net>
>  > >  Date:   Mon Nov 26 10:29:47 2007 -0600
>  > >
>  > >     b43: Changes to enable BCM4311 rev 02 with wireless core revision 13
>  > >
>  > >     The BCM94311MCG rev 02 chip has an 802.11 core with revision 13 and
>  > >     has not been supported until now. The changes include the following:
>  > >
>  > >  <snip>
>  > >
>  > >  If so, perhaps we should just consider applying this to -stable?
>  > >
>  >
>  > I see a few problems here:
>  >
>  > This commit seems to depend on some commits prior to it, as it touches
>  > drivers/net/wireless/b43/wa.c which is absent in 2.6.24.3. When I get home,
>  > I'll test if it still works if you ignore wa.c
>
>  Yeah, seems so. But if you are willing to test an adjusted version, I can
>  probably do a backported version of this patch.
>  This patch is well tested and does the right thing, so I'm willing to sign
>  off a backported version of this.
>  First need to find the place in the old phy.c code that matches the new wa.c code.
>
I'll do the testing.

>
>  > The patch may be too complex for -stable. I think there was a hard line
>  > count limit, like no more than 100 lines. I'm not sure if this still stands.
>  >
>  > This  does not fix the bcm43xx driver.
>
>  It does not even touch the bcm43xx driver.
>  Besides that the bcm43xx driver is not broken. That's the whole reason
>  this damn thread started at all. So it can't be broken.
>
Can't agree here. The bcm43xx driver used to work with 2.6.23 without requiring
any module magic.


>  --
>  Greetings Michael.
>

Re: bcm43xx regression in 2.6.24 (with patch)

[Michael Buesch]

On Tuesday 26 February 2008 23:12:32 Alexey Zaytsev wrote:
> >  Yeah, seems so. But if you are willing to test an adjusted version, I can
> >  probably do a backported version of this patch.
> >  This patch is well tested and does the right thing, so I'm willing to sign
> >  off a backported version of this.
> >  First need to find the place in the old phy.c code that matches the new wa.c code.
> >
> I'll do the testing.

Index: linux-2.6.24.3/drivers/net/wireless/b43/dma.c
===================================================================
--- linux-2.6.24.3.orig/drivers/net/wireless/b43/dma.c	2008-02-26 23:39:39.000000000 +0100
+++ linux-2.6.24.3/drivers/net/wireless/b43/dma.c	2008-02-26 23:39:43.000000000 +0100
@@ -165,7 +165,7 @@ static void op64_fill_descriptor(struct 
 	addrhi = (((u64) dmaaddr >> 32) & ~SSB_DMA_TRANSLATION_MASK);
 	addrext = (((u64) dmaaddr >> 32) & SSB_DMA_TRANSLATION_MASK)
 	    >> SSB_DMA_TRANSLATION_SHIFT;
-	addrhi |= ssb_dma_translation(ring->dev->dev);
+	addrhi |= (ssb_dma_translation(ring->dev->dev) << 1);
 	if (slot == ring->nr_slots - 1)
 		ctl0 |= B43_DMA64_DCTL0_DTABLEEND;
 	if (start)
@@ -426,9 +426,21 @@ static inline
 static int alloc_ringmemory(struct b43_dmaring *ring)
 {
 	struct device *dev = ring->dev->dev->dev;
+	gfp_t flags = GFP_KERNEL;
 
+	/* The specs call for 4K buffers for 30- and 32-bit DMA with 4K
+	 * alignment and 8K buffers for 64-bit DMA with 8K alignment. Testing
+	 * has shown that 4K is sufficient for the latter as long as the buffer
+	 * does not cross an 8K boundary.
+	 *
+	 * For unknown reasons - possibly a hardware error - the BCM4311 rev
+	 * 02, which uses 64-bit DMA, needs the ring buffer in very low memory,
+	 * which accounts for the GFP_DMA flag below.
+	 */
+	if (ring->dma64)
+		flags |= GFP_DMA;
 	ring->descbase = dma_alloc_coherent(dev, B43_DMA_RINGMEMSIZE,
-					    &(ring->dmabase), GFP_KERNEL);
+					    &(ring->dmabase), flags);
 	if (!ring->descbase) {
 		b43err(ring->dev->wl, "DMA ringmemory allocation failed\n");
 		return -ENOMEM;
@@ -483,7 +495,7 @@ int b43_dmacontroller_rx_reset(struct b4
 	return 0;
 }
 
-/* Reset the RX DMA channel */
+/* Reset the TX DMA channel */
 int b43_dmacontroller_tx_reset(struct b43_wldev *dev, u16 mmio_base, int dma64)
 {
 	int i;
@@ -647,7 +659,7 @@ static int dmacontroller_setup(struct b4
 			b43_dma_write(ring, B43_DMA64_TXRINGHI,
 				      ((ringbase >> 32) &
 				       ~SSB_DMA_TRANSLATION_MASK)
-				      | trans);
+				      | (trans << 1));
 		} else {
 			u32 ringbase = (u32) (ring->dmabase);
 
@@ -680,8 +692,9 @@ static int dmacontroller_setup(struct b4
 			b43_dma_write(ring, B43_DMA64_RXRINGHI,
 				      ((ringbase >> 32) &
 				       ~SSB_DMA_TRANSLATION_MASK)
-				      | trans);
-			b43_dma_write(ring, B43_DMA64_RXINDEX, 200);
+				      | (trans << 1));
+			b43_dma_write(ring, B43_DMA64_RXINDEX, ring->nr_slots *
+				      sizeof(struct b43_dmadesc64));
 		} else {
 			u32 ringbase = (u32) (ring->dmabase);
 
@@ -695,11 +708,12 @@ static int dmacontroller_setup(struct b4
 			b43_dma_write(ring, B43_DMA32_RXRING,
 				      (ringbase & ~SSB_DMA_TRANSLATION_MASK)
 				      | trans);
-			b43_dma_write(ring, B43_DMA32_RXINDEX, 200);
+			b43_dma_write(ring, B43_DMA32_RXINDEX, ring->nr_slots *
+				      sizeof(struct b43_dmadesc32));
 		}
 	}
 
-      out:
+out:
 	return err;
 }
 
Index: linux-2.6.24.3/drivers/net/wireless/b43/main.c
===================================================================
--- linux-2.6.24.3.orig/drivers/net/wireless/b43/main.c	2008-02-26 23:39:39.000000000 +0100
+++ linux-2.6.24.3/drivers/net/wireless/b43/main.c	2008-02-26 23:39:43.000000000 +0100
@@ -101,6 +101,7 @@ static const struct ssb_device_id b43_ss
 	SSB_DEVICE(SSB_VENDOR_BROADCOM, SSB_DEV_80211, 7),
 	SSB_DEVICE(SSB_VENDOR_BROADCOM, SSB_DEV_80211, 9),
 	SSB_DEVICE(SSB_VENDOR_BROADCOM, SSB_DEV_80211, 10),
+	SSB_DEVICE(SSB_VENDOR_BROADCOM, SSB_DEV_80211, 13),
 	SSB_DEVTABLE_END
 };
 
@@ -3079,7 +3080,7 @@ static int b43_phy_versioning(struct b43
 			unsupported = 1;
 		break;
 	case B43_PHYTYPE_G:
-		if (phy_rev > 8)
+		if (phy_rev > 9)
 			unsupported = 1;
 		break;
 	default:


-- 
Greetings Michael.

Re: bcm43xx regression in 2.6.24 (with patch)

[Michael Buesch]

On Tuesday 26 February 2008 23:41:55 Michael Buesch wrote:
> On Tuesday 26 February 2008 23:12:32 Alexey Zaytsev wrote:
> > >  Yeah, seems so. But if you are willing to test an adjusted version, I can
> > >  probably do a backported version of this patch.
> > >  This patch is well tested and does the right thing, so I'm willing to sign
> > >  off a backported version of this.
> > >  First need to find the place in the old phy.c code that matches the new wa.c code.
> > >
> > I'll do the testing.
> 
> Index: linux-2.6.24.3/drivers/net/wireless/b43/dma.c
> ===================================================================
> --- linux-2.6.24.3.orig/drivers/net/wireless/b43/dma.c	2008-02-26 23:39:39.000000000 +0100
> +++ linux-2.6.24.3/drivers/net/wireless/b43/dma.c	2008-02-26 23:39:43.000000000 +0100
> @@ -165,7 +165,7 @@ static void op64_fill_descriptor(struct 
>  	addrhi = (((u64) dmaaddr >> 32) & ~SSB_DMA_TRANSLATION_MASK);
>  	addrext = (((u64) dmaaddr >> 32) & SSB_DMA_TRANSLATION_MASK)
[snip]

So are you actually going to do this anytime soon?

-- 
Greetings Michael.

Re: bcm43xx regression in 2.6.24 (with patch)

[Alexey Zaytsev]

On Fri, Feb 29, 2008 at 1:03 AM, Michael Buesch <mb@bu3sch.de> wrote:
> On Tuesday 26 February 2008 23:41:55 Michael Buesch wrote:
>  > On Tuesday 26 February 2008 23:12:32 Alexey Zaytsev wrote:
>  > > >  Yeah, seems so. But if you are willing to test an adjusted version, I can
>  > > >  probably do a backported version of this patch.
>  > > >  This patch is well tested and does the right thing, so I'm willing to sign
>  > > >  off a backported version of this.
>  > > >  First need to find the place in the old phy.c code that matches the new wa.c code.
>  > > >
>  > > I'll do the testing.
>  >
>  > Index: linux-2.6.24.3/drivers/net/wireless/b43/dma.c
>  > ===================================================================
>  > --- linux-2.6.24.3.orig/drivers/net/wireless/b43/dma.c        2008-02-26 23:39:39.000000000 +0100
>  > +++ linux-2.6.24.3/drivers/net/wireless/b43/dma.c     2008-02-26 23:39:43.000000000 +0100
>  > @@ -165,7 +165,7 @@ static void op64_fill_descriptor(struct
>  >       addrhi = (((u64) dmaaddr >> 32) & ~SSB_DMA_TRANSLATION_MASK);
>  >       addrext = (((u64) dmaaddr >> 32) & SSB_DMA_TRANSLATION_MASK)
>  [snip]
>
>  So are you actually going to do this anytime soon?
>

Sorry for the delay. Yes, it worked fine.

Btw, when booting the system with the b43 driver, I get a
~30 socond delay after the boot scripts report
"Waiting for /dev to be fully populated" and all the modules
get loaded. Is this something expected?



>  --
>  Greetings Michael.
>

Re: bcm43xx regression in 2.6.24 (with patch)

[Stefan Lippers-Hollmann]

[-- Attachment #1: Type: text/plain, Size: 1901 bytes --]

Hi

On Donnerstag, 28. Februar 2008, Alexey Zaytsev wrote:
> On Fri, Feb 29, 2008 at 1:03 AM, Michael Buesch <mb@bu3sch.de> wrote:
> > On Tuesday 26 February 2008 23:41:55 Michael Buesch wrote:
> >  > On Tuesday 26 February 2008 23:12:32 Alexey Zaytsev wrote:
> >  > > >  Yeah, seems so. But if you are willing to test an adjusted version, I can
> >  > > >  probably do a backported version of this patch.
> >  > > >  This patch is well tested and does the right thing, so I'm willing to sign
> >  > > >  off a backported version of this.
> >  > > >  First need to find the place in the old phy.c code that matches the new wa.c code.
> >  > > >
> >  > > I'll do the testing.
> >  >
[...]
> >
> >  So are you actually going to do this anytime soon?
> >
> 
> Sorry for the delay. Yes, it worked fine.
> 
> Btw, when booting the system with the b43 driver, I get a
> ~30 socond delay after the boot scripts report
> "Waiting for /dev to be fully populated" and all the modules
> get loaded. Is this something expected?

That is most likely udev related, which doesn't really like module names 
and default interface names changing for a known MAC address, depending on 
your distribution (the following example assumes debian sid, udev 0.114-2) 
you might have something like /etc/udev/rules.d/z25_persistent-net.rules 
(which lists MAC addresses and the associated persistent interface name), 
removing the corresponding stanza for your Broadcom wireless card (or 
removing the whole file alltogether and letting udev (or rather 
z45_persistent-net-generator.rules) recreate it during the next boot will 
most likely fix it. 

This issue is in no way b43 related, it happens with "every" networking 
devices with changing module names (at least for every legacy wireless 
module being replaced by a new mac80211 based driver).

Regards
	Stefan Lippers-Hollmann

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: bcm43xx regression in 2.6.24 (with patch)

[Alexey Zaytsev]

On Fri, Feb 29, 2008 at 2:12 AM, Stefan Lippers-Hollmann <s.L-H@gmx.de> wrote:
[...]
>  >
>  > Btw, when booting the system with the b43 driver, I get a
>  > ~30 socond delay after the boot scripts report
>  > "Waiting for /dev to be fully populated" and all the modules
>  > get loaded. Is this something expected?
>
>  That is most likely udev related, which doesn't really like module names
>  and default interface names changing for a known MAC address, depending on
>  your distribution (the following example assumes debian sid, udev 0.114-2)
>  you might have something like /etc/udev/rules.d/z25_persistent-net.rules
>  (which lists MAC addresses and the associated persistent interface name),
>  removing the corresponding stanza for your Broadcom wireless card (or
>  removing the whole file alltogether and letting udev (or rather
>  z45_persistent-net-generator.rules) recreate it during the next boot will
>  most likely fix it.
>
>  This issue is in no way b43 related, it happens with "every" networking
>  devices with changing module names (at least for every legacy wireless
>  module being replaced by a new mac80211 based driver).

Removing the file helped. Thanks.

>
>  Regards
>         Stefan Lippers-Hollmann
>

Re: bcm43xx regression in 2.6.24 (with patch)

[Michael Buesch]

On Thursday 28 February 2008 23:48:48 Alexey Zaytsev wrote:
> On Fri, Feb 29, 2008 at 1:03 AM, Michael Buesch <mb@bu3sch.de> wrote:
> > On Tuesday 26 February 2008 23:41:55 Michael Buesch wrote:
> >  > On Tuesday 26 February 2008 23:12:32 Alexey Zaytsev wrote:
> >  > > >  Yeah, seems so. But if you are willing to test an adjusted version, I can
> >  > > >  probably do a backported version of this patch.
> >  > > >  This patch is well tested and does the right thing, so I'm willing to sign
> >  > > >  off a backported version of this.
> >  > > >  First need to find the place in the old phy.c code that matches the new wa.c code.
> >  > > >
> >  > > I'll do the testing.
> >  >
> >  > Index: linux-2.6.24.3/drivers/net/wireless/b43/dma.c
> >  > ===================================================================
> >  > --- linux-2.6.24.3.orig/drivers/net/wireless/b43/dma.c        2008-02-26 23:39:39.000000000 +0100
> >  > +++ linux-2.6.24.3/drivers/net/wireless/b43/dma.c     2008-02-26 23:39:43.000000000 +0100
> >  > @@ -165,7 +165,7 @@ static void op64_fill_descriptor(struct
> >  >       addrhi = (((u64) dmaaddr >> 32) & ~SSB_DMA_TRANSLATION_MASK);
> >  >       addrext = (((u64) dmaaddr >> 32) & SSB_DMA_TRANSLATION_MASK)
> >  [snip]
> >
> >  So are you actually going to do this anytime soon?
> >
> 
> Sorry for the delay. Yes, it worked fine.

Fine :)

> Btw, when booting the system with the b43 driver, I get a
> ~30 socond delay after the boot scripts report
> "Waiting for /dev to be fully populated" and all the modules
> get loaded. Is this something expected?

No. Probably firmware related.

-- 
Greetings Michael.

Re: bcm43xx regression in 2.6.24 (with patch)

[John W. Linville]

On Wed, Feb 27, 2008 at 01:12:32AM +0300, Alexey Zaytsev wrote:
> On Wed, Feb 27, 2008 at 1:04 AM, Michael Buesch <mb@bu3sch.de> wrote:

> >  Besides that the bcm43xx driver is not broken. That's the whole reason
> >  this damn thread started at all. So it can't be broken.
> >
> Can't agree here. The bcm43xx driver used to work with 2.6.23 without requiring
> any module magic.

At the risk of prolonging things... :-(

Isn't the fundamental problem here that the ssb driver claims the same
PCI IDs as the bcm43xx driver?  He have hit this same issue a number
of times: 8139too vs.  8139cp, eepro vs. e100, sk98lin vs. skge,
and I'm sure there are more.  I admit that this situation is a bit
more confusing, since the user is less likely to predict a conflict
between bcm43xx and the ssb driver.  This is especially true since
the user isn't even selecting ssb directly, but is instead selecting
the apparently unrelated b44.

Still, the bcm43xx driver is not fundamentally damaged.  This is
fundamentally a "two drivers claiming the same PCI ID" issue, not a
"you broke my driver" one.

John
-- 
John W. Linville
linville@tuxdriver.com

Re: bcm43xx regression in 2.6.24 (with patch)

[Michael Buesch]

On Tuesday 26 February 2008 23:47:57 John W. Linville wrote:
> On Wed, Feb 27, 2008 at 01:12:32AM +0300, Alexey Zaytsev wrote:
> > On Wed, Feb 27, 2008 at 1:04 AM, Michael Buesch <mb@bu3sch.de> wrote:
> 
> > >  Besides that the bcm43xx driver is not broken. That's the whole reason
> > >  this damn thread started at all. So it can't be broken.
> > >
> > Can't agree here. The bcm43xx driver used to work with 2.6.23 without requiring
> > any module magic.
> 
> At the risk of prolonging things... :-(
> 
> Isn't the fundamental problem here that the ssb driver claims the same
> PCI IDs as the bcm43xx driver?  He have hit this same issue a number
> of times: 8139too vs.  8139cp, eepro vs. e100, sk98lin vs. skge,
> and I'm sure there are more.  I admit that this situation is a bit
> more confusing, since the user is less likely to predict a conflict
> between bcm43xx and the ssb driver.  This is especially true since
> the user isn't even selecting ssb directly, but is instead selecting
> the apparently unrelated b44.
> 
> Still, the bcm43xx driver is not fundamentally damaged.  This is
> fundamentally a "two drivers claiming the same PCI ID" issue, not a
> "you broke my driver" one.

You got the point exactly right John. :)
The special "problem" about this is that the b44 driver, which _seems_
to have nothing to do with b43, registers the b43 PCI IDs.
(This is just for convenience. See the comment in the header of
b43_pci_bridge.c for details).
So if you use the b44 driver, but also want to use the bcm43xx driver,
you have to load the bcm43xx driver first to make it probe before
b44 registers the b43 PCI IDs.

The bcm43xx driver is not broken _at_ _all_. In fact, its code did not
even slightly change since ages.

I do see the issue, but I don't feel like changing the complicated SSB
KConfig stuff (it has lots of weird dependencies) to fix this bug
that is actually trivial to work around. Besides that, how many people
do we have, which do have a b44 card plus a bcm4311 rev 1 (there are _LOTS_
of revisions of the 43xx hardware)?

So I'd rather fix b43 to work on the card. See the patch I just sent.

-- 
Greetings Michael.

Re: bcm43xx regression in 2.6.24 (with patch)

[Alexey Zaytsev]

On Wed, Feb 27, 2008 at 1:47 AM, John W. Linville
<linville@tuxdriver.com> wrote:
> On Wed, Feb 27, 2008 at 01:12:32AM +0300, Alexey Zaytsev wrote:
>  > On Wed, Feb 27, 2008 at 1:04 AM, Michael Buesch <mb@bu3sch.de> wrote:
>
>
> > >  Besides that the bcm43xx driver is not broken. That's the whole reason
>  > >  this damn thread started at all. So it can't be broken.
>  > >
>  > Can't agree here. The bcm43xx driver used to work with 2.6.23 without requiring
>  > any module magic.
>
>  At the risk of prolonging things... :-(
>
>  Isn't the fundamental problem here that the ssb driver claims the same
>  PCI IDs as the bcm43xx driver?  He have hit this same issue a number
>  of times: 8139too vs.  8139cp, eepro vs. e100, sk98lin vs. skge,
>  and I'm sure there are more.  I admit that this situation is a bit
>  more confusing, since the user is less likely to predict a conflict
>  between bcm43xx and the ssb driver.  This is especially true since
>  the user isn't even selecting ssb directly, but is instead selecting
>  the apparently unrelated b44.
>
>  Still, the bcm43xx driver is not fundamentally damaged.  This is
>  fundamentally a "two drivers claiming the same PCI ID" issue, not a
>  "you broke my driver" one.

Is there any reason the ssb driver should claim the bcm43xx pci ids in
the first place? I have very little understanding what the Sonic Silicon
Backplane really is, but I see that the b44 driver claims its PCI ids
directly. I also think I understand why the b43/b43legacy drivers can't
claim the ids directly: because the driver-device matching is done not
with the pci bus methods, but with the ssb bus methods, and it would
be impossible to automatically choose the right driver for the right
device (with same ssb ids), as the first of the two drivers loaded would
succeed in probe()'ing the pci "ssb bridge" device, and not letting the
other to take control, even after moments later the ssb probe for the
non-supported ssb device would fail. (Or am I completely wrong?)

That said, I still think that the ssb driver claims the wrong pci ids,
which is especially wrong if the the b43/b43legacy drivers are not
even built. And my patch fixes exactly this problem - the ssb driver
no more claims the broadcom pci ids, when the b43/b43legacy drivers
are not built.

One better solution I think might be to move the b43_pci_bridge.c
code to a separate module, and let the b43/b43legacy drivers
depend on it, but as I said, I have little knowledge in the
ssb stuff, so I did it the easy way.

Btw, at least in the 8139cp/8139too case the right driver is
chosen automatically, because they both can detect
the problem at the pci probe() time and tell the device
is not supported early.

>
>  John
>  --
>
>
> John W. Linville
>  linville@tuxdriver.com
>

Re: bcm43xx regression in 2.6.24 (with patch)

[Michael Buesch]

On Wednesday 27 February 2008 01:23:17 Alexey Zaytsev wrote:
> On Wed, Feb 27, 2008 at 1:47 AM, John W. Linville
> <linville@tuxdriver.com> wrote:
> > On Wed, Feb 27, 2008 at 01:12:32AM +0300, Alexey Zaytsev wrote:
> >  > On Wed, Feb 27, 2008 at 1:04 AM, Michael Buesch <mb@bu3sch.de> wrote:
> >
> >
> > > >  Besides that the bcm43xx driver is not broken. That's the whole reason
> >  > >  this damn thread started at all. So it can't be broken.
> >  > >
> >  > Can't agree here. The bcm43xx driver used to work with 2.6.23 without requiring
> >  > any module magic.
> >
> >  At the risk of prolonging things... :-(
> >
> >  Isn't the fundamental problem here that the ssb driver claims the same
> >  PCI IDs as the bcm43xx driver?  He have hit this same issue a number
> >  of times: 8139too vs.  8139cp, eepro vs. e100, sk98lin vs. skge,
> >  and I'm sure there are more.  I admit that this situation is a bit
> >  more confusing, since the user is less likely to predict a conflict
> >  between bcm43xx and the ssb driver.  This is especially true since
> >  the user isn't even selecting ssb directly, but is instead selecting
> >  the apparently unrelated b44.
> >
> >  Still, the bcm43xx driver is not fundamentally damaged.  This is
> >  fundamentally a "two drivers claiming the same PCI ID" issue, not a
> >  "you broke my driver" one.
> 
> Is there any reason the ssb driver should claim the bcm43xx pci ids in
> the first place? I have very little understanding what the Sonic Silicon
> Backplane really is, but I see that the b44 driver claims its PCI ids
> directly. I also think I understand why the b43/b43legacy drivers can't
> claim the ids directly: because the driver-device matching is done not
> with the pci bus methods, but with the ssb bus methods, and it would
> be impossible to automatically choose the right driver for the right
> device (with same ssb ids), as the first of the two drivers loaded would
> succeed in probe()'ing the pci "ssb bridge" device, and not letting the
> other to take control, even after moments later the ssb probe for the
> non-supported ssb device would fail. (Or am I completely wrong?)
> 
> That said, I still think that the ssb driver claims the wrong pci ids,
> which is especially wrong if the the b43/b43legacy drivers are not
> even built. And my patch fixes exactly this problem - the ssb driver
> no more claims the broadcom pci ids, when the b43/b43legacy drivers
> are not built.
> 
> One better solution I think might be to move the b43_pci_bridge.c
> code to a separate module, and let the b43/b43legacy drivers
> depend on it, but as I said, I have little knowledge in the
> ssb stuff, so I did it the easy way.

See the comment in b43_pci_bridge.c

-- 
Greetings Michael.

Re: bcm43xx regression in 2.6.24 (with patch)

[Alexey Zaytsev]

On Wed, Feb 27, 2008 at 3:27 AM, Michael Buesch <mb@bu3sch.de> wrote:
>
> On Wednesday 27 February 2008 01:23:17 Alexey Zaytsev wrote:
>  > On Wed, Feb 27, 2008 at 1:47 AM, John W. Linville
>  > <linville@tuxdriver.com> wrote:
>  > > On Wed, Feb 27, 2008 at 01:12:32AM +0300, Alexey Zaytsev wrote:
>  > >  > On Wed, Feb 27, 2008 at 1:04 AM, Michael Buesch <mb@bu3sch.de> wrote:
>  > >
>  > >
>  > > > >  Besides that the bcm43xx driver is not broken. That's the whole reason
>  > >  > >  this damn thread started at all. So it can't be broken.
>  > >  > >
>  > >  > Can't agree here. The bcm43xx driver used to work with 2.6.23 without requiring
>  > >  > any module magic.
>  > >
>  > >  At the risk of prolonging things... :-(
>  > >
>  > >  Isn't the fundamental problem here that the ssb driver claims the same
>  > >  PCI IDs as the bcm43xx driver?  He have hit this same issue a number
>  > >  of times: 8139too vs.  8139cp, eepro vs. e100, sk98lin vs. skge,
>  > >  and I'm sure there are more.  I admit that this situation is a bit
>  > >  more confusing, since the user is less likely to predict a conflict
>  > >  between bcm43xx and the ssb driver.  This is especially true since
>  > >  the user isn't even selecting ssb directly, but is instead selecting
>  > >  the apparently unrelated b44.
>  > >
>  > >  Still, the bcm43xx driver is not fundamentally damaged.  This is
>  > >  fundamentally a "two drivers claiming the same PCI ID" issue, not a
>  > >  "you broke my driver" one.
>  >
>  > Is there any reason the ssb driver should claim the bcm43xx pci ids in
>  > the first place? I have very little understanding what the Sonic Silicon
>  > Backplane really is, but I see that the b44 driver claims its PCI ids
>  > directly. I also think I understand why the b43/b43legacy drivers can't
>  > claim the ids directly: because the driver-device matching is done not
>  > with the pci bus methods, but with the ssb bus methods, and it would
>  > be impossible to automatically choose the right driver for the right
>  > device (with same ssb ids), as the first of the two drivers loaded would
>  > succeed in probe()'ing the pci "ssb bridge" device, and not letting the
>  > other to take control, even after moments later the ssb probe for the
>  > non-supported ssb device would fail. (Or am I completely wrong?)
>  >
>  > That said, I still think that the ssb driver claims the wrong pci ids,
>  > which is especially wrong if the the b43/b43legacy drivers are not
>  > even built. And my patch fixes exactly this problem - the ssb driver
>  > no more claims the broadcom pci ids, when the b43/b43legacy drivers
>  > are not built.
>  >
>  > One better solution I think might be to move the b43_pci_bridge.c
>  > code to a separate module, and let the b43/b43legacy drivers
>  > depend on it, but as I said, I have little knowledge in the
>  > ssb stuff, so I did it the easy way.
>
>  See the comment in b43_pci_bridge.c
>
Yes, I've seen it. And this design, kind of, causes me some trouble. ^

>  --
>  Greetings Michael.
>

Re: bcm43xx regression in 2.6.24 (with patch)

[Michael Buesch]

On Wednesday 27 February 2008 01:32:21 Alexey Zaytsev wrote:
> On Wed, Feb 27, 2008 at 3:27 AM, Michael Buesch <mb@bu3sch.de> wrote:
> >
> > On Wednesday 27 February 2008 01:23:17 Alexey Zaytsev wrote:
> >  > On Wed, Feb 27, 2008 at 1:47 AM, John W. Linville
> >  > <linville@tuxdriver.com> wrote:
> >  > > On Wed, Feb 27, 2008 at 01:12:32AM +0300, Alexey Zaytsev wrote:
> >  > >  > On Wed, Feb 27, 2008 at 1:04 AM, Michael Buesch <mb@bu3sch.de> wrote:
> >  > >
> >  > >
> >  > > > >  Besides that the bcm43xx driver is not broken. That's the whole reason
> >  > >  > >  this damn thread started at all. So it can't be broken.
> >  > >  > >
> >  > >  > Can't agree here. The bcm43xx driver used to work with 2.6.23 without requiring
> >  > >  > any module magic.
> >  > >
> >  > >  At the risk of prolonging things... :-(
> >  > >
> >  > >  Isn't the fundamental problem here that the ssb driver claims the same
> >  > >  PCI IDs as the bcm43xx driver?  He have hit this same issue a number
> >  > >  of times: 8139too vs.  8139cp, eepro vs. e100, sk98lin vs. skge,
> >  > >  and I'm sure there are more.  I admit that this situation is a bit
> >  > >  more confusing, since the user is less likely to predict a conflict
> >  > >  between bcm43xx and the ssb driver.  This is especially true since
> >  > >  the user isn't even selecting ssb directly, but is instead selecting
> >  > >  the apparently unrelated b44.
> >  > >
> >  > >  Still, the bcm43xx driver is not fundamentally damaged.  This is
> >  > >  fundamentally a "two drivers claiming the same PCI ID" issue, not a
> >  > >  "you broke my driver" one.
> >  >
> >  > Is there any reason the ssb driver should claim the bcm43xx pci ids in
> >  > the first place? I have very little understanding what the Sonic Silicon
> >  > Backplane really is, but I see that the b44 driver claims its PCI ids
> >  > directly. I also think I understand why the b43/b43legacy drivers can't
> >  > claim the ids directly: because the driver-device matching is done not
> >  > with the pci bus methods, but with the ssb bus methods, and it would
> >  > be impossible to automatically choose the right driver for the right
> >  > device (with same ssb ids), as the first of the two drivers loaded would
> >  > succeed in probe()'ing the pci "ssb bridge" device, and not letting the
> >  > other to take control, even after moments later the ssb probe for the
> >  > non-supported ssb device would fail. (Or am I completely wrong?)
> >  >
> >  > That said, I still think that the ssb driver claims the wrong pci ids,
> >  > which is especially wrong if the the b43/b43legacy drivers are not
> >  > even built. And my patch fixes exactly this problem - the ssb driver
> >  > no more claims the broadcom pci ids, when the b43/b43legacy drivers
> >  > are not built.
> >  >
> >  > One better solution I think might be to move the b43_pci_bridge.c
> >  > code to a separate module, and let the b43/b43legacy drivers
> >  > depend on it, but as I said, I have little knowledge in the
> >  > ssb stuff, so I did it the easy way.
> >
> >  See the comment in b43_pci_bridge.c
> >
> Yes, I've seen it. And this design, kind of, causes me some trouble.

There are several reasons to not do a seperate module.

First one being: People won't load it and complain about a regression.
Yeah, stupid stuff like that happens all the time. That's the reason we
SELECT the ssb code instead of using DEPENDS ON. People will otherwise not
enable it and report regressions.
And please don't say this won't happen. It _did_ happen when b44-PCI was
a seperate kconfig option. People reported regressions, although there were none.

Second one being: It's overkill to have a seperate module for two lines of code
and a PCI ID table.

Third one being: We want the code to be as small as possible, as it must
all run on embedded machines. In general being as small as possible should
be the way to go.

-- 
Greetings Michael.

Re: bcm43xx regression in 2.6.24 (with patch)

[Alexey Zaytsev]

On Wed, Feb 27, 2008 at 3:43 AM, Michael Buesch <mb@bu3sch.de> wrote:
>
> On Wednesday 27 February 2008 01:32:21 Alexey Zaytsev wrote:
>  > On Wed, Feb 27, 2008 at 3:27 AM, Michael Buesch <mb@bu3sch.de> wrote:
>  > >
>  > > On Wednesday 27 February 2008 01:23:17 Alexey Zaytsev wrote:
>  > >  > On Wed, Feb 27, 2008 at 1:47 AM, John W. Linville
>  > >  > <linville@tuxdriver.com> wrote:
>  > >  > > On Wed, Feb 27, 2008 at 01:12:32AM +0300, Alexey Zaytsev wrote:
>  > >  > >  > On Wed, Feb 27, 2008 at 1:04 AM, Michael Buesch <mb@bu3sch.de> wrote:
>  > >  > >
>  > >  > >
>  > >  > > > >  Besides that the bcm43xx driver is not broken. That's the whole reason
>  > >  > >  > >  this damn thread started at all. So it can't be broken.
>  > >  > >  > >
>  > >  > >  > Can't agree here. The bcm43xx driver used to work with 2.6.23 without requiring
>  > >  > >  > any module magic.
>  > >  > >
>  > >  > >  At the risk of prolonging things... :-(
>  > >  > >
>  > >  > >  Isn't the fundamental problem here that the ssb driver claims the same
>  > >  > >  PCI IDs as the bcm43xx driver?  He have hit this same issue a number
>  > >  > >  of times: 8139too vs.  8139cp, eepro vs. e100, sk98lin vs. skge,
>  > >  > >  and I'm sure there are more.  I admit that this situation is a bit
>  > >  > >  more confusing, since the user is less likely to predict a conflict
>  > >  > >  between bcm43xx and the ssb driver.  This is especially true since
>  > >  > >  the user isn't even selecting ssb directly, but is instead selecting
>  > >  > >  the apparently unrelated b44.
>  > >  > >
>  > >  > >  Still, the bcm43xx driver is not fundamentally damaged.  This is
>  > >  > >  fundamentally a "two drivers claiming the same PCI ID" issue, not a
>  > >  > >  "you broke my driver" one.
>  > >  >
>  > >  > Is there any reason the ssb driver should claim the bcm43xx pci ids in
>  > >  > the first place? I have very little understanding what the Sonic Silicon
>  > >  > Backplane really is, but I see that the b44 driver claims its PCI ids
>  > >  > directly. I also think I understand why the b43/b43legacy drivers can't
>  > >  > claim the ids directly: because the driver-device matching is done not
>  > >  > with the pci bus methods, but with the ssb bus methods, and it would
>  > >  > be impossible to automatically choose the right driver for the right
>  > >  > device (with same ssb ids), as the first of the two drivers loaded would
>  > >  > succeed in probe()'ing the pci "ssb bridge" device, and not letting the
>  > >  > other to take control, even after moments later the ssb probe for the
>  > >  > non-supported ssb device would fail. (Or am I completely wrong?)
>  > >  >
>  > >  > That said, I still think that the ssb driver claims the wrong pci ids,
>  > >  > which is especially wrong if the the b43/b43legacy drivers are not
>  > >  > even built. And my patch fixes exactly this problem - the ssb driver
>  > >  > no more claims the broadcom pci ids, when the b43/b43legacy drivers
>  > >  > are not built.
>  > >  >
>  > >  > One better solution I think might be to move the b43_pci_bridge.c
>  > >  > code to a separate module, and let the b43/b43legacy drivers
>  > >  > depend on it, but as I said, I have little knowledge in the
>  > >  > ssb stuff, so I did it the easy way.
>  > >
>  > >  See the comment in b43_pci_bridge.c
>  > >
>  > Yes, I've seen it. And this design, kind of, causes me some trouble.
>
>  There are several reasons to not do a seperate module.
>
>  First one being: People won't load it and complain about a regression.
>  Yeah, stupid stuff like that happens all the time. That's the reason we
>  SELECT the ssb code instead of using DEPENDS ON. People will otherwise not
>  enable it and report regressions.
>  And please don't say this won't happen. It _did_ happen when b44-PCI was
>  a seperate kconfig option. People reported regressions, although there were none.
>
I mean, not to make it a separate config option, of course it should
be hidden and
autoselected, but a separate module with maybe a symbol both b43/b43legacy
could depend on.

>  Second one being: It's overkill to have a seperate module for two lines of code
>  and a PCI ID table.
>
>  Third one being: We want the code to be as small as possible, as it must
>  all run on embedded machines. In general being as small as possible should
>  be the way to go.
>

If compiled statically, which I think is the right thing for embedded systems,
it should not take much additional space when separated from the ssb code.