Message-ID: <f957e366-51e1-4447-982c-93374d0fde2e@linaro.org>
Date: Thu, 17 Apr 2025 09:28:43 +0200
From: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To: Johan Hovold <johan@...nel.org>
Cc: Johan Hovold <johan+linaro@...nel.org>,
 "Rafael J. Wysocki" <rafael@...nel.org>,
 Viresh Kumar <viresh.kumar@...aro.org>, "Rob Herring (Arm)"
 <robh@...nel.org>, linux-pm@...r.kernel.org, linux-kernel@...r.kernel.org,
 stable@...r.kernel.org
Subject: Re: [PATCH v2] cpufreq: fix compile-test defaults

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

Powered by blists - more mailing lists