[PATCH v3 0/3] Allow for rpmpd/rpmh/rpmhpd drivers to be loaded as permenent modules

[PATCH v3 0/3] Allow for rpmpd/rpmh/rpmhpd drivers to be loaded as permenent modules

[John Stultz]

This series simply allows the qcom rpmpd, rpmh and rpmhpd
drivers to be configured and loaded as permement modules.

This means the modules can be loaded, but not unloaded.

While maybe not ideal, this is an improvement over requiring the
drivers to be built in.

Feedback on this series would be welcome!

thanks
-john

New in v3:
* Added similar change to rpmh and rpmhpd drivers.

Cc: Todd Kjos <tkjos@google.com>
Cc: Saravana Kannan <saravanak@google.com>
Cc: Andy Gross <agross@kernel.org>
Cc: Bjorn Andersson <bjorn.andersson@linaro.org>
Cc: Rajendra Nayak <rnayak@codeaurora.org>
Cc: linux-arm-msm@vger.kernel.org

John Stultz (3):
  soc: qcom: rpmpd: Allow RPMPD driver to be loaded as a module
  soc: qcom: rpmh: Allow RPMH driver to be loaded as a module
  soc: qcom: rpmhpd: Allow RPMHPD driver to be loaded as a module

 drivers/soc/qcom/Kconfig    | 8 ++++----
 drivers/soc/qcom/rpmh-rsc.c | 6 ++++++
 drivers/soc/qcom/rpmhpd.c   | 5 +++++
 drivers/soc/qcom/rpmpd.c    | 6 ++++++
 4 files changed, 21 insertions(+), 4 deletions(-)

-- 
2.17.1

[PATCH v3 1/3] soc: qcom: rpmpd: Allow RPMPD driver to be loaded as a module

[John Stultz]

This patch allow the rpmpd driver to be loaded as a permenent
module. Meaning it can be loaded from a module, but then cannot
be unloaded.

Ideally, it would include a remove hook and related logic, but
apparently the genpd code isn't able to track usage and cleaning
things up? (See: https://lkml.org/lkml/2019/1/24/38)

So making it a permenent module at least improves things slightly
over requiring it to be a built in driver.

Feedback would be appreciated!

Cc: Todd Kjos <tkjos@google.com>
Cc: Saravana Kannan <saravanak@google.com>
Cc: Andy Gross <agross@kernel.org>
Cc: Bjorn Andersson <bjorn.andersson@linaro.org>
Cc: Rajendra Nayak <rnayak@codeaurora.org>
Cc: linux-arm-msm@vger.kernel.org
Acked-by: Saravana Kannan <saravanak@google.com>
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
v2:
* Fix MODULE_LICENSE to be GPL v2 as suggested by Bjorn
* Leave initcall as core_initcall, since that switches to module_initcall
  only when built as a module, also suggested by Bjorn
* Add module tags taken from Rajendra's earlier patch
---
 drivers/soc/qcom/Kconfig | 4 ++--
 drivers/soc/qcom/rpmpd.c | 6 ++++++
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig
index d0a73e76d563..af774555b9d2 100644
--- a/drivers/soc/qcom/Kconfig
+++ b/drivers/soc/qcom/Kconfig
@@ -123,8 +123,8 @@ config QCOM_RPMHPD
 	  for the voltage rail.
 
 config QCOM_RPMPD
-	bool "Qualcomm RPM Power domain driver"
-	depends on QCOM_SMD_RPM=y
+	tristate "Qualcomm RPM Power domain driver"
+	depends on QCOM_SMD_RPM
 	help
 	  QCOM RPM Power domain driver to support power-domains with
 	  performance states. The driver communicates a performance state
diff --git a/drivers/soc/qcom/rpmpd.c b/drivers/soc/qcom/rpmpd.c
index 2b1834c5609a..22fe94c03e79 100644
--- a/drivers/soc/qcom/rpmpd.c
+++ b/drivers/soc/qcom/rpmpd.c
@@ -5,6 +5,7 @@
 #include <linux/init.h>
 #include <linux/kernel.h>
 #include <linux/mutex.h>
+#include <linux/module.h>
 #include <linux/pm_domain.h>
 #include <linux/of.h>
 #include <linux/of_device.h>
@@ -226,6 +227,7 @@ static const struct of_device_id rpmpd_match_table[] = {
 	{ .compatible = "qcom,qcs404-rpmpd", .data = &qcs404_desc },
 	{ }
 };
+MODULE_DEVICE_TABLE(of, rpmpd_match_table);
 
 static int rpmpd_send_enable(struct rpmpd *pd, bool enable)
 {
@@ -422,3 +424,7 @@ static int __init rpmpd_init(void)
 	return platform_driver_register(&rpmpd_driver);
 }
 core_initcall(rpmpd_init);
+
+MODULE_DESCRIPTION("Qualcomm Technologies, Inc. RPM Power Domain Driver");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:qcom-rpmpd");
-- 
2.17.1

Re: [PATCH v3 1/3] soc: qcom: rpmpd: Allow RPMPD driver to be loaded as a module

[Bjorn Andersson]

On Thu 26 Mar 15:44 PDT 2020, John Stultz wrote:

> This patch allow the rpmpd driver to be loaded as a permenent
> module. Meaning it can be loaded from a module, but then cannot
> be unloaded.
> 
> Ideally, it would include a remove hook and related logic, but
> apparently the genpd code isn't able to track usage and cleaning
> things up? (See: https://lkml.org/lkml/2019/1/24/38)
> 
> So making it a permenent module at least improves things slightly
> over requiring it to be a built in driver.
> 
> Feedback would be appreciated!
> 
> Cc: Todd Kjos <tkjos@google.com>
> Cc: Saravana Kannan <saravanak@google.com>
> Cc: Andy Gross <agross@kernel.org>
> Cc: Bjorn Andersson <bjorn.andersson@linaro.org>
> Cc: Rajendra Nayak <rnayak@codeaurora.org>
> Cc: linux-arm-msm@vger.kernel.org
> Acked-by: Saravana Kannan <saravanak@google.com>
> Signed-off-by: John Stultz <john.stultz@linaro.org>
> ---
> v2:
> * Fix MODULE_LICENSE to be GPL v2 as suggested by Bjorn
> * Leave initcall as core_initcall, since that switches to module_initcall
>   only when built as a module, also suggested by Bjorn
> * Add module tags taken from Rajendra's earlier patch
> ---
>  drivers/soc/qcom/Kconfig | 4 ++--
>  drivers/soc/qcom/rpmpd.c | 6 ++++++
>  2 files changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig
> index d0a73e76d563..af774555b9d2 100644
> --- a/drivers/soc/qcom/Kconfig
> +++ b/drivers/soc/qcom/Kconfig
> @@ -123,8 +123,8 @@ config QCOM_RPMHPD
>  	  for the voltage rail.
>  
>  config QCOM_RPMPD
> -	bool "Qualcomm RPM Power domain driver"
> -	depends on QCOM_SMD_RPM=y
> +	tristate "Qualcomm RPM Power domain driver"
> +	depends on QCOM_SMD_RPM
>  	help
>  	  QCOM RPM Power domain driver to support power-domains with
>  	  performance states. The driver communicates a performance state
> diff --git a/drivers/soc/qcom/rpmpd.c b/drivers/soc/qcom/rpmpd.c
> index 2b1834c5609a..22fe94c03e79 100644
> --- a/drivers/soc/qcom/rpmpd.c
> +++ b/drivers/soc/qcom/rpmpd.c
> @@ -5,6 +5,7 @@
>  #include <linux/init.h>
>  #include <linux/kernel.h>
>  #include <linux/mutex.h>
> +#include <linux/module.h>

module comes before mutex in the alphabet.

>  #include <linux/pm_domain.h>
>  #include <linux/of.h>
>  #include <linux/of_device.h>
> @@ -226,6 +227,7 @@ static const struct of_device_id rpmpd_match_table[] = {
>  	{ .compatible = "qcom,qcs404-rpmpd", .data = &qcs404_desc },
>  	{ }
>  };
> +MODULE_DEVICE_TABLE(of, rpmpd_match_table);
>  
>  static int rpmpd_send_enable(struct rpmpd *pd, bool enable)
>  {
> @@ -422,3 +424,7 @@ static int __init rpmpd_init(void)
>  	return platform_driver_register(&rpmpd_driver);
>  }
>  core_initcall(rpmpd_init);
> +
> +MODULE_DESCRIPTION("Qualcomm Technologies, Inc. RPM Power Domain Driver");
> +MODULE_LICENSE("GPL v2");
> +MODULE_ALIAS("platform:qcom-rpmpd");

Is there any reason for this alias?

The module will be automatically loaded based on compatible and the
MODULE_DEVICE_TABLE() information above, and for ACPI would need a
similar acpi_device_id table.

Regards,
Bjorn

> -- 
> 2.17.1
> 

Re: [PATCH v3 1/3] soc: qcom: rpmpd: Allow RPMPD driver to be loaded as a module

[John Stultz]

On Tue, Apr 14, 2020 at 3:21 PM Bjorn Andersson
<bjorn.andersson@linaro.org> wrote:
>
> On Thu 26 Mar 15:44 PDT 2020, John Stultz wrote:
>
> > This patch allow the rpmpd driver to be loaded as a permenent
> > module. Meaning it can be loaded from a module, but then cannot
> > be unloaded.
> >
> > Ideally, it would include a remove hook and related logic, but
> > apparently the genpd code isn't able to track usage and cleaning
> > things up? (See: https://lkml.org/lkml/2019/1/24/38)
> >
> > So making it a permenent module at least improves things slightly
> > over requiring it to be a built in driver.
> >
> > Feedback would be appreciated!
> >
> > Cc: Todd Kjos <tkjos@google.com>
> > Cc: Saravana Kannan <saravanak@google.com>
> > Cc: Andy Gross <agross@kernel.org>
> > Cc: Bjorn Andersson <bjorn.andersson@linaro.org>
> > Cc: Rajendra Nayak <rnayak@codeaurora.org>
> > Cc: linux-arm-msm@vger.kernel.org
> > Acked-by: Saravana Kannan <saravanak@google.com>
> > Signed-off-by: John Stultz <john.stultz@linaro.org>
> > ---
> > v2:
> > * Fix MODULE_LICENSE to be GPL v2 as suggested by Bjorn
> > * Leave initcall as core_initcall, since that switches to module_initcall
> >   only when built as a module, also suggested by Bjorn
> > * Add module tags taken from Rajendra's earlier patch
> > ---
> >  drivers/soc/qcom/Kconfig | 4 ++--
> >  drivers/soc/qcom/rpmpd.c | 6 ++++++
> >  2 files changed, 8 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig
> > index d0a73e76d563..af774555b9d2 100644
> > --- a/drivers/soc/qcom/Kconfig
> > +++ b/drivers/soc/qcom/Kconfig
> > @@ -123,8 +123,8 @@ config QCOM_RPMHPD
> >         for the voltage rail.
> >
> >  config QCOM_RPMPD
> > -     bool "Qualcomm RPM Power domain driver"
> > -     depends on QCOM_SMD_RPM=y
> > +     tristate "Qualcomm RPM Power domain driver"
> > +     depends on QCOM_SMD_RPM
> >       help
> >         QCOM RPM Power domain driver to support power-domains with
> >         performance states. The driver communicates a performance state
> > diff --git a/drivers/soc/qcom/rpmpd.c b/drivers/soc/qcom/rpmpd.c
> > index 2b1834c5609a..22fe94c03e79 100644
> > --- a/drivers/soc/qcom/rpmpd.c
> > +++ b/drivers/soc/qcom/rpmpd.c
> > @@ -5,6 +5,7 @@
> >  #include <linux/init.h>
> >  #include <linux/kernel.h>
> >  #include <linux/mutex.h>
> > +#include <linux/module.h>
>
> module comes before mutex in the alphabet.

:) Thanks for catching that.

> >  #include <linux/pm_domain.h>
> >  #include <linux/of.h>
> >  #include <linux/of_device.h>
> > @@ -226,6 +227,7 @@ static const struct of_device_id rpmpd_match_table[] = {
> >       { .compatible = "qcom,qcs404-rpmpd", .data = &qcs404_desc },
> >       { }
> >  };
> > +MODULE_DEVICE_TABLE(of, rpmpd_match_table);
> >
> >  static int rpmpd_send_enable(struct rpmpd *pd, bool enable)
> >  {
> > @@ -422,3 +424,7 @@ static int __init rpmpd_init(void)
> >       return platform_driver_register(&rpmpd_driver);
> >  }
> >  core_initcall(rpmpd_init);
> > +
> > +MODULE_DESCRIPTION("Qualcomm Technologies, Inc. RPM Power Domain Driver");
> > +MODULE_LICENSE("GPL v2");
> > +MODULE_ALIAS("platform:qcom-rpmpd");
>
> Is there any reason for this alias?
>
> The module will be automatically loaded based on compatible and the
> MODULE_DEVICE_TABLE() information above, and for ACPI would need a
> similar acpi_device_id table.

I pulled it in from Rajendra's earlier patch. I'm ok to drop it though.

I'll fix these up and respin. Thanks for the review!

thanks
-john

Re: [PATCH v3 1/3] soc: qcom: rpmpd: Allow RPMPD driver to be loaded as a module

[Bjorn Andersson]

On Tue 14 Apr 15:24 PDT 2020, John Stultz wrote:

> On Tue, Apr 14, 2020 at 3:21 PM Bjorn Andersson
> <bjorn.andersson@linaro.org> wrote:
> >
> > On Thu 26 Mar 15:44 PDT 2020, John Stultz wrote:
> >
> > > This patch allow the rpmpd driver to be loaded as a permenent
> > > module. Meaning it can be loaded from a module, but then cannot
> > > be unloaded.
> > >
> > > Ideally, it would include a remove hook and related logic, but
> > > apparently the genpd code isn't able to track usage and cleaning
> > > things up? (See: https://lkml.org/lkml/2019/1/24/38)
> > >
> > > So making it a permenent module at least improves things slightly
> > > over requiring it to be a built in driver.
> > >
> > > Feedback would be appreciated!
> > >
> > > Cc: Todd Kjos <tkjos@google.com>
> > > Cc: Saravana Kannan <saravanak@google.com>
> > > Cc: Andy Gross <agross@kernel.org>
> > > Cc: Bjorn Andersson <bjorn.andersson@linaro.org>
> > > Cc: Rajendra Nayak <rnayak@codeaurora.org>
> > > Cc: linux-arm-msm@vger.kernel.org
> > > Acked-by: Saravana Kannan <saravanak@google.com>
> > > Signed-off-by: John Stultz <john.stultz@linaro.org>
> > > ---
> > > v2:
> > > * Fix MODULE_LICENSE to be GPL v2 as suggested by Bjorn
> > > * Leave initcall as core_initcall, since that switches to module_initcall
> > >   only when built as a module, also suggested by Bjorn
> > > * Add module tags taken from Rajendra's earlier patch
> > > ---
> > >  drivers/soc/qcom/Kconfig | 4 ++--
> > >  drivers/soc/qcom/rpmpd.c | 6 ++++++
> > >  2 files changed, 8 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig
> > > index d0a73e76d563..af774555b9d2 100644
> > > --- a/drivers/soc/qcom/Kconfig
> > > +++ b/drivers/soc/qcom/Kconfig
> > > @@ -123,8 +123,8 @@ config QCOM_RPMHPD
> > >         for the voltage rail.
> > >
> > >  config QCOM_RPMPD
> > > -     bool "Qualcomm RPM Power domain driver"
> > > -     depends on QCOM_SMD_RPM=y
> > > +     tristate "Qualcomm RPM Power domain driver"
> > > +     depends on QCOM_SMD_RPM
> > >       help
> > >         QCOM RPM Power domain driver to support power-domains with
> > >         performance states. The driver communicates a performance state
> > > diff --git a/drivers/soc/qcom/rpmpd.c b/drivers/soc/qcom/rpmpd.c
> > > index 2b1834c5609a..22fe94c03e79 100644
> > > --- a/drivers/soc/qcom/rpmpd.c
> > > +++ b/drivers/soc/qcom/rpmpd.c
> > > @@ -5,6 +5,7 @@
> > >  #include <linux/init.h>
> > >  #include <linux/kernel.h>
> > >  #include <linux/mutex.h>
> > > +#include <linux/module.h>
> >
> > module comes before mutex in the alphabet.
> 
> :) Thanks for catching that.
> 
> > >  #include <linux/pm_domain.h>
> > >  #include <linux/of.h>
> > >  #include <linux/of_device.h>
> > > @@ -226,6 +227,7 @@ static const struct of_device_id rpmpd_match_table[] = {
> > >       { .compatible = "qcom,qcs404-rpmpd", .data = &qcs404_desc },
> > >       { }
> > >  };
> > > +MODULE_DEVICE_TABLE(of, rpmpd_match_table);
> > >
> > >  static int rpmpd_send_enable(struct rpmpd *pd, bool enable)
> > >  {
> > > @@ -422,3 +424,7 @@ static int __init rpmpd_init(void)
> > >       return platform_driver_register(&rpmpd_driver);
> > >  }
> > >  core_initcall(rpmpd_init);
> > > +
> > > +MODULE_DESCRIPTION("Qualcomm Technologies, Inc. RPM Power Domain Driver");
> > > +MODULE_LICENSE("GPL v2");
> > > +MODULE_ALIAS("platform:qcom-rpmpd");
> >
> > Is there any reason for this alias?
> >
> > The module will be automatically loaded based on compatible and the
> > MODULE_DEVICE_TABLE() information above, and for ACPI would need a
> > similar acpi_device_id table.
> 
> I pulled it in from Rajendra's earlier patch. I'm ok to drop it though.
> 
> I'll fix these up and respin. Thanks for the review!
> 

No worries, I'll fix these two things and apply the patch. Just wanted
to check if I was missing something.

Regards,
Bjorn

[PATCH v3 2/3] soc: qcom: rpmh: Allow RPMH driver to be loaded as a module

[John Stultz]

This patch allow the rpmh driver to be loaded as a permenent
module. Meaning it can be loaded from a module, but then cannot
be unloaded.

Ideally, it would include a remove hook and related logic, but
the rpmh driver is fairly core to the system, so once its loaded
with almost anythign else to get the system to go, the dependencies
are not likely to ever also be removed.

So making it a permenent module at least improves things slightly
over requiring it to be a built in driver.

Feedback would be appreciated!

Cc: Todd Kjos <tkjos@google.com>
Cc: Saravana Kannan <saravanak@google.com>
Cc: Andy Gross <agross@kernel.org>
Cc: Bjorn Andersson <bjorn.andersson@linaro.org>
Cc: Rajendra Nayak <rnayak@codeaurora.org>
Cc: linux-arm-msm@vger.kernel.org
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
 drivers/soc/qcom/Kconfig    | 2 +-
 drivers/soc/qcom/rpmh-rsc.c | 6 ++++++
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig
index af774555b9d2..ac91eaf810f7 100644
--- a/drivers/soc/qcom/Kconfig
+++ b/drivers/soc/qcom/Kconfig
@@ -104,7 +104,7 @@ config QCOM_RMTFS_MEM
 	  Say y here if you intend to boot the modem remoteproc.
 
 config QCOM_RPMH
-	bool "Qualcomm RPM-Hardened (RPMH) Communication"
+	tristate "Qualcomm RPM-Hardened (RPMH) Communication"
 	depends on ARCH_QCOM && ARM64 || COMPILE_TEST
 	help
 	  Support for communication with the hardened-RPM blocks in
diff --git a/drivers/soc/qcom/rpmh-rsc.c b/drivers/soc/qcom/rpmh-rsc.c
index e278fc11fe5c..30585d98fdf1 100644
--- a/drivers/soc/qcom/rpmh-rsc.c
+++ b/drivers/soc/qcom/rpmh-rsc.c
@@ -11,6 +11,7 @@
 #include <linux/io.h>
 #include <linux/kernel.h>
 #include <linux/list.h>
+#include <linux/module.h>
 #include <linux/of.h>
 #include <linux/of_irq.h>
 #include <linux/of_platform.h>
@@ -679,6 +680,8 @@ static const struct of_device_id rpmh_drv_match[] = {
 	{ .compatible = "qcom,rpmh-rsc", },
 	{ }
 };
+MODULE_DEVICE_TABLE(of, rpmh_drv_match);
+
 
 static struct platform_driver rpmh_driver = {
 	.probe = rpmh_rsc_probe,
@@ -693,3 +696,6 @@ static int __init rpmh_driver_init(void)
 	return platform_driver_register(&rpmh_driver);
 }
 arch_initcall(rpmh_driver_init);
+
+MODULE_DESCRIPTION("Qualcomm Technologies, Inc. RPMh Driver");
+MODULE_LICENSE("GPL v2");
-- 
2.17.1

Re: [PATCH v3 2/3] soc: qcom: rpmh: Allow RPMH driver to be loaded as a module

[Bjorn Andersson]

On Thu 26 Mar 15:44 PDT 2020, John Stultz wrote:

> This patch allow the rpmh driver to be loaded as a permenent
> module. Meaning it can be loaded from a module, but then cannot
> be unloaded.
> 
> Ideally, it would include a remove hook and related logic, but
> the rpmh driver is fairly core to the system, so once its loaded
> with almost anythign else to get the system to go, the dependencies
> are not likely to ever also be removed.
> 
> So making it a permenent module at least improves things slightly
> over requiring it to be a built in driver.
> 
> Feedback would be appreciated!
> 
> Cc: Todd Kjos <tkjos@google.com>
> Cc: Saravana Kannan <saravanak@google.com>
> Cc: Andy Gross <agross@kernel.org>
> Cc: Bjorn Andersson <bjorn.andersson@linaro.org>
> Cc: Rajendra Nayak <rnayak@codeaurora.org>
> Cc: linux-arm-msm@vger.kernel.org
> Signed-off-by: John Stultz <john.stultz@linaro.org>

Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>

and applied.

Regards,
Bjorn

> ---
>  drivers/soc/qcom/Kconfig    | 2 +-
>  drivers/soc/qcom/rpmh-rsc.c | 6 ++++++
>  2 files changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig
> index af774555b9d2..ac91eaf810f7 100644
> --- a/drivers/soc/qcom/Kconfig
> +++ b/drivers/soc/qcom/Kconfig
> @@ -104,7 +104,7 @@ config QCOM_RMTFS_MEM
>  	  Say y here if you intend to boot the modem remoteproc.
>  
>  config QCOM_RPMH
> -	bool "Qualcomm RPM-Hardened (RPMH) Communication"
> +	tristate "Qualcomm RPM-Hardened (RPMH) Communication"
>  	depends on ARCH_QCOM && ARM64 || COMPILE_TEST
>  	help
>  	  Support for communication with the hardened-RPM blocks in
> diff --git a/drivers/soc/qcom/rpmh-rsc.c b/drivers/soc/qcom/rpmh-rsc.c
> index e278fc11fe5c..30585d98fdf1 100644
> --- a/drivers/soc/qcom/rpmh-rsc.c
> +++ b/drivers/soc/qcom/rpmh-rsc.c
> @@ -11,6 +11,7 @@
>  #include <linux/io.h>
>  #include <linux/kernel.h>
>  #include <linux/list.h>
> +#include <linux/module.h>
>  #include <linux/of.h>
>  #include <linux/of_irq.h>
>  #include <linux/of_platform.h>
> @@ -679,6 +680,8 @@ static const struct of_device_id rpmh_drv_match[] = {
>  	{ .compatible = "qcom,rpmh-rsc", },
>  	{ }
>  };
> +MODULE_DEVICE_TABLE(of, rpmh_drv_match);
> +
>  
>  static struct platform_driver rpmh_driver = {
>  	.probe = rpmh_rsc_probe,
> @@ -693,3 +696,6 @@ static int __init rpmh_driver_init(void)
>  	return platform_driver_register(&rpmh_driver);
>  }
>  arch_initcall(rpmh_driver_init);
> +
> +MODULE_DESCRIPTION("Qualcomm Technologies, Inc. RPMh Driver");
> +MODULE_LICENSE("GPL v2");
> -- 
> 2.17.1
> 

Re: [PATCH v3 2/3] soc: qcom: rpmh: Allow RPMH driver to be loaded as a module

[Matthias Kaehlcke]

Hi John,

with commit efde2659b0fe ("drivers: qcom: rpmh-rsc: Use rcuidle
tracepoints for rpmh") the rpmh-rsc driver fails to build as a
module:

drivers/soc/qcom/rpmh-rsc.c:281:3: error: implicit declaration of function 'trace_rpmh_send_msg_rcuidle' [-Werror,-Wimplicit-function-decr]
                trace_rpmh_send_msg_rcuidle(drv, tcs_id, j, msgid, cmd);


The problem is that the _rcuidle() functions are not generated for modules:

#ifndef MODULE
#define __DECLARE_TRACE_RCU(name, proto, args, cond, data_proto, data_args) \
	static inline void trace_##name##_rcuidle(proto)		\
	{								\
		if (static_key_false(&__tracepoint_##name.key))		\
			__DO_TRACE(&__tracepoint_##name,		\
				TP_PROTO(data_proto),			\
				TP_ARGS(data_args),			\
				TP_CONDITION(cond), 1);			\
	}
#else
#define __DECLARE_TRACE_RCU(name, proto, args, cond, data_proto, data_args)
#endif

Not sure what the best solution would be in this case. Having the macro
define a dummy function for modules would fix the build error, however it
would be confusing that the event is traced when the driver is built-in,
but not when it is built as a module.

I imagine the goal behind making this driver a module is to have a single
kernel image for multiple SoC platforms, without too much platform
specific code in the kernel image itself.

I guess the question is whether there any options for keeping the driver
modular and having consistent tracing behavior, short of removing the
tracepoint.

On Thu, Mar 26, 2020 at 10:44:58PM +0000, John Stultz wrote:
> This patch allow the rpmh driver to be loaded as a permenent
> module. Meaning it can be loaded from a module, but then cannot
> be unloaded.
> 
> Ideally, it would include a remove hook and related logic, but
> the rpmh driver is fairly core to the system, so once its loaded
> with almost anythign else to get the system to go, the dependencies
> are not likely to ever also be removed.
> 
> So making it a permenent module at least improves things slightly
> over requiring it to be a built in driver.
> 
> Feedback would be appreciated!
> 
> Cc: Todd Kjos <tkjos@google.com>
> Cc: Saravana Kannan <saravanak@google.com>
> Cc: Andy Gross <agross@kernel.org>
> Cc: Bjorn Andersson <bjorn.andersson@linaro.org>
> Cc: Rajendra Nayak <rnayak@codeaurora.org>
> Cc: linux-arm-msm@vger.kernel.org
> Signed-off-by: John Stultz <john.stultz@linaro.org>
> ---
>  drivers/soc/qcom/Kconfig    | 2 +-
>  drivers/soc/qcom/rpmh-rsc.c | 6 ++++++
>  2 files changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig
> index af774555b9d2..ac91eaf810f7 100644
> --- a/drivers/soc/qcom/Kconfig
> +++ b/drivers/soc/qcom/Kconfig
> @@ -104,7 +104,7 @@ config QCOM_RMTFS_MEM
>  	  Say y here if you intend to boot the modem remoteproc.
>  
>  config QCOM_RPMH
> -	bool "Qualcomm RPM-Hardened (RPMH) Communication"
> +	tristate "Qualcomm RPM-Hardened (RPMH) Communication"
>  	depends on ARCH_QCOM && ARM64 || COMPILE_TEST
>  	help
>  	  Support for communication with the hardened-RPM blocks in
> diff --git a/drivers/soc/qcom/rpmh-rsc.c b/drivers/soc/qcom/rpmh-rsc.c
> index e278fc11fe5c..30585d98fdf1 100644
> --- a/drivers/soc/qcom/rpmh-rsc.c
> +++ b/drivers/soc/qcom/rpmh-rsc.c
> @@ -11,6 +11,7 @@
>  #include <linux/io.h>
>  #include <linux/kernel.h>
>  #include <linux/list.h>
> +#include <linux/module.h>
>  #include <linux/of.h>
>  #include <linux/of_irq.h>
>  #include <linux/of_platform.h>
> @@ -679,6 +680,8 @@ static const struct of_device_id rpmh_drv_match[] = {
>  	{ .compatible = "qcom,rpmh-rsc", },
>  	{ }
>  };
> +MODULE_DEVICE_TABLE(of, rpmh_drv_match);
> +
>  
>  static struct platform_driver rpmh_driver = {
>  	.probe = rpmh_rsc_probe,
> @@ -693,3 +696,6 @@ static int __init rpmh_driver_init(void)
>  	return platform_driver_register(&rpmh_driver);
>  }
>  arch_initcall(rpmh_driver_init);
> +
> +MODULE_DESCRIPTION("Qualcomm Technologies, Inc. RPMh Driver");
> +MODULE_LICENSE("GPL v2");
> -- 
> 2.17.1
> 

Re: [PATCH v3 2/3] soc: qcom: rpmh: Allow RPMH driver to be loaded as a module

[John Stultz]

On Wed, Apr 15, 2020 at 11:25 AM Matthias Kaehlcke <mka@chromium.org> wrote:
>
> Hi John,
>
> with commit efde2659b0fe ("drivers: qcom: rpmh-rsc: Use rcuidle
> tracepoints for rpmh") the rpmh-rsc driver fails to build as a
> module:
>
> drivers/soc/qcom/rpmh-rsc.c:281:3: error: implicit declaration of function 'trace_rpmh_send_msg_rcuidle' [-Werror,-Wimplicit-function-decr]
>                 trace_rpmh_send_msg_rcuidle(drv, tcs_id, j, msgid, cmd);
>
>
> The problem is that the _rcuidle() functions are not generated for modules:
>
> #ifndef MODULE
> #define __DECLARE_TRACE_RCU(name, proto, args, cond, data_proto, data_args) \
>         static inline void trace_##name##_rcuidle(proto)                \
>         {                                                               \
>                 if (static_key_false(&__tracepoint_##name.key))         \
>                         __DO_TRACE(&__tracepoint_##name,                \
>                                 TP_PROTO(data_proto),                   \
>                                 TP_ARGS(data_args),                     \
>                                 TP_CONDITION(cond), 1);                 \
>         }
> #else
> #define __DECLARE_TRACE_RCU(name, proto, args, cond, data_proto, data_args)
> #endif
>
> Not sure what the best solution would be in this case. Having the macro
> define a dummy function for modules would fix the build error, however it
> would be confusing that the event is traced when the driver is built-in,
> but not when it is built as a module.
>
> I imagine the goal behind making this driver a module is to have a single
> kernel image for multiple SoC platforms, without too much platform
> specific code in the kernel image itself.
>
> I guess the question is whether there any options for keeping the driver
> modular and having consistent tracing behavior, short of removing the
> tracepoint.

Yea.  Stephen found that issue in -next last night once Bjorn added
the patches to his tree yesterday.

I've reached out to see if the restrictions on the trace_*_rcuidle
calls on modules is still necessary in this thread:
  https://lore.kernel.org/lkml/CALAqxLV4rM74wuzuZ+BkUi+keccxkAxv30N4vrFO7CVQ5vnT1A@mail.gmail.com/

For now, I suggested Bjorn revert the patch in his tree, and I'll try
to figure out an alternative solution to the trace call.

thanks
-john

[PATCH v3 3/3] soc: qcom: rpmhpd: Allow RPMHPD driver to be loaded as a module

[John Stultz]

This patch allow the rpmhpd driver to be loaded as a permenent
module. Meaning it can be loaded from a module, but then cannot
be unloaded.

Ideally, it would include a remove hook and related logic, but
apparently the genpd code isn't able to track usage and cleaning
things up?

So making it a permenent module at least improves things slightly
over requiring it to be a built in driver.

Feedback would be appreciated!

Cc: Todd Kjos <tkjos@google.com>
Cc: Saravana Kannan <saravanak@google.com>
Cc: Andy Gross <agross@kernel.org>
Cc: Bjorn Andersson <bjorn.andersson@linaro.org>
Cc: Rajendra Nayak <rnayak@codeaurora.org>
Cc: linux-arm-msm@vger.kernel.org
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
 drivers/soc/qcom/Kconfig  | 2 +-
 drivers/soc/qcom/rpmhpd.c | 5 +++++
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig
index ac91eaf810f7..ffc04285840b 100644
--- a/drivers/soc/qcom/Kconfig
+++ b/drivers/soc/qcom/Kconfig
@@ -114,7 +114,7 @@ config QCOM_RPMH
 	  help apply the aggregated state on the resource.
 
 config QCOM_RPMHPD
-	bool "Qualcomm RPMh Power domain driver"
+	tristate "Qualcomm RPMh Power domain driver"
 	depends on QCOM_RPMH && QCOM_COMMAND_DB
 	help
 	  QCOM RPMh Power domain driver to support power-domains with
diff --git a/drivers/soc/qcom/rpmhpd.c b/drivers/soc/qcom/rpmhpd.c
index 4d264d0672c4..0bb12d5870a7 100644
--- a/drivers/soc/qcom/rpmhpd.c
+++ b/drivers/soc/qcom/rpmhpd.c
@@ -4,6 +4,7 @@
 #include <linux/err.h>
 #include <linux/init.h>
 #include <linux/kernel.h>
+#include <linux/module.h>
 #include <linux/mutex.h>
 #include <linux/pm_domain.h>
 #include <linux/slab.h>
@@ -189,6 +190,7 @@ static const struct of_device_id rpmhpd_match_table[] = {
 	{ .compatible = "qcom,sm8150-rpmhpd", .data = &sm8150_desc },
 	{ }
 };
+MODULE_DEVICE_TABLE(of, rpmhpd_match_table);
 
 static int rpmhpd_send_corner(struct rpmhpd *pd, int state,
 			      unsigned int corner, bool sync)
@@ -460,3 +462,6 @@ static int __init rpmhpd_init(void)
 	return platform_driver_register(&rpmhpd_driver);
 }
 core_initcall(rpmhpd_init);
+
+MODULE_DESCRIPTION("Qualcomm Technologies, Inc. RPMh Power Domain Driver");
+MODULE_LICENSE("GPL v2");
-- 
2.17.1

Re: [PATCH v3 3/3] soc: qcom: rpmhpd: Allow RPMHPD driver to be loaded as a module

[Bjorn Andersson]

On Thu 26 Mar 15:44 PDT 2020, John Stultz wrote:

> This patch allow the rpmhpd driver to be loaded as a permenent
> module. Meaning it can be loaded from a module, but then cannot
> be unloaded.
> 
> Ideally, it would include a remove hook and related logic, but
> apparently the genpd code isn't able to track usage and cleaning
> things up?
> 
> So making it a permenent module at least improves things slightly
> over requiring it to be a built in driver.
> 
> Feedback would be appreciated!
> 
> Cc: Todd Kjos <tkjos@google.com>
> Cc: Saravana Kannan <saravanak@google.com>
> Cc: Andy Gross <agross@kernel.org>
> Cc: Bjorn Andersson <bjorn.andersson@linaro.org>
> Cc: Rajendra Nayak <rnayak@codeaurora.org>
> Cc: linux-arm-msm@vger.kernel.org
> Signed-off-by: John Stultz <john.stultz@linaro.org>

Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>

and applied.

Regards,
Bjorn

> ---
>  drivers/soc/qcom/Kconfig  | 2 +-
>  drivers/soc/qcom/rpmhpd.c | 5 +++++
>  2 files changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig
> index ac91eaf810f7..ffc04285840b 100644
> --- a/drivers/soc/qcom/Kconfig
> +++ b/drivers/soc/qcom/Kconfig
> @@ -114,7 +114,7 @@ config QCOM_RPMH
>  	  help apply the aggregated state on the resource.
>  
>  config QCOM_RPMHPD
> -	bool "Qualcomm RPMh Power domain driver"
> +	tristate "Qualcomm RPMh Power domain driver"
>  	depends on QCOM_RPMH && QCOM_COMMAND_DB
>  	help
>  	  QCOM RPMh Power domain driver to support power-domains with
> diff --git a/drivers/soc/qcom/rpmhpd.c b/drivers/soc/qcom/rpmhpd.c
> index 4d264d0672c4..0bb12d5870a7 100644
> --- a/drivers/soc/qcom/rpmhpd.c
> +++ b/drivers/soc/qcom/rpmhpd.c
> @@ -4,6 +4,7 @@
>  #include <linux/err.h>
>  #include <linux/init.h>
>  #include <linux/kernel.h>
> +#include <linux/module.h>
>  #include <linux/mutex.h>
>  #include <linux/pm_domain.h>
>  #include <linux/slab.h>
> @@ -189,6 +190,7 @@ static const struct of_device_id rpmhpd_match_table[] = {
>  	{ .compatible = "qcom,sm8150-rpmhpd", .data = &sm8150_desc },
>  	{ }
>  };
> +MODULE_DEVICE_TABLE(of, rpmhpd_match_table);
>  
>  static int rpmhpd_send_corner(struct rpmhpd *pd, int state,
>  			      unsigned int corner, bool sync)
> @@ -460,3 +462,6 @@ static int __init rpmhpd_init(void)
>  	return platform_driver_register(&rpmhpd_driver);
>  }
>  core_initcall(rpmhpd_init);
> +
> +MODULE_DESCRIPTION("Qualcomm Technologies, Inc. RPMh Power Domain Driver");
> +MODULE_LICENSE("GPL v2");
> -- 
> 2.17.1
> 

Re: [PATCH v3 0/3] Allow for rpmpd/rpmh/rpmhpd drivers to be loaded as permenent modules

[Saravana Kannan]

On Thu, Mar 26, 2020 at 3:45 PM John Stultz <john.stultz@linaro.org> wrote:
>
> This series simply allows the qcom rpmpd, rpmh and rpmhpd
> drivers to be configured and loaded as permement modules.
>
> This means the modules can be loaded, but not unloaded.
>
> While maybe not ideal, this is an improvement over requiring the
> drivers to be built in.
>
> Feedback on this series would be welcome!
>
> thanks
> -john
>
> New in v3:
> * Added similar change to rpmh and rpmhpd drivers.
>
> Cc: Todd Kjos <tkjos@google.com>
> Cc: Saravana Kannan <saravanak@google.com>
> Cc: Andy Gross <agross@kernel.org>
> Cc: Bjorn Andersson <bjorn.andersson@linaro.org>
> Cc: Rajendra Nayak <rnayak@codeaurora.org>
> Cc: linux-arm-msm@vger.kernel.org
>
> John Stultz (3):
>   soc: qcom: rpmpd: Allow RPMPD driver to be loaded as a module
>   soc: qcom: rpmh: Allow RPMH driver to be loaded as a module
>   soc: qcom: rpmhpd: Allow RPMHPD driver to be loaded as a module
>
>  drivers/soc/qcom/Kconfig    | 8 ++++----
>  drivers/soc/qcom/rpmh-rsc.c | 6 ++++++
>  drivers/soc/qcom/rpmhpd.c   | 5 +++++
>  drivers/soc/qcom/rpmpd.c    | 6 ++++++
>  4 files changed, 21 insertions(+), 4 deletions(-)

The whole series looks ok to me.
Acked-by: Saravana Kannan <saravanak@google.com>
for the whole series.

-Saravana