Re: [PATCH v2] cpupower : Handle set and info subcommands correctly

[shuah]

Hi Abhishek,

On 10/3/19 8:38 AM, shuah wrote:
> On 9/13/19 2:07 AM, Abhishek Goel wrote:
> > Cpupower tool has set and info options which are being used only by
> > x86 machines. This patch removes support for these two subcommands
> > from generic cpupower utility. Thus, these two subcommands will now be
> > available only for intel.
> > This removes the ambiguous error message while using set option in case
> > of using non-intel systems.
> >
> > Without this patch on a non-intel box:
> >
> > root@ubuntu:~# cpupower info
> > System does not support Intel's performance bias setting
> >
> > root@ubuntu:~# cpupower set -b 10
> > Error setting perf-bias value on CPU
> >
> > With this patch on a non-intel box:
> >
> > root@ubuntu:~# cpupower info
> > Supported commands are:
> > ÂÂÂÂÂÂÂÂ frequency-info
> > ÂÂÂÂÂÂÂÂ frequency-set
> > ÂÂÂÂÂÂÂÂ idle-info
> > ÂÂÂÂÂÂÂÂ idle-set
> > ÂÂÂÂÂÂÂÂ monitor
> > ÂÂÂÂÂÂÂÂ help
> >
> > Same result for set subcommand.
> >
> > This patch does not affect results on a intel box.
> >
> > Signed-off-by: Abhishek Goel <huntbag@xxxxxxxxxxxxxxxxxx>
> > Acked-by: Thomas Renninger <trenn@xxxxxxx>
> > ---
> >
> > changes from v1:
> > ÂÂÂÂInstead of bailing out early in set and info commands, in V2, we
> > ÂÂÂÂare cutting out support for these two commands for non-intel
> > ÂÂÂÂsystems.
>
> thanks. I will get this in for 5.4-rc3 veru likely. Definitely in 5.4

Okay I almost applied this and decided it needs improvements.

I don't like using #if defined(__x86_64__) || defined(__i386__)

tools/power/cpupower/utils/cpupower.c main() already does this
dynamically using uname(). Please use the same logic do this,
instead of adding compile time code.

thanks,
-- Shuah