Patchwork brcm80211 breakage..

login
register
mail settings
Submitter Arend van Spriel
Date Jan. 11, 2012, 10:44 a.m.
Message ID <4F0D6806.4080201@broadcom.com>
Download mbox | patch
Permalink /patch/135375/
State Not Applicable
Delegated to: David Miller
Headers show

Comments

Arend van Spriel - Jan. 11, 2012, 10:44 a.m.
On 01/10/2012 09:15 PM, Linus Torvalds wrote:
> So the wireless on my Macbook Air no longer seems to work with the
> current -git tree.
> 
> The BRCMSMAC driver *used* to work, and no longer does. Also, very
> annoyingly, it's even hard to *compile* the thing, because it used to
> be
> 
>   depends on BCMA=n
> 
> but now it is the exact reverse:
> 
>   depends on BCMA
> 
> so there is no sane way to carry a configuration over from before, and
> things like bisection is a major pain due to having to play idiotic
> configuration games to get it to work across all these modifications.

BCMA support has been added to brcmsmac so now we depend on it. As BCMA
claims the PCI device we had the BCMA=n in place before. I agree that it
makes bisecting a pain in the...

> Ragardless, even once you actually enable BCMA and can get the driver
> to come back, it just doesn't do anything. Maybe there is some
> remaining config problem, but I don't think so. I think the driver is
> just buggered.

Looking at the output I had deja-vu feeling. The issue popped up when we
published part of the patches for BCMA support. I looked into it with
Larry Finger and Rafał Miłecki. We found the reason for the failure, but
it seemed resolved after submitting the remaining patches for BCMA
support. However, it seems to have raised its ugly head.

> Please look into this. I'll obviously be happy test any reasonable suggestions,
> 
>                  Linus
> 

My theory is that the BAR window is not mapped to a valid address. Could
you try to revert the following commit:

commit 439678f8b0fca7aeca06c6581e3679eef618721a
Author: Rafał Miłecki <zajec5@gmail.com>
Date:   Mon Dec 5 19:13:39 2011 +0100

    bcma: pci: use fixed windows when possible

    Some cores are mapped in the fixed way, they registers can be accessed
    all the time.

    Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
    Signed-off-by: John W. Linville <linville@tuxdriver.com>

I have attached the 'revert patch' based on your git master branch.

Gr. AvS
From 71e949a668bc8c371dc21f17af01284719f1b383 Mon Sep 17 00:00:00 2001
From: Arend van Spriel <arend@broadcom.com>
Date: Wed, 11 Jan 2012 11:35:05 +0100
Subject: [PATCH] Revert "bcma: pci: use fixed windows when possible"

This reverts commit 439678f8b0fca7aeca06c6581e3679eef618721a.
---
 drivers/bcma/host_pci.c |   32 +++++++++++---------------------
 1 files changed, 11 insertions(+), 21 deletions(-)
Linus Torvalds - Jan. 11, 2012, 3:05 p.m.
On Wed, Jan 11, 2012 at 2:44 AM, Arend van Spriel <arend@broadcom.com> wrote:
>
> My theory is that the BAR window is not mapped to a valid address. Could
> you try to revert the following commit:

Sadly, that one makes no difference. I'll enable BRCM/BCMA debugging
and see if that gives any more information, but will be unable to do
much for the next few hours, so it will likely not be until this
afternoon.

                            Linus
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Linus Torvalds - Jan. 11, 2012, 4:04 p.m.
On Wed, Jan 11, 2012 at 7:05 AM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> Sadly, that one makes no difference. I'll enable BRCM/BCMA debugging
> and see if that gives any more information, but will be unable to do
> much for the next few hours, so it will likely not be until this
> afternoon.

Ok, could do it now. But that really doesn't give much more
information. Here it is anyway:

[    0.746530] bcma-pci-bridge 0000:02:00.0: PCI INT A -> GSI 17
(level, low) -> IRQ 17
[    0.746544] bcma-pci-bridge 0000:02:00.0: setting latency timer to 64
[    0.746617] bcma: Core 0 found: ChipCommon (manuf 0x4BF, id 0x800,
rev 0x22, class 0x0)
[    0.746646] bcma: Core 1 found: IEEE 802.11 (manuf 0x4BF, id 0x812,
rev 0x17, class 0x0)
[    0.746709] bcma: Core 2 found: PCIe (manuf 0x4BF, id 0x820, rev
0x0F, class 0x0)
[    0.746845] bcma: Switched to core: 0x800
[    0.746859] bcma: Found rev 6 PMU (capabilities 0x108C2606)
[    0.746880] bcma: Switched to core: 0x820
[    0.775510] bcma: Switched to core: 0x800
[    0.815750] bcma: Unsupported SPROM revision: 255
[    0.815804] bcma: No SPROM available
[    0.815909] bcma: Bus registered
[   41.797178] brcmsmac bcma0:0: mfg 4bf core 812 rev 23 class 0 irq 17
[   41.952030] bcma: Switched to core: 0x820
[   41.952050] bcma: Switched to core: 0x800
[   41.953881] ieee80211 phy0: wl0: brcms_b_attach: si_attach failed
[   41.956397] ieee80211 phy0: wl0: brcms_b_attach: failed with err 11
[   41.958443] ieee80211 phy0: wl0: brcms_c_attach: failed with err 11
[   41.960468] ieee80211 phy0: brcmsmac: attach() failed with code 11
[   41.962316] brcmsmac: brcms_bcma_probe: brcms_attach failed!
[   41.967275] brcms_module_init: register returned 0

hopefully this gives people *some* idea, but I'm not seeing anything.

         Linus
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ben Hutchings - Jan. 11, 2012, 5:15 p.m.
On Wed, 2012-01-11 at 11:44 +0100, Arend van Spriel wrote:
> On 01/10/2012 09:15 PM, Linus Torvalds wrote:
> > So the wireless on my Macbook Air no longer seems to work with the
> > current -git tree.
> > 
> > The BRCMSMAC driver *used* to work, and no longer does. Also, very
> > annoyingly, it's even hard to *compile* the thing, because it used to
> > be
> > 
> >   depends on BCMA=n
> > 
> > but now it is the exact reverse:
> > 
> >   depends on BCMA
> > 
> > so there is no sane way to carry a configuration over from before, and
> > things like bisection is a major pain due to having to play idiotic
> > configuration games to get it to work across all these modifications.
> 
> BCMA support has been added to brcmsmac so now we depend on it. As BCMA
> claims the PCI device we had the BCMA=n in place before. I agree that it
> makes bisecting a pain in the...
[...]

Maybe BRCMSMAC should select BCMA rather than depending on it.  This bus
seems like kind of an implementation detail that people are unlikely to
be aware of, unlike say PCI.

Ben.
Arend van Spriel - Jan. 11, 2012, 9:01 p.m.
On 01/11/2012 05:04 PM, Linus Torvalds wrote:
> On Wed, Jan 11, 2012 at 7:05 AM, Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
>>
>> Sadly, that one makes no difference. I'll enable BRCM/BCMA debugging
>> and see if that gives any more information, but will be unable to do
>> much for the next few hours, so it will likely not be until this
>> afternoon.

There goes my theory. I have no MacBook Air over here and we have
nightly testing running for this chip. I will have it run on your branch
iso wireless to see if I can reproduce it over here.

> Ok, could do it now. But that really doesn't give much more
> information. Here it is anyway:
> [   41.797178] brcmsmac bcma0:0: mfg 4bf core 812 rev 23 class 0 irq 17
> [   41.952030] bcma: Switched to core: 0x820
> [   41.952050] bcma: Switched to core: 0x800
> [   41.953881] ieee80211 phy0: wl0: brcms_b_attach: si_attach failed
> [   41.956397] ieee80211 phy0: wl0: brcms_b_attach: failed with err 11
> [   41.958443] ieee80211 phy0: wl0: brcms_c_attach: failed with err 11
> [   41.960468] ieee80211 phy0: brcmsmac: attach() failed with code 11
> [   41.962316] brcmsmac: brcms_bcma_probe: brcms_attach failed!
> [   41.967275] brcms_module_init: register returned 0
> 
> hopefully this gives people *some* idea, but I'm not seeing anything.
> 
>          Linus
> 

I have some idea, but no success in reproducing it. Could you provide
the .config used on this?

Gr. AvS

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Linus Torvalds - Jan. 12, 2012, 12:07 a.m.
2012/1/11 Arend van Spriel <arend@broadcom.com>:
>
> I have some idea, but no success in reproducing it. Could you provide
> the .config used on this?

Attached.

I'm also attaching the lspci output from a working configuration and a
nonworking one. The only thing that stands out is the ">TAbort+" in
the status of the broken one. Some transfer has gone wrong, it looks
like.

              Linus
Linus Torvalds - Jan. 12, 2012, 12:10 a.m.
On Wed, Jan 11, 2012 at 9:15 AM, Ben Hutchings
<bhutchings@solarflare.com> wrote:
>
> Maybe BRCMSMAC should select BCMA rather than depending on it.  This bus
> seems like kind of an implementation detail that people are unlikely to
> be aware of, unlike say PCI.

Yes, please. This is exactly the kind of thing "select" exists for.
Asking the user about some crazy proprietary broadcom bus interface is
useless, since no sane user should ever know or care about that kind
of thing.

                         Linus
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/drivers/bcma/host_pci.c b/drivers/bcma/host_pci.c
index 443b83a..990f5a8 100644
--- a/drivers/bcma/host_pci.c
+++ b/drivers/bcma/host_pci.c
@@ -21,58 +21,48 @@  static void bcma_host_pci_switch_core(struct bcma_device *core)
 	pr_debug("Switched to core: 0x%X\n", core->id.id);
 }
 
-/* Provides access to the requested core. Returns base offset that has to be
- * used. It makes use of fixed windows when possible. */
-static u16 bcma_host_pci_provide_access_to_core(struct bcma_device *core)
+static u8 bcma_host_pci_read8(struct bcma_device *core, u16 offset)
 {
-	switch (core->id.id) {
-	case BCMA_CORE_CHIPCOMMON:
-		return 3 * BCMA_CORE_SIZE;
-	case BCMA_CORE_PCIE:
-		return 2 * BCMA_CORE_SIZE;
-	}
-
 	if (core->bus->mapped_core != core)
 		bcma_host_pci_switch_core(core);
-	return 0;
-}
-
-static u8 bcma_host_pci_read8(struct bcma_device *core, u16 offset)
-{
-	offset += bcma_host_pci_provide_access_to_core(core);
 	return ioread8(core->bus->mmio + offset);
 }
 
 static u16 bcma_host_pci_read16(struct bcma_device *core, u16 offset)
 {
-	offset += bcma_host_pci_provide_access_to_core(core);
+	if (core->bus->mapped_core != core)
+		bcma_host_pci_switch_core(core);
 	return ioread16(core->bus->mmio + offset);
 }
 
 static u32 bcma_host_pci_read32(struct bcma_device *core, u16 offset)
 {
-	offset += bcma_host_pci_provide_access_to_core(core);
+	if (core->bus->mapped_core != core)
+		bcma_host_pci_switch_core(core);
 	return ioread32(core->bus->mmio + offset);
 }
 
 static void bcma_host_pci_write8(struct bcma_device *core, u16 offset,
 				 u8 value)
 {
-	offset += bcma_host_pci_provide_access_to_core(core);
+	if (core->bus->mapped_core != core)
+		bcma_host_pci_switch_core(core);
 	iowrite8(value, core->bus->mmio + offset);
 }
 
 static void bcma_host_pci_write16(struct bcma_device *core, u16 offset,
 				 u16 value)
 {
-	offset += bcma_host_pci_provide_access_to_core(core);
+	if (core->bus->mapped_core != core)
+		bcma_host_pci_switch_core(core);
 	iowrite16(value, core->bus->mmio + offset);
 }
 
 static void bcma_host_pci_write32(struct bcma_device *core, u16 offset,
 				 u32 value)
 {
-	offset += bcma_host_pci_provide_access_to_core(core);
+	if (core->bus->mapped_core != core)
+		bcma_host_pci_switch_core(core);
 	iowrite32(value, core->bus->mmio + offset);
 }