Skip to contentSkip to conversation

i.MX: Standardize debug code

PATCH Superseded Jean-Christophe Dubois 2 participants 14 messages Oct 20, 2015 Arm i.MX and Layerscape

v1 of 4 Superseded Latest is v4 →

lore
Share & tools
lore
#0 lore raw

cover i.MX: Standardize debug code

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(-)
Signature · 1 line
2.1.4
#1 lore raw

PATCH 1/8 i.MX: Standardize i.MX serial debug.

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

---
 hw/char/imx_serial.c | 39 +++++++++++++++++----------------------
 1 file changed, 17 insertions(+), 22 deletions(-)
hw/char/imx_serial.c +17 -22
index f0c4c72..08274e0 100644
@@ -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);
     }
 }
 
Signature · 1 line
2.1.4
#2 lore raw

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

Quoted 2 lines · Jean-Christophe Dubois

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.

Quoted 45 lines
---
 hw/char/imx_serial.c | 39 +++++++++++++++++----------------------
 1 file changed, 17 insertions(+), 22 deletions(-)
hw/char/imx_serial.c +9 -17
index f0c4c72..08274e0 100644
@@ -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,

hw/char/imx_serial.c static uint64_t imx_serial_read(void *opaque, hwaddr offset, · line 110

Quoted 5 lines
hw/char/imx_serial.c +1 -1
@@ -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

Quoted 42 lines
hw/char/imx_serial.c +7 -5
     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

#3 lore raw

PATCH 2/8 i.MX: Standardize i.MX GPIO debug

In reply to Jean-Christophe Dubois
---
 hw/gpio/imx_gpio.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
hw/gpio/imx_gpio.c +2 -1
index d56ffcd..db6f22c 100644
@@ -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)
 
Signature · 1 line
2.1.4
#4 lore raw

PATCH 3/8 i.MX: Standardize i.MX I2C debug

In reply to Jean-Christophe Dubois

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

---
 hw/i2c/imx_i2c.c | 25 +++++++++++++------------
 1 file changed, 13 insertions(+), 12 deletions(-)
hw/i2c/imx_i2c.c +13 -12
index 8474872..8926f7a 100644
@@ -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;
 
Signature · 1 line
2.1.4
#5 lore raw

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

Quoted 45 lines · Jean-Christophe Dubois

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

---
 hw/i2c/imx_i2c.c | 25 +++++++++++++------------
 1 file changed, 13 insertions(+), 12 deletions(-)
hw/i2c/imx_i2c.c +9 -8
index 8474872..8926f7a 100644
@@ -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)
 {

hw/i2c/imx_i2c.c static uint64_t imx_i2c_read(void *opaque, hwaddr offset, · line 159

Quoted 7 lines
hw/i2c/imx_i2c.c +2 -2
@@ -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

Quoted 17 lines
hw/i2c/imx_i2c.c +2 -3
     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

#6 lore raw

PATCH 4/8 i.MX: Standardize i.MX AVIC debug

In reply to Jean-Christophe Dubois

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

---
 hw/intc/imx_avic.c | 40 ++++++++++++++++------------------------
 1 file changed, 16 insertions(+), 24 deletions(-)
hw/intc/imx_avic.c +16 -24
index 96c376b..11960aa 100644
@@ -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);
 }
Signature · 1 line
2.1.4
#7 lore raw

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

Quoted 80 lines · Jean-Christophe Dubois

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

---
 hw/intc/imx_avic.c | 40 ++++++++++++++++------------------------
 1 file changed, 16 insertions(+), 24 deletions(-)
hw/intc/imx_avic.c +14 -23
index 96c376b..11960aa 100644
@@ -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;
     }
 

hw/intc/imx_avic.c static void imx_avic_write(void *opaque, hwaddr offset, · line 302

Quoted 8 lines
hw/intc/imx_avic.c +2 -1
@@ -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

Quoted 4 lines
hw/intc/imx_avic.c +0 -1
 }
--

2.1.4

#8 lore raw

PATCH 5/8 i.MX: Standardize i.MX CCM debug

In reply to Jean-Christophe Dubois

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

---
 hw/misc/imx_ccm.c | 30 ++++++++++++++++++++----------
 1 file changed, 20 insertions(+), 10 deletions(-)
hw/misc/imx_ccm.c +20 -10
index 2e19dbb..f137de1 100644
@@ -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);
Signature · 1 line
2.1.4
#9 lore raw

PATCH 6/8 i.MX: Standardize i.MX FEC debug

In reply to Jean-Christophe Dubois

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

---
 hw/net/imx_fec.c | 38 ++++++++++++++++++--------------------
 1 file changed, 18 insertions(+), 20 deletions(-)
hw/net/imx_fec.c +18 -20
index 725f3fa..b39fc8d 100644
@@ -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,
Signature · 1 line
2.1.4
#10 lore raw

PATCH 7/8 i.MX: Standardize i.MX EPIT debug

In reply to Jean-Christophe Dubois

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

---
 hw/timer/imx_epit.c | 37 +++++++++++++++----------------------
 1 file changed, 15 insertions(+), 22 deletions(-)
hw/timer/imx_epit.c +15 -22
index 9649851..82792b1 100644
@@ -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;
     }
Signature · 1 line
2.1.4
#11 lore raw

PATCH 8/8 i.MX: Standardize i.MX GPT debug

In reply to Jean-Christophe Dubois

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

---
 hw/timer/imx_gpt.c | 40 +++++++++++++++-------------------------
 1 file changed, 15 insertions(+), 25 deletions(-)
hw/timer/imx_gpt.c +15 -25
index 4bac67d..b217d86 100644
@@ -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;
     }
 }
Signature · 1 line
2.1.4
#12 lore raw

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

Quoted 80 lines · Jean-Christophe Dubois

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

---
 hw/timer/imx_gpt.c | 40 +++++++++++++++-------------------------
 1 file changed, 15 insertions(+), 25 deletions(-)
hw/timer/imx_gpt.c +15 -26
index 4bac67d..b217d86 100644
@@ -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;
     }
 }
--
Quoted 2 lines

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

#13 lore raw
In reply to Jean-Christophe Dubois
Quoted 4 lines · 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.

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

Quoted 23 lines

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