Re: [PATCH] tools/power: amd_pstate_trace: fix help path dependencies

[K Prateek Nayak]

Hello Rafael,

On 7/22/2026 6:18 PM, Rafael J. Wysocki (Intel) wrote:
> On Wed, Jun 24, 2026 at 2:31 PM Yousef Alhouseen
> <alhouseenyousef@xxxxxxxxx> wrote:
>>
>> amd_pstate_trace imports Gnuplot and numpy before parsing command-line
>> options. As a result, even "-h" fails if those optional runtime modules are
>> not installed.
>>
>> It also handles "-h" by printing a blank line and exiting. Use the existing
>> help printer for "-h", and move the numpy import to the path that needs it.
>> The direct Gnuplot import is unused, so remove it.
>>
>> Signed-off-by: Yousef Alhouseen <alhouseenyousef@xxxxxxxxx>
> 
> AMD folks, if this change is fine with you, I can take it into
> pm-tools, but an ACK (or equivalent) is needed.

Sorry for the delay in reviewing! I have some comments below for Yousef.

> 
> Thanks!
> 
>> ---
>>  tools/power/x86/amd_pstate_tracer/amd_pstate_trace.py | 11 +++++------
>>  1 file changed, 5 insertions(+), 6 deletions(-)
>>
>> diff --git a/tools/power/x86/amd_pstate_tracer/amd_pstate_trace.py b/tools/power/x86/amd_pstate_tracer/amd_pstate_trace.py
>> index 875b08655..b1b201b2e 100755
>> --- a/tools/power/x86/amd_pstate_tracer/amd_pstate_trace.py
>> +++ b/tools/power/x86/amd_pstate_tracer/amd_pstate_trace.py
>> @@ -27,8 +27,6 @@ import re
>>  import signal
>>  import sys
>>  import getopt
>> -import Gnuplot

We don't use Gnuplot so removing that is fine ...

>> -from numpy import *

... but we want to keep all the imports together at top to easily audit
dependencies. Afacit, most python scripts in tools/power/x86/ follow
that convention.

There exists a big fat comment on top of the file reading:

  Prerequisites:
      Python version 2.7.x or higher
      gnuplot 5.0 or higher
      gnuplot-py 1.8 or higher

which is a good indication of whatis actually required to run the
script. If users prefer, we can add a requirements.txt that pip can
consume to install the necessary dependencies and make it clear.

>>  from decimal import *
>>  sys.path.append(os.path.join(os.path.dirname(__file__), "..", "intel_pstate_tracer"))
>>  import intel_pstate_tracer as ipt
>> @@ -260,9 +258,6 @@ graph_data_present = False;
>>  valid1 = False
>>  valid2 = False
>>
>> -cpu_mask = zeros((MAX_CPUS,), dtype=int)
>> -
>> -
>>  try:
>>      opts, args = getopt.getopt(sys.argv[1:],"ht:i:c:n:m:",["help","trace_file=","interval=","cpu=","name=","memory="])
>>  except getopt.GetoptError:
>> @@ -270,7 +265,7 @@ except getopt.GetoptError:
>>      sys.exit(2)
>>  for opt, arg in opts:
>>      if opt == '-h':
>> -        print()
>> +        ipt.print_help('amd_pstate')
>>          sys.exit()
>>      elif opt in ("-t", "--trace_file"):
>>          valid1 = True
>> @@ -291,6 +286,10 @@ if not (valid1 and valid2):
>>      ipt.print_help('amd_pstate')
>>      sys.exit()
>>
>> +from numpy import zeros

Having it here needs users to skim through the file to know what
external packages the script needs and we don't want that.

>> +
>> +cpu_mask = zeros((MAX_CPUS,), dtype=int)
>> +
>>  if cpu_list:
>>      for p in re.split("[,]", cpu_list):
>>          if int(p) < MAX_CPUS :
>> --
>> 2.54.0
>>
>>

-- 
Thanks and Regards,
Prateek