Re: [PATCH 1/2] PCI: add workaround for PLX PCI 9050 bug

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Oct 30, 2012 at 5:03 AM, Ian Abbott <abbotti@xxxxxxxxx> 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@xxxxxxxxx> 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@xxxxxxxxx>
>>>> ---
>>>>    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@xxxxxxxxx>        )=-
> -=( Tel: +44 (0)161 477 1898   FAX: +44 (0)161 718 3587         )=-
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux