| Message ID | 1295001146-14910-2-git-send-email-jslaby@suse.cz |
|---|---|
| State | Not Applicable |
| Delegated to: | David Miller |
| Headers | show |
Hello. On 14-01-2011 13:32, Jiri Slaby wrote: > Some broken BIOSes on ICH4 chipset report an ACPI region which is in > conflict with legacy IDE ports when ACPI is disabled. Even though the > regions overlap, IDE ports are working correctly (we cannot find out > the decoding rules on chipsets). > > So the only problem is the reported region itself, if we don't reserve > the region in the quirk everything works as expected. > This patch avoids reserving any quirk regions below PCIBIOS_MIN_IO > which is 0x1000. Some regions might be (and are by a fast google > query) below this border, but the only difference is that they won't > be reserved anymore. They should still work though the same as before. > The conflicts look like (1f.0 is bridge, 1f.1 is IDE ctrl): > pci 0000:00:1f.1: address space collision: [io 0x0170-0x0177] conflicts with 0000:00:1f.0 [io 0x0100-0x017f] > At 0x0100 a 128 bytes long ACPI region is reported in the quirk for > ICH4. ata_piix then fails to find disks because the IDE legacy ports > are zeroed: > ata_piix 0000:00:1f.1: device not available (can't reserve [io 0x0000-0x0007]) > References: https://bugzilla.novell.com/show_bug.cgi?id=558740 > Signed-off-by: Jiri Slaby <jslaby@suse.cz> > Cc: Bjorn Helgaas <bjorn.helgaas@hp.com> > Cc: Jesse Barnes <jbarnes@virtuousgeek.org> > Cc: "David S. Miller" <davem@davemloft.net> > Cc: Thomas Renninger <trenn@suse.de> > Cc: Linus Torvalds <torvalds@linux-foundation.org> [...] > diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c > index 8db2426..b3ab2f7 100644 > --- a/drivers/pci/quirks.c > +++ b/drivers/pci/quirks.c > @@ -554,18 +554,30 @@ static void __devinit quirk_ich4_lpc_acpi(struct pci_dev *dev) > u32 region; > u8 enable; > > + /* > + * The check for PCIBIOS_MIN_IO is to ensure we won't create a conflict > + * with low legacy (and fixed) ports. We don't know the decoding > + * priority and can't tell whether the legacy device or the one created > + * here is really at that address. This happens on boards with broken > + * BIOSes. > + */ > + > pci_read_config_byte(dev, ICH_ACPI_CNTL,&enable); > if (enable& ICH4_ACPI_EN) { > pci_read_config_dword(dev, ICH_PMBASE,®ion); > - quirk_io_region(dev, region, 128, PCI_BRIDGE_RESOURCES, > - "ICH4 ACPI/GPIO/TCO"); > + region &= PCI_BASE_ADDRESS_IO_MASK; Why don't you do the masking right in the patch #1? And is it really necessary if the region size is known to be 128 bytes? WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 02/11/2011 01:09 PM, Sergei Shtylyov wrote: >> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c >> index 8db2426..b3ab2f7 100644 >> --- a/drivers/pci/quirks.c >> +++ b/drivers/pci/quirks.c >> @@ -554,18 +554,30 @@ static void __devinit quirk_ich4_lpc_acpi(struct >> pci_dev *dev) >> u32 region; >> u8 enable; >> >> + /* >> + * The check for PCIBIOS_MIN_IO is to ensure we won't create a >> conflict >> + * with low legacy (and fixed) ports. We don't know the decoding >> + * priority and can't tell whether the legacy device or the one >> created >> + * here is really at that address. This happens on boards with >> broken >> + * BIOSes. >> + */ >> + >> pci_read_config_byte(dev, ICH_ACPI_CNTL,&enable); >> if (enable& ICH4_ACPI_EN) { >> pci_read_config_dword(dev, ICH_PMBASE,®ion); >> - quirk_io_region(dev, region, 128, PCI_BRIDGE_RESOURCES, >> - "ICH4 ACPI/GPIO/TCO"); >> + region &= PCI_BASE_ADDRESS_IO_MASK; > > Why don't you do the masking right in the patch #1? And is it really > necessary if the region size is known to be 128 bytes? The region here contains also the low flag bits. Yes, it can be a part of 1/1. But I don't think it matters. regards,
diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c index 8db2426..b3ab2f7 100644 --- a/drivers/pci/quirks.c +++ b/drivers/pci/quirks.c @@ -554,18 +554,30 @@ static void __devinit quirk_ich4_lpc_acpi(struct pci_dev *dev) u32 region; u8 enable; + /* + * The check for PCIBIOS_MIN_IO is to ensure we won't create a conflict + * with low legacy (and fixed) ports. We don't know the decoding + * priority and can't tell whether the legacy device or the one created + * here is really at that address. This happens on boards with broken + * BIOSes. + */ + pci_read_config_byte(dev, ICH_ACPI_CNTL, &enable); if (enable & ICH4_ACPI_EN) { pci_read_config_dword(dev, ICH_PMBASE, ®ion); - quirk_io_region(dev, region, 128, PCI_BRIDGE_RESOURCES, - "ICH4 ACPI/GPIO/TCO"); + region &= PCI_BASE_ADDRESS_IO_MASK; + if (region >= PCIBIOS_MIN_IO) + quirk_io_region(dev, region, 128, PCI_BRIDGE_RESOURCES, + "ICH4 ACPI/GPIO/TCO"); } pci_read_config_byte(dev, ICH4_GPIO_CNTL, &enable); if (enable & ICH4_GPIO_EN) { pci_read_config_dword(dev, ICH4_GPIOBASE, ®ion); - quirk_io_region(dev, region, 64, PCI_BRIDGE_RESOURCES + 1, - "ICH4 GPIO"); + region &= PCI_BASE_ADDRESS_IO_MASK; + if (region >= PCIBIOS_MIN_IO) + quirk_io_region(dev, region, 64, + PCI_BRIDGE_RESOURCES + 1, "ICH4 GPIO"); } } DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_82801AA_0, quirk_ich4_lpc_acpi); @@ -587,15 +599,19 @@ static void __devinit ich6_lpc_acpi_gpio(struct pci_dev *dev) pci_read_config_byte(dev, ICH_ACPI_CNTL, &enable); if (enable & ICH6_ACPI_EN) { pci_read_config_dword(dev, ICH_PMBASE, ®ion); - quirk_io_region(dev, region, 128, PCI_BRIDGE_RESOURCES, - "ICH6 ACPI/GPIO/TCO"); + region &= PCI_BASE_ADDRESS_IO_MASK; + if (region >= PCIBIOS_MIN_IO) + quirk_io_region(dev, region, 128, PCI_BRIDGE_RESOURCES, + "ICH6 ACPI/GPIO/TCO"); } pci_read_config_byte(dev, ICH6_GPIO_CNTL, &enable); if (enable & ICH4_GPIO_EN) { pci_read_config_dword(dev, ICH6_GPIOBASE, ®ion); - quirk_io_region(dev, region, 64, PCI_BRIDGE_RESOURCES + 1, - "ICH6 GPIO"); + region &= PCI_BASE_ADDRESS_IO_MASK; + if (region >= PCIBIOS_MIN_IO) + quirk_io_region(dev, region, 64, + PCI_BRIDGE_RESOURCES + 1, "ICH6 GPIO"); } }
Some broken BIOSes on ICH4 chipset report an ACPI region which is in conflict with legacy IDE ports when ACPI is disabled. Even though the regions overlap, IDE ports are working correctly (we cannot find out the decoding rules on chipsets). So the only problem is the reported region itself, if we don't reserve the region in the quirk everything works as expected. This patch avoids reserving any quirk regions below PCIBIOS_MIN_IO which is 0x1000. Some regions might be (and are by a fast google query) below this border, but the only difference is that they won't be reserved anymore. They should still work though the same as before. The conflicts look like (1f.0 is bridge, 1f.1 is IDE ctrl): pci 0000:00:1f.1: address space collision: [io 0x0170-0x0177] conflicts with 0000:00:1f.0 [io 0x0100-0x017f] At 0x0100 a 128 bytes long ACPI region is reported in the quirk for ICH4. ata_piix then fails to find disks because the IDE legacy ports are zeroed: ata_piix 0000:00:1f.1: device not available (can't reserve [io 0x0000-0x0007]) References: https://bugzilla.novell.com/show_bug.cgi?id=558740 Signed-off-by: Jiri Slaby <jslaby@suse.cz> Cc: Bjorn Helgaas <bjorn.helgaas@hp.com> Cc: Jesse Barnes <jbarnes@virtuousgeek.org> Cc: "David S. Miller" <davem@davemloft.net> Cc: Thomas Renninger <trenn@suse.de> Cc: Linus Torvalds <torvalds@linux-foundation.org> --- drivers/pci/quirks.c | 32 ++++++++++++++++++++++++-------- 1 files changed, 24 insertions(+), 8 deletions(-)