On Wed, Aug 11, 2010 at 5:42 PM, Amit Kucheria <amit.kuche...@linaro.org> wrote: > Some comments: > > 1. Please run scripts/checkpatch.pl on the patches and fixup the > whitespace warnings. Sure.
> 2. In print_cstates(), where you have #ifdef'ed __i386__, the #else > should also be restricted to __arm__ IMO. OK. > 3. On my quad core desktop, it doesn't show C1 as a supported C-state > on startup. > (Supported C-states : C2 C3) That could be because print_cstates() shows only those C states which support MWAIT (have mwait in "desc" file for that particular C state). Should this be different for ARM ? Should we show all the states (whether they have mwait or not in "desc") ? On a second thought, print_cstates() for intel makes sense, since they want to tell which states are supported by BIOS and which are supported by the CPU (using, cpuid). But, for ARM, we have only one set of supported states and these will be same as what will be shown in the next screen. Thats why I have kept the second patch separate, since we can remove that if its really not required. Thoughts ? > 4. Works on Beagleboard. Nice. > Could you please try and post this to the powertop list this week? Sure. Will wait for one more day for others to comment and then post it. Thanks! Regards, Amit Arora > Regards, > Amit _______________________________________________ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
