2009-06-17 20:42:55

by Matthew Wilcox

[permalink] [raw]
Subject:

Hi Linus,

This series of four patches are a result of the resource problem Andrew
Patterson ran into on the HP rx6600. First, I fix pci_claim_resource(),
then I get rid of the now-unused pcibios_select_root(). The third patch
changes x86 to use pci_claim_resource (since it's now essentially equivalent).
The fourth patch actually fixes Andrew's problem by changing where ia64
sets up the resource pointers in the root pci_bus.

If you don't want to take all this, only patches 1 and 4 are bug-fixes.
Patch 2 might have a problem if someone chooses to start using
pcibios_select_root (I can't imagine they will ...), and patch 3 is the
most risky.

Diffstat:

arch/alpha/include/asm/pci.h | 13 -------------
arch/arm/include/asm/pci.h | 13 -------------
arch/ia64/include/asm/pci.h | 13 -------------
arch/ia64/pci/pci.c | 4 ++--
arch/mips/include/asm/pci.h | 13 -------------
arch/mn10300/include/asm/pci.h | 13 -------------
arch/parisc/include/asm/pci.h | 13 -------------
arch/powerpc/include/asm/pci.h | 13 -------------
arch/sh/include/asm/pci.h | 13 -------------
arch/sparc/include/asm/pci_64.h | 2 --
arch/sparc/kernel/pci.c | 13 -------------
arch/x86/pci/i386.c | 17 +++++++----------
drivers/pci/setup-res.c | 4 ++--
include/asm-generic/pci.h | 13 -------------
14 files changed, 11 insertions(+), 146 deletions(-)


2009-06-17 20:42:13

by Matthew Wilcox

[permalink] [raw]
Subject: [PATCH 1/4] Fix pci_claim_resource

Instead of starting from the iomem or ioport roots, start from the
parent bus' resources. This fixes a bug where child resources would
appear above their parents resources if they had the same size.

Signed-off-by: Matthew Wilcox <[email protected]>
Tested-by: Andrew Patterson <[email protected]>
---
drivers/pci/setup-res.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/setup-res.c b/drivers/pci/setup-res.c
index 3039fcb..1240351 100644
--- a/drivers/pci/setup-res.c
+++ b/drivers/pci/setup-res.c
@@ -99,11 +99,11 @@ void pci_update_resource(struct pci_dev *dev, int resno)
int pci_claim_resource(struct pci_dev *dev, int resource)
{
struct resource *res = &dev->resource[resource];
- struct resource *root = NULL;
+ struct resource *root;
char *dtype = resource < PCI_BRIDGE_RESOURCES ? "device" : "bridge";
int err;

- root = pcibios_select_root(dev, res);
+ root = pci_find_parent_resource(dev, res);

err = -EINVAL;
if (root != NULL)
--
1.6.3.1

2009-06-17 20:42:28

by Matthew Wilcox

[permalink] [raw]
Subject: [PATCH 4/4] ia64: Fix resource assignment for root busses

ia64 was assigning resources to root busses after allocations had
been made for child busses. Calling pcibios_setup_root_windows() from
pcibios_fixup_bus() solves this problem by assigning the resources to
the root bus before child busses are scanned.

Signed-off-by: Matthew Wilcox <[email protected]>
Tested-by: Andrew Patterson <[email protected]>
---
arch/ia64/pci/pci.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/ia64/pci/pci.c b/arch/ia64/pci/pci.c
index 61f1af5..ae5ee8a 100644
--- a/arch/ia64/pci/pci.c
+++ b/arch/ia64/pci/pci.c
@@ -371,8 +371,6 @@ pci_acpi_scan_root(struct acpi_device *device, int domain, int bus)
* such quirk. So we just ignore the case now.
*/
pbus = pci_scan_bus_parented(NULL, bus, &pci_root_ops, controller);
- if (pbus)
- pcibios_setup_root_windows(pbus, controller);

return pbus;

@@ -490,6 +488,8 @@ pcibios_fixup_bus (struct pci_bus *b)
if (b->self) {
pci_read_bridge_bases(b);
pcibios_fixup_bridge_resources(b->self);
+ } else {
+ pcibios_setup_root_windows(b, b->sysdata);
}
list_for_each_entry(dev, &b->devices, bus_list)
pcibios_fixup_device_resources(dev);
--
1.6.3.1

2009-06-17 20:42:41

by Matthew Wilcox

[permalink] [raw]
Subject: [PATCH 3/4] x86: Use pci_claim_resource

Instead of open-coding pci_find_parent_resource and request_resource,
just call pci_claim_resource.

Signed-off-by: Matthew Wilcox <[email protected]>
---
arch/x86/pci/i386.c | 17 +++++++----------
1 files changed, 7 insertions(+), 10 deletions(-)

diff --git a/arch/x86/pci/i386.c b/arch/x86/pci/i386.c
index a85bef2..0fb56db 100644
--- a/arch/x86/pci/i386.c
+++ b/arch/x86/pci/i386.c
@@ -116,7 +116,7 @@ static void __init pcibios_allocate_bus_resources(struct list_head *bus_list)
struct pci_bus *bus;
struct pci_dev *dev;
int idx;
- struct resource *r, *pr;
+ struct resource *r;

/* Depth-First Search on bus tree */
list_for_each_entry(bus, bus_list, node) {
@@ -126,9 +126,8 @@ static void __init pcibios_allocate_bus_resources(struct list_head *bus_list)
r = &dev->resource[idx];
if (!r->flags)
continue;
- pr = pci_find_parent_resource(dev, r);
- if (!r->start || !pr ||
- request_resource(pr, r) < 0) {
+ if (!r->start ||
+ pci_claim_resource(dev, idx) < 0) {
dev_info(&dev->dev, "BAR %d: can't allocate resource\n", idx);
/*
* Something is wrong with the region.
@@ -149,7 +148,7 @@ static void __init pcibios_allocate_resources(int pass)
struct pci_dev *dev = NULL;
int idx, disabled;
u16 command;
- struct resource *r, *pr;
+ struct resource *r;

for_each_pci_dev(dev) {
pci_read_config_word(dev, PCI_COMMAND, &command);
@@ -168,8 +167,7 @@ static void __init pcibios_allocate_resources(int pass)
(unsigned long long) r->start,
(unsigned long long) r->end,
r->flags, disabled, pass);
- pr = pci_find_parent_resource(dev, r);
- if (!pr || request_resource(pr, r) < 0) {
+ if (pci_claim_resource(dev, idx) < 0) {
dev_info(&dev->dev, "BAR %d: can't allocate resource\n", idx);
/* We'll assign a new address later */
r->end -= r->start;
@@ -197,7 +195,7 @@ static void __init pcibios_allocate_resources(int pass)
static int __init pcibios_assign_resources(void)
{
struct pci_dev *dev = NULL;
- struct resource *r, *pr;
+ struct resource *r;

if (!(pci_probe & PCI_ASSIGN_ROMS)) {
/*
@@ -209,8 +207,7 @@ static int __init pcibios_assign_resources(void)
r = &dev->resource[PCI_ROM_RESOURCE];
if (!r->flags || !r->start)
continue;
- pr = pci_find_parent_resource(dev, r);
- if (!pr || request_resource(pr, r) < 0) {
+ if (pci_claim_resource(dev, PCI_ROM_RESOURCE) < 0) {
r->end -= r->start;
r->start = 0;
}
--
1.6.3.1

2009-06-17 20:43:19

by Matthew Wilcox

[permalink] [raw]
Subject: [PATCH 2/4] Delete pcibios_select_root

This function was only used by pci_claim_resource(), and the last commit
deleted that use.

Signed-off-by: Matthew Wilcox <[email protected]>
---
arch/alpha/include/asm/pci.h | 13 -------------
arch/arm/include/asm/pci.h | 13 -------------
arch/ia64/include/asm/pci.h | 13 -------------
arch/mips/include/asm/pci.h | 13 -------------
arch/mn10300/include/asm/pci.h | 13 -------------
arch/parisc/include/asm/pci.h | 13 -------------
arch/powerpc/include/asm/pci.h | 13 -------------
arch/sh/include/asm/pci.h | 13 -------------
arch/sparc/include/asm/pci_64.h | 2 --
arch/sparc/kernel/pci.c | 13 -------------
include/asm-generic/pci.h | 13 -------------
11 files changed, 0 insertions(+), 132 deletions(-)

diff --git a/arch/alpha/include/asm/pci.h b/arch/alpha/include/asm/pci.h
index cb04eaa..d22ace9 100644
--- a/arch/alpha/include/asm/pci.h
+++ b/arch/alpha/include/asm/pci.h
@@ -237,19 +237,6 @@ extern void pcibios_resource_to_bus(struct pci_dev *, struct pci_bus_region *,
extern void pcibios_bus_to_resource(struct pci_dev *dev, struct resource *res,
struct pci_bus_region *region);

-static inline struct resource *
-pcibios_select_root(struct pci_dev *pdev, struct resource *res)
-{
- struct resource *root = NULL;
-
- if (res->flags & IORESOURCE_IO)
- root = &ioport_resource;
- if (res->flags & IORESOURCE_MEM)
- root = &iomem_resource;
-
- return root;
-}
-
#define pci_domain_nr(bus) ((struct pci_controller *)(bus)->sysdata)->index

static inline int pci_proc_domain(struct pci_bus *bus)
diff --git a/arch/arm/include/asm/pci.h b/arch/arm/include/asm/pci.h
index 918d0cb..0abf386 100644
--- a/arch/arm/include/asm/pci.h
+++ b/arch/arm/include/asm/pci.h
@@ -65,19 +65,6 @@ extern void
pcibios_bus_to_resource(struct pci_dev *dev, struct resource *res,
struct pci_bus_region *region);

-static inline struct resource *
-pcibios_select_root(struct pci_dev *pdev, struct resource *res)
-{
- struct resource *root = NULL;
-
- if (res->flags & IORESOURCE_IO)
- root = &ioport_resource;
- if (res->flags & IORESOURCE_MEM)
- root = &iomem_resource;
-
- return root;
-}
-
/*
* Dummy implementation; always return 0.
*/
diff --git a/arch/ia64/include/asm/pci.h b/arch/ia64/include/asm/pci.h
index 1d660d8..fcfca56 100644
--- a/arch/ia64/include/asm/pci.h
+++ b/arch/ia64/include/asm/pci.h
@@ -135,19 +135,6 @@ extern void pcibios_resource_to_bus(struct pci_dev *dev,
extern void pcibios_bus_to_resource(struct pci_dev *dev,
struct resource *res, struct pci_bus_region *region);

-static inline struct resource *
-pcibios_select_root(struct pci_dev *pdev, struct resource *res)
-{
- struct resource *root = NULL;
-
- if (res->flags & IORESOURCE_IO)
- root = &ioport_resource;
- if (res->flags & IORESOURCE_MEM)
- root = &iomem_resource;
-
- return root;
-}
-
#define pcibios_scan_all_fns(a, b) 0

#define HAVE_ARCH_PCI_GET_LEGACY_IDE_IRQ
diff --git a/arch/mips/include/asm/pci.h b/arch/mips/include/asm/pci.h
index 053e463..a68d111 100644
--- a/arch/mips/include/asm/pci.h
+++ b/arch/mips/include/asm/pci.h
@@ -142,19 +142,6 @@ extern void pcibios_resource_to_bus(struct pci_dev *dev,
extern void pcibios_bus_to_resource(struct pci_dev *dev, struct resource *res,
struct pci_bus_region *region);

-static inline struct resource *
-pcibios_select_root(struct pci_dev *pdev, struct resource *res)
-{
- struct resource *root = NULL;
-
- if (res->flags & IORESOURCE_IO)
- root = &ioport_resource;
- if (res->flags & IORESOURCE_MEM)
- root = &iomem_resource;
-
- return root;
-}
-
#define pci_domain_nr(bus) ((struct pci_controller *)(bus)->sysdata)->index

static inline int pci_proc_domain(struct pci_bus *bus)
diff --git a/arch/mn10300/include/asm/pci.h b/arch/mn10300/include/asm/pci.h
index 0517b45..e58b9a4 100644
--- a/arch/mn10300/include/asm/pci.h
+++ b/arch/mn10300/include/asm/pci.h
@@ -106,19 +106,6 @@ extern void pcibios_bus_to_resource(struct pci_dev *dev,
struct resource *res,
struct pci_bus_region *region);

-static inline struct resource *
-pcibios_select_root(struct pci_dev *pdev, struct resource *res)
-{
- struct resource *root = NULL;
-
- if (res->flags & IORESOURCE_IO)
- root = &ioport_resource;
- if (res->flags & IORESOURCE_MEM)
- root = &iomem_resource;
-
- return root;
-}
-
#define pcibios_scan_all_fns(a, b) 0

static inline int pci_get_legacy_ide_irq(struct pci_dev *dev, int channel)
diff --git a/arch/parisc/include/asm/pci.h b/arch/parisc/include/asm/pci.h
index 4ba868f..7d842d6 100644
--- a/arch/parisc/include/asm/pci.h
+++ b/arch/parisc/include/asm/pci.h
@@ -268,19 +268,6 @@ extern void
pcibios_bus_to_resource(struct pci_dev *dev, struct resource *res,
struct pci_bus_region *region);

-static inline struct resource *
-pcibios_select_root(struct pci_dev *pdev, struct resource *res)
-{
- struct resource *root = NULL;
-
- if (res->flags & IORESOURCE_IO)
- root = &ioport_resource;
- if (res->flags & IORESOURCE_MEM)
- root = &iomem_resource;
-
- return root;
-}
-
static inline void pcibios_penalize_isa_irq(int irq, int active)
{
/* We don't need to penalize isa irq's */
diff --git a/arch/powerpc/include/asm/pci.h b/arch/powerpc/include/asm/pci.h
index ba17d5d..d9483c5 100644
--- a/arch/powerpc/include/asm/pci.h
+++ b/arch/powerpc/include/asm/pci.h
@@ -195,19 +195,6 @@ extern void pcibios_bus_to_resource(struct pci_dev *dev,
struct resource *res,
struct pci_bus_region *region);

-static inline struct resource *pcibios_select_root(struct pci_dev *pdev,
- struct resource *res)
-{
- struct resource *root = NULL;
-
- if (res->flags & IORESOURCE_IO)
- root = &ioport_resource;
- if (res->flags & IORESOURCE_MEM)
- root = &iomem_resource;
-
- return root;
-}
-
extern void pcibios_claim_one_bus(struct pci_bus *b);

extern void pcibios_finish_adding_to_bus(struct pci_bus *bus);
diff --git a/arch/sh/include/asm/pci.h b/arch/sh/include/asm/pci.h
index ae0da6f..d3633f5 100644
--- a/arch/sh/include/asm/pci.h
+++ b/arch/sh/include/asm/pci.h
@@ -137,19 +137,6 @@ extern void pcibios_resource_to_bus(struct pci_dev *dev,
extern void pcibios_bus_to_resource(struct pci_dev *dev, struct resource *res,
struct pci_bus_region *region);

-static inline struct resource *
-pcibios_select_root(struct pci_dev *pdev, struct resource *res)
-{
- struct resource *root = NULL;
-
- if (res->flags & IORESOURCE_IO)
- root = &ioport_resource;
- if (res->flags & IORESOURCE_MEM)
- root = &iomem_resource;
-
- return root;
-}
-
/* Chances are this interrupt is wired PC-style ... */
static inline int pci_get_legacy_ide_irq(struct pci_dev *dev, int channel)
{
diff --git a/arch/sparc/include/asm/pci_64.h b/arch/sparc/include/asm/pci_64.h
index 4f79a54..7a1e356 100644
--- a/arch/sparc/include/asm/pci_64.h
+++ b/arch/sparc/include/asm/pci_64.h
@@ -191,8 +191,6 @@ extern void
pcibios_bus_to_resource(struct pci_dev *dev, struct resource *res,
struct pci_bus_region *region);

-extern struct resource *pcibios_select_root(struct pci_dev *, struct resource *);
-
static inline int pci_get_legacy_ide_irq(struct pci_dev *dev, int channel)
{
return PCI_IRQ_NONE;
diff --git a/arch/sparc/kernel/pci.c b/arch/sparc/kernel/pci.c
index 4638fba..57859ad 100644
--- a/arch/sparc/kernel/pci.c
+++ b/arch/sparc/kernel/pci.c
@@ -711,19 +711,6 @@ void __devinit pcibios_fixup_bus(struct pci_bus *pbus)
pbus->resource[1] = &pbm->mem_space;
}

-struct resource *pcibios_select_root(struct pci_dev *pdev, struct resource *r)
-{
- struct pci_pbm_info *pbm = pdev->bus->sysdata;
- struct resource *root = NULL;
-
- if (r->flags & IORESOURCE_IO)
- root = &pbm->io_space;
- if (r->flags & IORESOURCE_MEM)
- root = &pbm->mem_space;
-
- return root;
-}
-
void pcibios_update_irq(struct pci_dev *pdev, int irq)
{
}
diff --git a/include/asm-generic/pci.h b/include/asm-generic/pci.h
index 515c6e2..b4326b5 100644
--- a/include/asm-generic/pci.h
+++ b/include/asm-generic/pci.h
@@ -30,19 +30,6 @@ pcibios_bus_to_resource(struct pci_dev *dev, struct resource *res,
res->end = region->end;
}

-static inline struct resource *
-pcibios_select_root(struct pci_dev *pdev, struct resource *res)
-{
- struct resource *root = NULL;
-
- if (res->flags & IORESOURCE_IO)
- root = &ioport_resource;
- if (res->flags & IORESOURCE_MEM)
- root = &iomem_resource;
-
- return root;
-}
-
#define pcibios_scan_all_fns(a, b) 0

#ifndef HAVE_ARCH_PCI_GET_LEGACY_IDE_IRQ
--
1.6.3.1

2009-06-17 20:56:49

by Linus Torvalds

[permalink] [raw]
Subject: Re: [PATCH 4/4] ia64: Fix resource assignment for root busses



On Wed, 17 Jun 2009, Matthew Wilcox wrote:
>
> ia64 was assigning resources to root busses after allocations had
> been made for child busses. Calling pcibios_setup_root_windows() from
> pcibios_fixup_bus() solves this problem by assigning the resources to
> the root bus before child busses are scanned.
>
> Signed-off-by: Matthew Wilcox <[email protected]>
> Tested-by: Andrew Patterson <[email protected]>

So I assume this "Tested-by:" means that Andrew confirmed that this
actually fixes the nesting? I wasn't cc'd on that, so I'm just checking..

Linus

2009-06-17 20:59:17

by Matthew Wilcox

[permalink] [raw]
Subject: Re: [PATCH 4/4] ia64: Fix resource assignment for root busses

On Wed, Jun 17, 2009 at 01:56:39PM -0700, Linus Torvalds wrote:
> On Wed, 17 Jun 2009, Matthew Wilcox wrote:
> >
> > ia64 was assigning resources to root busses after allocations had
> > been made for child busses. Calling pcibios_setup_root_windows() from
> > pcibios_fixup_bus() solves this problem by assigning the resources to
> > the root bus before child busses are scanned.
> >
> > Signed-off-by: Matthew Wilcox <[email protected]>
> > Tested-by: Andrew Patterson <[email protected]>
>
> So I assume this "Tested-by:" means that Andrew confirmed that this
> actually fixes the nesting? I wasn't cc'd on that, so I'm just checking..

Yes, he says it fixes the nesting problem, and as a result hotplug now works.

--
Matthew Wilcox Intel Open Source Technology Centre
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours. We can't possibly take such
a retrograde step."

2009-06-17 21:02:58

by Linus Torvalds

[permalink] [raw]
Subject: Re: [PATCH 4/4] ia64: Fix resource assignment for root busses



On Wed, 17 Jun 2009, Matthew Wilcox wrote:

> On Wed, Jun 17, 2009 at 01:56:39PM -0700, Linus Torvalds wrote:
> >
> > So I assume this "Tested-by:" means that Andrew confirmed that this
> > actually fixes the nesting? I wasn't cc'd on that, so I'm just checking..
>
> Yes, he says it fixes the nesting problem, and as a result hotplug now works.

Ok, I'll take the whole series. Thanks for chasing this down,

Linus