On Wed, Aug 11, 2010 at 10:10 AM, Amit Arora <amit.ar...@linaro.org> wrote:

>
> What to review:
> --------------------
> Once you checkout the "linaro" branch, as suggested above. You can
> review the two patches with commit id "09049b42e3" and "783a3e6bbe".
>
> What to test:
> ----------------
> Some of the things which could be tested is,
> o Compile and test on any ARM board that you may have. You can also
> test on any x86 / x86_64 systems, to check if nothing has been broken.
> o Compare the output from "master" and "linaro" branches ("-d" dump
> option may help, if you wish to take a diff)
> o Check if the number of C states is correct
> (/sys/devices/system/cpu/cpu0/cpuidle/)
> o Check if the number of P states is correct
> (/sys/devices/system/cpu/cpu0/cpufreq/scaling_available_frequencies)
> o Check if the % values makes sense (say, if total doesn't add to 100!
> Or, if its a very huge value [1] ).
> o If number of Wakeups-per-second [1] looks reasonable.
> o Top causes for wakeups is shown properly
> o Suggestions are shown properly. Some of the suggestions may show a
> hot key on the status bar (at bottom).
>
> [1] There is a known issue of %age values and Wakeups-per-sec number
> being shown very high (in the order of thousands), if some key is hit
> (say, 'r' to refresh or any other key). This problem can be seen in
> "master" branch (upstream) too, and hence is not introduced by the new
> patches. Since the goal of these patches is different, I haven't
> addressed this issue here.


Some comments:

1. Please run scripts/checkpatch.pl on the patches and fixup the
whitespace warnings.
2. In print_cstates(), where you have #ifdef'ed __i386__, the #else
should also be restricted to __arm__ IMO.
3. On my quad core desktop, it doesn't show C1 as a supported C-state
on startup.
                     (Supported C-states : C2 C3)
4. Works on Beagleboard.

Could you please try and post this to the powertop list this week?

Regards,
Amit

_______________________________________________
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev

Reply via email to