Re: [PATCH 21/22] adb: use adb_device prefix for ADB device trace events

[Mark Cave-Ayland]

On 14/06/2020 18:20, Philippe Mathieu-Daudé wrote:

> On 6/14/20 4:28 PM, Mark Cave-Ayland wrote:
>> This is to allow us to distinguish between ADB device events and ADB
>> bus events separately.
>>
>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>> ---
>>  hw/input/adb-kbd.c    | 12 ++++++------
>>  hw/input/adb-mouse.c  | 12 ++++++------
>>  hw/input/trace-events | 20 ++++++++++----------
>>  3 files changed, 22 insertions(+), 22 deletions(-)
>>
>> diff --git a/hw/input/adb-kbd.c b/hw/input/adb-kbd.c
>> index 23760ecf7b..3cfb6a7a20 100644
>> --- a/hw/input/adb-kbd.c
>> +++ b/hw/input/adb-kbd.c
>> @@ -243,7 +243,7 @@ static int adb_kbd_request(ADBDevice *d, uint8_t *obuf,
>>      olen = 0;
>>      switch (cmd) {
>>      case ADB_WRITEREG:
>> -        trace_adb_kbd_writereg(reg, buf[1]);
>> +        trace_adb_device_kbd_writereg(reg, buf[1]);
>>          switch (reg) {
>>          case 2:
>>              /* LED status */
>> @@ -256,7 +256,7 @@ static int adb_kbd_request(ADBDevice *d, uint8_t *obuf,
>>              case ADB_CMD_CHANGE_ID_AND_ACT:
>>              case ADB_CMD_CHANGE_ID_AND_ENABLE:
>>                  d->devaddr = buf[1] & 0xf;
>> -                trace_adb_kbd_request_change_addr(d->devaddr);
>> +                trace_adb_device_kbd_request_change_addr(d->devaddr);
>>                  break;
>>              default:
>>                  d->devaddr = buf[1] & 0xf;
>> @@ -270,8 +270,8 @@ static int adb_kbd_request(ADBDevice *d, uint8_t *obuf,
>>                      d->handler = buf[2];
>>                  }
>>  
>> -                trace_adb_kbd_request_change_addr_and_handler(d->devaddr,
>> -                                                              d->handler);
>> +                trace_adb_device_kbd_request_change_addr_and_handler(
>> +                    d->devaddr, d->handler);
>>                  break;
>>              }
>>          }
>> @@ -294,7 +294,7 @@ static int adb_kbd_request(ADBDevice *d, uint8_t *obuf,
>>              olen = 2;
>>              break;
>>          }
>> -        trace_adb_kbd_readreg(reg, obuf[0], obuf[1]);
>> +        trace_adb_device_kbd_readreg(reg, obuf[0], obuf[1]);
>>          break;
>>      }
>>      return olen;
>> @@ -321,7 +321,7 @@ static void adb_keyboard_event(DeviceState *dev, 
>> QemuConsole *src,
>>      /* FIXME: take handler into account when translating qcode */
>>      keycode = qcode_to_adb_keycode[qcode];
>>      if (keycode == NO_KEY) {  /* We don't want to send this to the guest */
>> -        trace_adb_kbd_no_key();
>> +        trace_adb_device_kbd_no_key();
>>          return;
>>      }
>>      if (evt->u.key.data->down == false) { /* if key release event */
>> diff --git a/hw/input/adb-mouse.c b/hw/input/adb-mouse.c
>> index e2359fd74d..577a38ff2e 100644
>> --- a/hw/input/adb-mouse.c
>> +++ b/hw/input/adb-mouse.c
>> @@ -121,7 +121,7 @@ static int adb_mouse_request(ADBDevice *d, uint8_t *obuf,
>>          s->dx = 0;
>>          s->dy = 0;
>>          s->dz = 0;
>> -        trace_adb_mouse_flush();
>> +        trace_adb_device_mouse_flush();
>>          return 0;
>>      }
>>  
>> @@ -130,7 +130,7 @@ static int adb_mouse_request(ADBDevice *d, uint8_t *obuf,
>>      olen = 0;
>>      switch (cmd) {
>>      case ADB_WRITEREG:
>> -        trace_adb_mouse_writereg(reg, buf[1]);
>> +        trace_adb_device_mouse_writereg(reg, buf[1]);
>>          switch (reg) {
>>          case 2:
>>              break;
>> @@ -152,7 +152,7 @@ static int adb_mouse_request(ADBDevice *d, uint8_t *obuf,
>>              case ADB_CMD_CHANGE_ID_AND_ACT:
>>              case ADB_CMD_CHANGE_ID_AND_ENABLE:
>>                  d->devaddr = buf[1] & 0xf;
>> -                trace_adb_mouse_request_change_addr(d->devaddr);
>> +                trace_adb_device_mouse_request_change_addr(d->devaddr);
>>                  break;
>>              default:
>>                  d->devaddr = buf[1] & 0xf;
>> @@ -172,8 +172,8 @@ static int adb_mouse_request(ADBDevice *d, uint8_t *obuf,
>>                      d->handler = buf[2];
>>                  }
>>  
>> -                trace_adb_mouse_request_change_addr_and_handler(d->devaddr,
>> -                                                                d->handler);
>> +                trace_adb_device_mouse_request_change_addr_and_handler(
>> +                    d->devaddr, d->handler);
>>                  break;
>>              }
>>          }
>> @@ -191,7 +191,7 @@ static int adb_mouse_request(ADBDevice *d, uint8_t *obuf,
>>              olen = 2;
>>              break;
>>          }
>> -        trace_adb_mouse_readreg(reg, obuf[0], obuf[1]);
>> +        trace_adb_device_mouse_readreg(reg, obuf[0], obuf[1]);
>>          break;
>>      }
>>      return olen;
>> diff --git a/hw/input/trace-events b/hw/input/trace-events
>> index a2888fd10c..6f0d78241c 100644
>> --- a/hw/input/trace-events
>> +++ b/hw/input/trace-events
>> @@ -1,18 +1,18 @@
>>  # See docs/devel/tracing.txt for syntax documentation.
>>  
>>  # adb-kbd.c
>> -adb_kbd_no_key(void) "Ignoring NO_KEY"
>> -adb_kbd_writereg(int reg, uint8_t val) "reg %d val 0x%2.2x"
>> -adb_kbd_readreg(int reg, uint8_t val0, uint8_t val1) "reg %d obuf[0] 
>> 0x%2.2x obuf[1] 0x%2.2x"
>> -adb_kbd_request_change_addr(int devaddr) "change addr to 0x%x"
>> -adb_kbd_request_change_addr_and_handler(int devaddr, int handler) "change 
>> addr and handler to 0x%x, 0x%x"
>> +adb_device_kbd_no_key(void) "Ignoring NO_KEY"
>> +adb_device_kbd_writereg(int reg, uint8_t val) "reg %d val 0x%2.2x"
>> +adb_device_kbd_readreg(int reg, uint8_t val0, uint8_t val1) "reg %d obuf[0] 
>> 0x%2.2x obuf[1] 0x%2.2x"
>> +adb_device_kbd_request_change_addr(int devaddr) "change addr to 0x%x"
>> +adb_device_kbd_request_change_addr_and_handler(int devaddr, int handler) 
>> "change addr and handler to 0x%x, 0x%x"
>>  
>>  # adb-mouse.c
>> -adb_mouse_flush(void) "flush"
>> -adb_mouse_writereg(int reg, uint8_t val) "reg %d val 0x%2.2x"
>> -adb_mouse_readreg(int reg, uint8_t val0, uint8_t val1) "reg %d obuf[0] 
>> 0x%2.2x obuf[1] 0x%2.2x"
>> -adb_mouse_request_change_addr(int devaddr) "change addr to 0x%x"
>> -adb_mouse_request_change_addr_and_handler(int devaddr, int handler) "change 
>> addr and handler to 0x%x, 0x%x"
>> +adb_device_mouse_flush(void) "flush"
> 
> For the following: ...
> 
>> +adb_device_mouse_writereg(int reg, uint8_t val) "reg %d val 0x%2.2x"
>> +adb_device_mouse_readreg(int reg, uint8_t val0, uint8_t val1) "reg %d 
>> obuf[0] 0x%2.2x obuf[1] 0x%2.2x"
>> +adb_device_mouse_request_change_addr(int devaddr) "change addr to 0x%x"
>> +adb_device_mouse_request_change_addr_and_handler(int devaddr, int handler) 
>> "change addr and handler to 0x%x, 0x%x"
> 
> ... you could use a common trace, using a device_type argument, i.e.:
> 
> adb_device_writereg(const char *device_type, int reg, uint8_t val) "%s
> reg %d val 0x%2.2x"
> 
> But then you can not filter traces for a particular device.
> 
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Yeah that's right. Generally whilst working on this I've tended to want to see 
either
everything or individual devices, so I'll leave this one as it is for now. 
Thanks for
the review though!


ATB,

Mark.