All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] PCI: add workaround for PLX PCI 9050 bug
@ 2012-10-29 14:40 Ian Abbott
  2012-10-29 14:40 ` [PATCH 2/2] PCI: add PLX PCI 9050 workaround for some Meilhaus DAQ cards Ian Abbott
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Ian Abbott @ 2012-10-29 14:40 UTC (permalink / raw)
  To: linux-pci; +Cc: Bjorn Helgaas, Ian Abbott

The PLX PCI 9050 PCI Target bridge controller has a bug that prevents
its local configuration registers being read through BAR0 (memory) or
BAR1 (i/o) if the base address lies on an odd 128-byte boundary, i.e. if
bit 7 of the base address is non-zero.  This bug is described in the PCI
9050 errata list, version 1.4, May 2005.  It was fixed in the
pin-compatible PCI 9052, which can be distinguished from the PCI 9050 by
checking the revision in the PCI header, which is hard-coded for these
chips.

Workaround the problem by re-allocating the affected regions to a
256-byte boundary.  Note that BAR0 and/or BAR1 may have been disabled
(size 0) during initialization of the PCI chip when its configuration is
read from a serial EEPROM.

Currently, the fix-up has only been used for devices with the default
vendor and device ID of the PLX PCI 9050.  The PCI 9052 shares the same
default device ID as the PCI 9050 but they have different PCI revision
codes.

Signed-off-by: Ian Abbott <abbotti@mev.co.uk>
---
 drivers/pci/quirks.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 7a451ff..7e6be56 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -1790,6 +1790,31 @@ DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_TOSHIBA_2,
 			 PCI_DEVICE_ID_TOSHIBA_TC86C001_IDE,
 			 quirk_tc86c001_ide);
 
+/*
+ * PLX PCI 9050 PCI Target bridge controller has an errata that prevents the
+ * local configuration registers accessible via BAR0 (memory) or BAR1 (i/o)
+ * being read correctly if bit 7 of the base address is set.
+ * The BAR0 or BAR1 region may be disabled (size 0) or enabled (size 128).
+ * Re-allocate the regions to a 256-byte boundary if necessary.
+ */
+static void __devinit quirk_plx_pci9050(struct pci_dev *dev)
+{
+	unsigned int bar;
+
+	/* Fixed in revision 2 (PCI 9052). */
+	if (dev->revision >= 2)
+		return;
+	for (bar = 0; bar <= 1; bar++)
+		if (pci_resource_len(dev, bar) == 0x80 &&
+		    (pci_resource_start(dev, bar) & 0x80)) {
+			struct resource *r = &dev->resource[bar];
+			r->start = 0;
+			r->end = 0xff;
+		}
+}
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_PLX, PCI_DEVICE_ID_PLX_9050,
+			 quirk_plx_pci9050);
+
 static void __devinit quirk_netmos(struct pci_dev *dev)
 {
 	unsigned int num_parallel = (dev->subsystem_device & 0xf0) >> 4;
-- 
1.7.12.4


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 2/2] PCI: add PLX PCI 9050 workaround for some Meilhaus DAQ cards
  2012-10-29 14:40 [PATCH 1/2] PCI: add workaround for PLX PCI 9050 bug Ian Abbott
@ 2012-10-29 14:40 ` Ian Abbott
  2012-10-30  3:13 ` [PATCH 1/2] PCI: add workaround for PLX PCI 9050 bug Bjorn Helgaas
  2012-10-30 17:25 ` [PATCH 1/2 v2] " Ian Abbott
  2 siblings, 0 replies; 8+ messages in thread
From: Ian Abbott @ 2012-10-29 14:40 UTC (permalink / raw)
  To: linux-pci; +Cc: Bjorn Helgaas, Ian Abbott

The Meilhaus ME-2000i and ME-2600i data acquisition cards supported by
the Comedi "me_daq" driver use the PLX PCI 9050 PCI Target bridge chip
affected by the bug that prevents the chip's local configuration
registers being read from BAR0 or BAR1 base addresses that are an odd
multiple of 128 bytes.  Use the PLX PCI 9050 quirk handler for these
devices to re-allocate affected regions to a 256-byte boundary.

Signed-off-by: Ian Abbott <abbotti@mev.co.uk>
---
 drivers/pci/quirks.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 7e6be56..f9a1b33 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -1814,6 +1814,17 @@ static void __devinit quirk_plx_pci9050(struct pci_dev *dev)
 }
 DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_PLX, PCI_DEVICE_ID_PLX_9050,
 			 quirk_plx_pci9050);
+/*
+ * The following Meilhaus (vendor ID 0x1402) device IDs (amongst others)
+ * may be using the PLX PCI 9050: 0x0630, 0x0940, 0x0950, 0x0960, 0x100b,
+ * 0x1400, 0x140a, 0x140b, 0x14e0, 0x14ea, 0x14eb, 0x1604, 0x1608, 0x160c,
+ * 0x168f, 0x2000, 0x2600, 0x3000, 0x810a, 0x810b.
+ *
+ * Currently, device IDs 0x2000 and 0x2600 are used by the Comedi "me_daq"
+ * driver.
+ */
+DECLARE_PCI_FIXUP_HEADER(0x1402, 0x2000, quirk_plx_pci9050);
+DECLARE_PCI_FIXUP_HEADER(0x1402, 0x2600, quirk_plx_pci9050);
 
 static void __devinit quirk_netmos(struct pci_dev *dev)
 {
-- 
1.7.12.4


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/2] PCI: add workaround for PLX PCI 9050 bug
  2012-10-29 14:40 [PATCH 1/2] PCI: add workaround for PLX PCI 9050 bug Ian Abbott
  2012-10-29 14:40 ` [PATCH 2/2] PCI: add PLX PCI 9050 workaround for some Meilhaus DAQ cards Ian Abbott
@ 2012-10-30  3:13 ` Bjorn Helgaas
  2012-10-30  8:01   ` Ian Abbott
  2012-10-30 17:25 ` [PATCH 1/2 v2] " Ian Abbott
  2 siblings, 1 reply; 8+ messages in thread
From: Bjorn Helgaas @ 2012-10-30  3:13 UTC (permalink / raw)
  To: Ian Abbott; +Cc: linux-pci

On Mon, Oct 29, 2012 at 8:40 AM, Ian Abbott <abbotti@mev.co.uk> wrote:
> The PLX PCI 9050 PCI Target bridge controller has a bug that prevents
> its local configuration registers being read through BAR0 (memory) or
> BAR1 (i/o) if the base address lies on an odd 128-byte boundary, i.e. if
> bit 7 of the base address is non-zero.  This bug is described in the PCI
> 9050 errata list, version 1.4, May 2005.  It was fixed in the
> pin-compatible PCI 9052, which can be distinguished from the PCI 9050 by
> checking the revision in the PCI header, which is hard-coded for these
> chips.
>
> Workaround the problem by re-allocating the affected regions to a
> 256-byte boundary.  Note that BAR0 and/or BAR1 may have been disabled
> (size 0) during initialization of the PCI chip when its configuration is
> read from a serial EEPROM.
>
> Currently, the fix-up has only been used for devices with the default
> vendor and device ID of the PLX PCI 9050.  The PCI 9052 shares the same
> default device ID as the PCI 9050 but they have different PCI revision
> codes.
>
> Signed-off-by: Ian Abbott <abbotti@mev.co.uk>
> ---
>  drivers/pci/quirks.c | 25 +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
>
> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> index 7a451ff..7e6be56 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -1790,6 +1790,31 @@ DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_TOSHIBA_2,
>                          PCI_DEVICE_ID_TOSHIBA_TC86C001_IDE,
>                          quirk_tc86c001_ide);
>
> +/*
> + * PLX PCI 9050 PCI Target bridge controller has an errata that prevents the
> + * local configuration registers accessible via BAR0 (memory) or BAR1 (i/o)
> + * being read correctly if bit 7 of the base address is set.
> + * The BAR0 or BAR1 region may be disabled (size 0) or enabled (size 128).
> + * Re-allocate the regions to a 256-byte boundary if necessary.
> + */
> +static void __devinit quirk_plx_pci9050(struct pci_dev *dev)
> +{
> +       unsigned int bar;
> +
> +       /* Fixed in revision 2 (PCI 9052). */
> +       if (dev->revision >= 2)
> +               return;
> +       for (bar = 0; bar <= 1; bar++)
> +               if (pci_resource_len(dev, bar) == 0x80 &&
> +                   (pci_resource_start(dev, bar) & 0x80)) {
> +                       struct resource *r = &dev->resource[bar];
> +                       r->start = 0;
> +                       r->end = 0xff;

I assume the intent here is to make these BARs "unassigned" so they
will be reassigned later?  We don't yet have a clean generic way of
indicating "unassigned," so "r->start = 0" is the best we can do for
now.

I think it'd be nice to have a dev_info() here to explain what's going
on.  Otherwise, the dmesg will be a bit mysterious.

> +               }
> +}
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_PLX, PCI_DEVICE_ID_PLX_9050,
> +                        quirk_plx_pci9050);
> +
>  static void __devinit quirk_netmos(struct pci_dev *dev)
>  {
>         unsigned int num_parallel = (dev->subsystem_device & 0xf0) >> 4;
> --
> 1.7.12.4
>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/2] PCI: add workaround for PLX PCI 9050 bug
  2012-10-30  3:13 ` [PATCH 1/2] PCI: add workaround for PLX PCI 9050 bug Bjorn Helgaas
@ 2012-10-30  8:01   ` Ian Abbott
  2012-10-30 11:03     ` Ian Abbott
  0 siblings, 1 reply; 8+ messages in thread
From: Ian Abbott @ 2012-10-30  8:01 UTC (permalink / raw)
  To: Bjorn Helgaas; +Cc: Ian Abbott, linux-pci

On 30/10/12 03:13, Bjorn Helgaas wrote:
> On Mon, Oct 29, 2012 at 8:40 AM, Ian Abbott <abbotti@mev.co.uk> wrote:
>> The PLX PCI 9050 PCI Target bridge controller has a bug that prevents
>> its local configuration registers being read through BAR0 (memory) or
>> BAR1 (i/o) if the base address lies on an odd 128-byte boundary, i.e. if
>> bit 7 of the base address is non-zero.  This bug is described in the PCI
>> 9050 errata list, version 1.4, May 2005.  It was fixed in the
>> pin-compatible PCI 9052, which can be distinguished from the PCI 9050 by
>> checking the revision in the PCI header, which is hard-coded for these
>> chips.
>>
>> Workaround the problem by re-allocating the affected regions to a
>> 256-byte boundary.  Note that BAR0 and/or BAR1 may have been disabled
>> (size 0) during initialization of the PCI chip when its configuration is
>> read from a serial EEPROM.
>>
>> Currently, the fix-up has only been used for devices with the default
>> vendor and device ID of the PLX PCI 9050.  The PCI 9052 shares the same
>> default device ID as the PCI 9050 but they have different PCI revision
>> codes.
>>
>> Signed-off-by: Ian Abbott <abbotti@mev.co.uk>
>> ---
>>   drivers/pci/quirks.c | 25 +++++++++++++++++++++++++
>>   1 file changed, 25 insertions(+)
>>
>> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
>> index 7a451ff..7e6be56 100644
>> --- a/drivers/pci/quirks.c
>> +++ b/drivers/pci/quirks.c
>> @@ -1790,6 +1790,31 @@ DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_TOSHIBA_2,
>>                           PCI_DEVICE_ID_TOSHIBA_TC86C001_IDE,
>>                           quirk_tc86c001_ide);
>>
>> +/*
>> + * PLX PCI 9050 PCI Target bridge controller has an errata that prevents the
>> + * local configuration registers accessible via BAR0 (memory) or BAR1 (i/o)
>> + * being read correctly if bit 7 of the base address is set.
>> + * The BAR0 or BAR1 region may be disabled (size 0) or enabled (size 128).
>> + * Re-allocate the regions to a 256-byte boundary if necessary.
>> + */
>> +static void __devinit quirk_plx_pci9050(struct pci_dev *dev)
>> +{
>> +       unsigned int bar;
>> +
>> +       /* Fixed in revision 2 (PCI 9052). */
>> +       if (dev->revision >= 2)
>> +               return;
>> +       for (bar = 0; bar <= 1; bar++)
>> +               if (pci_resource_len(dev, bar) == 0x80 &&
>> +                   (pci_resource_start(dev, bar) & 0x80)) {
>> +                       struct resource *r = &dev->resource[bar];
>> +                       r->start = 0;
>> +                       r->end = 0xff;
>
> I assume the intent here is to make these BARs "unassigned" so they
> will be reassigned later?  We don't yet have a clean generic way of
> indicating "unassigned," so "r->start = 0" is the best we can do for
> now.

I more-or-less copied the method from quirk_tc86c001_ide().  I don't 
have any prior experience with writing PCI quirks, so I don't know if 
this is the best way to do it!  All I really care about is that these 
BARs don't have bit 7 set.

> I think it'd be nice to have a dev_info() here to explain what's going
> on.  Otherwise, the dmesg will be a bit mysterious.

OK, I'll add that in the next version of this patch.


-- 
-=( Ian Abbott @ MEV Ltd.    E-mail: <abbotti@mev.co.uk>        )=-
-=( Tel: +44 (0)161 477 1898   FAX: +44 (0)161 718 3587         )=-

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/2] PCI: add workaround for PLX PCI 9050 bug
  2012-10-30  8:01   ` Ian Abbott
@ 2012-10-30 11:03     ` Ian Abbott
  2012-10-30 17:04       ` Bjorn Helgaas
  0 siblings, 1 reply; 8+ messages in thread
From: Ian Abbott @ 2012-10-30 11:03 UTC (permalink / raw)
  To: Bjorn Helgaas; +Cc: Ian Abbott, linux-pci

On 2012-10-30 08:01, Ian Abbott wrote:
> On 30/10/12 03:13, Bjorn Helgaas wrote:
>> On Mon, Oct 29, 2012 at 8:40 AM, Ian Abbott <abbotti@mev.co.uk> wrote:
>>> The PLX PCI 9050 PCI Target bridge controller has a bug that prevents
>>> its local configuration registers being read through BAR0 (memory) or
>>> BAR1 (i/o) if the base address lies on an odd 128-byte boundary, i.e. if
>>> bit 7 of the base address is non-zero.  This bug is described in the PCI
>>> 9050 errata list, version 1.4, May 2005.  It was fixed in the
>>> pin-compatible PCI 9052, which can be distinguished from the PCI 9050 by
>>> checking the revision in the PCI header, which is hard-coded for these
>>> chips.
>>>
>>> Workaround the problem by re-allocating the affected regions to a
>>> 256-byte boundary.  Note that BAR0 and/or BAR1 may have been disabled
>>> (size 0) during initialization of the PCI chip when its configuration is
>>> read from a serial EEPROM.
>>>
>>> Currently, the fix-up has only been used for devices with the default
>>> vendor and device ID of the PLX PCI 9050.  The PCI 9052 shares the same
>>> default device ID as the PCI 9050 but they have different PCI revision
>>> codes.
>>>
>>> Signed-off-by: Ian Abbott <abbotti@mev.co.uk>
>>> ---
>>>    drivers/pci/quirks.c | 25 +++++++++++++++++++++++++
>>>    1 file changed, 25 insertions(+)
>>>
>>> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
>>> index 7a451ff..7e6be56 100644
>>> --- a/drivers/pci/quirks.c
>>> +++ b/drivers/pci/quirks.c
>>> @@ -1790,6 +1790,31 @@ DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_TOSHIBA_2,
>>>                            PCI_DEVICE_ID_TOSHIBA_TC86C001_IDE,
>>>                            quirk_tc86c001_ide);
>>>
>>> +/*
>>> + * PLX PCI 9050 PCI Target bridge controller has an errata that prevents the
>>> + * local configuration registers accessible via BAR0 (memory) or BAR1 (i/o)
>>> + * being read correctly if bit 7 of the base address is set.
>>> + * The BAR0 or BAR1 region may be disabled (size 0) or enabled (size 128).
>>> + * Re-allocate the regions to a 256-byte boundary if necessary.
>>> + */
>>> +static void __devinit quirk_plx_pci9050(struct pci_dev *dev)
>>> +{
>>> +       unsigned int bar;
>>> +
>>> +       /* Fixed in revision 2 (PCI 9052). */
>>> +       if (dev->revision >= 2)
>>> +               return;
>>> +       for (bar = 0; bar <= 1; bar++)
>>> +               if (pci_resource_len(dev, bar) == 0x80 &&
>>> +                   (pci_resource_start(dev, bar) & 0x80)) {
>>> +                       struct resource *r = &dev->resource[bar];
>>> +                       r->start = 0;
>>> +                       r->end = 0xff;
>>
>> I assume the intent here is to make these BARs "unassigned" so they
>> will be reassigned later?  We don't yet have a clean generic way of
>> indicating "unassigned," so "r->start = 0" is the best we can do for
>> now.
>
> I more-or-less copied the method from quirk_tc86c001_ide().  I don't
> have any prior experience with writing PCI quirks, so I don't know if
> this is the best way to do it!  All I really care about is that these
> BARs don't have bit 7 set.
>
>> I think it'd be nice to have a dev_info() here to explain what's going
>> on.  Otherwise, the dmesg will be a bit mysterious.
>
> OK, I'll add that in the next version of this patch.

I've added a dev_info() on my local system that I'll submit in version 2 
of the patch later but first I thought I'd show the dmesg output I get 
with this patch:

pci 0000:01:08.0: [14dc:0004] type 00 class 0x068000
pci 0000:01:08.0: reg 14: [io  0xd400-0xd47f]
pci 0000:01:08.0: reg 18: [io  0xd000-0xd007]
pci 0000:01:08.0: reg 1c: [io  0xcc00-0xcc07]
pci 0000:01:08.0: Re-allocating PLX PCI 9050 BAR 1 to avoid 0x80 
boundary bug

That last message is the one I added locally.  The vendor and device ID 
in the first message is different than the ones I'm applying in these 
patches; it's just a local change.  You may also notice that BAR 1 (reg 
14) doesn't have bit 7 set to 1, so I had to temporarily disable that 
check in the code to test the re-allocation mechanism.  This particular 
device has BAR 0 (reg 10) disabled (size 0) but it would normally be a 
memory region of size 128.

Resetting the region's start address to 0 had some knock on effects that 
I'm a bit concerned about:

...
pnp 00:02: [io  0x0010-0x001f]
pnp 00:02: [io  0x0022-0x003f]
pnp 00:02: [io  0x0044-0x005f]
pnp 00:02: [io  0x0062-0x0063]
pnp 00:02: [io  0x0065-0x006f]
pnp 00:02: [io  0x0074-0x007f]
pnp 00:02: [io  0x0091-0x0093]
pnp 00:02: [io  0x00a2-0x00bf]
pnp 00:02: [io  0x00e0-0x00ef]
pnp 00:02: [io  0x04d0-0x04d1]
pnp 00:02: [io  0x0800-0x087f]
pnp 00:02: [io  0x0290-0x0297]
pnp 00:02: disabling [io  0x0010-0x001f] because it overlaps 
0000:01:08.0 BAR 1 [io  0x0000-0x00ff]
pnp 00:02: disabling [io  0x0022-0x003f] because it overlaps 
0000:01:08.0 BAR 1 [io  0x0000-0x00ff]
pnp 00:02: disabling [io  0x0044-0x005f] because it overlaps 
0000:01:08.0 BAR 1 [io  0x0000-0x00ff]
pnp 00:02: disabling [io  0x0062-0x0063] because it overlaps 
0000:01:08.0 BAR 1 [io  0x0000-0x00ff]
pnp 00:02: disabling [io  0x0065-0x006f] because it overlaps 
0000:01:08.0 BAR 1 [io  0x0000-0x00ff]
pnp 00:02: disabling [io  0x0074-0x007f] because it overlaps 
0000:01:08.0 BAR 1 [io  0x0000-0x00ff]
pnp 00:02: disabling [io  0x0091-0x0093] because it overlaps 
0000:01:08.0 BAR 1 [io  0x0000-0x00ff]
pnp 00:02: disabling [io  0x00a2-0x00bf] because it overlaps 
0000:01:08.0 BAR 1 [io  0x0000-0x00ff]
pnp 00:02: disabling [io  0x00e0-0x00ef] because it overlaps 
0000:01:08.0 BAR 1 [io  0x0000-0x00ff]
system 00:02: [io  0x04d0-0x04d1] has been reserved
system 00:02: [io  0x0800-0x087f] has been reserved
system 00:02: [io  0x0290-0x0297] has been reserved
system 00:02: Plug and Play ACPI device, IDs PNP0c02 (active)

Eventually, BAR 1 of the PCI device gets re-assigned:

...
pci 0000:01:08.0: BAR 1: assigned [io  0xc000-0xc0ff]

-- 
-=( Ian Abbott @ MEV Ltd.    E-mail: <abbotti@mev.co.uk>        )=-
-=( Tel: +44 (0)161 477 1898   FAX: +44 (0)161 718 3587         )=-

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/2] PCI: add workaround for PLX PCI 9050 bug
  2012-10-30 11:03     ` Ian Abbott
@ 2012-10-30 17:04       ` Bjorn Helgaas
  0 siblings, 0 replies; 8+ messages in thread
From: Bjorn Helgaas @ 2012-10-30 17:04 UTC (permalink / raw)
  To: Ian Abbott; +Cc: Ian Abbott, linux-pci

On Tue, Oct 30, 2012 at 5:03 AM, Ian Abbott <abbotti@mev.co.uk> wrote:
> On 2012-10-30 08:01, Ian Abbott wrote:
>>
>> On 30/10/12 03:13, Bjorn Helgaas wrote:
>>>
>>> On Mon, Oct 29, 2012 at 8:40 AM, Ian Abbott <abbotti@mev.co.uk> wrote:
>>>>
>>>> The PLX PCI 9050 PCI Target bridge controller has a bug that prevents
>>>> its local configuration registers being read through BAR0 (memory) or
>>>> BAR1 (i/o) if the base address lies on an odd 128-byte boundary, i.e. if
>>>> bit 7 of the base address is non-zero.  This bug is described in the PCI
>>>> 9050 errata list, version 1.4, May 2005.  It was fixed in the
>>>> pin-compatible PCI 9052, which can be distinguished from the PCI 9050 by
>>>> checking the revision in the PCI header, which is hard-coded for these
>>>> chips.
>>>>
>>>> Workaround the problem by re-allocating the affected regions to a
>>>> 256-byte boundary.  Note that BAR0 and/or BAR1 may have been disabled
>>>> (size 0) during initialization of the PCI chip when its configuration is
>>>> read from a serial EEPROM.
>>>>
>>>> Currently, the fix-up has only been used for devices with the default
>>>> vendor and device ID of the PLX PCI 9050.  The PCI 9052 shares the same
>>>> default device ID as the PCI 9050 but they have different PCI revision
>>>> codes.
>>>>
>>>> Signed-off-by: Ian Abbott <abbotti@mev.co.uk>
>>>> ---
>>>>    drivers/pci/quirks.c | 25 +++++++++++++++++++++++++
>>>>    1 file changed, 25 insertions(+)
>>>>
>>>> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
>>>> index 7a451ff..7e6be56 100644
>>>> --- a/drivers/pci/quirks.c
>>>> +++ b/drivers/pci/quirks.c
>>>> @@ -1790,6 +1790,31 @@ DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_TOSHIBA_2,
>>>>                            PCI_DEVICE_ID_TOSHIBA_TC86C001_IDE,
>>>>                            quirk_tc86c001_ide);
>>>>
>>>> +/*
>>>> + * PLX PCI 9050 PCI Target bridge controller has an errata that
>>>> prevents the
>>>> + * local configuration registers accessible via BAR0 (memory) or BAR1
>>>> (i/o)
>>>> + * being read correctly if bit 7 of the base address is set.
>>>> + * The BAR0 or BAR1 region may be disabled (size 0) or enabled (size
>>>> 128).
>>>> + * Re-allocate the regions to a 256-byte boundary if necessary.
>>>> + */
>>>> +static void __devinit quirk_plx_pci9050(struct pci_dev *dev)
>>>> +{
>>>> +       unsigned int bar;
>>>> +
>>>> +       /* Fixed in revision 2 (PCI 9052). */
>>>> +       if (dev->revision >= 2)
>>>> +               return;
>>>> +       for (bar = 0; bar <= 1; bar++)
>>>> +               if (pci_resource_len(dev, bar) == 0x80 &&
>>>> +                   (pci_resource_start(dev, bar) & 0x80)) {
>>>> +                       struct resource *r = &dev->resource[bar];
>>>> +                       r->start = 0;
>>>> +                       r->end = 0xff;
>>>
>>>
>>> I assume the intent here is to make these BARs "unassigned" so they
>>> will be reassigned later?  We don't yet have a clean generic way of
>>> indicating "unassigned," so "r->start = 0" is the best we can do for
>>> now.
>>
>>
>> I more-or-less copied the method from quirk_tc86c001_ide().  I don't
>> have any prior experience with writing PCI quirks, so I don't know if
>> this is the best way to do it!  All I really care about is that these
>> BARs don't have bit 7 set.
>>
>>> I think it'd be nice to have a dev_info() here to explain what's going
>>> on.  Otherwise, the dmesg will be a bit mysterious.
>>
>>
>> OK, I'll add that in the next version of this patch.
>
>
> I've added a dev_info() on my local system that I'll submit in version 2 of
> the patch later but first I thought I'd show the dmesg output I get with
> this patch:
>
> pci 0000:01:08.0: [14dc:0004] type 00 class 0x068000
> pci 0000:01:08.0: reg 14: [io  0xd400-0xd47f]
> pci 0000:01:08.0: reg 18: [io  0xd000-0xd007]
> pci 0000:01:08.0: reg 1c: [io  0xcc00-0xcc07]
> pci 0000:01:08.0: Re-allocating PLX PCI 9050 BAR 1 to avoid 0x80 boundary
> bug
>
> That last message is the one I added locally.  The vendor and device ID in
> the first message is different than the ones I'm applying in these patches;
> it's just a local change.  You may also notice that BAR 1 (reg 14) doesn't
> have bit 7 set to 1, so I had to temporarily disable that check in the code
> to test the re-allocation mechanism.  This particular device has BAR 0 (reg
> 10) disabled (size 0) but it would normally be a memory region of size 128.
>
> Resetting the region's start address to 0 had some knock on effects that I'm
> a bit concerned about:
>
> ...
> pnp 00:02: [io  0x0010-0x001f]
> pnp 00:02: [io  0x0022-0x003f]
> pnp 00:02: [io  0x0044-0x005f]
> pnp 00:02: [io  0x0062-0x0063]
> pnp 00:02: [io  0x0065-0x006f]
> pnp 00:02: [io  0x0074-0x007f]
> pnp 00:02: [io  0x0091-0x0093]
> pnp 00:02: [io  0x00a2-0x00bf]
> pnp 00:02: [io  0x00e0-0x00ef]
> pnp 00:02: [io  0x04d0-0x04d1]
> pnp 00:02: [io  0x0800-0x087f]
> pnp 00:02: [io  0x0290-0x0297]
> pnp 00:02: disabling [io  0x0010-0x001f] because it overlaps 0000:01:08.0
> BAR 1 [io  0x0000-0x00ff]
> pnp 00:02: disabling [io  0x0022-0x003f] because it overlaps 0000:01:08.0
> BAR 1 [io  0x0000-0x00ff]
> pnp 00:02: disabling [io  0x0044-0x005f] because it overlaps 0000:01:08.0
> BAR 1 [io  0x0000-0x00ff]
> pnp 00:02: disabling [io  0x0062-0x0063] because it overlaps 0000:01:08.0
> BAR 1 [io  0x0000-0x00ff]
> pnp 00:02: disabling [io  0x0065-0x006f] because it overlaps 0000:01:08.0
> BAR 1 [io  0x0000-0x00ff]
> pnp 00:02: disabling [io  0x0074-0x007f] because it overlaps 0000:01:08.0
> BAR 1 [io  0x0000-0x00ff]
> pnp 00:02: disabling [io  0x0091-0x0093] because it overlaps 0000:01:08.0
> BAR 1 [io  0x0000-0x00ff]
> pnp 00:02: disabling [io  0x00a2-0x00bf] because it overlaps 0000:01:08.0
> BAR 1 [io  0x0000-0x00ff]
> pnp 00:02: disabling [io  0x00e0-0x00ef] because it overlaps 0000:01:08.0
> BAR 1 [io  0x0000-0x00ff]

Yep, this is a long-standing issue, nothing to do with your quirk
specifically.  We need to make this overlap check smarter, but you
don't have to worry about it.

> system 00:02: [io  0x04d0-0x04d1] has been reserved
> system 00:02: [io  0x0800-0x087f] has been reserved
> system 00:02: [io  0x0290-0x0297] has been reserved
> system 00:02: Plug and Play ACPI device, IDs PNP0c02 (active)
>
> Eventually, BAR 1 of the PCI device gets re-assigned:
>
> ...
> pci 0000:01:08.0: BAR 1: assigned [io  0xc000-0xc0ff]
>
>
> --
> -=( Ian Abbott @ MEV Ltd.    E-mail: <abbotti@mev.co.uk>        )=-
> -=( Tel: +44 (0)161 477 1898   FAX: +44 (0)161 718 3587         )=-

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 1/2 v2] PCI: add workaround for PLX PCI 9050 bug
  2012-10-29 14:40 [PATCH 1/2] PCI: add workaround for PLX PCI 9050 bug Ian Abbott
  2012-10-29 14:40 ` [PATCH 2/2] PCI: add PLX PCI 9050 workaround for some Meilhaus DAQ cards Ian Abbott
  2012-10-30  3:13 ` [PATCH 1/2] PCI: add workaround for PLX PCI 9050 bug Bjorn Helgaas
@ 2012-10-30 17:25 ` Ian Abbott
  2012-11-05 22:08   ` Bjorn Helgaas
  2 siblings, 1 reply; 8+ messages in thread
From: Ian Abbott @ 2012-10-30 17:25 UTC (permalink / raw)
  To: linux-pci; +Cc: Bjorn Helgaas, Ian Abbott

The PLX PCI 9050 PCI Target bridge controller has a bug that prevents
its local configuration registers being read through BAR0 (memory) or
BAR1 (i/o) if the base address lies on an odd 128-byte boundary, i.e. if
bit 7 of the base address is non-zero.  This bug is described in the PCI
9050 errata list, version 1.4, May 2005.  It was fixed in the
pin-compatible PCI 9052, which can be distinguished from the PCI 9050 by
checking the revision in the PCI header, which is hard-coded for these
chips.

Workaround the problem by re-allocating the affected regions to a
256-byte boundary.  Note that BAR0 and/or BAR1 may have been disabled
(size 0) during initialization of the PCI chip when its configuration is
read from a serial EEPROM.

Currently, the fix-up has only been used for devices with the default
vendor and device ID of the PLX PCI 9050.  The PCI 9052 shares the same
default device ID as the PCI 9050 but they have different PCI revision
codes.

Signed-off-by: Ian Abbott <abbotti@mev.co.uk>
---
v2: Use dev_info() to report when quirk applied.
---
 drivers/pci/quirks.c | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 7a451ff..2d0d1c2 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -1790,6 +1790,34 @@ DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_TOSHIBA_2,
 			 PCI_DEVICE_ID_TOSHIBA_TC86C001_IDE,
 			 quirk_tc86c001_ide);
 
+/*
+ * PLX PCI 9050 PCI Target bridge controller has an errata that prevents the
+ * local configuration registers accessible via BAR0 (memory) or BAR1 (i/o)
+ * being read correctly if bit 7 of the base address is set.
+ * The BAR0 or BAR1 region may be disabled (size 0) or enabled (size 128).
+ * Re-allocate the regions to a 256-byte boundary if necessary.
+ */
+static void __devinit quirk_plx_pci9050(struct pci_dev *dev)
+{
+	unsigned int bar;
+
+	/* Fixed in revision 2 (PCI 9052). */
+	if (dev->revision >= 2)
+		return;
+	for (bar = 0; bar <= 1; bar++)
+		if (pci_resource_len(dev, bar) == 0x80 &&
+		    (pci_resource_start(dev, bar) & 0x80)) {
+			struct resource *r = &dev->resource[bar];
+			dev_info(&dev->dev,
+				 "Re-allocating PLX PCI 9050 BAR %u to length 256 to avoid bit 7 bug\n",
+				 bar);
+			r->start = 0;
+			r->end = 0xff;
+		}
+}
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_PLX, PCI_DEVICE_ID_PLX_9050,
+			 quirk_plx_pci9050);
+
 static void __devinit quirk_netmos(struct pci_dev *dev)
 {
 	unsigned int num_parallel = (dev->subsystem_device & 0xf0) >> 4;
-- 
1.7.12.4


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/2 v2] PCI: add workaround for PLX PCI 9050 bug
  2012-10-30 17:25 ` [PATCH 1/2 v2] " Ian Abbott
@ 2012-11-05 22:08   ` Bjorn Helgaas
  0 siblings, 0 replies; 8+ messages in thread
From: Bjorn Helgaas @ 2012-11-05 22:08 UTC (permalink / raw)
  To: Ian Abbott; +Cc: linux-pci

On Tue, Oct 30, 2012 at 11:25 AM, Ian Abbott <abbotti@mev.co.uk> wrote:
> The PLX PCI 9050 PCI Target bridge controller has a bug that prevents
> its local configuration registers being read through BAR0 (memory) or
> BAR1 (i/o) if the base address lies on an odd 128-byte boundary, i.e. if
> bit 7 of the base address is non-zero.  This bug is described in the PCI
> 9050 errata list, version 1.4, May 2005.  It was fixed in the
> pin-compatible PCI 9052, which can be distinguished from the PCI 9050 by
> checking the revision in the PCI header, which is hard-coded for these
> chips.
>
> Workaround the problem by re-allocating the affected regions to a
> 256-byte boundary.  Note that BAR0 and/or BAR1 may have been disabled
> (size 0) during initialization of the PCI chip when its configuration is
> read from a serial EEPROM.
>
> Currently, the fix-up has only been used for devices with the default
> vendor and device ID of the PLX PCI 9050.  The PCI 9052 shares the same
> default device ID as the PCI 9050 but they have different PCI revision
> codes.
>
> Signed-off-by: Ian Abbott <abbotti@mev.co.uk>
> ---
> v2: Use dev_info() to report when quirk applied.
> ---
>  drivers/pci/quirks.c | 28 ++++++++++++++++++++++++++++
>  1 file changed, 28 insertions(+)
>
> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> index 7a451ff..2d0d1c2 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -1790,6 +1790,34 @@ DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_TOSHIBA_2,
>                          PCI_DEVICE_ID_TOSHIBA_TC86C001_IDE,
>                          quirk_tc86c001_ide);
>
> +/*
> + * PLX PCI 9050 PCI Target bridge controller has an errata that prevents the
> + * local configuration registers accessible via BAR0 (memory) or BAR1 (i/o)
> + * being read correctly if bit 7 of the base address is set.
> + * The BAR0 or BAR1 region may be disabled (size 0) or enabled (size 128).
> + * Re-allocate the regions to a 256-byte boundary if necessary.
> + */
> +static void __devinit quirk_plx_pci9050(struct pci_dev *dev)
> +{
> +       unsigned int bar;
> +
> +       /* Fixed in revision 2 (PCI 9052). */
> +       if (dev->revision >= 2)
> +               return;
> +       for (bar = 0; bar <= 1; bar++)
> +               if (pci_resource_len(dev, bar) == 0x80 &&
> +                   (pci_resource_start(dev, bar) & 0x80)) {
> +                       struct resource *r = &dev->resource[bar];
> +                       dev_info(&dev->dev,
> +                                "Re-allocating PLX PCI 9050 BAR %u to length 256 to avoid bit 7 bug\n",
> +                                bar);
> +                       r->start = 0;
> +                       r->end = 0xff;
> +               }
> +}
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_PLX, PCI_DEVICE_ID_PLX_9050,
> +                        quirk_plx_pci9050);
> +
>  static void __devinit quirk_netmos(struct pci_dev *dev)
>  {
>         unsigned int num_parallel = (dev->subsystem_device & 0xf0) >> 4;
> --

I applied both these patches to my pci/misc branch as v3.8 material.

Thanks!

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2012-11-05 22:08 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-10-29 14:40 [PATCH 1/2] PCI: add workaround for PLX PCI 9050 bug Ian Abbott
2012-10-29 14:40 ` [PATCH 2/2] PCI: add PLX PCI 9050 workaround for some Meilhaus DAQ cards Ian Abbott
2012-10-30  3:13 ` [PATCH 1/2] PCI: add workaround for PLX PCI 9050 bug Bjorn Helgaas
2012-10-30  8:01   ` Ian Abbott
2012-10-30 11:03     ` Ian Abbott
2012-10-30 17:04       ` Bjorn Helgaas
2012-10-30 17:25 ` [PATCH 1/2 v2] " Ian Abbott
2012-11-05 22:08   ` Bjorn Helgaas

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.