| Message ID | 20200520103147.985326-2-kw@linux.com |
|---|---|
| State | New |
| Headers | show |
| Series | PCI: Reference bridge window resources explicitly | expand |
On Wed, May 20, 2020 at 10:31:45AM +0000, Krzysztof Wilczynski wrote: > As per the specifications from the vendor: > > - "M1543 Preliminary Data Sheet", "M1543: Desktop South Bridge", > Acer Laboratories Inc., January 1998, Version 1.25, p. 78 > > - "M1543C Preliminary Datasheet", "M1543C Desktop Southbridge", > Acer Laboratories Inc., September 1998, Version 1.10, p. 126 > > Both the ACPI I/O and SMB I/O registers should be mapped into I/O space, > but the second quirk used to claimed an I/O resource from the memory > window which is not correct. > > Signed-off-by: Krzysztof Wilczynski <kw@linux.com> > --- > drivers/pci/quirks.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c > index ca9ed5774eb1..c71fdd8bd6f8 100644 > --- a/drivers/pci/quirks.c > +++ b/drivers/pci/quirks.c > @@ -654,7 +654,7 @@ DECLARE_PCI_FIXUP_CLASS_HEADER(PCI_VENDOR_ID_SYNOPSYS, PCI_ANY_ID, > static void quirk_ali7101_acpi(struct pci_dev *dev) > { > quirk_io_region(dev, 0xE0, 64, PCI_BRIDGE_RESOURCES, "ali7101 ACPI"); > - quirk_io_region(dev, 0xE2, 32, PCI_BRIDGE_RESOURCES+1, "ali7101 SMB"); > + quirk_io_region(dev, 0xE2, 32, PCI_BRIDGE_RESOURCES, "ali7101 SMB"); Wow, this is way too subtle. I proposed this fix, but I was wrong. I thought PCI_BRIDGE_RESOURCES was identifying a bridge window that the region should be claimed from: dev->resource[PCI_BRIDGE_RESOURCES] would be the I/O port window of a bridge. But that's not what's happening at all. quirk_io_region() is using that index to fill in a dev->resource[] entry with an IORESOURCE_IO resource, and then it calls pci_claim_resource(), which looks for an upstream bridge window that contains the resource (in pci_find_parent_resource()). So the use of PCI_BRIDGE_RESOURCES here is just a convenient way to find an empty slot in dev->resource[]. We know it's empty because the ali7101 device is not a bridge, so it has no windows. And PCI_BRIDGE_RESOURCES+1 *looks* like a reference to a bridge's memory window, but it's not; it's just a way to find another empty dev->resource[] slot. Sigh. This is ugly and misleading. But in the absence of good ideas, I guess we should just leave it alone. > } > DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_AL, PCI_DEVICE_ID_AL_M7101, quirk_ali7101_acpi); > > -- > 2.26.2 >
diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c index ca9ed5774eb1..c71fdd8bd6f8 100644 --- a/drivers/pci/quirks.c +++ b/drivers/pci/quirks.c @@ -654,7 +654,7 @@ DECLARE_PCI_FIXUP_CLASS_HEADER(PCI_VENDOR_ID_SYNOPSYS, PCI_ANY_ID, static void quirk_ali7101_acpi(struct pci_dev *dev) { quirk_io_region(dev, 0xE0, 64, PCI_BRIDGE_RESOURCES, "ali7101 ACPI"); - quirk_io_region(dev, 0xE2, 32, PCI_BRIDGE_RESOURCES+1, "ali7101 SMB"); + quirk_io_region(dev, 0xE2, 32, PCI_BRIDGE_RESOURCES, "ali7101 SMB"); } DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_AL, PCI_DEVICE_ID_AL_M7101, quirk_ali7101_acpi);
As per the specifications from the vendor: - "M1543 Preliminary Data Sheet", "M1543: Desktop South Bridge", Acer Laboratories Inc., January 1998, Version 1.25, p. 78 - "M1543C Preliminary Datasheet", "M1543C Desktop Southbridge", Acer Laboratories Inc., September 1998, Version 1.10, p. 126 Both the ACPI I/O and SMB I/O registers should be mapped into I/O space, but the second quirk used to claimed an I/O resource from the memory window which is not correct. Signed-off-by: Krzysztof Wilczynski <kw@linux.com> --- drivers/pci/quirks.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)