Re: [PATCH v3 1/9] PCI: Move ATS declarations outside of CONFIG_PCI

From: Bjorn Helgaas <helgaas@kernel.org>
To: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>
Cc: lorenzo.pieralisi@arm.com, eric.auger@redhat.com,
	zhongmiao@hisilicon.com, rjw@rjwysocki.net, joro@8bytes.org,
	will.deacon@arm.com, linux-pci@vger.kernel.org,
	linux-acpi@vger.kernel.org, iommu@lists.linux-foundation.org,
	hanjun.guo@linaro.org, sudeep.holla@arm.com, okaya@kernel.org,
	robin.murphy@arm.com, linux-arm-kernel@lists.infradead.org,
	lenb@kernel.org
Subject: Re: [PATCH v3 1/9] PCI: Move ATS declarations outside of CONFIG_PCI
Date: Wed, 17 Apr 2019 14:47:47 -0500	[thread overview]
Message-ID: <20190417194747.GA18308@google.com> (raw)
In-Reply-To: <20190417182448.12382-2-jean-philippe.brucker@arm.com>

On Wed, Apr 17, 2019 at 07:24:40PM +0100, Jean-Philippe Brucker wrote:
> At the moment, the ATS functions are only defined when CONFIG_PCI is
> enabled. Since we're about to use them in the Arm SMMUv3 driver, which
> could be built with CONFIG_PCI disabled, and they are already guarded by
> CONFIG_PCI_ATS which depends on CONFIG_PCI, move the definitions outside
> of CONFIG_PCI.
> 
> Signed-off-by: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>

I guess this is OK with me, although AFAICS they're only used in
arm_smmu_enable_ats() and arm_smmu_disable_ats() and I personally
wouldn't find it objectionable to wrap the bodies of those functions
in "#ifdef CONFIG_PCI".  That might even be a useful hint to the
reader, as opposed to relying on all these stub functions
(dev_is_pci(), pci_ats_disabled(), pci_enable_ats(),
pci_disable_ats(), as well as the complete struct pci_dev declaration)
that depend on config settings that aren't obvious in the caller.

Acked-by: Bjorn Helgaas <bhelgaas@google.com>

> ---
>  include/linux/pci.h | 30 +++++++++++++++---------------
>  1 file changed, 15 insertions(+), 15 deletions(-)
> 
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 77448215ef5b..169c6a18d0b0 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -1521,21 +1521,6 @@ static inline void pcie_ecrc_get_policy(char *str) { }
>  
>  bool pci_ats_disabled(void);
>  
> -#ifdef CONFIG_PCI_ATS
> -/* Address Translation Service */
> -void pci_ats_init(struct pci_dev *dev);
> -int pci_enable_ats(struct pci_dev *dev, int ps);
> -void pci_disable_ats(struct pci_dev *dev);
> -int pci_ats_queue_depth(struct pci_dev *dev);
> -int pci_ats_page_aligned(struct pci_dev *dev);
> -#else
> -static inline void pci_ats_init(struct pci_dev *d) { }
> -static inline int pci_enable_ats(struct pci_dev *d, int ps) { return -ENODEV; }
> -static inline void pci_disable_ats(struct pci_dev *d) { }
> -static inline int pci_ats_queue_depth(struct pci_dev *d) { return -ENODEV; }
> -static inline int pci_ats_page_aligned(struct pci_dev *dev) { return 0; }
> -#endif
> -
>  #ifdef CONFIG_PCIE_PTM
>  int pci_enable_ptm(struct pci_dev *dev, u8 *granularity);
>  #else
> @@ -1730,6 +1715,21 @@ static inline const struct pci_device_id *pci_match_id(const struct pci_device_i
>  { return NULL; }
>  #endif /* CONFIG_PCI */
>  
> +#ifdef CONFIG_PCI_ATS
> +/* Address Translation Service */
> +void pci_ats_init(struct pci_dev *dev);
> +int pci_enable_ats(struct pci_dev *dev, int ps);
> +void pci_disable_ats(struct pci_dev *dev);
> +int pci_ats_queue_depth(struct pci_dev *dev);
> +int pci_ats_page_aligned(struct pci_dev *dev);
> +#else
> +static inline void pci_ats_init(struct pci_dev *d) { }
> +static inline int pci_enable_ats(struct pci_dev *d, int ps) { return -ENODEV; }
> +static inline void pci_disable_ats(struct pci_dev *d) { }
> +static inline int pci_ats_queue_depth(struct pci_dev *d) { return -ENODEV; }
> +static inline int pci_ats_page_aligned(struct pci_dev *dev) { return 0; }
> +#endif
> +
>  /* Include architecture-dependent settings and functions */
>  
>  #include <asm/pci.h>
> -- 
> 2.21.0
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel