lkml.org 
[lkml]   [2009]   [Aug]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [Regression] PCI resources allocation problem on HP nx6325
From
Date
On Sun, 2009-08-02 at 09:39 -0700, Linus Torvalds wrote:
>
> On Sun, 2 Aug 2009, Rafael J. Wysocki wrote:
> >
> > Hi Matthew,
> >
> > As reported at
> >
> > http://bugzilla.kernel.org/show_bug.cgi?id=13891
> >
> > there is a problem with allocating PCI resources on HP nx6325 introduced by
> > your commit a76117dfd687ec4be0a9a05214f3009cc5f73a42
> > (x86: Use pci_claim_resource).
>
> Ooh, interesting. I thought that patch was a functionally equivalent
> cleanup of
>
> pr = pci_find_parent_resource(dev, r);
> if (!pr || request_resource(pr, r) < 0) {
>
> to
>
> if (pci_claim_resource(dev, idx) < 0) {
>
> but yeah, it's not exactly the same. pci_claim_resource() uses
> 'insert_resource()' rather than 'request_resource()'.
>
> We could certainly revert the commit, but I also wonder whether we should
> just change 'pci_claim_resource()' to use request_resource() instead.
>
> I _think_ the use of "insert_resource()" is purely historical, and is
> because that broken function _used_ to not look up the parent, but instead
> do that crazy "pcibios_select_root()" thing, and then it really does need
> to recurse down and "insert" the resource in the right place.
>
> We should no longer _need_ to do the "insert_resource()" thing, since we
> are inserting it into the exact parent that we want (as of commit
> cebd78a8c: "Fix pci_claim_resource").
>
> And if that "insert_resource()" in pci_claim_resource() ever does anything
> fancier than the raw "request_resource()", then that's a problem anyway.
>
> Willy, comments? x86 historically has never used pci_claim_resource() at
> all (it always open-coded the above) except for some quirk handling. So
> I'm pretty sure that a patch like the below should be safe and correct.
> But it's parisc machines that always seem to break.
>
> Added Andrew Patterson to the Cc, because his report was what caused us to
> originally look at pci_claim_resource() and make it use
> "pci_find_parent_resource()". We just never went whole hog, and we left
> that broken "insert_resource()" around.
>
> So Rafael and AndrewP, does this work for you? (I also moved the "dtype"
> thing around, it bothered me).

It works fine for me on the original hardware where the problem was
reported. I don't see any change between iomem/ioport layout between the
kernel without this patch and the kernel with this patch. I don't have
the problem card I had in the same slot, so I would like to move it and
run another test.

Andrew

>
> Linus
>
> ---
> 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 b711fb7..1898c7b 100644
> --- a/drivers/pci/setup-res.c
> +++ b/drivers/pci/setup-res.c
> @@ -100,16 +100,16 @@ int pci_claim_resource(struct pci_dev *dev, int resource)
> {
> struct resource *res = &dev->resource[resource];
> struct resource *root;
> - char *dtype = resource < PCI_BRIDGE_RESOURCES ? "device" : "bridge";
> int err;
>
> root = pci_find_parent_resource(dev, res);
>
> err = -EINVAL;
> if (root != NULL)
> - err = insert_resource(root, res);
> + err = request_resource(root, res);
>
> if (err) {
> + const char *dtype = resource < PCI_BRIDGE_RESOURCES ? "device" : "bridge";
> dev_err(&dev->dev, "BAR %d: %s of %s %pR\n",
> resource,
> root ? "address space collision on" :
>
--
Andrew Patterson
Hewlett-Packard



\
 
 \ /
  Last update: 2009-08-03 05:13    [W:0.066 / U:34.116 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site