[RFC] use alternate SPROM offset for 43224

[RFC] use alternate SPROM offset for 43224

[Saul St. John]

I don't know if this is correct in the general sense, but the wireless on my
mid-2010 MacBook Pro doesn't work without it.

Signed-off-by: Saul St. John <saul.stjohn@gmail.com>
---
 drivers/bcma/sprom.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/bcma/sprom.c b/drivers/bcma/sprom.c
index 6f230fb..06c87b5 100644
--- a/drivers/bcma/sprom.c
+++ b/drivers/bcma/sprom.c
@@ -228,8 +228,8 @@ int bcma_sprom_get(struct bcma_bus *bus)
 	/* Most cards have SPROM moved by additional offset 0x30 (48 dwords).
 	 * According to brcm80211 this applies to cards with PCIe rev >= 6
 	 * TODO: understand this condition and use it */
-	offset = (bus->chipinfo.id == 0x4331) ? BCMA_CC_SPROM :
-		BCMA_CC_SPROM_PCIE6;
+	offset = (bus->chipinfo.id == 0x4331 || bus->chipinfo.id == 43224) ? 
+ 			BCMA_CC_SPROM : BCMA_CC_SPROM_PCIE6;
 	bcma_sprom_read(bus, offset, sprom);
 
 	if (bus->chipinfo.id == 0x4331)
-- 
1.7.9

RE: [RFC] use alternate SPROM offset for 43224

[Arend Van Spriel]

> owner@vger.kernel.org] On Behalf Of Saul St. John
> Sent: dinsdag 14 februari 2012 5:01
> 
> I don't know if this is correct in the general sense, but the wireless
> on my
> mid-2010 MacBook Pro doesn't work without it.
> 
> Signed-off-by: Saul St. John <saul.stjohn@gmail.com>

Does this patch solve the same issue as [1] with title:

[PATCH] bcma: don't fail for bad SPROM CRC

Gr. AvS

[1] http://www.spinics.net/lists/netdev/msg187943.html

Re: [RFC] use alternate SPROM offset for 43224

[Rafał Miłecki]

W dniu 14 lutego 2012 11:07 użytkownik Arend Van Spriel
<arend@broadcom.com> napisał:
>> owner@vger.kernel.org] On Behalf Of Saul St. John
>> Sent: dinsdag 14 februari 2012 5:01
>>
>> I don't know if this is correct in the general sense, but the wireless
>> on my
>> mid-2010 MacBook Pro doesn't work without it.
>>
>> Signed-off-by: Saul St. John <saul.stjohn@gmail.com>
>
> Does this patch solve the same issue as [1] with title:
>
> [PATCH] bcma: don't fail for bad SPROM CRC
>
> Gr. AvS
>
> [1] http://www.spinics.net/lists/netdev/msg187943.html

Not really, that patch didn't fix reading SPROM data.

-- 
Rafał

Re: [RFC] use alternate SPROM offset for 43224

[Rafał Miłecki]

W dniu 14 lutego 2012 05:01 użytkownik Saul St. John
<saul.stjohn@gmail.com> napisał:
> I don't know if this is correct in the general sense, but the wireless on my
> mid-2010 MacBook Pro doesn't work without it.
>
> Signed-off-by: Saul St. John <saul.stjohn@gmail.com>
> ---
>  drivers/bcma/sprom.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/bcma/sprom.c b/drivers/bcma/sprom.c
> index 6f230fb..06c87b5 100644
> --- a/drivers/bcma/sprom.c
> +++ b/drivers/bcma/sprom.c
> @@ -228,8 +228,8 @@ int bcma_sprom_get(struct bcma_bus *bus)
>        /* Most cards have SPROM moved by additional offset 0x30 (48 dwords).
>         * According to brcm80211 this applies to cards with PCIe rev >= 6
>         * TODO: understand this condition and use it */
> -       offset = (bus->chipinfo.id == 0x4331) ? BCMA_CC_SPROM :
> -               BCMA_CC_SPROM_PCIE6;
> +       offset = (bus->chipinfo.id == 0x4331 || bus->chipinfo.id == 43224) ?
> +                       BCMA_CC_SPROM : BCMA_CC_SPROM_PCIE6;
>        bcma_sprom_read(bus, offset, sprom);
>
>        if (bus->chipinfo.id == 0x4331)

I'm quite sure it'll break my BCM43224. It's not chip-specific,
probably some status bit specific.

-- 
Rafał

Re: [RFC] use alternate SPROM offset for 43224

[Saul St. John]

On Tue, Feb 14, 2012 at 7:34 AM, Rafał Miłecki <zajec5@gmail.com> wrote:
> W dniu 14 lutego 2012 05:01 użytkownik Saul St. John
> <saul.stjohn@gmail.com> napisał:
>> I don't know if this is correct in the general sense, but the wireless on my
>> mid-2010 MacBook Pro doesn't work without it.
>>
>> Signed-off-by: Saul St. John <saul.stjohn@gmail.com>
>> ---
>>  drivers/bcma/sprom.c |    4 ++--
>>  1 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/bcma/sprom.c b/drivers/bcma/sprom.c
>> index 6f230fb..06c87b5 100644
>> --- a/drivers/bcma/sprom.c
>> +++ b/drivers/bcma/sprom.c
>> @@ -228,8 +228,8 @@ int bcma_sprom_get(struct bcma_bus *bus)
>>        /* Most cards have SPROM moved by additional offset 0x30 (48 dwords).
>>         * According to brcm80211 this applies to cards with PCIe rev >= 6
>>         * TODO: understand this condition and use it */
>> -       offset = (bus->chipinfo.id == 0x4331) ? BCMA_CC_SPROM :
>> -               BCMA_CC_SPROM_PCIE6;
>> +       offset = (bus->chipinfo.id == 0x4331 || bus->chipinfo.id == 43224) ?
>> +                       BCMA_CC_SPROM : BCMA_CC_SPROM_PCIE6;
>>        bcma_sprom_read(bus, offset, sprom);
>>
>>        if (bus->chipinfo.id == 0x4331)
>
> I'm quite sure it'll break my BCM43224. It's not chip-specific,
> probably some status bit specific.
>
> --
> Rafał

My BCM43324 was broken by bmca up until "[PATCH] bcma: don't fail for
bad SPROM CRC." Even with that patch, I still get "bmca: Failed to get
SPROM: -71" in the dmesg log. Is that error harmless?

The CRC check appears to pass without issue when using the 0x800
offset on my device.

-saul

Re: [RFC] use alternate SPROM offset for 43224

[Rafał Miłecki]

W dniu 14 lutego 2012 17:22 użytkownik Saul St. John
<saul.stjohn@gmail.com> napisał:
> On Tue, Feb 14, 2012 at 7:34 AM, Rafał Miłecki <zajec5@gmail.com> wrote:
>> W dniu 14 lutego 2012 05:01 użytkownik Saul St. John
>> <saul.stjohn@gmail.com> napisał:
>>> I don't know if this is correct in the general sense, but the wireless on my
>>> mid-2010 MacBook Pro doesn't work without it.
>>>
>>> Signed-off-by: Saul St. John <saul.stjohn@gmail.com>
>>> ---
>>>  drivers/bcma/sprom.c |    4 ++--
>>>  1 files changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/bcma/sprom.c b/drivers/bcma/sprom.c
>>> index 6f230fb..06c87b5 100644
>>> --- a/drivers/bcma/sprom.c
>>> +++ b/drivers/bcma/sprom.c
>>> @@ -228,8 +228,8 @@ int bcma_sprom_get(struct bcma_bus *bus)
>>>        /* Most cards have SPROM moved by additional offset 0x30 (48 dwords).
>>>         * According to brcm80211 this applies to cards with PCIe rev >= 6
>>>         * TODO: understand this condition and use it */
>>> -       offset = (bus->chipinfo.id == 0x4331) ? BCMA_CC_SPROM :
>>> -               BCMA_CC_SPROM_PCIE6;
>>> +       offset = (bus->chipinfo.id == 0x4331 || bus->chipinfo.id == 43224) ?
>>> +                       BCMA_CC_SPROM : BCMA_CC_SPROM_PCIE6;
>>>        bcma_sprom_read(bus, offset, sprom);
>>>
>>>        if (bus->chipinfo.id == 0x4331)
>>
>> I'm quite sure it'll break my BCM43224. It's not chip-specific,
>> probably some status bit specific.
>>
>> --
>> Rafał
>
> My BCM43324 was broken by bmca up until "[PATCH] bcma: don't fail for
> bad SPROM CRC." Even with that patch, I still get "bmca: Failed to get
> SPROM: -71" in the dmesg log. Is that error harmless?

It's harmless for brcmsmac, which doesn't use SPROM struct of bcma bus
driver. This bug should be fixed and brcmsmac should be improved in
many contexts: using SPROM, standard bcma module functions, dropping
other cores initializing. For now you can live with this.


> The CRC check appears to pass without issue when using the 0x800
> offset on my device.

The quick fix would be probably to implement in bcma two tries of
reading SPROM. One for 0x800 second for 0x830 (base address). The real
fix is to grab the real condition from specs/brcmsmac and implement it
in bcma.

-- 
Rafał

Re: [RFC] use alternate SPROM offset for 43224

[Hauke Mehrtens]

On 02/14/2012 06:29 PM, Rafał Miłecki wrote:
> W dniu 14 lutego 2012 17:22 użytkownik Saul St. John
> <saul.stjohn@gmail.com> napisał:
>> On Tue, Feb 14, 2012 at 7:34 AM, Rafał Miłecki <zajec5@gmail.com> wrote:
>>> W dniu 14 lutego 2012 05:01 użytkownik Saul St. John
>>> <saul.stjohn@gmail.com> napisał:
>>>> I don't know if this is correct in the general sense, but the wireless on my
>>>> mid-2010 MacBook Pro doesn't work without it.
>>>>
>>>> Signed-off-by: Saul St. John <saul.stjohn@gmail.com>
>>>> ---
>>>>  drivers/bcma/sprom.c |    4 ++--
>>>>  1 files changed, 2 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/bcma/sprom.c b/drivers/bcma/sprom.c
>>>> index 6f230fb..06c87b5 100644
>>>> --- a/drivers/bcma/sprom.c
>>>> +++ b/drivers/bcma/sprom.c
>>>> @@ -228,8 +228,8 @@ int bcma_sprom_get(struct bcma_bus *bus)
>>>>        /* Most cards have SPROM moved by additional offset 0x30 (48 dwords).
>>>>         * According to brcm80211 this applies to cards with PCIe rev >= 6
>>>>         * TODO: understand this condition and use it */
>>>> -       offset = (bus->chipinfo.id == 0x4331) ? BCMA_CC_SPROM :
>>>> -               BCMA_CC_SPROM_PCIE6;
>>>> +       offset = (bus->chipinfo.id == 0x4331 || bus->chipinfo.id == 43224) ?
>>>> +                       BCMA_CC_SPROM : BCMA_CC_SPROM_PCIE6;
>>>>        bcma_sprom_read(bus, offset, sprom);
>>>>
>>>>        if (bus->chipinfo.id == 0x4331)
>>>
>>> I'm quite sure it'll break my BCM43224. It's not chip-specific,
>>> probably some status bit specific.
>>>
>>> --
>>> Rafał
>>
>> My BCM43324 was broken by bmca up until "[PATCH] bcma: don't fail for
>> bad SPROM CRC." Even with that patch, I still get "bmca: Failed to get
>> SPROM: -71" in the dmesg log. Is that error harmless?
> 
> It's harmless for brcmsmac, which doesn't use SPROM struct of bcma bus
> driver. This bug should be fixed and brcmsmac should be improved in
> many contexts: using SPROM, standard bcma module functions, dropping
> other cores initializing. For now you can live with this.
> 
> 
>> The CRC check appears to pass without issue when using the 0x800
>> offset on my device.
> 
> The quick fix would be probably to implement in bcma two tries of
> reading SPROM. One for 0x800 second for 0x830 (base address). The real
> fix is to grab the real condition from specs/brcmsmac and implement it
> in bcma.
> 
Hi,

I am unable to find the 0x830 address in brcmsmac or any other source
code from Broadcom.

This is the place where brcmsmac gets the offset:

/* determine core to read */
if (ai_get_ccrev(sih) < 32) {
	core = ai_findcore(sih, BCMA_CORE_80211, 0);
	sprom_offset = PCI_BAR0_SPROM_OFFSET; // (4 * 1024)
} else {
	core = ai_findcore(sih, BCMA_CORE_CHIPCOMMON, 0);
	sprom_offset = CHIPCREGOFFS(sromotp); // 0x800
}

What chipcommon rev does the device have?
This device is probably using the otp instead of a sprom and that is not
implemented in bcma.

The thing with the pcie core rev >= 6, is about sprom rev >= 4 being
bigger than the old ones which is fully implemented in bcma.

Hauke

Re: [RFC] use alternate SPROM offset for 43224

[Saul St. John]

On Tue, Feb 14, 2012 at 06:29:59PM +0100, Rafał Miłecki wrote:
> W dniu 14 lutego 2012 17:22 użytkownik Saul St. John
> <saul.stjohn@gmail.com> napisał:
> > On Tue, Feb 14, 2012 at 7:34 AM, Rafał Miłecki <zajec5@gmail.com> wrote:
> >> W dniu 14 lutego 2012 05:01 użytkownik Saul St. John
> >> <saul.stjohn@gmail.com> napisał:
> >>> I don't know if this is correct in the general sense, but the wireless on my
> >>> mid-2010 MacBook Pro doesn't work without it.
> >>>
> >>> Signed-off-by: Saul St. John <saul.stjohn@gmail.com>
> >>> ---
> >>>  drivers/bcma/sprom.c |    4 ++--
> >>>  1 files changed, 2 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/drivers/bcma/sprom.c b/drivers/bcma/sprom.c
> >>> index 6f230fb..06c87b5 100644
> >>> --- a/drivers/bcma/sprom.c
> >>> +++ b/drivers/bcma/sprom.c
> >>> @@ -228,8 +228,8 @@ int bcma_sprom_get(struct bcma_bus *bus)
> >>>        /* Most cards have SPROM moved by additional offset 0x30 (48 dwords).
> >>>         * According to brcm80211 this applies to cards with PCIe rev >= 6
> >>>         * TODO: understand this condition and use it */
> >>> -       offset = (bus->chipinfo.id == 0x4331) ? BCMA_CC_SPROM :
> >>> -               BCMA_CC_SPROM_PCIE6;
> >>> +       offset = (bus->chipinfo.id == 0x4331 || bus->chipinfo.id == 43224) ?
> >>> +                       BCMA_CC_SPROM : BCMA_CC_SPROM_PCIE6;
> >>>        bcma_sprom_read(bus, offset, sprom);
> >>>
> >>>        if (bus->chipinfo.id == 0x4331)
> >>
> >> I'm quite sure it'll break my BCM43224. It's not chip-specific,
> >> probably some status bit specific.
> >>
> >> --
> >> Rafał
> >
> > My BCM43324 was broken by bmca up until "[PATCH] bcma: don't fail for
> > bad SPROM CRC." Even with that patch, I still get "bmca: Failed to get
> > SPROM: -71" in the dmesg log. Is that error harmless?
> 
> It's harmless for brcmsmac, which doesn't use SPROM struct of bcma bus
> driver. This bug should be fixed and brcmsmac should be improved in
> many contexts: using SPROM, standard bcma module functions, dropping
> other cores initializing. For now you can live with this.
> 
My worry is that, were the brcmsmac driver to be improved in the ways you
suggest, the BCM43224 in my MBP would start failing again.
> 
> > The CRC check appears to pass without issue when using the 0x800
> > offset on my device.
> 
> The quick fix would be probably to implement in bcma two tries of
> reading SPROM. One for 0x800 second for 0x830 (base address). The real
> fix is to grab the real condition from specs/brcmsmac and implement it
> in bcma.
> 
Real fix: any leads on where the real condition can be found? I looked through
the brcmsmac driver, but it wasn't obvious to me.

Quick fix: something like this?

Signed-off-by: Saul St. John <saul.stjohn@gmail.com>
---
 drivers/bcma/sprom.c |   24 ++++++++++++++----------
 1 files changed, 14 insertions(+), 10 deletions(-)

diff --git a/drivers/bcma/sprom.c b/drivers/bcma/sprom.c
index 6f230fb..e99807e 100644
--- a/drivers/bcma/sprom.c
+++ b/drivers/bcma/sprom.c
@@ -207,7 +207,7 @@ static void bcma_sprom_extract_r8(struct bcma_bus *bus, const u16 *sprom)
 
 int bcma_sprom_get(struct bcma_bus *bus)
 {
-	u16 offset;
+	u16 offset = 0;
 	u16 *sprom;
 	int err = 0;
 
@@ -222,20 +222,24 @@ int bcma_sprom_get(struct bcma_bus *bus)
 	if (!sprom)
 		return -ENOMEM;
 
-	if (bus->chipinfo.id == 0x4331)
-		bcma_chipco_bcm4331_ext_pa_lines_ctl(&bus->drv_cc, false);
-
 	/* Most cards have SPROM moved by additional offset 0x30 (48 dwords).
 	 * According to brcm80211 this applies to cards with PCIe rev >= 6
 	 * TODO: understand this condition and use it */
-	offset = (bus->chipinfo.id == 0x4331) ? BCMA_CC_SPROM :
-		BCMA_CC_SPROM_PCIE6;
-	bcma_sprom_read(bus, offset, sprom);
+	do {
+		if (bus->chipinfo.id == 0x4331)
+			bcma_chipco_bcm4331_ext_pa_lines_ctl(&bus->drv_cc, 
+							     false);
+
+		offset = (offset == 0) ? BCMA_CC_SPROM : BCMA_CC_SPROM_PCIE6;
+		bcma_sprom_read(bus, offset, sprom);
+
+		if (bus->chipinfo.id == 0x4331)
+			bcma_chipco_bcm4331_ext_pa_lines_ctl(&bus->drv_cc, 
+							     true);
 
-	if (bus->chipinfo.id == 0x4331)
-		bcma_chipco_bcm4331_ext_pa_lines_ctl(&bus->drv_cc, true);
+		err = bcma_sprom_valid(sprom);
+	} while (err && (offset != BCMA_CC_SPROM_PCIE6));
 
-	err = bcma_sprom_valid(sprom);
 	if (err)
 		goto out;
 
-- 
1.7.9

Re: [RFC] use alternate SPROM offset for 43224

[Larry Finger]

On 02/14/2012 12:52 PM, Saul St. John wrote:
> On Tue, Feb 14, 2012 at 06:29:59PM +0100, Rafał Miłecki wrote:
>> W dniu 14 lutego 2012 17:22 użytkownik Saul St. John
>> <saul.stjohn@gmail.com>  napisał:
>>> On Tue, Feb 14, 2012 at 7:34 AM, Rafał Miłecki<zajec5@gmail.com>  wrote:
>>>> W dniu 14 lutego 2012 05:01 użytkownik Saul St. John
>>>> <saul.stjohn@gmail.com>  napisał:
>>>>> I don't know if this is correct in the general sense, but the wireless on my
>>>>> mid-2010 MacBook Pro doesn't work without it.
>>>>>
>>>>> Signed-off-by: Saul St. John<saul.stjohn@gmail.com>
>>>>> ---
>>>>>   drivers/bcma/sprom.c |    4 ++--
>>>>>   1 files changed, 2 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/drivers/bcma/sprom.c b/drivers/bcma/sprom.c
>>>>> index 6f230fb..06c87b5 100644
>>>>> --- a/drivers/bcma/sprom.c
>>>>> +++ b/drivers/bcma/sprom.c
>>>>> @@ -228,8 +228,8 @@ int bcma_sprom_get(struct bcma_bus *bus)
>>>>>         /* Most cards have SPROM moved by additional offset 0x30 (48 dwords).
>>>>>          * According to brcm80211 this applies to cards with PCIe rev>= 6
>>>>>          * TODO: understand this condition and use it */
>>>>> -       offset = (bus->chipinfo.id == 0x4331) ? BCMA_CC_SPROM :
>>>>> -               BCMA_CC_SPROM_PCIE6;
>>>>> +       offset = (bus->chipinfo.id == 0x4331 || bus->chipinfo.id == 43224) ?
>>>>> +                       BCMA_CC_SPROM : BCMA_CC_SPROM_PCIE6;
>>>>>         bcma_sprom_read(bus, offset, sprom);
>>>>>
>>>>>         if (bus->chipinfo.id == 0x4331)
>>>>
>>>> I'm quite sure it'll break my BCM43224. It's not chip-specific,
>>>> probably some status bit specific.
>>>>
>>>> --
>>>> Rafał
>>>
>>> My BCM43324 was broken by bmca up until "[PATCH] bcma: don't fail for
>>> bad SPROM CRC." Even with that patch, I still get "bmca: Failed to get
>>> SPROM: -71" in the dmesg log. Is that error harmless?
>>
>> It's harmless for brcmsmac, which doesn't use SPROM struct of bcma bus
>> driver. This bug should be fixed and brcmsmac should be improved in
>> many contexts: using SPROM, standard bcma module functions, dropping
>> other cores initializing. For now you can live with this.
>>
> My worry is that, were the brcmsmac driver to be improved in the ways you
> suggest, the BCM43224 in my MBP would start failing again.
>>
>>> The CRC check appears to pass without issue when using the 0x800
>>> offset on my device.
>>
>> The quick fix would be probably to implement in bcma two tries of
>> reading SPROM. One for 0x800 second for 0x830 (base address). The real
>> fix is to grab the real condition from specs/brcmsmac and implement it
>> in bcma.
>>
> Real fix: any leads on where the real condition can be found? I looked through
> the brcmsmac driver, but it wasn't obvious to me.
>
> Quick fix: something like this?
>
> Signed-off-by: Saul St. John<saul.stjohn@gmail.com>
> ---
>   drivers/bcma/sprom.c |   24 ++++++++++++++----------
>   1 files changed, 14 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/bcma/sprom.c b/drivers/bcma/sprom.c
> index 6f230fb..e99807e 100644
> --- a/drivers/bcma/sprom.c
> +++ b/drivers/bcma/sprom.c
> @@ -207,7 +207,7 @@ static void bcma_sprom_extract_r8(struct bcma_bus *bus, const u16 *sprom)
>
>   int bcma_sprom_get(struct bcma_bus *bus)
>   {
> -	u16 offset;
> +	u16 offset = 0;
>   	u16 *sprom;
>   	int err = 0;
>
> @@ -222,20 +222,24 @@ int bcma_sprom_get(struct bcma_bus *bus)
>   	if (!sprom)
>   		return -ENOMEM;
>
> -	if (bus->chipinfo.id == 0x4331)
> -		bcma_chipco_bcm4331_ext_pa_lines_ctl(&bus->drv_cc, false);
> -
>   	/* Most cards have SPROM moved by additional offset 0x30 (48 dwords).
>   	 * According to brcm80211 this applies to cards with PCIe rev>= 6
>   	 * TODO: understand this condition and use it */
> -	offset = (bus->chipinfo.id == 0x4331) ? BCMA_CC_SPROM :
> -		BCMA_CC_SPROM_PCIE6;
> -	bcma_sprom_read(bus, offset, sprom);
> +	do {
> +		if (bus->chipinfo.id == 0x4331)
> +			bcma_chipco_bcm4331_ext_pa_lines_ctl(&bus->drv_cc,
> +							     false);
> +
> +		offset = (offset == 0) ? BCMA_CC_SPROM : BCMA_CC_SPROM_PCIE6;
> +		bcma_sprom_read(bus, offset, sprom);
> +
> +		if (bus->chipinfo.id == 0x4331)
> +			bcma_chipco_bcm4331_ext_pa_lines_ctl(&bus->drv_cc,
> +							     true);
>
> -	if (bus->chipinfo.id == 0x4331)
> -		bcma_chipco_bcm4331_ext_pa_lines_ctl(&bus->drv_cc, true);
> +		err = bcma_sprom_valid(sprom);
> +	} while (err&&  (offset != BCMA_CC_SPROM_PCIE6));
>
> -	err = bcma_sprom_valid(sprom);
>   	if (err)
>   		goto out;
>

I think it is more complicated than the above. On my 43224, I get the message 
"No SPROM available", which arises because bcma_sprom_get() is returning 
-ENOENT. The reason is that the value tested in "if (!(sromctrl & 
BCMA_CC_SROM_CONTROL_PRESENT))" is zero. The contents of sromctl are 0x12, and 
the mask is 1.

I am still investigating. When I comment out the test of sromctl, then the 
driver seems to find the SPROM at offset 0x830, but neither b43 nor brcmsmac works.

Larry



//                      return -ENOENT;

Re: [RFC] use alternate SPROM offset for 43224

[Hauke Mehrtens]

On 02/14/2012 09:10 PM, Larry Finger wrote:
> On 02/14/2012 12:52 PM, Saul St. John wrote:
>> On Tue, Feb 14, 2012 at 06:29:59PM +0100, Rafał Miłecki wrote:
>>> W dniu 14 lutego 2012 17:22 użytkownik Saul St. John
>>> <saul.stjohn@gmail.com>  napisał:
>>>> On Tue, Feb 14, 2012 at 7:34 AM, Rafał Miłecki<zajec5@gmail.com> 
>>>> wrote:
>>>>> W dniu 14 lutego 2012 05:01 użytkownik Saul St. John
>>>>> <saul.stjohn@gmail.com>  napisał:
>>>>>> I don't know if this is correct in the general sense, but the
>>>>>> wireless on my
>>>>>> mid-2010 MacBook Pro doesn't work without it.
>>>>>>
>>>>>> Signed-off-by: Saul St. John<saul.stjohn@gmail.com>
>>>>>> ---
>>>>>>   drivers/bcma/sprom.c |    4 ++--
>>>>>>   1 files changed, 2 insertions(+), 2 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/bcma/sprom.c b/drivers/bcma/sprom.c
>>>>>> index 6f230fb..06c87b5 100644
>>>>>> --- a/drivers/bcma/sprom.c
>>>>>> +++ b/drivers/bcma/sprom.c
>>>>>> @@ -228,8 +228,8 @@ int bcma_sprom_get(struct bcma_bus *bus)
>>>>>>         /* Most cards have SPROM moved by additional offset 0x30
>>>>>> (48 dwords).
>>>>>>          * According to brcm80211 this applies to cards with PCIe
>>>>>> rev>= 6
>>>>>>          * TODO: understand this condition and use it */
>>>>>> -       offset = (bus->chipinfo.id == 0x4331) ? BCMA_CC_SPROM :
>>>>>> -               BCMA_CC_SPROM_PCIE6;
>>>>>> +       offset = (bus->chipinfo.id == 0x4331 || bus->chipinfo.id
>>>>>> == 43224) ?
>>>>>> +                       BCMA_CC_SPROM : BCMA_CC_SPROM_PCIE6;
>>>>>>         bcma_sprom_read(bus, offset, sprom);
>>>>>>
>>>>>>         if (bus->chipinfo.id == 0x4331)
>>>>>
>>>>> I'm quite sure it'll break my BCM43224. It's not chip-specific,
>>>>> probably some status bit specific.
>>>>>
>>>>> -- 
>>>>> Rafał
>>>>
>>>> My BCM43324 was broken by bmca up until "[PATCH] bcma: don't fail for
>>>> bad SPROM CRC." Even with that patch, I still get "bmca: Failed to get
>>>> SPROM: -71" in the dmesg log. Is that error harmless?
>>>
>>> It's harmless for brcmsmac, which doesn't use SPROM struct of bcma bus
>>> driver. This bug should be fixed and brcmsmac should be improved in
>>> many contexts: using SPROM, standard bcma module functions, dropping
>>> other cores initializing. For now you can live with this.
>>>
>> My worry is that, were the brcmsmac driver to be improved in the ways you
>> suggest, the BCM43224 in my MBP would start failing again.
>>>
>>>> The CRC check appears to pass without issue when using the 0x800
>>>> offset on my device.
>>>
>>> The quick fix would be probably to implement in bcma two tries of
>>> reading SPROM. One for 0x800 second for 0x830 (base address). The real
>>> fix is to grab the real condition from specs/brcmsmac and implement it
>>> in bcma.
>>>
>> Real fix: any leads on where the real condition can be found? I looked
>> through
>> the brcmsmac driver, but it wasn't obvious to me.
>>
>> Quick fix: something like this?
>>
>> Signed-off-by: Saul St. John<saul.stjohn@gmail.com>
>> ---
>>   drivers/bcma/sprom.c |   24 ++++++++++++++----------
>>   1 files changed, 14 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/bcma/sprom.c b/drivers/bcma/sprom.c
>> index 6f230fb..e99807e 100644
>> --- a/drivers/bcma/sprom.c
>> +++ b/drivers/bcma/sprom.c
>> @@ -207,7 +207,7 @@ static void bcma_sprom_extract_r8(struct bcma_bus
>> *bus, const u16 *sprom)
>>
>>   int bcma_sprom_get(struct bcma_bus *bus)
>>   {
>> -    u16 offset;
>> +    u16 offset = 0;
>>       u16 *sprom;
>>       int err = 0;
>>
>> @@ -222,20 +222,24 @@ int bcma_sprom_get(struct bcma_bus *bus)
>>       if (!sprom)
>>           return -ENOMEM;
>>
>> -    if (bus->chipinfo.id == 0x4331)
>> -        bcma_chipco_bcm4331_ext_pa_lines_ctl(&bus->drv_cc, false);
>> -
>>       /* Most cards have SPROM moved by additional offset 0x30 (48
>> dwords).
>>        * According to brcm80211 this applies to cards with PCIe rev>= 6
>>        * TODO: understand this condition and use it */
>> -    offset = (bus->chipinfo.id == 0x4331) ? BCMA_CC_SPROM :
>> -        BCMA_CC_SPROM_PCIE6;
>> -    bcma_sprom_read(bus, offset, sprom);
>> +    do {
>> +        if (bus->chipinfo.id == 0x4331)
>> +            bcma_chipco_bcm4331_ext_pa_lines_ctl(&bus->drv_cc,
>> +                                 false);
>> +
>> +        offset = (offset == 0) ? BCMA_CC_SPROM : BCMA_CC_SPROM_PCIE6;
>> +        bcma_sprom_read(bus, offset, sprom);
>> +
>> +        if (bus->chipinfo.id == 0x4331)
>> +            bcma_chipco_bcm4331_ext_pa_lines_ctl(&bus->drv_cc,
>> +                                 true);
>>
>> -    if (bus->chipinfo.id == 0x4331)
>> -        bcma_chipco_bcm4331_ext_pa_lines_ctl(&bus->drv_cc, true);
>> +        err = bcma_sprom_valid(sprom);
>> +    } while (err&&  (offset != BCMA_CC_SPROM_PCIE6));
>>
>> -    err = bcma_sprom_valid(sprom);
>>       if (err)
>>           goto out;
>>
> 
> I think it is more complicated than the above. On my 43224, I get the
> message "No SPROM available", which arises because bcma_sprom_get() is
> returning -ENOENT. The reason is that the value tested in "if
> (!(sromctrl & BCMA_CC_SROM_CONTROL_PRESENT))" is zero. The contents of
> sromctl are 0x12, and the mask is 1.
This check is the main part of ai_is_sprom_available() in brcmsmac. If
this check fails, like in your case, brcmsmac tries otp_read_pci() to
read out the sprom, which is not implemented in bcma.

> I am still investigating. When I comment out the test of sromctl, then
> the driver seems to find the SPROM at offset 0x830, but neither b43 nor
> brcmsmac works.
Where did you get the 0x830 from and for which devices do you get a
correct sprom at that position?

Hauke

Re: [RFC] use alternate SPROM offset for 43224

[Larry Finger]

On 02/14/2012 02:20 PM, Hauke Mehrtens wrote:
>> >  I think it is more complicated than the above. On my 43224, I get the
>> >  message "No SPROM available", which arises because bcma_sprom_get() is
>> >  returning -ENOENT. The reason is that the value tested in "if
>> >  (!(sromctrl&  BCMA_CC_SROM_CONTROL_PRESENT))" is zero. The contents of
>> >  sromctl are 0x12, and the mask is 1.
> This check is the main part of ai_is_sprom_available() in brcmsmac. If
> this check fails, like in your case, brcmsmac tries otp_read_pci() to
> read out the sprom, which is not implemented in bcma.

I will try implementing a similar routine in bcma.

>> >  I am still investigating. When I comment out the test of sromctl, then
>> >  the driver seems to find the SPROM at offset 0x830, but neither b43 nor
>> >  brcmsmac works.
> Where did you get the 0x830 from and for which devices do you get a
> correct sprom at that position?

I don't know that I found a correct SPROM at that location. As I said, the 
device does not work. All I know is that the following fragment did not report 
an error. When the offset is 0x800, err is -71.

         /* Try to get SPROM */
         err = bcma_sprom_get(bus);
         if (err == -ENOENT) {
                 pr_err("No SPROM available\n");
         } else if (err)
                 pr_err("Failed to get SPROM: %d\n", err);

The chip is reported by lspci as "Network controller [0280]: Broadcom 
Corporation BCM43224 802.11a/b/g/n [14e4:4353] (rev 01)"

>From bcma, the core info is

bcma: Found chip with id 0xA8D8, rev 0x01 and package 0x08
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: bus->drv_cc.capabilities 0x58500000
bcma: sromctrl 0x12
bcma: SPROM offset 0x830

That offset comes from

         offset = bus->chipinfo.id == (0x4331 || bus->chipinfo.id == 43224) ?
                         BCMA_CC_SPROM : BCMA_CC_SPROM_PCIE6;

which includes the trial patch from the initial posting in this thread.

RE: [RFC] use alternate SPROM offset for 43224

[Arend Van Spriel]

PiBTZW50OiBkaW5zZGFnIDE0IGZlYnJ1YXJpIDIwMTIgMjM6MDQNCj4gDQo+IE9uIDAyLzE0LzIw
MTIgMDI6MjAgUE0sIEhhdWtlIE1laHJ0ZW5zIHdyb3RlOg0KPiA+PiA+ICBJIHRoaW5rIGl0IGlz
IG1vcmUgY29tcGxpY2F0ZWQgdGhhbiB0aGUgYWJvdmUuIE9uIG15IDQzMjI0LCBJIGdldA0KPiB0
aGUNCj4gPj4gPiAgbWVzc2FnZSAiTm8gU1BST00gYXZhaWxhYmxlIiwgd2hpY2ggYXJpc2VzIGJl
Y2F1c2UNCj4gYmNtYV9zcHJvbV9nZXQoKSBpcw0KPiA+PiA+ICByZXR1cm5pbmcgLUVOT0VOVC4g
VGhlIHJlYXNvbiBpcyB0aGF0IHRoZSB2YWx1ZSB0ZXN0ZWQgaW4gImlmDQo+ID4+ID4gICghKHNy
b21jdHJsJiAgQkNNQV9DQ19TUk9NX0NPTlRST0xfUFJFU0VOVCkpIiBpcyB6ZXJvLiBUaGUNCj4g
Y29udGVudHMgb2YNCj4gPj4gPiAgc3JvbWN0bCBhcmUgMHgxMiwgYW5kIHRoZSBtYXNrIGlzIDEu
DQo+ID4gVGhpcyBjaGVjayBpcyB0aGUgbWFpbiBwYXJ0IG9mIGFpX2lzX3Nwcm9tX2F2YWlsYWJs
ZSgpIGluIGJyY21zbWFjLg0KPiBJZg0KPiA+IHRoaXMgY2hlY2sgZmFpbHMsIGxpa2UgaW4geW91
ciBjYXNlLCBicmNtc21hYyB0cmllcyBvdHBfcmVhZF9wY2koKSB0bw0KPiA+IHJlYWQgb3V0IHRo
ZSBzcHJvbSwgd2hpY2ggaXMgbm90IGltcGxlbWVudGVkIGluIGJjbWEuDQo+IA0KPiBJIHdpbGwg
dHJ5IGltcGxlbWVudGluZyBhIHNpbWlsYXIgcm91dGluZSBpbiBiY21hLg0KPiANCg0KSSBzdGFy
dGVkIGFkZGluZyBwcm9wZXIgT1RQIHN1cHBvcnQgaW4gQkNNQSwgYnV0IEkgYW0gY3VycmVudGx5
IHdvcmtpbmcgb24gYXN5bmMgZmlybXdhcmUgbG9hZGluZyBmZWF0dXJlLiBEaWZmZXJlbnQgY2hp
cHNldHMgY2FuIGVpdGhlciBoYXZlIGFuIGV4dGVybmFsIFNQUk9NIG9yIG9uLWNoaXAgT1RQLiBT
b21lIGhhdmUgYm90aCwgYnV0IG9ubHkgb25lIGhvbGRzIHRoZSBkYXRhLg0KDQpHci4gQXZTDQo=

Re: [RFC] use alternate SPROM offset for 43224

[Hauke Mehrtens]

On 02/14/2012 11:39 PM, Arend Van Spriel wrote:
>> Sent: dinsdag 14 februari 2012 23:04
>>
>> On 02/14/2012 02:20 PM, Hauke Mehrtens wrote:
>>>>>  I think it is more complicated than the above. On my 43224, I get
>> the
>>>>>  message "No SPROM available", which arises because
>> bcma_sprom_get() is
>>>>>  returning -ENOENT. The reason is that the value tested in "if
>>>>>  (!(sromctrl&  BCMA_CC_SROM_CONTROL_PRESENT))" is zero. The
>> contents of
>>>>>  sromctl are 0x12, and the mask is 1.
>>> This check is the main part of ai_is_sprom_available() in brcmsmac.
>> If
>>> this check fails, like in your case, brcmsmac tries otp_read_pci() to
>>> read out the sprom, which is not implemented in bcma.
>>
>> I will try implementing a similar routine in bcma.
>>
> 
> I started adding proper OTP support in BCMA, but I am currently working on async firmware loading feature. Different chipsets can either have an external SPROM or on-chip OTP. Some have both, but only one holds the data.
> 
> Gr. AvS
Nice to hear you are working on OTP support in bcma. In that process,
are you also working on making brcmsmac using the sprom provided by bcma?

Hauke

Re: [RFC] use alternate SPROM offset for 43224

[Arend van Spriel]

On 02/19/2012 04:37 PM, Hauke Mehrtens wrote:
> On 02/14/2012 11:39 PM, Arend Van Spriel wrote:
>>> Sent: dinsdag 14 februari 2012 23:04
>>>
>>> On 02/14/2012 02:20 PM, Hauke Mehrtens wrote:
>>>>>>   I think it is more complicated than the above. On my 43224, I get
>>> the
>>>>>>   message "No SPROM available", which arises because
>>> bcma_sprom_get() is
>>>>>>   returning -ENOENT. The reason is that the value tested in "if
>>>>>>   (!(sromctrl&   BCMA_CC_SROM_CONTROL_PRESENT))" is zero. The
>>> contents of
>>>>>>   sromctl are 0x12, and the mask is 1.
>>>> This check is the main part of ai_is_sprom_available() in brcmsmac.
>>> If
>>>> this check fails, like in your case, brcmsmac tries otp_read_pci() to
>>>> read out the sprom, which is not implemented in bcma.
>>>
>>> I will try implementing a similar routine in bcma.
>>>
>>
>> I started adding proper OTP support in BCMA, but I am currently working on async firmware loading feature. Different chipsets can either have an external SPROM or on-chip OTP. Some have both, but only one holds the data.
>>
>> Gr. AvS
> Nice to hear you are working on OTP support in bcma. In that process,
> are you also working on making brcmsmac using the sprom provided by bcma?
>
> Hauke
>
>

Yep, no sense having it implemented in two places. I think I will do it 
in one patch series.

Gr. AvS