Skip to contentSkip to conversation

cpufreq: fix compile-test defaults

PATCH Superseded Johan Hovold 3 participants 6 messages Apr 17, 2025

Subsystems (2)

v2 of 3 Superseded Latest is v3 →

lore
Share & tools
lore
#0 lore raw

PATCH v2 1/1 cpufreq: fix compile-test defaults

Commit 3f66425a4fc8 ("cpufreq: Enable COMPILE_TEST on Arm drivers") enabled compile testing of most Arm CPUFreq drivers but left the existing default values unchanged so that many drivers are enabled by default whenever COMPILE_TEST is selected.

This specifically results in the S3C64XX CPUFreq driver being enabled and initialised during boot of non-S3C64XX platforms with the following error logged:

cpufreq: Unable to obtain ARMCLK: -2

Commit d4f610a9bafd ("cpufreq: Do not enable by default during compile testing") recently fixed most of the default values, but two entries were missed and two could use a more specific default condition.

Fix the default values for drivers that can be compile tested and that should be enabled by default when not compile testing.

Fixes 3f66425a4fc8 ("cpufreq: Enable COMPILE_TEST on Arm drivers")
Fixes d4f610a9bafd ("cpufreq: Do not enable by default during compile testing")
Cc Rob Herring (Arm) <[email protected]>
Cc Krzysztof Kozlowski <[email protected]>

---

Changes in v2:
- rebase on commit d4f610a9bafd ("cpufreq: Do not enable by default during compile testing")

 drivers/cpufreq/Kconfig.arm | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
drivers/cpufreq/Kconfig.arm +4 -4
index d4d625ded285..0d46402e3094 100644
@@ -76,7 +76,7 @@ config ARM_VEXPRESS_SPC_CPUFREQ
 config ARM_BRCMSTB_AVS_CPUFREQ
 	tristate "Broadcom STB AVS CPUfreq driver"
 	depends on (ARCH_BRCMSTB && !ARM_SCMI_CPUFREQ) || COMPILE_TEST
-	default ARCH_BRCMSTB
+	default y if ARCH_BRCMSTB && !ARM_SCMI_CPUFREQ
 	help
 	  Some Broadcom STB SoCs use a co-processor running proprietary firmware
 	  ("AVS") to handle voltage and frequency scaling. This driver provides
@@ -88,7 +88,7 @@ config ARM_HIGHBANK_CPUFREQ
 	tristate "Calxeda Highbank-based"
 	depends on ARCH_HIGHBANK || COMPILE_TEST
 	depends on CPUFREQ_DT && REGULATOR && PL320_MBOX
-	default m
+	default m if ARCH_HIGHBANK
 	help
 	  This adds the CPUFreq driver for Calxeda Highbank SoC
 	  based boards.
@@ -133,7 +133,7 @@ config ARM_MEDIATEK_CPUFREQ
 config ARM_MEDIATEK_CPUFREQ_HW
 	tristate "MediaTek CPUFreq HW driver"
 	depends on ARCH_MEDIATEK || COMPILE_TEST
-	default m
+	default m if ARCH_MEDIATEK
 	help
 	  Support for the CPUFreq HW driver.
 	  Some MediaTek chipsets have a HW engine to offload the steps
@@ -256,7 +256,7 @@ config ARM_TEGRA194_CPUFREQ
 	tristate "Tegra194 CPUFreq support"
 	depends on ARCH_TEGRA_194_SOC || ARCH_TEGRA_234_SOC || (64BIT && COMPILE_TEST)
 	depends on TEGRA_BPMP
-	default ARCH_TEGRA
+	default ARCH_TEGRA_194_SOC || ARCH_TEGRA_234_SOC
 	help
 	  This adds CPU frequency driver support for Tegra194 SOCs.
 
Signature · 1 line
2.49.0
#1 lore raw
Quoted 10 lines · Johan Hovold

Commit 3f66425a4fc8 ("cpufreq: Enable COMPILE_TEST on Arm drivers") enabled compile testing of most Arm CPUFreq drivers but left the existing default values unchanged so that many drivers are enabled by default whenever COMPILE_TEST is selected.

This specifically results in the S3C64XX CPUFreq driver being enabled and initialised during boot of non-S3C64XX platforms with the following error logged:

cpufreq: Unable to obtain ARMCLK: -2

But isn't this fixed by my commit (d4f610a9bafd)? How is it possible to reproduce above error when you are NOT test compiling?

Quoted 4 lines

Commit d4f610a9bafd ("cpufreq: Do not enable by default during compile testing") recently fixed most of the default values, but two entries were missed

That's not really a bug to be fixed. No things got worse by missing two entries, so how this part could be called something needing fixing?

Quoted 1 line

and two could use a more specific default condition.

Two entries for more specific default - before they were ALWAYS default, so again I narrowed it from wide default. Nothing to fix here. You can narrow it further but claiming that my commit made something worse looks like a stretch - and that's a meaning of fixing someone's commit.

Quoted 8 lines

Fix the default values for drivers that can be compile tested and that should be enabled by default when not compile testing.

Fixes 3f66425a4fc8 ("cpufreq: Enable COMPILE_TEST on Arm drivers")
Fixes d4f610a9bafd ("cpufreq: Do not enable by default during compile testing")

That's not correct tag - it introduced no new issues, did not make things worse, so nothing to fix there, if I understand correctly.

Quoted 10 lines
Cc Rob Herring (Arm) <[email protected]>
Cc Krzysztof Kozlowski <[email protected]>

---

Changes in v2:
- rebase on commit d4f610a9bafd ("cpufreq: Do not enable by default during compile testing")

Best regards,
Krzysztof

#2 lore raw
Quoted 6 lines · Krzysztof Kozlowski
Quoted 5 lines · Johan Hovold

Commit 3f66425a4fc8 ("cpufreq: Enable COMPILE_TEST on Arm drivers") enabled compile testing of most Arm CPUFreq drivers but left the existing default values unchanged so that many drivers are enabled by default whenever COMPILE_TEST is selected.

Quoted 8 lines
Quoted 5 lines

This specifically results in the S3C64XX CPUFreq driver being enabled and initialised during boot of non-S3C64XX platforms with the following error logged:

cpufreq: Unable to obtain ARMCLK: -2

But isn't this fixed by my commit (d4f610a9bafd)? How is it possible to reproduce above error when you are NOT test compiling?

Correct, but this was how I found the issue and motivation for backporting the fixes including yours which was not marked for stable.

Quoted 6 lines
Quoted 3 lines

Commit d4f610a9bafd ("cpufreq: Do not enable by default during compile testing") recently fixed most of the default values, but two entries were missed

That's not really a bug to be fixed. No things got worse by missing two entries, so how this part could be called something needing fixing?

I'm not saying it's buggy, I'm explaining that the identified issue was recently fixed partially.

Quoted 6 lines
Quoted 1 line

and two could use a more specific default condition.

Two entries for more specific default - before they were ALWAYS default, so again I narrowed it from wide default. Nothing to fix here. You can narrow it further but claiming that my commit made something worse looks like a stretch - and that's a meaning of fixing someone's commit.

Relax. I'm not blaming you for doing anything wrong here.

I sent a fix for the same issues you addressed and Viresh let me know that he had already merged a fix for most of the issues:

	https://lore.kernel.org/lkml/[email protected]/
Quoted 10 lines
Quoted 7 lines

Fix the default values for drivers that can be compile tested and that should be enabled by default when not compile testing.

Fixes 3f66425a4fc8 ("cpufreq: Enable COMPILE_TEST on Arm drivers")
Fixes d4f610a9bafd ("cpufreq: Do not enable by default during compile testing")

That's not correct tag - it introduced no new issues, did not make things worse, so nothing to fix there, if I understand correctly.

Fair enough, I could have used dependency notation for this one.

Let me do that in v3.

Quoted 3 lines
Quoted 3 lines

Changes in v2:
- rebase on commit d4f610a9bafd ("cpufreq: Do not enable by default during compile testing")

Johan

#3 lore raw
Quoted 11 lines · Johan Hovold
Quoted 10 lines · Krzysztof Kozlowski
Quoted 9 lines · Johan Hovold

Commit 3f66425a4fc8 ("cpufreq: Enable COMPILE_TEST on Arm drivers") enabled compile testing of most Arm CPUFreq drivers but left the existing default values unchanged so that many drivers are enabled by default whenever COMPILE_TEST is selected.

This specifically results in the S3C64XX CPUFreq driver being enabled and initialised during boot of non-S3C64XX platforms with the following error logged:

Quoted 7 lines
Quoted 4 lines
Quoted 1 line

cpufreq: Unable to obtain ARMCLK: -2

But isn't this fixed by my commit (d4f610a9bafd)? How is it possible to reproduce above error when you are NOT test compiling?

Correct, but this was how I found the issue and motivation for backporting the fixes including yours which was not marked for stable.

OK, just does not feel up to date anymore.

Quoted 31 lines
Quoted 7 lines
Quoted 4 lines

Commit d4f610a9bafd ("cpufreq: Do not enable by default during compile testing") recently fixed most of the default values, but two entries were missed

That's not really a bug to be fixed. No things got worse by missing two entries, so how this part could be called something needing fixing?

I'm not saying it's buggy, I'm explaining that the identified issue was recently fixed partially.

Quoted 6 lines
Quoted 1 line

and two could use a more specific default condition.

Two entries for more specific default - before they were ALWAYS default, so again I narrowed it from wide default. Nothing to fix here. You can narrow it further but claiming that my commit made something worse looks like a stretch - and that's a meaning of fixing someone's commit.

Relax. I'm not blaming you for doing anything wrong here.

I sent a fix for the same issues you addressed and Viresh let me know that he had already merged a fix for most of the issues:

	https://lore.kernel.org/lkml/[email protected]/
Quoted 6 lines
Quoted 4 lines

Fix the default values for drivers that can be compile tested and that should be enabled by default when not compile testing.

Fixes 3f66425a4fc8 ("cpufreq: Enable COMPILE_TEST on Arm drivers")
Quoted 8 lines
Quoted 4 lines
Quoted 1 line
Fixes d4f610a9bafd ("cpufreq: Do not enable by default during compile testing")

That's not correct tag - it introduced no new issues, did not make things worse, so nothing to fix there, if I understand correctly.

Fair enough, I could have used dependency notation for this one.

Let me do that in v3.

OK. I have doubts that this should be marked as a fix in the first place
- even skipping my commit. Some (several?) people were always considering COMPILE_TEST as enable everything, thus for them this was the intention, even if it causes such S3C64xx cpufreq warnings:

https://lore.kernel.org/all/[email protected]/

I had also talks about this in the past that one should never boot compile test kernel.

Best regards,
Krzysztof

#4 lore raw
Quoted 15 lines · Krzysztof Kozlowski
Quoted 13 lines · Johan Hovold
Quoted 10 lines
Quoted 7 lines

Fix the default values for drivers that can be compile tested and that should be enabled by default when not compile testing.

Fixes 3f66425a4fc8 ("cpufreq: Enable COMPILE_TEST on Arm drivers")
Fixes d4f610a9bafd ("cpufreq: Do not enable by default during compile testing")

That's not correct tag - it introduced no new issues, did not make things worse, so nothing to fix there, if I understand correctly.

Fair enough, I could have used dependency notation for this one.

Quoted 8 lines
Quoted 1 line

Let me do that in v3.

OK. I have doubts that this should be marked as a fix in the first place
- even skipping my commit. Some (several?) people were always considering COMPILE_TEST as enable everything, thus for them this was the intention, even if it causes such S3C64xx cpufreq warnings:

https://lore.kernel.org/all/[email protected]/

Sounds like you, me and Arnd and least have the same understanding of how COMPILE_TEST should work.

I use it all the time when fixing issues that have been reproduced in several drivers which I then enable manually. And I usually keep them enabled in my development kernels for a while after in case something needs to be reworked.

If you want to compile everything as well you should do an allmodconfig build.

Quoted 2 lines

I had also talks about this in the past that one should never boot compile test kernel.

I have never noticed any issues with that until the other day with the cpufreq driver, but yeah, I can imagine that other "default y" entries could potentially cause issues.

Johan

#5 lore raw
Quoted 23 lines · Johan Hovold
Quoted 22 lines · Krzysztof Kozlowski
Quoted 6 lines · Johan Hovold
Quoted 6 lines
Quoted 4 lines

Fix the default values for drivers that can be compile tested and that should be enabled by default when not compile testing.

Fixes 3f66425a4fc8 ("cpufreq: Enable COMPILE_TEST on Arm drivers")
Quoted 8 lines
Quoted 4 lines
Quoted 1 line
Fixes d4f610a9bafd ("cpufreq: Do not enable by default during compile testing")

That's not correct tag - it introduced no new issues, did not make things worse, so nothing to fix there, if I understand correctly.

Fair enough, I could have used dependency notation for this one.

Let me do that in v3.

OK. I have doubts that this should be marked as a fix in the first place
- even skipping my commit. Some (several?) people were always considering COMPILE_TEST as enable everything, thus for them this was the intention, even if it causes such S3C64xx cpufreq warnings:

Quoted 4 lines
Quoted 1 line

https://lore.kernel.org/all/[email protected]/

Sounds like you, me and Arnd and least have the same understanding of how COMPILE_TEST should work.

Yes. It does not make it necessarily a bug (It's not a bug, it's a feature). Anyway, I don't mind so up to you folks.

Best regards,
Krzysztof