[2/2] PCI: Drop controller CONFIG_OF dependencies

  From: Bjorn Helgaas <bhelgaas@google.com>

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 <bhelgaas@google.com>
---
 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(-)
  

Hi Bjorn,

On Tue, Oct 25, 2022 at 9:16 PM Bjorn Helgaas <helgaas@kernel.org> wrote:
> From: Bjorn Helgaas <bhelgaas@google.com>
>
> 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 <bhelgaas@google.com>

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 -- geert@linux-m68k.org

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
  

On Tue, Nov 15, 2022 at 04:56:10PM +0100, Geert Uytterhoeven wrote:
> On Tue, Oct 25, 2022 at 9:16 PM Bjorn Helgaas <helgaas@kernel.org> wrote:
> > From: Bjorn Helgaas <bhelgaas@google.com>
> >
> > 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
  

Hi Bjorn,

On Tue, Nov 15, 2022 at 7:07 PM Bjorn Helgaas <helgaas@kernel.org> 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 <helgaas@kernel.org> wrote:
> > > From: Bjorn Helgaas <bhelgaas@google.com>
> > >
> > > 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 -- geert@linux-m68k.org

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
  

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 <helgaas@kernel.org> wrote:
> > > From: Bjorn Helgaas <bhelgaas@google.com>
> > >
> > > 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.
  

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 <helgaas@kernel.org> wrote:
> > > > From: Bjorn Helgaas <bhelgaas@google.com>
> > > >
> > > > 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
  

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 <helgaas@kernel.org> wrote:
> > > > > From: Bjorn Helgaas <bhelgaas@google.com>
> > > > >
> > > > > 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