On Tue, 2011-01-11 at 12:48 +0000, Jan Beulich wrote:
> >>> On 11.01.11 at 12:50, Ian Campbell <ian.campbell@xxxxxxxxxx> wrote:
> > This is the correct interface to use and something has broken the use
> > of the previous incorrect interface (which fails because the request
> > conflicts with the resources assigned for the PCI device itself
> > instead of nesting like the PCI interfaces do).
>
> While I agree that using the PCI interface is preferable, I can't see
> how this can make a functional difference: __pci_request_region()
> calls request_region() for the port resource and
> __request_mem_region() for the mmio one exactly like what was
> open coded before (request_mem_region() being equivalent to
> __request_mem_region(..., 0), so I don't follow how this fixes
> anything
request_mem_region was failing, the requested region appeared to
conflict with the top level "PCI mem" resource. I don't know why and I
didn't investigate deeper because using the pci_* interfaces is correct
in its own right.
The old way was correct circa 2.6.37-rc5 but broken in 2.6.37 final,
there were a bunch of (reverts of) PCI resource changes in that window,
If I had to make a guess I'd say it was something in there which did it.
> but half of the potentially incorrect use of "long" for
> mmio_addr.
Hmm yes, that should probably have a better type, although we probably
know it's going to be <4G. Can you spin a patch please?
> I'm mainly asking because if it really addresses some problem,
> then unmodified_drivers/linux-2.6/platform-pci/platform-pci.c
> in -unstable should probably get updated too.
Assuming the pci_* interfaces exist back as far as we care about for
those drivers then I don't see why not to apply it.
Ian.
>
> Jan
>
> > Signed-off-by: Ian Campbell <ian.campbell@xxxxxxxxxx>
> > Cc: stable@xxxxxxxxxx # 2.6.37 only
> > ---
> > drivers/xen/platform-pci.c | 21 +++++++--------------
> > 1 files changed, 7 insertions(+), 14 deletions(-)
> >
> > diff --git a/drivers/xen/platform-pci.c b/drivers/xen/platform-pci.c
> > index c01b5dd..afbe041 100644
> > --- a/drivers/xen/platform-pci.c
> > +++ b/drivers/xen/platform-pci.c
> > @@ -105,7 +105,7 @@ static int __devinit platform_pci_init(struct pci_dev
> > *pdev,
> > const struct pci_device_id *ent)
> > {
> > int i, ret;
> > - long ioaddr, iolen;
> > + long ioaddr;
> > long mmio_addr, mmio_len;
> > unsigned int max_nr_gframes;
> >
> > @@ -114,7 +114,6 @@ static int __devinit platform_pci_init(struct pci_dev
> > *pdev,
> > return i;
> >
> > ioaddr = pci_resource_start(pdev, 0);
> > - iolen = pci_resource_len(pdev, 0);
> >
> > mmio_addr = pci_resource_start(pdev, 1);
> > mmio_len = pci_resource_len(pdev, 1);
> > @@ -125,19 +124,13 @@ static int __devinit platform_pci_init(struct pci_dev
> > *pdev,
> > goto pci_out;
> > }
> >
> > - if (request_mem_region(mmio_addr, mmio_len, DRV_NAME) == NULL) {
> > - dev_err(&pdev->dev, "MEM I/O resource 0x%lx @ 0x%lx busy\n",
> > - mmio_addr, mmio_len);
> > - ret = -EBUSY;
> > + ret = pci_request_region(pdev, 1, DRV_NAME);
> > + if (ret < 0)
> > goto pci_out;
> > - }
> >
> > - if (request_region(ioaddr, iolen, DRV_NAME) == NULL) {
> > - dev_err(&pdev->dev, "I/O resource 0x%lx @ 0x%lx busy\n",
> > - iolen, ioaddr);
> > - ret = -EBUSY;
> > + ret = pci_request_region(pdev, 0, DRV_NAME);
> > + if (ret < 0)
> > goto mem_out;
> > - }
> >
> > platform_mmio = mmio_addr;
> > platform_mmiolen = mmio_len;
> > @@ -169,9 +162,9 @@ static int __devinit platform_pci_init(struct pci_dev
> > *pdev,
> > return 0;
> >
> > out:
> > - release_region(ioaddr, iolen);
> > + pci_release_region(pdev, 0);
> > mem_out:
> > - release_mem_region(mmio_addr, mmio_len);
> > + pci_release_region(pdev, 1);
> > pci_out:
> > pci_disable_device(pdev);
> > return ret;
>
>
>
>
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
Home