[PATCH 0/2] Support building tegra124-cpufreq as a module

[PATCH 0/2] Support building tegra124-cpufreq as a module

[Aaron Kling via B4 Relay]

This driver handles power management while offloading the primary
cpufreq operations to cpufreq-dt. It fully disables cpufreq during
resume if clocks fail, thus it needs access to a previously unexported
cpufreq symbol.

Signed-off-by: Aaron Kling <webgeek1234@gmail.com>
---
Aaron Kling (2):
      cpufreq: Export disable_cpufreq()
      cpufreq: tegra124: Allow building as a module

 drivers/cpufreq/Kconfig.arm        | 2 +-
 drivers/cpufreq/cpufreq.c          | 1 +
 drivers/cpufreq/tegra124-cpufreq.c | 2 ++
 3 files changed, 4 insertions(+), 1 deletion(-)
---
base-commit: 91e5bfe317d8f8471fbaa3e70cf66cae1314a516
change-id: 20250401-tegra124-cpufreq-fd944fdce62c

Best regards,
-- 
Aaron Kling <webgeek1234@gmail.com>

[PATCH 1/2] cpufreq: Export disable_cpufreq()

[Aaron Kling via B4 Relay]

From: Aaron Kling <webgeek1234@gmail.com>

This is used by the tegra124-cpufreq driver.

Signed-off-by: Aaron Kling <webgeek1234@gmail.com>
---
 drivers/cpufreq/cpufreq.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 0cf5a320bb5e864709b259249fb1af8bfbc0b04b..78cddc78ee98630f99ccc332c64c94f437297177 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -109,6 +109,7 @@ void disable_cpufreq(void)
 {
 	off = 1;
 }
+EXPORT_SYMBOL_GPL(disable_cpufreq);
 static DEFINE_MUTEX(cpufreq_governor_mutex);
 
 bool have_governor_per_policy(void)

-- 
2.48.1

[PATCH 2/2] cpufreq: tegra124: Allow building as a module

[Aaron Kling via B4 Relay]

From: Aaron Kling <webgeek1234@gmail.com>

This requires a soft dependency on cpufreq-dt as this driver only
handles power management and cpufreq-dt does the real operations. Other
than that, this works fine as a module, so allow building as such.

Signed-off-by: Aaron Kling <webgeek1234@gmail.com>
---
 drivers/cpufreq/Kconfig.arm        | 2 +-
 drivers/cpufreq/tegra124-cpufreq.c | 2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
index 4f9cb943d945c244eb2b29f543d14df6cac4e5d4..625f6fbdaaf5fd774e3b0bb996eb7ce980da41ee 100644
--- a/drivers/cpufreq/Kconfig.arm
+++ b/drivers/cpufreq/Kconfig.arm
@@ -238,7 +238,7 @@ config ARM_TEGRA20_CPUFREQ
 	  This adds the CPUFreq driver support for Tegra20/30 SOCs.
 
 config ARM_TEGRA124_CPUFREQ
-	bool "Tegra124 CPUFreq support"
+	tristate "Tegra124 CPUFreq support"
 	depends on ARCH_TEGRA || COMPILE_TEST
 	depends on CPUFREQ_DT
 	default y
diff --git a/drivers/cpufreq/tegra124-cpufreq.c b/drivers/cpufreq/tegra124-cpufreq.c
index 514146d98bca2d8aa59980a14dff3487cd8045f6..9e46b53487fafdc1e9e95e765b9b490d9c95f2c6 100644
--- a/drivers/cpufreq/tegra124-cpufreq.c
+++ b/drivers/cpufreq/tegra124-cpufreq.c
@@ -211,5 +211,7 @@ static int __init tegra_cpufreq_init(void)
 }
 module_init(tegra_cpufreq_init);
 
+MODULE_SOFTDEP("pre: cpufreq-dt");
 MODULE_AUTHOR("Tuomas Tynkkynen <ttynkkynen@nvidia.com>");
 MODULE_DESCRIPTION("cpufreq driver for NVIDIA Tegra124");
+MODULE_LICENSE("GPL");

-- 
2.48.1

Re: [PATCH 1/2] cpufreq: Export disable_cpufreq()

[Viresh Kumar]

On 20-04-25, 15:42, Aaron Kling via B4 Relay wrote:
> From: Aaron Kling <webgeek1234@gmail.com>
> 
> This is used by the tegra124-cpufreq driver.
> 
> Signed-off-by: Aaron Kling <webgeek1234@gmail.com>
> ---
>  drivers/cpufreq/cpufreq.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index 0cf5a320bb5e864709b259249fb1af8bfbc0b04b..78cddc78ee98630f99ccc332c64c94f437297177 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -109,6 +109,7 @@ void disable_cpufreq(void)
>  {
>  	off = 1;
>  }
> +EXPORT_SYMBOL_GPL(disable_cpufreq);
>  static DEFINE_MUTEX(cpufreq_governor_mutex);
>  
>  bool have_governor_per_policy(void)

I didn't realize earlier that tegra124 uses this call directly. I
think this call was rather meant for other parts of the kernel
(non-cpufreq drivers) to disable the frequency switching. It may not
fit really well here.

Instead what Tegra's driver could do is remove cpufreq_dt_pdev, which
shall lead to unregistering the cpufreq driver completely.

-- 
viresh

Re: [PATCH 2/2] cpufreq: tegra124: Allow building as a module

[Viresh Kumar]

On 20-04-25, 15:42, Aaron Kling via B4 Relay wrote:
> From: Aaron Kling <webgeek1234@gmail.com>
> 
> This requires a soft dependency on cpufreq-dt as this driver only
> handles power management and cpufreq-dt does the real operations. Other
> than that, this works fine as a module, so allow building as such.
> 
> Signed-off-by: Aaron Kling <webgeek1234@gmail.com>
> ---
>  drivers/cpufreq/Kconfig.arm        | 2 +-
>  drivers/cpufreq/tegra124-cpufreq.c | 2 ++
>  2 files changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
> index 4f9cb943d945c244eb2b29f543d14df6cac4e5d4..625f6fbdaaf5fd774e3b0bb996eb7ce980da41ee 100644
> --- a/drivers/cpufreq/Kconfig.arm
> +++ b/drivers/cpufreq/Kconfig.arm
> @@ -238,7 +238,7 @@ config ARM_TEGRA20_CPUFREQ
>  	  This adds the CPUFreq driver support for Tegra20/30 SOCs.
>  
>  config ARM_TEGRA124_CPUFREQ
> -	bool "Tegra124 CPUFreq support"
> +	tristate "Tegra124 CPUFreq support"
>  	depends on ARCH_TEGRA || COMPILE_TEST
>  	depends on CPUFREQ_DT
>  	default y
> diff --git a/drivers/cpufreq/tegra124-cpufreq.c b/drivers/cpufreq/tegra124-cpufreq.c
> index 514146d98bca2d8aa59980a14dff3487cd8045f6..9e46b53487fafdc1e9e95e765b9b490d9c95f2c6 100644
> --- a/drivers/cpufreq/tegra124-cpufreq.c
> +++ b/drivers/cpufreq/tegra124-cpufreq.c
> @@ -211,5 +211,7 @@ static int __init tegra_cpufreq_init(void)
>  }
>  module_init(tegra_cpufreq_init);
>  
> +MODULE_SOFTDEP("pre: cpufreq-dt");
>  MODULE_AUTHOR("Tuomas Tynkkynen <ttynkkynen@nvidia.com>");
>  MODULE_DESCRIPTION("cpufreq driver for NVIDIA Tegra124");
> +MODULE_LICENSE("GPL");

Can you try inserting/removing/and inserting the driver again ? I
believe it will lead to some failures as no cleanup is being done by
the driver.

-- 
viresh

Re: [PATCH 2/2] cpufreq: tegra124: Allow building as a module

[Aaron Kling]

On Mon, Apr 21, 2025 at 12:45 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
>
> On 20-04-25, 15:42, Aaron Kling via B4 Relay wrote:
> > From: Aaron Kling <webgeek1234@gmail.com>
> >
> > This requires a soft dependency on cpufreq-dt as this driver only
> > handles power management and cpufreq-dt does the real operations. Other
> > than that, this works fine as a module, so allow building as such.
> >
> > Signed-off-by: Aaron Kling <webgeek1234@gmail.com>
> > ---
> >  drivers/cpufreq/Kconfig.arm        | 2 +-
> >  drivers/cpufreq/tegra124-cpufreq.c | 2 ++
> >  2 files changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
> > index 4f9cb943d945c244eb2b29f543d14df6cac4e5d4..625f6fbdaaf5fd774e3b0bb996eb7ce980da41ee 100644
> > --- a/drivers/cpufreq/Kconfig.arm
> > +++ b/drivers/cpufreq/Kconfig.arm
> > @@ -238,7 +238,7 @@ config ARM_TEGRA20_CPUFREQ
> >         This adds the CPUFreq driver support for Tegra20/30 SOCs.
> >
> >  config ARM_TEGRA124_CPUFREQ
> > -     bool "Tegra124 CPUFreq support"
> > +     tristate "Tegra124 CPUFreq support"
> >       depends on ARCH_TEGRA || COMPILE_TEST
> >       depends on CPUFREQ_DT
> >       default y
> > diff --git a/drivers/cpufreq/tegra124-cpufreq.c b/drivers/cpufreq/tegra124-cpufreq.c
> > index 514146d98bca2d8aa59980a14dff3487cd8045f6..9e46b53487fafdc1e9e95e765b9b490d9c95f2c6 100644
> > --- a/drivers/cpufreq/tegra124-cpufreq.c
> > +++ b/drivers/cpufreq/tegra124-cpufreq.c
> > @@ -211,5 +211,7 @@ static int __init tegra_cpufreq_init(void)
> >  }
> >  module_init(tegra_cpufreq_init);
> >
> > +MODULE_SOFTDEP("pre: cpufreq-dt");
> >  MODULE_AUTHOR("Tuomas Tynkkynen <ttynkkynen@nvidia.com>");
> >  MODULE_DESCRIPTION("cpufreq driver for NVIDIA Tegra124");
> > +MODULE_LICENSE("GPL");
>
> Can you try inserting/removing/and inserting the driver again ? I
> believe it will lead to some failures as no cleanup is being done by
> the driver.
lsmod lists the module as "[permanent]" and if I try to rmmod the
module, the command throws 'device or resource busy'.

Sincerely,
Aaron

Re: [PATCH 1/2] cpufreq: Export disable_cpufreq()

[Aaron Kling]

On Mon, Apr 21, 2025 at 12:44 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
>
> On 20-04-25, 15:42, Aaron Kling via B4 Relay wrote:
> > From: Aaron Kling <webgeek1234@gmail.com>
> >
> > This is used by the tegra124-cpufreq driver.
> >
> > Signed-off-by: Aaron Kling <webgeek1234@gmail.com>
> > ---
> >  drivers/cpufreq/cpufreq.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> > index 0cf5a320bb5e864709b259249fb1af8bfbc0b04b..78cddc78ee98630f99ccc332c64c94f437297177 100644
> > --- a/drivers/cpufreq/cpufreq.c
> > +++ b/drivers/cpufreq/cpufreq.c
> > @@ -109,6 +109,7 @@ void disable_cpufreq(void)
> >  {
> >       off = 1;
> >  }
> > +EXPORT_SYMBOL_GPL(disable_cpufreq);
> >  static DEFINE_MUTEX(cpufreq_governor_mutex);
> >
> >  bool have_governor_per_policy(void)
>
> I didn't realize earlier that tegra124 uses this call directly. I
> think this call was rather meant for other parts of the kernel
> (non-cpufreq drivers) to disable the frequency switching. It may not
> fit really well here.
>
> Instead what Tegra's driver could do is remove cpufreq_dt_pdev, which
> shall lead to unregistering the cpufreq driver completely.
This makes a lot more sense. I will look into that and upload a v2
swapping this patch for that once I verify it.

Sincerely,
Aaron

Re: [PATCH 2/2] cpufreq: tegra124: Allow building as a module

[Viresh Kumar]

On 21-04-25, 00:52, Aaron Kling wrote:
> lsmod lists the module as "[permanent]" and if I try to rmmod the
> module, the command throws 'device or resource busy'.

Ahh, I thought kernel will give error on double insertion instead. But
it looks like we keep some sort of refcount for the devices registered
by the driver and don't let it unload.

Anyway, if you want it to be a module, then it must unload as well I
guess.

You need to do the reverse of tegra_cpufreq_init() from module_exit to
get it working I guess.

-- 
viresh

Re: [PATCH 2/2] cpufreq: tegra124: Allow building as a module

[Aaron Kling]

On Mon, Apr 21, 2025 at 12:58 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
>
> On 21-04-25, 00:52, Aaron Kling wrote:
> > lsmod lists the module as "[permanent]" and if I try to rmmod the
> > module, the command throws 'device or resource busy'.
>
> Ahh, I thought kernel will give error on double insertion instead. But
> it looks like we keep some sort of refcount for the devices registered
> by the driver and don't let it unload.

This situation piqued my interest since I was unfamiliar with the
permanent flag in lsmod. And I found this stackoverflow answer [0]
that explained it. If no module_exit is defined, the module cannot be
removed.

>
> Anyway, if you want it to be a module, then it must unload as well I
> guess.
>
> You need to do the reverse of tegra_cpufreq_init() from module_exit to
> get it working I guess.
I will look into adding an exit for v2.

Sincerely,
Aaron

[0] https://stackoverflow.com/a/33464275