| Message ID | 20210623140103.47818-1-andriy.shevchenko@linux.intel.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v1,1/2] PCI: dwc: Clean up Kconfig dependencies (PCIE_DW_HOST) | expand |
On Wed, Jun 23, 2021 at 05:01:02PM +0300, Andy Shevchenko wrote: > First of all, the "depends on" is no-op in the selectable options. > Second, no need to repeat menu dependencies (PCI). > > Clean up the users of PCIE_DW_HOST and introduce idiom > > depends on PCI_MSI_IRQ_DOMAIN > select PCIE_DW_HOST > > for all of them. Any comments on the series?
On Wed, Jun 23, 2021 at 05:01:02PM +0300, Andy Shevchenko wrote: > First of all, the "depends on" is no-op in the selectable options. > Second, no need to repeat menu dependencies (PCI). I believe you need to rewrite the commit log in a more descriptive way - as it is it is not very descriptive. Define which specific "depends on" you are referring to. I appreciate the intent and I believe the patch is sound. > Clean up the users of PCIE_DW_HOST and introduce idiom > > depends on PCI_MSI_IRQ_DOMAIN > select PCIE_DW_HOST > > for all of them. "All of them" ? We need something more explicit for this log to be useful, it took me a while to understand the end result you are achieving. Thanks, Lorenzo > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > --- > drivers/pci/controller/dwc/Kconfig | 9 ++++----- > 1 file changed, 4 insertions(+), 5 deletions(-) > > diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig > index 423d35872ce4..9bfd41eadd5e 100644 > --- a/drivers/pci/controller/dwc/Kconfig > +++ b/drivers/pci/controller/dwc/Kconfig > @@ -8,7 +8,6 @@ config PCIE_DW > > config PCIE_DW_HOST > bool > - depends on PCI_MSI_IRQ_DOMAIN > select PCIE_DW > > config PCIE_DW_EP > @@ -22,8 +21,8 @@ config PCI_DRA7XX > config PCI_DRA7XX_HOST > bool "TI DRA7xx PCIe controller Host Mode" > depends on SOC_DRA7XX || COMPILE_TEST > - depends on PCI_MSI_IRQ_DOMAIN > depends on OF && HAS_IOMEM && TI_PIPE3 > + depends on PCI_MSI_IRQ_DOMAIN > select PCIE_DW_HOST > select PCI_DRA7XX > default y if SOC_DRA7XX > @@ -55,7 +54,7 @@ config PCIE_DW_PLAT > > config PCIE_DW_PLAT_HOST > bool "Platform bus based DesignWare PCIe Controller - Host mode" > - depends on PCI && PCI_MSI_IRQ_DOMAIN > + depends on PCI_MSI_IRQ_DOMAIN > select PCIE_DW_HOST > select PCIE_DW_PLAT > help > @@ -138,8 +137,8 @@ config PCI_LAYERSCAPE > bool "Freescale Layerscape PCIe controller - Host mode" > depends on OF && (ARM || ARCH_LAYERSCAPE || COMPILE_TEST) > depends on PCI_MSI_IRQ_DOMAIN > - select MFD_SYSCON > select PCIE_DW_HOST > + select MFD_SYSCON > help > Say Y here if you want to enable PCIe controller support on Layerscape > SoCs to work in Host mode. > @@ -244,8 +243,8 @@ config PCIE_HISI_STB > > config PCI_MESON > tristate "MESON PCIe controller" > - depends on PCI_MSI_IRQ_DOMAIN > default m if ARCH_MESON > + depends on PCI_MSI_IRQ_DOMAIN > select PCIE_DW_HOST > help > Say Y here if you want to enable PCI controller support on Amlogic > -- > 2.30.2 >
On Thu, Aug 05, 2021 at 02:52:34PM +0100, Lorenzo Pieralisi wrote: > On Wed, Jun 23, 2021 at 05:01:02PM +0300, Andy Shevchenko wrote: > > First of all, the "depends on" is no-op in the selectable options. > > Second, no need to repeat menu dependencies (PCI). > Define which specific "depends on" you are referring to. I didn't get this because it stands right. It's in general. I can be more specific since it's in align with the code, though. All the rest I agree with.
On Thu, Aug 05, 2021 at 07:35:38PM +0300, Andy Shevchenko wrote: > On Thu, Aug 05, 2021 at 02:52:34PM +0100, Lorenzo Pieralisi wrote: > > On Wed, Jun 23, 2021 at 05:01:02PM +0300, Andy Shevchenko wrote: > > > First of all, the "depends on" is no-op in the selectable options. > > > Second, no need to repeat menu dependencies (PCI). > > > Define which specific "depends on" you are referring to. > > I didn't get this because it stands right. It's in general. Ok, understood what you meant now - I read it as if you were referring to a specific Kconfig entry that this patch is fixing. Maybe: "The "depends on" Kconfig construct is a no-op in options that are selected and therefore has no effect. Remove it.". > I can be more specific since it's in align with the code, > though. > > All the rest I agree with. > > -- > With Best Regards, > Andy Shevchenko > >
On Wed, 23 Jun 2021 17:01:02 +0300, Andy Shevchenko wrote: > First of all, the "depends on" is no-op in the selectable options. > Second, no need to repeat menu dependencies (PCI). > > Clean up the users of PCIE_DW_HOST and introduce idiom > > depends on PCI_MSI_IRQ_DOMAIN > select PCIE_DW_HOST > > [...] Applied to pci/dwc, thanks! [1/2] PCI: dwc: Clean up Kconfig dependencies (PCIE_DW_HOST) https://git.kernel.org/lpieralisi/pci/c/a0ac7ee069 [2/2] PCI: dwc: Clean up Kconfig dependencies (PCIE_DW_EP) https://git.kernel.org/lpieralisi/pci/c/2d614eea21 Thanks, Lorenzo
On Tue, Oct 05, 2021 at 10:36:09AM +0100, Lorenzo Pieralisi wrote: > On Wed, 23 Jun 2021 17:01:02 +0300, Andy Shevchenko wrote: > > First of all, the "depends on" is no-op in the selectable options. > > Second, no need to repeat menu dependencies (PCI). > > > > Clean up the users of PCIE_DW_HOST and introduce idiom > > > > depends on PCI_MSI_IRQ_DOMAIN > > select PCIE_DW_HOST > > > > [...] > > Applied to pci/dwc, thanks! Sorry I have had no time to address your comments, but it seems you did it for me, thank you!
diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig index 423d35872ce4..9bfd41eadd5e 100644 --- a/drivers/pci/controller/dwc/Kconfig +++ b/drivers/pci/controller/dwc/Kconfig @@ -8,7 +8,6 @@ config PCIE_DW config PCIE_DW_HOST bool - depends on PCI_MSI_IRQ_DOMAIN select PCIE_DW config PCIE_DW_EP @@ -22,8 +21,8 @@ config PCI_DRA7XX config PCI_DRA7XX_HOST bool "TI DRA7xx PCIe controller Host Mode" depends on SOC_DRA7XX || COMPILE_TEST - depends on PCI_MSI_IRQ_DOMAIN depends on OF && HAS_IOMEM && TI_PIPE3 + depends on PCI_MSI_IRQ_DOMAIN select PCIE_DW_HOST select PCI_DRA7XX default y if SOC_DRA7XX @@ -55,7 +54,7 @@ config PCIE_DW_PLAT config PCIE_DW_PLAT_HOST bool "Platform bus based DesignWare PCIe Controller - Host mode" - depends on PCI && PCI_MSI_IRQ_DOMAIN + depends on PCI_MSI_IRQ_DOMAIN select PCIE_DW_HOST select PCIE_DW_PLAT help @@ -138,8 +137,8 @@ config PCI_LAYERSCAPE bool "Freescale Layerscape PCIe controller - Host mode" depends on OF && (ARM || ARCH_LAYERSCAPE || COMPILE_TEST) depends on PCI_MSI_IRQ_DOMAIN - select MFD_SYSCON select PCIE_DW_HOST + select MFD_SYSCON help Say Y here if you want to enable PCIe controller support on Layerscape SoCs to work in Host mode. @@ -244,8 +243,8 @@ config PCIE_HISI_STB config PCI_MESON tristate "MESON PCIe controller" - depends on PCI_MSI_IRQ_DOMAIN default m if ARCH_MESON + depends on PCI_MSI_IRQ_DOMAIN select PCIE_DW_HOST help Say Y here if you want to enable PCI controller support on Amlogic
First of all, the "depends on" is no-op in the selectable options. Second, no need to repeat menu dependencies (PCI). Clean up the users of PCIE_DW_HOST and introduce idiom depends on PCI_MSI_IRQ_DOMAIN select PCIE_DW_HOST for all of them. Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> --- drivers/pci/controller/dwc/Kconfig | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-)