* [PATCH v4] PCI: improve host drivers compile test coverage
@ 2018-04-05 19:31 Rob Herring
2018-05-18 21:42 ` Bjorn Helgaas
2018-06-12 17:02 ` [v4] " Guenter Roeck
0 siblings, 2 replies; 17+ messages in thread
From: Rob Herring @ 2018-04-05 19:31 UTC (permalink / raw)
To: Lorenzo Pieralisi, Bjorn Helgaas; +Cc: Scott Branden, linux-pci
Add COMPILE_TEST on driver config options with it. Some ARM drivers
still have arch dependencies, so we have to keep those dependent on ARM.
Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: linux-pci@vger.kernel.org
Signed-off-by: Rob Herring <robh@kernel.org>
---
v4: Drop iProc hunks
drivers/pci/dwc/Kconfig | 24 ++++++++++++------------
drivers/pci/host/Kconfig | 31 ++++++++++++++-----------------
2 files changed, 26 insertions(+), 29 deletions(-)
diff --git a/drivers/pci/dwc/Kconfig b/drivers/pci/dwc/Kconfig
index 8c1a5167fb5b..2902544f5834 100644
--- a/drivers/pci/dwc/Kconfig
+++ b/drivers/pci/dwc/Kconfig
@@ -64,21 +64,21 @@ config PCIE_DW_PLAT
config PCI_EXYNOS
bool "Samsung Exynos PCIe controller"
- depends on SOC_EXYNOS5440
+ depends on SOC_EXYNOS5440 || COMPILE_TEST
depends on PCI_MSI_IRQ_DOMAIN
select PCIEPORTBUS
select PCIE_DW_HOST
config PCI_IMX6
bool "Freescale i.MX6 PCIe controller"
- depends on SOC_IMX6Q
+ depends on SOC_IMX6Q || (ARM && COMPILE_TEST)
depends on PCI_MSI_IRQ_DOMAIN
select PCIEPORTBUS
select PCIE_DW_HOST
config PCIE_SPEAR13XX
bool "STMicroelectronics SPEAr PCIe controller"
- depends on ARCH_SPEAR13XX
+ depends on ARCH_SPEAR13XX || COMPILE_TEST
depends on PCI_MSI_IRQ_DOMAIN
select PCIEPORTBUS
select PCIE_DW_HOST
@@ -87,7 +87,7 @@ config PCIE_SPEAR13XX
config PCI_KEYSTONE
bool "TI Keystone PCIe controller"
- depends on ARCH_KEYSTONE
+ depends on ARCH_KEYSTONE || (ARM && COMPILE_TEST)
depends on PCI_MSI_IRQ_DOMAIN
select PCIEPORTBUS
select PCIE_DW_HOST
@@ -99,7 +99,7 @@ config PCI_KEYSTONE
config PCI_LAYERSCAPE
bool "Freescale Layerscape PCIe controller"
- depends on OF && (ARM || ARCH_LAYERSCAPE)
+ depends on OF && (ARM || ARCH_LAYERSCAPE || COMPILE_TEST)
depends on PCI_MSI_IRQ_DOMAIN
select MFD_SYSCON
select PCIE_DW_HOST
@@ -107,7 +107,7 @@ config PCI_LAYERSCAPE
Say Y here if you want PCIe controller support on Layerscape SoCs.
config PCI_HISI
- depends on OF && ARM64
+ depends on OF && (ARM64 || COMPILE_TEST)
bool "HiSilicon Hip05 and Hip06 SoCs PCIe controllers"
depends on PCI_MSI_IRQ_DOMAIN
select PCIEPORTBUS
@@ -119,7 +119,7 @@ config PCI_HISI
config PCIE_QCOM
bool "Qualcomm PCIe controller"
- depends on ARCH_QCOM && OF
+ depends on OF && (ARCH_QCOM || COMPILE_TEST)
depends on PCI_MSI_IRQ_DOMAIN
select PCIEPORTBUS
select PCIE_DW_HOST
@@ -130,7 +130,7 @@ config PCIE_QCOM
config PCIE_ARMADA_8K
bool "Marvell Armada-8K PCIe controller"
- depends on ARCH_MVEBU
+ depends on ARCH_MVEBU || COMPILE_TEST
depends on PCI_MSI_IRQ_DOMAIN
select PCIEPORTBUS
select PCIE_DW_HOST
@@ -145,7 +145,7 @@ config PCIE_ARTPEC6
config PCIE_ARTPEC6_HOST
bool "Axis ARTPEC-6 PCIe controller Host Mode"
- depends on MACH_ARTPEC6
+ depends on MACH_ARTPEC6 || COMPILE_TEST
depends on PCI_MSI_IRQ_DOMAIN
select PCIEPORTBUS
select PCIE_DW_HOST
@@ -156,7 +156,7 @@ config PCIE_ARTPEC6_HOST
config PCIE_ARTPEC6_EP
bool "Axis ARTPEC-6 PCIe controller Endpoint Mode"
- depends on MACH_ARTPEC6
+ depends on MACH_ARTPEC6 || COMPILE_TEST
depends on PCI_ENDPOINT
select PCIE_DW_EP
select PCIE_ARTPEC6
@@ -165,7 +165,7 @@ config PCIE_ARTPEC6_EP
endpoint mode. This uses the DesignWare core.
config PCIE_KIRIN
- depends on OF && ARM64
+ depends on OF && (ARM64 || COMPILE_TEST)
bool "HiSilicon Kirin series SoCs PCIe controllers"
depends on PCI_MSI_IRQ_DOMAIN
select PCIEPORTBUS
@@ -176,7 +176,7 @@ config PCIE_KIRIN
config PCIE_HISI_STB
bool "HiSilicon STB SoCs PCIe controllers"
- depends on ARCH_HISI
+ depends on ARCH_HISI || COMPILE_TEST
depends on PCI_MSI_IRQ_DOMAIN
select PCIEPORTBUS
select PCIE_DW_HOST
diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
index 0d0177ce436c..3d180b594ff2 100644
--- a/drivers/pci/host/Kconfig
+++ b/drivers/pci/host/Kconfig
@@ -5,13 +5,13 @@ menu "PCI host controller drivers"
config PCI_MVEBU
bool "Marvell EBU PCIe controller"
- depends on ARCH_MVEBU || ARCH_DOVE
+ depends on ARCH_MVEBU || ARCH_DOVE || COMPILE_TEST
depends on ARM
depends on OF
config PCI_AARDVARK
bool "Aardvark PCIe controller"
- depends on ARCH_MVEBU && ARM64
+ depends on (ARCH_MVEBU && ARM64) || COMPILE_TEST
depends on OF
depends on PCI_MSI_IRQ_DOMAIN
help
@@ -21,7 +21,7 @@ config PCI_AARDVARK
config PCIE_XILINX_NWL
bool "NWL PCIe Core"
- depends on ARCH_ZYNQMP
+ depends on ARCH_ZYNQMP || COMPILE_TEST
depends on PCI_MSI_IRQ_DOMAIN
help
Say 'Y' here if you want kernel support for Xilinx
@@ -32,12 +32,11 @@ config PCIE_XILINX_NWL
config PCI_FTPCI100
bool "Faraday Technology FTPCI100 PCI controller"
depends on OF
- depends on ARM
default ARCH_GEMINI
config PCI_TEGRA
bool "NVIDIA Tegra PCIe controller"
- depends on ARCH_TEGRA
+ depends on ARCH_TEGRA || COMPILE_TEST
depends on PCI_MSI_IRQ_DOMAIN
help
Say Y here if you want support for the PCIe host controller found
@@ -45,8 +44,8 @@ config PCI_TEGRA
config PCI_RCAR_GEN2
bool "Renesas R-Car Gen2 Internal PCI controller"
- depends on ARM
depends on ARCH_RENESAS || COMPILE_TEST
+ depends on ARM
help
Say Y here if you want internal PCI support on R-Car Gen2 SoC.
There are 3 internal PCI controllers available with a single
@@ -54,7 +53,7 @@ config PCI_RCAR_GEN2
config PCIE_RCAR
bool "Renesas R-Car PCIe controller"
- depends on ARCH_RENESAS || (ARM && COMPILE_TEST)
+ depends on ARCH_RENESAS || COMPILE_TEST
depends on PCI_MSI_IRQ_DOMAIN
help
Say Y here if you want PCIe controller support on R-Car SoCs.
@@ -65,7 +64,7 @@ config PCI_HOST_COMMON
config PCI_HOST_GENERIC
bool "Generic PCI host controller"
- depends on (ARM || ARM64) && OF
+ depends on OF
select PCI_HOST_COMMON
select IRQ_DOMAIN
help
@@ -74,14 +73,14 @@ config PCI_HOST_GENERIC
config PCIE_XILINX
bool "Xilinx AXI PCIe host bridge support"
- depends on ARCH_ZYNQ || MICROBLAZE || (MIPS && PCI_DRIVERS_GENERIC)
+ depends on ARCH_ZYNQ || MICROBLAZE || (MIPS && PCI_DRIVERS_GENERIC) || COMPILE_TEST
help
Say 'Y' here if you want kernel to support the Xilinx AXI PCIe
Host Bridge driver.
config PCI_XGENE
bool "X-Gene PCIe controller"
- depends on ARM64
+ depends on ARM64 || COMPILE_TEST
depends on OF || (ACPI && PCI_QUIRKS)
select PCIEPORTBUS
help
@@ -101,7 +100,7 @@ config PCI_XGENE_MSI
config PCI_V3_SEMI
bool "V3 Semiconductor PCI controller"
depends on OF
- depends on ARM
+ depends on ARM || COMPILE_TEST
default ARCH_INTEGRATOR_AP
config PCI_VERSATILE
@@ -147,8 +146,7 @@ config PCIE_IPROC_MSI
config PCIE_ALTERA
bool "Altera PCIe controller"
- depends on ARM || NIOS2
- depends on OF_PCI
+ depends on ARM || NIOS2 || COMPILE_TEST
select PCI_DOMAINS
help
Say Y here if you want to enable PCIe controller support on Altera
@@ -164,7 +162,7 @@ config PCIE_ALTERA_MSI
config PCI_HOST_THUNDER_PEM
bool "Cavium Thunder PCIe controller to off-chip devices"
- depends on ARM64
+ depends on ARM64 || COMPILE_TEST
depends on OF || (ACPI && PCI_QUIRKS)
select PCI_HOST_COMMON
help
@@ -172,7 +170,7 @@ config PCI_HOST_THUNDER_PEM
config PCI_HOST_THUNDER_ECAM
bool "Cavium Thunder ECAM controller to on-chip devices on pass-1.x silicon"
- depends on ARM64
+ depends on ARM64 || COMPILE_TEST
depends on OF || (ACPI && PCI_QUIRKS)
select PCI_HOST_COMMON
help
@@ -191,9 +189,8 @@ config PCIE_ROCKCHIP
config PCIE_MEDIATEK
bool "MediaTek PCIe controller"
- depends on (ARM || ARM64) && (ARCH_MEDIATEK || COMPILE_TEST)
+ depends on ARCH_MEDIATEK || COMPILE_TEST
depends on OF
- depends on PCI
select PCIEPORTBUS
help
Say Y here if you want to enable PCIe controller support on
--
2.14.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH v4] PCI: improve host drivers compile test coverage
2018-04-05 19:31 [PATCH v4] PCI: improve host drivers compile test coverage Rob Herring
@ 2018-05-18 21:42 ` Bjorn Helgaas
2018-05-19 19:22 ` Rob Herring
2018-06-12 17:02 ` [v4] " Guenter Roeck
1 sibling, 1 reply; 17+ messages in thread
From: Bjorn Helgaas @ 2018-05-18 21:42 UTC (permalink / raw)
To: Rob Herring; +Cc: Lorenzo Pieralisi, Bjorn Helgaas, Scott Branden, linux-pci
Hi Rob,
On Thu, Apr 05, 2018 at 02:31:54PM -0500, Rob Herring wrote:
> Add COMPILE_TEST on driver config options with it. Some ARM drivers
> still have arch dependencies, so we have to keep those dependent on ARM.
>
> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: linux-pci@vger.kernel.org
> Signed-off-by: Rob Herring <robh@kernel.org>
> config PCIE_ALTERA
> bool "Altera PCIe controller"
> - depends on ARM || NIOS2
> - depends on OF_PCI
> + depends on ARM || NIOS2 || COMPILE_TEST
Did you intend to drop the OF_PCI dependency?
I see that CONFIG_OF_PCI doesn't exist, so that looks bogus.
But drivers/pci/host/pcie-altera.c still uses
devm_of_pci_get_host_bridge_resources(), so I think this should probably
still depend on CONFIG_OF?
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v4] PCI: improve host drivers compile test coverage
2018-05-18 21:42 ` Bjorn Helgaas
@ 2018-05-19 19:22 ` Rob Herring
0 siblings, 0 replies; 17+ messages in thread
From: Rob Herring @ 2018-05-19 19:22 UTC (permalink / raw)
To: Bjorn Helgaas; +Cc: Lorenzo Pieralisi, Bjorn Helgaas, Scott Branden, linux-pci
On Fri, May 18, 2018 at 4:42 PM, Bjorn Helgaas <helgaas@kernel.org> wrote:
> Hi Rob,
>
> On Thu, Apr 05, 2018 at 02:31:54PM -0500, Rob Herring wrote:
>> Add COMPILE_TEST on driver config options with it. Some ARM drivers
>> still have arch dependencies, so we have to keep those dependent on ARM.
>>
>> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
>> Cc: Bjorn Helgaas <bhelgaas@google.com>
>> Cc: linux-pci@vger.kernel.org
>> Signed-off-by: Rob Herring <robh@kernel.org>
>
>> config PCIE_ALTERA
>> bool "Altera PCIe controller"
>> - depends on ARM || NIOS2
>> - depends on OF_PCI
>> + depends on ARM || NIOS2 || COMPILE_TEST
>
> Did you intend to drop the OF_PCI dependency?
No, that should have been removed when OF_PCI was in commit
4670d610d592 ("PCI: Move OF-related PCI functions into PCI core").
> I see that CONFIG_OF_PCI doesn't exist, so that looks bogus.
>
> But drivers/pci/host/pcie-altera.c still uses
> devm_of_pci_get_host_bridge_resources(), so I think this should probably
> still depend on CONFIG_OF?
There's an empty version so it shouldn't matter.
Rob
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [v4] PCI: improve host drivers compile test coverage
2018-04-05 19:31 [PATCH v4] PCI: improve host drivers compile test coverage Rob Herring
2018-05-18 21:42 ` Bjorn Helgaas
@ 2018-06-12 17:02 ` Guenter Roeck
2018-06-13 10:09 ` Lorenzo Pieralisi
1 sibling, 1 reply; 17+ messages in thread
From: Guenter Roeck @ 2018-06-12 17:02 UTC (permalink / raw)
To: Rob Herring; +Cc: Lorenzo Pieralisi, Bjorn Helgaas, Scott Branden, linux-pci
On Thu, Apr 05, 2018 at 02:31:54PM -0500, Rob Herring wrote:
> Add COMPILE_TEST on driver config options with it. Some ARM drivers
> still have arch dependencies, so we have to keep those dependent on ARM.
>
> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: linux-pci@vger.kernel.org
> Signed-off-by: Rob Herring <robh@kernel.org>
This patch has the undesirable side effect that it selects PCI_DOMAINS for
sparc32:allmodconfig, which in turn results in
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'?
Unfortunately, 37bd62d224c82 ("PCI: Enable PCI_DOMAINS along with generic
PCI host controller") has pretty much the same result. No idea how to fix
the problem, so I won't even try.
Guenter
> ---
> v4: Drop iProc hunks
>
> drivers/pci/dwc/Kconfig | 24 ++++++++++++------------
> drivers/pci/host/Kconfig | 31 ++++++++++++++-----------------
> 2 files changed, 26 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/pci/dwc/Kconfig b/drivers/pci/dwc/Kconfig
> index 8c1a5167fb5b..2902544f5834 100644
> --- a/drivers/pci/dwc/Kconfig
> +++ b/drivers/pci/dwc/Kconfig
> @@ -64,21 +64,21 @@ config PCIE_DW_PLAT
>
> config PCI_EXYNOS
> bool "Samsung Exynos PCIe controller"
> - depends on SOC_EXYNOS5440
> + depends on SOC_EXYNOS5440 || COMPILE_TEST
> depends on PCI_MSI_IRQ_DOMAIN
> select PCIEPORTBUS
> select PCIE_DW_HOST
>
> config PCI_IMX6
> bool "Freescale i.MX6 PCIe controller"
> - depends on SOC_IMX6Q
> + depends on SOC_IMX6Q || (ARM && COMPILE_TEST)
> depends on PCI_MSI_IRQ_DOMAIN
> select PCIEPORTBUS
> select PCIE_DW_HOST
>
> config PCIE_SPEAR13XX
> bool "STMicroelectronics SPEAr PCIe controller"
> - depends on ARCH_SPEAR13XX
> + depends on ARCH_SPEAR13XX || COMPILE_TEST
> depends on PCI_MSI_IRQ_DOMAIN
> select PCIEPORTBUS
> select PCIE_DW_HOST
> @@ -87,7 +87,7 @@ config PCIE_SPEAR13XX
>
> config PCI_KEYSTONE
> bool "TI Keystone PCIe controller"
> - depends on ARCH_KEYSTONE
> + depends on ARCH_KEYSTONE || (ARM && COMPILE_TEST)
> depends on PCI_MSI_IRQ_DOMAIN
> select PCIEPORTBUS
> select PCIE_DW_HOST
> @@ -99,7 +99,7 @@ config PCI_KEYSTONE
>
> config PCI_LAYERSCAPE
> bool "Freescale Layerscape PCIe controller"
> - depends on OF && (ARM || ARCH_LAYERSCAPE)
> + depends on OF && (ARM || ARCH_LAYERSCAPE || COMPILE_TEST)
> depends on PCI_MSI_IRQ_DOMAIN
> select MFD_SYSCON
> select PCIE_DW_HOST
> @@ -107,7 +107,7 @@ config PCI_LAYERSCAPE
> Say Y here if you want PCIe controller support on Layerscape SoCs.
>
> config PCI_HISI
> - depends on OF && ARM64
> + depends on OF && (ARM64 || COMPILE_TEST)
> bool "HiSilicon Hip05 and Hip06 SoCs PCIe controllers"
> depends on PCI_MSI_IRQ_DOMAIN
> select PCIEPORTBUS
> @@ -119,7 +119,7 @@ config PCI_HISI
>
> config PCIE_QCOM
> bool "Qualcomm PCIe controller"
> - depends on ARCH_QCOM && OF
> + depends on OF && (ARCH_QCOM || COMPILE_TEST)
> depends on PCI_MSI_IRQ_DOMAIN
> select PCIEPORTBUS
> select PCIE_DW_HOST
> @@ -130,7 +130,7 @@ config PCIE_QCOM
>
> config PCIE_ARMADA_8K
> bool "Marvell Armada-8K PCIe controller"
> - depends on ARCH_MVEBU
> + depends on ARCH_MVEBU || COMPILE_TEST
> depends on PCI_MSI_IRQ_DOMAIN
> select PCIEPORTBUS
> select PCIE_DW_HOST
> @@ -145,7 +145,7 @@ config PCIE_ARTPEC6
>
> config PCIE_ARTPEC6_HOST
> bool "Axis ARTPEC-6 PCIe controller Host Mode"
> - depends on MACH_ARTPEC6
> + depends on MACH_ARTPEC6 || COMPILE_TEST
> depends on PCI_MSI_IRQ_DOMAIN
> select PCIEPORTBUS
> select PCIE_DW_HOST
> @@ -156,7 +156,7 @@ config PCIE_ARTPEC6_HOST
>
> config PCIE_ARTPEC6_EP
> bool "Axis ARTPEC-6 PCIe controller Endpoint Mode"
> - depends on MACH_ARTPEC6
> + depends on MACH_ARTPEC6 || COMPILE_TEST
> depends on PCI_ENDPOINT
> select PCIE_DW_EP
> select PCIE_ARTPEC6
> @@ -165,7 +165,7 @@ config PCIE_ARTPEC6_EP
> endpoint mode. This uses the DesignWare core.
>
> config PCIE_KIRIN
> - depends on OF && ARM64
> + depends on OF && (ARM64 || COMPILE_TEST)
> bool "HiSilicon Kirin series SoCs PCIe controllers"
> depends on PCI_MSI_IRQ_DOMAIN
> select PCIEPORTBUS
> @@ -176,7 +176,7 @@ config PCIE_KIRIN
>
> config PCIE_HISI_STB
> bool "HiSilicon STB SoCs PCIe controllers"
> - depends on ARCH_HISI
> + depends on ARCH_HISI || COMPILE_TEST
> depends on PCI_MSI_IRQ_DOMAIN
> select PCIEPORTBUS
> select PCIE_DW_HOST
> diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
> index 0d0177ce436c..3d180b594ff2 100644
> --- a/drivers/pci/host/Kconfig
> +++ b/drivers/pci/host/Kconfig
> @@ -5,13 +5,13 @@ menu "PCI host controller drivers"
>
> config PCI_MVEBU
> bool "Marvell EBU PCIe controller"
> - depends on ARCH_MVEBU || ARCH_DOVE
> + depends on ARCH_MVEBU || ARCH_DOVE || COMPILE_TEST
> depends on ARM
> depends on OF
>
> config PCI_AARDVARK
> bool "Aardvark PCIe controller"
> - depends on ARCH_MVEBU && ARM64
> + depends on (ARCH_MVEBU && ARM64) || COMPILE_TEST
> depends on OF
> depends on PCI_MSI_IRQ_DOMAIN
> help
> @@ -21,7 +21,7 @@ config PCI_AARDVARK
>
> config PCIE_XILINX_NWL
> bool "NWL PCIe Core"
> - depends on ARCH_ZYNQMP
> + depends on ARCH_ZYNQMP || COMPILE_TEST
> depends on PCI_MSI_IRQ_DOMAIN
> help
> Say 'Y' here if you want kernel support for Xilinx
> @@ -32,12 +32,11 @@ config PCIE_XILINX_NWL
> config PCI_FTPCI100
> bool "Faraday Technology FTPCI100 PCI controller"
> depends on OF
> - depends on ARM
> default ARCH_GEMINI
>
> config PCI_TEGRA
> bool "NVIDIA Tegra PCIe controller"
> - depends on ARCH_TEGRA
> + depends on ARCH_TEGRA || COMPILE_TEST
> depends on PCI_MSI_IRQ_DOMAIN
> help
> Say Y here if you want support for the PCIe host controller found
> @@ -45,8 +44,8 @@ config PCI_TEGRA
>
> config PCI_RCAR_GEN2
> bool "Renesas R-Car Gen2 Internal PCI controller"
> - depends on ARM
> depends on ARCH_RENESAS || COMPILE_TEST
> + depends on ARM
> help
> Say Y here if you want internal PCI support on R-Car Gen2 SoC.
> There are 3 internal PCI controllers available with a single
> @@ -54,7 +53,7 @@ config PCI_RCAR_GEN2
>
> config PCIE_RCAR
> bool "Renesas R-Car PCIe controller"
> - depends on ARCH_RENESAS || (ARM && COMPILE_TEST)
> + depends on ARCH_RENESAS || COMPILE_TEST
> depends on PCI_MSI_IRQ_DOMAIN
> help
> Say Y here if you want PCIe controller support on R-Car SoCs.
> @@ -65,7 +64,7 @@ config PCI_HOST_COMMON
>
> config PCI_HOST_GENERIC
> bool "Generic PCI host controller"
> - depends on (ARM || ARM64) && OF
> + depends on OF
> select PCI_HOST_COMMON
> select IRQ_DOMAIN
> help
> @@ -74,14 +73,14 @@ config PCI_HOST_GENERIC
>
> config PCIE_XILINX
> bool "Xilinx AXI PCIe host bridge support"
> - depends on ARCH_ZYNQ || MICROBLAZE || (MIPS && PCI_DRIVERS_GENERIC)
> + depends on ARCH_ZYNQ || MICROBLAZE || (MIPS && PCI_DRIVERS_GENERIC) || COMPILE_TEST
> help
> Say 'Y' here if you want kernel to support the Xilinx AXI PCIe
> Host Bridge driver.
>
> config PCI_XGENE
> bool "X-Gene PCIe controller"
> - depends on ARM64
> + depends on ARM64 || COMPILE_TEST
> depends on OF || (ACPI && PCI_QUIRKS)
> select PCIEPORTBUS
> help
> @@ -101,7 +100,7 @@ config PCI_XGENE_MSI
> config PCI_V3_SEMI
> bool "V3 Semiconductor PCI controller"
> depends on OF
> - depends on ARM
> + depends on ARM || COMPILE_TEST
> default ARCH_INTEGRATOR_AP
>
> config PCI_VERSATILE
> @@ -147,8 +146,7 @@ config PCIE_IPROC_MSI
>
> config PCIE_ALTERA
> bool "Altera PCIe controller"
> - depends on ARM || NIOS2
> - depends on OF_PCI
> + depends on ARM || NIOS2 || COMPILE_TEST
> select PCI_DOMAINS
> help
> Say Y here if you want to enable PCIe controller support on Altera
> @@ -164,7 +162,7 @@ config PCIE_ALTERA_MSI
>
> config PCI_HOST_THUNDER_PEM
> bool "Cavium Thunder PCIe controller to off-chip devices"
> - depends on ARM64
> + depends on ARM64 || COMPILE_TEST
> depends on OF || (ACPI && PCI_QUIRKS)
> select PCI_HOST_COMMON
> help
> @@ -172,7 +170,7 @@ config PCI_HOST_THUNDER_PEM
>
> config PCI_HOST_THUNDER_ECAM
> bool "Cavium Thunder ECAM controller to on-chip devices on pass-1.x silicon"
> - depends on ARM64
> + depends on ARM64 || COMPILE_TEST
> depends on OF || (ACPI && PCI_QUIRKS)
> select PCI_HOST_COMMON
> help
> @@ -191,9 +189,8 @@ config PCIE_ROCKCHIP
>
> config PCIE_MEDIATEK
> bool "MediaTek PCIe controller"
> - depends on (ARM || ARM64) && (ARCH_MEDIATEK || COMPILE_TEST)
> + depends on ARCH_MEDIATEK || COMPILE_TEST
> depends on OF
> - depends on PCI
> select PCIEPORTBUS
> help
> Say Y here if you want to enable PCIe controller support on
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [v4] PCI: improve host drivers compile test coverage
2018-06-12 17:02 ` [v4] " Guenter Roeck
@ 2018-06-13 10:09 ` Lorenzo Pieralisi
2018-06-13 18:11 ` Guenter Roeck
0 siblings, 1 reply; 17+ messages in thread
From: Lorenzo Pieralisi @ 2018-06-13 10:09 UTC (permalink / raw)
To: Guenter Roeck
Cc: Rob Herring, Bjorn Helgaas, Scott Branden, linux-pci, Jan Kiszka,
Ley Foon Tan, Russell King
[+Jan, Ley Foon, RMK]
On Tue, Jun 12, 2018 at 10:02:29AM -0700, Guenter Roeck wrote:
> On Thu, Apr 05, 2018 at 02:31:54PM -0500, Rob Herring wrote:
> > Add COMPILE_TEST on driver config options with it. Some ARM drivers
> > still have arch dependencies, so we have to keep those dependent on ARM.
> >
> > Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> > Cc: Bjorn Helgaas <bhelgaas@google.com>
> > Cc: linux-pci@vger.kernel.org
> > Signed-off-by: Rob Herring <robh@kernel.org>
>
> This patch has the undesirable side effect that it selects PCI_DOMAINS for
> sparc32:allmodconfig, which in turn results in
>
> 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'?
>
> Unfortunately, 37bd62d224c82 ("PCI: Enable PCI_DOMAINS along with generic
> PCI host controller") has pretty much the same result. No idea how to fix
> the problem, so I won't even try.
Sorry about that.
One option would consist in removing all PCI_DOMAINS selection from
drivers/pci/controller/Kconfig and delegate it to arches even though
this would force PCI_DOMAINS selection on all ARM platforms (it is
already selected for ARCH_MULTIPLATFORM).
Or we add back arch dependency to the relevant host bridges.
Everything else I have in mind seems overkill to me given that this
patch was added to improve test coverage (we could add a default
pci_domain_nr() stub - weak or #define - that returns 0 in case arches
do not provide an implementation but do we really want to do that ?).
Thoughts appreciated.
Lorenzo
> Guenter
>
> > ---
> > v4: Drop iProc hunks
> >
> > drivers/pci/dwc/Kconfig | 24 ++++++++++++------------
> > drivers/pci/host/Kconfig | 31 ++++++++++++++-----------------
> > 2 files changed, 26 insertions(+), 29 deletions(-)
> >
> > diff --git a/drivers/pci/dwc/Kconfig b/drivers/pci/dwc/Kconfig
> > index 8c1a5167fb5b..2902544f5834 100644
> > --- a/drivers/pci/dwc/Kconfig
> > +++ b/drivers/pci/dwc/Kconfig
> > @@ -64,21 +64,21 @@ config PCIE_DW_PLAT
> >
> > config PCI_EXYNOS
> > bool "Samsung Exynos PCIe controller"
> > - depends on SOC_EXYNOS5440
> > + depends on SOC_EXYNOS5440 || COMPILE_TEST
> > depends on PCI_MSI_IRQ_DOMAIN
> > select PCIEPORTBUS
> > select PCIE_DW_HOST
> >
> > config PCI_IMX6
> > bool "Freescale i.MX6 PCIe controller"
> > - depends on SOC_IMX6Q
> > + depends on SOC_IMX6Q || (ARM && COMPILE_TEST)
> > depends on PCI_MSI_IRQ_DOMAIN
> > select PCIEPORTBUS
> > select PCIE_DW_HOST
> >
> > config PCIE_SPEAR13XX
> > bool "STMicroelectronics SPEAr PCIe controller"
> > - depends on ARCH_SPEAR13XX
> > + depends on ARCH_SPEAR13XX || COMPILE_TEST
> > depends on PCI_MSI_IRQ_DOMAIN
> > select PCIEPORTBUS
> > select PCIE_DW_HOST
> > @@ -87,7 +87,7 @@ config PCIE_SPEAR13XX
> >
> > config PCI_KEYSTONE
> > bool "TI Keystone PCIe controller"
> > - depends on ARCH_KEYSTONE
> > + depends on ARCH_KEYSTONE || (ARM && COMPILE_TEST)
> > depends on PCI_MSI_IRQ_DOMAIN
> > select PCIEPORTBUS
> > select PCIE_DW_HOST
> > @@ -99,7 +99,7 @@ config PCI_KEYSTONE
> >
> > config PCI_LAYERSCAPE
> > bool "Freescale Layerscape PCIe controller"
> > - depends on OF && (ARM || ARCH_LAYERSCAPE)
> > + depends on OF && (ARM || ARCH_LAYERSCAPE || COMPILE_TEST)
> > depends on PCI_MSI_IRQ_DOMAIN
> > select MFD_SYSCON
> > select PCIE_DW_HOST
> > @@ -107,7 +107,7 @@ config PCI_LAYERSCAPE
> > Say Y here if you want PCIe controller support on Layerscape SoCs.
> >
> > config PCI_HISI
> > - depends on OF && ARM64
> > + depends on OF && (ARM64 || COMPILE_TEST)
> > bool "HiSilicon Hip05 and Hip06 SoCs PCIe controllers"
> > depends on PCI_MSI_IRQ_DOMAIN
> > select PCIEPORTBUS
> > @@ -119,7 +119,7 @@ config PCI_HISI
> >
> > config PCIE_QCOM
> > bool "Qualcomm PCIe controller"
> > - depends on ARCH_QCOM && OF
> > + depends on OF && (ARCH_QCOM || COMPILE_TEST)
> > depends on PCI_MSI_IRQ_DOMAIN
> > select PCIEPORTBUS
> > select PCIE_DW_HOST
> > @@ -130,7 +130,7 @@ config PCIE_QCOM
> >
> > config PCIE_ARMADA_8K
> > bool "Marvell Armada-8K PCIe controller"
> > - depends on ARCH_MVEBU
> > + depends on ARCH_MVEBU || COMPILE_TEST
> > depends on PCI_MSI_IRQ_DOMAIN
> > select PCIEPORTBUS
> > select PCIE_DW_HOST
> > @@ -145,7 +145,7 @@ config PCIE_ARTPEC6
> >
> > config PCIE_ARTPEC6_HOST
> > bool "Axis ARTPEC-6 PCIe controller Host Mode"
> > - depends on MACH_ARTPEC6
> > + depends on MACH_ARTPEC6 || COMPILE_TEST
> > depends on PCI_MSI_IRQ_DOMAIN
> > select PCIEPORTBUS
> > select PCIE_DW_HOST
> > @@ -156,7 +156,7 @@ config PCIE_ARTPEC6_HOST
> >
> > config PCIE_ARTPEC6_EP
> > bool "Axis ARTPEC-6 PCIe controller Endpoint Mode"
> > - depends on MACH_ARTPEC6
> > + depends on MACH_ARTPEC6 || COMPILE_TEST
> > depends on PCI_ENDPOINT
> > select PCIE_DW_EP
> > select PCIE_ARTPEC6
> > @@ -165,7 +165,7 @@ config PCIE_ARTPEC6_EP
> > endpoint mode. This uses the DesignWare core.
> >
> > config PCIE_KIRIN
> > - depends on OF && ARM64
> > + depends on OF && (ARM64 || COMPILE_TEST)
> > bool "HiSilicon Kirin series SoCs PCIe controllers"
> > depends on PCI_MSI_IRQ_DOMAIN
> > select PCIEPORTBUS
> > @@ -176,7 +176,7 @@ config PCIE_KIRIN
> >
> > config PCIE_HISI_STB
> > bool "HiSilicon STB SoCs PCIe controllers"
> > - depends on ARCH_HISI
> > + depends on ARCH_HISI || COMPILE_TEST
> > depends on PCI_MSI_IRQ_DOMAIN
> > select PCIEPORTBUS
> > select PCIE_DW_HOST
> > diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
> > index 0d0177ce436c..3d180b594ff2 100644
> > --- a/drivers/pci/host/Kconfig
> > +++ b/drivers/pci/host/Kconfig
> > @@ -5,13 +5,13 @@ menu "PCI host controller drivers"
> >
> > config PCI_MVEBU
> > bool "Marvell EBU PCIe controller"
> > - depends on ARCH_MVEBU || ARCH_DOVE
> > + depends on ARCH_MVEBU || ARCH_DOVE || COMPILE_TEST
> > depends on ARM
> > depends on OF
> >
> > config PCI_AARDVARK
> > bool "Aardvark PCIe controller"
> > - depends on ARCH_MVEBU && ARM64
> > + depends on (ARCH_MVEBU && ARM64) || COMPILE_TEST
> > depends on OF
> > depends on PCI_MSI_IRQ_DOMAIN
> > help
> > @@ -21,7 +21,7 @@ config PCI_AARDVARK
> >
> > config PCIE_XILINX_NWL
> > bool "NWL PCIe Core"
> > - depends on ARCH_ZYNQMP
> > + depends on ARCH_ZYNQMP || COMPILE_TEST
> > depends on PCI_MSI_IRQ_DOMAIN
> > help
> > Say 'Y' here if you want kernel support for Xilinx
> > @@ -32,12 +32,11 @@ config PCIE_XILINX_NWL
> > config PCI_FTPCI100
> > bool "Faraday Technology FTPCI100 PCI controller"
> > depends on OF
> > - depends on ARM
> > default ARCH_GEMINI
> >
> > config PCI_TEGRA
> > bool "NVIDIA Tegra PCIe controller"
> > - depends on ARCH_TEGRA
> > + depends on ARCH_TEGRA || COMPILE_TEST
> > depends on PCI_MSI_IRQ_DOMAIN
> > help
> > Say Y here if you want support for the PCIe host controller found
> > @@ -45,8 +44,8 @@ config PCI_TEGRA
> >
> > config PCI_RCAR_GEN2
> > bool "Renesas R-Car Gen2 Internal PCI controller"
> > - depends on ARM
> > depends on ARCH_RENESAS || COMPILE_TEST
> > + depends on ARM
> > help
> > Say Y here if you want internal PCI support on R-Car Gen2 SoC.
> > There are 3 internal PCI controllers available with a single
> > @@ -54,7 +53,7 @@ config PCI_RCAR_GEN2
> >
> > config PCIE_RCAR
> > bool "Renesas R-Car PCIe controller"
> > - depends on ARCH_RENESAS || (ARM && COMPILE_TEST)
> > + depends on ARCH_RENESAS || COMPILE_TEST
> > depends on PCI_MSI_IRQ_DOMAIN
> > help
> > Say Y here if you want PCIe controller support on R-Car SoCs.
> > @@ -65,7 +64,7 @@ config PCI_HOST_COMMON
> >
> > config PCI_HOST_GENERIC
> > bool "Generic PCI host controller"
> > - depends on (ARM || ARM64) && OF
> > + depends on OF
> > select PCI_HOST_COMMON
> > select IRQ_DOMAIN
> > help
> > @@ -74,14 +73,14 @@ config PCI_HOST_GENERIC
> >
> > config PCIE_XILINX
> > bool "Xilinx AXI PCIe host bridge support"
> > - depends on ARCH_ZYNQ || MICROBLAZE || (MIPS && PCI_DRIVERS_GENERIC)
> > + depends on ARCH_ZYNQ || MICROBLAZE || (MIPS && PCI_DRIVERS_GENERIC) || COMPILE_TEST
> > help
> > Say 'Y' here if you want kernel to support the Xilinx AXI PCIe
> > Host Bridge driver.
> >
> > config PCI_XGENE
> > bool "X-Gene PCIe controller"
> > - depends on ARM64
> > + depends on ARM64 || COMPILE_TEST
> > depends on OF || (ACPI && PCI_QUIRKS)
> > select PCIEPORTBUS
> > help
> > @@ -101,7 +100,7 @@ config PCI_XGENE_MSI
> > config PCI_V3_SEMI
> > bool "V3 Semiconductor PCI controller"
> > depends on OF
> > - depends on ARM
> > + depends on ARM || COMPILE_TEST
> > default ARCH_INTEGRATOR_AP
> >
> > config PCI_VERSATILE
> > @@ -147,8 +146,7 @@ config PCIE_IPROC_MSI
> >
> > config PCIE_ALTERA
> > bool "Altera PCIe controller"
> > - depends on ARM || NIOS2
> > - depends on OF_PCI
> > + depends on ARM || NIOS2 || COMPILE_TEST
> > select PCI_DOMAINS
> > help
> > Say Y here if you want to enable PCIe controller support on Altera
> > @@ -164,7 +162,7 @@ config PCIE_ALTERA_MSI
> >
> > config PCI_HOST_THUNDER_PEM
> > bool "Cavium Thunder PCIe controller to off-chip devices"
> > - depends on ARM64
> > + depends on ARM64 || COMPILE_TEST
> > depends on OF || (ACPI && PCI_QUIRKS)
> > select PCI_HOST_COMMON
> > help
> > @@ -172,7 +170,7 @@ config PCI_HOST_THUNDER_PEM
> >
> > config PCI_HOST_THUNDER_ECAM
> > bool "Cavium Thunder ECAM controller to on-chip devices on pass-1.x silicon"
> > - depends on ARM64
> > + depends on ARM64 || COMPILE_TEST
> > depends on OF || (ACPI && PCI_QUIRKS)
> > select PCI_HOST_COMMON
> > help
> > @@ -191,9 +189,8 @@ config PCIE_ROCKCHIP
> >
> > config PCIE_MEDIATEK
> > bool "MediaTek PCIe controller"
> > - depends on (ARM || ARM64) && (ARCH_MEDIATEK || COMPILE_TEST)
> > + depends on ARCH_MEDIATEK || COMPILE_TEST
> > depends on OF
> > - depends on PCI
> > select PCIEPORTBUS
> > help
> > Say Y here if you want to enable PCIe controller support on
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [v4] PCI: improve host drivers compile test coverage
2018-06-13 10:09 ` Lorenzo Pieralisi
@ 2018-06-13 18:11 ` Guenter Roeck
2018-06-14 16:53 ` Lorenzo Pieralisi
2018-06-15 12:58 ` Lorenzo Pieralisi
0 siblings, 2 replies; 17+ messages in thread
From: Guenter Roeck @ 2018-06-13 18:11 UTC (permalink / raw)
To: Lorenzo Pieralisi
Cc: Rob Herring, Bjorn Helgaas, Scott Branden, linux-pci, Jan Kiszka,
Ley Foon Tan, Russell King
On Wed, Jun 13, 2018 at 11:09:35AM +0100, Lorenzo Pieralisi wrote:
> [+Jan, Ley Foon, RMK]
>
> On Tue, Jun 12, 2018 at 10:02:29AM -0700, Guenter Roeck wrote:
> > On Thu, Apr 05, 2018 at 02:31:54PM -0500, Rob Herring wrote:
> > > Add COMPILE_TEST on driver config options with it. Some ARM drivers
> > > still have arch dependencies, so we have to keep those dependent on ARM.
> > >
> > > Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> > > Cc: Bjorn Helgaas <bhelgaas@google.com>
> > > Cc: linux-pci@vger.kernel.org
> > > Signed-off-by: Rob Herring <robh@kernel.org>
> >
> > This patch has the undesirable side effect that it selects PCI_DOMAINS for
> > sparc32:allmodconfig, which in turn results in
> >
> > 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'?
> >
> > Unfortunately, 37bd62d224c82 ("PCI: Enable PCI_DOMAINS along with generic
> > PCI host controller") has pretty much the same result. No idea how to fix
> > the problem, so I won't even try.
>
> Sorry about that.
>
> One option would consist in removing all PCI_DOMAINS selection from
> drivers/pci/controller/Kconfig and delegate it to arches even though
> this would force PCI_DOMAINS selection on all ARM platforms (it is
> already selected for ARCH_MULTIPLATFORM).
>
> Or we add back arch dependency to the relevant host bridges.
>
> Everything else I have in mind seems overkill to me given that this
> patch was added to improve test coverage (we could add a default
> pci_domain_nr() stub - weak or #define - that returns 0 in case arches
> do not provide an implementation but do we really want to do that ?).
>
> Thoughts appreciated.
>
>From the definition of PCI_DOMAINS, I suspect the original idea was that
drivers should depend on it, not select it. Especially auto-selecting
it with PCI_HOST_GENERIC seems like a bad idea to me. However, that is
just me. I'll leave it up to Bjorn to decide what if anything he wants
to do about it.
Guenter
> Lorenzo
>
> > Guenter
> >
> > > ---
> > > v4: Drop iProc hunks
> > >
> > > drivers/pci/dwc/Kconfig | 24 ++++++++++++------------
> > > drivers/pci/host/Kconfig | 31 ++++++++++++++-----------------
> > > 2 files changed, 26 insertions(+), 29 deletions(-)
> > >
> > > diff --git a/drivers/pci/dwc/Kconfig b/drivers/pci/dwc/Kconfig
> > > index 8c1a5167fb5b..2902544f5834 100644
> > > --- a/drivers/pci/dwc/Kconfig
> > > +++ b/drivers/pci/dwc/Kconfig
> > > @@ -64,21 +64,21 @@ config PCIE_DW_PLAT
> > >
> > > config PCI_EXYNOS
> > > bool "Samsung Exynos PCIe controller"
> > > - depends on SOC_EXYNOS5440
> > > + depends on SOC_EXYNOS5440 || COMPILE_TEST
> > > depends on PCI_MSI_IRQ_DOMAIN
> > > select PCIEPORTBUS
> > > select PCIE_DW_HOST
> > >
> > > config PCI_IMX6
> > > bool "Freescale i.MX6 PCIe controller"
> > > - depends on SOC_IMX6Q
> > > + depends on SOC_IMX6Q || (ARM && COMPILE_TEST)
> > > depends on PCI_MSI_IRQ_DOMAIN
> > > select PCIEPORTBUS
> > > select PCIE_DW_HOST
> > >
> > > config PCIE_SPEAR13XX
> > > bool "STMicroelectronics SPEAr PCIe controller"
> > > - depends on ARCH_SPEAR13XX
> > > + depends on ARCH_SPEAR13XX || COMPILE_TEST
> > > depends on PCI_MSI_IRQ_DOMAIN
> > > select PCIEPORTBUS
> > > select PCIE_DW_HOST
> > > @@ -87,7 +87,7 @@ config PCIE_SPEAR13XX
> > >
> > > config PCI_KEYSTONE
> > > bool "TI Keystone PCIe controller"
> > > - depends on ARCH_KEYSTONE
> > > + depends on ARCH_KEYSTONE || (ARM && COMPILE_TEST)
> > > depends on PCI_MSI_IRQ_DOMAIN
> > > select PCIEPORTBUS
> > > select PCIE_DW_HOST
> > > @@ -99,7 +99,7 @@ config PCI_KEYSTONE
> > >
> > > config PCI_LAYERSCAPE
> > > bool "Freescale Layerscape PCIe controller"
> > > - depends on OF && (ARM || ARCH_LAYERSCAPE)
> > > + depends on OF && (ARM || ARCH_LAYERSCAPE || COMPILE_TEST)
> > > depends on PCI_MSI_IRQ_DOMAIN
> > > select MFD_SYSCON
> > > select PCIE_DW_HOST
> > > @@ -107,7 +107,7 @@ config PCI_LAYERSCAPE
> > > Say Y here if you want PCIe controller support on Layerscape SoCs.
> > >
> > > config PCI_HISI
> > > - depends on OF && ARM64
> > > + depends on OF && (ARM64 || COMPILE_TEST)
> > > bool "HiSilicon Hip05 and Hip06 SoCs PCIe controllers"
> > > depends on PCI_MSI_IRQ_DOMAIN
> > > select PCIEPORTBUS
> > > @@ -119,7 +119,7 @@ config PCI_HISI
> > >
> > > config PCIE_QCOM
> > > bool "Qualcomm PCIe controller"
> > > - depends on ARCH_QCOM && OF
> > > + depends on OF && (ARCH_QCOM || COMPILE_TEST)
> > > depends on PCI_MSI_IRQ_DOMAIN
> > > select PCIEPORTBUS
> > > select PCIE_DW_HOST
> > > @@ -130,7 +130,7 @@ config PCIE_QCOM
> > >
> > > config PCIE_ARMADA_8K
> > > bool "Marvell Armada-8K PCIe controller"
> > > - depends on ARCH_MVEBU
> > > + depends on ARCH_MVEBU || COMPILE_TEST
> > > depends on PCI_MSI_IRQ_DOMAIN
> > > select PCIEPORTBUS
> > > select PCIE_DW_HOST
> > > @@ -145,7 +145,7 @@ config PCIE_ARTPEC6
> > >
> > > config PCIE_ARTPEC6_HOST
> > > bool "Axis ARTPEC-6 PCIe controller Host Mode"
> > > - depends on MACH_ARTPEC6
> > > + depends on MACH_ARTPEC6 || COMPILE_TEST
> > > depends on PCI_MSI_IRQ_DOMAIN
> > > select PCIEPORTBUS
> > > select PCIE_DW_HOST
> > > @@ -156,7 +156,7 @@ config PCIE_ARTPEC6_HOST
> > >
> > > config PCIE_ARTPEC6_EP
> > > bool "Axis ARTPEC-6 PCIe controller Endpoint Mode"
> > > - depends on MACH_ARTPEC6
> > > + depends on MACH_ARTPEC6 || COMPILE_TEST
> > > depends on PCI_ENDPOINT
> > > select PCIE_DW_EP
> > > select PCIE_ARTPEC6
> > > @@ -165,7 +165,7 @@ config PCIE_ARTPEC6_EP
> > > endpoint mode. This uses the DesignWare core.
> > >
> > > config PCIE_KIRIN
> > > - depends on OF && ARM64
> > > + depends on OF && (ARM64 || COMPILE_TEST)
> > > bool "HiSilicon Kirin series SoCs PCIe controllers"
> > > depends on PCI_MSI_IRQ_DOMAIN
> > > select PCIEPORTBUS
> > > @@ -176,7 +176,7 @@ config PCIE_KIRIN
> > >
> > > config PCIE_HISI_STB
> > > bool "HiSilicon STB SoCs PCIe controllers"
> > > - depends on ARCH_HISI
> > > + depends on ARCH_HISI || COMPILE_TEST
> > > depends on PCI_MSI_IRQ_DOMAIN
> > > select PCIEPORTBUS
> > > select PCIE_DW_HOST
> > > diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
> > > index 0d0177ce436c..3d180b594ff2 100644
> > > --- a/drivers/pci/host/Kconfig
> > > +++ b/drivers/pci/host/Kconfig
> > > @@ -5,13 +5,13 @@ menu "PCI host controller drivers"
> > >
> > > config PCI_MVEBU
> > > bool "Marvell EBU PCIe controller"
> > > - depends on ARCH_MVEBU || ARCH_DOVE
> > > + depends on ARCH_MVEBU || ARCH_DOVE || COMPILE_TEST
> > > depends on ARM
> > > depends on OF
> > >
> > > config PCI_AARDVARK
> > > bool "Aardvark PCIe controller"
> > > - depends on ARCH_MVEBU && ARM64
> > > + depends on (ARCH_MVEBU && ARM64) || COMPILE_TEST
> > > depends on OF
> > > depends on PCI_MSI_IRQ_DOMAIN
> > > help
> > > @@ -21,7 +21,7 @@ config PCI_AARDVARK
> > >
> > > config PCIE_XILINX_NWL
> > > bool "NWL PCIe Core"
> > > - depends on ARCH_ZYNQMP
> > > + depends on ARCH_ZYNQMP || COMPILE_TEST
> > > depends on PCI_MSI_IRQ_DOMAIN
> > > help
> > > Say 'Y' here if you want kernel support for Xilinx
> > > @@ -32,12 +32,11 @@ config PCIE_XILINX_NWL
> > > config PCI_FTPCI100
> > > bool "Faraday Technology FTPCI100 PCI controller"
> > > depends on OF
> > > - depends on ARM
> > > default ARCH_GEMINI
> > >
> > > config PCI_TEGRA
> > > bool "NVIDIA Tegra PCIe controller"
> > > - depends on ARCH_TEGRA
> > > + depends on ARCH_TEGRA || COMPILE_TEST
> > > depends on PCI_MSI_IRQ_DOMAIN
> > > help
> > > Say Y here if you want support for the PCIe host controller found
> > > @@ -45,8 +44,8 @@ config PCI_TEGRA
> > >
> > > config PCI_RCAR_GEN2
> > > bool "Renesas R-Car Gen2 Internal PCI controller"
> > > - depends on ARM
> > > depends on ARCH_RENESAS || COMPILE_TEST
> > > + depends on ARM
> > > help
> > > Say Y here if you want internal PCI support on R-Car Gen2 SoC.
> > > There are 3 internal PCI controllers available with a single
> > > @@ -54,7 +53,7 @@ config PCI_RCAR_GEN2
> > >
> > > config PCIE_RCAR
> > > bool "Renesas R-Car PCIe controller"
> > > - depends on ARCH_RENESAS || (ARM && COMPILE_TEST)
> > > + depends on ARCH_RENESAS || COMPILE_TEST
> > > depends on PCI_MSI_IRQ_DOMAIN
> > > help
> > > Say Y here if you want PCIe controller support on R-Car SoCs.
> > > @@ -65,7 +64,7 @@ config PCI_HOST_COMMON
> > >
> > > config PCI_HOST_GENERIC
> > > bool "Generic PCI host controller"
> > > - depends on (ARM || ARM64) && OF
> > > + depends on OF
> > > select PCI_HOST_COMMON
> > > select IRQ_DOMAIN
> > > help
> > > @@ -74,14 +73,14 @@ config PCI_HOST_GENERIC
> > >
> > > config PCIE_XILINX
> > > bool "Xilinx AXI PCIe host bridge support"
> > > - depends on ARCH_ZYNQ || MICROBLAZE || (MIPS && PCI_DRIVERS_GENERIC)
> > > + depends on ARCH_ZYNQ || MICROBLAZE || (MIPS && PCI_DRIVERS_GENERIC) || COMPILE_TEST
> > > help
> > > Say 'Y' here if you want kernel to support the Xilinx AXI PCIe
> > > Host Bridge driver.
> > >
> > > config PCI_XGENE
> > > bool "X-Gene PCIe controller"
> > > - depends on ARM64
> > > + depends on ARM64 || COMPILE_TEST
> > > depends on OF || (ACPI && PCI_QUIRKS)
> > > select PCIEPORTBUS
> > > help
> > > @@ -101,7 +100,7 @@ config PCI_XGENE_MSI
> > > config PCI_V3_SEMI
> > > bool "V3 Semiconductor PCI controller"
> > > depends on OF
> > > - depends on ARM
> > > + depends on ARM || COMPILE_TEST
> > > default ARCH_INTEGRATOR_AP
> > >
> > > config PCI_VERSATILE
> > > @@ -147,8 +146,7 @@ config PCIE_IPROC_MSI
> > >
> > > config PCIE_ALTERA
> > > bool "Altera PCIe controller"
> > > - depends on ARM || NIOS2
> > > - depends on OF_PCI
> > > + depends on ARM || NIOS2 || COMPILE_TEST
> > > select PCI_DOMAINS
> > > help
> > > Say Y here if you want to enable PCIe controller support on Altera
> > > @@ -164,7 +162,7 @@ config PCIE_ALTERA_MSI
> > >
> > > config PCI_HOST_THUNDER_PEM
> > > bool "Cavium Thunder PCIe controller to off-chip devices"
> > > - depends on ARM64
> > > + depends on ARM64 || COMPILE_TEST
> > > depends on OF || (ACPI && PCI_QUIRKS)
> > > select PCI_HOST_COMMON
> > > help
> > > @@ -172,7 +170,7 @@ config PCI_HOST_THUNDER_PEM
> > >
> > > config PCI_HOST_THUNDER_ECAM
> > > bool "Cavium Thunder ECAM controller to on-chip devices on pass-1.x silicon"
> > > - depends on ARM64
> > > + depends on ARM64 || COMPILE_TEST
> > > depends on OF || (ACPI && PCI_QUIRKS)
> > > select PCI_HOST_COMMON
> > > help
> > > @@ -191,9 +189,8 @@ config PCIE_ROCKCHIP
> > >
> > > config PCIE_MEDIATEK
> > > bool "MediaTek PCIe controller"
> > > - depends on (ARM || ARM64) && (ARCH_MEDIATEK || COMPILE_TEST)
> > > + depends on ARCH_MEDIATEK || COMPILE_TEST
> > > depends on OF
> > > - depends on PCI
> > > select PCIEPORTBUS
> > > help
> > > Say Y here if you want to enable PCIe controller support on
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [v4] PCI: improve host drivers compile test coverage
2018-06-13 18:11 ` Guenter Roeck
@ 2018-06-14 16:53 ` Lorenzo Pieralisi
2018-06-15 12:58 ` Lorenzo Pieralisi
1 sibling, 0 replies; 17+ messages in thread
From: Lorenzo Pieralisi @ 2018-06-14 16:53 UTC (permalink / raw)
To: Guenter Roeck
Cc: Rob Herring, Bjorn Helgaas, Scott Branden, linux-pci, Jan Kiszka,
Ley Foon Tan, Russell King
On Wed, Jun 13, 2018 at 11:11:46AM -0700, Guenter Roeck wrote:
> On Wed, Jun 13, 2018 at 11:09:35AM +0100, Lorenzo Pieralisi wrote:
> > [+Jan, Ley Foon, RMK]
> >
> > On Tue, Jun 12, 2018 at 10:02:29AM -0700, Guenter Roeck wrote:
> > > On Thu, Apr 05, 2018 at 02:31:54PM -0500, Rob Herring wrote:
> > > > Add COMPILE_TEST on driver config options with it. Some ARM drivers
> > > > still have arch dependencies, so we have to keep those dependent on ARM.
> > > >
> > > > Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> > > > Cc: Bjorn Helgaas <bhelgaas@google.com>
> > > > Cc: linux-pci@vger.kernel.org
> > > > Signed-off-by: Rob Herring <robh@kernel.org>
> > >
> > > This patch has the undesirable side effect that it selects PCI_DOMAINS for
> > > sparc32:allmodconfig, which in turn results in
> > >
> > > 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'?
> > >
> > > Unfortunately, 37bd62d224c82 ("PCI: Enable PCI_DOMAINS along with generic
> > > PCI host controller") has pretty much the same result. No idea how to fix
> > > the problem, so I won't even try.
> >
> > Sorry about that.
> >
> > One option would consist in removing all PCI_DOMAINS selection from
> > drivers/pci/controller/Kconfig and delegate it to arches even though
> > this would force PCI_DOMAINS selection on all ARM platforms (it is
> > already selected for ARCH_MULTIPLATFORM).
> >
> > Or we add back arch dependency to the relevant host bridges.
> >
> > Everything else I have in mind seems overkill to me given that this
> > patch was added to improve test coverage (we could add a default
> > pci_domain_nr() stub - weak or #define - that returns 0 in case arches
> > do not provide an implementation but do we really want to do that ?).
> >
> > Thoughts appreciated.
> >
>
> From the definition of PCI_DOMAINS, I suspect the original idea was that
> drivers should depend on it, not select it.
I do not think that's the case, a host controller should not really
depend on it.
> Especially auto-selecting it with PCI_HOST_GENERIC seems like a bad
> idea to me. However, that is just me. I'll leave it up to Bjorn to
> decide what if anything he wants to do about it.
I think that on ARM (32-bit) systems it is supposed to be selected
so that it is not pulled in if not "needed". As I said I could make
it:
def_bool PCI
on ARM but my worry is that it has consequences on legacy ARM PCI host
drivers in arch/arm, I really do not know what happens if I enable
it by default.
Another option would consist in making PCI_DOMAINS a visible menu
option on ARM, so that it can be configured in the kernel if needed,
in that case I can safely remove all selection from
drivers/pci/controller/Kconfig; I am not a big fan of this solution
either but I think that's the least evil.
I do not think there is any other quick option available.
Lorenzo
>
> Guenter
>
> > Lorenzo
> >
> > > Guenter
> > >
> > > > ---
> > > > v4: Drop iProc hunks
> > > >
> > > > drivers/pci/dwc/Kconfig | 24 ++++++++++++------------
> > > > drivers/pci/host/Kconfig | 31 ++++++++++++++-----------------
> > > > 2 files changed, 26 insertions(+), 29 deletions(-)
> > > >
> > > > diff --git a/drivers/pci/dwc/Kconfig b/drivers/pci/dwc/Kconfig
> > > > index 8c1a5167fb5b..2902544f5834 100644
> > > > --- a/drivers/pci/dwc/Kconfig
> > > > +++ b/drivers/pci/dwc/Kconfig
> > > > @@ -64,21 +64,21 @@ config PCIE_DW_PLAT
> > > >
> > > > config PCI_EXYNOS
> > > > bool "Samsung Exynos PCIe controller"
> > > > - depends on SOC_EXYNOS5440
> > > > + depends on SOC_EXYNOS5440 || COMPILE_TEST
> > > > depends on PCI_MSI_IRQ_DOMAIN
> > > > select PCIEPORTBUS
> > > > select PCIE_DW_HOST
> > > >
> > > > config PCI_IMX6
> > > > bool "Freescale i.MX6 PCIe controller"
> > > > - depends on SOC_IMX6Q
> > > > + depends on SOC_IMX6Q || (ARM && COMPILE_TEST)
> > > > depends on PCI_MSI_IRQ_DOMAIN
> > > > select PCIEPORTBUS
> > > > select PCIE_DW_HOST
> > > >
> > > > config PCIE_SPEAR13XX
> > > > bool "STMicroelectronics SPEAr PCIe controller"
> > > > - depends on ARCH_SPEAR13XX
> > > > + depends on ARCH_SPEAR13XX || COMPILE_TEST
> > > > depends on PCI_MSI_IRQ_DOMAIN
> > > > select PCIEPORTBUS
> > > > select PCIE_DW_HOST
> > > > @@ -87,7 +87,7 @@ config PCIE_SPEAR13XX
> > > >
> > > > config PCI_KEYSTONE
> > > > bool "TI Keystone PCIe controller"
> > > > - depends on ARCH_KEYSTONE
> > > > + depends on ARCH_KEYSTONE || (ARM && COMPILE_TEST)
> > > > depends on PCI_MSI_IRQ_DOMAIN
> > > > select PCIEPORTBUS
> > > > select PCIE_DW_HOST
> > > > @@ -99,7 +99,7 @@ config PCI_KEYSTONE
> > > >
> > > > config PCI_LAYERSCAPE
> > > > bool "Freescale Layerscape PCIe controller"
> > > > - depends on OF && (ARM || ARCH_LAYERSCAPE)
> > > > + depends on OF && (ARM || ARCH_LAYERSCAPE || COMPILE_TEST)
> > > > depends on PCI_MSI_IRQ_DOMAIN
> > > > select MFD_SYSCON
> > > > select PCIE_DW_HOST
> > > > @@ -107,7 +107,7 @@ config PCI_LAYERSCAPE
> > > > Say Y here if you want PCIe controller support on Layerscape SoCs.
> > > >
> > > > config PCI_HISI
> > > > - depends on OF && ARM64
> > > > + depends on OF && (ARM64 || COMPILE_TEST)
> > > > bool "HiSilicon Hip05 and Hip06 SoCs PCIe controllers"
> > > > depends on PCI_MSI_IRQ_DOMAIN
> > > > select PCIEPORTBUS
> > > > @@ -119,7 +119,7 @@ config PCI_HISI
> > > >
> > > > config PCIE_QCOM
> > > > bool "Qualcomm PCIe controller"
> > > > - depends on ARCH_QCOM && OF
> > > > + depends on OF && (ARCH_QCOM || COMPILE_TEST)
> > > > depends on PCI_MSI_IRQ_DOMAIN
> > > > select PCIEPORTBUS
> > > > select PCIE_DW_HOST
> > > > @@ -130,7 +130,7 @@ config PCIE_QCOM
> > > >
> > > > config PCIE_ARMADA_8K
> > > > bool "Marvell Armada-8K PCIe controller"
> > > > - depends on ARCH_MVEBU
> > > > + depends on ARCH_MVEBU || COMPILE_TEST
> > > > depends on PCI_MSI_IRQ_DOMAIN
> > > > select PCIEPORTBUS
> > > > select PCIE_DW_HOST
> > > > @@ -145,7 +145,7 @@ config PCIE_ARTPEC6
> > > >
> > > > config PCIE_ARTPEC6_HOST
> > > > bool "Axis ARTPEC-6 PCIe controller Host Mode"
> > > > - depends on MACH_ARTPEC6
> > > > + depends on MACH_ARTPEC6 || COMPILE_TEST
> > > > depends on PCI_MSI_IRQ_DOMAIN
> > > > select PCIEPORTBUS
> > > > select PCIE_DW_HOST
> > > > @@ -156,7 +156,7 @@ config PCIE_ARTPEC6_HOST
> > > >
> > > > config PCIE_ARTPEC6_EP
> > > > bool "Axis ARTPEC-6 PCIe controller Endpoint Mode"
> > > > - depends on MACH_ARTPEC6
> > > > + depends on MACH_ARTPEC6 || COMPILE_TEST
> > > > depends on PCI_ENDPOINT
> > > > select PCIE_DW_EP
> > > > select PCIE_ARTPEC6
> > > > @@ -165,7 +165,7 @@ config PCIE_ARTPEC6_EP
> > > > endpoint mode. This uses the DesignWare core.
> > > >
> > > > config PCIE_KIRIN
> > > > - depends on OF && ARM64
> > > > + depends on OF && (ARM64 || COMPILE_TEST)
> > > > bool "HiSilicon Kirin series SoCs PCIe controllers"
> > > > depends on PCI_MSI_IRQ_DOMAIN
> > > > select PCIEPORTBUS
> > > > @@ -176,7 +176,7 @@ config PCIE_KIRIN
> > > >
> > > > config PCIE_HISI_STB
> > > > bool "HiSilicon STB SoCs PCIe controllers"
> > > > - depends on ARCH_HISI
> > > > + depends on ARCH_HISI || COMPILE_TEST
> > > > depends on PCI_MSI_IRQ_DOMAIN
> > > > select PCIEPORTBUS
> > > > select PCIE_DW_HOST
> > > > diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
> > > > index 0d0177ce436c..3d180b594ff2 100644
> > > > --- a/drivers/pci/host/Kconfig
> > > > +++ b/drivers/pci/host/Kconfig
> > > > @@ -5,13 +5,13 @@ menu "PCI host controller drivers"
> > > >
> > > > config PCI_MVEBU
> > > > bool "Marvell EBU PCIe controller"
> > > > - depends on ARCH_MVEBU || ARCH_DOVE
> > > > + depends on ARCH_MVEBU || ARCH_DOVE || COMPILE_TEST
> > > > depends on ARM
> > > > depends on OF
> > > >
> > > > config PCI_AARDVARK
> > > > bool "Aardvark PCIe controller"
> > > > - depends on ARCH_MVEBU && ARM64
> > > > + depends on (ARCH_MVEBU && ARM64) || COMPILE_TEST
> > > > depends on OF
> > > > depends on PCI_MSI_IRQ_DOMAIN
> > > > help
> > > > @@ -21,7 +21,7 @@ config PCI_AARDVARK
> > > >
> > > > config PCIE_XILINX_NWL
> > > > bool "NWL PCIe Core"
> > > > - depends on ARCH_ZYNQMP
> > > > + depends on ARCH_ZYNQMP || COMPILE_TEST
> > > > depends on PCI_MSI_IRQ_DOMAIN
> > > > help
> > > > Say 'Y' here if you want kernel support for Xilinx
> > > > @@ -32,12 +32,11 @@ config PCIE_XILINX_NWL
> > > > config PCI_FTPCI100
> > > > bool "Faraday Technology FTPCI100 PCI controller"
> > > > depends on OF
> > > > - depends on ARM
> > > > default ARCH_GEMINI
> > > >
> > > > config PCI_TEGRA
> > > > bool "NVIDIA Tegra PCIe controller"
> > > > - depends on ARCH_TEGRA
> > > > + depends on ARCH_TEGRA || COMPILE_TEST
> > > > depends on PCI_MSI_IRQ_DOMAIN
> > > > help
> > > > Say Y here if you want support for the PCIe host controller found
> > > > @@ -45,8 +44,8 @@ config PCI_TEGRA
> > > >
> > > > config PCI_RCAR_GEN2
> > > > bool "Renesas R-Car Gen2 Internal PCI controller"
> > > > - depends on ARM
> > > > depends on ARCH_RENESAS || COMPILE_TEST
> > > > + depends on ARM
> > > > help
> > > > Say Y here if you want internal PCI support on R-Car Gen2 SoC.
> > > > There are 3 internal PCI controllers available with a single
> > > > @@ -54,7 +53,7 @@ config PCI_RCAR_GEN2
> > > >
> > > > config PCIE_RCAR
> > > > bool "Renesas R-Car PCIe controller"
> > > > - depends on ARCH_RENESAS || (ARM && COMPILE_TEST)
> > > > + depends on ARCH_RENESAS || COMPILE_TEST
> > > > depends on PCI_MSI_IRQ_DOMAIN
> > > > help
> > > > Say Y here if you want PCIe controller support on R-Car SoCs.
> > > > @@ -65,7 +64,7 @@ config PCI_HOST_COMMON
> > > >
> > > > config PCI_HOST_GENERIC
> > > > bool "Generic PCI host controller"
> > > > - depends on (ARM || ARM64) && OF
> > > > + depends on OF
> > > > select PCI_HOST_COMMON
> > > > select IRQ_DOMAIN
> > > > help
> > > > @@ -74,14 +73,14 @@ config PCI_HOST_GENERIC
> > > >
> > > > config PCIE_XILINX
> > > > bool "Xilinx AXI PCIe host bridge support"
> > > > - depends on ARCH_ZYNQ || MICROBLAZE || (MIPS && PCI_DRIVERS_GENERIC)
> > > > + depends on ARCH_ZYNQ || MICROBLAZE || (MIPS && PCI_DRIVERS_GENERIC) || COMPILE_TEST
> > > > help
> > > > Say 'Y' here if you want kernel to support the Xilinx AXI PCIe
> > > > Host Bridge driver.
> > > >
> > > > config PCI_XGENE
> > > > bool "X-Gene PCIe controller"
> > > > - depends on ARM64
> > > > + depends on ARM64 || COMPILE_TEST
> > > > depends on OF || (ACPI && PCI_QUIRKS)
> > > > select PCIEPORTBUS
> > > > help
> > > > @@ -101,7 +100,7 @@ config PCI_XGENE_MSI
> > > > config PCI_V3_SEMI
> > > > bool "V3 Semiconductor PCI controller"
> > > > depends on OF
> > > > - depends on ARM
> > > > + depends on ARM || COMPILE_TEST
> > > > default ARCH_INTEGRATOR_AP
> > > >
> > > > config PCI_VERSATILE
> > > > @@ -147,8 +146,7 @@ config PCIE_IPROC_MSI
> > > >
> > > > config PCIE_ALTERA
> > > > bool "Altera PCIe controller"
> > > > - depends on ARM || NIOS2
> > > > - depends on OF_PCI
> > > > + depends on ARM || NIOS2 || COMPILE_TEST
> > > > select PCI_DOMAINS
> > > > help
> > > > Say Y here if you want to enable PCIe controller support on Altera
> > > > @@ -164,7 +162,7 @@ config PCIE_ALTERA_MSI
> > > >
> > > > config PCI_HOST_THUNDER_PEM
> > > > bool "Cavium Thunder PCIe controller to off-chip devices"
> > > > - depends on ARM64
> > > > + depends on ARM64 || COMPILE_TEST
> > > > depends on OF || (ACPI && PCI_QUIRKS)
> > > > select PCI_HOST_COMMON
> > > > help
> > > > @@ -172,7 +170,7 @@ config PCI_HOST_THUNDER_PEM
> > > >
> > > > config PCI_HOST_THUNDER_ECAM
> > > > bool "Cavium Thunder ECAM controller to on-chip devices on pass-1.x silicon"
> > > > - depends on ARM64
> > > > + depends on ARM64 || COMPILE_TEST
> > > > depends on OF || (ACPI && PCI_QUIRKS)
> > > > select PCI_HOST_COMMON
> > > > help
> > > > @@ -191,9 +189,8 @@ config PCIE_ROCKCHIP
> > > >
> > > > config PCIE_MEDIATEK
> > > > bool "MediaTek PCIe controller"
> > > > - depends on (ARM || ARM64) && (ARCH_MEDIATEK || COMPILE_TEST)
> > > > + depends on ARCH_MEDIATEK || COMPILE_TEST
> > > > depends on OF
> > > > - depends on PCI
> > > > select PCIEPORTBUS
> > > > help
> > > > Say Y here if you want to enable PCIe controller support on
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [v4] PCI: improve host drivers compile test coverage
2018-06-13 18:11 ` Guenter Roeck
2018-06-14 16:53 ` Lorenzo Pieralisi
@ 2018-06-15 12:58 ` Lorenzo Pieralisi
2018-06-15 17:55 ` Scott Branden
1 sibling, 1 reply; 17+ messages in thread
From: Lorenzo Pieralisi @ 2018-06-15 12:58 UTC (permalink / raw)
To: Guenter Roeck
Cc: Rob Herring, Bjorn Helgaas, Scott Branden, linux-pci, Jan Kiszka,
Ley Foon Tan, Russell King
On Wed, Jun 13, 2018 at 11:11:46AM -0700, Guenter Roeck wrote:
> On Wed, Jun 13, 2018 at 11:09:35AM +0100, Lorenzo Pieralisi wrote:
> > [+Jan, Ley Foon, RMK]
> >
> > On Tue, Jun 12, 2018 at 10:02:29AM -0700, Guenter Roeck wrote:
> > > On Thu, Apr 05, 2018 at 02:31:54PM -0500, Rob Herring wrote:
> > > > Add COMPILE_TEST on driver config options with it. Some ARM drivers
> > > > still have arch dependencies, so we have to keep those dependent on ARM.
> > > >
> > > > Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> > > > Cc: Bjorn Helgaas <bhelgaas@google.com>
> > > > Cc: linux-pci@vger.kernel.org
> > > > Signed-off-by: Rob Herring <robh@kernel.org>
> > >
> > > This patch has the undesirable side effect that it selects PCI_DOMAINS for
> > > sparc32:allmodconfig, which in turn results in
> > >
> > > 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'?
> > >
> > > Unfortunately, 37bd62d224c82 ("PCI: Enable PCI_DOMAINS along with generic
> > > PCI host controller") has pretty much the same result. No idea how to fix
> > > the problem, so I won't even try.
> >
> > Sorry about that.
> >
> > One option would consist in removing all PCI_DOMAINS selection from
> > drivers/pci/controller/Kconfig and delegate it to arches even though
> > this would force PCI_DOMAINS selection on all ARM platforms (it is
> > already selected for ARCH_MULTIPLATFORM).
> >
> > Or we add back arch dependency to the relevant host bridges.
> >
> > Everything else I have in mind seems overkill to me given that this
> > patch was added to improve test coverage (we could add a default
> > pci_domain_nr() stub - weak or #define - that returns 0 in case arches
> > do not provide an implementation but do we really want to do that ?).
> >
> > Thoughts appreciated.
> >
>
> From the definition of PCI_DOMAINS, I suspect the original idea was that
> drivers should depend on it, not select it. Especially auto-selecting
> it with PCI_HOST_GENERIC seems like a bad idea to me. However, that is
> just me. I'll leave it up to Bjorn to decide what if anything he wants
> to do about it.
Here is a patch that should reinstate the previous behaviour but
it will make PCI_DOMAINS a visible option on ARM 32-bit systems; whether
that's acceptable that's the question I need to answer, it should
honour old configs and it does not force PCI_DOMAINS selection on
non-DT arch/arm PCI host controllers (that do not need PCI_DOMAINS
anyway so I suspect that enabling it on all ARM 32-bit platforms
should not break anything but I preferred to be cautious).
If I hear no complaints I will post it for inclusion, we cannot leave
things as they are.
Lorenzo
-- >8 --
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 2a78bdef9a24..1d415aeb54b4 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1244,8 +1244,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/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.
--
2.15.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [v4] PCI: improve host drivers compile test coverage
2018-06-15 12:58 ` Lorenzo Pieralisi
@ 2018-06-15 17:55 ` Scott Branden
2018-06-15 17:58 ` Scott Branden
2018-06-15 18:34 ` Rob Herring
0 siblings, 2 replies; 17+ messages in thread
From: Scott Branden @ 2018-06-15 17:55 UTC (permalink / raw)
To: Lorenzo Pieralisi, Guenter Roeck
Cc: Rob Herring, Bjorn Helgaas, linux-pci, Jan Kiszka, Ley Foon Tan,
Russell King
Hi Lorenzo,
On 18-06-15 05:58 AM, Lorenzo Pieralisi wrote:
> On Wed, Jun 13, 2018 at 11:11:46AM -0700, Guenter Roeck wrote:
>> On Wed, Jun 13, 2018 at 11:09:35AM +0100, Lorenzo Pieralisi wrote:
>>> [+Jan, Ley Foon, RMK]
>>>
>>> On Tue, Jun 12, 2018 at 10:02:29AM -0700, Guenter Roeck wrote:
>>>> On Thu, Apr 05, 2018 at 02:31:54PM -0500, Rob Herring wrote:
>>>>> Add COMPILE_TEST on driver config options with it. Some ARM drivers
>>>>> still have arch dependencies, so we have to keep those dependent on ARM.
>>>>>
>>>>> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
>>>>> Cc: Bjorn Helgaas <bhelgaas@google.com>
>>>>> Cc: linux-pci@vger.kernel.org
>>>>> Signed-off-by: Rob Herring <robh@kernel.org>
>>>> This patch has the undesirable side effect that it selects PCI_DOMAINS for
>>>> sparc32:allmodconfig, which in turn results in
>>>>
>>>> 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'?
>>>>
>>>> Unfortunately, 37bd62d224c82 ("PCI: Enable PCI_DOMAINS along with generic
>>>> PCI host controller") has pretty much the same result. No idea how to fix
>>>> the problem, so I won't even try.
>>> Sorry about that.
>>>
>>> One option would consist in removing all PCI_DOMAINS selection from
>>> drivers/pci/controller/Kconfig and delegate it to arches even though
>>> this would force PCI_DOMAINS selection on all ARM platforms (it is
>>> already selected for ARCH_MULTIPLATFORM).
>>>
>>> Or we add back arch dependency to the relevant host bridges.
>>>
>>> Everything else I have in mind seems overkill to me given that this
>>> patch was added to improve test coverage (we could add a default
>>> pci_domain_nr() stub - weak or #define - that returns 0 in case arches
>>> do not provide an implementation but do we really want to do that ?).
>>>
>>> Thoughts appreciated.
>>>
>> From the definition of PCI_DOMAINS, I suspect the original idea was that
>> drivers should depend on it, not select it. Especially auto-selecting
>> it with PCI_HOST_GENERIC seems like a bad idea to me. However, that is
>> just me. I'll leave it up to Bjorn to decide what if anything he wants
>> to do about it.
> Here is a patch that should reinstate the previous behaviour but
> it will make PCI_DOMAINS a visible option on ARM 32-bit systems; whether
> that's acceptable that's the question I need to answer, it should
> honour old configs and it does not force PCI_DOMAINS selection on
> non-DT arch/arm PCI host controllers (that do not need PCI_DOMAINS
> anyway so I suspect that enabling it on all ARM 32-bit platforms
> should not break anything but I preferred to be cautious).
I think this change will also require a patch enabling
CONFIG_PCI_DOMAINS in multi_v7_defconfig and iproc_defconfig at the very
least?
>
> If I hear no complaints I will post it for inclusion, we cannot leave
> things as they are.
>
> Lorenzo
>
> -- >8 --
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index 2a78bdef9a24..1d415aeb54b4 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -1244,8 +1244,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/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.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [v4] PCI: improve host drivers compile test coverage
2018-06-15 17:55 ` Scott Branden
@ 2018-06-15 17:58 ` Scott Branden
2018-06-18 11:34 ` Lorenzo Pieralisi
2018-06-15 18:34 ` Rob Herring
1 sibling, 1 reply; 17+ messages in thread
From: Scott Branden @ 2018-06-15 17:58 UTC (permalink / raw)
To: Lorenzo Pieralisi, Guenter Roeck
Cc: Rob Herring, Bjorn Helgaas, linux-pci, Jan Kiszka, Ley Foon Tan,
Russell King
On 18-06-15 10:55 AM, Scott Branden wrote:
> Hi Lorenzo,
>
>
> On 18-06-15 05:58 AM, Lorenzo Pieralisi wrote:
>> On Wed, Jun 13, 2018 at 11:11:46AM -0700, Guenter Roeck wrote:
>>> On Wed, Jun 13, 2018 at 11:09:35AM +0100, Lorenzo Pieralisi wrote:
>>>> [+Jan, Ley Foon, RMK]
>>>>
>>>> On Tue, Jun 12, 2018 at 10:02:29AM -0700, Guenter Roeck wrote:
>>>>> On Thu, Apr 05, 2018 at 02:31:54PM -0500, Rob Herring wrote:
>>>>>> Add COMPILE_TEST on driver config options with it. Some ARM drivers
>>>>>> still have arch dependencies, so we have to keep those dependent
>>>>>> on ARM.
>>>>>>
>>>>>> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
>>>>>> Cc: Bjorn Helgaas <bhelgaas@google.com>
>>>>>> Cc: linux-pci@vger.kernel.org
>>>>>> Signed-off-by: Rob Herring <robh@kernel.org>
>>>>> This patch has the undesirable side effect that it selects
>>>>> PCI_DOMAINS for
>>>>> sparc32:allmodconfig, which in turn results in
>>>>>
>>>>> 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'?
>>>>>
>>>>> Unfortunately, 37bd62d224c82 ("PCI: Enable PCI_DOMAINS along with
>>>>> generic
>>>>> PCI host controller") has pretty much the same result. No idea how
>>>>> to fix
>>>>> the problem, so I won't even try.
>>>> Sorry about that.
>>>>
>>>> One option would consist in removing all PCI_DOMAINS selection from
>>>> drivers/pci/controller/Kconfig and delegate it to arches even though
>>>> this would force PCI_DOMAINS selection on all ARM platforms (it is
>>>> already selected for ARCH_MULTIPLATFORM).
>>>>
>>>> Or we add back arch dependency to the relevant host bridges.
>>>>
>>>> Everything else I have in mind seems overkill to me given that this
>>>> patch was added to improve test coverage (we could add a default
>>>> pci_domain_nr() stub - weak or #define - that returns 0 in case arches
>>>> do not provide an implementation but do we really want to do that ?).
>>>>
>>>> Thoughts appreciated.
>>>>
>>> From the definition of PCI_DOMAINS, I suspect the original idea was
>>> that
>>> drivers should depend on it, not select it. Especially auto-selecting
>>> it with PCI_HOST_GENERIC seems like a bad idea to me. However, that is
>>> just me. I'll leave it up to Bjorn to decide what if anything he wants
>>> to do about it.
>> Here is a patch that should reinstate the previous behaviour but
>> it will make PCI_DOMAINS a visible option on ARM 32-bit systems; whether
>> that's acceptable that's the question I need to answer, it should
>> honour old configs and it does not force PCI_DOMAINS selection on
>> non-DT arch/arm PCI host controllers (that do not need PCI_DOMAINS
>> anyway so I suspect that enabling it on all ARM 32-bit platforms
>> should not break anything but I preferred to be cautious).
> I think this change will also require a patch enabling
> CONFIG_PCI_DOMAINS in multi_v7_defconfig and iproc_defconfig at the
> very least?
as well as arm64/configs/defconfig? Rather than having to change all
the defconfigs perhaps can to make default y in Kconfig for
architectures that should have it as such?
>
>>
>> If I hear no complaints I will post it for inclusion, we cannot leave
>> things as they are.
>>
>> Lorenzo
>>
>> -- >8 --
>> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
>> index 2a78bdef9a24..1d415aeb54b4 100644
>> --- a/arch/arm/Kconfig
>> +++ b/arch/arm/Kconfig
>> @@ -1244,8 +1244,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/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.
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [v4] PCI: improve host drivers compile test coverage
2018-06-15 17:55 ` Scott Branden
2018-06-15 17:58 ` Scott Branden
@ 2018-06-15 18:34 ` Rob Herring
2018-06-18 9:32 ` Lorenzo Pieralisi
1 sibling, 1 reply; 17+ messages in thread
From: Rob Herring @ 2018-06-15 18:34 UTC (permalink / raw)
To: Scott Branden
Cc: Lorenzo Pieralisi, Guenter Roeck, Bjorn Helgaas, linux-pci,
Jan Kiszka, Ley Foon Tan, Russell King
On Fri, Jun 15, 2018 at 11:55 AM, Scott Branden
<scott.branden@broadcom.com> wrote:
> Hi Lorenzo,
>
>
>
> On 18-06-15 05:58 AM, Lorenzo Pieralisi wrote:
>>
>> On Wed, Jun 13, 2018 at 11:11:46AM -0700, Guenter Roeck wrote:
>>>
>>> On Wed, Jun 13, 2018 at 11:09:35AM +0100, Lorenzo Pieralisi wrote:
>>>>
>>>> [+Jan, Ley Foon, RMK]
>>>>
>>>> On Tue, Jun 12, 2018 at 10:02:29AM -0700, Guenter Roeck wrote:
>>>>>
>>>>> On Thu, Apr 05, 2018 at 02:31:54PM -0500, Rob Herring wrote:
>>>>>>
>>>>>> Add COMPILE_TEST on driver config options with it. Some ARM drivers
>>>>>> still have arch dependencies, so we have to keep those dependent on
>>>>>> ARM.
>>>>>>
>>>>>> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
>>>>>> Cc: Bjorn Helgaas <bhelgaas@google.com>
>>>>>> Cc: linux-pci@vger.kernel.org
>>>>>> Signed-off-by: Rob Herring <robh@kernel.org>
>>>>>
>>>>> This patch has the undesirable side effect that it selects PCI_DOMAINS
>>>>> for
>>>>> sparc32:allmodconfig, which in turn results in
>>>>>
>>>>> 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'?
>>>>>
>>>>> Unfortunately, 37bd62d224c82 ("PCI: Enable PCI_DOMAINS along with
>>>>> generic
>>>>> PCI host controller") has pretty much the same result. No idea how to
>>>>> fix
>>>>> the problem, so I won't even try.
>>>>
>>>> Sorry about that.
>>>>
>>>> One option would consist in removing all PCI_DOMAINS selection from
>>>> drivers/pci/controller/Kconfig and delegate it to arches even though
>>>> this would force PCI_DOMAINS selection on all ARM platforms (it is
>>>> already selected for ARCH_MULTIPLATFORM).
>>>>
>>>> Or we add back arch dependency to the relevant host bridges.
>>>>
>>>> Everything else I have in mind seems overkill to me given that this
>>>> patch was added to improve test coverage (we could add a default
>>>> pci_domain_nr() stub - weak or #define - that returns 0 in case arches
>>>> do not provide an implementation but do we really want to do that ?).
>>>>
>>>> Thoughts appreciated.
>>>>
>>> From the definition of PCI_DOMAINS, I suspect the original idea was that
>>> drivers should depend on it, not select it. Especially auto-selecting
>>> it with PCI_HOST_GENERIC seems like a bad idea to me. However, that is
>>> just me. I'll leave it up to Bjorn to decide what if anything he wants
>>> to do about it.
>>
>> Here is a patch that should reinstate the previous behaviour but
>> it will make PCI_DOMAINS a visible option on ARM 32-bit systems; whether
>> that's acceptable that's the question I need to answer, it should
>> honour old configs and it does not force PCI_DOMAINS selection on
>> non-DT arch/arm PCI host controllers (that do not need PCI_DOMAINS
>> anyway so I suspect that enabling it on all ARM 32-bit platforms
>> should not break anything but I preferred to be cautious).
>
> I think this change will also require a patch enabling CONFIG_PCI_DOMAINS in
> multi_v7_defconfig and iproc_defconfig at the very least?
Perhaps the sub-arches that want this should select it. It is more a
platform option/decision more than a controller option.
Rob
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [v4] PCI: improve host drivers compile test coverage
2018-06-15 18:34 ` Rob Herring
@ 2018-06-18 9:32 ` Lorenzo Pieralisi
2018-06-18 11:42 ` Jan Kiszka
0 siblings, 1 reply; 17+ messages in thread
From: Lorenzo Pieralisi @ 2018-06-18 9:32 UTC (permalink / raw)
To: Rob Herring, Jan Kiszka
Cc: Scott Branden, Guenter Roeck, Bjorn Helgaas, linux-pci,
Ley Foon Tan, Russell King
On Fri, Jun 15, 2018 at 12:34:51PM -0600, Rob Herring wrote:
> On Fri, Jun 15, 2018 at 11:55 AM, Scott Branden
> <scott.branden@broadcom.com> wrote:
> > Hi Lorenzo,
> >
> >
> >
> > On 18-06-15 05:58 AM, Lorenzo Pieralisi wrote:
> >>
> >> On Wed, Jun 13, 2018 at 11:11:46AM -0700, Guenter Roeck wrote:
> >>>
> >>> On Wed, Jun 13, 2018 at 11:09:35AM +0100, Lorenzo Pieralisi wrote:
> >>>>
> >>>> [+Jan, Ley Foon, RMK]
> >>>>
> >>>> On Tue, Jun 12, 2018 at 10:02:29AM -0700, Guenter Roeck wrote:
> >>>>>
> >>>>> On Thu, Apr 05, 2018 at 02:31:54PM -0500, Rob Herring wrote:
> >>>>>>
> >>>>>> Add COMPILE_TEST on driver config options with it. Some ARM drivers
> >>>>>> still have arch dependencies, so we have to keep those dependent on
> >>>>>> ARM.
> >>>>>>
> >>>>>> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> >>>>>> Cc: Bjorn Helgaas <bhelgaas@google.com>
> >>>>>> Cc: linux-pci@vger.kernel.org
> >>>>>> Signed-off-by: Rob Herring <robh@kernel.org>
> >>>>>
> >>>>> This patch has the undesirable side effect that it selects PCI_DOMAINS
> >>>>> for
> >>>>> sparc32:allmodconfig, which in turn results in
> >>>>>
> >>>>> 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'?
> >>>>>
> >>>>> Unfortunately, 37bd62d224c82 ("PCI: Enable PCI_DOMAINS along with
> >>>>> generic
> >>>>> PCI host controller") has pretty much the same result. No idea how to
> >>>>> fix
> >>>>> the problem, so I won't even try.
> >>>>
> >>>> Sorry about that.
> >>>>
> >>>> One option would consist in removing all PCI_DOMAINS selection from
> >>>> drivers/pci/controller/Kconfig and delegate it to arches even though
> >>>> this would force PCI_DOMAINS selection on all ARM platforms (it is
> >>>> already selected for ARCH_MULTIPLATFORM).
> >>>>
> >>>> Or we add back arch dependency to the relevant host bridges.
> >>>>
> >>>> Everything else I have in mind seems overkill to me given that this
> >>>> patch was added to improve test coverage (we could add a default
> >>>> pci_domain_nr() stub - weak or #define - that returns 0 in case arches
> >>>> do not provide an implementation but do we really want to do that ?).
> >>>>
> >>>> Thoughts appreciated.
> >>>>
> >>> From the definition of PCI_DOMAINS, I suspect the original idea was that
> >>> drivers should depend on it, not select it. Especially auto-selecting
> >>> it with PCI_HOST_GENERIC seems like a bad idea to me. However, that is
> >>> just me. I'll leave it up to Bjorn to decide what if anything he wants
> >>> to do about it.
> >>
> >> Here is a patch that should reinstate the previous behaviour but
> >> it will make PCI_DOMAINS a visible option on ARM 32-bit systems; whether
> >> that's acceptable that's the question I need to answer, it should
> >> honour old configs and it does not force PCI_DOMAINS selection on
> >> non-DT arch/arm PCI host controllers (that do not need PCI_DOMAINS
> >> anyway so I suspect that enabling it on all ARM 32-bit platforms
> >> should not break anything but I preferred to be cautious).
> >
> > I think this change will also require a patch enabling CONFIG_PCI_DOMAINS in
> > multi_v7_defconfig and iproc_defconfig at the very least?
>
> Perhaps the sub-arches that want this should select it. It is more a
> platform option/decision more than a controller option.
Yes, that makes sense, I assume ARCH_VIRT is a sensible choice for
virtual machines configuration, Jan ?
I will add a PCI_DOMAINS selection to all (ARM) arches that select
PCI_DOMAINS in drivers/pci/controller/Kconfig.
For Jailhouse configurations I need Jan's input, I assume adding
the selection to ARCH_VIRT is the correct way forward, please let
me know asap.
Lorenzo
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [v4] PCI: improve host drivers compile test coverage
2018-06-15 17:58 ` Scott Branden
@ 2018-06-18 11:34 ` Lorenzo Pieralisi
0 siblings, 0 replies; 17+ messages in thread
From: Lorenzo Pieralisi @ 2018-06-18 11:34 UTC (permalink / raw)
To: Scott Branden
Cc: Guenter Roeck, Rob Herring, Bjorn Helgaas, linux-pci, Jan Kiszka,
Ley Foon Tan, Russell King
On Fri, Jun 15, 2018 at 10:58:19AM -0700, Scott Branden wrote:
>
>
> On 18-06-15 10:55 AM, Scott Branden wrote:
> >Hi Lorenzo,
> >
> >
> >On 18-06-15 05:58 AM, Lorenzo Pieralisi wrote:
> >>On Wed, Jun 13, 2018 at 11:11:46AM -0700, Guenter Roeck wrote:
> >>>On Wed, Jun 13, 2018 at 11:09:35AM +0100, Lorenzo Pieralisi wrote:
> >>>>[+Jan, Ley Foon, RMK]
> >>>>
> >>>>On Tue, Jun 12, 2018 at 10:02:29AM -0700, Guenter Roeck wrote:
> >>>>>On Thu, Apr 05, 2018 at 02:31:54PM -0500, Rob Herring wrote:
> >>>>>>Add COMPILE_TEST on driver config options with it. Some ARM drivers
> >>>>>>still have arch dependencies, so we have to keep those
> >>>>>>dependent on ARM.
> >>>>>>
> >>>>>>Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> >>>>>>Cc: Bjorn Helgaas <bhelgaas@google.com>
> >>>>>>Cc: linux-pci@vger.kernel.org
> >>>>>>Signed-off-by: Rob Herring <robh@kernel.org>
> >>>>>This patch has the undesirable side effect that it selects
> >>>>>PCI_DOMAINS for
> >>>>>sparc32:allmodconfig, which in turn results in
> >>>>>
> >>>>>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'?
> >>>>>
> >>>>>Unfortunately, 37bd62d224c82 ("PCI: Enable PCI_DOMAINS
> >>>>>along with generic
> >>>>>PCI host controller") has pretty much the same result. No
> >>>>>idea how to fix
> >>>>>the problem, so I won't even try.
> >>>>Sorry about that.
> >>>>
> >>>>One option would consist in removing all PCI_DOMAINS selection from
> >>>>drivers/pci/controller/Kconfig and delegate it to arches even though
> >>>>this would force PCI_DOMAINS selection on all ARM platforms (it is
> >>>>already selected for ARCH_MULTIPLATFORM).
> >>>>
> >>>>Or we add back arch dependency to the relevant host bridges.
> >>>>
> >>>>Everything else I have in mind seems overkill to me given that this
> >>>>patch was added to improve test coverage (we could add a default
> >>>>pci_domain_nr() stub - weak or #define - that returns 0 in case arches
> >>>>do not provide an implementation but do we really want to do that ?).
> >>>>
> >>>>Thoughts appreciated.
> >>>>
> >>> From the definition of PCI_DOMAINS, I suspect the original
> >>>idea was that
> >>>drivers should depend on it, not select it. Especially auto-selecting
> >>>it with PCI_HOST_GENERIC seems like a bad idea to me. However, that is
> >>>just me. I'll leave it up to Bjorn to decide what if anything he wants
> >>>to do about it.
> >>Here is a patch that should reinstate the previous behaviour but
> >>it will make PCI_DOMAINS a visible option on ARM 32-bit systems; whether
> >>that's acceptable that's the question I need to answer, it should
> >>honour old configs and it does not force PCI_DOMAINS selection on
> >>non-DT arch/arm PCI host controllers (that do not need PCI_DOMAINS
> >>anyway so I suspect that enabling it on all ARM 32-bit platforms
> >>should not break anything but I preferred to be cautious).
> >I think this change will also require a patch enabling
> >CONFIG_PCI_DOMAINS in multi_v7_defconfig and iproc_defconfig at
> >the very least?
> as well as arm64/configs/defconfig? Rather than having to change
> all the defconfigs perhaps can to make default y in Kconfig for
> architectures that should have it as such?
I can't do that on ARM, this would force PCI_DOMAINS on platforms
that never relied on it and that can trigger regressions.
It is unfortunate but the safest option consists in selecting
PCI_DOMAINS in the respective ARCH_* options I think.
Lorenzo
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [v4] PCI: improve host drivers compile test coverage
2018-06-18 9:32 ` Lorenzo Pieralisi
@ 2018-06-18 11:42 ` Jan Kiszka
2018-06-18 12:52 ` Lorenzo Pieralisi
0 siblings, 1 reply; 17+ messages in thread
From: Jan Kiszka @ 2018-06-18 11:42 UTC (permalink / raw)
To: Lorenzo Pieralisi, Rob Herring
Cc: Scott Branden, Guenter Roeck, Bjorn Helgaas, linux-pci,
Ley Foon Tan, Russell King
On 2018-06-18 11:32, Lorenzo Pieralisi wrote:
> On Fri, Jun 15, 2018 at 12:34:51PM -0600, Rob Herring wrote:
>> On Fri, Jun 15, 2018 at 11:55 AM, Scott Branden
>> <scott.branden@broadcom.com> wrote:
>>> Hi Lorenzo,
>>>
>>>
>>>
>>> On 18-06-15 05:58 AM, Lorenzo Pieralisi wrote:
>>>>
>>>> On Wed, Jun 13, 2018 at 11:11:46AM -0700, Guenter Roeck wrote:
>>>>>
>>>>> On Wed, Jun 13, 2018 at 11:09:35AM +0100, Lorenzo Pieralisi wrote:
>>>>>>
>>>>>> [+Jan, Ley Foon, RMK]
>>>>>>
>>>>>> On Tue, Jun 12, 2018 at 10:02:29AM -0700, Guenter Roeck wrote:
>>>>>>>
>>>>>>> On Thu, Apr 05, 2018 at 02:31:54PM -0500, Rob Herring wrote:
>>>>>>>>
>>>>>>>> Add COMPILE_TEST on driver config options with it. Some ARM drivers
>>>>>>>> still have arch dependencies, so we have to keep those dependent on
>>>>>>>> ARM.
>>>>>>>>
>>>>>>>> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
>>>>>>>> Cc: Bjorn Helgaas <bhelgaas@google.com>
>>>>>>>> Cc: linux-pci@vger.kernel.org
>>>>>>>> Signed-off-by: Rob Herring <robh@kernel.org>
>>>>>>>
>>>>>>> This patch has the undesirable side effect that it selects PCI_DOMAINS
>>>>>>> for
>>>>>>> sparc32:allmodconfig, which in turn results in
>>>>>>>
>>>>>>> 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'?
>>>>>>>
>>>>>>> Unfortunately, 37bd62d224c82 ("PCI: Enable PCI_DOMAINS along with
>>>>>>> generic
>>>>>>> PCI host controller") has pretty much the same result. No idea how to
>>>>>>> fix
>>>>>>> the problem, so I won't even try.
>>>>>>
>>>>>> Sorry about that.
>>>>>>
>>>>>> One option would consist in removing all PCI_DOMAINS selection from
>>>>>> drivers/pci/controller/Kconfig and delegate it to arches even though
>>>>>> this would force PCI_DOMAINS selection on all ARM platforms (it is
>>>>>> already selected for ARCH_MULTIPLATFORM).
>>>>>>
>>>>>> Or we add back arch dependency to the relevant host bridges.
>>>>>>
>>>>>> Everything else I have in mind seems overkill to me given that this
>>>>>> patch was added to improve test coverage (we could add a default
>>>>>> pci_domain_nr() stub - weak or #define - that returns 0 in case arches
>>>>>> do not provide an implementation but do we really want to do that ?).
>>>>>>
>>>>>> Thoughts appreciated.
>>>>>>
>>>>> From the definition of PCI_DOMAINS, I suspect the original idea was that
>>>>> drivers should depend on it, not select it. Especially auto-selecting
>>>>> it with PCI_HOST_GENERIC seems like a bad idea to me. However, that is
>>>>> just me. I'll leave it up to Bjorn to decide what if anything he wants
>>>>> to do about it.
>>>>
>>>> Here is a patch that should reinstate the previous behaviour but
>>>> it will make PCI_DOMAINS a visible option on ARM 32-bit systems; whether
>>>> that's acceptable that's the question I need to answer, it should
>>>> honour old configs and it does not force PCI_DOMAINS selection on
>>>> non-DT arch/arm PCI host controllers (that do not need PCI_DOMAINS
>>>> anyway so I suspect that enabling it on all ARM 32-bit platforms
>>>> should not break anything but I preferred to be cautious).
>>>
>>> I think this change will also require a patch enabling CONFIG_PCI_DOMAINS in
>>> multi_v7_defconfig and iproc_defconfig at the very least?
>>
>> Perhaps the sub-arches that want this should select it. It is more a
>> platform option/decision more than a controller option.
>
> Yes, that makes sense, I assume ARCH_VIRT is a sensible choice for
> virtual machines configuration, Jan ?
>
> I will add a PCI_DOMAINS selection to all (ARM) arches that select
> PCI_DOMAINS in drivers/pci/controller/Kconfig.
>
> For Jailhouse configurations I need Jan's input, I assume adding
> the selection to ARCH_VIRT is the correct way forward, please let
> me know asap.
So far, there is no need on ARM or ARM64 declare a special platform in
order to run as Jailhouse (secondary) guest.
My original patch was just about making PCI_DOMAINS manually
configurable, which would have been fine for our use case.
Jan
--
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [v4] PCI: improve host drivers compile test coverage
2018-06-18 11:42 ` Jan Kiszka
@ 2018-06-18 12:52 ` Lorenzo Pieralisi
2018-06-18 16:53 ` Jan Kiszka
0 siblings, 1 reply; 17+ messages in thread
From: Lorenzo Pieralisi @ 2018-06-18 12:52 UTC (permalink / raw)
To: Jan Kiszka
Cc: Rob Herring, Scott Branden, Guenter Roeck, Bjorn Helgaas,
linux-pci, Ley Foon Tan, Russell King
On Mon, Jun 18, 2018 at 01:42:25PM +0200, Jan Kiszka wrote:
> On 2018-06-18 11:32, Lorenzo Pieralisi wrote:
> > On Fri, Jun 15, 2018 at 12:34:51PM -0600, Rob Herring wrote:
> >> On Fri, Jun 15, 2018 at 11:55 AM, Scott Branden
> >> <scott.branden@broadcom.com> wrote:
> >>> Hi Lorenzo,
> >>>
> >>>
> >>>
> >>> On 18-06-15 05:58 AM, Lorenzo Pieralisi wrote:
> >>>>
> >>>> On Wed, Jun 13, 2018 at 11:11:46AM -0700, Guenter Roeck wrote:
> >>>>>
> >>>>> On Wed, Jun 13, 2018 at 11:09:35AM +0100, Lorenzo Pieralisi wrote:
> >>>>>>
> >>>>>> [+Jan, Ley Foon, RMK]
> >>>>>>
> >>>>>> On Tue, Jun 12, 2018 at 10:02:29AM -0700, Guenter Roeck wrote:
> >>>>>>>
> >>>>>>> On Thu, Apr 05, 2018 at 02:31:54PM -0500, Rob Herring wrote:
> >>>>>>>>
> >>>>>>>> Add COMPILE_TEST on driver config options with it. Some ARM drivers
> >>>>>>>> still have arch dependencies, so we have to keep those dependent on
> >>>>>>>> ARM.
> >>>>>>>>
> >>>>>>>> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> >>>>>>>> Cc: Bjorn Helgaas <bhelgaas@google.com>
> >>>>>>>> Cc: linux-pci@vger.kernel.org
> >>>>>>>> Signed-off-by: Rob Herring <robh@kernel.org>
> >>>>>>>
> >>>>>>> This patch has the undesirable side effect that it selects PCI_DOMAINS
> >>>>>>> for
> >>>>>>> sparc32:allmodconfig, which in turn results in
> >>>>>>>
> >>>>>>> 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'?
> >>>>>>>
> >>>>>>> Unfortunately, 37bd62d224c82 ("PCI: Enable PCI_DOMAINS along with
> >>>>>>> generic
> >>>>>>> PCI host controller") has pretty much the same result. No idea how to
> >>>>>>> fix
> >>>>>>> the problem, so I won't even try.
> >>>>>>
> >>>>>> Sorry about that.
> >>>>>>
> >>>>>> One option would consist in removing all PCI_DOMAINS selection from
> >>>>>> drivers/pci/controller/Kconfig and delegate it to arches even though
> >>>>>> this would force PCI_DOMAINS selection on all ARM platforms (it is
> >>>>>> already selected for ARCH_MULTIPLATFORM).
> >>>>>>
> >>>>>> Or we add back arch dependency to the relevant host bridges.
> >>>>>>
> >>>>>> Everything else I have in mind seems overkill to me given that this
> >>>>>> patch was added to improve test coverage (we could add a default
> >>>>>> pci_domain_nr() stub - weak or #define - that returns 0 in case arches
> >>>>>> do not provide an implementation but do we really want to do that ?).
> >>>>>>
> >>>>>> Thoughts appreciated.
> >>>>>>
> >>>>> From the definition of PCI_DOMAINS, I suspect the original idea was that
> >>>>> drivers should depend on it, not select it. Especially auto-selecting
> >>>>> it with PCI_HOST_GENERIC seems like a bad idea to me. However, that is
> >>>>> just me. I'll leave it up to Bjorn to decide what if anything he wants
> >>>>> to do about it.
> >>>>
> >>>> Here is a patch that should reinstate the previous behaviour but
> >>>> it will make PCI_DOMAINS a visible option on ARM 32-bit systems; whether
> >>>> that's acceptable that's the question I need to answer, it should
> >>>> honour old configs and it does not force PCI_DOMAINS selection on
> >>>> non-DT arch/arm PCI host controllers (that do not need PCI_DOMAINS
> >>>> anyway so I suspect that enabling it on all ARM 32-bit platforms
> >>>> should not break anything but I preferred to be cautious).
> >>>
> >>> I think this change will also require a patch enabling CONFIG_PCI_DOMAINS in
> >>> multi_v7_defconfig and iproc_defconfig at the very least?
> >>
> >> Perhaps the sub-arches that want this should select it. It is more a
> >> platform option/decision more than a controller option.
> >
> > Yes, that makes sense, I assume ARCH_VIRT is a sensible choice for
> > virtual machines configuration, Jan ?
> >
> > I will add a PCI_DOMAINS selection to all (ARM) arches that select
> > PCI_DOMAINS in drivers/pci/controller/Kconfig.
> >
> > For Jailhouse configurations I need Jan's input, I assume adding
> > the selection to ARCH_VIRT is the correct way forward, please let
> > me know asap.
>
> So far, there is no need on ARM or ARM64 declare a special platform in
> order to run as Jailhouse (secondary) guest.
>
> My original patch was just about making PCI_DOMAINS manually
> configurable, which would have been fine for our use case.
I need more details on your system configuration to make sure we can fix
this in a way that does not upset anybody; I am not a big fan of making
PCI_DOMAINS visible since it affects other platforms and it is different
from how it is managed for all other arches, so please provide details.
Thanks,
Lorenzo
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [v4] PCI: improve host drivers compile test coverage
2018-06-18 12:52 ` Lorenzo Pieralisi
@ 2018-06-18 16:53 ` Jan Kiszka
2018-06-19 9:27 ` Lorenzo Pieralisi
0 siblings, 1 reply; 17+ messages in thread
From: Jan Kiszka @ 2018-06-18 16:53 UTC (permalink / raw)
To: Lorenzo Pieralisi
Cc: Rob Herring, Scott Branden, Guenter Roeck, Bjorn Helgaas,
linux-pci, Ley Foon Tan, Russell King
On 2018-06-18 14:52, Lorenzo Pieralisi wrote:
> On Mon, Jun 18, 2018 at 01:42:25PM +0200, Jan Kiszka wrote:
>> On 2018-06-18 11:32, Lorenzo Pieralisi wrote:
>>> On Fri, Jun 15, 2018 at 12:34:51PM -0600, Rob Herring wrote:
>>>> On Fri, Jun 15, 2018 at 11:55 AM, Scott Branden
>>>> <scott.branden@broadcom.com> wrote:
>>>>> Hi Lorenzo,
>>>>>
>>>>>
>>>>>
>>>>> On 18-06-15 05:58 AM, Lorenzo Pieralisi wrote:
>>>>>>
>>>>>> On Wed, Jun 13, 2018 at 11:11:46AM -0700, Guenter Roeck wrote:
>>>>>>>
>>>>>>> On Wed, Jun 13, 2018 at 11:09:35AM +0100, Lorenzo Pieralisi wrote:
>>>>>>>>
>>>>>>>> [+Jan, Ley Foon, RMK]
>>>>>>>>
>>>>>>>> On Tue, Jun 12, 2018 at 10:02:29AM -0700, Guenter Roeck wrote:
>>>>>>>>>
>>>>>>>>> On Thu, Apr 05, 2018 at 02:31:54PM -0500, Rob Herring wrote:
>>>>>>>>>>
>>>>>>>>>> Add COMPILE_TEST on driver config options with it. Some ARM drivers
>>>>>>>>>> still have arch dependencies, so we have to keep those dependent on
>>>>>>>>>> ARM.
>>>>>>>>>>
>>>>>>>>>> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
>>>>>>>>>> Cc: Bjorn Helgaas <bhelgaas@google.com>
>>>>>>>>>> Cc: linux-pci@vger.kernel.org
>>>>>>>>>> Signed-off-by: Rob Herring <robh@kernel.org>
>>>>>>>>>
>>>>>>>>> This patch has the undesirable side effect that it selects PCI_DOMAINS
>>>>>>>>> for
>>>>>>>>> sparc32:allmodconfig, which in turn results in
>>>>>>>>>
>>>>>>>>> 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'?
>>>>>>>>>
>>>>>>>>> Unfortunately, 37bd62d224c82 ("PCI: Enable PCI_DOMAINS along with
>>>>>>>>> generic
>>>>>>>>> PCI host controller") has pretty much the same result. No idea how to
>>>>>>>>> fix
>>>>>>>>> the problem, so I won't even try.
>>>>>>>>
>>>>>>>> Sorry about that.
>>>>>>>>
>>>>>>>> One option would consist in removing all PCI_DOMAINS selection from
>>>>>>>> drivers/pci/controller/Kconfig and delegate it to arches even though
>>>>>>>> this would force PCI_DOMAINS selection on all ARM platforms (it is
>>>>>>>> already selected for ARCH_MULTIPLATFORM).
>>>>>>>>
>>>>>>>> Or we add back arch dependency to the relevant host bridges.
>>>>>>>>
>>>>>>>> Everything else I have in mind seems overkill to me given that this
>>>>>>>> patch was added to improve test coverage (we could add a default
>>>>>>>> pci_domain_nr() stub - weak or #define - that returns 0 in case arches
>>>>>>>> do not provide an implementation but do we really want to do that ?).
>>>>>>>>
>>>>>>>> Thoughts appreciated.
>>>>>>>>
>>>>>>> From the definition of PCI_DOMAINS, I suspect the original idea was that
>>>>>>> drivers should depend on it, not select it. Especially auto-selecting
>>>>>>> it with PCI_HOST_GENERIC seems like a bad idea to me. However, that is
>>>>>>> just me. I'll leave it up to Bjorn to decide what if anything he wants
>>>>>>> to do about it.
>>>>>>
>>>>>> Here is a patch that should reinstate the previous behaviour but
>>>>>> it will make PCI_DOMAINS a visible option on ARM 32-bit systems; whether
>>>>>> that's acceptable that's the question I need to answer, it should
>>>>>> honour old configs and it does not force PCI_DOMAINS selection on
>>>>>> non-DT arch/arm PCI host controllers (that do not need PCI_DOMAINS
>>>>>> anyway so I suspect that enabling it on all ARM 32-bit platforms
>>>>>> should not break anything but I preferred to be cautious).
>>>>>
>>>>> I think this change will also require a patch enabling CONFIG_PCI_DOMAINS in
>>>>> multi_v7_defconfig and iproc_defconfig at the very least?
>>>>
>>>> Perhaps the sub-arches that want this should select it. It is more a
>>>> platform option/decision more than a controller option.
>>>
>>> Yes, that makes sense, I assume ARCH_VIRT is a sensible choice for
>>> virtual machines configuration, Jan ?
>>>
>>> I will add a PCI_DOMAINS selection to all (ARM) arches that select
>>> PCI_DOMAINS in drivers/pci/controller/Kconfig.
>>>
>>> For Jailhouse configurations I need Jan's input, I assume adding
>>> the selection to ARCH_VIRT is the correct way forward, please let
>>> me know asap.
>>
>> So far, there is no need on ARM or ARM64 declare a special platform in
>> order to run as Jailhouse (secondary) guest.
>>
>> My original patch was just about making PCI_DOMAINS manually
>> configurable, which would have been fine for our use case.
>
> I need more details on your system configuration to make sure we can fix
> this in a way that does not upset anybody; I am not a big fan of making
> PCI_DOMAINS visible since it affects other platforms and it is different
> from how it is managed for all other arches, so please provide details.
Our setup is as follows: A platform, e.g. Jetson TK1 or TX1/2, already
has one PCI host controller. When enabling Jailhouse on it (from within
a running Linux), this adds the generic PCI host controller as virtual
one. So we need to configure the system to support both controllers and
PCI domains. But, otherwise, the system does not look different from a
physical one.
Jan
--
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [v4] PCI: improve host drivers compile test coverage
2018-06-18 16:53 ` Jan Kiszka
@ 2018-06-19 9:27 ` Lorenzo Pieralisi
0 siblings, 0 replies; 17+ messages in thread
From: Lorenzo Pieralisi @ 2018-06-19 9:27 UTC (permalink / raw)
To: Jan Kiszka
Cc: Rob Herring, Scott Branden, Guenter Roeck, Bjorn Helgaas,
linux-pci, Ley Foon Tan, Russell King
On Mon, Jun 18, 2018 at 06:53:10PM +0200, Jan Kiszka wrote:
> On 2018-06-18 14:52, Lorenzo Pieralisi wrote:
> > On Mon, Jun 18, 2018 at 01:42:25PM +0200, Jan Kiszka wrote:
> >> On 2018-06-18 11:32, Lorenzo Pieralisi wrote:
> >>> On Fri, Jun 15, 2018 at 12:34:51PM -0600, Rob Herring wrote:
> >>>> On Fri, Jun 15, 2018 at 11:55 AM, Scott Branden
> >>>> <scott.branden@broadcom.com> wrote:
> >>>>> Hi Lorenzo,
> >>>>>
> >>>>>
> >>>>>
> >>>>> On 18-06-15 05:58 AM, Lorenzo Pieralisi wrote:
> >>>>>>
> >>>>>> On Wed, Jun 13, 2018 at 11:11:46AM -0700, Guenter Roeck wrote:
> >>>>>>>
> >>>>>>> On Wed, Jun 13, 2018 at 11:09:35AM +0100, Lorenzo Pieralisi wrote:
> >>>>>>>>
> >>>>>>>> [+Jan, Ley Foon, RMK]
> >>>>>>>>
> >>>>>>>> On Tue, Jun 12, 2018 at 10:02:29AM -0700, Guenter Roeck wrote:
> >>>>>>>>>
> >>>>>>>>> On Thu, Apr 05, 2018 at 02:31:54PM -0500, Rob Herring wrote:
> >>>>>>>>>>
> >>>>>>>>>> Add COMPILE_TEST on driver config options with it. Some ARM drivers
> >>>>>>>>>> still have arch dependencies, so we have to keep those dependent on
> >>>>>>>>>> ARM.
> >>>>>>>>>>
> >>>>>>>>>> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> >>>>>>>>>> Cc: Bjorn Helgaas <bhelgaas@google.com>
> >>>>>>>>>> Cc: linux-pci@vger.kernel.org
> >>>>>>>>>> Signed-off-by: Rob Herring <robh@kernel.org>
> >>>>>>>>>
> >>>>>>>>> This patch has the undesirable side effect that it selects PCI_DOMAINS
> >>>>>>>>> for
> >>>>>>>>> sparc32:allmodconfig, which in turn results in
> >>>>>>>>>
> >>>>>>>>> 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'?
> >>>>>>>>>
> >>>>>>>>> Unfortunately, 37bd62d224c82 ("PCI: Enable PCI_DOMAINS along with
> >>>>>>>>> generic
> >>>>>>>>> PCI host controller") has pretty much the same result. No idea how to
> >>>>>>>>> fix
> >>>>>>>>> the problem, so I won't even try.
> >>>>>>>>
> >>>>>>>> Sorry about that.
> >>>>>>>>
> >>>>>>>> One option would consist in removing all PCI_DOMAINS selection from
> >>>>>>>> drivers/pci/controller/Kconfig and delegate it to arches even though
> >>>>>>>> this would force PCI_DOMAINS selection on all ARM platforms (it is
> >>>>>>>> already selected for ARCH_MULTIPLATFORM).
> >>>>>>>>
> >>>>>>>> Or we add back arch dependency to the relevant host bridges.
> >>>>>>>>
> >>>>>>>> Everything else I have in mind seems overkill to me given that this
> >>>>>>>> patch was added to improve test coverage (we could add a default
> >>>>>>>> pci_domain_nr() stub - weak or #define - that returns 0 in case arches
> >>>>>>>> do not provide an implementation but do we really want to do that ?).
> >>>>>>>>
> >>>>>>>> Thoughts appreciated.
> >>>>>>>>
> >>>>>>> From the definition of PCI_DOMAINS, I suspect the original idea was that
> >>>>>>> drivers should depend on it, not select it. Especially auto-selecting
> >>>>>>> it with PCI_HOST_GENERIC seems like a bad idea to me. However, that is
> >>>>>>> just me. I'll leave it up to Bjorn to decide what if anything he wants
> >>>>>>> to do about it.
> >>>>>>
> >>>>>> Here is a patch that should reinstate the previous behaviour but
> >>>>>> it will make PCI_DOMAINS a visible option on ARM 32-bit systems; whether
> >>>>>> that's acceptable that's the question I need to answer, it should
> >>>>>> honour old configs and it does not force PCI_DOMAINS selection on
> >>>>>> non-DT arch/arm PCI host controllers (that do not need PCI_DOMAINS
> >>>>>> anyway so I suspect that enabling it on all ARM 32-bit platforms
> >>>>>> should not break anything but I preferred to be cautious).
> >>>>>
> >>>>> I think this change will also require a patch enabling CONFIG_PCI_DOMAINS in
> >>>>> multi_v7_defconfig and iproc_defconfig at the very least?
> >>>>
> >>>> Perhaps the sub-arches that want this should select it. It is more a
> >>>> platform option/decision more than a controller option.
> >>>
> >>> Yes, that makes sense, I assume ARCH_VIRT is a sensible choice for
> >>> virtual machines configuration, Jan ?
> >>>
> >>> I will add a PCI_DOMAINS selection to all (ARM) arches that select
> >>> PCI_DOMAINS in drivers/pci/controller/Kconfig.
> >>>
> >>> For Jailhouse configurations I need Jan's input, I assume adding
> >>> the selection to ARCH_VIRT is the correct way forward, please let
> >>> me know asap.
> >>
> >> So far, there is no need on ARM or ARM64 declare a special platform in
> >> order to run as Jailhouse (secondary) guest.
> >>
> >> My original patch was just about making PCI_DOMAINS manually
> >> configurable, which would have been fine for our use case.
> >
> > I need more details on your system configuration to make sure we can fix
> > this in a way that does not upset anybody; I am not a big fan of making
> > PCI_DOMAINS visible since it affects other platforms and it is different
> > from how it is managed for all other arches, so please provide details.
>
> Our setup is as follows: A platform, e.g. Jetson TK1 or TX1/2, already
> has one PCI host controller. When enabling Jailhouse on it (from within
> a running Linux), this adds the generic PCI host controller as virtual
> one. So we need to configure the system to support both controllers and
> PCI domains. But, otherwise, the system does not look different from a
> physical one.
Well, it looks like the only sensible way forward is to amend the
patch below and make PCI_DOMAINS a visible option on ARM and still
select it from the sub-arch entries that need it:
https://marc.info/?l=linux-pci&m=152932092612352&w=2
If there are other ideas I am happy to hear them.
Lorenzo
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2018-06-19 9:27 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-05 19:31 [PATCH v4] PCI: improve host drivers compile test coverage Rob Herring
2018-05-18 21:42 ` Bjorn Helgaas
2018-05-19 19:22 ` Rob Herring
2018-06-12 17:02 ` [v4] " Guenter Roeck
2018-06-13 10:09 ` Lorenzo Pieralisi
2018-06-13 18:11 ` Guenter Roeck
2018-06-14 16:53 ` Lorenzo Pieralisi
2018-06-15 12:58 ` Lorenzo Pieralisi
2018-06-15 17:55 ` Scott Branden
2018-06-15 17:58 ` Scott Branden
2018-06-18 11:34 ` Lorenzo Pieralisi
2018-06-15 18:34 ` Rob Herring
2018-06-18 9:32 ` Lorenzo Pieralisi
2018-06-18 11:42 ` Jan Kiszka
2018-06-18 12:52 ` Lorenzo Pieralisi
2018-06-18 16:53 ` Jan Kiszka
2018-06-19 9:27 ` Lorenzo Pieralisi
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.