| Message ID | 20250417065535.21358-1-johan+linaro@kernel.org (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | [v2] cpufreq: fix compile-test defaults | expand |
On 17/04/2025 08:55, Johan Hovold wrote: > 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? > > 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? > 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. > > 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. > Cc: stable@vger.kernel.org # 6.12 > Cc: Rob Herring (Arm) <robh@kernel.org> > Cc: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > Signed-off-by: Johan Hovold <johan+linaro@kernel.org> > --- > > Changes in v2: > - rebase on commit d4f610a9bafd ("cpufreq: Do not enable by default > during compile testing") > Best regards, Krzysztof
On Thu, Apr 17, 2025 at 09:10:09AM +0200, Krzysztof Kozlowski wrote: > On 17/04/2025 08:55, Johan Hovold wrote: > > 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? Correct, but this was how I found the issue and motivation for backporting the fixes including yours which was not marked for stable. > > 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. > > 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/20250416134331.7604-1-johan+linaro@kernel.org/ > > 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. > > Changes in v2: > > - rebase on commit d4f610a9bafd ("cpufreq: Do not enable by default > > during compile testing") Johan
On 17/04/2025 09:22, Johan Hovold wrote: > On Thu, Apr 17, 2025 at 09:10:09AM +0200, Krzysztof Kozlowski wrote: >> On 17/04/2025 08:55, Johan Hovold wrote: >>> 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? > > 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. > >>> 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. > >>> 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/20250416134331.7604-1-johan+linaro@kernel.org/ > >>> 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. 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/8b6ede05-281a-4fb1-bcdc-457e6f2610ff@roeck-us.net/ I had also talks about this in the past that one should never boot compile test kernel. Best regards, Krzysztof
On Thu, Apr 17, 2025 at 09:28:43AM +0200, Krzysztof Kozlowski wrote: > On 17/04/2025 09:22, Johan Hovold wrote: > >>> 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. > > 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/8b6ede05-281a-4fb1-bcdc-457e6f2610ff@roeck-us.net/ 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. > 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
On 17/04/2025 09:46, Johan Hovold wrote: > On Thu, Apr 17, 2025 at 09:28:43AM +0200, Krzysztof Kozlowski wrote: >> On 17/04/2025 09:22, Johan Hovold wrote: > >>>>> 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. >> >> 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/8b6ede05-281a-4fb1-bcdc-457e6f2610ff@roeck-us.net/ > > 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
diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm index d4d625ded285..0d46402e3094 100644 --- a/drivers/cpufreq/Kconfig.arm +++ b/drivers/cpufreq/Kconfig.arm @@ -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.
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: stable@vger.kernel.org # 6.12 Cc: Rob Herring (Arm) <robh@kernel.org> Cc: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> Signed-off-by: Johan Hovold <johan+linaro@kernel.org> --- 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(-)