Patchwork brcm80211 breakage..

login
register
mail settings
Submitter John W. Linville
Date Jan. 31, 2012, 7:25 p.m.
Message ID <20120131192520.GA2974@tuxdriver.com>
Download mbox | patch
Permalink /patch/138845/
State Not Applicable
Delegated to: David Miller
Headers show

Comments

John W. Linville - Jan. 31, 2012, 7:25 p.m.
On Tue, Jan 31, 2012 at 08:08:06PM +0100, Arend van Spriel wrote:
> On 01/31/2012 06:02 PM, Linus Torvalds wrote:
> > On Tue, Jan 31, 2012 at 4:08 AM, Henrik Rydberg <rydberg@euromail.se> wrote:
> >>
> >> probing simply fails on my machine,
> >>
> >> [  882.064120] bcma: Failed to get SPROM: -71
> >> [  882.064256] bcma-pci-bridge: probe of 0000:01:00.0 failed with error -2
> >>
> >> resulting in no wireless. Since the bcma SPROM setup allegedly is
> >> unused, simply ignoring the error with this hack
> > 
> > I personally hit the "No SPROM available" case, which is apparently
> > why it works for me.
> > 
> > That said, I obviously agree with your patch: if we don't fail for the
> > "No SPROM available" case, then we damn well shouldn't fail just
> > because it *is* available but we can't read it or whatever.
> > 
> > So Ack on the patch from me, but it should go through the network
> > guys. Arend, John?
> > 
> 
> The SPROM code in bcma needs some attention, because the availability of
> it depends between chipsets. I think in bcma not all variations are
> known and thus lacking implementation to handle those. So ultimately
> bcma sprom code needs to be extended (looking at brcmsmac code). The
> sprom code in brcmsmac can go when that has happened.
> 
> I will dive into that. I have not hit the "Failed to get SPROM" case so
> any chipset info would be appreciated (Henrik?).

Making the SPROM code be better informed sounds great.  In the
meantime, Henrik's patch seems reasonable.  Any objections?

---

From 1f3587dd7cdb8e2a505c76041bd5b816e7bcfb67 Mon Sep 17 00:00:00 2001
From: Henrik Rydberg <rydberg@euromail.se>
Date: Tue, 31 Jan 2012 14:22:15 -0500
Subject: [PATCH] bcma: don't fail for bad SPROM CRC

If it is OK not to have an SPROM, it should be OK to have a bad one...?

Signed-off-by: John W. Linville <linville@tuxdriver.com>
---
Henrik, may I add a Signed-off-by line for you?

 drivers/bcma/main.c |    4 +---
 1 files changed, 1 insertions(+), 3 deletions(-)
Arend van Spriel - Jan. 31, 2012, 7:35 p.m.
On 01/31/2012 08:25 PM, John W. Linville wrote:
> On Tue, Jan 31, 2012 at 08:08:06PM +0100, Arend van Spriel wrote:
>> On 01/31/2012 06:02 PM, Linus Torvalds wrote:
>>> On Tue, Jan 31, 2012 at 4:08 AM, Henrik Rydberg <rydberg@euromail.se> wrote:
>>>>
>>>> probing simply fails on my machine,
>>>>
>>>> [  882.064120] bcma: Failed to get SPROM: -71
>>>> [  882.064256] bcma-pci-bridge: probe of 0000:01:00.0 failed with error -2
>>>>
>>>> resulting in no wireless. Since the bcma SPROM setup allegedly is
>>>> unused, simply ignoring the error with this hack
>>>
>>> I personally hit the "No SPROM available" case, which is apparently
>>> why it works for me.
>>>
>>> That said, I obviously agree with your patch: if we don't fail for the
>>> "No SPROM available" case, then we damn well shouldn't fail just
>>> because it *is* available but we can't read it or whatever.
>>>
>>> So Ack on the patch from me, but it should go through the network
>>> guys. Arend, John?
>>>
>>
>> The SPROM code in bcma needs some attention, because the availability of
>> it depends between chipsets. I think in bcma not all variations are
>> known and thus lacking implementation to handle those. So ultimately
>> bcma sprom code needs to be extended (looking at brcmsmac code). The
>> sprom code in brcmsmac can go when that has happened.
>>
>> I will dive into that. I have not hit the "Failed to get SPROM" case so
>> any chipset info would be appreciated (Henrik?).
> 
> Making the SPROM code be better informed sounds great.  In the
> meantime, Henrik's patch seems reasonable.  Any objections?
> 

Yeah. Forgot to mention, but as it does not break stuff Ack from me.

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
Henrik Rydberg - Jan. 31, 2012, 7:58 p.m.
> From 1f3587dd7cdb8e2a505c76041bd5b816e7bcfb67 Mon Sep 17 00:00:00 2001
> From: Henrik Rydberg <rydberg@euromail.se>
> Date: Tue, 31 Jan 2012 14:22:15 -0500
> Subject: [PATCH] bcma: don't fail for bad SPROM CRC
> 
> If it is OK ...?

How about this:

The brcmsmac driver is now using the bcma SPROM logic, which does not
recognize all chipsets that were functional prior to the switch. In
particular, the current code bails out on odd CRC errors in recent
Macbooks. This patch ignores those errors, with the argument that an
unrecognized SPROM should be treated similarly to a non-existing one.

> 
> Signed-off-by: John W. Linville <linville@tuxdriver.com>
> ---
> Henrik, may I add a Signed-off-by line for you?

By all means, thanks.

>  drivers/bcma/main.c |    4 +---
>  1 files changed, 1 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/bcma/main.c b/drivers/bcma/main.c
> index febbc0a..ec31f7d 100644
> --- a/drivers/bcma/main.c
> +++ b/drivers/bcma/main.c
> @@ -169,10 +169,8 @@ int bcma_bus_register(struct bcma_bus *bus)
>  	err = bcma_sprom_get(bus);
>  	if (err == -ENOENT) {
>  		pr_err("No SPROM available\n");
> -	} else if (err) {
> +	} else if (err)
>  		pr_err("Failed to get SPROM: %d\n", err);
> -		return -ENOENT;
> -	}
>  
>  	/* Register found cores */
>  	bcma_register_cores(bus);
> -- 
> 1.7.4.4

Henrik
--
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
Arend van Spriel - Jan. 31, 2012, 8:21 p.m.
On 01/31/2012 08:58 PM, Henrik Rydberg wrote:
>> From 1f3587dd7cdb8e2a505c76041bd5b816e7bcfb67 Mon Sep 17 00:00:00 2001
>> From: Henrik Rydberg <rydberg@euromail.se>
>> Date: Tue, 31 Jan 2012 14:22:15 -0500
>> Subject: [PATCH] bcma: don't fail for bad SPROM CRC
>>
>> If it is OK ...?
> 
> How about this:
> 
> The brcmsmac driver is now using the bcma SPROM logic, which does not
> recognize all chipsets that were functional prior to the switch. In
> particular, the current code bails out on odd CRC errors in recent
> Macbooks. This patch ignores those errors, with the argument that an
> unrecognized SPROM should be treated similarly to a non-existing one.
>

Not entirely correct. BCMA sprom logic is currently not used by
brcmsmac, but it sure (as hell) makes sense to do so. Currently, the
bcma SPROM logic is used in SoC chipsets (bcm47xx) and the b43 driver.

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
Henrik Rydberg - Jan. 31, 2012, 10:23 p.m.
> > The brcmsmac driver is now using the bcma SPROM logic, which does not
> > recognize all chipsets that were functional prior to the switch. In
> > particular, the current code bails out on odd CRC errors in recent
> > Macbooks. This patch ignores those errors, with the argument that an
> > unrecognized SPROM should be treated similarly to a non-existing one.
> >
> 
> Not entirely correct. BCMA sprom logic is currently not used by
> brcmsmac, but it sure (as hell) makes sense to do so. Currently, the
> bcma SPROM logic is used in SoC chipsets (bcm47xx) and the b43 driver.

Let's change that to "bcma SPROM CRC check", then. With -rc2 just out, I
take it the patch will appear in -rc3?

Thanks,
Henrik
--
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/main.c b/drivers/bcma/main.c
index febbc0a..ec31f7d 100644
--- a/drivers/bcma/main.c
+++ b/drivers/bcma/main.c
@@ -169,10 +169,8 @@  int bcma_bus_register(struct bcma_bus *bus)
 	err = bcma_sprom_get(bus);
 	if (err == -ENOENT) {
 		pr_err("No SPROM available\n");
-	} else if (err) {
+	} else if (err)
 		pr_err("Failed to get SPROM: %d\n", err);
-		return -ENOENT;
-	}
 
 	/* Register found cores */
 	bcma_register_cores(bus);