diff mbox

brcm80211 breakage..

Message ID 4F0DC030.6090500@lwfinger.net
State Not Applicable, archived
Delegated to: David Miller
Headers show

Commit Message

Larry Finger Jan. 11, 2012, 5 p.m. UTC
On 01/11/2012 10:04 AM, Linus Torvalds wrote:
> On Wed, Jan 11, 2012 at 7:05 AM, Linus Torvalds
>
> 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

I'm not a bcma or brcmsmac expert, but the above result of SPROM version 0xFF 
looks as if the read was from a non-existent register. If you get a chance, 
please try this patch, which will dump the offset that is being used.


As the TODO indicates, this part is not understood.

One other thing to try. If the new output says that the offset is 0x830, try 
forcing it to 0x800, or vice versa. I don't think you have a 4331, and I expect 
that the current code is trying 0x830, but should be using 0x800.

Larry
--
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

Comments

Linus Torvalds Jan. 12, 2012, 1:06 a.m. UTC | #1
On Wed, Jan 11, 2012 at 9:00 AM, Larry Finger <Larry.Finger@lwfinger.net> wrote:
> --- wireless-testing-new.orig/drivers/bcma/sprom.c
> +++ wireless-testing-new/drivers/bcma/sprom.c
> @@ -230,6 +230,7 @@ int bcma_sprom_get(struct bcma_bus *bus)
>         * TODO: understand this condition and use it */
>        offset = (bus->chipinfo.id == 0x4331) ? BCMA_CC_SPROM :
>                BCMA_CC_SPROM_PCIE6;
> +       pr_debug("SPROM offset 0x%x\n", offset);
>        bcma_sprom_read(bus, offset, sprom);
>
>        if (bus->chipinfo.id == 0x4331)

So I did that - the offset is printed out as 0x0830
(BCMA_CC_SPROM_PCIE6). And when I change the offset to 0x0800
(BCMA_CC_SPROM), the complaint about SPROM version goes away.

IOW, these messages no longer exist:

[    0.815750] bcma: Unsupported SPROM revision: 255
[    0.815804] bcma: No SPROM available

but that doesn't actually make anything *work*. The brcms errors still
remain the same.

So the SPROM issue seems to be real, but irrelevant.

I also noticed that the CONFIG_BRCMDBG onyl enables pr_debug(), but
doesn't actually #define DEBUG, so pr_debug() gets compiled out
anyway.

I'll try with -DDEBUG in the brcm80211 subdirectory and see if I get
more interesting output that way.

                       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
Larry Finger Jan. 12, 2012, 1:46 a.m. UTC | #2
On 01/11/2012 07:06 PM, Linus Torvalds wrote:
> On Wed, Jan 11, 2012 at 9:00 AM, Larry Finger<Larry.Finger@lwfinger.net>  wrote:
>> --- wireless-testing-new.orig/drivers/bcma/sprom.c
>> +++ wireless-testing-new/drivers/bcma/sprom.c
>> @@ -230,6 +230,7 @@ int bcma_sprom_get(struct bcma_bus *bus)
>>          * TODO: understand this condition and use it */
>>         offset = (bus->chipinfo.id == 0x4331) ? BCMA_CC_SPROM :
>>                 BCMA_CC_SPROM_PCIE6;
>> +       pr_debug("SPROM offset 0x%x\n", offset);
>>         bcma_sprom_read(bus, offset, sprom);
>>
>>         if (bus->chipinfo.id == 0x4331)
>
> So I did that - the offset is printed out as 0x0830
> (BCMA_CC_SPROM_PCIE6). And when I change the offset to 0x0800
> (BCMA_CC_SPROM), the complaint about SPROM version goes away.
>
> IOW, these messages no longer exist:
>
> [    0.815750] bcma: Unsupported SPROM revision: 255
> [    0.815804] bcma: No SPROM available
>
> but that doesn't actually make anything *work*. The brcms errors still
> remain the same.
>
> So the SPROM issue seems to be real, but irrelevant.
>
> I also noticed that the CONFIG_BRCMDBG onyl enables pr_debug(), but
> doesn't actually #define DEBUG, so pr_debug() gets compiled out
> anyway.
>
> I'll try with -DDEBUG in the brcm80211 subdirectory and see if I get
> more interesting output that way.

OK. The SPROM issue is not completely irrelevant as that has to be correct, but 
obviously not sufficient.

What is the PCI ID for your device?

Larry
--
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, 1:58 a.m. UTC | #3
On Wed, Jan 11, 2012 at 5:46 PM, Larry Finger <Larry.Finger@lwfinger.net> wrote:
>
> What is the PCI ID for your device?

You can decode it from the lspci output posted in another email in
this thread, but it's basically 14e4:4353 (rev 01).

                     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. 12, 2012, 2:11 a.m. UTC | #4
On Wed, Jan 11, 2012 at 5:46 PM, Larry Finger <Larry.Finger@lwfinger.net> wrote:
>
> OK. The SPROM issue is not completely irrelevant as that has to be correct,
> but obviously not sufficient.

Hmm. Having dug a bit deeper, I do think it's kind of related.

I get -ENODATA form sprom_read_pci(), but that function actually seems
to get the offset *right*.

Some printout shows that for that chip, I have

 - ai_get_ccrev(sih) = 34
 - sprom_offset = 0x800

but then it apparently reads all ones anyway. At least in the first
word. So then I get that -ENODATA error.

So once more, it's somehow related to the sprom, just in a new place:
sprom_read_pci() in brcmsmac/srom.c instead of drivers/bcma/sprom.c.

Does that give people any new ideas to try out?

                      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
Larry Finger Jan. 12, 2012, 4:15 a.m. UTC | #5
On 01/11/2012 08:11 PM, Linus Torvalds wrote:
> On Wed, Jan 11, 2012 at 5:46 PM, Larry Finger<Larry.Finger@lwfinger.net>  wrote:
>>
>> OK. The SPROM issue is not completely irrelevant as that has to be correct,
>> but obviously not sufficient.
>
> Hmm. Having dug a bit deeper, I do think it's kind of related.
>
> I get -ENODATA form sprom_read_pci(), but that function actually seems
> to get the offset *right*.
>
> Some printout shows that for that chip, I have
>
>   - ai_get_ccrev(sih) = 34
>   - sprom_offset = 0x800
>
> but then it apparently reads all ones anyway. At least in the first
> word. So then I get that -ENODATA error.
>
> So once more, it's somehow related to the sprom, just in a new place:
> sprom_read_pci() in brcmsmac/srom.c instead of drivers/bcma/sprom.c.
>
> Does that give people any new ideas to try out?

Things are getting curious. I have a 14e4:4353 device, which works with both b43 
and brcmsmac using mainline v3.2-6271-g925b5d2. The output of

dmesg | egrep "bcma|brcm"

with some extra debugging added yields:

bcma-pci-bridge 0000:06:00.0: PCI INT A -> Link[LK1E] -> GSI 22 (level, low) -> 
IRQ 22
bcma-pci-bridge 0000:06:00.0: setting latency timer to 64
bcma: Core 0 found: ChipCommon (manuf 0x4BF, id 0x800, rev 0x22, class 0x0)
bcma: Core 1 found: IEEE 802.11 (manuf 0x4BF, id 0x812, rev 0x17, class 0x0)
bcma: Core 2 found: PCIe (manuf 0x4BF, id 0x820, rev 0x0F, class 0x0)
bcma: Found rev 6 PMU (capabilities 0x108C2606)
bcma: SPROM offset 0x830
bcma: Found SPROM Revision 8
bcma: Bus registered
brcmsmac bcma0:0: mfg 4bf core 812 rev 23 class 0 irq 22
brcmsmac: Found chip type AI (0x1381a8d8)
brcmsmac: Applying 43224B0+ WARs
bcma: Switched to core: 0x812
brcms_module_init: register returned 0

I see no difference in the core revisions, etc. to explain why mine should work, 
and yours fail.

Arend: Any particular place we should look?

Larry

--
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, 5:20 a.m. UTC | #6
On Wed, Jan 11, 2012 at 8:15 PM, Larry Finger <Larry.Finger@lwfinger.net> wrote:
>
> I see no difference in the core revisions, etc. to explain why mine should
> work, and yours fail.

Maybe your BIOS firmware sets things up, and the Apple Macbook Air
doesn't? And the driver used to initialize things sufficiently, and
the changes have broken that?

Apple is famous for being contrary. They tend to wire things up oddly,
they don't initialize things in the BIOS (they don't have a BIOS at
all, they use EFI, but even there they use their own abortion of an
EFI rather than what everybody else does), yadda yadda.

But the real point is: it used to work, and now it doesn't. This needs
to  get fixed, or it will get reverted.

                             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. 12, 2012, 5:30 a.m. UTC | #7
On Wed, Jan 11, 2012 at 9:20 PM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> Maybe your BIOS firmware sets things up, and the Apple Macbook Air
> doesn't? And the driver used to initialize things sufficiently, and
> the changes have broken that?

The code in v3.2 also used to support srom rev < 4, the new code
doesn't seem to do that.

Looking at the older initvars_srom_pci() (now "srom_var_init()") the
old code started out reading just SROM_WORDS, and then reading more
only if it found the srom4 signature. The new code always reads
SROM4_WORDS.

I dunno. But it looks to me like the new driver has dropped some logic
that used to exist in that driver. I don't know if it was ever used,
though, nor do I know if it's relevant. But if tat "dropped some
logic" is more widespread, it might well also cover some
initialization code.

                        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
Rafał Miłecki Jan. 12, 2012, 7:08 a.m. UTC | #8
2012/1/12 Linus Torvalds <torvalds@linux-foundation.org>:
> On Wed, Jan 11, 2012 at 9:20 PM, Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
>>
>> Maybe your BIOS firmware sets things up, and the Apple Macbook Air
>> doesn't? And the driver used to initialize things sufficiently, and
>> the changes have broken that?
>
> The code in v3.2 also used to support srom rev < 4, the new code
> doesn't seem to do that.

After hacking bcma to read SPROM from the correct location, what did
you get in dmesg? There should be info about version of SPROM.

There are 2 possible reasons for this issue:
1) bcma doesn't fully init your card
2) brcmsmac does sth wrong incorrectly as init

The story with brcmsmac is that it had code duplicated with bcma at
first. Now they partially switched to bcma, which inits bus for them,
and provide access to bus devices. However I'm afraid Broadcom didn't
remove duplicated init code (correct me if i'm wrong!). They may be
re-initializing bus and it's devices which cause some problems...

Linus: is this possible for you to give b43 a chance? It has to be compiled with
B43_BCMA
B43_PHY_N
Unfortunately you have to install firmware manually to get b43 working
:( Howto is located at:
http://wireless.kernel.org/en/users/Drivers/b43
("Install b43-fwcutter" and "If you are using the b43 driver from 3.2
kernel or newer:").

b43 doesn't re-init bus devices, so this test will check if bcma is OK or not.
Rafał Miłecki Jan. 12, 2012, 7:10 a.m. UTC | #9
W dniu 12 stycznia 2012 08:08 użytkownik Rafał Miłecki
<zajec5@gmail.com> napisał:
> Linus: is this possible for you to give b43 a chance? It has to be compiled with
> B43_BCMA
> B43_PHY_N
> Unfortunately you have to install firmware manually to get b43 working
> :( Howto is located at:
> http://wireless.kernel.org/en/users/Drivers/b43
> ("Install b43-fwcutter" and "If you are using the b43 driver from 3.2
> kernel or newer:").
>
> b43 doesn't re-init bus devices, so this test will check if bcma is OK or not.

That probably will require blacklisting brcmsmac for a moment to
prevent bcma auto-loading it. Testing from cold-boot is recommended.
Linus Torvalds Jan. 12, 2012, 7:13 a.m. UTC | #10
On Wed, Jan 11, 2012 at 9:30 PM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> The code in v3.2 also used to support srom rev < 4, the new code
> doesn't seem to do that.

I ended up trying to bisect this, and seem to have been successful.
The problem came in not with the BCMA use, but with commit
888153b3db3f ("brcm80211: smac: avoid sprom endianess conversions for
crc8 check")

I haven't yet tried to revert this on top of the current tree, but I'm
pretty sure about the bisection. That commit changes things to be read
a byte at a time, but it *also* removes the old code that only read
SROM_WORDS from the SROM.

Might the apple parts have different SROM contents? I assume "SROM" is
just a serial rom, which may be external and contain things like the ?

All the BCMA changes make the revert somewhat non-trivial, could
somebody who knows the code better please try to do it for me? It
doesn't look complicated, and I can try to do it myself tomorrow if
nobody else steps up, but I'd *really* prefer the guilty parties
themselves to do it, ok?

Arend - it's your commit...

                    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
Rafał Miłecki Jan. 12, 2012, 7:18 a.m. UTC | #11
2012/1/12 Linus Torvalds <torvalds@linux-foundation.org>:
> On Wed, Jan 11, 2012 at 9:30 PM, Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
>>
>> The code in v3.2 also used to support srom rev < 4, the new code
>> doesn't seem to do that.
>
> I ended up trying to bisect this, and seem to have been successful.
> The problem came in not with the BCMA use, but with commit
> 888153b3db3f ("brcm80211: smac: avoid sprom endianess conversions for
> crc8 check")

Uh, OK, that was tricky. BCMA switch was the biggest change and we
were assuming it has to be the guilty one. Thanks for bisecting!
Linus Torvalds Jan. 12, 2012, 7:18 a.m. UTC | #12
2012/1/11 Rafał Miłecki <zajec5@gmail.com>:
>
> After hacking bcma to read SPROM from the correct location, what did
> you get in dmesg? There should be info about version of SPROM.

When I changed the offset from 830 to 800, I didn't get any sprom
version information at all.

Anyway, as mentioned in another email I just sent out, I bisected it,
and the problem actually predates the move to bcma.

                          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. 12, 2012, 7:22 a.m. UTC | #13
2012/1/11 Rafał Miłecki <zajec5@gmail.com>:
>
> Uh, OK, that was tricky. BCMA switch was the biggest change and we
> were assuming it has to be the guilty one. Thanks for bisecting!

Hey, I blamed the BCMA switch too, since it was the obvious big
change, and also caused the config problems I saw. So when the bisect
blithely went right past it and showed the exact same "si_attach
failed" messages even before that, I was all surprised.

Anyway, I'm done for today, and am hoping that somebody can pick up on
the revert effort.

                  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
Arend van Spriel Jan. 12, 2012, 10:03 a.m. UTC | #14
On 01/12/2012 08:13 AM, Linus Torvalds wrote:
> On Wed, Jan 11, 2012 at 9:30 PM, Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
>>
>> The code in v3.2 also used to support srom rev < 4, the new code
>> doesn't seem to do that.
> 
> I ended up trying to bisect this, and seem to have been successful.
> The problem came in not with the BCMA use, but with commit
> 888153b3db3f ("brcm80211: smac: avoid sprom endianess conversions for
> crc8 check")

Thanks for doing that while I had put my head on a pillow.

> I haven't yet tried to revert this on top of the current tree, but I'm
> pretty sure about the bisection. That commit changes things to be read
> a byte at a time, but it *also* removes the old code that only read
> SROM_WORDS from the SROM.
> 
> Might the apple parts have different SROM contents? I assume "SROM" is
> just a serial rom, which may be external and contain things like the ?

Mostly configuration parameters for RF/PHY.

> All the BCMA changes make the revert somewhat non-trivial, could
> somebody who knows the code better please try to do it for me? It
> doesn't look complicated, and I can try to do it myself tomorrow if
> nobody else steps up, but I'd *really* prefer the guilty parties
> themselves to do it, ok?
> 
> Arend - it's your commit...

Ok. I know a hint when I see one ;-)

>                     Linus
> 

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
Arend van Spriel Jan. 12, 2012, 1:13 p.m. UTC | #15
On 01/12/2012 05:15 AM, Larry Finger wrote:
> On 01/11/2012 08:11 PM, Linus Torvalds wrote:
>> On Wed, Jan 11, 2012 at 5:46 PM, Larry Finger<Larry.Finger@lwfinger.net>  wrote:
>>>
>>> OK. The SPROM issue is not completely irrelevant as that has to be correct,
>>> but obviously not sufficient.
>>
>> Hmm. Having dug a bit deeper, I do think it's kind of related.
>>
>> I get -ENODATA form sprom_read_pci(), but that function actually seems
>> to get the offset *right*.
>>
>> Some printout shows that for that chip, I have
>>
>>   - ai_get_ccrev(sih) = 34
>>   - sprom_offset = 0x800
>>
>> but then it apparently reads all ones anyway. At least in the first
>> word. So then I get that -ENODATA error.
>>
>> So once more, it's somehow related to the sprom, just in a new place:
>> sprom_read_pci() in brcmsmac/srom.c instead of drivers/bcma/sprom.c.
>>
>> Does that give people any new ideas to try out?
> 
> Things are getting curious. I have a 14e4:4353 device, which works with both b43 
> and brcmsmac using mainline v3.2-6271-g925b5d2. The output of
> 
> dmesg | egrep "bcma|brcm"
> 
> with some extra debugging added yields:
> 
> bcma-pci-bridge 0000:06:00.0: PCI INT A -> Link[LK1E] -> GSI 22 (level, low) -> 
> IRQ 22
> bcma-pci-bridge 0000:06:00.0: setting latency timer to 64
> bcma: Core 0 found: ChipCommon (manuf 0x4BF, id 0x800, rev 0x22, class 0x0)
> bcma: Core 1 found: IEEE 802.11 (manuf 0x4BF, id 0x812, rev 0x17, class 0x0)
> bcma: Core 2 found: PCIe (manuf 0x4BF, id 0x820, rev 0x0F, class 0x0)
> bcma: Found rev 6 PMU (capabilities 0x108C2606)
> bcma: SPROM offset 0x830
> bcma: Found SPROM Revision 8
> bcma: Bus registered
> brcmsmac bcma0:0: mfg 4bf core 812 rev 23 class 0 irq 22
> brcmsmac: Found chip type AI (0x1381a8d8)
> brcmsmac: Applying 43224B0+ WARs
> bcma: Switched to core: 0x812
> brcms_module_init: register returned 0
> 
> I see no difference in the core revisions, etc. to explain why mine should work, 
> and yours fail.
> 
> Arend: Any particular place we should look?
> 
> Larry
> 

Hi, Larry

I am surprised that we end up on sprom_read_pci(). That suggests that
MacBook Air has an external sprom. Can you tell me what the function
ai_is_sprom_available() returns on your system?

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
Larry Finger Jan. 12, 2012, 3:39 p.m. UTC | #16
On 01/12/2012 01:08 AM, Rafał Miłecki wrote:
> 2012/1/12 Linus Torvalds<torvalds@linux-foundation.org>:
>> On Wed, Jan 11, 2012 at 9:20 PM, Linus Torvalds
>> <torvalds@linux-foundation.org>  wrote:
>>>
>>> Maybe your BIOS firmware sets things up, and the Apple Macbook Air
>>> doesn't? And the driver used to initialize things sufficiently, and
>>> the changes have broken that?
>>
>> The code in v3.2 also used to support srom rev<  4, the new code
>> doesn't seem to do that.
>
> After hacking bcma to read SPROM from the correct location, what did
> you get in dmesg? There should be info about version of SPROM.

In the bcma version of the SPROM reading, there is no error logging other than 
the incorrect version message. In particular, there is no message when the CRC 
test fails, and the version will not be logged.

Larry


--
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
Rafał Miłecki Jan. 12, 2012, 3:46 p.m. UTC | #17
W dniu 12 stycznia 2012 16:39 użytkownik Larry Finger
<Larry.Finger@lwfinger.net> napisał:
> On 01/12/2012 01:08 AM, Rafał Miłecki wrote:
>>
>> 2012/1/12 Linus Torvalds<torvalds@linux-foundation.org>:
>>>
>>> On Wed, Jan 11, 2012 at 9:20 PM, Linus Torvalds
>>> <torvalds@linux-foundation.org>  wrote:
>>>>
>>>>
>>>> Maybe your BIOS firmware sets things up, and the Apple Macbook Air
>>>> doesn't? And the driver used to initialize things sufficiently, and
>>>> the changes have broken that?
>>>
>>>
>>> The code in v3.2 also used to support srom rev<  4, the new code
>>> doesn't seem to do that.
>>
>>
>> After hacking bcma to read SPROM from the correct location, what did
>> you get in dmesg? There should be info about version of SPROM.
>
>
> In the bcma version of the SPROM reading, there is no error logging other
> than the incorrect version message. In particular, there is no message when
> the CRC test fails, and the version will not be logged.

Oops, you're right. I forgot bcma doesn't have that (on the difference to ssb).
Linus Torvalds Jan. 12, 2012, 3:51 p.m. UTC | #18
2012/1/12 Arend van Spriel <arend@broadcom.com>:
>
> Ok. I know a hint when I see one ;-)

Yeah. I'm real subtle.

          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
Larry Finger Jan. 12, 2012, 4:22 p.m. UTC | #19
On 01/12/2012 07:13 AM, Arend van Spriel wrote:
>
> Hi, Larry
>
> I am surprised that we end up on sprom_read_pci(). That suggests that
> MacBook Air has an external sprom. Can you tell me what the function
> ai_is_sprom_available() returns on your system?

It returns false. My card uses OTP.

Larry

--
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
Larry Finger Jan. 12, 2012, 5:18 p.m. UTC | #20
On 01/12/2012 07:13 AM, Arend van Spriel wrote:
>
> I am surprised that we end up on sprom_read_pci(). That suggests that
> MacBook Air has an external sprom. Can you tell me what the function
> ai_is_sprom_available() returns on your system?

For completeness, sromctrl is 0x12, thus bit 1 (SRC_PRESENT) is not set, and my 
device has an OTP, not an SPROM.

I do not see anything wrong with commit 888153b3db3f, but I realize that my card 
really does not test any of those changes.

Larry
--
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, 5:31 p.m. UTC | #21
On Thu, Jan 12, 2012 at 9:18 AM, Larry Finger <Larry.Finger@lwfinger.net> wrote:
>
> For completeness, sromctrl is 0x12, thus bit 1 (SRC_PRESENT) is not set, and
> my device has an OTP, not an SPROM.

So this is again something that apple is *famous* for.

They try to control their hardware very tightly, and OS X will (for
example) not use non-apple wireless cards as "Airport" cards, and will
do things like dropping features ("Oh, you tried to save money by
buying a generic wireless minipci card instead of the apple branded
one? Well, that's fine, but now I'll make your network flaky and will
refuse to support 802.11n just to make a point.").

Never mind that the hardware is the same - they'll literally look at
the PCI subvendor ID and things like that, and if it doesn't say
"Apple", they will simply not enable all the features, or won't even
connect to it.

They've done this forever. Others do it too (I think both HP and IBM
have done the exact same thing with minipci wireless cards - back when
WiFi used to be a "premium" thing in a laptop, and vendors charged
quite a bit extra for it, gah!). But Apple does it for a *lot* of
things, presumably because they want to make it extra hard for clone
makers (or just tinkerers that would try to run OS X on a regular PC
that just happened to have the exact same hardware as a Macbook).

Seriously. I really like my Macbook Air hardware, but the moment some
non-apple supplier makes anything comparable, I'll drop it like the
crap it is. Exactly because Apple uses software to make it harder to
use. Installing Linux on that thing is "interesting" - Linux works
perfectly fine on it, but with all the special Apple firmware crap,
you have to jump through hoops.

> I do not see anything wrong with commit 888153b3db3f, but I realize that my
> card really does not test any of those changes.

I suspect the big change is the version check and the size of the
sprom image. Apple probably has an older version. I assume that the
subvendor ID etc comes from the srom?

                            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
Larry Finger Jan. 12, 2012, 5:44 p.m. UTC | #22
On 01/12/2012 11:31 AM, Linus Torvalds wrote:
> On Thu, Jan 12, 2012 at 9:18 AM, Larry Finger<Larry.Finger@lwfinger.net>  wrote:
>>
>> For completeness, sromctrl is 0x12, thus bit 1 (SRC_PRESENT) is not set, and
>> my device has an OTP, not an SPROM.
>
> So this is again something that apple is *famous* for.
>
> They try to control their hardware very tightly, and OS X will (for
> example) not use non-apple wireless cards as "Airport" cards, and will
> do things like dropping features ("Oh, you tried to save money by
> buying a generic wireless minipci card instead of the apple branded
> one? Well, that's fine, but now I'll make your network flaky and will
> refuse to support 802.11n just to make a point.").
>
> Never mind that the hardware is the same - they'll literally look at
> the PCI subvendor ID and things like that, and if it doesn't say
> "Apple", they will simply not enable all the features, or won't even
> connect to it.
>
> They've done this forever. Others do it too (I think both HP and IBM
> have done the exact same thing with minipci wireless cards - back when
> WiFi used to be a "premium" thing in a laptop, and vendors charged
> quite a bit extra for it, gah!). But Apple does it for a *lot* of
> things, presumably because they want to make it extra hard for clone
> makers (or just tinkerers that would try to run OS X on a regular PC
> that just happened to have the exact same hardware as a Macbook).
>
> Seriously. I really like my Macbook Air hardware, but the moment some
> non-apple supplier makes anything comparable, I'll drop it like the
> crap it is. Exactly because Apple uses software to make it harder to
> use. Installing Linux on that thing is "interesting" - Linux works
> perfectly fine on it, but with all the special Apple firmware crap,
> you have to jump through hoops.
>
>> I do not see anything wrong with commit 888153b3db3f, but I realize that my
>> card really does not test any of those changes.
>
> I suspect the big change is the version check and the size of the
> sprom image. Apple probably has an older version. I assume that the
> subvendor ID etc comes from the srom?

HP is not at all subtle. Their BIOS checks the hardware in the internal PCIe 
slot. If it is wifi and not on their whitelist, the computer will not boot. For 
my testing, Realtek sent me an extender that plugs into an Express Card slot. 
When this machine dies, I'm not sure what I'll do as I have not found a modern 
laptop with such a slot.

I too would like to blame Apple, but there is one factoid that I just noticed 
and I'm still exploring. When I run bcma/b43, the software says I have a Rev 8 
SPROM at offset 0x830, but bcma/brcmsmac says my card has no SPROM and it uses 
the OTP branch! Why, and what does it mean? Any thoughts from the Broadcom guys?

Larry

--
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, 7 p.m. UTC | #23
On Wed, Jan 11, 2012 at 11:13 PM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> All the BCMA changes make the revert somewhat non-trivial, could
> somebody who knows the code better please try to do it for me? It
> doesn't look complicated, and I can try to do it myself tomorrow if
> nobody else steps up, but I'd *really* prefer the guilty parties
> themselves to do it, ok?

Since I had the hardware to test, I could work on this and try to
figure out exactly what went wrong in that commit.

The problem seems to be simple: the SPROM contents *have* to be read
as aligned 16-bit words. Anything else seems to return 0xff and just
fails the transaction.

I didn't check all the combinations, of course, so who knows what the
exact details are, but it does look like the sprom has very limited
pci decode and simply refuses to touch anything but the one case it
can handle.

I'll send out a patch that seems to get things to a working state for
me. At least I have wireless connectivity again, I don't know if there
are some other problems remaining.

                      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
Arend van Spriel Jan. 12, 2012, 7:46 p.m. UTC | #24
On 01/12/2012 08:00 PM, Linus Torvalds wrote:
> On Wed, Jan 11, 2012 at 11:13 PM, Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
>>
>> All the BCMA changes make the revert somewhat non-trivial, could
>> somebody who knows the code better please try to do it for me? It
>> doesn't look complicated, and I can try to do it myself tomorrow if
>> nobody else steps up, but I'd *really* prefer the guilty parties
>> themselves to do it, ok?
> 
> Since I had the hardware to test, I could work on this and try to
> figure out exactly what went wrong in that commit.

I was trying to get my hands on a card with SPROM to dig in and I still
intend to get it because Apples are no pears.

> The problem seems to be simple: the SPROM contents *have* to be read
> as aligned 16-bit words. Anything else seems to return 0xff and just
> fails the transaction.

I was already wondering what aspect of the patch was causing the issue.
The transaction size requirement seems likely and I was not aware.

> I didn't check all the combinations, of course, so who knows what the
> exact details are, but it does look like the sprom has very limited
> pci decode and simply refuses to touch anything but the one case it
> can handle.
> 
> I'll send out a patch that seems to get things to a working state for
> me. At least I have wireless connectivity again, I don't know if there
> are some other problems remaining.
> 
>                       Linus
> 

Thanks for putting the effort into this in the middle of a merge window.

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
diff mbox

Patch

Index: wireless-testing-new/drivers/bcma/sprom.c
===================================================================
--- wireless-testing-new.orig/drivers/bcma/sprom.c
+++ wireless-testing-new/drivers/bcma/sprom.c
@@ -230,6 +230,7 @@  int bcma_sprom_get(struct bcma_bus *bus)
  	 * TODO: understand this condition and use it */
  	offset = (bus->chipinfo.id == 0x4331) ? BCMA_CC_SPROM :
  		BCMA_CC_SPROM_PCIE6;
+	pr_debug("SPROM offset 0x%x\n", offset);
  	bcma_sprom_read(bus, offset, sprom);

  	if (bus->chipinfo.id == 0x4331)