2022-10-25 19:17:18

by Bjorn Helgaas

[permalink] [raw]
Subject: [PATCH 0/2] PCI: Drop controller CONFIG_OF dependencies

From: Bjorn Helgaas <[email protected]>

Drop controller CONFIG_OF dependencies so we can compile-test drivers even
without CONFIG_OF being enabled. OF provides stub functions when CONFIG_OF
is not set.

Bjorn Helgaas (2):
PCI: Drop of_match_ptr() to avoid unused variables
PCI: Drop controller CONFIG_OF dependencies

drivers/pci/controller/Kconfig | 15 ++-------------
drivers/pci/controller/cadence/Kconfig | 6 ------
drivers/pci/controller/dwc/Kconfig | 25 ++++++++++++-------------
drivers/pci/controller/mobiveil/Kconfig | 1 -
drivers/pci/controller/pci-ftpci100.c | 2 +-
drivers/pci/controller/pci-v3-semi.c | 2 +-
6 files changed, 16 insertions(+), 35 deletions(-)

--
2.25.1



2022-10-25 19:17:18

by Bjorn Helgaas

[permalink] [raw]
Subject: [PATCH 2/2] PCI: Drop controller CONFIG_OF dependencies

From: Bjorn Helgaas <[email protected]>

Many drivers depend on OF interfaces, so they won't be functional if
CONFIG_OF is not set. But OF provides stub functions in that case, so drop
the OF dependencies so we can at least compile-test the drivers.

This means we can compile-test the following drivers even without CONFIG_OF
enabled (many still require either CONFIG_COMPILE_TEST or the relevant
arch):

aardvark
al
apple
brcmstb
cadence-host
cadence-platform-host
cadence-ep
dra7xx-host
dra7xx-ep
dw-rockchip
ftpci100
hisi
intel-gw
ixp4xx
j721e-host
j721e-ep
kirin
layerscape-host
layerscape-ep
mediatek
microchip-host
mobiveil
mvebu
qcom-host
qcom-ep
rockchip-host
rockchip-ep
rockchip-dwc
pci-host-generic
uniphier-host
uniphier-ep
v3-semi
xilinx

Signed-off-by: Bjorn Helgaas <[email protected]>
---
drivers/pci/controller/Kconfig | 15 ++-------------
drivers/pci/controller/cadence/Kconfig | 6 ------
drivers/pci/controller/dwc/Kconfig | 25 ++++++++++++-------------
drivers/pci/controller/mobiveil/Kconfig | 1 -
4 files changed, 14 insertions(+), 33 deletions(-)

diff --git a/drivers/pci/controller/Kconfig b/drivers/pci/controller/Kconfig
index bfd9bac37e24..76806dc52d1b 100644
--- a/drivers/pci/controller/Kconfig
+++ b/drivers/pci/controller/Kconfig
@@ -8,7 +8,6 @@ config PCI_MVEBU
depends on ARCH_MVEBU || ARCH_DOVE || COMPILE_TEST
depends on MVEBU_MBUS
depends on ARM
- depends on OF
select PCI_BRIDGE_EMUL
help
Add support for Marvell EBU PCIe controller. This PCIe controller
@@ -18,7 +17,6 @@ config PCI_MVEBU
config PCI_AARDVARK
tristate "Aardvark PCIe controller"
depends on (ARCH_MVEBU && ARM64) || COMPILE_TEST
- depends on OF
depends on PCI_MSI_IRQ_DOMAIN
select PCI_BRIDGE_EMUL
help
@@ -38,13 +36,12 @@ config PCIE_XILINX_NWL

config PCI_FTPCI100
bool "Faraday Technology FTPCI100 PCI controller"
- depends on OF
default ARCH_GEMINI

config PCI_IXP4XX
bool "Intel IXP4xx PCI controller"
- depends on ARM && OF
depends on ARCH_IXP4XX || COMPILE_TEST
+ depends on ARM
default ARCH_IXP4XX
help
Say Y here if you want support for the PCI host controller found
@@ -89,7 +86,6 @@ config PCI_HOST_COMMON

config PCI_HOST_GENERIC
tristate "Generic PCI host controller"
- depends on OF
select PCI_HOST_COMMON
select IRQ_DOMAIN
help
@@ -98,7 +94,6 @@ config PCI_HOST_GENERIC

config PCIE_XILINX
bool "Xilinx AXI PCIe host bridge support"
- depends on OF || COMPILE_TEST
depends on PCI_MSI_IRQ_DOMAIN
help
Say 'Y' here if you want kernel to support the Xilinx AXI PCIe
@@ -132,7 +127,6 @@ config PCI_XGENE_MSI

config PCI_V3_SEMI
bool "V3 Semiconductor PCI controller"
- depends on OF
depends on ARM || COMPILE_TEST
default ARCH_INTEGRATOR_AP

@@ -214,7 +208,6 @@ config PCIE_ROCKCHIP
config PCIE_ROCKCHIP_HOST
tristate "Rockchip PCIe host controller"
depends on ARCH_ROCKCHIP || COMPILE_TEST
- depends on OF
depends on PCI_MSI_IRQ_DOMAIN
select MFD_SYSCON
select PCIE_ROCKCHIP
@@ -226,7 +219,6 @@ config PCIE_ROCKCHIP_HOST
config PCIE_ROCKCHIP_EP
bool "Rockchip PCIe endpoint controller"
depends on ARCH_ROCKCHIP || COMPILE_TEST
- depends on OF
depends on PCI_ENDPOINT
select MFD_SYSCON
select PCIE_ROCKCHIP
@@ -238,7 +230,6 @@ config PCIE_ROCKCHIP_EP
config PCIE_MEDIATEK
tristate "MediaTek PCIe controller"
depends on ARCH_AIROHA || ARCH_MEDIATEK || COMPILE_TEST
- depends on OF
depends on PCI_MSI_IRQ_DOMAIN
help
Say Y here if you want to enable PCIe controller support on
@@ -276,7 +267,6 @@ config PCIE_BRCMSTB
tristate "Broadcom Brcmstb PCIe host controller"
depends on ARCH_BRCMSTB || ARCH_BCM2835 || ARCH_BCMBCA || \
BMIPS_GENERIC || COMPILE_TEST
- depends on OF
depends on PCI_MSI_IRQ_DOMAIN
default ARCH_BRCMSTB || BMIPS_GENERIC
help
@@ -302,7 +292,7 @@ config PCI_LOONGSON

config PCIE_MICROCHIP_HOST
bool "Microchip AXI PCIe host bridge support"
- depends on PCI_MSI && OF
+ depends on PCI_MSI
select PCI_MSI_IRQ_DOMAIN
select GENERIC_MSI_IRQ_DOMAIN
select PCI_HOST_COMMON
@@ -325,7 +315,6 @@ config PCIE_APPLE_MSI_DOORBELL_ADDR
config PCIE_APPLE
tristate "Apple PCIe controller"
depends on ARCH_APPLE || COMPILE_TEST
- depends on OF
depends on PCI_MSI_IRQ_DOMAIN
select PCI_HOST_COMMON
help
diff --git a/drivers/pci/controller/cadence/Kconfig b/drivers/pci/controller/cadence/Kconfig
index 5d30564190e1..cb3b19f801a2 100644
--- a/drivers/pci/controller/cadence/Kconfig
+++ b/drivers/pci/controller/cadence/Kconfig
@@ -8,13 +8,11 @@ config PCIE_CADENCE

config PCIE_CADENCE_HOST
bool
- depends on OF
select IRQ_DOMAIN
select PCIE_CADENCE

config PCIE_CADENCE_EP
bool
- depends on OF
depends on PCI_ENDPOINT
select PCIE_CADENCE

@@ -23,7 +21,6 @@ config PCIE_CADENCE_PLAT

config PCIE_CADENCE_PLAT_HOST
bool "Cadence PCIe platform host controller"
- depends on OF
select PCIE_CADENCE_HOST
select PCIE_CADENCE_PLAT
help
@@ -33,7 +30,6 @@ config PCIE_CADENCE_PLAT_HOST

config PCIE_CADENCE_PLAT_EP
bool "Cadence PCIe platform endpoint controller"
- depends on OF
depends on PCI_ENDPOINT
select PCIE_CADENCE_EP
select PCIE_CADENCE_PLAT
@@ -47,7 +43,6 @@ config PCI_J721E

config PCI_J721E_HOST
bool "TI J721E PCIe platform host controller"
- depends on OF
select PCIE_CADENCE_HOST
select PCI_J721E
help
@@ -57,7 +52,6 @@ config PCI_J721E_HOST

config PCI_J721E_EP
bool "TI J721E PCIe platform endpoint controller"
- depends on OF
depends on PCI_ENDPOINT
select PCIE_CADENCE_EP
select PCI_J721E
diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
index 62ce3abf0f19..627006ac2cc0 100644
--- a/drivers/pci/controller/dwc/Kconfig
+++ b/drivers/pci/controller/dwc/Kconfig
@@ -20,7 +20,7 @@ config PCI_DRA7XX
config PCI_DRA7XX_HOST
tristate "TI DRA7xx PCIe controller Host Mode"
depends on SOC_DRA7XX || COMPILE_TEST
- depends on OF && HAS_IOMEM && TI_PIPE3
+ depends on HAS_IOMEM && TI_PIPE3
depends on PCI_MSI_IRQ_DOMAIN
select PCIE_DW_HOST
select PCI_DRA7XX
@@ -36,7 +36,7 @@ config PCI_DRA7XX_HOST
config PCI_DRA7XX_EP
tristate "TI DRA7xx PCIe controller Endpoint Mode"
depends on SOC_DRA7XX || COMPILE_TEST
- depends on OF && HAS_IOMEM && TI_PIPE3
+ depends on HAS_IOMEM && TI_PIPE3
depends on PCI_ENDPOINT
select PCIE_DW_EP
select PCI_DRA7XX
@@ -134,7 +134,7 @@ config PCI_KEYSTONE_EP

config PCI_LAYERSCAPE
bool "Freescale Layerscape PCIe controller - Host mode"
- depends on OF && (ARM || ARCH_LAYERSCAPE || COMPILE_TEST)
+ depends on ARM || ARCH_LAYERSCAPE || COMPILE_TEST
depends on PCI_MSI_IRQ_DOMAIN
select PCIE_DW_HOST
select MFD_SYSCON
@@ -147,7 +147,7 @@ config PCI_LAYERSCAPE

config PCI_LAYERSCAPE_EP
bool "Freescale Layerscape PCIe controller - Endpoint mode"
- depends on OF && (ARM || ARCH_LAYERSCAPE || COMPILE_TEST)
+ depends on ARM || ARCH_LAYERSCAPE || COMPILE_TEST
depends on PCI_ENDPOINT
select PCIE_DW_EP
help
@@ -158,7 +158,7 @@ config PCI_LAYERSCAPE_EP
controller works in RC mode.

config PCI_HISI
- depends on OF && (ARM64 || COMPILE_TEST)
+ depends on ARM64 || COMPILE_TEST
bool "HiSilicon Hip05 and Hip06 SoCs PCIe controllers"
depends on PCI_MSI_IRQ_DOMAIN
select PCIE_DW_HOST
@@ -169,7 +169,7 @@ config PCI_HISI

config PCIE_QCOM
bool "Qualcomm PCIe controller"
- depends on OF && (ARCH_QCOM || COMPILE_TEST)
+ depends on ARCH_QCOM || COMPILE_TEST
depends on PCI_MSI_IRQ_DOMAIN
select PCIE_DW_HOST
select CRC8
@@ -180,7 +180,7 @@ config PCIE_QCOM

config PCIE_QCOM_EP
tristate "Qualcomm PCIe controller - Endpoint mode"
- depends on OF && (ARCH_QCOM || COMPILE_TEST)
+ depends on ARCH_QCOM || COMPILE_TEST
depends on PCI_ENDPOINT
select PCIE_DW_EP
help
@@ -228,14 +228,13 @@ config PCIE_ROCKCHIP_DW_HOST
select PCIE_DW_HOST
depends on PCI_MSI_IRQ_DOMAIN
depends on ARCH_ROCKCHIP || COMPILE_TEST
- depends on OF
help
Enables support for the DesignWare PCIe controller in the
Rockchip SoC except RK3399.

config PCIE_INTEL_GW
bool "Intel Gateway PCIe host controller support"
- depends on OF && (X86 || COMPILE_TEST)
+ depends on X86 || COMPILE_TEST
depends on PCI_MSI_IRQ_DOMAIN
select PCIE_DW_HOST
help
@@ -273,7 +272,7 @@ config PCIE_KEEMBAY_EP
DesignWare core functions.

config PCIE_KIRIN
- depends on OF && (ARM64 || COMPILE_TEST)
+ depends on ARM64 || COMPILE_TEST
tristate "HiSilicon Kirin series SoCs PCIe controllers"
depends on PCI_MSI_IRQ_DOMAIN
select PCIE_DW_HOST
@@ -345,7 +344,7 @@ config PCIE_VISCONTI_HOST
config PCIE_UNIPHIER
bool "Socionext UniPhier PCIe host controllers"
depends on ARCH_UNIPHIER || COMPILE_TEST
- depends on OF && HAS_IOMEM
+ depends on HAS_IOMEM
depends on PCI_MSI_IRQ_DOMAIN
select PCIE_DW_HOST
help
@@ -355,7 +354,7 @@ config PCIE_UNIPHIER
config PCIE_UNIPHIER_EP
bool "Socionext UniPhier PCIe endpoint controllers"
depends on ARCH_UNIPHIER || COMPILE_TEST
- depends on OF && HAS_IOMEM
+ depends on HAS_IOMEM
depends on PCI_ENDPOINT
select PCIE_DW_EP
help
@@ -364,7 +363,7 @@ config PCIE_UNIPHIER_EP

config PCIE_AL
bool "Amazon Annapurna Labs PCIe controller"
- depends on OF && (ARM64 || COMPILE_TEST)
+ depends on ARM64 || COMPILE_TEST
depends on PCI_MSI_IRQ_DOMAIN
select PCIE_DW_HOST
select PCI_ECAM
diff --git a/drivers/pci/controller/mobiveil/Kconfig b/drivers/pci/controller/mobiveil/Kconfig
index e4643fb94e78..10f32dd123f6 100644
--- a/drivers/pci/controller/mobiveil/Kconfig
+++ b/drivers/pci/controller/mobiveil/Kconfig
@@ -14,7 +14,6 @@ config PCIE_MOBIVEIL_HOST
config PCIE_MOBIVEIL_PLAT
bool "Mobiveil AXI PCIe controller"
depends on ARCH_ZYNQMP || COMPILE_TEST
- depends on OF
depends on PCI_MSI_IRQ_DOMAIN
select PCIE_MOBIVEIL_HOST
help
--
2.25.1


2022-11-10 21:15:31

by Bjorn Helgaas

[permalink] [raw]
Subject: Re: [PATCH 0/2] PCI: Drop controller CONFIG_OF dependencies

On Tue, Oct 25, 2022 at 02:13:37PM -0500, Bjorn Helgaas wrote:
> From: Bjorn Helgaas <[email protected]>
>
> Drop controller CONFIG_OF dependencies so we can compile-test drivers even
> without CONFIG_OF being enabled. OF provides stub functions when CONFIG_OF
> is not set.
>
> Bjorn Helgaas (2):
> PCI: Drop of_match_ptr() to avoid unused variables
> PCI: Drop controller CONFIG_OF dependencies
>
> drivers/pci/controller/Kconfig | 15 ++-------------
> drivers/pci/controller/cadence/Kconfig | 6 ------
> drivers/pci/controller/dwc/Kconfig | 25 ++++++++++++-------------
> drivers/pci/controller/mobiveil/Kconfig | 1 -
> drivers/pci/controller/pci-ftpci100.c | 2 +-
> drivers/pci/controller/pci-v3-semi.c | 2 +-
> 6 files changed, 16 insertions(+), 35 deletions(-)

I put these on my pci/kbuild branch and propose to merge them at the
end. Lorenzo, let me know if you prefer something else.

2022-11-15 16:57:11

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH 2/2] PCI: Drop controller CONFIG_OF dependencies

Hi Bjorn,

On Tue, Oct 25, 2022 at 9:16 PM Bjorn Helgaas <[email protected]> wrote:
> From: Bjorn Helgaas <[email protected]>
>
> Many drivers depend on OF interfaces, so they won't be functional if
> CONFIG_OF is not set. But OF provides stub functions in that case, so drop
> the OF dependencies so we can at least compile-test the drivers.
>
> This means we can compile-test the following drivers even without CONFIG_OF
> enabled (many still require either CONFIG_COMPILE_TEST or the relevant
> arch):
>
> aardvark
> al
> apple
> brcmstb
> cadence-host
> cadence-platform-host
> cadence-ep
> dra7xx-host
> dra7xx-ep
> dw-rockchip
> ftpci100
> hisi
> intel-gw
> ixp4xx
> j721e-host
> j721e-ep
> kirin
> layerscape-host
> layerscape-ep
> mediatek
> microchip-host
> mobiveil
> mvebu
> qcom-host
> qcom-ep
> rockchip-host
> rockchip-ep
> rockchip-dwc
> pci-host-generic
> uniphier-host
> uniphier-ep
> v3-semi
> xilinx
>
> Signed-off-by: Bjorn Helgaas <[email protected]>

Thanks for your patch, which is now commit 51dfb612ab3becf9 ("PCI: Drop
controller CONFIG_OF dependencies") in pci/next.

> --- a/drivers/pci/controller/Kconfig
> +++ b/drivers/pci/controller/Kconfig
> @@ -8,7 +8,6 @@ config PCI_MVEBU
> depends on ARCH_MVEBU || ARCH_DOVE || COMPILE_TEST
> depends on MVEBU_MBUS
> depends on ARM
> - depends on OF

This is exactly why we have the COMPILE_TEST symbol.
There is no point in bothering all users who configure kernels with
questions about drivers that won't function anyway due to missing
dependencies, unless the user explicitly wants to do compile-testing.

So all of these should become:

depends on OF || COMPILE_TEST

> select PCI_BRIDGE_EMUL
> help
> Add support for Marvell EBU PCIe controller. This PCIe controller

> @@ -98,7 +94,6 @@ config PCI_HOST_GENERIC
>
> config PCIE_XILINX
> bool "Xilinx AXI PCIe host bridge support"
> - depends on OF || COMPILE_TEST

This one was fine.

> depends on PCI_MSI_IRQ_DOMAIN
> help
> Say 'Y' here if you want kernel to support the Xilinx AXI PCIe

Thanks in advance for reverting or fixing!

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

2022-11-15 18:34:30

by Bjorn Helgaas

[permalink] [raw]
Subject: Re: [PATCH 2/2] PCI: Drop controller CONFIG_OF dependencies

On Tue, Nov 15, 2022 at 04:56:10PM +0100, Geert Uytterhoeven wrote:
> On Tue, Oct 25, 2022 at 9:16 PM Bjorn Helgaas <[email protected]> wrote:
> > From: Bjorn Helgaas <[email protected]>
> >
> > Many drivers depend on OF interfaces, so they won't be functional if
> > CONFIG_OF is not set. But OF provides stub functions in that case, so drop
> > the OF dependencies so we can at least compile-test the drivers.

> > --- a/drivers/pci/controller/Kconfig
> > +++ b/drivers/pci/controller/Kconfig
> > @@ -8,7 +8,6 @@ config PCI_MVEBU
> > depends on ARCH_MVEBU || ARCH_DOVE || COMPILE_TEST
> > depends on MVEBU_MBUS
> > depends on ARM
> > - depends on OF
>
> This is exactly why we have the COMPILE_TEST symbol.
> There is no point in bothering all users who configure kernels with
> questions about drivers that won't function anyway due to missing
> dependencies, unless the user explicitly wants to do compile-testing.
>
> So all of these should become:
>
> depends on OF || COMPILE_TEST

Oh, yes, thanks for pointing this out, I totally blew it here. I
dropped this while we figure it out.

Do you have a preference between this:

depends on ARCH_MVEBU || ARCH_DOVE || COMPILE_TEST
depends on OF || COMPILE_TEST

and this:

depends on ((ARCH_MVEBU || ARCH_DOVE) && OF) || COMPILE_TEST

The latter seems more common and doesn't repeat "COMPILE_TEST", but I
can see advantages to the former.

Bjorn

2022-11-15 19:36:06

by Pali Rohár

[permalink] [raw]
Subject: Re: [PATCH 2/2] PCI: Drop controller CONFIG_OF dependencies

On Tuesday 15 November 2022 12:07:34 Bjorn Helgaas wrote:
> On Tue, Nov 15, 2022 at 04:56:10PM +0100, Geert Uytterhoeven wrote:
> > On Tue, Oct 25, 2022 at 9:16 PM Bjorn Helgaas <[email protected]> wrote:
> > > From: Bjorn Helgaas <[email protected]>
> > >
> > > Many drivers depend on OF interfaces, so they won't be functional if
> > > CONFIG_OF is not set. But OF provides stub functions in that case, so drop
> > > the OF dependencies so we can at least compile-test the drivers.
>
> > > --- a/drivers/pci/controller/Kconfig
> > > +++ b/drivers/pci/controller/Kconfig
> > > @@ -8,7 +8,6 @@ config PCI_MVEBU
> > > depends on ARCH_MVEBU || ARCH_DOVE || COMPILE_TEST
> > > depends on MVEBU_MBUS
> > > depends on ARM
> > > - depends on OF
> >
> > This is exactly why we have the COMPILE_TEST symbol.
> > There is no point in bothering all users who configure kernels with
> > questions about drivers that won't function anyway due to missing
> > dependencies, unless the user explicitly wants to do compile-testing.
> >
> > So all of these should become:
> >
> > depends on OF || COMPILE_TEST
>
> Oh, yes, thanks for pointing this out, I totally blew it here. I
> dropped this while we figure it out.

I agree too, it really makes sense to not provide pci-mvebu driver to
end users without OF - it would not work.

Anyway, it is needed to declare "depends on ARM" for pci-mvebu?
Both supported architectures (mvebu and dove) are ARM.

> Do you have a preference between this:
>
> depends on ARCH_MVEBU || ARCH_DOVE || COMPILE_TEST
> depends on OF || COMPILE_TEST
>
> and this:
>
> depends on ((ARCH_MVEBU || ARCH_DOVE) && OF) || COMPILE_TEST
>
> The latter seems more common and doesn't repeat "COMPILE_TEST", but I
> can see advantages to the former.
>
> Bjorn

First one looks to be more readable as it does not contain parenthesis.

And if one day there would be support also for other Marvell SoCs
currently missing in pci-mvebu.c then diff for new ARCH_ in Kconfig
would be smaller.

2022-11-15 20:23:02

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH 2/2] PCI: Drop controller CONFIG_OF dependencies

Hi Bjorn,

On Tue, Nov 15, 2022 at 7:07 PM Bjorn Helgaas <[email protected]> wrote:
> On Tue, Nov 15, 2022 at 04:56:10PM +0100, Geert Uytterhoeven wrote:
> > On Tue, Oct 25, 2022 at 9:16 PM Bjorn Helgaas <[email protected]> wrote:
> > > From: Bjorn Helgaas <[email protected]>
> > >
> > > Many drivers depend on OF interfaces, so they won't be functional if
> > > CONFIG_OF is not set. But OF provides stub functions in that case, so drop
> > > the OF dependencies so we can at least compile-test the drivers.
>
> > > --- a/drivers/pci/controller/Kconfig
> > > +++ b/drivers/pci/controller/Kconfig
> > > @@ -8,7 +8,6 @@ config PCI_MVEBU
> > > depends on ARCH_MVEBU || ARCH_DOVE || COMPILE_TEST
> > > depends on MVEBU_MBUS
> > > depends on ARM
> > > - depends on OF
> >
> > This is exactly why we have the COMPILE_TEST symbol.
> > There is no point in bothering all users who configure kernels with
> > questions about drivers that won't function anyway due to missing
> > dependencies, unless the user explicitly wants to do compile-testing.
> >
> > So all of these should become:
> >
> > depends on OF || COMPILE_TEST
>
> Oh, yes, thanks for pointing this out, I totally blew it here. I
> dropped this while we figure it out.
>
> Do you have a preference between this:
>
> depends on ARCH_MVEBU || ARCH_DOVE || COMPILE_TEST
> depends on OF || COMPILE_TEST
>
> and this:
>
> depends on ((ARCH_MVEBU || ARCH_DOVE) && OF) || COMPILE_TEST
>
> The latter seems more common and doesn't repeat "COMPILE_TEST", but I
> can see advantages to the former.

Personally, I prefer the former, as it is easier to parse for a human.
Thanks!

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

2022-11-15 22:13:50

by Bjorn Helgaas

[permalink] [raw]
Subject: Re: [PATCH 2/2] PCI: Drop controller CONFIG_OF dependencies

On Tue, Nov 15, 2022 at 08:31:55PM +0100, Pali Roh?r wrote:
> On Tuesday 15 November 2022 12:07:34 Bjorn Helgaas wrote:
> > On Tue, Nov 15, 2022 at 04:56:10PM +0100, Geert Uytterhoeven wrote:
> > > On Tue, Oct 25, 2022 at 9:16 PM Bjorn Helgaas <[email protected]> wrote:
> > > > From: Bjorn Helgaas <[email protected]>
> > > >
> > > > Many drivers depend on OF interfaces, so they won't be functional if
> > > > CONFIG_OF is not set. But OF provides stub functions in that case, so drop
> > > > the OF dependencies so we can at least compile-test the drivers.
> >
> > > > --- a/drivers/pci/controller/Kconfig
> > > > +++ b/drivers/pci/controller/Kconfig
> > > > @@ -8,7 +8,6 @@ config PCI_MVEBU
> > > > depends on ARCH_MVEBU || ARCH_DOVE || COMPILE_TEST
> > > > depends on MVEBU_MBUS
> > > > depends on ARM
> > > > - depends on OF
> > >
> > > This is exactly why we have the COMPILE_TEST symbol.
> > > There is no point in bothering all users who configure kernels with
> > > questions about drivers that won't function anyway due to missing
> > > dependencies, unless the user explicitly wants to do compile-testing.
> > >
> > > So all of these should become:
> > >
> > > depends on OF || COMPILE_TEST
> >
> > Oh, yes, thanks for pointing this out, I totally blew it here. I
> > dropped this while we figure it out.
>
> I agree too, it really makes sense to not provide pci-mvebu driver to
> end users without OF - it would not work.
>
> Anyway, it is needed to declare "depends on ARM" for pci-mvebu?
> Both supported architectures (mvebu and dove) are ARM.

If you're asking whether "depends on ARM" can be removed for
PCI_MVEBU, I don't know the answer. I do see that CONFIG_ARCH_MVEBU
is mentioned in arch/arm64, which doesn't seem to set CONFIG_ARM.

In any event, I would consider a change like that to be a separate
patch. Here I just want to focus on CONFIG_OF.

Bjorn

2022-11-15 22:14:56

by Pali Rohár

[permalink] [raw]
Subject: Re: [PATCH 2/2] PCI: Drop controller CONFIG_OF dependencies

On Tuesday 15 November 2022 14:53:43 Bjorn Helgaas wrote:
> On Tue, Nov 15, 2022 at 08:31:55PM +0100, Pali Rohár wrote:
> > On Tuesday 15 November 2022 12:07:34 Bjorn Helgaas wrote:
> > > On Tue, Nov 15, 2022 at 04:56:10PM +0100, Geert Uytterhoeven wrote:
> > > > On Tue, Oct 25, 2022 at 9:16 PM Bjorn Helgaas <[email protected]> wrote:
> > > > > From: Bjorn Helgaas <[email protected]>
> > > > >
> > > > > Many drivers depend on OF interfaces, so they won't be functional if
> > > > > CONFIG_OF is not set. But OF provides stub functions in that case, so drop
> > > > > the OF dependencies so we can at least compile-test the drivers.
> > >
> > > > > --- a/drivers/pci/controller/Kconfig
> > > > > +++ b/drivers/pci/controller/Kconfig
> > > > > @@ -8,7 +8,6 @@ config PCI_MVEBU
> > > > > depends on ARCH_MVEBU || ARCH_DOVE || COMPILE_TEST
> > > > > depends on MVEBU_MBUS
> > > > > depends on ARM
> > > > > - depends on OF
> > > >
> > > > This is exactly why we have the COMPILE_TEST symbol.
> > > > There is no point in bothering all users who configure kernels with
> > > > questions about drivers that won't function anyway due to missing
> > > > dependencies, unless the user explicitly wants to do compile-testing.
> > > >
> > > > So all of these should become:
> > > >
> > > > depends on OF || COMPILE_TEST
> > >
> > > Oh, yes, thanks for pointing this out, I totally blew it here. I
> > > dropped this while we figure it out.
> >
> > I agree too, it really makes sense to not provide pci-mvebu driver to
> > end users without OF - it would not work.
> >
> > Anyway, it is needed to declare "depends on ARM" for pci-mvebu?
> > Both supported architectures (mvebu and dove) are ARM.
>
> If you're asking whether "depends on ARM" can be removed for
> PCI_MVEBU, I don't know the answer. I do see that CONFIG_ARCH_MVEBU
> is mentioned in arch/arm64, which doesn't seem to set CONFIG_ARM.

Ou, you are right. Correct interpretation is "depends on 32-bit mvebu".
And because ARCH_MVEBU means "32-bit mvebu OR 64-bit mvebu" there is
additional "depends on ARM" to filter-out 64-bit mvebu.

So _architecture_ dependency based on intention should be:

(ARCH_MVEBU && ARM) || ARCH_DOVE || COMPILE_TEST

But because there is no 64-bit Dove, it has same meaning as moving ARM
from inner conjunction to outer.

I do not know which syntax is better. So choose what you prefer (current
version or some modification).

> In any event, I would consider a change like that to be a separate
> patch. Here I just want to focus on CONFIG_OF.
>
> Bjorn