[v4,2/4] soc: ti: Add module build support

Message ID 20221108181144.433087-3-nfrayer@baylibre.com
State New
Headers
Series soc: ti: Add module build support to the socinfo |

Commit Message

Nicolas Frayer Nov. 8, 2022, 6:11 p.m. UTC
  Added module build support for the TI K3 SoC info driver.

Signed-off-by: Nicolas Frayer <nfrayer@baylibre.com>
---
 arch/arm64/Kconfig.platforms |  1 -
 drivers/soc/ti/Kconfig       |  3 ++-
 drivers/soc/ti/k3-socinfo.c  | 11 +++++++++++
 3 files changed, 13 insertions(+), 2 deletions(-)
  

Comments

Randy Dunlap Nov. 8, 2022, 6:18 p.m. UTC | #1
Hi--

On 11/8/22 10:11, Nicolas Frayer wrote:
> Added module build support for the TI K3 SoC info driver.
> 
> Signed-off-by: Nicolas Frayer <nfrayer@baylibre.com>
> ---
>  arch/arm64/Kconfig.platforms |  1 -
>  drivers/soc/ti/Kconfig       |  3 ++-
>  drivers/soc/ti/k3-socinfo.c  | 11 +++++++++++
>  3 files changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms
> index 76580b932e44..4f2f92eb499f 100644
> --- a/arch/arm64/Kconfig.platforms
> +++ b/arch/arm64/Kconfig.platforms
> @@ -130,7 +130,6 @@ config ARCH_K3
>  	select TI_SCI_PROTOCOL
>  	select TI_SCI_INTR_IRQCHIP
>  	select TI_SCI_INTA_IRQCHIP
> -	select TI_K3_SOCINFO
>  	help
>  	  This enables support for Texas Instruments' K3 multicore SoC
>  	  architecture.
> diff --git a/drivers/soc/ti/Kconfig b/drivers/soc/ti/Kconfig
> index 7e2fb1c16af1..1a730c057cce 100644
> --- a/drivers/soc/ti/Kconfig
> +++ b/drivers/soc/ti/Kconfig
> @@ -74,7 +74,8 @@ config TI_K3_RINGACC
>  	  If unsure, say N.
>  
>  config TI_K3_SOCINFO
> -	bool
> +	tristate "TI K3 SoC info driver"
> +	default y

Maybe
	default ARCH_K3
?

>  	depends on ARCH_K3 || COMPILE_TEST
>  	select SOC_BUS
>  	select MFD_SYSCON
  
Péter Ujfalusi Nov. 24, 2022, 7:54 a.m. UTC | #2
On 08/11/2022 20:11, Nicolas Frayer wrote:
> Added module build support for the TI K3 SoC info driver.

Subject: "soc: ti: k3-socinfo: ..."

> 
> Signed-off-by: Nicolas Frayer <nfrayer@baylibre.com>
> ---
>   arch/arm64/Kconfig.platforms |  1 -
>   drivers/soc/ti/Kconfig       |  3 ++-
>   drivers/soc/ti/k3-socinfo.c  | 11 +++++++++++
>   3 files changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms
> index 76580b932e44..4f2f92eb499f 100644
> --- a/arch/arm64/Kconfig.platforms
> +++ b/arch/arm64/Kconfig.platforms
> @@ -130,7 +130,6 @@ config ARCH_K3
>   	select TI_SCI_PROTOCOL
>   	select TI_SCI_INTR_IRQCHIP
>   	select TI_SCI_INTA_IRQCHIP
> -	select TI_K3_SOCINFO
>   	help
>   	  This enables support for Texas Instruments' K3 multicore SoC
>   	  architecture.
> diff --git a/drivers/soc/ti/Kconfig b/drivers/soc/ti/Kconfig
> index 7e2fb1c16af1..1a730c057cce 100644
> --- a/drivers/soc/ti/Kconfig
> +++ b/drivers/soc/ti/Kconfig
> @@ -74,7 +74,8 @@ config TI_K3_RINGACC
>   	  If unsure, say N.
>   
>   config TI_K3_SOCINFO
> -	bool
> +	tristate "TI K3 SoC info driver"
> +	default y

Why it is a good thing to have this driver as module compared to always 
built in?
It has no dependencies, just things depending on it.
It is small, just couple of lines long

I don't really see the benefit of building it as a module, not even an 
academic one...


>   	depends on ARCH_K3 || COMPILE_TEST
>   	select SOC_BUS
>   	select MFD_SYSCON
> diff --git a/drivers/soc/ti/k3-socinfo.c b/drivers/soc/ti/k3-socinfo.c
> index 19f3e74f5376..98348f998e0f 100644
> --- a/drivers/soc/ti/k3-socinfo.c
> +++ b/drivers/soc/ti/k3-socinfo.c
> @@ -13,6 +13,7 @@
>   #include <linux/slab.h>
>   #include <linux/string.h>
>   #include <linux/sys_soc.h>
> +#include <linux/module.h>
>   
>   #define CTRLMMR_WKUP_JTAGID_REG		0
>   /*
> @@ -141,6 +142,7 @@ static const struct of_device_id k3_chipinfo_of_match[] = {
>   	{ .compatible = "ti,am654-chipid", },
>   	{ /* sentinel */ },
>   };
> +MODULE_DEVICE_TABLE(of, k3_chipinfo_of_match);
>   
>   static struct platform_driver k3_chipinfo_driver = {
>   	.driver = {
> @@ -156,3 +158,12 @@ static int __init k3_chipinfo_init(void)
>   	return platform_driver_register(&k3_chipinfo_driver);
>   }
>   subsys_initcall(k3_chipinfo_init);

subsys_initcall for a module?

> +
> +static void __exit k3_chipinfo_exit(void)
> +{
> +	platform_driver_unregister(&k3_chipinfo_driver);
> +}
> +module_exit(k3_chipinfo_exit);
> +
> +MODULE_DESCRIPTION("TI K3 SoC info driver");
> +MODULE_LICENSE("GPL");
  
Nicolas Frayer Nov. 24, 2022, 9:01 a.m. UTC | #3
Le jeu. 24 nov. 2022 à 08:53, Péter Ujfalusi
<peter.ujfalusi@gmail.com> a écrit :

Hi Peter,
>
>
>
> On 08/11/2022 20:11, Nicolas Frayer wrote:
> > Added module build support for the TI K3 SoC info driver.
>
> Subject: "soc: ti: k3-socinfo: ..."
>
> >
> > Signed-off-by: Nicolas Frayer <nfrayer@baylibre.com>
> > ---
> >   arch/arm64/Kconfig.platforms |  1 -
> >   drivers/soc/ti/Kconfig       |  3 ++-
> >   drivers/soc/ti/k3-socinfo.c  | 11 +++++++++++
> >   3 files changed, 13 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms
> > index 76580b932e44..4f2f92eb499f 100644
> > --- a/arch/arm64/Kconfig.platforms
> > +++ b/arch/arm64/Kconfig.platforms
> > @@ -130,7 +130,6 @@ config ARCH_K3
> >       select TI_SCI_PROTOCOL
> >       select TI_SCI_INTR_IRQCHIP
> >       select TI_SCI_INTA_IRQCHIP
> > -     select TI_K3_SOCINFO
> >       help
> >         This enables support for Texas Instruments' K3 multicore SoC
> >         architecture.
> > diff --git a/drivers/soc/ti/Kconfig b/drivers/soc/ti/Kconfig
> > index 7e2fb1c16af1..1a730c057cce 100644
> > --- a/drivers/soc/ti/Kconfig
> > +++ b/drivers/soc/ti/Kconfig
> > @@ -74,7 +74,8 @@ config TI_K3_RINGACC
> >         If unsure, say N.
> >
> >   config TI_K3_SOCINFO
> > -     bool
> > +     tristate "TI K3 SoC info driver"
> > +     default y
>
> Why it is a good thing to have this driver as module compared to always
> built in?
> It has no dependencies, just things depending on it.
> It is small, just couple of lines long
>
> I don't really see the benefit of building it as a module, not even an
> academic one...
>

Just to give an update, this series will be dropped as it introduces
dependency issues
with consumer drivers.
The reason why I've enabled the module build support is because it is
required to build
vendor drivers as modules for Android GKI feature.

>
> >       depends on ARCH_K3 || COMPILE_TEST
> >       select SOC_BUS
> >       select MFD_SYSCON
> > diff --git a/drivers/soc/ti/k3-socinfo.c b/drivers/soc/ti/k3-socinfo.c
> > index 19f3e74f5376..98348f998e0f 100644
> > --- a/drivers/soc/ti/k3-socinfo.c
> > +++ b/drivers/soc/ti/k3-socinfo.c
> > @@ -13,6 +13,7 @@
> >   #include <linux/slab.h>
> >   #include <linux/string.h>
> >   #include <linux/sys_soc.h>
> > +#include <linux/module.h>
> >
> >   #define CTRLMMR_WKUP_JTAGID_REG             0
> >   /*
> > @@ -141,6 +142,7 @@ static const struct of_device_id k3_chipinfo_of_match[] = {
> >       { .compatible = "ti,am654-chipid", },
> >       { /* sentinel */ },
> >   };
> > +MODULE_DEVICE_TABLE(of, k3_chipinfo_of_match);
> >
> >   static struct platform_driver k3_chipinfo_driver = {
> >       .driver = {
> > @@ -156,3 +158,12 @@ static int __init k3_chipinfo_init(void)
> >       return platform_driver_register(&k3_chipinfo_driver);
> >   }
> >   subsys_initcall(k3_chipinfo_init);
>
> subsys_initcall for a module?

By including module.h, the subsys_initcall() is redefined as
module_init() when built
as a module. When built-in, it is redefined as the usual __initcall.

>
> > +
> > +static void __exit k3_chipinfo_exit(void)
> > +{
> > +     platform_driver_unregister(&k3_chipinfo_driver);
> > +}
> > +module_exit(k3_chipinfo_exit);
> > +
> > +MODULE_DESCRIPTION("TI K3 SoC info driver");
> > +MODULE_LICENSE("GPL");
>
> --
> Péter
  
Nicolas Frayer Nov. 24, 2022, 9:04 a.m. UTC | #4
Hi Randy,

Le mar. 8 nov. 2022 à 19:18, Randy Dunlap <rdunlap@infradead.org> a écrit :
>
> Hi--
>
> On 11/8/22 10:11, Nicolas Frayer wrote:
> > Added module build support for the TI K3 SoC info driver.
> >
> > Signed-off-by: Nicolas Frayer <nfrayer@baylibre.com>
> > ---
> >  arch/arm64/Kconfig.platforms |  1 -
> >  drivers/soc/ti/Kconfig       |  3 ++-
> >  drivers/soc/ti/k3-socinfo.c  | 11 +++++++++++
> >  3 files changed, 13 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms
> > index 76580b932e44..4f2f92eb499f 100644
> > --- a/arch/arm64/Kconfig.platforms
> > +++ b/arch/arm64/Kconfig.platforms
> > @@ -130,7 +130,6 @@ config ARCH_K3
> >       select TI_SCI_PROTOCOL
> >       select TI_SCI_INTR_IRQCHIP
> >       select TI_SCI_INTA_IRQCHIP
> > -     select TI_K3_SOCINFO
> >       help
> >         This enables support for Texas Instruments' K3 multicore SoC
> >         architecture.
> > diff --git a/drivers/soc/ti/Kconfig b/drivers/soc/ti/Kconfig
> > index 7e2fb1c16af1..1a730c057cce 100644
> > --- a/drivers/soc/ti/Kconfig
> > +++ b/drivers/soc/ti/Kconfig
> > @@ -74,7 +74,8 @@ config TI_K3_RINGACC
> >         If unsure, say N.
> >
> >  config TI_K3_SOCINFO
> > -     bool
> > +     tristate "TI K3 SoC info driver"
> > +     default y
>
> Maybe
>         default ARCH_K3
> ?

You're correct this should be defaulted to ARCH_K3.
This series will be dropped as it introduces dependency issues with
consumer drivers.

>
> >       depends on ARCH_K3 || COMPILE_TEST
> >       select SOC_BUS
> >       select MFD_SYSCON
>
>
> --
> ~Randy
Thanks,
Nicolas
  

Patch

diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms
index 76580b932e44..4f2f92eb499f 100644
--- a/arch/arm64/Kconfig.platforms
+++ b/arch/arm64/Kconfig.platforms
@@ -130,7 +130,6 @@  config ARCH_K3
 	select TI_SCI_PROTOCOL
 	select TI_SCI_INTR_IRQCHIP
 	select TI_SCI_INTA_IRQCHIP
-	select TI_K3_SOCINFO
 	help
 	  This enables support for Texas Instruments' K3 multicore SoC
 	  architecture.
diff --git a/drivers/soc/ti/Kconfig b/drivers/soc/ti/Kconfig
index 7e2fb1c16af1..1a730c057cce 100644
--- a/drivers/soc/ti/Kconfig
+++ b/drivers/soc/ti/Kconfig
@@ -74,7 +74,8 @@  config TI_K3_RINGACC
 	  If unsure, say N.
 
 config TI_K3_SOCINFO
-	bool
+	tristate "TI K3 SoC info driver"
+	default y
 	depends on ARCH_K3 || COMPILE_TEST
 	select SOC_BUS
 	select MFD_SYSCON
diff --git a/drivers/soc/ti/k3-socinfo.c b/drivers/soc/ti/k3-socinfo.c
index 19f3e74f5376..98348f998e0f 100644
--- a/drivers/soc/ti/k3-socinfo.c
+++ b/drivers/soc/ti/k3-socinfo.c
@@ -13,6 +13,7 @@ 
 #include <linux/slab.h>
 #include <linux/string.h>
 #include <linux/sys_soc.h>
+#include <linux/module.h>
 
 #define CTRLMMR_WKUP_JTAGID_REG		0
 /*
@@ -141,6 +142,7 @@  static const struct of_device_id k3_chipinfo_of_match[] = {
 	{ .compatible = "ti,am654-chipid", },
 	{ /* sentinel */ },
 };
+MODULE_DEVICE_TABLE(of, k3_chipinfo_of_match);
 
 static struct platform_driver k3_chipinfo_driver = {
 	.driver = {
@@ -156,3 +158,12 @@  static int __init k3_chipinfo_init(void)
 	return platform_driver_register(&k3_chipinfo_driver);
 }
 subsys_initcall(k3_chipinfo_init);
+
+static void __exit k3_chipinfo_exit(void)
+{
+	platform_driver_unregister(&k3_chipinfo_driver);
+}
+module_exit(k3_chipinfo_exit);
+
+MODULE_DESCRIPTION("TI K3 SoC info driver");
+MODULE_LICENSE("GPL");