Hi Viresh,

On 23 July 2014 13:08, Lukasz Majewski [off-list ref] wrote:

quoted

Do you want to say that we have enough tests and we don't need
more ?

No. We don't have any tests at all :)

Then we should encourage as many developers as possible to share their
private tests with us.

quoted

I always thought that we shall have as much regression tests as
possible.

Yeah, tests are welcomed but the question is where should they get
added. Don't know if its common to add tests directly to kernel.

There was a similar discussion with device tree and finally it was
included in the mainline repository.

And also if the test is really good, not discouraging your work.

quoted

quoted

On 21 July 2014 12:32, Lukasz Majewski [off-list ref]
wrote:

quoted

This commit adds first regression test "cpufreq_freq_test.sh" for
the cpufreq subsystem.

That's not enough, Tell us why we should continue reading this
mail..

Hmm... If "regression" and "test" don't catch the attention of a
diligent maintainer, then I cannot do much more to encourage him to
read the whole e-mail :-)

What I meant to say was, your subject and body must be good enough
to answer most of the things. You don't have to tell much about the
implementation but other things should be pretty clear from logs.

Your current logs are quite short for something that's not a normal
practice.

It is hard for me to agree on this issue.

quoted

I can imagine that maintainers are very busy, therefore I've
prepared README file with detailed description of the script
operation.

Yeah, a README is welcomed and would be useful for users as well..

quoted

quoted

I couldn't make out the purpose of this test and why we need it.
How do we ensure that "cpufreq attributes exported by sysfs are
exposing correct values"?

First of all the cpufreq attributes are part of the subsystem API.
There are systems which actually depend on them, so we would be
better off to test if they work as intended.

Secondly, the test takes those values and then with use of other
attribute enforce the value, which is then read via cat'ing
cpufreq_cur_freq. If any of the attributes is wrong then we will
spot the error immediately.

Shouldn't you use userspace governor then instead of performance?

Performance assures that we will have the right frequency set.

However, there can be a similar patch to use userspace governor and
various load to fail if ondemand's frequency flipping is detected.

And then we don't need the gzip stuff at all. We can just set it to
the right freq and get current freq to see if it matches?

Sometimes "interresting" things show up when you have 100% CPU load and
you try to switch frequency.
In my opinion usage of gzip makes the test more difficult to pass.

And now that we are starting to get tests added into the kernel (will
still wait to see what Rafael has to advice),

Ok. Lets wait for Rafael's opinion.

we better think of the
way these are going to get added. Probably a single script with
parameters like what to test?

It is one possible solution, where another one is to run the all scripts
in the directory.

I'm curious about Rafael's opinion.

quoted

quoted

And actually what do we mean by this statement even? What kind of
errors can be there in exposing these values.

Errors with cpufreq and CCF cooperation - especially when some
parts of cpufreq code uses direct write to MUX, DIV or PLL SoC
registers.

Also, one can check if permutations of changing all available
frequencies are working properly.

Yeah, that would be fine. Probably need to think more about scripts
name.

-- 
Best regards,

Lukasz Majewski

Samsung R&D Institute Poland (SRPOL) | Linux Platform Group