Re: [PATCH] selftests: cpufreq: Check cpuinfo_cur_freq set as expected
From: Viresh Kumar
Date: Thu Jul 13 2017 - 04:55:56 EST
On 12-07-17, 14:29, Leonard Crestez wrote:
> This checks that the cpufreq driver actually sets the requested
> frequency.
>
> Signed-off-by: Leonard Crestez <leonard.crestez@xxxxxxx>
>
> ---
>
> I've been looking at using kselftests for imx. This patch exposes an
> issue with the imx6 cpufreq driver on imx6sx where frequencies are set
> incorrectly because of clk mishandling. This is already caught by some
> internal test scripts which also run against upstream but it's nice to
> make this visible through kselftest.
Sure, thanks for that.
> I'm not sure it's correct to check that frequency matches exactly,
> perhaps something like a 5% tolerance should be included for complex
> drivers where the target freq is only a "hint"?
We can do better, see below..
> I checked intel_pstate
> but it doesn't even seem to expose an userspace governor for manual
> frequency selection anyway.
Sure, and so that wouldn't be affected by this.
> Unfortunately cpufreq selftests don't seem to have a clear idea of
> "pass" or "fail" results.
Yeah, I had this test setup for a while and just pushed it through.
Over that many tests aren't really tests but just looking out for
crashes, etc. Never got a chance to improve it :(
> This patch will just print some TAP-like
> "ok" and "not ok" lines but failures are not actually propagated upwards
> in a well-defined way.
That would be fine for now.
> Have you considered what it would take to TAP-ify the output of cpufreq
> tests? Output is very complex so perhaps it might make sense to adopt some
> sort of subtest syntax for kselftest, something like this:
Not yet :(
> diff --git a/tools/testing/selftests/cpufreq/cpufreq.sh b/tools/testing/selftests/cpufreq/cpufreq.sh
> index 1ed3832..323b5bb 100755
> --- a/tools/testing/selftests/cpufreq/cpufreq.sh
> +++ b/tools/testing/selftests/cpufreq/cpufreq.sh
> @@ -151,6 +151,14 @@ test_all_frequencies()
> # Set all frequencies one-by-one
> for freq in $freqs; do
> set_cpu_frequency $1 $freq
> +
> + local cur_freq
> + cur_freq=`cat $CPUFREQROOT/$1/cpuinfo_cur_freq`
Yes, we want to verify if freq change happened or not, but may be only
reading scaling_cur_freq would be enough for now?
And that wouldn't be a problem for X86 (which Rafael mentioned) as
well IIUC.
--
viresh