2004-03-19 00:26:40

by Matthew Wilcox

[permalink] [raw]
Subject: [0/3] Make pci resources show up in iomem/ioport on ia64


I decided to do one simple little thing -- make PCI device resources show
up in /proc/iomem and /proc/ioport on ia64 just like they do on i386.
Of course, I found two bugs in the process so we have three logically
separate patches that I shall be sending in reply to this message.

--
"Next the statesmen will invent cheap lies, putting the blame upon
the nation that is attacked, and every man will be glad of those
conscience-soothing falsities, and will diligently study them, and refuse
to examine any refutations of them; and thus he will by and by convince
himself that the war is just, and will thank God for the better sleep
he enjoys after this process of grotesque self-deception." -- Mark Twain


2004-03-18 23:56:12

by Matthew Wilcox

[permalink] [raw]
Subject: [2/3] Use insert_resource in pci_claim_resource


On ia64, the parent resources are not necessarily PCI resources and
so won't get found by pci_find_parent_resource. Use the shiny new
insert_resource() function instead, which I think we would have used
here had it been available at the time.

Index: drivers/pci/setup-res.c
===================================================================
RCS file: /var/cvs/linux-2.6/drivers/pci/setup-res.c,v
retrieving revision 1.7
diff -u -p -r1.7 setup-res.c
--- a/drivers/pci/setup-res.c 10 Mar 2004 02:27:48 -0000 1.7
+++ b/drivers/pci/setup-res.c 18 Mar 2004 23:40:56 -0000
@@ -94,13 +94,18 @@ int __init
pci_claim_resource(struct pci_dev *dev, int resource)
{
struct resource *res = &dev->resource[resource];
- struct resource *root = pci_find_parent_resource(dev, res);
+ struct resource *root = NULL;
char *dtype = resource < PCI_BRIDGE_RESOURCES ? "device" : "bridge";
int err;

+ if (res->flags & IORESOURCE_IO)
+ root = &ioport_resource;
+ if (res->flags & IORESOURCE_MEM)
+ root = &iomem_resource;
+
err = -EINVAL;
if (root != NULL)
- err = request_resource(root, res);
+ err = insert_resource(root, res);

if (err) {
printk(KERN_ERR "PCI: %s region %d of %s %s [%lx:%lx]\n",

--
"Next the statesmen will invent cheap lies, putting the blame upon
the nation that is attacked, and every man will be glad of those
conscience-soothing falsities, and will diligently study them, and refuse
to examine any refutations of them; and thus he will by and by convince
himself that the war is just, and will thank God for the better sleep
he enjoys after this process of grotesque self-deception." -- Mark Twain

2004-03-18 23:56:12

by Matthew Wilcox

[permalink] [raw]
Subject: [3/3] claim PCI resources on ia64


Call pci_claim_resources() so we can see what PCI resources are being used.

Index: arch/ia64/pci/pci.c
===================================================================
RCS file: /var/cvs/linux-2.6/arch/ia64/pci/pci.c,v
retrieving revision 1.7
diff -u -p -r1.7 pci.c
--- a/arch/ia64/pci/pci.c 16 Mar 2004 15:39:51 -0000 1.7
+++ b/arch/ia64/pci/pci.c 18 Mar 2004 23:40:52 -0000
@@ -367,6 +366,7 @@ pcibios_fixup_device_resources (struct p
dev->resource[i].end += window->offset;
}
}
+ pci_claim_resource(dev, i);
}
}


--
"Next the statesmen will invent cheap lies, putting the blame upon
the nation that is attacked, and every man will be glad of those
conscience-soothing falsities, and will diligently study them, and refuse
to examine any refutations of them; and thus he will by and by convince
himself that the war is just, and will thank God for the better sleep
he enjoys after this process of grotesque self-deception." -- Mark Twain

2004-03-19 00:18:00

by Matthew Wilcox

[permalink] [raw]
Subject: [1/3] insert_resource can succeed and return an error


If we start again, we can return an error even if we were successful.
Reset the result to 0 before beginning again. Why don't we use a
tailcall here?

Index: kernel/resource.c
===================================================================
RCS file: /var/cvs/linux-2.6/kernel/resource.c,v
retrieving revision 1.12
diff -u -p -r1.12 resource.c
--- a/kernel/resource.c 28 Feb 2004 01:51:35 -0000 1.12
+++ b/kernel/resource.c 18 Mar 2004 23:41:01 -0000
@@ -337,6 +337,7 @@ int insert_resource(struct resource *par
/* existing resource overlaps end of new resource */
if (next->end > new->end) {
parent = next;
+ result = 0;
goto begin;
}


--
"Next the statesmen will invent cheap lies, putting the blame upon
the nation that is attacked, and every man will be glad of those
conscience-soothing falsities, and will diligently study them, and refuse
to examine any refutations of them; and thus he will by and by convince
himself that the war is just, and will thank God for the better sleep
he enjoys after this process of grotesque self-deception." -- Mark Twain

2004-03-19 01:48:20

by David Mosberger

[permalink] [raw]
Subject: Re: [3/3] claim PCI resources on ia64

Matthew tells me that applying this patch without the others will result
in error messages (harmless, but still...). I'm OK with the patch, so
I'd appreciate it if whoever takes the other patches could apply this
one, too.

Thanks,

-----------------------------------------------------------------------
From: Matthew Wilcox <[email protected]>
Sender: <willy@http://www.linux.org.uk>
To: Matthew Wilcox <[email protected]>
Cc: Linus Torvalds <[email protected]>, Andrew Morton <[email protected]>,
Greg KH <[email protected]>, David Mosberger <[email protected]>,
[email protected], [email protected]
Subject: [3/3] claim PCI resources on ia64
Date: Thu, 18 Mar 2004 23:52:50 +0000


Call pci_claim_resources() so we can see what PCI resources are being used.

Index: arch/ia64/pci/pci.c
===================================================================
RCS file: /var/cvs/linux-2.6/arch/ia64/pci/pci.c,v
retrieving revision 1.7
diff -u -p -r1.7 pci.c
--- a/arch/ia64/pci/pci.c 16 Mar 2004 15:39:51 -0000 1.7
+++ b/arch/ia64/pci/pci.c 18 Mar 2004 23:40:52 -0000
@@ -367,6 +366,7 @@ pcibios_fixup_device_resources (struct p
dev->resource[i].end += window->offset;
}
}
+ pci_claim_resource(dev, i);
}
}


--

2004-03-19 09:56:14

by Russell King

[permalink] [raw]
Subject: Re: [2/3] Use insert_resource in pci_claim_resource

On Thu, Mar 18, 2004 at 11:52:17PM +0000, Matthew Wilcox wrote:
> On ia64, the parent resources are not necessarily PCI resources and
> so won't get found by pci_find_parent_resource. Use the shiny new
> insert_resource() function instead, which I think we would have used
> here had it been available at the time.

I think we want to preserve the existing behaviour rather than change
it. We really do want to request the device resource against its
immediate parent because that is the way PCI works - if a devices
resources don't fall within the parent bus resources, we want to
know about it.

May I suggest that ia64 sets the parent bus resources appropriately,
which should relieve this problem (iow, pci_root_bus->resource[0..3])?
If pci_find_parent_resource() is returning the wrong thing, its likely
that other users of this function will also be getting the wrong answer.

--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of: 2.6 PCMCIA - http://pcmcia.arm.linux.org.uk/
2.6 Serial core

2004-03-19 14:52:22

by Matthew Wilcox

[permalink] [raw]
Subject: Re: [2/3] Use insert_resource in pci_claim_resource

On Fri, Mar 19, 2004 at 09:56:00AM +0000, Russell King wrote:
> I think we want to preserve the existing behaviour rather than change
> it. We really do want to request the device resource against its
> immediate parent because that is the way PCI works - if a devices
> resources don't fall within the parent bus resources, we want to
> know about it.
>
> May I suggest that ia64 sets the parent bus resources appropriately,
> which should relieve this problem (iow, pci_root_bus->resource[0..3])?
> If pci_find_parent_resource() is returning the wrong thing, its likely
> that other users of this function will also be getting the wrong answer.

I see what's going on ... pci_read_bridge_bases() returns immediately
because this is the root bus. So we need to point the root bus resources
elsewhere ... this looks doable.

But I do think insert_resource is the right call to make. If the device has
the wrong resources, that means something's gone awfully wrong earlier in
the pci code.

--
"Next the statesmen will invent cheap lies, putting the blame upon
the nation that is attacked, and every man will be glad of those
conscience-soothing falsities, and will diligently study them, and refuse
to examine any refutations of them; and thus he will by and by convince
himself that the war is just, and will thank God for the better sleep
he enjoys after this process of grotesque self-deception." -- Mark Twain

2004-03-19 15:36:53

by Russell King

[permalink] [raw]
Subject: Re: [2/3] Use insert_resource in pci_claim_resource

On Fri, Mar 19, 2004 at 02:52:12PM +0000, Matthew Wilcox wrote:
> But I do think insert_resource is the right call to make. If the device has
> the wrong resources, that means something's gone awfully wrong earlier in
> the pci code.

Sure, but due to the request_resource semantics, it provides a good way
to catch this should it occur.

--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of: 2.6 PCMCIA - http://pcmcia.arm.linux.org.uk/
2.6 Serial core

2004-03-19 17:09:20

by Matthew Wilcox

[permalink] [raw]
Subject: Re: [2/3] Use insert_resource in pci_claim_resource

On Fri, Mar 19, 2004 at 03:36:39PM +0000, Russell King wrote:
> On Fri, Mar 19, 2004 at 02:52:12PM +0000, Matthew Wilcox wrote:
> > But I do think insert_resource is the right call to make. If the device has
> > the wrong resources, that means something's gone awfully wrong earlier in
> > the pci code.
>
> Sure, but due to the request_resource semantics, it provides a good way
> to catch this should it occur.

Now that I try it, we lose. ACPI allows us an unlimited number of resources
that can be routed to each bus, and pci_bus only has space for 4. On my
rx2600 box, we have 5 resources pointing to bus 0x20:

# cat /proc/iomem /proc/ioports |grep :20
90000000-97ffffff : PCI Bus 0000:20
ff5e0000-ff5e0007 : PCI Bus 0000:20
ff5e2000-ff5e2007 : PCI Bus 0000:20
90004000000-90103fffffe : PCI Bus 0000:20
00002000-00002fff : PCI Bus 0000:20

Sure, you can argue that some of these should be coalesced and others
aren't used, but I'm not comfortable diddling with that in the kernel,
or asking firmware to change that. And nobody knows what tomorrow will
bring in terms of PCI bus topologies. So I'd like the insert_resource
patch to go in.

--
"Next the statesmen will invent cheap lies, putting the blame upon
the nation that is attacked, and every man will be glad of those
conscience-soothing falsities, and will diligently study them, and refuse
to examine any refutations of them; and thus he will by and by convince
himself that the war is just, and will thank God for the better sleep
he enjoys after this process of grotesque self-deception." -- Mark Twain

2004-03-19 22:17:40

by Greg KH

[permalink] [raw]
Subject: Re: [0/3] Make pci resources show up in iomem/ioport on ia64

On Thu, Mar 18, 2004 at 11:50:24PM +0000, Matthew Wilcox wrote:
>
> I decided to do one simple little thing -- make PCI device resources show
> up in /proc/iomem and /proc/ioport on ia64 just like they do on i386.
> Of course, I found two bugs in the process so we have three logically
> separate patches that I shall be sending in reply to this message.

I've applied all 3 of these patches, thanks.

greg k-h