[PATCH 5/6] hw/i2c/pm_smbus: Convert DPRINTF to trace events

Bernhard Beschow posted 6 patches 1 year, 1 month ago
Maintainers: Stefan Hajnoczi <stefanha@redhat.com>, Mads Ynddal <mads@ynddal.dk>, "Michael S. Tsirkin" <mst@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, Peter Maydell <peter.maydell@linaro.org>, Jean-Christophe Dubois <jcd@tribudubois.net>, Andrey Smirnov <andrew.smirnov@gmail.com>, Peter Xu <peterx@redhat.com>, David Hildenbrand <david@redhat.com>, "Philippe Mathieu-Daudé" <philmd@linaro.org>
[PATCH 5/6] hw/i2c/pm_smbus: Convert DPRINTF to trace events
Posted by Bernhard Beschow 1 year, 1 month ago
Let the trace messages slightly deviate from the function names
("smb" -> "smbus") being traced in order to avoid conflights with the SMB
protocol.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/i2c/pm_smbus.c   | 18 ++++--------------
 hw/i2c/trace-events |  6 ++++++
 2 files changed, 10 insertions(+), 14 deletions(-)

diff --git a/hw/i2c/pm_smbus.c b/hw/i2c/pm_smbus.c
index 4e1b8a5182..78e7c229a8 100644
--- a/hw/i2c/pm_smbus.c
+++ b/hw/i2c/pm_smbus.c
@@ -23,6 +23,7 @@
 #include "hw/i2c/pm_smbus.h"
 #include "hw/i2c/smbus_master.h"
 #include "migration/vmstate.h"
+#include "trace.h"
 
 #define SMBHSTSTS       0x00
 #define SMBHSTCNT       0x02
@@ -64,15 +65,6 @@
 #define AUX_BLK       (1 << 1)
 #define AUX_MASK      0x3
 
-/*#define DEBUG*/
-
-#ifdef DEBUG
-# define SMBUS_DPRINTF(format, ...)     printf(format, ## __VA_ARGS__)
-#else
-# define SMBUS_DPRINTF(format, ...)     do { } while (0)
-#endif
-
-
 static void smb_transaction(PMSMBus *s)
 {
     uint8_t prot = (s->smb_ctl >> 2) & 0x07;
@@ -82,7 +74,7 @@ static void smb_transaction(PMSMBus *s)
     I2CBus *bus = s->smbus;
     int ret;
 
-    SMBUS_DPRINTF("SMBus trans addr=0x%02x prot=0x%02x\n", addr, prot);
+    trace_smbus_transaction(addr, prot);
     /* Transaction isn't exec if STS_DEV_ERR bit set */
     if ((s->smb_stat & STS_DEV_ERR) != 0)  {
         goto error;
@@ -258,8 +250,7 @@ static void smb_ioport_writeb(void *opaque, hwaddr addr, uint64_t val,
     PMSMBus *s = opaque;
     uint8_t clear_byte_done;
 
-    SMBUS_DPRINTF("SMB writeb port=0x%04" HWADDR_PRIx
-                  " val=0x%02" PRIx64 "\n", addr, val);
+    trace_smbus_ioport_writeb(addr, val);
     switch(addr) {
     case SMBHSTSTS:
         clear_byte_done = s->smb_stat & val & STS_BYTE_DONE;
@@ -429,8 +420,7 @@ static uint64_t smb_ioport_readb(void *opaque, hwaddr addr, unsigned width)
         val = 0;
         break;
     }
-    SMBUS_DPRINTF("SMB readb port=0x%04" HWADDR_PRIx " val=0x%02x\n",
-                  addr, val);
+    trace_smbus_ioport_readb(addr, val);
 
     if (s->set_irq) {
         s->set_irq(s, smb_irq_value(s));
diff --git a/hw/i2c/trace-events b/hw/i2c/trace-events
index d7b1e25858..6900e06eda 100644
--- a/hw/i2c/trace-events
+++ b/hw/i2c/trace-events
@@ -15,6 +15,12 @@ i2c_send_async(uint8_t address, uint8_t data) "send_async(addr:0x%02x) data:0x%0
 i2c_recv(uint8_t address, uint8_t data) "recv(addr:0x%02x) data:0x%02x"
 i2c_ack(void) ""
 
+# pm_smbus.c
+
+smbus_ioport_readb(uint16_t addr, uint8_t data) "[0x%04" PRIx16 "] -> val=0x%02x"
+smbus_ioport_writeb(uint16_t addr, uint8_t data) "[0x%04" PRIx16 "] <- val=0x%02x"
+smbus_transaction(uint8_t addr, uint8_t prot) "addr=0x%02x prot=0x%02x"
+
 # allwinner_i2c.c
 
 allwinner_i2c_read(const char* reg_name, uint64_t offset, uint64_t value) "read %s [0x%" PRIx64 "]: -> 0x%" PRIx64
-- 
2.42.0
Re: [PATCH 5/6] hw/i2c/pm_smbus: Convert DPRINTF to trace events
Posted by Corey Minyard 1 year ago
On Sat, Oct 28, 2023 at 02:24:14PM +0200, Bernhard Beschow wrote:
> Let the trace messages slightly deviate from the function names
> ("smb" -> "smbus") being traced in order to avoid conflights with the SMB
> protocol.
> 
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>

Looks good to me.

Acked-by: Corey Minyard <cminyard@mvista.com>

> ---
>  hw/i2c/pm_smbus.c   | 18 ++++--------------
>  hw/i2c/trace-events |  6 ++++++
>  2 files changed, 10 insertions(+), 14 deletions(-)
> 
> diff --git a/hw/i2c/pm_smbus.c b/hw/i2c/pm_smbus.c
> index 4e1b8a5182..78e7c229a8 100644
> --- a/hw/i2c/pm_smbus.c
> +++ b/hw/i2c/pm_smbus.c
> @@ -23,6 +23,7 @@
>  #include "hw/i2c/pm_smbus.h"
>  #include "hw/i2c/smbus_master.h"
>  #include "migration/vmstate.h"
> +#include "trace.h"
>  
>  #define SMBHSTSTS       0x00
>  #define SMBHSTCNT       0x02
> @@ -64,15 +65,6 @@
>  #define AUX_BLK       (1 << 1)
>  #define AUX_MASK      0x3
>  
> -/*#define DEBUG*/
> -
> -#ifdef DEBUG
> -# define SMBUS_DPRINTF(format, ...)     printf(format, ## __VA_ARGS__)
> -#else
> -# define SMBUS_DPRINTF(format, ...)     do { } while (0)
> -#endif
> -
> -
>  static void smb_transaction(PMSMBus *s)
>  {
>      uint8_t prot = (s->smb_ctl >> 2) & 0x07;
> @@ -82,7 +74,7 @@ static void smb_transaction(PMSMBus *s)
>      I2CBus *bus = s->smbus;
>      int ret;
>  
> -    SMBUS_DPRINTF("SMBus trans addr=0x%02x prot=0x%02x\n", addr, prot);
> +    trace_smbus_transaction(addr, prot);
>      /* Transaction isn't exec if STS_DEV_ERR bit set */
>      if ((s->smb_stat & STS_DEV_ERR) != 0)  {
>          goto error;
> @@ -258,8 +250,7 @@ static void smb_ioport_writeb(void *opaque, hwaddr addr, uint64_t val,
>      PMSMBus *s = opaque;
>      uint8_t clear_byte_done;
>  
> -    SMBUS_DPRINTF("SMB writeb port=0x%04" HWADDR_PRIx
> -                  " val=0x%02" PRIx64 "\n", addr, val);
> +    trace_smbus_ioport_writeb(addr, val);
>      switch(addr) {
>      case SMBHSTSTS:
>          clear_byte_done = s->smb_stat & val & STS_BYTE_DONE;
> @@ -429,8 +420,7 @@ static uint64_t smb_ioport_readb(void *opaque, hwaddr addr, unsigned width)
>          val = 0;
>          break;
>      }
> -    SMBUS_DPRINTF("SMB readb port=0x%04" HWADDR_PRIx " val=0x%02x\n",
> -                  addr, val);
> +    trace_smbus_ioport_readb(addr, val);
>  
>      if (s->set_irq) {
>          s->set_irq(s, smb_irq_value(s));
> diff --git a/hw/i2c/trace-events b/hw/i2c/trace-events
> index d7b1e25858..6900e06eda 100644
> --- a/hw/i2c/trace-events
> +++ b/hw/i2c/trace-events
> @@ -15,6 +15,12 @@ i2c_send_async(uint8_t address, uint8_t data) "send_async(addr:0x%02x) data:0x%0
>  i2c_recv(uint8_t address, uint8_t data) "recv(addr:0x%02x) data:0x%02x"
>  i2c_ack(void) ""
>  
> +# pm_smbus.c
> +
> +smbus_ioport_readb(uint16_t addr, uint8_t data) "[0x%04" PRIx16 "] -> val=0x%02x"
> +smbus_ioport_writeb(uint16_t addr, uint8_t data) "[0x%04" PRIx16 "] <- val=0x%02x"
> +smbus_transaction(uint8_t addr, uint8_t prot) "addr=0x%02x prot=0x%02x"
> +
>  # allwinner_i2c.c
>  
>  allwinner_i2c_read(const char* reg_name, uint64_t offset, uint64_t value) "read %s [0x%" PRIx64 "]: -> 0x%" PRIx64
> -- 
> 2.42.0
> 
>
Re: [PATCH 5/6] hw/i2c/pm_smbus: Convert DPRINTF to trace events
Posted by Philippe Mathieu-Daudé 1 year ago
On 28/10/23 14:24, Bernhard Beschow wrote:
> Let the trace messages slightly deviate from the function names
> ("smb" -> "smbus") being traced in order to avoid conflights with the SMB
> protocol.
> 
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> ---
>   hw/i2c/pm_smbus.c   | 18 ++++--------------
>   hw/i2c/trace-events |  6 ++++++
>   2 files changed, 10 insertions(+), 14 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>