Signed-off-by: Rafał Miłecki <[email protected]>
---
John: it's for 2.6.38.
---
drivers/ssb/pci.c | 7 +++----
1 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/ssb/pci.c b/drivers/ssb/pci.c
index 3226832..b5343ac 100644
--- a/drivers/ssb/pci.c
+++ b/drivers/ssb/pci.c
@@ -619,7 +619,7 @@ static int ssb_pci_sprom_get(struct ssb_bus *bus,
struct ssb_sprom *sprom)
{
const struct ssb_sprom *fallback;
- int err = -ENOMEM;
+ int err;
u16 *buf;
if (!ssb_is_sprom_available(bus)) {
@@ -646,7 +646,7 @@ static int ssb_pci_sprom_get(struct ssb_bus *bus,
buf = kcalloc(SSB_SPROMSIZE_WORDS_R123, sizeof(u16), GFP_KERNEL);
if (!buf)
- goto out;
+ return -ENOMEM;
bus->sprom_size = SSB_SPROMSIZE_WORDS_R123;
sprom_do_read(bus, buf);
err = sprom_check_crc(buf, bus->sprom_size);
@@ -656,7 +656,7 @@ static int ssb_pci_sprom_get(struct ssb_bus *bus,
buf = kcalloc(SSB_SPROMSIZE_WORDS_R4, sizeof(u16),
GFP_KERNEL);
if (!buf)
- goto out;
+ return -ENOMEM;
bus->sprom_size = SSB_SPROMSIZE_WORDS_R4;
sprom_do_read(bus, buf);
err = sprom_check_crc(buf, bus->sprom_size);
@@ -678,7 +678,6 @@ static int ssb_pci_sprom_get(struct ssb_bus *bus,
out_free:
kfree(buf);
-out:
return err;
}
--
1.6.0.4
Signed-off-by: Rafał Miłecki <[email protected]>
---
As noticed my Michael we have this left from old times of limited amount of
revisions. It's not reliable to treat unknown as rev1.
Michael: would you like to add Reported-by?
John: it's for 2.6.38.
---
drivers/ssb/pci.c | 8 +++-----
1 files changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/ssb/pci.c b/drivers/ssb/pci.c
index b5343ac..41a7337 100644
--- a/drivers/ssb/pci.c
+++ b/drivers/ssb/pci.c
@@ -600,11 +600,9 @@ static int sprom_extract(struct ssb_bus *bus, struct ssb_sprom *out,
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);
+ ssb_printk(KERN_ERR PFX "Unsupported SPROM revision %d"
+ " detected\n", out->revision);
+ return -EINVAL;
}
if (out->boardflags_lo == 0xFFFF)
--
1.6.0.4
On Thu, 2010-11-18 at 11:27 -0500, John W. Linville wrote:
> On Wed, Nov 17, 2010 at 06:12:56PM +0100, Michael Büsch wrote:
> > On Tue, 2010-11-16 at 16:23 -0500, John W. Linville wrote:
> > > > - ssb_printk(KERN_WARNING PFX "Unsupported SPROM"
> > > > - " revision %d detected. Will extract"
> > > > - " v1\n", out->revision);
> > > > - out->revision = 1;
> > > > - sprom_extract_r123(out, in);
> > > > + ssb_printk(KERN_ERR PFX "Unsupported SPROM revision %d"
> > > > + " detected\n", out->revision);
> > > > + return -EINVAL;
> > > > }
> > > >
> > > > if (out->boardflags_lo == 0xFFFF)
> > >
> > > I think this is going to make my b43 PCI-E card not work...I'll try
> > > it and get back to you...
> >
> > Hm, what version does it report?
> [ 1036.293865] ssb: Unsupported SPROM revision 255 detected. Will extract v1
So what about specialcasing 255 instead of defaulting to 1 in general?
if (rev == 255)
rev = 1;
255 basically means "Vendor forgot to set this field". So it would only
default to 1 for those broken sproms.
--
Greetings Michael.
On Wed, Nov 03, 2010 at 11:28:46PM +0100, Rafał Miłecki wrote:
> Signed-off-by: Rafał Miłecki <[email protected]>
> ---
> As noticed my Michael we have this left from old times of limited amount of
> revisions. It's not reliable to treat unknown as rev1.
>
> Michael: would you like to add Reported-by?
>
> John: it's for 2.6.38.
> ---
> drivers/ssb/pci.c | 8 +++-----
> 1 files changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/ssb/pci.c b/drivers/ssb/pci.c
> index b5343ac..41a7337 100644
> --- a/drivers/ssb/pci.c
> +++ b/drivers/ssb/pci.c
> @@ -600,11 +600,9 @@ static int sprom_extract(struct ssb_bus *bus, struct ssb_sprom *out,
> 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);
> + ssb_printk(KERN_ERR PFX "Unsupported SPROM revision %d"
> + " detected\n", out->revision);
> + return -EINVAL;
> }
>
> if (out->boardflags_lo == 0xFFFF)
I think this is going to make my b43 PCI-E card not work...I'll try
it and get back to you...
John
--
John W. Linville Someday the world will need a hero, and you
[email protected] might be all we have. Be ready.
On 11/18/2010 11:07 AM, Michael Büsch wrote:
> On Thu, 2010-11-18 at 11:02 -0600, Larry Finger wrote:
>> On 11/18/2010 10:47 AM, Michael Büsch wrote:
>>> If it would really succeed to initialize the device, this would be a
>>> regulatory issue, because the sprom contains various power amplifier
>>> calibration data. I think it should rather fail and be fixed correctly
>>> instead of incorrectly using rev1 in that case.
>>
>> I agree that it is better to fail than use incorrect power data.
>>
>> Would it be useful if the SPROM data were logged when the revision is crap?
>
>
> We need to keep in mind that there will be no new SSB devices.
> It seems pretty much EOL'ed by Broadcom.
> So I'm not sure whether this would be of any use or just random dead
> code.
Good point. When we get the data for the one case we know exists, we will have a
better idea if a special quirk for this case is feasible. Assuming that this is
not the only example of this hardware, then we might limit the breakage from
this patch. At least the random code would be needed and useful in keeping our
maintainer happy, which has a some merit.
Larry
W dniu 3 listopada 2010 23:28 użytkownik Rafał Miłecki
<[email protected]> napisał:
> 1.6.0.4
Whoops, that old git doesn't nicely numerate patches like 1/2, 2/2. Sorry.
--
Rafał
On Tue, Nov 16, 2010 at 04:23:21PM -0500, John W. Linville wrote:
> On Wed, Nov 03, 2010 at 11:28:46PM +0100, Rafał Miłecki wrote:
> > Signed-off-by: Rafał Miłecki <[email protected]>
> > ---
> > As noticed my Michael we have this left from old times of limited amount of
> > revisions. It's not reliable to treat unknown as rev1.
> >
> > Michael: would you like to add Reported-by?
> >
> > John: it's for 2.6.38.
> > ---
> > drivers/ssb/pci.c | 8 +++-----
> > 1 files changed, 3 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/ssb/pci.c b/drivers/ssb/pci.c
> > index b5343ac..41a7337 100644
> > --- a/drivers/ssb/pci.c
> > +++ b/drivers/ssb/pci.c
> > @@ -600,11 +600,9 @@ static int sprom_extract(struct ssb_bus *bus, struct ssb_sprom *out,
> > 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);
> > + ssb_printk(KERN_ERR PFX "Unsupported SPROM revision %d"
> > + " detected\n", out->revision);
> > + return -EINVAL;
> > }
> >
> > if (out->boardflags_lo == 0xFFFF)
>
> I think this is going to make my b43 PCI-E card not work...I'll try
> it and get back to you...
Yeah...
[ 125.520348] b43-pci-bridge 0000:04:00.0: enabling device (0000 -> 0002)
[ 125.520359] b43-pci-bridge 0000:04:00.0: PCI INT A -> GSI 18 (level, low) -> IRQ 18
[ 125.520373] b43-pci-bridge 0000:04:00.0: setting latency timer to 64
[ 125.529241] ssb: Core 0 found: ChipCommon (cc 0x800, rev 0x11, vendor 0x4243)
[ 125.529255] ssb: Core 1 found: IEEE 802.11 (cc 0x812, rev 0x0A, vendor 0x4243)
[ 125.529268] ssb: Core 2 found: USB 1.1 Host (cc 0x817, rev 0x03, vendor 0x4243)
[ 125.529280] ssb: Core 3 found: PCI-E (cc 0x820, rev 0x01, vendor 0x4243)
[ 125.545750] ssb: WARNING: Invalid SPROM CRC (corrupt SPROM)
[ 125.545752] ssb: Unsupported SPROM revision 255 detected
[ 125.545797] ssb: Failed to register PCI version of SSB with error -22
[ 125.545810] b43-pci-bridge 0000:04:00.0: PCI INT A disabled
[ 125.545827] b43-pci-bridge: probe of 0000:04:00.0 failed with error -22
--
John W. Linville Someday the world will need a hero, and you
[email protected] might be all we have. Be ready.
2010/11/18 Michael Büsch <[email protected]>:
> On Thu, 2010-11-18 at 11:27 -0500, John W. Linville wrote:
>> On Wed, Nov 17, 2010 at 06:12:56PM +0100, Michael Büsch wrote:
>> > On Tue, 2010-11-16 at 16:23 -0500, John W. Linville wrote:
>> > > > - ssb_printk(KERN_WARNING PFX "Unsupported SPROM"
>> > > > - " revision %d detected. Will extract"
>> > > > - " v1\n", out->revision);
>> > > > - out->revision = 1;
>> > > > - sprom_extract_r123(out, in);
>> > > > + ssb_printk(KERN_ERR PFX "Unsupported SPROM revision %d"
>> > > > + " detected\n", out->revision);
>> > > > + return -EINVAL;
>> > > > }
>> > > >
>> > > > if (out->boardflags_lo == 0xFFFF)
>> > >
>> > > I think this is going to make my b43 PCI-E card not work...I'll try
>> > > it and get back to you...
>> >
>> > Hm, what version does it report?
>
>> [ 1036.293865] ssb: Unsupported SPROM revision 255 detected. Will extract v1
>
> So what about specialcasing 255 instead of defaulting to 1 in general?
>
> if (rev == 255)
> rev = 1;
>
> 255 basically means "Vendor forgot to set this field". So it would only
> default to 1 for those broken sproms.
Will work as long as there won't appear new vendor who will forget to
set this and will use new SPROM...
But hopefully it won't happen and it should not hurt too much to
register device with incorrectly parsed SPROM.
--
Rafał
On Wed, 2010-11-03 at 23:28 +0100, Rafał Miłecki wrote:
> Signed-off-by: Rafał Miłecki <[email protected]>
> ---
> As noticed my Michael we have this left from old times of limited amount of
> revisions. It's not reliable to treat unknown as rev1.
>
> Michael: would you like to add Reported-by?
Isn't that only for bugs? This isn't a bug.
But you can add my acked-by, if that's desired.
> John: it's for 2.6.38.
> ---
> drivers/ssb/pci.c | 8 +++-----
> 1 files changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/ssb/pci.c b/drivers/ssb/pci.c
> index b5343ac..41a7337 100644
> --- a/drivers/ssb/pci.c
> +++ b/drivers/ssb/pci.c
> @@ -600,11 +600,9 @@ static int sprom_extract(struct ssb_bus *bus, struct ssb_sprom *out,
> 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);
> + ssb_printk(KERN_ERR PFX "Unsupported SPROM revision %d"
> + " detected\n", out->revision);
> + return -EINVAL;
> }
>
> if (out->boardflags_lo == 0xFFFF)
--
Greetings Michael.
On Thu, 2010-11-18 at 11:02 -0600, Larry Finger wrote:
> On 11/18/2010 10:47 AM, Michael Büsch wrote:
> > If it would really succeed to initialize the device, this would be a
> > regulatory issue, because the sprom contains various power amplifier
> > calibration data. I think it should rather fail and be fixed correctly
> > instead of incorrectly using rev1 in that case.
>
> I agree that it is better to fail than use incorrect power data.
>
> Would it be useful if the SPROM data were logged when the revision is crap?
We need to keep in mind that there will be no new SSB devices.
It seems pretty much EOL'ed by Broadcom.
So I'm not sure whether this would be of any use or just random dead
code.
--
Greetings Michael.
On Wed, Nov 17, 2010 at 06:12:56PM +0100, Michael B?sch wrote:
> On Tue, 2010-11-16 at 16:23 -0500, John W. Linville wrote:
> > > - ssb_printk(KERN_WARNING PFX "Unsupported SPROM"
> > > - " revision %d detected. Will extract"
> > > - " v1\n", out->revision);
> > > - out->revision = 1;
> > > - sprom_extract_r123(out, in);
> > > + ssb_printk(KERN_ERR PFX "Unsupported SPROM revision %d"
> > > + " detected\n", out->revision);
> > > + return -EINVAL;
> > > }
> > >
> > > if (out->boardflags_lo == 0xFFFF)
> >
> > I think this is going to make my b43 PCI-E card not work...I'll try
> > it and get back to you...
>
> Hm, what version does it report?
This is what I see in dmesg (w/o the patch):
[ 1036.277235] ssb: Core 0 found: ChipCommon (cc 0x800, rev 0x11, vendor 0x4243)
[ 1036.277249] ssb: Core 1 found: IEEE 802.11 (cc 0x812, rev 0x0A, vendor 0x4243)
[ 1036.277267] ssb: Core 2 found: USB 1.1 Host (cc 0x817, rev 0x03, vendor 0x4243)
[ 1036.277277] ssb: Core 3 found: PCI-E (cc 0x820, rev 0x01, vendor 0x4243)
[ 1036.293856] ssb: WARNING: Invalid SPROM CRC (corrupt SPROM)
[ 1036.293865] ssb: Unsupported SPROM revision 255 detected. Will extract v1
[ 1036.301254] ssb: Sonics Silicon Backplane found on PCI device 0000:04:00.0
[ 1036.420643] b43-phy1: Broadcom 4311 WLAN found (core revision 10)
[ 1036.435060] b43-phy1 debug: Found PHY: Analog 4, Type 2, Revision 8
[ 1036.435082] b43-phy1 debug: Found Radio: Manuf 0x17F, Version 0x2050, Revision 2
[ 1036.445105] ieee80211 phy1: Selected rate control algorithm 'minstrel_ht'
[ 1036.450914] Registered led device: b43-phy1::tx
[ 1036.451466] Registered led device: b43-phy1::rx
[ 1036.451890] Registered led device: b43-phy1::radio
[ 1036.452226] Broadcom 43xx driver loaded [ Features: PMLS, Firmware-ID: FW13 ]
[ 1036.480736] udev[2440]: renamed network interface wlan1 to wlan2
[ 1036.513712] cfg80211: Calling CRDA for country: US
[ 1036.623061] b43-phy1: Loading OpenSource firmware version 410.31754
[ 1036.623075] b43-phy1: Hardware crypto acceleration not supported by firmware
[ 1036.623083] b43-phy1: QoS not supported by firmware
[ 1036.656362] b43-phy1 debug: Chip initialized
[ 1036.661477] b43-phy1 debug: 32-bit DMA initialized
[ 1036.661499] b43-phy1 debug: QoS disabled
[ 1036.672035] b43-phy1 debug: Wireless interface started
[ 1036.674628] b43-phy1 debug: Adding Interface type 2
--
John W. Linville Someday the world will need a hero, and you
[email protected] might be all we have. Be ready.
On 11/18/2010 10:47 AM, Michael Büsch wrote:
> If it would really succeed to initialize the device, this would be a
> regulatory issue, because the sprom contains various power amplifier
> calibration data. I think it should rather fail and be fixed correctly
> instead of incorrectly using rev1 in that case.
I agree that it is better to fail than use incorrect power data.
Would it be useful if the SPROM data were logged when the revision is crap?
John: could you dump and post the data from yours? I would like to see how bad
it is.
Larry
On Thu, 2010-11-18 at 17:44 +0100, Rafał Miłecki wrote:
> 2010/11/18 Michael Büsch <[email protected]>:
> >> [ 1036.293865] ssb: Unsupported SPROM revision 255 detected. Will extract v1
> >
> > So what about specialcasing 255 instead of defaulting to 1 in general?
> >
> > if (rev == 255)
> > rev = 1;
> >
> > 255 basically means "Vendor forgot to set this field". So it would only
> > default to 1 for those broken sproms.
>
> Will work as long as there won't appear new vendor who will forget to
> set this and will use new SPROM...
The old code will break for that, too.
> But hopefully it won't happen and it should not hurt too much to
> register device with incorrectly parsed SPROM.
If it would really succeed to initialize the device, this would be a
regulatory issue, because the sprom contains various power amplifier
calibration data. I think it should rather fail and be fixed correctly
instead of incorrectly using rev1 in that case.
--
Greetings Michael.
2010/11/18 John W. Linville <[email protected]>:
> On Tue, Nov 16, 2010 at 04:23:21PM -0500, John W. Linville wrote:
>> On Wed, Nov 03, 2010 at 11:28:46PM +0100, Rafał Miłecki wrote:
>> > Signed-off-by: Rafał Miłecki <[email protected]>
>> > ---
>> > As noticed my Michael we have this left from old times of limited amount of
>> > revisions. It's not reliable to treat unknown as rev1.
>> >
>> > Michael: would you like to add Reported-by?
>> >
>> > John: it's for 2.6.38.
>> > ---
>> > drivers/ssb/pci.c | 8 +++-----
>> > 1 files changed, 3 insertions(+), 5 deletions(-)
>> >
>> > diff --git a/drivers/ssb/pci.c b/drivers/ssb/pci.c
>> > index b5343ac..41a7337 100644
>> > --- a/drivers/ssb/pci.c
>> > +++ b/drivers/ssb/pci.c
>> > @@ -600,11 +600,9 @@ static int sprom_extract(struct ssb_bus *bus, struct ssb_sprom *out,
>> > 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);
>> > + ssb_printk(KERN_ERR PFX "Unsupported SPROM revision %d"
>> > + " detected\n", out->revision);
>> > + return -EINVAL;
>> > }
>> >
>> > if (out->boardflags_lo == 0xFFFF)
>>
>> I think this is going to make my b43 PCI-E card not work...I'll try
>> it and get back to you...
>
> Yeah...
>
> [ 125.520348] b43-pci-bridge 0000:04:00.0: enabling device (0000 -> 0002)
> [ 125.520359] b43-pci-bridge 0000:04:00.0: PCI INT A -> GSI 18 (level, low) -> IRQ 18
> [ 125.520373] b43-pci-bridge 0000:04:00.0: setting latency timer to 64
> [ 125.529241] ssb: Core 0 found: ChipCommon (cc 0x800, rev 0x11, vendor 0x4243)
> [ 125.529255] ssb: Core 1 found: IEEE 802.11 (cc 0x812, rev 0x0A, vendor 0x4243)
> [ 125.529268] ssb: Core 2 found: USB 1.1 Host (cc 0x817, rev 0x03, vendor 0x4243)
> [ 125.529280] ssb: Core 3 found: PCI-E (cc 0x820, rev 0x01, vendor 0x4243)
> [ 125.545750] ssb: WARNING: Invalid SPROM CRC (corrupt SPROM)
> [ 125.545752] ssb: Unsupported SPROM revision 255 detected
> [ 125.545797] ssb: Failed to register PCI version of SSB with error -22
> [ 125.545810] b43-pci-bridge 0000:04:00.0: PCI INT A disabled
> [ 125.545827] b43-pci-bridge: probe of 0000:04:00.0 failed with error -22
Hm, now I wonder if we should create sth like list of devices with
invalid SPROM, or maybe we should keep current fallback method?
--
Rafał
On Tue, 2010-11-16 at 16:23 -0500, John W. Linville wrote:
> > - ssb_printk(KERN_WARNING PFX "Unsupported SPROM"
> > - " revision %d detected. Will extract"
> > - " v1\n", out->revision);
> > - out->revision = 1;
> > - sprom_extract_r123(out, in);
> > + ssb_printk(KERN_ERR PFX "Unsupported SPROM revision %d"
> > + " detected\n", out->revision);
> > + return -EINVAL;
> > }
> >
> > if (out->boardflags_lo == 0xFFFF)
>
> I think this is going to make my b43 PCI-E card not work...I'll try
> it and get back to you...
Hm, what version does it report?
--
Greetings Michael.