[Qemu-devel] [PATCH 0/8] i.MX: Standardize debug code

[Qemu-devel] [PATCH 0/8] i.MX: Standardize debug code

[Jean-Christophe Dubois]

We fix all i.MX driver files to use the same type of debug code.

The goal is to have debug code always compiled during build.

Jean-Christophe Dubois (8):
  i.MX: Standardize i.MX serial debug.
  i.MX: Standardize i.MX GPIO debug
  i.MX: Standardize i.MX I2C debug
  i.MX: Standardize i.MX AVIC debug
  i.MX: Standardize i.MX CCM debug
  i.MX: Standardize i.MX FEC debug
  i.MX: Standardize i.MX EPIT debug
  i.MX: Standardize i.MX GPT debug

 hw/char/imx_serial.c | 39 +++++++++++++++++----------------------
 hw/gpio/imx_gpio.c   |  3 ++-
 hw/i2c/imx_i2c.c     | 25 +++++++++++++------------
 hw/intc/imx_avic.c   | 40 ++++++++++++++++------------------------
 hw/misc/imx_ccm.c    | 30 ++++++++++++++++++++----------
 hw/net/imx_fec.c     | 38 ++++++++++++++++++--------------------
 hw/timer/imx_epit.c  | 37 +++++++++++++++----------------------
 hw/timer/imx_gpt.c   | 40 +++++++++++++++-------------------------
 8 files changed, 116 insertions(+), 136 deletions(-)

-- 
2.1.4

[Qemu-devel] [PATCH 1/8] i.MX: Standardize i.MX serial debug.

[Jean-Christophe Dubois]

The goal is to have debug code always compiled during build.

Signed-off-by: Jean-Christophe Dubois <jcd@tribudubois.net>
---
 hw/char/imx_serial.c | 39 +++++++++++++++++----------------------
 1 file changed, 17 insertions(+), 22 deletions(-)

diff --git a/hw/char/imx_serial.c b/hw/char/imx_serial.c
index f0c4c72..08274e0 100644
--- a/hw/char/imx_serial.c
+++ b/hw/char/imx_serial.c
@@ -22,25 +22,17 @@
 #include "sysemu/sysemu.h"
 #include "sysemu/char.h"
 
-//#define DEBUG_SERIAL 1
-#ifdef DEBUG_SERIAL
-#define DPRINTF(fmt, args...) \
-do { printf("%s: " fmt , TYPE_IMX_SERIAL, ##args); } while (0)
-#else
-#define DPRINTF(fmt, args...) do {} while (0)
+#ifndef DEBUG_IMX_UART
+#define DEBUG_IMX_UART 0
 #endif
 
-/*
- * Define to 1 for messages about attempts to
- * access unimplemented registers or similar.
- */
-//#define DEBUG_IMPLEMENTATION 1
-#ifdef DEBUG_IMPLEMENTATION
-#  define IPRINTF(fmt, args...) \
-    do  { fprintf(stderr, "%s: " fmt, TYPE_IMX_SERIAL, ##args); } while (0)
-#else
-#  define IPRINTF(fmt, args...) do {} while (0)
-#endif
+#define DPRINTF(fmt, args...) \
+          do { \
+              if (DEBUG_IMX_UART) { \
+                  fprintf(stderr, "[%s]%s: " fmt , TYPE_IMX_SERIAL, \
+                                                   __func__, ##args); \
+              } \
+          } while (0)
 
 static const VMStateDescription vmstate_imx_serial = {
     .name = TYPE_IMX_SERIAL,
@@ -115,7 +107,7 @@ static uint64_t imx_serial_read(void *opaque, hwaddr offset,
     IMXSerialState *s = (IMXSerialState *)opaque;
     uint32_t c;
 
-    DPRINTF("read(offset=%x)\n", offset >> 2);
+    DPRINTF("read(offset=%x)\n", (int)(offset >> 2));
     switch (offset >> 2) {
     case 0x0: /* URXD */
         c = s->readbuff;
@@ -167,7 +159,8 @@ static uint64_t imx_serial_read(void *opaque, hwaddr offset,
         return 0x0; /* TODO */
 
     default:
-        IPRINTF("%s: bad offset: 0x%x\n", __func__, (int)offset);
+	qemu_log_mask(LOG_GUEST_ERROR, "%s[%s]: Bad register at offset %d\n",
+                      TYPE_IMX_SERIAL, __func__, (int)offset);
         return 0;
     }
 }
@@ -179,7 +172,7 @@ static void imx_serial_write(void *opaque, hwaddr offset,
     unsigned char ch;
 
     DPRINTF("write(offset=%x, value = %x) to %s\n",
-            offset >> 2,
+            (int)(offset >> 2),
             (unsigned int)value, s->chr ? s->chr->label : "NODEV");
 
     switch (offset >> 2) {
@@ -266,12 +259,14 @@ static void imx_serial_write(void *opaque, hwaddr offset,
 
     case 0x2d: /* UTS1 */
     case 0x23: /* UCR4 */
-        IPRINTF("Unimplemented Register %x written to\n", offset >> 2);
+	qemu_log_mask(LOG_UNIMP, "%s[%s]: Unimplemented reg %d\n",
+                      TYPE_IMX_SERIAL, __func__, (int)(offset >> 2));
         /* TODO */
         break;
 
     default:
-        IPRINTF("%s: Bad offset 0x%x\n", __func__, (int)offset);
+	qemu_log_mask(LOG_GUEST_ERROR, "%s[%s]: Bad register at offset %d\n",
+                      TYPE_IMX_SERIAL, __func__, (int)offset);
     }
 }
 
-- 
2.1.4

[Qemu-devel] [PATCH 2/8] i.MX: Standardize i.MX GPIO debug

[Jean-Christophe Dubois]

Signed-off-by: Jean-Christophe Dubois <jcd@tribudubois.net>
---
 hw/gpio/imx_gpio.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/gpio/imx_gpio.c b/hw/gpio/imx_gpio.c
index d56ffcd..db6f22c 100644
--- a/hw/gpio/imx_gpio.c
+++ b/hw/gpio/imx_gpio.c
@@ -31,7 +31,8 @@ typedef enum IMXGPIOLevel {
 #define DPRINTF(fmt, args...) \
           do { \
               if (DEBUG_IMX_GPIO) { \
-                  fprintf(stderr, "%s: " fmt , __func__, ##args); \
+                  fprintf(stderr, "[%s]%s: " fmt , TYPE_IMX_GPIO, \
+                                                   __func__, ##args); \
               } \
           } while (0)
 
-- 
2.1.4

[Qemu-devel] [PATCH 3/8] i.MX: Standardize i.MX I2C debug

[Jean-Christophe Dubois]

The goal is to have debug code always compiled during build.

Signed-off-by: Jean-Christophe Dubois <jcd@tribudubois.net>
---
 hw/i2c/imx_i2c.c | 25 +++++++++++++------------
 1 file changed, 13 insertions(+), 12 deletions(-)

diff --git a/hw/i2c/imx_i2c.c b/hw/i2c/imx_i2c.c
index 8474872..8926f7a 100644
--- a/hw/i2c/imx_i2c.c
+++ b/hw/i2c/imx_i2c.c
@@ -21,13 +21,17 @@
 #include "hw/i2c/imx_i2c.h"
 #include "hw/i2c/i2c.h"
 
-#ifndef IMX_I2C_DEBUG
-#define IMX_I2C_DEBUG                 0
+#ifndef DEBUG_IMX_I2C
+#define DEBUG_IMX_I2C 0
 #endif
 
-#if IMX_I2C_DEBUG
-#define DPRINT(fmt, args...)              \
-    do { fprintf(stderr, "%s: "fmt, __func__, ## args); } while (0)
+#define DPRINTF(fmt, args...) \
+          do { \
+              if (DEBUG_IMX_I2C) { \
+                  fprintf(stderr, "[%s]%s: " fmt , TYPE_IMX_I2C, \
+                                                   __func__, ##args); \
+              } \
+          } while (0)
 
 static const char *imx_i2c_get_regname(unsigned offset)
 {
@@ -46,9 +50,6 @@ static const char *imx_i2c_get_regname(unsigned offset)
         return "[?]";
     }
 }
-#else
-#define DPRINT(fmt, args...)              do { } while (0)
-#endif
 
 static inline bool imx_i2c_is_enabled(IMXI2CState *s)
 {
@@ -154,8 +155,8 @@ static uint64_t imx_i2c_read(void *opaque, hwaddr offset,
         break;
     }
 
-    DPRINT("read %s [0x%02x] -> 0x%02x\n", imx_i2c_get_regname(offset),
-           (unsigned int)offset, value);
+    DPRINTF("read %s [0x%02x] -> 0x%02x\n", imx_i2c_get_regname(offset),
+            (unsigned int)offset, value);
 
     return (uint64_t)value;
 }
@@ -165,8 +166,8 @@ static void imx_i2c_write(void *opaque, hwaddr offset,
 {
     IMXI2CState *s = IMX_I2C(opaque);
 
-    DPRINT("write %s [0x%02x] <- 0x%02x\n", imx_i2c_get_regname(offset),
-           (unsigned int)offset, (int)value);
+    DPRINTF("write %s [0x%02x] <- 0x%02x\n", imx_i2c_get_regname(offset),
+            (unsigned int)offset, (int)value);
 
     value &= 0xff;
 
-- 
2.1.4

[Qemu-devel] [PATCH 4/8] i.MX: Standardize i.MX AVIC debug

[Jean-Christophe Dubois]

The goal is to have debug code always compiled during build.

Signed-off-by: Jean-Christophe Dubois <jcd@tribudubois.net>
---
 hw/intc/imx_avic.c | 40 ++++++++++++++++------------------------
 1 file changed, 16 insertions(+), 24 deletions(-)

diff --git a/hw/intc/imx_avic.c b/hw/intc/imx_avic.c
index 96c376b..11960aa 100644
--- a/hw/intc/imx_avic.c
+++ b/hw/intc/imx_avic.c
@@ -17,27 +17,17 @@
 
 #include "hw/intc/imx_avic.h"
 
-#define DEBUG_INT 1
-#undef DEBUG_INT /* comment out for debugging */
-
-#ifdef DEBUG_INT
-#define DPRINTF(fmt, args...) \
-do { printf("%s: " fmt , TYPE_IMX_AVIC, ##args); } while (0)
-#else
-#define DPRINTF(fmt, args...) do {} while (0)
+#ifndef DEBUG_IMX_AVIC
+#define DEBUG_IMX_AVIC 0
 #endif
 
-/*
- * Define to 1 for messages about attempts to
- * access unimplemented registers or similar.
- */
-#define DEBUG_IMPLEMENTATION 1
-#if DEBUG_IMPLEMENTATION
-#  define IPRINTF(fmt, args...) \
-    do  { fprintf(stderr, "%s: " fmt, TYPE_IMX_AVIC, ##args); } while (0)
-#else
-#  define IPRINTF(fmt, args...) do {} while (0)
-#endif
+#define DPRINTF(fmt, args...) \
+          do { \
+              if (DEBUG_IMX_AVIC) { \
+                  fprintf(stderr, "[%s]%s: " fmt , TYPE_IMX_AVIC, \
+                                                   __func__, ##args); \
+              } \
+          } while (0)
 
 static const VMStateDescription vmstate_imx_avic = {
     .name = TYPE_IMX_AVIC,
@@ -115,8 +105,8 @@ static uint64_t imx_avic_read(void *opaque,
 {
     IMXAVICState *s = (IMXAVICState *)opaque;
 
+    DPRINTF("read(offset = 0x%x)\n", (int)(offset >> 2));
 
-    DPRINTF("read(offset = 0x%x)\n", offset >> 2);
     switch (offset >> 2) {
     case 0: /* INTCNTL */
         return s->intcntl;
@@ -213,7 +203,8 @@ static uint64_t imx_avic_read(void *opaque,
         return 0x4;
 
     default:
-        IPRINTF("%s: Bad offset 0x%x\n", __func__, (int)offset);
+        qemu_log_mask(LOG_GUEST_ERROR, "%s[%s]: Bad register at offset %d\n",
+                      TYPE_IMX_AVIC, __func__, (int)offset);
         return 0;
     }
 }
@@ -225,8 +216,8 @@ static void imx_avic_write(void *opaque, hwaddr offset,
 
     /* Vector Registers not yet supported */
     if (offset >= 0x100 && offset <= 0x2fc) {
-        IPRINTF("%s to vector register %d ignored\n", __func__,
-                (unsigned int)((offset - 0x100) >> 2));
+        qemu_log_mask(LOG_UNIMP, "%s[%s]: vector %d ignored\n",
+                      TYPE_IMX_AVIC, __func__, (int)((offset - 0x100) >> 2));
         return;
     }
 
@@ -305,7 +296,8 @@ static void imx_avic_write(void *opaque, hwaddr offset,
         return;
 
     default:
-        IPRINTF("%s: Bad offset %x\n", __func__, (int)offset);
+        qemu_log_mask(LOG_GUEST_ERROR, "%s[%s]: Bad register at offset %d\n",
+                      TYPE_IMX_AVIC, __func__, (int)offset);
     }
     imx_avic_update(s);
 }
-- 
2.1.4

[Qemu-devel] [PATCH 5/8] i.MX: Standardize i.MX CCM debug

[Jean-Christophe Dubois]

The goal is to have debug code always compiled during build.

Signed-off-by: Jean-Christophe Dubois <jcd@tribudubois.net>
---
 hw/misc/imx_ccm.c | 30 ++++++++++++++++++++----------
 1 file changed, 20 insertions(+), 10 deletions(-)

diff --git a/hw/misc/imx_ccm.c b/hw/misc/imx_ccm.c
index 2e19dbb..f137de1 100644
--- a/hw/misc/imx_ccm.c
+++ b/hw/misc/imx_ccm.c
@@ -16,14 +16,18 @@
 #define CKIH_FREQ 26000000 /* 26MHz crystal input */
 #define CKIL_FREQ    32768 /* nominal 32khz clock */
 
-//#define DEBUG_CCM 1
-#ifdef DEBUG_CCM
-#define DPRINTF(fmt, args...) \
-do { printf("%s: " fmt , TYPE_IMX_CCM, ##args); } while (0)
-#else
-#define DPRINTF(fmt, args...) do {} while (0)
+#ifndef DEBUG_IMX_CCM
+#define DEBUG_IMX_CCM 0
 #endif
 
+#define DPRINTF(fmt, args...) \
+          do { \
+              if (DEBUG_IMX_CCM) { \
+                  fprintf(stderr, "[%s]%s: " fmt , TYPE_IMX_CCM, \
+                                                   __func__, ##args); \
+              } \
+          } while (0)
+
 static int imx_ccm_post_load(void *opaque, int version_id);
 
 static const VMStateDescription vmstate_imx_ccm = {
@@ -135,7 +139,8 @@ static uint64_t imx_ccm_read(void *opaque, hwaddr offset,
 {
     IMXCCMState *s = (IMXCCMState *)opaque;
 
-    DPRINTF("%s(offset=%x)", __func__, offset >> 2);
+    DPRINTF("%s(offset=%x)", __func__, (int)(offset >> 2));
+
     switch (offset >> 2) {
     case 0: /* CCMR */
         DPRINTF(" ccmr = 0x%x\n", s->ccmr);
@@ -166,9 +171,11 @@ static uint64_t imx_ccm_read(void *opaque, hwaddr offset,
     case 23:
         DPRINTF(" pcmr0 = 0x%x\n", s->pmcr0);
         return s->pmcr0;
+    default:
+        qemu_log_mask(LOG_GUEST_ERROR, "%s[%s]: Bad register at offset %d\n",
+                      TYPE_IMX_CCM, __func__, (int)offset);
+        return 0;
     }
-    DPRINTF(" return 0\n");
-    return 0;
 }
 
 static void imx_ccm_write(void *opaque, hwaddr offset,
@@ -177,7 +184,8 @@ static void imx_ccm_write(void *opaque, hwaddr offset,
     IMXCCMState *s = (IMXCCMState *)opaque;
 
     DPRINTF("%s(offset=%x, value = %x)\n", __func__,
-            offset >> 2, (unsigned int)value);
+            (int)(offset >> 2), (unsigned int)value);
+
     switch (offset >> 2) {
     case 0:
         s->ccmr = CCMR_FPMF | (value & 0x3b6fdfff);
@@ -205,6 +213,8 @@ static void imx_ccm_write(void *opaque, hwaddr offset,
         return;
 
     default:
+        qemu_log_mask(LOG_GUEST_ERROR, "%s[%s]: Bad register at offset %d\n",
+                      TYPE_IMX_CCM, __func__, (int)offset);
         return;
     }
     update_clocks(s);
-- 
2.1.4

[Qemu-devel] [PATCH 6/8] i.MX: Standardize i.MX FEC debug

[Jean-Christophe Dubois]

The goal is to have debug code always compiled during build.

Signed-off-by: Jean-Christophe Dubois <jcd@tribudubois.net>
---
 hw/net/imx_fec.c | 38 ++++++++++++++++++--------------------
 1 file changed, 18 insertions(+), 20 deletions(-)

diff --git a/hw/net/imx_fec.c b/hw/net/imx_fec.c
index 725f3fa..b39fc8d 100644
--- a/hw/net/imx_fec.c
+++ b/hw/net/imx_fec.c
@@ -27,31 +27,29 @@
 /* For crc32 */
 #include <zlib.h>
 
-#ifndef IMX_FEC_DEBUG
-#define IMX_FEC_DEBUG          0
+#ifndef DEBUG_IMX_FEC
+#define DEBUG_IMX_FEC 0
 #endif
 
-#ifndef IMX_PHY_DEBUG
-#define IMX_PHY_DEBUG          0
-#endif
+#define FEC_PRINTF(fmt, args...) \
+          do { \
+              if (DEBUG_IMX_FEC) { \
+                  fprintf(stderr, "[%s]%s: " fmt , TYPE_IMX_FEC, \
+                                                   __func__, ##args); \
+              } \
+          } while (0)
 
-#if IMX_FEC_DEBUG
-#define FEC_PRINTF(fmt, ...) \
-    do { fprintf(stderr, "%s[%s]: " fmt , TYPE_IMX_FEC, __func__, \
-                 ## __VA_ARGS__); \
-    } while (0)
-#else
-#define FEC_PRINTF(fmt, ...) do {} while (0)
+#ifndef DEBUG_IMX_PHY
+#define DEBUG_IMX_PHY 0
 #endif
 
-#if IMX_PHY_DEBUG
-#define PHY_PRINTF(fmt, ...) \
-    do { fprintf(stderr, "%s.phy[%s]: " fmt , TYPE_IMX_FEC, __func__, \
-                 ## __VA_ARGS__); \
-    } while (0)
-#else
-#define PHY_PRINTF(fmt, ...) do {} while (0)
-#endif
+#define PHY_PRINTF(fmt, args...) \
+          do { \
+              if (DEBUG_IMX_PHY) { \
+                  fprintf(stderr, "[%s.phy]%s: " fmt , TYPE_IMX_FEC, \
+                                                       __func__, ##args); \
+              } \
+          } while (0)
 
 static const VMStateDescription vmstate_imx_fec = {
     .name = TYPE_IMX_FEC,
-- 
2.1.4

[Qemu-devel] [PATCH 7/8] i.MX: Standardize i.MX EPIT debug

[Jean-Christophe Dubois]

The goal is to have debug code always compiled during build.

Signed-off-by: Jean-Christophe Dubois <jcd@tribudubois.net>
---
 hw/timer/imx_epit.c | 37 +++++++++++++++----------------------
 1 file changed, 15 insertions(+), 22 deletions(-)

diff --git a/hw/timer/imx_epit.c b/hw/timer/imx_epit.c
index 9649851..82792b1 100644
--- a/hw/timer/imx_epit.c
+++ b/hw/timer/imx_epit.c
@@ -16,8 +16,17 @@
 #include "hw/misc/imx_ccm.h"
 #include "qemu/main-loop.h"
 
-#define DEBUG_TIMER 0
-#if DEBUG_TIMER
+#ifndef DEBUG_IMX_EPIT
+#define DEBUG_IMX_EPIT 0
+#endif
+
+#define DPRINTF(fmt, args...) \
+          do { \
+              if (DEBUG_IMX_EPIT) { \
+                  fprintf(stderr, "[%s]%s: " fmt , TYPE_IMX_EPIT, \
+                                                   __func__, ##args); \
+              } \
+          } while (0)
 
 static char const *imx_epit_reg_name(uint32_t reg)
 {
@@ -37,24 +46,6 @@ static char const *imx_epit_reg_name(uint32_t reg)
     }
 }
 
-#  define DPRINTF(fmt, args...) \
-    do { fprintf(stderr, "%s: " fmt , __func__, ##args); } while (0)
-#else
-#  define DPRINTF(fmt, args...) do {} while (0)
-#endif
-
-/*
- * Define to 1 for messages about attempts to
- * access unimplemented registers or similar.
- */
-#define DEBUG_IMPLEMENTATION 1
-#if DEBUG_IMPLEMENTATION
-#  define IPRINTF(fmt, args...) \
-          do { fprintf(stderr, "%s: " fmt, __func__, ##args); } while (0)
-#else
-#  define IPRINTF(fmt, args...) do {} while (0)
-#endif
-
 /*
  * Exact clock frequencies vary from board to board.
  * These are typical.
@@ -161,7 +152,8 @@ static uint64_t imx_epit_read(void *opaque, hwaddr offset, unsigned size)
         break;
 
     default:
-        IPRINTF("Bad offset %x\n", reg);
+        qemu_log_mask(LOG_GUEST_ERROR, "%s[%s]: Bad register at offset %d\n",
+                      TYPE_IMX_EPIT, __func__, reg);
         break;
     }
 
@@ -271,7 +263,8 @@ static void imx_epit_write(void *opaque, hwaddr offset, uint64_t value,
         break;
 
     default:
-        IPRINTF("Bad offset %x\n", reg);
+        qemu_log_mask(LOG_GUEST_ERROR, "%s[%s]: Bad register at offset %d\n",
+                      TYPE_IMX_EPIT, __func__, reg);
 
         break;
     }
-- 
2.1.4

[Qemu-devel] [PATCH 8/8] i.MX: Standardize i.MX GPT debug

[Jean-Christophe Dubois]

The goal is to have debug code always compiled during build.

Signed-off-by: Jean-Christophe Dubois <jcd@tribudubois.net>
---
 hw/timer/imx_gpt.c | 40 +++++++++++++++-------------------------
 1 file changed, 15 insertions(+), 25 deletions(-)

diff --git a/hw/timer/imx_gpt.c b/hw/timer/imx_gpt.c
index 4bac67d..b217d86 100644
--- a/hw/timer/imx_gpt.c
+++ b/hw/timer/imx_gpt.c
@@ -16,11 +16,17 @@
 #include "hw/misc/imx_ccm.h"
 #include "qemu/main-loop.h"
 
-/*
- * Define to 1 for debug messages
- */
-#define DEBUG_TIMER 0
-#if DEBUG_TIMER
+#ifndef DEBUG_IMX_GPT
+#define DEBUG_IMX_GPT 0
+#endif
+
+#define DPRINTF(fmt, args...) \
+          do { \
+              if (DEBUG_IMX_GPT) { \
+                  fprintf(stderr, "[%s]%s: " fmt , TYPE_IMX_GPT, \
+                                                   __func__, ##args); \
+              } \
+          } while (0)
 
 static char const *imx_gpt_reg_name(uint32_t reg)
 {
@@ -50,24 +56,6 @@ static char const *imx_gpt_reg_name(uint32_t reg)
     }
 }
 
-#  define DPRINTF(fmt, args...) \
-          do { printf("%s: " fmt , __func__, ##args); } while (0)
-#else
-#  define DPRINTF(fmt, args...) do {} while (0)
-#endif
-
-/*
- * Define to 1 for messages about attempts to
- * access unimplemented registers or similar.
- */
-#define DEBUG_IMPLEMENTATION 1
-#if DEBUG_IMPLEMENTATION
-#  define IPRINTF(fmt, args...) \
-          do { fprintf(stderr, "%s: " fmt, __func__, ##args); } while (0)
-#else
-#  define IPRINTF(fmt, args...) do {} while (0)
-#endif
-
 static const VMStateDescription vmstate_imx_timer_gpt = {
     .name = TYPE_IMX_GPT,
     .version_id = 3,
@@ -271,7 +259,8 @@ static uint64_t imx_gpt_read(void *opaque, hwaddr offset, unsigned size)
         break;
 
     default:
-        IPRINTF("Bad offset %x\n", reg);
+        qemu_log_mask(LOG_GUEST_ERROR, "%s[%s]: Bad register at offset %d\n",
+                      TYPE_IMX_GPT, __func__, (int)reg);
         break;
     }
 
@@ -403,7 +392,8 @@ static void imx_gpt_write(void *opaque, hwaddr offset, uint64_t value,
         break;
 
     default:
-        IPRINTF("Bad offset %x\n", reg);
+        qemu_log_mask(LOG_GUEST_ERROR, "%s[%s]: Bad register at offset %d\n",
+                      TYPE_IMX_GPT, __func__, (int)reg);
         break;
     }
 }
-- 
2.1.4

Re: [Qemu-devel] [PATCH 1/8] i.MX: Standardize i.MX serial debug.

[Peter Crosthwaite]

On Tue, Oct 20, 2015 at 2:19 PM, Jean-Christophe Dubois
<jcd@tribudubois.net> wrote:
> The goal is to have debug code always compiled during build.
>

Elaborate message to indicate that you convert some messages to
LOG_GUEST_ERROR and LOG_UNIMP as appropriate, obsoleting the need for
IPRINTF.

> Signed-off-by: Jean-Christophe Dubois <jcd@tribudubois.net>
> ---
>  hw/char/imx_serial.c | 39 +++++++++++++++++----------------------
>  1 file changed, 17 insertions(+), 22 deletions(-)
>
> diff --git a/hw/char/imx_serial.c b/hw/char/imx_serial.c
> index f0c4c72..08274e0 100644
> --- a/hw/char/imx_serial.c
> +++ b/hw/char/imx_serial.c
> @@ -22,25 +22,17 @@
>  #include "sysemu/sysemu.h"
>  #include "sysemu/char.h"
>
> -//#define DEBUG_SERIAL 1
> -#ifdef DEBUG_SERIAL
> -#define DPRINTF(fmt, args...) \
> -do { printf("%s: " fmt , TYPE_IMX_SERIAL, ##args); } while (0)
> -#else
> -#define DPRINTF(fmt, args...) do {} while (0)
> +#ifndef DEBUG_IMX_UART
> +#define DEBUG_IMX_UART 0
>  #endif
>
> -/*
> - * Define to 1 for messages about attempts to
> - * access unimplemented registers or similar.
> - */
> -//#define DEBUG_IMPLEMENTATION 1
> -#ifdef DEBUG_IMPLEMENTATION
> -#  define IPRINTF(fmt, args...) \
> -    do  { fprintf(stderr, "%s: " fmt, TYPE_IMX_SERIAL, ##args); } while (0)
> -#else
> -#  define IPRINTF(fmt, args...) do {} while (0)
> -#endif
> +#define DPRINTF(fmt, args...) \
> +          do { \
> +              if (DEBUG_IMX_UART) { \
> +                  fprintf(stderr, "[%s]%s: " fmt , TYPE_IMX_SERIAL, \
> +                                                   __func__, ##args); \
> +              } \
> +          } while (0)
>
>  static const VMStateDescription vmstate_imx_serial = {
>      .name = TYPE_IMX_SERIAL,
> @@ -115,7 +107,7 @@ static uint64_t imx_serial_read(void *opaque, hwaddr offset,
>      IMXSerialState *s = (IMXSerialState *)opaque;
>      uint32_t c;
>
> -    DPRINTF("read(offset=%x)\n", offset >> 2);
> +    DPRINTF("read(offset=%x)\n", (int)(offset >> 2));

rather than do argument casts, you case use HWADDR_PRIx to get the %
format correct for the target type.

Regards,
Peter

>      switch (offset >> 2) {
>      case 0x0: /* URXD */
>          c = s->readbuff;
> @@ -167,7 +159,8 @@ static uint64_t imx_serial_read(void *opaque, hwaddr offset,
>          return 0x0; /* TODO */
>
>      default:
> -        IPRINTF("%s: bad offset: 0x%x\n", __func__, (int)offset);
> +       qemu_log_mask(LOG_GUEST_ERROR, "%s[%s]: Bad register at offset %d\n",
> +                      TYPE_IMX_SERIAL, __func__, (int)offset);
>          return 0;
>      }
>  }
> @@ -179,7 +172,7 @@ static void imx_serial_write(void *opaque, hwaddr offset,
>      unsigned char ch;
>
>      DPRINTF("write(offset=%x, value = %x) to %s\n",
> -            offset >> 2,
> +            (int)(offset >> 2),
>              (unsigned int)value, s->chr ? s->chr->label : "NODEV");
>
>      switch (offset >> 2) {
> @@ -266,12 +259,14 @@ static void imx_serial_write(void *opaque, hwaddr offset,
>
>      case 0x2d: /* UTS1 */
>      case 0x23: /* UCR4 */
> -        IPRINTF("Unimplemented Register %x written to\n", offset >> 2);
> +       qemu_log_mask(LOG_UNIMP, "%s[%s]: Unimplemented reg %d\n",
> +                      TYPE_IMX_SERIAL, __func__, (int)(offset >> 2));
>          /* TODO */
>          break;
>
>      default:
> -        IPRINTF("%s: Bad offset 0x%x\n", __func__, (int)offset);
> +       qemu_log_mask(LOG_GUEST_ERROR, "%s[%s]: Bad register at offset %d\n",
> +                      TYPE_IMX_SERIAL, __func__, (int)offset);
>      }
>  }
>
> --
> 2.1.4
>

Re: [Qemu-devel] [PATCH 0/8] i.MX: Standardize debug code

[Peter Crosthwaite]

On Tue, Oct 20, 2015 at 2:19 PM, Jean-Christophe Dubois
<jcd@tribudubois.net> wrote:
> We fix all i.MX driver files to use the same type of debug code.
>
> The goal is to have debug code always compiled during build.
>

It goes beyond that, it that the logs are now going to be way easier
to read with a consistent formatting of verbose messages. I think this
is a good idea, and with a lack of precedent (that I know of) you may
be setting a tree-wide standard here. CC Markus who is into logs and
errors. Patch 2 sums up nicely the proposed format:

               if (DEBUG_IMX_GPIO) { \
-                  fprintf(stderr, "%s: " fmt , __func__, ##args); \
+                  fprintf(stderr, "[%s]%s: " fmt , TYPE_IMX_GPIO, \
+                                                   __func__, ##args); \

which is:

[QOM_TYPE_NAME]reporting_function: some printf type message

so general RFC on what other think of it. Are there any existing
in-tree standards to factor in? I am OK in principle.

Regards,
Peter

> Jean-Christophe Dubois (8):
>   i.MX: Standardize i.MX serial debug.
>   i.MX: Standardize i.MX GPIO debug
>   i.MX: Standardize i.MX I2C debug
>   i.MX: Standardize i.MX AVIC debug
>   i.MX: Standardize i.MX CCM debug
>   i.MX: Standardize i.MX FEC debug
>   i.MX: Standardize i.MX EPIT debug
>   i.MX: Standardize i.MX GPT debug
>
>  hw/char/imx_serial.c | 39 +++++++++++++++++----------------------
>  hw/gpio/imx_gpio.c   |  3 ++-
>  hw/i2c/imx_i2c.c     | 25 +++++++++++++------------
>  hw/intc/imx_avic.c   | 40 ++++++++++++++++------------------------
>  hw/misc/imx_ccm.c    | 30 ++++++++++++++++++++----------
>  hw/net/imx_fec.c     | 38 ++++++++++++++++++--------------------
>  hw/timer/imx_epit.c  | 37 +++++++++++++++----------------------
>  hw/timer/imx_gpt.c   | 40 +++++++++++++++-------------------------
>  8 files changed, 116 insertions(+), 136 deletions(-)
>
> --
> 2.1.4
>

Re: [Qemu-devel] [PATCH 3/8] i.MX: Standardize i.MX I2C debug

[Peter Crosthwaite]

On Tue, Oct 20, 2015 at 2:19 PM, Jean-Christophe Dubois
<jcd@tribudubois.net> wrote:
> The goal is to have debug code always compiled during build.
>
> Signed-off-by: Jean-Christophe Dubois <jcd@tribudubois.net>
> ---
>  hw/i2c/imx_i2c.c | 25 +++++++++++++------------
>  1 file changed, 13 insertions(+), 12 deletions(-)
>
> diff --git a/hw/i2c/imx_i2c.c b/hw/i2c/imx_i2c.c
> index 8474872..8926f7a 100644
> --- a/hw/i2c/imx_i2c.c
> +++ b/hw/i2c/imx_i2c.c
> @@ -21,13 +21,17 @@
>  #include "hw/i2c/imx_i2c.h"
>  #include "hw/i2c/i2c.h"
>
> -#ifndef IMX_I2C_DEBUG
> -#define IMX_I2C_DEBUG                 0
> +#ifndef DEBUG_IMX_I2C
> +#define DEBUG_IMX_I2C 0
>  #endif
>
> -#if IMX_I2C_DEBUG
> -#define DPRINT(fmt, args...)              \
> -    do { fprintf(stderr, "%s: "fmt, __func__, ## args); } while (0)
> +#define DPRINTF(fmt, args...) \
> +          do { \
> +              if (DEBUG_IMX_I2C) { \
> +                  fprintf(stderr, "[%s]%s: " fmt , TYPE_IMX_I2C, \
> +                                                   __func__, ##args); \
> +              } \
> +          } while (0)
>
>  static const char *imx_i2c_get_regname(unsigned offset)
>  {
> @@ -46,9 +50,6 @@ static const char *imx_i2c_get_regname(unsigned offset)
>          return "[?]";
>      }
>  }
> -#else
> -#define DPRINT(fmt, args...)              do { } while (0)
> -#endif
>
>  static inline bool imx_i2c_is_enabled(IMXI2CState *s)
>  {
> @@ -154,8 +155,8 @@ static uint64_t imx_i2c_read(void *opaque, hwaddr offset,
>          break;
>      }
>
> -    DPRINT("read %s [0x%02x] -> 0x%02x\n", imx_i2c_get_regname(offset),
> -           (unsigned int)offset, value);
> +    DPRINTF("read %s [0x%02x] -> 0x%02x\n", imx_i2c_get_regname(offset),
> +            (unsigned int)offset, value);

Convert args to PRI while fixing. here and below.

Regards,
Peter

>
>      return (uint64_t)value;
>  }
> @@ -165,8 +166,8 @@ static void imx_i2c_write(void *opaque, hwaddr offset,
>  {
>      IMXI2CState *s = IMX_I2C(opaque);
>
> -    DPRINT("write %s [0x%02x] <- 0x%02x\n", imx_i2c_get_regname(offset),
> -           (unsigned int)offset, (int)value);
> +    DPRINTF("write %s [0x%02x] <- 0x%02x\n", imx_i2c_get_regname(offset),
> +            (unsigned int)offset, (int)value);
>
>      value &= 0xff;
>
> --
> 2.1.4
>

Re: [Qemu-devel] [PATCH 4/8] i.MX: Standardize i.MX AVIC debug

[Peter Crosthwaite]

On Tue, Oct 20, 2015 at 2:20 PM, Jean-Christophe Dubois
<jcd@tribudubois.net> wrote:
> The goal is to have debug code always compiled during build.
>
> Signed-off-by: Jean-Christophe Dubois <jcd@tribudubois.net>
> ---
>  hw/intc/imx_avic.c | 40 ++++++++++++++++------------------------
>  1 file changed, 16 insertions(+), 24 deletions(-)
>
> diff --git a/hw/intc/imx_avic.c b/hw/intc/imx_avic.c
> index 96c376b..11960aa 100644
> --- a/hw/intc/imx_avic.c
> +++ b/hw/intc/imx_avic.c
> @@ -17,27 +17,17 @@
>
>  #include "hw/intc/imx_avic.h"
>
> -#define DEBUG_INT 1
> -#undef DEBUG_INT /* comment out for debugging */
> -
> -#ifdef DEBUG_INT
> -#define DPRINTF(fmt, args...) \
> -do { printf("%s: " fmt , TYPE_IMX_AVIC, ##args); } while (0)
> -#else
> -#define DPRINTF(fmt, args...) do {} while (0)
> +#ifndef DEBUG_IMX_AVIC
> +#define DEBUG_IMX_AVIC 0
>  #endif
>
> -/*
> - * Define to 1 for messages about attempts to
> - * access unimplemented registers or similar.
> - */
> -#define DEBUG_IMPLEMENTATION 1
> -#if DEBUG_IMPLEMENTATION
> -#  define IPRINTF(fmt, args...) \
> -    do  { fprintf(stderr, "%s: " fmt, TYPE_IMX_AVIC, ##args); } while (0)
> -#else
> -#  define IPRINTF(fmt, args...) do {} while (0)
> -#endif
> +#define DPRINTF(fmt, args...) \
> +          do { \
> +              if (DEBUG_IMX_AVIC) { \
> +                  fprintf(stderr, "[%s]%s: " fmt , TYPE_IMX_AVIC, \
> +                                                   __func__, ##args); \
> +              } \
> +          } while (0)
>
>  static const VMStateDescription vmstate_imx_avic = {
>      .name = TYPE_IMX_AVIC,
> @@ -115,8 +105,8 @@ static uint64_t imx_avic_read(void *opaque,
>  {
>      IMXAVICState *s = (IMXAVICState *)opaque;
>
> +    DPRINTF("read(offset = 0x%x)\n", (int)(offset >> 2));
>
> -    DPRINTF("read(offset = 0x%x)\n", offset >> 2);
>      switch (offset >> 2) {
>      case 0: /* INTCNTL */
>          return s->intcntl;
> @@ -213,7 +203,8 @@ static uint64_t imx_avic_read(void *opaque,
>          return 0x4;
>
>      default:
> -        IPRINTF("%s: Bad offset 0x%x\n", __func__, (int)offset);
> +        qemu_log_mask(LOG_GUEST_ERROR, "%s[%s]: Bad register at offset %d\n",
> +                      TYPE_IMX_AVIC, __func__, (int)offset);
>          return 0;
>      }
>  }
> @@ -225,8 +216,8 @@ static void imx_avic_write(void *opaque, hwaddr offset,
>
>      /* Vector Registers not yet supported */
>      if (offset >= 0x100 && offset <= 0x2fc) {
> -        IPRINTF("%s to vector register %d ignored\n", __func__,
> -                (unsigned int)((offset - 0x100) >> 2));
> +        qemu_log_mask(LOG_UNIMP, "%s[%s]: vector %d ignored\n",
> +                      TYPE_IMX_AVIC, __func__, (int)((offset - 0x100) >> 2));
>          return;
>      }
>
> @@ -305,7 +296,8 @@ static void imx_avic_write(void *opaque, hwaddr offset,
>          return;
>
>      default:
> -        IPRINTF("%s: Bad offset %x\n", __func__, (int)offset);
> +        qemu_log_mask(LOG_GUEST_ERROR, "%s[%s]: Bad register at offset %d\n",
> +                      TYPE_IMX_AVIC, __func__, (int)offset);
>      }
>      imx_avic_update(s);

Same comments as P1.

Regards,
Peter

>  }
> --
> 2.1.4
>

Re: [Qemu-devel] [PATCH 8/8] i.MX: Standardize i.MX GPT debug

[Peter Crosthwaite]

On Tue, Oct 20, 2015 at 2:20 PM, Jean-Christophe Dubois
<jcd@tribudubois.net> wrote:
> The goal is to have debug code always compiled during build.
>
> Signed-off-by: Jean-Christophe Dubois <jcd@tribudubois.net>
> ---
>  hw/timer/imx_gpt.c | 40 +++++++++++++++-------------------------
>  1 file changed, 15 insertions(+), 25 deletions(-)
>
> diff --git a/hw/timer/imx_gpt.c b/hw/timer/imx_gpt.c
> index 4bac67d..b217d86 100644
> --- a/hw/timer/imx_gpt.c
> +++ b/hw/timer/imx_gpt.c
> @@ -16,11 +16,17 @@
>  #include "hw/misc/imx_ccm.h"
>  #include "qemu/main-loop.h"
>
> -/*
> - * Define to 1 for debug messages
> - */
> -#define DEBUG_TIMER 0
> -#if DEBUG_TIMER
> +#ifndef DEBUG_IMX_GPT
> +#define DEBUG_IMX_GPT 0
> +#endif
> +
> +#define DPRINTF(fmt, args...) \
> +          do { \
> +              if (DEBUG_IMX_GPT) { \
> +                  fprintf(stderr, "[%s]%s: " fmt , TYPE_IMX_GPT, \
> +                                                   __func__, ##args); \
> +              } \
> +          } while (0)
>
>  static char const *imx_gpt_reg_name(uint32_t reg)
>  {
> @@ -50,24 +56,6 @@ static char const *imx_gpt_reg_name(uint32_t reg)
>      }
>  }
>
> -#  define DPRINTF(fmt, args...) \
> -          do { printf("%s: " fmt , __func__, ##args); } while (0)
> -#else
> -#  define DPRINTF(fmt, args...) do {} while (0)
> -#endif
> -
> -/*
> - * Define to 1 for messages about attempts to
> - * access unimplemented registers or similar.
> - */
> -#define DEBUG_IMPLEMENTATION 1
> -#if DEBUG_IMPLEMENTATION
> -#  define IPRINTF(fmt, args...) \
> -          do { fprintf(stderr, "%s: " fmt, __func__, ##args); } while (0)
> -#else
> -#  define IPRINTF(fmt, args...) do {} while (0)
> -#endif
> -
>  static const VMStateDescription vmstate_imx_timer_gpt = {
>      .name = TYPE_IMX_GPT,
>      .version_id = 3,
> @@ -271,7 +259,8 @@ static uint64_t imx_gpt_read(void *opaque, hwaddr offset, unsigned size)
>          break;
>
>      default:
> -        IPRINTF("Bad offset %x\n", reg);
> +        qemu_log_mask(LOG_GUEST_ERROR, "%s[%s]: Bad register at offset %d\n",
> +                      TYPE_IMX_GPT, __func__, (int)reg);
>          break;
>      }
>
> @@ -403,7 +392,8 @@ static void imx_gpt_write(void *opaque, hwaddr offset, uint64_t value,
>          break;
>
>      default:
> -        IPRINTF("Bad offset %x\n", reg);
> +        qemu_log_mask(LOG_GUEST_ERROR, "%s[%s]: Bad register at offset %d\n",
> +                      TYPE_IMX_GPT, __func__, (int)reg);
>          break;
>      }
>  }
> --
> 2.1.4
>

Remainder of series looks good too, just needs a general sweep for the
comments I made on P1, using PRIs when possible and expanding commit
messages to indicate the changes made.

Regards.
Peter