Re: [PATCH v2 11/13] hw/i2c/imx_i2c: Convert DPRINTF() to trace events

[Corey Minyard <corey@minyard.net>]

On Sat, Jan 11, 2025 at 07:37:09PM +0100, Bernhard Beschow wrote:
> Also print the QOM canonical path when tracing which allows for distinguishing
> the many instances a typical i.MX SoC has.
> 
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Tested-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>

This seems reasonable.

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

> ---
>  hw/i2c/imx_i2c.c    | 21 +++++----------------
>  hw/i2c/trace-events |  5 +++++
>  2 files changed, 10 insertions(+), 16 deletions(-)
> 
> diff --git a/hw/i2c/imx_i2c.c b/hw/i2c/imx_i2c.c
> index c565fd5b8a..d62213b9e0 100644
> --- a/hw/i2c/imx_i2c.c
> +++ b/hw/i2c/imx_i2c.c
> @@ -25,18 +25,7 @@
>  #include "hw/i2c/i2c.h"
>  #include "qemu/log.h"
>  #include "qemu/module.h"
> -
> -#ifndef DEBUG_IMX_I2C
> -#define DEBUG_IMX_I2C 0
> -#endif
> -
> -#define DPRINTF(fmt, args...) \
> -    do { \
> -        if (DEBUG_IMX_I2C) { \
> -            fprintf(stderr, "[%s]%s: " fmt , TYPE_IMX_I2C, \
> -                                             __func__, ##args); \
> -        } \
> -    } while (0)
> +#include "trace.h"
>  
>  static const char *imx_i2c_get_regname(unsigned offset)
>  {
> @@ -152,8 +141,8 @@ static uint64_t imx_i2c_read(void *opaque, hwaddr offset,
>          break;
>      }
>  
> -    DPRINTF("read %s [0x%" HWADDR_PRIx "] -> 0x%02x\n",
> -            imx_i2c_get_regname(offset), offset, value);
> +    trace_imx_i2c_read(DEVICE(s)->canonical_path, imx_i2c_get_regname(offset),
> +                       offset, value);
>  
>      return (uint64_t)value;
>  }
> @@ -163,8 +152,8 @@ static void imx_i2c_write(void *opaque, hwaddr offset,
>  {
>      IMXI2CState *s = IMX_I2C(opaque);
>  
> -    DPRINTF("write %s [0x%" HWADDR_PRIx "] <- 0x%02x\n",
> -            imx_i2c_get_regname(offset), offset, (int)value);
> +    trace_imx_i2c_read(DEVICE(s)->canonical_path, imx_i2c_get_regname(offset),
> +                       offset, value);
>  
>      value &= 0xff;
>  
> diff --git a/hw/i2c/trace-events b/hw/i2c/trace-events
> index f708a7ace1..1ad0e95c0e 100644
> --- a/hw/i2c/trace-events
> +++ b/hw/i2c/trace-events
> @@ -56,3 +56,8 @@ npcm7xx_smbus_recv_fifo(const char *id, uint8_t received, uint8_t expected) "%s
>  
>  pca954x_write_bytes(uint8_t value) "PCA954X write data: 0x%02x"
>  pca954x_read_data(uint8_t value) "PCA954X read data: 0x%02x"
> +
> +# imx_i2c.c
> +
> +imx_i2c_read(const char *id, const char *reg, uint64_t ofs, uint64_t value) "%s:[%s (0x%" PRIx64 ")] -> 0x%02" PRIx64
> +imx_i2c_write(const char *id, const char *reg, uint64_t ofs, uint64_t value) "%s:[%s (0x%" PRIx64 ")] <- 0x%02" PRIx64
> -- 
> 2.48.0
> 
>