Re: [PATCH v6 5/6] PCI: fu740: Add SiFive FU740 PCIe host controller driver

[Bjorn Helgaas]

On Wed, May 05, 2021 at 12:26:31PM +0800, Greentime Hu wrote:
> Bjorn Helgaas <helgaas@xxxxxxxxxx> 於 2021年5月4日 週二 下午9:46寫道：
> > On Tue, May 04, 2021 at 06:59:39PM +0800, Greentime Hu wrote:
> > > From: Paul Walmsley <paul.walmsley@xxxxxxxxxx>
> > >
> > > Add driver for the SiFive FU740 PCIe host controller.
> > > This controller is based on the DesignWare PCIe core.
> > >
> > > Signed-off-by: Paul Walmsley <paul.walmsley@xxxxxxxxxx>
> > > Co-developed-by: Henry Styles <hes@xxxxxxxxxx>
> > > Signed-off-by: Henry Styles <hes@xxxxxxxxxx>
> > > Co-developed-by: Erik Danie <erik.danie@xxxxxxxxxx>
> > > Signed-off-by: Erik Danie <erik.danie@xxxxxxxxxx>
> > > Co-developed-by: Greentime Hu <greentime.hu@xxxxxxxxxx>
> > > Signed-off-by: Greentime Hu <greentime.hu@xxxxxxxxxx>
> > > ---
> > >  drivers/pci/controller/dwc/Kconfig      |  10 +
> > >  drivers/pci/controller/dwc/Makefile     |   1 +
> > >  drivers/pci/controller/dwc/pcie-fu740.c | 309 ++++++++++++++++++++++++
> > >  3 files changed, 320 insertions(+)
> > >  create mode 100644 drivers/pci/controller/dwc/pcie-fu740.c
> > >
> > > diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
> > > index 22c5529e9a65..255d43b1661b 100644
> > > --- a/drivers/pci/controller/dwc/Kconfig
> > > +++ b/drivers/pci/controller/dwc/Kconfig
> > > @@ -318,4 +318,14 @@ config PCIE_AL
> > >         required only for DT-based platforms. ACPI platforms with the
> > >         Annapurna Labs PCIe controller don't need to enable this.
> > >
> > > +config PCIE_FU740
> > > +     bool "SiFive FU740 PCIe host controller"
> > > +     depends on PCI_MSI_IRQ_DOMAIN
> > > +     depends on SOC_SIFIVE || COMPILE_TEST
> > > +     depends on GPIOLIB
> > ...
> > 2) I would prefer not to depend on GPIOLIB because it reduces
> >    compile-test coverage.  For example, the x86_64 defconfig does not
> >    enable GPIOLIB, so one must manually enable it to even be able to
> >    enable PCIE_FU740.
> > ...

> Sorry for late to debug this case. I was working on other works and
> just missed the email.
> How about this?

We already dropped the "depends on GPIOLIB" for v5.13.

You can add a select later, for v5.14.  Of course, you should post a
complete patch including commit log and signed-off-by.

And please take a look at how other drivers handle this so you do it
the same way.

> diff --git a/arch/riscv/Kconfig.socs b/arch/riscv/Kconfig.socs
> index e1b2690b6e45..66f57f2db49d 100644
> --- a/arch/riscv/Kconfig.socs
> +++ b/arch/riscv/Kconfig.socs
> @@ -7,6 +7,7 @@ config SOC_SIFIVE
>         select CLK_SIFIVE
>         select CLK_SIFIVE_PRCI
>         select SIFIVE_PLIC
> +       select GPIOLIB if PCIE_FU740
>         help
>           This enables support for SiFive SoC platform hardware.
> 
> diff --git a/drivers/pci/controller/dwc/Kconfig
> b/drivers/pci/controller/dwc/Kconfig
> index 255d43b1661b..0a37d21ed64e 100644
> --- a/drivers/pci/controller/dwc/Kconfig
> +++ b/drivers/pci/controller/dwc/Kconfig
> @@ -322,7 +322,6 @@ config PCIE_FU740
>         bool "SiFive FU740 PCIe host controller"
>         depends on PCI_MSI_IRQ_DOMAIN
>         depends on SOC_SIFIVE || COMPILE_TEST
> -       depends on GPIOLIB
>         select PCIE_DW_HOST
>         help
>           Say Y here if you want PCIe controller support for the SiFive