Re: bcm43xx regression in 2.6.24 (with patch)

From: Michael Buesch
Date: Tue Feb 26 2008 - 19:44:19 EST


On Wednesday 27 February 2008 01:32:21 Alexey Zaytsev wrote:
> On Wed, Feb 27, 2008 at 3:27 AM, Michael Buesch <mb@xxxxxxxxx> 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@xxxxxxxxxxxxx> 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@xxxxxxxxx> 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.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/