PNP: don't check for conflicts with bridge windows
Subsystems (2)
PATCH 1/1 PNP: don't check for conflicts with bridge windows
With fa35b4926 (adding PNPACPI support for bridge windows), I think I broke most ISAPNP resource assignment. That was a post-2.6.33 change, so this is regression, and I think this patch needs to go in 2.6.34.
---
Bjorn Helgaas (1):
PNP: don't check for conflicts with bridge windows
drivers/pnp/resource.c | 4 ++++ 1 files changed, 4 insertions(+), 0 deletions(-)
PATCH 1/1 PNP: don't check for conflicts with bridge windows
With fa35b4926, I broke a lot of PNP resource assignment. That commit made PNPACPI include bridge windows as PNP resources, and PNP resource assignment treats any enabled overlapping PNP resources as conflicts. Since PCI host bridge windows typically include most of the I/O port space, this makes PNP port assigments fail.
The PCI host bridge driver will eventually use those PNP window resources, so we should make PNP ignore them when checking for conflicts.
This fixes https://bugzilla.kernel.org/show_bug.cgi?id=15903
---
drivers/pnp/resource.c | 4 ++++ 1 files changed, 4 insertions(+), 0 deletions(-)
drivers/pnp/resource.c +4 -0
@@ -211,6 +211,8 @@ int pnp_check_port(struct pnp_dev *dev, struct resource *res)if (tres->flags &IORESOURCE_IO ) {if (cannot_compare (tres->flags))continue ; +if (tres->flags &IORESOURCE_WINDOW ) +continue ; tport = &tres->start; tend = &tres->end;if (ranged_conflict (port, end, tport, tend))
@@ -271,6 +273,8 @@ int pnp_check_mem(struct pnp_dev *dev, struct resource *res)if (tres->flags &IORESOURCE_MEM ) {if (cannot_compare (tres->flags))continue ; +if (tres->flags &IORESOURCE_WINDOW ) +continue ; taddr = &tres->start; tend = &tres->end;if (ranged_conflict (addr, end, taddr, tend))
Quoted 9 lines · Bjorn Helgaas
drivers/pnp/resource.c +2
-0if (tres->flags &IORESOURCE_IO ) {if (cannot_compare (tres->flags))continue ; +if (tres->flags &IORESOURCE_WINDOW ) +continue ; tport = &tres->start; tend = &tres->end;if (ranged_conflict (port, end, tport, tend))
drivers/pnp/resource.c int pnp_check_mem(struct pnp_dev *dev, struct resource *res) · line 277
Quoted 5 lines
drivers/pnp/resource.c +2
-0@@ -271,6 +273,8 @@ int pnp_check_mem(struct pnp_dev *dev, struct resource *res)if (tres->flags &IORESOURCE_MEM ) {if (cannot_compare (tres->flags))continue ; +if (tres->flags &IORESOURCE_WINDOW ) +continue ;
Hmm. Looking at the patch, I am wondering if it wouldn't make _more_ sense to instead say that you never mix IORESOURCE_WINDOW with IORESOURCE_IO/MEM? That would make the above patch unnecessary, since it would never trigger the test for IORESOURCE_IO/MEM in the first place.
A resource window is a window - it's not the resource itself. It's not IO or MEM - that could/should be an attribute of what resource _tree_ the window is linked into, not the resource itself.
Hmm?
Linus
Quoted 22 lines · Linus Torvalds
Quoted 9 lines · Bjorn Helgaas
drivers/pnp/resource.c +2
-0if (tres->flags &IORESOURCE_IO ) {if (cannot_compare (tres->flags))continue ; +if (tres->flags &IORESOURCE_WINDOW ) +continue ; tport = &tres->start; tend = &tres->end;if (ranged_conflict (port, end, tport, tend))drivers/pnp/resource.c int pnp_check_mem(struct pnp_dev *dev, struct resource *res) · line 277
Quoted 5 lines
drivers/pnp/resource.c +2
-0@@ -271,6 +273,8 @@ int pnp_check_mem(struct pnp_dev *dev, struct resource *res)if (tres->flags &IORESOURCE_MEM ) {if (cannot_compare (tres->flags))continue ; +if (tres->flags &IORESOURCE_WINDOW ) +continue ;Hmm. Looking at the patch, I am wondering if it wouldn't make _more_ sense to instead say that you never mix
IORESOURCE_WINDOWwith IORESOURCE_IO/MEM? That would make the above patch unnecessary, since it would never trigger the test for IORESOURCE_IO/MEM in the first place.
Quoted 3 lines
A resource window is a window - it's not the resource itself. It's not IO or MEM - that could/should be an attribute of what resource _tree_ the window is linked into, not the resource itself.
That's an interesting idea, and it might make more sense. I haven't done anything useful with IORESOURCE_WINDOW yet, so I don't know how it will work out.
My *plan* was to make pci_root.c a PNP driver, so it would use the pre-parsed PNP resources to find the host bridge windows rather than re-parsing the ACPI _CRS info by hand. Those PNP resources are not in the resource tree when the driver first sees them (which might be a deficiency of our current PNP core), so I don't think there's currently a way for the driver to distinguish IO from MEM unless it's in the resource itself.
At this stage of the release, my preference would be to use the patch above (with the intention of coming back later for some more work), or to just revert fa35b4926 (PNPACPI window support) and maybe 9d7cca04 (generic window support). If we revert those patches, I'll have to figure out another way to clean up pci_root.c.
Bjorn
Quoted 9 lines · Bjorn Helgaas
My *plan* was to make
pci_root.ca PNP driver, so it would use the pre-parsed PNP resources to find the host bridge windows rather than re-parsing the ACPI_CRSinfo by hand. Those PNP resources are not in the resource tree when the driver first sees them (which might be a deficiency of our current PNP core), so I don't think there's currently a way for the driver to distinguish IO from MEM unless it's in the resource itself.
Quoted 5 lines
At this stage of the release, my preference would be to use the patch above (with the intention of coming back later for some more work), or to just revert
fa35b4926(PNPACPI window support) and maybe9d7cca04(generic window support). If we revert those patches, I'll have to figure out another way to clean uppci_root.c.
Oh, I'm fine with your patch, I just reacted to it wondering whether maybe there was some cleaner way of solving the problem. I guess annotating the resource itself is ok.
So don't take my comment as a NAK, more of a "hmm, how about.. "
Linus
Len Brown
reviewerapplied for 2.6.34
thanks,
Len Brown, Intel Open Source Technology Center