diff mbox series

[v2,2/2] PCI: j721e: Add support to build pci-j721e as module.

Message ID 20230108155755.2614147-3-a-verma1@ti.com (mailing list archive)
State Changes Requested
Delegated to: Krzysztof WilczyƄski
Headers show
Series Add support to build pci-j721e as a module. | expand

Commit Message

Achal Verma Jan. 8, 2023, 3:57 p.m. UTC
Add support to build pci-j721e as module.

Signed-off-by: Achal Verma <a-verma1@ti.com>
---
 drivers/pci/controller/cadence/Kconfig     | 10 +++++-----
 drivers/pci/controller/cadence/pci-j721e.c |  6 +++++-
 2 files changed, 10 insertions(+), 6 deletions(-)

Comments

Vignesh Raghavendra Jan. 9, 2023, 3:53 a.m. UTC | #1
Hi Achal,

On 08/01/23 21:27, Achal Verma wrote:
> Add support to build pci-j721e as module.
> 
> Signed-off-by: Achal Verma <a-verma1@ti.com>
> ---
>  drivers/pci/controller/cadence/Kconfig     | 10 +++++-----
>  drivers/pci/controller/cadence/pci-j721e.c |  6 +++++-
>  2 files changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/pci/controller/cadence/Kconfig b/drivers/pci/controller/cadence/Kconfig
> index 693c41fe32ce..51edf723586c 100644
> --- a/drivers/pci/controller/cadence/Kconfig
> +++ b/drivers/pci/controller/cadence/Kconfig
> @@ -43,12 +43,13 @@ config PCIE_CADENCE_PLAT_EP
>  	  different vendors SoCs.
>  
>  config PCI_J721E
> -	bool
> +	tristate
> +	select PCIE_CADENCE_HOST
> +	select PCIE_CADENCE_EP
>  

Please don't use select when symbol being selected, depends on
additional configs

Documentation/kbuild/kconfig-language.rst::

select should be used with care. select will force
a symbol to a value without visiting the dependencies.
By abusing select you are able to select a symbol FOO even
if FOO depends on BAR that is not set.


>  config PCI_J721E_HOST
> -	bool "TI J721E PCIe platform host controller"
> +	tristate "TI J721E PCIe platform host controller"
>  	depends on OF
> -	select PCIE_CADENCE_HOST
>  	select PCI_J721E
>  	help
>  	  Say Y here if you want to support the TI J721E PCIe platform
> @@ -56,10 +57,9 @@ config PCI_J721E_HOST
>  	  core.
>  
>  config PCI_J721E_EP
> -	bool "TI J721E PCIe platform endpoint controller"
> +	tristate "TI J721E PCIe platform endpoint controller"
>  	depends on OF
>  	depends on PCI_ENDPOINT
> -	select PCIE_CADENCE_EP
>  	select PCI_J721E
>  	help
>  	  Say Y here if you want to support the TI J721E PCIe platform
> diff --git a/drivers/pci/controller/cadence/pci-j721e.c b/drivers/pci/controller/cadence/pci-j721e.c
> index cc83a8925ce0..c4017fa6ae61 100644
> --- a/drivers/pci/controller/cadence/pci-j721e.c
> +++ b/drivers/pci/controller/cadence/pci-j721e.c
> @@ -13,6 +13,7 @@
>  #include <linux/irqchip/chained_irq.h>
>  #include <linux/irqdomain.h>
>  #include <linux/mfd/syscon.h>
> +#include <linux/module.h>
>  #include <linux/of.h>
>  #include <linux/of_device.h>
>  #include <linux/pci.h>
> @@ -565,4 +566,7 @@ static struct platform_driver j721e_pcie_driver = {
>  		.suppress_bind_attrs = true,
>  	},
>  };
> -builtin_platform_driver(j721e_pcie_driver);
> +module_platform_driver(j721e_pcie_driver);
> +
> +MODULE_AUTHOR("Kishon Vijay Abraham I <kishon@ti.com>");
> +MODULE_LICENSE("GPL v2");
Vignesh Raghavendra Jan. 9, 2023, 3:36 p.m. UTC | #2
On 09/01/23 6:05 pm, Achal Verma wrote:
> Discussed with Vignesh the current config dependency of pcie-cadence and pci-j721e modules,
> it seems like for now to modularize these drivers with minimal changes is to use "select"
> as they were used before in PCI_J721E_HOST and PCI_J721E_EP config options.
> 

With this patch its now impossible to build PCI_J721E_HOST without
pcie endpoint support (as PCI_ENDPOINT is now a dependency). I don't
know a way to achieve this via Kconfig magic w/o splitting pci-j721e.c
into EP/RC (like pcie-rcar* or pcie-rockchip*)


> Will push updated version with "depends on PCI_ENDPOINT" in PCI_J721E config to check
> dependency on PCI_ENDPOINT before selecting PCIE_CADENCE_EP.
> 

Please don't top post and respond inline:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

Regards
Vignesh

[...]
Bjorn Helgaas Jan. 10, 2023, 3:38 p.m. UTC | #3
On Mon, Jan 09, 2023 at 09:06:14PM +0530, Vignesh Raghavendra wrote:
> On 09/01/23 6:05 pm, Achal Verma wrote:
> > Discussed with Vignesh the current config dependency of pcie-cadence and pci-j721e modules,
> > it seems like for now to modularize these drivers with minimal changes is to use "select"
> > as they were used before in PCI_J721E_HOST and PCI_J721E_EP config options.
> 
> With this patch its now impossible to build PCI_J721E_HOST without
> pcie endpoint support (as PCI_ENDPOINT is now a dependency). I don't
> know a way to achieve this via Kconfig magic w/o splitting pci-j721e.c
> into EP/RC (like pcie-rcar* or pcie-rockchip*)
> 
> > Will push updated version with "depends on PCI_ENDPOINT" in PCI_J721E config to check
> > dependency on PCI_ENDPOINT before selecting PCIE_CADENCE_EP.
> > 
> 
> Please don't top post and respond inline:
> https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

Apparently there was also email from Achal to Vignesh that didn't make
it to the archives, probably because it was HTML or other "fancy"
email.  See the thread overview here, which is missing something:
https://lore.kernel.org/all/20230108155755.2614147-1-a-verma1@ti.com/

It's best to use plain text email when possible.  See
http://vger.kernel.org/majordomo-info.html for details.

Bjorn
diff mbox series

Patch

diff --git a/drivers/pci/controller/cadence/Kconfig b/drivers/pci/controller/cadence/Kconfig
index 693c41fe32ce..51edf723586c 100644
--- a/drivers/pci/controller/cadence/Kconfig
+++ b/drivers/pci/controller/cadence/Kconfig
@@ -43,12 +43,13 @@  config PCIE_CADENCE_PLAT_EP
 	  different vendors SoCs.
 
 config PCI_J721E
-	bool
+	tristate
+	select PCIE_CADENCE_HOST
+	select PCIE_CADENCE_EP
 
 config PCI_J721E_HOST
-	bool "TI J721E PCIe platform host controller"
+	tristate "TI J721E PCIe platform host controller"
 	depends on OF
-	select PCIE_CADENCE_HOST
 	select PCI_J721E
 	help
 	  Say Y here if you want to support the TI J721E PCIe platform
@@ -56,10 +57,9 @@  config PCI_J721E_HOST
 	  core.
 
 config PCI_J721E_EP
-	bool "TI J721E PCIe platform endpoint controller"
+	tristate "TI J721E PCIe platform endpoint controller"
 	depends on OF
 	depends on PCI_ENDPOINT
-	select PCIE_CADENCE_EP
 	select PCI_J721E
 	help
 	  Say Y here if you want to support the TI J721E PCIe platform
diff --git a/drivers/pci/controller/cadence/pci-j721e.c b/drivers/pci/controller/cadence/pci-j721e.c
index cc83a8925ce0..c4017fa6ae61 100644
--- a/drivers/pci/controller/cadence/pci-j721e.c
+++ b/drivers/pci/controller/cadence/pci-j721e.c
@@ -13,6 +13,7 @@ 
 #include <linux/irqchip/chained_irq.h>
 #include <linux/irqdomain.h>
 #include <linux/mfd/syscon.h>
+#include <linux/module.h>
 #include <linux/of.h>
 #include <linux/of_device.h>
 #include <linux/pci.h>
@@ -565,4 +566,7 @@  static struct platform_driver j721e_pcie_driver = {
 		.suppress_bind_attrs = true,
 	},
 };
-builtin_platform_driver(j721e_pcie_driver);
+module_platform_driver(j721e_pcie_driver);
+
+MODULE_AUTHOR("Kishon Vijay Abraham I <kishon@ti.com>");
+MODULE_LICENSE("GPL v2");