[PATCH 07/11] hw/gpio/avr_gpio: Add tracing for reads and writes

Philippe Mathieu-Daudé posted 11 patches 4 years, 11 months ago
[PATCH 07/11] hw/gpio/avr_gpio: Add tracing for reads and writes
Posted by Philippe Mathieu-Daudé 4 years, 11 months ago
From: G S Niteesh Babu <niteesh.gs@gmail.com>

Added tracing for gpio read, write, and update output irq.

1) trace_avr_gpio_update_ouput_irq
2) trace_avr_gpio_read
3) trace_avr_gpio_write

Signed-off-by: G S Niteesh Babu <niteesh.gs@gmail.com>
Reviewed-by: Michael Rolnik <mrolnik@gmail.com>
Message-Id: <20210311135539.10206-3-niteesh.gs@gmail.com>
[PMD: Added port_name(), display port name in trace events]
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/gpio/avr_gpio.c   | 26 +++++++++++++++++++++-----
 hw/gpio/trace-events |  5 +++++
 2 files changed, 26 insertions(+), 5 deletions(-)

diff --git a/hw/gpio/avr_gpio.c b/hw/gpio/avr_gpio.c
index e4c7122e62c..29252d6ccfe 100644
--- a/hw/gpio/avr_gpio.c
+++ b/hw/gpio/avr_gpio.c
@@ -2,6 +2,7 @@
  * AVR processors GPIO registers emulation.
  *
  * Copyright (C) 2020 Heecheol Yang <heecheol.yang@outlook.com>
+ * Copyright (C) 2021 Niteesh Babu G S <niteesh.gs@gmail.com>
  *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU General Public License as
@@ -26,6 +27,12 @@
 #include "hw/gpio/avr_gpio.h"
 #include "hw/qdev-properties.h"
 #include "migration/vmstate.h"
+#include "trace.h"
+
+static char port_name(AVRGPIOState *s)
+{
+    return 'A' + s->id;
+}
 
 static void avr_gpio_reset(DeviceState *dev)
 {
@@ -47,32 +54,41 @@ static void avr_gpio_write_port(AVRGPIOState *s, uint64_t value)
 
         if (cur_ddr_pin_val && (cur_port_pin_val != new_port_pin_val)) {
             qemu_set_irq(s->out[pin], new_port_pin_val);
+            trace_avr_gpio_update_output_irq(port_name(s), pin, new_port_pin_val);
         }
     }
     s->reg.port = value & s->reg.ddr;
 }
 static uint64_t avr_gpio_read(void *opaque, hwaddr offset, unsigned int size)
 {
+    uint8_t val = 0;
     AVRGPIOState *s = (AVRGPIOState *)opaque;
     switch (offset) {
     case GPIO_PIN:
-        return s->reg.pin;
+        val = s->reg.pin;
+        break;
     case GPIO_DDR:
-        return s->reg.ddr;
+        val = s->reg.ddr;
+        break;
     case GPIO_PORT:
-        return s->reg.port;
+        val = s->reg.port;
+        break;
     default:
         g_assert_not_reached();
         break;
     }
-    return 0;
+
+    trace_avr_gpio_read(port_name(s), offset, val);
+    return val;
 }
 
 static void avr_gpio_write(void *opaque, hwaddr offset, uint64_t value,
                                 unsigned int size)
 {
     AVRGPIOState *s = (AVRGPIOState *)opaque;
-    value = value & 0xF;
+    value = value & 0xFF;
+
+    trace_avr_gpio_write(port_name(s), offset, value);
     switch (offset) {
     case GPIO_PIN:
         s->reg.pin = value;
diff --git a/hw/gpio/trace-events b/hw/gpio/trace-events
index 46ab9323bd0..640834597a8 100644
--- a/hw/gpio/trace-events
+++ b/hw/gpio/trace-events
@@ -18,3 +18,8 @@ sifive_gpio_read(uint64_t offset, uint64_t r) "offset 0x%" PRIx64 " value 0x%" P
 sifive_gpio_write(uint64_t offset, uint64_t value) "offset 0x%" PRIx64 " value 0x%" PRIx64
 sifive_gpio_set(int64_t line, int64_t value) "line %" PRIi64 " value %" PRIi64
 sifive_gpio_update_output_irq(int64_t line, int64_t value) "line %" PRIi64 " value %" PRIi64
+
+# avr_gpio.c
+avr_gpio_read(unsigned id, uint64_t offset, uint64_t r) "port %c offset 0x%" PRIx64 " value 0x%" PRIx64
+avr_gpio_write(unsigned id, uint64_t offset, uint64_t value) "port %c offset 0x%" PRIx64 " value 0x%" PRIx64
+avr_gpio_update_output_irq(unsigned id, int64_t line, int64_t value) "port %c pin %" PRIi64 " value %" PRIi64
-- 
2.26.2

Re: [PATCH 07/11] hw/gpio/avr_gpio: Add tracing for reads and writes
Posted by Niteesh G. S. 4 years, 11 months ago
Reviewed-by: Niteesh G S <niteesh.gs@gmail.com>

On Sat, Mar 13, 2021 at 10:25 PM Philippe Mathieu-Daudé <f4bug@amsat.org>
wrote:

> From: G S Niteesh Babu <niteesh.gs@gmail.com>
>
> Added tracing for gpio read, write, and update output irq.
>
> 1) trace_avr_gpio_update_ouput_irq
> 2) trace_avr_gpio_read
> 3) trace_avr_gpio_write
>
> Signed-off-by: G S Niteesh Babu <niteesh.gs@gmail.com>
> Reviewed-by: Michael Rolnik <mrolnik@gmail.com>
> Message-Id: <20210311135539.10206-3-niteesh.gs@gmail.com>
> [PMD: Added port_name(), display port name in trace events]
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  hw/gpio/avr_gpio.c   | 26 +++++++++++++++++++++-----
>  hw/gpio/trace-events |  5 +++++
>  2 files changed, 26 insertions(+), 5 deletions(-)
>
> diff --git a/hw/gpio/avr_gpio.c b/hw/gpio/avr_gpio.c
> index e4c7122e62c..29252d6ccfe 100644
> --- a/hw/gpio/avr_gpio.c
> +++ b/hw/gpio/avr_gpio.c
> @@ -2,6 +2,7 @@
>   * AVR processors GPIO registers emulation.
>   *
>   * Copyright (C) 2020 Heecheol Yang <heecheol.yang@outlook.com>
> + * Copyright (C) 2021 Niteesh Babu G S <niteesh.gs@gmail.com>
>   *
>   * This program is free software; you can redistribute it and/or
>   * modify it under the terms of the GNU General Public License as
> @@ -26,6 +27,12 @@
>  #include "hw/gpio/avr_gpio.h"
>  #include "hw/qdev-properties.h"
>  #include "migration/vmstate.h"
> +#include "trace.h"
> +
> +static char port_name(AVRGPIOState *s)
> +{
> +    return 'A' + s->id;
> +}
>
>  static void avr_gpio_reset(DeviceState *dev)
>  {
> @@ -47,32 +54,41 @@ static void avr_gpio_write_port(AVRGPIOState *s,
> uint64_t value)
>
>          if (cur_ddr_pin_val && (cur_port_pin_val != new_port_pin_val)) {
>              qemu_set_irq(s->out[pin], new_port_pin_val);
> +            trace_avr_gpio_update_output_irq(port_name(s), pin,
> new_port_pin_val);
>          }
>      }
>      s->reg.port = value & s->reg.ddr;
>  }
>  static uint64_t avr_gpio_read(void *opaque, hwaddr offset, unsigned int
> size)
>  {
> +    uint8_t val = 0;
>      AVRGPIOState *s = (AVRGPIOState *)opaque;
>      switch (offset) {
>      case GPIO_PIN:
> -        return s->reg.pin;
> +        val = s->reg.pin;
> +        break;
>      case GPIO_DDR:
> -        return s->reg.ddr;
> +        val = s->reg.ddr;
> +        break;
>      case GPIO_PORT:
> -        return s->reg.port;
> +        val = s->reg.port;
> +        break;
>      default:
>          g_assert_not_reached();
>          break;
>      }
> -    return 0;
> +
> +    trace_avr_gpio_read(port_name(s), offset, val);
> +    return val;
>  }
>
>  static void avr_gpio_write(void *opaque, hwaddr offset, uint64_t value,
>                                  unsigned int size)
>  {
>      AVRGPIOState *s = (AVRGPIOState *)opaque;
> -    value = value & 0xF;
> +    value = value & 0xFF;
> +
> +    trace_avr_gpio_write(port_name(s), offset, value);
>      switch (offset) {
>      case GPIO_PIN:
>          s->reg.pin = value;
> diff --git a/hw/gpio/trace-events b/hw/gpio/trace-events
> index 46ab9323bd0..640834597a8 100644
> --- a/hw/gpio/trace-events
> +++ b/hw/gpio/trace-events
> @@ -18,3 +18,8 @@ sifive_gpio_read(uint64_t offset, uint64_t r) "offset
> 0x%" PRIx64 " value 0x%" P
>  sifive_gpio_write(uint64_t offset, uint64_t value) "offset 0x%" PRIx64 "
> value 0x%" PRIx64
>  sifive_gpio_set(int64_t line, int64_t value) "line %" PRIi64 " value %"
> PRIi64
>  sifive_gpio_update_output_irq(int64_t line, int64_t value) "line %"
> PRIi64 " value %" PRIi64
> +
> +# avr_gpio.c
> +avr_gpio_read(unsigned id, uint64_t offset, uint64_t r) "port %c offset
> 0x%" PRIx64 " value 0x%" PRIx64
> +avr_gpio_write(unsigned id, uint64_t offset, uint64_t value) "port %c
> offset 0x%" PRIx64 " value 0x%" PRIx64
> +avr_gpio_update_output_irq(unsigned id, int64_t line, int64_t value)
> "port %c pin %" PRIi64 " value %" PRIi64
> --
> 2.26.2
>
>
Re: [PATCH 07/11] hw/gpio/avr_gpio: Add tracing for reads and writes
Posted by Niteesh G. S. 4 years, 10 months ago
Hii Phil,

A gentle reminder to push these patches.

Thanks,
Niteesh.

On Sat, Mar 13, 2021 at 10:51 PM Niteesh G. S. <niteesh.gs@gmail.com> wrote:

> Reviewed-by: Niteesh G S <niteesh.gs@gmail.com>
>
> On Sat, Mar 13, 2021 at 10:25 PM Philippe Mathieu-Daudé <f4bug@amsat.org>
> wrote:
>
>> From: G S Niteesh Babu <niteesh.gs@gmail.com>
>>
>> Added tracing for gpio read, write, and update output irq.
>>
>> 1) trace_avr_gpio_update_ouput_irq
>> 2) trace_avr_gpio_read
>> 3) trace_avr_gpio_write
>>
>> Signed-off-by: G S Niteesh Babu <niteesh.gs@gmail.com>
>> Reviewed-by: Michael Rolnik <mrolnik@gmail.com>
>> Message-Id: <20210311135539.10206-3-niteesh.gs@gmail.com>
>> [PMD: Added port_name(), display port name in trace events]
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>>  hw/gpio/avr_gpio.c   | 26 +++++++++++++++++++++-----
>>  hw/gpio/trace-events |  5 +++++
>>  2 files changed, 26 insertions(+), 5 deletions(-)
>>
>> diff --git a/hw/gpio/avr_gpio.c b/hw/gpio/avr_gpio.c
>> index e4c7122e62c..29252d6ccfe 100644
>> --- a/hw/gpio/avr_gpio.c
>> +++ b/hw/gpio/avr_gpio.c
>> @@ -2,6 +2,7 @@
>>   * AVR processors GPIO registers emulation.
>>   *
>>   * Copyright (C) 2020 Heecheol Yang <heecheol.yang@outlook.com>
>> + * Copyright (C) 2021 Niteesh Babu G S <niteesh.gs@gmail.com>
>>   *
>>   * This program is free software; you can redistribute it and/or
>>   * modify it under the terms of the GNU General Public License as
>> @@ -26,6 +27,12 @@
>>  #include "hw/gpio/avr_gpio.h"
>>  #include "hw/qdev-properties.h"
>>  #include "migration/vmstate.h"
>> +#include "trace.h"
>> +
>> +static char port_name(AVRGPIOState *s)
>> +{
>> +    return 'A' + s->id;
>> +}
>>
>>  static void avr_gpio_reset(DeviceState *dev)
>>  {
>> @@ -47,32 +54,41 @@ static void avr_gpio_write_port(AVRGPIOState *s,
>> uint64_t value)
>>
>>          if (cur_ddr_pin_val && (cur_port_pin_val != new_port_pin_val)) {
>>              qemu_set_irq(s->out[pin], new_port_pin_val);
>> +            trace_avr_gpio_update_output_irq(port_name(s), pin,
>> new_port_pin_val);
>>          }
>>      }
>>      s->reg.port = value & s->reg.ddr;
>>  }
>>  static uint64_t avr_gpio_read(void *opaque, hwaddr offset, unsigned int
>> size)
>>  {
>> +    uint8_t val = 0;
>>      AVRGPIOState *s = (AVRGPIOState *)opaque;
>>      switch (offset) {
>>      case GPIO_PIN:
>> -        return s->reg.pin;
>> +        val = s->reg.pin;
>> +        break;
>>      case GPIO_DDR:
>> -        return s->reg.ddr;
>> +        val = s->reg.ddr;
>> +        break;
>>      case GPIO_PORT:
>> -        return s->reg.port;
>> +        val = s->reg.port;
>> +        break;
>>      default:
>>          g_assert_not_reached();
>>          break;
>>      }
>> -    return 0;
>> +
>> +    trace_avr_gpio_read(port_name(s), offset, val);
>> +    return val;
>>  }
>>
>>  static void avr_gpio_write(void *opaque, hwaddr offset, uint64_t value,
>>                                  unsigned int size)
>>  {
>>      AVRGPIOState *s = (AVRGPIOState *)opaque;
>> -    value = value & 0xF;
>> +    value = value & 0xFF;
>> +
>> +    trace_avr_gpio_write(port_name(s), offset, value);
>>      switch (offset) {
>>      case GPIO_PIN:
>>          s->reg.pin = value;
>> diff --git a/hw/gpio/trace-events b/hw/gpio/trace-events
>> index 46ab9323bd0..640834597a8 100644
>> --- a/hw/gpio/trace-events
>> +++ b/hw/gpio/trace-events
>> @@ -18,3 +18,8 @@ sifive_gpio_read(uint64_t offset, uint64_t r) "offset
>> 0x%" PRIx64 " value 0x%" P
>>  sifive_gpio_write(uint64_t offset, uint64_t value) "offset 0x%" PRIx64 "
>> value 0x%" PRIx64
>>  sifive_gpio_set(int64_t line, int64_t value) "line %" PRIi64 " value %"
>> PRIi64
>>  sifive_gpio_update_output_irq(int64_t line, int64_t value) "line %"
>> PRIi64 " value %" PRIi64
>> +
>> +# avr_gpio.c
>> +avr_gpio_read(unsigned id, uint64_t offset, uint64_t r) "port %c offset
>> 0x%" PRIx64 " value 0x%" PRIx64
>> +avr_gpio_write(unsigned id, uint64_t offset, uint64_t value) "port %c
>> offset 0x%" PRIx64 " value 0x%" PRIx64
>> +avr_gpio_update_output_irq(unsigned id, int64_t line, int64_t value)
>> "port %c pin %" PRIi64 " value %" PRIi64
>> --
>> 2.26.2
>>
>>
Re: [PATCH 07/11] hw/gpio/avr_gpio: Add tracing for reads and writes
Posted by Philippe Mathieu-Daudé 4 years, 10 months ago
Hi Niteesh,

On 3/23/21 3:42 AM, Niteesh G. S. wrote:
> Hii Phil,
> 
> A gentle reminder to push these patches.

Unfortunately the series is blocked by a legal issue in a previous
patch:
https://www.mail-archive.com/qemu-devel@nongnu.org/msg791443.html

We are pending an update from Heecheol Yang.

Regards,

Phil.