[PATCH umr] Allow * for ipname in --read/--write/--write-bit

[Tom St Denis]

On 23/02/17 01:41 PM, Andres Rodriguez wrote:
>
>
> On 2017-02-23 09:46 AM, Tom St Denis wrote:
>> To simplify scripting you can now use * for ipnames as well.
>
> Thanks for the patch, this change is very welcome.
>
>>
>> Signed-off-by: Tom St Denis <tom.stdenis at amd.com>
>> ---
>>  doc/umr.1         |   6 ++-
>>  src/app/main.c    |   5 ++-
>>  src/app/scan.c    |  21 ++++++++++
>>  src/app/set_bit.c | 117
>> +++++++++++++++++++++++++++---------------------------
>>  src/app/set_reg.c |  91 +++++++++++++++++++++---------------------
>>  5 files changed, 131 insertions(+), 109 deletions(-)
>>
>> diff --git a/doc/umr.1 b/doc/umr.1
>> index 5ed2c3bb93ae..a12dfb0a00ff 100644
>> --- a/doc/umr.1
>> +++ b/doc/umr.1
>> @@ -34,7 +34,7 @@ Look up an MMIO register by address and bitfield
>> decode the value specified.
>>  Write a value specified in hex to a register specified with a complete
>>  register path in the form <
>>  .B asicname.ipname.regname
>> ->.  For example, fiji.uvd6.mmUVD_CGC_GATE.  The value of asicname can be
>> +>.  For example, fiji.uvd6.mmUVD_CGC_GATE.  The value of asicname
>> and/or ipname can be
>>  .B *
>>  to simplify scripting.  This command can be used multiple times to
>>  write to multiple registers in a single invocation.
>> @@ -52,7 +52,9 @@ command but also allows
>>  for the regname field.
>>  .IP "--scan, -s <string>"
>>  Scan and print an IP block by name, for example,
>> -.B uvd5.
>> +.B uvd6
>> +or
>> +.B carrizo.uvd6.
>>  Can be used multiple times in a single invocation.
>>  .IP "--ring, -R <string>(from:to)"
>>  Read the contents of a ring named by the string without the
>> diff --git a/src/app/main.c b/src/app/main.c
>> index 8364d54c5227..99e014888571 100644
>> --- a/src/app/main.c
>> +++ b/src/app/main.c
>> @@ -364,12 +364,13 @@ int main(int argc, char **argv)
>>  "\n\t--write, -w <address> <number>\n\t\tWrite a value in hex to a
>> register specified as a register path in the"
>>      "\n\t\tform <asicname.ipname.regname>.  For instance
>> \"tonga.uvd5.mmUVD_SOFT_RESET\"."
>>      "\n\t\tCan be used multiple times to set multiple registers.  You
>> can"
>> -    "\n\t\tspecify * for asicname to simplify scripts.\n"
>> +    "\n\t\tspecify * for asicname and/or ipname to simplify scripts.\n"
>>  "\n\t--writebit, -wb <string> <number>\n\t\tWrite a value in hex to a
>> register bitfield specified as in --write but"
>>      "\n\t\tthe addition of the bitfield name.  For instance:
>> \"*.gfx80.mmRLC_PG_CNTL.PG_OVERRIDE\"\n"
>>  "\n\t--read, -r <string>\n\t\tRead a value from a register and print
>> it to stdout.  This command"
>>      "\n\t\tuses the same path notation as --write.  It also accepts *
>> for regname.\n"
>> -"\n\t--scan, -s <string>\n\t\tScan and print an ip block by name,
>> e.g. \"uvd5\".  Can be used multiple times.\n"
>> +"\n\t--scan, -s <string>\n\t\tScan and print an ip block by name,
>> e.g. \"uvd6\" or \"carrizo.uvd6\"."
>> +    "\n\t\tCan be used multiple times.\n"
>>  "\n\t--ring, -R <string>([from:to])\n\t\tRead the contents of a ring
>> named by the string without the amdgpu_ring_ prefix. "
>>      "\n\t\tBy default it will read and display the entire ring.  A
>> starting and ending "
>>      "\n\t\taddress can be specified in decimal or a '.' can be used
>> to indicate relative "
>> diff --git a/src/app/scan.c b/src/app/scan.c
>> index 19c97fe1499d..fbb8f5838069 100644
>> --- a/src/app/scan.c
>> +++ b/src/app/scan.c
>> @@ -31,6 +31,27 @@ int umr_scan_asic(struct umr_asic *asic, char
>> *asicname, char *ipname, char *reg
>>      uint64_t addr, scale;
>>      char buf[256];
>>
>> +    // if ipname == '*' scan for a match and stop on first one
>> +    if (ipname[0] == '*' && regname[0] != '*') {
>> +        for (i = 0; i < asic->no_blocks; i++) {
>> +            for (j = 0; j < asic->blocks[i]->no_regs; j++) {
>> +                if (!strcmp(regname,
>> asic->blocks[i]->regs[j].regname) ||
>> +                    (options.many &&
>> strstr(asic->blocks[i]->regs[j].regname, regname))) {
>> +                    ipname = asic->blocks[i]->ipname;
>> +                    goto ip_found;
>
> Since the star globs usually imply a wildcard match, I think it is a
> little weird to truncate the output to only the first match.
>
> If a star glob is requested by the user, I think it should be okay to
> loop and print multiple outputs as usually happens when options.many is
> enabled.
>
> Alternative approach would be to print an 'ambiguous argument' error.
> That way the user will at least have some feedback. Although I think the
> behaviour of printing multiple results is preferable.

Now that I think about it is possible that future asics have multiples 
of IP blocks so ideally we should continue on for as many collisions.

I think the '-O named' option should then print the IP name as well.

I'll (v2) this shortly.

Thanks,
Tom