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

[Nicolas Frayer]

Le jeu. 24 nov. 2022 à 08:53, Péter Ujfalusi
<peter.ujfalusi@xxxxxxxxx> 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@xxxxxxxxxxxx>
> > ---
> >   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