Re: [PATCH 02/17] pci: Add a generic, weakly-linked pcibios_align_resource

From: Bjorn Helgaas
Date: Wed Jul 12 2017 - 18:50:51 EST


On Tue, Jul 11, 2017 at 06:31:15PM -0700, Palmer Dabbelt wrote:
> Multiple architectures define this as trivial function, and I'm adding
> another one as part of the RISC-V port. This adds a __weak version of
> pcibios_align_resource and deletes the now obselete ones in a handful of
> ports.
>
> The only functional change should be that a handful of ports used to
> export pcibios_fixup_bus. Only some architectures export this, so I
> just dropped it.
>
> Signed-off-by: Palmer Dabbelt <palmer@xxxxxxxxxxx>
> ---
> arch/arc/kernel/pcibios.c | 13 -------------
> arch/arm64/kernel/pci.c | 17 -----------------
> arch/ia64/pci/pci.c | 7 -------
> arch/microblaze/pci/pci-common.c | 7 -------
> arch/sparc/kernel/leon_pci.c | 6 ------
> arch/sparc/kernel/pci.c | 10 ----------
> arch/sparc/kernel/pcic.c | 6 ------
> arch/tile/kernel/pci.c | 10 ----------
> arch/tile/kernel/pci_gx.c | 9 ---------
> drivers/pci/setup-res.c | 12 ++++++++++++
> 10 files changed, 12 insertions(+), 85 deletions(-)

I think you're making your life harder by including these cleanup
patches in your RISC-V support. This patch makes sense (after sorting
out the issues Luis pointed out), but I think the simplest thing to
expedite merging is to add the empty stubs for RISC-V like everybody
else does, then come back after RISC-V gets merged and do the cleanup.
Then the cleanup clearly goes via the PCI tree and isn't entangled
with anything else.

Bjorn

> diff --git a/arch/arc/kernel/pcibios.c b/arch/arc/kernel/pcibios.c
> index 72e1d73d0bd6..05aba5a7b5d2 100644
> --- a/arch/arc/kernel/pcibios.c
> +++ b/arch/arc/kernel/pcibios.c
> @@ -7,16 +7,3 @@
> */
>
> #include <linux/pci.h>
> -
> -/*
> - * We don't have to worry about legacy ISA devices, so nothing to do here
> - */
> -resource_size_t pcibios_align_resource(void *data, const struct resource *res,
> - resource_size_t size, resource_size_t align)
> -{
> - return res->start;
> -}
> -
> -void pcibios_fixup_bus(struct pci_bus *bus)
> -{
> -}
> diff --git a/arch/arm64/kernel/pci.c b/arch/arm64/kernel/pci.c
> index e2b7e4f9cc31..0e2ea1c78542 100644
> --- a/arch/arm64/kernel/pci.c
> +++ b/arch/arm64/kernel/pci.c
> @@ -22,23 +22,6 @@
> #include <linux/pci-ecam.h>
> #include <linux/slab.h>
>
> -/*
> - * Called after each bus is probed, but before its children are examined
> - */
> -void pcibios_fixup_bus(struct pci_bus *bus)
> -{
> - /* nothing to do, expected to be removed in the future */
> -}
> -
> -/*
> - * We don't have to worry about legacy ISA devices, so nothing to do here
> - */
> -resource_size_t pcibios_align_resource(void *data, const struct resource *res,
> - resource_size_t size, resource_size_t align)
> -{
> - return res->start;
> -}
> -
> #ifdef CONFIG_ACPI
> /*
> * Try to assign the IRQ number when probing a new device
> diff --git a/arch/ia64/pci/pci.c b/arch/ia64/pci/pci.c
> index 4068bde623dc..f5ec736100ee 100644
> --- a/arch/ia64/pci/pci.c
> +++ b/arch/ia64/pci/pci.c
> @@ -411,13 +411,6 @@ pcibios_disable_device (struct pci_dev *dev)
> acpi_pci_irq_disable(dev);
> }
>
> -resource_size_t
> -pcibios_align_resource (void *data, const struct resource *res,
> - resource_size_t size, resource_size_t align)
> -{
> - return res->start;
> -}
> -
> /**
> * ia64_pci_get_legacy_mem - generic legacy mem routine
> * @bus: bus to get legacy memory base address for
> diff --git a/arch/microblaze/pci/pci-common.c b/arch/microblaze/pci/pci-common.c
> index 404fb38d06b7..7a332e9f1de0 100644
> --- a/arch/microblaze/pci/pci-common.c
> +++ b/arch/microblaze/pci/pci-common.c
> @@ -829,13 +829,6 @@ EXPORT_SYMBOL(pcibios_fixup_bus);
> * but we want to try to avoid allocating at 0x2900-0x2bff
> * which might have be mirrored at 0x0100-0x03ff..
> */
> -resource_size_t pcibios_align_resource(void *data, const struct resource *res,
> - resource_size_t size, resource_size_t align)
> -{
> - return res->start;
> -}
> -EXPORT_SYMBOL(pcibios_align_resource);
> -
> int pcibios_add_device(struct pci_dev *dev)
> {
> dev->irq = of_irq_parse_and_map_pci(dev, 0, 0);
> diff --git a/arch/sparc/kernel/leon_pci.c b/arch/sparc/kernel/leon_pci.c
> index 4371f72ff025..0eafdf3d036d 100644
> --- a/arch/sparc/kernel/leon_pci.c
> +++ b/arch/sparc/kernel/leon_pci.c
> @@ -94,9 +94,3 @@ void pcibios_fixup_bus(struct pci_bus *pbus)
> }
> }
> }
> -
> -resource_size_t pcibios_align_resource(void *data, const struct resource *res,
> - resource_size_t size, resource_size_t align)
> -{
> - return res->start;
> -}
> diff --git a/arch/sparc/kernel/pci.c b/arch/sparc/kernel/pci.c
> index 7eceaa10836f..3f8670c92951 100644
> --- a/arch/sparc/kernel/pci.c
> +++ b/arch/sparc/kernel/pci.c
> @@ -690,16 +690,6 @@ struct pci_bus *pci_scan_one_pbm(struct pci_pbm_info *pbm,
> return bus;
> }
>
> -void pcibios_fixup_bus(struct pci_bus *pbus)
> -{
> -}
> -
> -resource_size_t pcibios_align_resource(void *data, const struct resource *res,
> - resource_size_t size, resource_size_t align)
> -{
> - return res->start;
> -}
> -
> int pcibios_enable_device(struct pci_dev *dev, int mask)
> {
> u16 cmd, oldcmd;
> diff --git a/arch/sparc/kernel/pcic.c b/arch/sparc/kernel/pcic.c
> index a38787b84322..e038e343f2c1 100644
> --- a/arch/sparc/kernel/pcic.c
> +++ b/arch/sparc/kernel/pcic.c
> @@ -746,12 +746,6 @@ static void watchdog_reset() {
> }
> #endif
>
> -resource_size_t pcibios_align_resource(void *data, const struct resource *res,
> - resource_size_t size, resource_size_t align)
> -{
> - return res->start;
> -}
> -
> int pcibios_enable_device(struct pci_dev *pdev, int mask)
> {
> return 0;
> diff --git a/arch/tile/kernel/pci.c b/arch/tile/kernel/pci.c
> index bc6656b5708b..284ed23b3914 100644
> --- a/arch/tile/kernel/pci.c
> +++ b/arch/tile/kernel/pci.c
> @@ -67,16 +67,6 @@ static struct pci_ops tile_cfg_ops;
>
>
> /*
> - * We don't need to worry about the alignment of resources.
> - */
> -resource_size_t pcibios_align_resource(void *data, const struct resource *res,
> - resource_size_t size, resource_size_t align)
> -{
> - return res->start;
> -}
> -EXPORT_SYMBOL(pcibios_align_resource);
> -
> -/*
> * Open a FD to the hypervisor PCI device.
> *
> * controller_id is the controller number, config type is 0 or 1 for
> diff --git a/arch/tile/kernel/pci_gx.c b/arch/tile/kernel/pci_gx.c
> index b554a68eea1b..bdc623b91f7a 100644
> --- a/arch/tile/kernel/pci_gx.c
> +++ b/arch/tile/kernel/pci_gx.c
> @@ -108,15 +108,6 @@ static struct pci_ops tile_cfg_ops;
> /* Mask of CPUs that should receive PCIe interrupts. */
> static struct cpumask intr_cpus_map;
>
> -/* We don't need to worry about the alignment of resources. */
> -resource_size_t pcibios_align_resource(void *data, const struct resource *res,
> - resource_size_t size,
> - resource_size_t align)
> -{
> - return res->start;
> -}
> -EXPORT_SYMBOL(pcibios_align_resource);
> -
> /*
> * Pick a CPU to receive and handle the PCIe interrupts, based on the IRQ #.
> * For now, we simply send interrupts to non-dataplane CPUs.
> diff --git a/drivers/pci/setup-res.c b/drivers/pci/setup-res.c
> index 85774b7a316a..597ed1f8b15c 100644
> --- a/drivers/pci/setup-res.c
> +++ b/drivers/pci/setup-res.c
> @@ -234,6 +234,18 @@ static int pci_revert_fw_address(struct resource *res, struct pci_dev *dev,
> return 0;
> }
>
> +/*
> + * We don't have to worry about legacy ISA devices, so nothing to do here.
> + * This is marked as __weak because multiple architectures define it, it should
> + * eventually go away.
> + */
> +__attribute__ ((weak))
> +resource_size_t pcibios_align_resource(void *data, const struct resource *res,
> + resource_size_t size, resource_size_t align)
> +{
> + return res->start;
> +}
> +
> static int __pci_assign_resource(struct pci_bus *bus, struct pci_dev *dev,
> int resno, resource_size_t size, resource_size_t align)
> {
> --
> 2.13.0
>