linux-clk.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] clk: tegra: fix SMP build
@ 2018-12-11 14:35 Arnd Bergmann
  2018-12-11 17:21 ` Stephen Boyd
  0 siblings, 1 reply; 5+ messages in thread
From: Arnd Bergmann @ 2018-12-11 14:35 UTC (permalink / raw)
  To: Peter De Schrijver, Prashant Gaikwad, Michael Turquette, Stephen Boyd
  Cc: Arnd Bergmann, Thierry Reding, Jonathan Hunter, Dmitry Osipenko,
	Aapo Vienamo, linux-clk, linux-tegra, linux-kernel

When CONFIG_SMP is disabled, the tegra clk driver now fails to build:

drivers/clk/tegra/clk-tegra30.c: In function 'tegra30_cpu_rail_off_ready':
drivers/clk/tegra/clk-tegra30.c:1151:19: error: implicit declaration of function 'tegra_pmc_cpu_is_powered'; did you mean 'tegra_powergate_is_powered'? [-Werror=implicit-function-declaration]
  cpu_pwr_status = tegra_pmc_cpu_is_powered(1) ||

I don't know if tegra works without CONFIG_SMP, but we can get it to
build by making the calls conditional, and removing the pointless
ifdef around the declaration. The assumption now is that in a
non-SMP system, the secondary CPUs are always disabled.

Fixes: 61866523ed6e ("clk: tegra30: Use Tegra CPU powergate helper function")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
Not sure if this is the best solution. If you think it's not, please
submit a different fix.
---
 drivers/clk/tegra/clk-tegra30.c | 10 +++++++---
 include/soc/tegra/pmc.h         |  2 --
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/clk/tegra/clk-tegra30.c b/drivers/clk/tegra/clk-tegra30.c
index fa8d573ac626..1505185936f4 100644
--- a/drivers/clk/tegra/clk-tegra30.c
+++ b/drivers/clk/tegra/clk-tegra30.c
@@ -1148,9 +1148,13 @@ static bool tegra30_cpu_rail_off_ready(void)
 
 	cpu_rst_status = readl(clk_base +
 				TEGRA30_CLK_RST_CONTROLLER_CPU_CMPLX_STATUS);
-	cpu_pwr_status = tegra_pmc_cpu_is_powered(1) ||
-			 tegra_pmc_cpu_is_powered(2) ||
-			 tegra_pmc_cpu_is_powered(3);
+
+	if (IS_ENABLED(CONFIG_SMP))
+		cpu_pwr_status = tegra_pmc_cpu_is_powered(1) ||
+				 tegra_pmc_cpu_is_powered(2) ||
+				 tegra_pmc_cpu_is_powered(3);
+	else
+		cpu_pwr_status = 0;
 
 	if (((cpu_rst_status & 0xE) != 0xE) || cpu_pwr_status)
 		return false;
diff --git a/include/soc/tegra/pmc.h b/include/soc/tegra/pmc.h
index fd816f6aa9cc..a9db1b501de1 100644
--- a/include/soc/tegra/pmc.h
+++ b/include/soc/tegra/pmc.h
@@ -26,11 +26,9 @@
 struct clk;
 struct reset_control;
 
-#ifdef CONFIG_SMP
 bool tegra_pmc_cpu_is_powered(unsigned int cpuid);
 int tegra_pmc_cpu_power_on(unsigned int cpuid);
 int tegra_pmc_cpu_remove_clamping(unsigned int cpuid);
-#endif /* CONFIG_SMP */
 
 /*
  * powergate and I/O rail APIs
-- 
2.20.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] clk: tegra: fix SMP build
  2018-12-11 14:35 [PATCH] clk: tegra: fix SMP build Arnd Bergmann
@ 2018-12-11 17:21 ` Stephen Boyd
  2018-12-12 11:47   ` Jon Hunter
  0 siblings, 1 reply; 5+ messages in thread
From: Stephen Boyd @ 2018-12-11 17:21 UTC (permalink / raw)
  To: Arnd Bergmann, Michael Turquette, Peter De Schrijver, Prashant Gaikwad
  Cc: Arnd Bergmann, Thierry Reding, Jonathan Hunter, Dmitry Osipenko,
	Aapo Vienamo, linux-clk, linux-tegra, linux-kernel

Quoting Arnd Bergmann (2018-12-11 06:35:07)
> When CONFIG_SMP is disabled, the tegra clk driver now fails to build:
> 
> drivers/clk/tegra/clk-tegra30.c: In function 'tegra30_cpu_rail_off_ready':
> drivers/clk/tegra/clk-tegra30.c:1151:19: error: implicit declaration of function 'tegra_pmc_cpu_is_powered'; did you mean 'tegra_powergate_is_powered'? [-Werror=implicit-function-declaration]
>   cpu_pwr_status = tegra_pmc_cpu_is_powered(1) ||
> 
> I don't know if tegra works without CONFIG_SMP, but we can get it to
> build by making the calls conditional, and removing the pointless
> ifdef around the declaration. The assumption now is that in a
> non-SMP system, the secondary CPUs are always disabled.
> 
> Fixes: 61866523ed6e ("clk: tegra30: Use Tegra CPU powergate helper function")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> Not sure if this is the best solution. If you think it's not, please
> submit a different fix.

Hmm.. Is there any reason why the implementation of
tegra_pmc_cpu_is_powered() is under an ifdef CONFIG_SMP? I'd rather not
have to think about SMP or not in this clk code and have the
tegra_pmc_cpu_is_powered() function do the UP vs SMP code optimization.


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] clk: tegra: fix SMP build
  2018-12-11 17:21 ` Stephen Boyd
@ 2018-12-12 11:47   ` Jon Hunter
  2018-12-12 13:31     ` Thierry Reding
  0 siblings, 1 reply; 5+ messages in thread
From: Jon Hunter @ 2018-12-12 11:47 UTC (permalink / raw)
  To: Stephen Boyd, Arnd Bergmann, Michael Turquette,
	Peter De Schrijver, Prashant Gaikwad
  Cc: Thierry Reding, Dmitry Osipenko, Aapo Vienamo, linux-clk,
	linux-tegra, linux-kernel


On 11/12/2018 17:21, Stephen Boyd wrote:
> Quoting Arnd Bergmann (2018-12-11 06:35:07)
>> When CONFIG_SMP is disabled, the tegra clk driver now fails to build:
>>
>> drivers/clk/tegra/clk-tegra30.c: In function 'tegra30_cpu_rail_off_ready':
>> drivers/clk/tegra/clk-tegra30.c:1151:19: error: implicit declaration of function 'tegra_pmc_cpu_is_powered'; did you mean 'tegra_powergate_is_powered'? [-Werror=implicit-function-declaration]
>>   cpu_pwr_status = tegra_pmc_cpu_is_powered(1) ||
>>
>> I don't know if tegra works without CONFIG_SMP, but we can get it to
>> build by making the calls conditional, and removing the pointless
>> ifdef around the declaration. The assumption now is that in a
>> non-SMP system, the secondary CPUs are always disabled.
>>
>> Fixes: 61866523ed6e ("clk: tegra30: Use Tegra CPU powergate helper function")
>> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>> ---
>> Not sure if this is the best solution. If you think it's not, please
>> submit a different fix.
> 
> Hmm.. Is there any reason why the implementation of
> tegra_pmc_cpu_is_powered() is under an ifdef CONFIG_SMP? I'd rather not
> have to think about SMP or not in this clk code and have the
> tegra_pmc_cpu_is_powered() function do the UP vs SMP code optimization.

Not that I know of. I just think that the function should/would not be
used for non-SMP.

I was actually thinking that we could just leave the clk code as it is
and simply drop the CONFIG_SMP from pmc.h. That would be fine with me.

Cheers
Jon

-- 
nvpublic

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] clk: tegra: fix SMP build
  2018-12-12 11:47   ` Jon Hunter
@ 2018-12-12 13:31     ` Thierry Reding
  2018-12-12 17:20       ` Jon Hunter
  0 siblings, 1 reply; 5+ messages in thread
From: Thierry Reding @ 2018-12-12 13:31 UTC (permalink / raw)
  To: Jon Hunter
  Cc: Stephen Boyd, Arnd Bergmann, Michael Turquette,
	Peter De Schrijver, Prashant Gaikwad, Dmitry Osipenko,
	Aapo Vienamo, linux-clk, linux-tegra, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 2185 bytes --]

On Wed, Dec 12, 2018 at 11:47:32AM +0000, Jon Hunter wrote:
> 
> On 11/12/2018 17:21, Stephen Boyd wrote:
> > Quoting Arnd Bergmann (2018-12-11 06:35:07)
> >> When CONFIG_SMP is disabled, the tegra clk driver now fails to build:
> >>
> >> drivers/clk/tegra/clk-tegra30.c: In function 'tegra30_cpu_rail_off_ready':
> >> drivers/clk/tegra/clk-tegra30.c:1151:19: error: implicit declaration of function 'tegra_pmc_cpu_is_powered'; did you mean 'tegra_powergate_is_powered'? [-Werror=implicit-function-declaration]
> >>   cpu_pwr_status = tegra_pmc_cpu_is_powered(1) ||
> >>
> >> I don't know if tegra works without CONFIG_SMP, but we can get it to
> >> build by making the calls conditional, and removing the pointless
> >> ifdef around the declaration. The assumption now is that in a
> >> non-SMP system, the secondary CPUs are always disabled.
> >>
> >> Fixes: 61866523ed6e ("clk: tegra30: Use Tegra CPU powergate helper function")
> >> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> >> ---
> >> Not sure if this is the best solution. If you think it's not, please
> >> submit a different fix.
> > 
> > Hmm.. Is there any reason why the implementation of
> > tegra_pmc_cpu_is_powered() is under an ifdef CONFIG_SMP? I'd rather not
> > have to think about SMP or not in this clk code and have the
> > tegra_pmc_cpu_is_powered() function do the UP vs SMP code optimization.
> 
> Not that I know of. I just think that the function should/would not be
> used for non-SMP.
> 
> I was actually thinking that we could just leave the clk code as it is
> and simply drop the CONFIG_SMP from pmc.h. That would be fine with me.

Yeah, I'd be fine keeping that code around whether or not we enable SMP.
Chances are people won't disable it anyway. If they do, then most likely
only for testing purposes, in which case I'm sure they won't mind the
extra couple of bytes.

I think if we remove CONFIG_SMP from pmc.h we also need to remove it
from drivers/soc/tegra/pmc.c to make sure these functions are available,
otherwise we'll likely run into linker errors.

Jon, is that something I can interest you in? If not, I can easily do
that myself.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] clk: tegra: fix SMP build
  2018-12-12 13:31     ` Thierry Reding
@ 2018-12-12 17:20       ` Jon Hunter
  0 siblings, 0 replies; 5+ messages in thread
From: Jon Hunter @ 2018-12-12 17:20 UTC (permalink / raw)
  To: Thierry Reding
  Cc: Stephen Boyd, Arnd Bergmann, Michael Turquette,
	Peter De Schrijver, Prashant Gaikwad, Dmitry Osipenko,
	Aapo Vienamo, linux-clk, linux-tegra, linux-kernel


On 12/12/2018 13:31, Thierry Reding wrote:
> On Wed, Dec 12, 2018 at 11:47:32AM +0000, Jon Hunter wrote:
>>
>> On 11/12/2018 17:21, Stephen Boyd wrote:
>>> Quoting Arnd Bergmann (2018-12-11 06:35:07)
>>>> When CONFIG_SMP is disabled, the tegra clk driver now fails to build:
>>>>
>>>> drivers/clk/tegra/clk-tegra30.c: In function 'tegra30_cpu_rail_off_ready':
>>>> drivers/clk/tegra/clk-tegra30.c:1151:19: error: implicit declaration of function 'tegra_pmc_cpu_is_powered'; did you mean 'tegra_powergate_is_powered'? [-Werror=implicit-function-declaration]
>>>>   cpu_pwr_status = tegra_pmc_cpu_is_powered(1) ||
>>>>
>>>> I don't know if tegra works without CONFIG_SMP, but we can get it to
>>>> build by making the calls conditional, and removing the pointless
>>>> ifdef around the declaration. The assumption now is that in a
>>>> non-SMP system, the secondary CPUs are always disabled.
>>>>
>>>> Fixes: 61866523ed6e ("clk: tegra30: Use Tegra CPU powergate helper function")
>>>> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>>>> ---
>>>> Not sure if this is the best solution. If you think it's not, please
>>>> submit a different fix.
>>>
>>> Hmm.. Is there any reason why the implementation of
>>> tegra_pmc_cpu_is_powered() is under an ifdef CONFIG_SMP? I'd rather not
>>> have to think about SMP or not in this clk code and have the
>>> tegra_pmc_cpu_is_powered() function do the UP vs SMP code optimization.
>>
>> Not that I know of. I just think that the function should/would not be
>> used for non-SMP.
>>
>> I was actually thinking that we could just leave the clk code as it is
>> and simply drop the CONFIG_SMP from pmc.h. That would be fine with me.
> 
> Yeah, I'd be fine keeping that code around whether or not we enable SMP.
> Chances are people won't disable it anyway. If they do, then most likely
> only for testing purposes, in which case I'm sure they won't mind the
> extra couple of bytes.
> 
> I think if we remove CONFIG_SMP from pmc.h we also need to remove it
> from drivers/soc/tegra/pmc.c to make sure these functions are available,
> otherwise we'll likely run into linker errors.
> 
> Jon, is that something I can interest you in? If not, I can easily do
> that myself.

Yes I can do it (tomorrow).

Cheers
Jon

-- 
nvpublic

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2018-12-12 17:20 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-12-11 14:35 [PATCH] clk: tegra: fix SMP build Arnd Bergmann
2018-12-11 17:21 ` Stephen Boyd
2018-12-12 11:47   ` Jon Hunter
2018-12-12 13:31     ` Thierry Reding
2018-12-12 17:20       ` Jon Hunter

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).