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

[Bjorn Helgaas]

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
>