Re: [PATCH v2] PCI: controller: Move PCI_DOMAINS selection to arch Kconfig
From: Lorenzo Pieralisi
Date: Tue Jun 26 2018 - 07:22:24 EST
On Mon, Jun 25, 2018 at 11:48:11PM -0700, Scott Branden wrote:
> Hi Lorenzo,
>
>
> On 18-06-25 02:27 AM, Lorenzo Pieralisi wrote:
> >On Fri, Jun 22, 2018 at 11:59:08PM +0800, Ley Foon Tan wrote:
> >>On Thu, 2018-06-21 at 22:08 -0700, Scott Branden wrote:
> >>>
> >>>On 18-06-21 09:54 AM, Lorenzo Pieralisi wrote:
> >>>>
> >>>>On Wed, Jun 20, 2018 at 10:07:33AM +0200, Jan Kiszka wrote:
> >>>>>
> >>>>>On 2018-06-19 13:21, Lorenzo Pieralisi wrote:
> >>>>>>
> >>>>>>Commit 51bc085d6454 ("PCI: Improve host drivers compile test
> >>>>>>coverage")
> >>>>>>added configuration options to allow PCI host controller
> >>>>>>drivers to be
> >>>>>>compile tested on all architectures.
> >>>>>>
> >>>>>>Some host controller drivers (eg PCIE_ALTERA) config entries
> >>>>>>select
> >>>>>>the PCI_DOMAINS config option to enable PCI domains management
> >>>>>>in
> >>>>>>the kernel. Now that host controller drivers can be compiled on
> >>>>>>all architectures, this triggers build regressions on arches
> >>>>>>that
> >>>>>>do not implement the PCI_DOMAINS required API (ie
> >>>>>>pci_domain_nr()):
> >>>>>>
> >>>>>>drivers/ata/pata_ali.c: In function 'ali_init_chipset':
> >>>>>>drivers/ata/pata_ali.c:469:38: error:
> >>>>>> implicit declaration of function 'pci_domain_nr';
> >>>>>> did you mean
> >>>>>>'pci_iomap_wc'?
> >>>>>>
> >>>>>>Furthemore, some software configurations (ie Jailhouse) require
> >>>>>>a
> >>>>>>PCI_DOMAINS enabled kernel to configure multiple host
> >>>>>>controllers
> >>>>>>without having an explicit dependency on the ARM platform on
> >>>>>>which
> >>>>>>they run.
> >>>>>>
> >>>>>>Make PCI_DOMAINS a visible configuration option on ARM so that
> >>>>>>software
> >>>>>>configurations that need it can manually select it and move the
> >>>>>>PCI_DOMAINS selection from PCI controllers configuration file
> >>>>>>to ARM
> >>>>>>sub-arch config entries that currently require it, fixing the
> >>>>>>issue.
> >>>>>>
> >>>>>>Fixes: 51bc085d6454 ("PCI: Improve host drivers compile test
> >>>>>>coverage")
> >>>>>>Link: https://lkml.kernel.org/r/20180612170229.GA10141@roeck-us
> >>>>>>.net
> >>>>>>Reported-by: Guenter Roeck <linux@xxxxxxxxxxxx>
> >>>>>>Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@xxxxxxx>
> >>>>>>Cc: Scott Branden <scott.branden@xxxxxxxxxxxx>
> >>>>>>Cc: Will Deacon <will.deacon@xxxxxxx>
> >>>>>>Cc: Bjorn Helgaas <bhelgaas@xxxxxxxxxx>
> >>>>>>Cc: Rob Herring <robh@xxxxxxxxxx>
> >>>>>>Cc: Russell King <linux@xxxxxxxxxxxxxxx>
> >>>>>>Cc: Jan Kiszka <jan.kiszka@xxxxxxxxxxx>
> >>>>>>Cc: Guenter Roeck <linux@xxxxxxxxxxxx>
> >>>>>>Cc: Ley Foon Tan <lftan@xxxxxxxxxx>
> >>>>>>---
> >>>>>>v1 -> v2
> >>>>>> - Removed ARCH_VIRT PCI_DOMAINS selection
> >>>>>> - Added PCI_DOMAINS visible config option
> >>>>>>
> >>>>>>v1: https://marc.info/?l=linux-pci&m=152932092612352&w=2
> >>>>>>
> >>>>>> arch/arm/Kconfig | 8 +++++++-
> >>>>>> arch/arm/mach-bcm/Kconfig | 1 +
> >>>>>> arch/arm/mach-socfpga/Kconfig | 1 +
> >>>>>> drivers/pci/controller/Kconfig | 3 ---
> >>>>>> 4 files changed, 9 insertions(+), 4 deletions(-)
> >>>>>>
> >>>>>>diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> >>>>>>index 54eeb8d00bc6..843edfd000be 100644
> >>>>>>--- a/arch/arm/Kconfig
> >>>>>>+++ b/arch/arm/Kconfig
> >>>>>>@@ -1245,8 +1245,14 @@ config PCI
> >>>>>> VESA. If you have PCI, say Y, otherwise N.
> >>>>>>
> >>>>>> config PCI_DOMAINS
> >>>>>>- bool
> >>>>>>+ bool "Support for multiple PCI domains"
> >>>>>> depends on PCI
> >>>>>>+ help
> >>>>>>+ Enable PCI domains kernel management. Say Y if your
> >>>>>>machine
> >>>>>>+ has a PCI bus hierarchy that requires more than one PCI
> >>>>>>+ domain (aka segment) to be correctly managed. Say N
> >>>>>>otherwise.
> >>>>>>+
> >>>>>>+ If you don't know what to do here, say N.
> >>>>>>
> >>>>>> config PCI_DOMAINS_GENERIC
> >>>>>> def_bool PCI_DOMAINS
> >>>>>>diff --git a/arch/arm/mach-bcm/Kconfig b/arch/arm/mach-
> >>>>>>bcm/Kconfig
> >>>>>>index c46a728df44e..25aac6ee2ab1 100644
> >>>>>>--- a/arch/arm/mach-bcm/Kconfig
> >>>>>>+++ b/arch/arm/mach-bcm/Kconfig
> >>>>>>@@ -20,6 +20,7 @@ config ARCH_BCM_IPROC
> >>>>>> select GPIOLIB
> >>>>>> select ARM_AMBA
> >>>>>> select PINCTRL
> >>>>>>+ select PCI_DOMAINS if PCI
> >>>>>> help
> >>>>>> This enables support for systems based on Broadcom IPROC
> >>>>>>architected SoCs.
> >>>>>> The IPROC complex contains one or more ARM CPUs along
> >>>>>>with common
> >>>>>>diff --git a/arch/arm/mach-socfpga/Kconfig b/arch/arm/mach-
> >>>>>>socfpga/Kconfig
> >>>>>>index d0f62eacf59d..4adb901dd5eb 100644
> >>>>>>--- a/arch/arm/mach-socfpga/Kconfig
> >>>>>>+++ b/arch/arm/mach-socfpga/Kconfig
> >>>>>>@@ -10,6 +10,7 @@ menuconfig ARCH_SOCFPGA
> >>>>>> select HAVE_ARM_SCU
> >>>>>> select HAVE_ARM_TWD if SMP
> >>>>>> select MFD_SYSCON
> >>>>>>+ select PCI_DOMAINS if PCI
> >>>>>>
> >>>>>> if ARCH_SOCFPGA
> >>>>>> config SOCFPGA_SUSPEND
> >>>>>>diff --git a/drivers/pci/controller/Kconfig
> >>>>>>b/drivers/pci/controller/Kconfig
> >>>>>>index 18fa09b3ac8f..cc9fa02d32a0 100644
> >>>>>>--- a/drivers/pci/controller/Kconfig
> >>>>>>+++ b/drivers/pci/controller/Kconfig
> >>>>>>@@ -96,7 +96,6 @@ config PCI_HOST_GENERIC
> >>>>>> depends on OF
> >>>>>> select PCI_HOST_COMMON
> >>>>>> select IRQ_DOMAIN
> >>>>>>- select PCI_DOMAINS
> >>>>>> help
> >>>>>> Say Y here if you want to support a simple generic PCI
> >>>>>>host
> >>>>>> controller, such as the one emulated by kvmtool.
> >>>>>>@@ -138,7 +137,6 @@ config PCI_VERSATILE
> >>>>>>
> >>>>>> config PCIE_IPROC
> >>>>>> tristate
> >>>>>>- select PCI_DOMAINS
> >>>>>> help
> >>>>>> This enables the iProc PCIe core controller support for
> >>>>>>Broadcom's
> >>>>>> iProc family of SoCs. An appropriate bus interface driver
> >>>>>>needs
> >>>>>>@@ -176,7 +174,6 @@ config PCIE_IPROC_MSI
> >>>>>> config PCIE_ALTERA
> >>>>>> bool "Altera PCIe controller"
> >>>>>> depends on ARM || NIOS2 || COMPILE_TEST
> >>>>>>- select PCI_DOMAINS
> >>>>>> help
> >>>>>> Say Y here if you want to enable PCIe controller support
> >>>>>>on Altera
> >>>>>> FPGA.
> >>>>>>
> >>>>>Acked-by: Jan Kiszka <jan.kiszka@xxxxxxxxxxx>
> >>>>Thanks a lot. Scott, Ley Foon, does it work for you ? I would like
> >>>>to ask Bjorn to send it asap since the build on sparc is still
> >>>>broken.
> >>>Change will have to be made for arm64 for ARCH_BCM_IPROC as well.
> >>Yes, need to add it to arm64 too. Others are okay.
> >On arm64 CONFIG_PCI_DOMAINS defaults to CONFIG_PCI so there is no need
> >to select it, if that's all you need changed there is nothing to
> >change and this patch is read to be merged.
> I see that now: arm64 is doing things properly. The same should be
> done for arm: default to CONFIG_PCI so there is no need to select
> it.
> I didn't like the change to have select to begin with.
I can't select PCI_DOMAINS by default on ARM 32-bit since this would
force it on platforms (eg non-DT legacy) that never relied on it; this
can easily trigger regressions. This patch is ready to go as-is, I do
not like it either but as I tried to explain it is the only reasonable
way forward.
Thanks,
Lorenzo