| Message ID | 16371687.jmfn381VuV@wuerfel (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
On 2015/5/28 3:51, Arnd Bergmann wrote: > On Wednesday 27 May 2015 18:19:57 Fabrice Gasnier wrote: >> On 05/27/2015 05:43 PM, Arnd Bergmann wrote: >>> On Wednesday 27 May 2015 17:31:46 Arnd Bergmann wrote: >>>> Yes, that works. However there are two problems with the approach: >>>> >>>> - we have to change all PCI host drivers on ARM to do this in order to remove >>>> the ARM-specific pcibios_msi_controller() function >>>> - it's possible that there are dw_pcie implementations that do not include >>>> an MSI controller, so that pointer would be NULL, which leads to the >>>> core code to still call the ARM-specific pcibios_msi_controller() function >>>> unless we remove it. >>> I wonder if this simple patch would be sufficient to kill off >>> pcibios_msi_controller(). >>> >>> Can one of you try it? >>> >>> Arnd >> >> Hi Arnd, >> I tested it quickly on my platform, with and without CONFIG_PCI_MSI. >> In both case it seems to run fine with dw pcie. >> > > Awesome. Now we just need to find a way to move the align_resource callback. > I wonder if the approach below would be acceptable. > > Arnd > Hi Arnd, Many thanks for your kindly suggestions. > diff --git a/arch/arm/kernel/bios32.c b/arch/arm/kernel/bios32.c > index fcbbbb1b9e95..20839d26a490 100644 > --- a/arch/arm/kernel/bios32.c > +++ b/arch/arm/kernel/bios32.c > @@ -597,9 +588,6 @@ resource_size_t pcibios_align_resource(void *data, const struct resource *res, > > start = (start + align - 1) & ~(align - 1); > > - if (sys->align_resource) > - return sys->align_resource(dev, res, start, size, align); > - > return start; > } > > diff --git a/drivers/pci/host/pci-mvebu.c b/drivers/pci/host/pci-mvebu.c > index 1ab863551920..74216c9c2822 100644 > --- a/drivers/pci/host/pci-mvebu.c > +++ b/drivers/pci/host/pci-mvebu.c > @@ -751,27 +751,20 @@ static int mvebu_pcie_setup(int nr, struct pci_sys_data *sys) > return 1; > } > > -static struct pci_bus *mvebu_pcie_scan_bus(int nr, struct pci_sys_data *sys) > +static resource_size_t mvebu_pcie_align_resource(void *data, > + const struct resource *res, > + resource_size_t size, > + resource_size_t align) > { > - struct mvebu_pcie *pcie = sys_to_pcie(sys); > - struct pci_bus *bus; > + struct device *dev = data; > > - bus = pci_create_root_bus(&pcie->pdev->dev, sys->busnr, > - &mvebu_pcie_ops, sys, &sys->resources); > - if (!bus) > - return NULL; > + resource_size_t start = res->start; > > - pci_scan_child_bus(bus); > + if (res->flags & IORESOURCE_IO && start & 0x300) > + start = (start + 0x3ff) & ~0x3ff; > > - return bus; > -} > + start = (start + align - 1) & ~(align - 1); > > -static resource_size_t mvebu_pcie_align_resource(struct pci_dev *dev, > - const struct resource *res, > - resource_size_t start, > - resource_size_t size, > - resource_size_t align) > -{ > if (dev->bus->number != 0) > return start; > > @@ -796,6 +789,25 @@ static resource_size_t mvebu_pcie_align_resource(struct pci_dev *dev, > return start; > } > > +static struct pci_bus *mvebu_pcie_scan_bus(int nr, struct pci_sys_data *sys) > +{ > + struct mvebu_pcie *pcie = sys_to_pcie(sys); > + struct pci_host_bridge *phb; > + struct pci_bus *bus; > + > + bus = pci_create_root_bus(&pcie->pdev->dev, sys->busnr, > + &mvebu_pcie_ops, sys, &sys->resources); > + if (!bus) > + return NULL; > + > + phb = pci_find_host_bridge(bus); > + phb->align_resource = mvebu_pcie_align_resource; > + > + pci_scan_child_bus(bus); > + > + return bus; > +} > + > static void mvebu_pcie_enable(struct mvebu_pcie *pcie) > { > struct hw_pci hw; > @@ -812,7 +824,6 @@ static void mvebu_pcie_enable(struct mvebu_pcie *pcie) > hw.scan = mvebu_pcie_scan_bus; > hw.map_irq = of_irq_parse_and_map_pci; > hw.ops = &mvebu_pcie_ops; > - hw.align_resource = mvebu_pcie_align_resource; > > pci_common_init(&hw); > } > diff --git a/drivers/pci/setup-res.c b/drivers/pci/setup-res.c > index 232f9254c11a..e1f5e13c4636 100644 > --- a/drivers/pci/setup-res.c > +++ b/drivers/pci/setup-res.c > @@ -200,7 +200,11 @@ static int pci_revert_fw_address(struct resource *res, struct pci_dev *dev, > } > > static int __pci_assign_resource(struct pci_bus *bus, struct pci_dev *dev, > - int resno, resource_size_t size, resource_size_t align) > + int resno, resource_size_t size, resource_size_t align, > + resource_size_t (*alignf)(void *, > + const struct resource *, > + resource_size_t, > + resource_size_t)) > { > struct resource *res = dev->resource + resno; > resource_size_t min; > @@ -217,7 +221,7 @@ static int __pci_assign_resource(struct pci_bus *bus, struct pci_dev *dev, > */ > ret = pci_bus_alloc_resource(bus, res, size, align, min, > IORESOURCE_PREFETCH | IORESOURCE_MEM_64, > - pcibios_align_resource, dev); > + alignf, dev); > if (ret == 0) > return 0; > > @@ -229,7 +233,7 @@ static int __pci_assign_resource(struct pci_bus *bus, struct pci_dev *dev, > (IORESOURCE_PREFETCH | IORESOURCE_MEM_64)) { > ret = pci_bus_alloc_resource(bus, res, size, align, min, > IORESOURCE_PREFETCH, > - pcibios_align_resource, dev); > + alignf, dev); > if (ret == 0) > return 0; > } > @@ -242,7 +246,7 @@ static int __pci_assign_resource(struct pci_bus *bus, struct pci_dev *dev, > */ > if (res->flags & (IORESOURCE_PREFETCH | IORESOURCE_MEM_64)) > ret = pci_bus_alloc_resource(bus, res, size, align, min, 0, > - pcibios_align_resource, dev); > + alignf, dev); > > return ret; > } > @@ -251,10 +255,19 @@ static int _pci_assign_resource(struct pci_dev *dev, int resno, > resource_size_t size, resource_size_t min_align) > { > struct pci_bus *bus; > + struct pci_host_bridge *phb; > int ret; > > bus = dev->bus; > - while ((ret = __pci_assign_resource(bus, dev, resno, size, min_align))) { > + phb = pci_find_host_bridge(bus); > + > + if (phb->align_resource) > + alignf = phb->align_resource; > + else > + alignf = pcibios_align_resource; > + > + while ((ret = __pci_assign_resource(bus, dev, resno, size, > + min_align, alignf))) { > if (!bus->parent || !bus->self->transparent) > break; > bus = bus->parent; > I think this code is fine, and we also should add a funtion point in struct pci_host_bridge to store mvebu_pcie_align_resource. How about I make a new version patchset with above codes for further reviewing ? Best Regards, Zhou > > . > -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/arm/kernel/bios32.c b/arch/arm/kernel/bios32.c index fcbbbb1b9e95..20839d26a490 100644 --- a/arch/arm/kernel/bios32.c +++ b/arch/arm/kernel/bios32.c @@ -597,9 +588,6 @@ resource_size_t pcibios_align_resource(void *data, const struct resource *res, start = (start + align - 1) & ~(align - 1); - if (sys->align_resource) - return sys->align_resource(dev, res, start, size, align); - return start; } diff --git a/drivers/pci/host/pci-mvebu.c b/drivers/pci/host/pci-mvebu.c index 1ab863551920..74216c9c2822 100644 --- a/drivers/pci/host/pci-mvebu.c +++ b/drivers/pci/host/pci-mvebu.c @@ -751,27 +751,20 @@ static int mvebu_pcie_setup(int nr, struct pci_sys_data *sys) return 1; } -static struct pci_bus *mvebu_pcie_scan_bus(int nr, struct pci_sys_data *sys) +static resource_size_t mvebu_pcie_align_resource(void *data, + const struct resource *res, + resource_size_t size, + resource_size_t align) { - struct mvebu_pcie *pcie = sys_to_pcie(sys); - struct pci_bus *bus; + struct device *dev = data; - bus = pci_create_root_bus(&pcie->pdev->dev, sys->busnr, - &mvebu_pcie_ops, sys, &sys->resources); - if (!bus) - return NULL; + resource_size_t start = res->start; - pci_scan_child_bus(bus); + if (res->flags & IORESOURCE_IO && start & 0x300) + start = (start + 0x3ff) & ~0x3ff; - return bus; -} + start = (start + align - 1) & ~(align - 1); -static resource_size_t mvebu_pcie_align_resource(struct pci_dev *dev, - const struct resource *res, - resource_size_t start, - resource_size_t size, - resource_size_t align) -{ if (dev->bus->number != 0) return start; @@ -796,6 +789,25 @@ static resource_size_t mvebu_pcie_align_resource(struct pci_dev *dev, return start; } +static struct pci_bus *mvebu_pcie_scan_bus(int nr, struct pci_sys_data *sys) +{ + struct mvebu_pcie *pcie = sys_to_pcie(sys); + struct pci_host_bridge *phb; + struct pci_bus *bus; + + bus = pci_create_root_bus(&pcie->pdev->dev, sys->busnr, + &mvebu_pcie_ops, sys, &sys->resources); + if (!bus) + return NULL; + + phb = pci_find_host_bridge(bus); + phb->align_resource = mvebu_pcie_align_resource; + + pci_scan_child_bus(bus); + + return bus; +} + static void mvebu_pcie_enable(struct mvebu_pcie *pcie) { struct hw_pci hw; @@ -812,7 +824,6 @@ static void mvebu_pcie_enable(struct mvebu_pcie *pcie) hw.scan = mvebu_pcie_scan_bus; hw.map_irq = of_irq_parse_and_map_pci; hw.ops = &mvebu_pcie_ops; - hw.align_resource = mvebu_pcie_align_resource; pci_common_init(&hw); } diff --git a/drivers/pci/setup-res.c b/drivers/pci/setup-res.c index 232f9254c11a..e1f5e13c4636 100644 --- a/drivers/pci/setup-res.c +++ b/drivers/pci/setup-res.c @@ -200,7 +200,11 @@ static int pci_revert_fw_address(struct resource *res, struct pci_dev *dev, } static int __pci_assign_resource(struct pci_bus *bus, struct pci_dev *dev, - int resno, resource_size_t size, resource_size_t align) + int resno, resource_size_t size, resource_size_t align, + resource_size_t (*alignf)(void *, + const struct resource *, + resource_size_t, + resource_size_t)) { struct resource *res = dev->resource + resno; resource_size_t min; @@ -217,7 +221,7 @@ static int __pci_assign_resource(struct pci_bus *bus, struct pci_dev *dev, */ ret = pci_bus_alloc_resource(bus, res, size, align, min, IORESOURCE_PREFETCH | IORESOURCE_MEM_64, - pcibios_align_resource, dev); + alignf, dev); if (ret == 0) return 0; @@ -229,7 +233,7 @@ static int __pci_assign_resource(struct pci_bus *bus, struct pci_dev *dev, (IORESOURCE_PREFETCH | IORESOURCE_MEM_64)) { ret = pci_bus_alloc_resource(bus, res, size, align, min, IORESOURCE_PREFETCH, - pcibios_align_resource, dev); + alignf, dev); if (ret == 0) return 0; } @@ -242,7 +246,7 @@ static int __pci_assign_resource(struct pci_bus *bus, struct pci_dev *dev, */ if (res->flags & (IORESOURCE_PREFETCH | IORESOURCE_MEM_64)) ret = pci_bus_alloc_resource(bus, res, size, align, min, 0, - pcibios_align_resource, dev); + alignf, dev); return ret; } @@ -251,10 +255,19 @@ static int _pci_assign_resource(struct pci_dev *dev, int resno, resource_size_t size, resource_size_t min_align) { struct pci_bus *bus; + struct pci_host_bridge *phb; int ret; bus = dev->bus; - while ((ret = __pci_assign_resource(bus, dev, resno, size, min_align))) { + phb = pci_find_host_bridge(bus); + + if (phb->align_resource) + alignf = phb->align_resource; + else + alignf = pcibios_align_resource; + + while ((ret = __pci_assign_resource(bus, dev, resno, size, + min_align, alignf))) { if (!bus->parent || !bus->self->transparent) break; bus = bus->parent;