2010-11-03 21:07:15

by Rafał Miłecki

Subject: [PATCH] ssb: workarounds: be verbose about hacking SPROM revision, don't duplicate code

Signed-off-by: Rafał Miłecki <[email protected]>
---
This changes code flow the way even if workaround is needed, standard extracing
calls are used. Additionaly if user wants to know SPROM version (enabled
debugging) we really should let know also about hacking it!

John: this is 2.6.38 material. Is this OK to submit this via wireless tree?
---
drivers/ssb/pci.c | 45 +++++++++++++++++++++++----------------------
1 files changed, 23 insertions(+), 22 deletions(-)

diff --git a/drivers/ssb/pci.c b/drivers/ssb/pci.c
index 6e88d2b..3226832 100644
--- a/drivers/ssb/pci.c
+++ b/drivers/ssb/pci.c
@@ -573,37 +573,38 @@ static int sprom_extract(struct ssb_bus *bus, struct ssb_sprom *out,
ssb_dprintk(KERN_DEBUG PFX "SPROM revision %d detected.\n", out->revision);
memset(out->et0mac, 0xFF, 6); /* preset et0 and et1 mac */
memset(out->et1mac, 0xFF, 6);
+
if ((bus->chip_id & 0xFF00) == 0x4400) {
/* Workaround: The BCM44XX chip has a stupid revision
* number stored in the SPROM.
* Always extract r1. */
out->revision = 1;
- sprom_extract_r123(out, in);
+ ssb_dprintk(KERN_DEBUG PFX "SPROM treated as revision %d\n", out->revision);
} else if (bus->chip_id == 0x4321) {
/* the BCM4328 has a chipid == 0x4321 and a rev 4 SPROM */
out->revision = 4;
+ ssb_dprintk(KERN_DEBUG PFX "SPROM treated as revision %d\n", out->revision);
+ }
+
+ switch (out->revision) {
+ case 1:
+ case 2:
+ case 3:
+ sprom_extract_r123(out, in);
+ break;
+ case 4:
+ case 5:
sprom_extract_r45(out, in);
- } else {
- switch (out->revision) {
- case 1:
- case 2:
- case 3:
- sprom_extract_r123(out, in);
- break;
- case 4:
- case 5:
- sprom_extract_r45(out, in);
- break;
- case 8:
- sprom_extract_r8(out, in);
- break;
- default:
- ssb_printk(KERN_WARNING PFX "Unsupported SPROM"
- " revision %d detected. Will extract"
- " v1\n", out->revision);
- out->revision = 1;
- sprom_extract_r123(out, in);
- }
+ break;
+ case 8:
+ sprom_extract_r8(out, in);
+ break;
+ default:
+ ssb_printk(KERN_WARNING PFX "Unsupported SPROM"
+ " revision %d detected. Will extract"
+ " v1\n", out->revision);
+ out->revision = 1;
+ sprom_extract_r123(out, in);
}

if (out->boardflags_lo == 0xFFFF)
--
1.6.0.4

2010-11-03 21:59:23

by Larry Finger

Subject: Re: [PATCH] ssb: workarounds: be verbose about hacking SPROM revision, don't duplicate code

On 11/03/2010 04:53 PM, Michael Büsch wrote:
> On Wed, 2010-11-03 at 16:52 -0500, Larry Finger wrote:
>> On 11/03/2010 04:43 PM, Michael Büsch wrote:
>>> On Wed, 2010-11-03 at 22:06 +0100, Rafał Miłecki wrote:
>>>> + default:
>>>> + ssb_printk(KERN_WARNING PFX "Unsupported SPROM"
>>>> + " revision %d detected. Will extract"
>>>> + " v1\n", out->revision);
>>>> + out->revision = 1;
>>>> + sprom_extract_r123(out, in);
>>>
>>> I think we should change this to throw a hard error if the sprom is
>>> unknown. Extracting r123 is unlikely to do any good these days.
>>> This workaround was only useful back in the days where 95% of the
>>> cards out there were r123. But today that's not the case.
>>>
>>>> }
>>>>
>>>> if (out->boardflags_lo == 0xFFFF)
>>
>> In any case, the out->revision = 1 statement should be before the ssb_printk() call.
>
> I don't think so.

You are right. I misread the code.

Larry

2010-11-03 21:53:45

by Michael Büsch

Subject: Re: [PATCH] ssb: workarounds: be verbose about hacking SPROM revision, don't duplicate code

On Wed, 2010-11-03 at 16:52 -0500, Larry Finger wrote:
> On 11/03/2010 04:43 PM, Michael Büsch wrote:
> > On Wed, 2010-11-03 at 22:06 +0100, Rafał Miłecki wrote:
> >> + default:
> >> + ssb_printk(KERN_WARNING PFX "Unsupported SPROM"
> >> + " revision %d detected. Will extract"
> >> + " v1\n", out->revision);
> >> + out->revision = 1;
> >> + sprom_extract_r123(out, in);
> >
> > I think we should change this to throw a hard error if the sprom is
> > unknown. Extracting r123 is unlikely to do any good these days.
> > This workaround was only useful back in the days where 95% of the
> > cards out there were r123. But today that's not the case.
> >
> >> }
> >>
> >> if (out->boardflags_lo == 0xFFFF)
>
> In any case, the out->revision = 1 statement should be before the ssb_printk() call.

I don't think so.

--
Greetings Michael.

2010-11-03 21:43:51

by Michael Büsch

Subject: Re: [PATCH] ssb: workarounds: be verbose about hacking SPROM revision, don't duplicate code

On Wed, 2010-11-03 at 22:06 +0100, Rafał Miłecki wrote:
> + default:
> + ssb_printk(KERN_WARNING PFX "Unsupported SPROM"
> + " revision %d detected. Will extract"
> + " v1\n", out->revision);
> + out->revision = 1;
> + sprom_extract_r123(out, in);

I think we should change this to throw a hard error if the sprom is
unknown. Extracting r123 is unlikely to do any good these days.
This workaround was only useful back in the days where 95% of the
cards out there were r123. But today that's not the case.

> }
>
> if (out->boardflags_lo == 0xFFFF)


--
Greetings Michael.

2010-11-03 21:52:07

by Larry Finger

Subject: Re: [PATCH] ssb: workarounds: be verbose about hacking SPROM revision, don't duplicate code

On 11/03/2010 04:43 PM, Michael Büsch wrote:
> On Wed, 2010-11-03 at 22:06 +0100, Rafał Miłecki wrote:
>> + default:
>> + ssb_printk(KERN_WARNING PFX "Unsupported SPROM"
>> + " revision %d detected. Will extract"
>> + " v1\n", out->revision);
>> + out->revision = 1;
>> + sprom_extract_r123(out, in);
>
> I think we should change this to throw a hard error if the sprom is
> unknown. Extracting r123 is unlikely to do any good these days.
> This workaround was only useful back in the days where 95% of the
> cards out there were r123. But today that's not the case.
>
>> }
>>
>> if (out->boardflags_lo == 0xFFFF)

In any case, the out->revision = 1 statement should be before the ssb_printk() call.

I agree - throw a hard error.

Larry