1. Patchew
  2. QEMU
  3. [PATCH v2 00/10] Cadence GEM Fixes
  4. View patch

[PATCH v2 01/10] net: cadence_gem: Fix debug statements

Sai Pavan Boddu posted 10 patches 5 years, 9 months ago
Maintainers: Alistair Francis <alistair@alistair23.me>, "Edgar E. Iglesias" <edgar.iglesias@gmail.com>, Jason Wang <jasowang@redhat.com>, Peter Maydell <peter.maydell@linaro.org>
There is a newer version of this series
[PATCH v2 01/10] net: cadence_gem: Fix debug statements
Posted by Sai Pavan Boddu 5 years, 9 months ago 
Enabling debug breaks the build, Fix them and make debug statements
always compilable. Fix few statements to use sized integer casting.

Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
---
 hw/net/cadence_gem.c | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
index 22a0b1b..2f244eb 100644
--- a/hw/net/cadence_gem.c
+++ b/hw/net/cadence_gem.c
@@ -35,14 +35,13 @@
 #include "sysemu/dma.h"
 #include "net/checksum.h"
 
-#ifdef CADENCE_GEM_ERR_DEBUG
-#define DB_PRINT(...) do { \
-    fprintf(stderr,  ": %s: ", __func__); \
-    fprintf(stderr, ## __VA_ARGS__); \
-    } while (0)
-#else
-    #define DB_PRINT(...)
-#endif
+#define CADENCE_GEM_ERR_DEBUG 0
+#define DB_PRINT(...) do {\
+    if (CADENCE_GEM_ERR_DEBUG) {   \
+        qemu_log(": %s: ", __func__); \
+        qemu_log(__VA_ARGS__); \
+    } \
+} while (0)
 
 #define GEM_NWCTRL        (0x00000000/4) /* Network Control reg */
 #define GEM_NWCFG         (0x00000004/4) /* Network Config reg */
@@ -979,7 +978,8 @@ static ssize_t gem_receive(NetClientState *nc, const uint8_t *buf, size_t size)
         size += 4;
     }
 
-    DB_PRINT("config bufsize: %d packet size: %ld\n", rxbufsize, size);
+    DB_PRINT("config bufsize: %" PRIu64 " packet size: %" PRIu64 "\n",
+             (uint64_t) rxbufsize, (uint64_t) size);
 
     /* Find which queue we are targeting */
     q = get_queue_from_screen(s, rxbuf_ptr, rxbufsize);
@@ -992,9 +992,9 @@ static ssize_t gem_receive(NetClientState *nc, const uint8_t *buf, size_t size)
             return -1;
         }
 
-        DB_PRINT("copy %u bytes to 0x%" PRIx64 "\n",
-                 MIN(bytes_to_copy, rxbufsize),
-                 rx_desc_get_buffer(s, s->rx_desc[q]));
+        DB_PRINT("copy %" PRIu32 " bytes to 0x%" PRIx64 "\n",
+                MIN(bytes_to_copy, rxbufsize),
+                rx_desc_get_buffer(s, s->rx_desc[q] + rxbuf_offset));
 
         /* Copy packet data to emulated DMA buffer */
         address_space_write(&s->dma_as, rx_desc_get_buffer(s, s->rx_desc[q]) +
@@ -1160,8 +1160,8 @@ static void gem_transmit(CadenceGEMState *s)
              */
             if ((tx_desc_get_buffer(s, desc) == 0) ||
                 (tx_desc_get_length(desc) == 0)) {
-                DB_PRINT("Invalid TX descriptor @ 0x%x\n",
-                         (unsigned)packet_desc_addr);
+                DB_PRINT("Invalid TX descriptor @ 0x%" HWADDR_PRIx "\n",
+                         packet_desc_addr);
                 break;
             }
 
-- 
2.7.4
Re: [PATCH v2 01/10] net: cadence_gem: Fix debug statements
Posted by Edgar E. Iglesias 5 years, 9 months ago 
On Mon, May 04, 2020 at 07:35:59PM +0530, Sai Pavan Boddu wrote:
> Enabling debug breaks the build, Fix them and make debug statements
> always compilable. Fix few statements to use sized integer casting.
> 
> Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
> ---
>  hw/net/cadence_gem.c | 28 ++++++++++++++--------------
>  1 file changed, 14 insertions(+), 14 deletions(-)
> 
> diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
> index 22a0b1b..2f244eb 100644
> --- a/hw/net/cadence_gem.c
> +++ b/hw/net/cadence_gem.c
> @@ -35,14 +35,13 @@
>  #include "sysemu/dma.h"
>  #include "net/checksum.h"
>  
> -#ifdef CADENCE_GEM_ERR_DEBUG
> -#define DB_PRINT(...) do { \
> -    fprintf(stderr,  ": %s: ", __func__); \
> -    fprintf(stderr, ## __VA_ARGS__); \
> -    } while (0)
> -#else
> -    #define DB_PRINT(...)
> -#endif
> +#define CADENCE_GEM_ERR_DEBUG 0
> +#define DB_PRINT(...) do {\
> +    if (CADENCE_GEM_ERR_DEBUG) {   \
> +        qemu_log(": %s: ", __func__); \
> +        qemu_log(__VA_ARGS__); \
> +    } \
> +} while (0)
>  
>  #define GEM_NWCTRL        (0x00000000/4) /* Network Control reg */
>  #define GEM_NWCFG         (0x00000004/4) /* Network Config reg */
> @@ -979,7 +978,8 @@ static ssize_t gem_receive(NetClientState *nc, const uint8_t *buf, size_t size)
>          size += 4;
>      }
>  
> -    DB_PRINT("config bufsize: %d packet size: %ld\n", rxbufsize, size);
> +    DB_PRINT("config bufsize: %" PRIu64 " packet size: %" PRIu64 "\n",
> +             (uint64_t) rxbufsize, (uint64_t) size);

Shouldn't these be %u and %zd rather than casting to uint64_t?


>  
>      /* Find which queue we are targeting */
>      q = get_queue_from_screen(s, rxbuf_ptr, rxbufsize);
> @@ -992,9 +992,9 @@ static ssize_t gem_receive(NetClientState *nc, const uint8_t *buf, size_t size)
>              return -1;
>          }
>  
> -        DB_PRINT("copy %u bytes to 0x%" PRIx64 "\n",
> -                 MIN(bytes_to_copy, rxbufsize),
> -                 rx_desc_get_buffer(s, s->rx_desc[q]));
> +        DB_PRINT("copy %" PRIu32 " bytes to 0x%" PRIx64 "\n",
> +                MIN(bytes_to_copy, rxbufsize),
> +                rx_desc_get_buffer(s, s->rx_desc[q] + rxbuf_offset));

Looks like this is changing what we print (+ rxbuf_offset), was
that intentional? (it was not mentioned in the commit message)


>  
>          /* Copy packet data to emulated DMA buffer */
>          address_space_write(&s->dma_as, rx_desc_get_buffer(s, s->rx_desc[q]) +
> @@ -1160,8 +1160,8 @@ static void gem_transmit(CadenceGEMState *s)
>               */
>              if ((tx_desc_get_buffer(s, desc) == 0) ||
>                  (tx_desc_get_length(desc) == 0)) {
> -                DB_PRINT("Invalid TX descriptor @ 0x%x\n",
> -                         (unsigned)packet_desc_addr);
> +                DB_PRINT("Invalid TX descriptor @ 0x%" HWADDR_PRIx "\n",
> +                         packet_desc_addr);
>                  break;
>              }
>  
> -- 
> 2.7.4
>
RE: [PATCH v2 01/10] net: cadence_gem: Fix debug statements
Posted by Sai Pavan Boddu 5 years, 9 months ago 
Hi Edgar,

Below comments will be taken care in V3.

Thanks,
Sai Pavan

> -----Original Message-----
> From: Edgar E. Iglesias <edgar.iglesias@gmail.com>
> Sent: Monday, May 4, 2020 8:09 PM
> To: Sai Pavan Boddu <saipava@xilinx.com>
> Cc: Alistair Francis <Alistair.Francis@wdc.com>; Peter Maydell
> <peter.maydell@linaro.org>; Jason Wang <jasowang@redhat.com>; Markus
> Armbruster <armbru@redhat.com>; Philippe Mathieu-Daudé
> <philmd@redhat.com>; Tong Ho <tongh@xilinx.com>; Ramon Fried
> <rfried.dev@gmail.com>; qemu-arm@nongnu.org; qemu-
> devel@nongnu.org
> Subject: Re: [PATCH v2 01/10] net: cadence_gem: Fix debug statements
> 
> On Mon, May 04, 2020 at 07:35:59PM +0530, Sai Pavan Boddu wrote:
> > Enabling debug breaks the build, Fix them and make debug statements
> > always compilable. Fix few statements to use sized integer casting.
> >
> > Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
> > ---
> >  hw/net/cadence_gem.c | 28 ++++++++++++++--------------
> >  1 file changed, 14 insertions(+), 14 deletions(-)
> >
> > diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c index
> > 22a0b1b..2f244eb 100644
> > --- a/hw/net/cadence_gem.c
> > +++ b/hw/net/cadence_gem.c
> > @@ -35,14 +35,13 @@
> >  #include "sysemu/dma.h"
> >  #include "net/checksum.h"
> >
> > -#ifdef CADENCE_GEM_ERR_DEBUG
> > -#define DB_PRINT(...) do { \
> > -    fprintf(stderr,  ": %s: ", __func__); \
> > -    fprintf(stderr, ## __VA_ARGS__); \
> > -    } while (0)
> > -#else
> > -    #define DB_PRINT(...)
> > -#endif
> > +#define CADENCE_GEM_ERR_DEBUG 0
> > +#define DB_PRINT(...) do {\
> > +    if (CADENCE_GEM_ERR_DEBUG) {   \
> > +        qemu_log(": %s: ", __func__); \
> > +        qemu_log(__VA_ARGS__); \
> > +    } \
> > +} while (0)
> >
> >  #define GEM_NWCTRL        (0x00000000/4) /* Network Control reg */
> >  #define GEM_NWCFG         (0x00000004/4) /* Network Config reg */
> > @@ -979,7 +978,8 @@ static ssize_t gem_receive(NetClientState *nc,
> const uint8_t *buf, size_t size)
> >          size += 4;
> >      }
> >
> > -    DB_PRINT("config bufsize: %d packet size: %ld\n", rxbufsize, size);
> > +    DB_PRINT("config bufsize: %" PRIu64 " packet size: %" PRIu64 "\n",
> > +             (uint64_t) rxbufsize, (uint64_t) size);
> 
> Shouldn't these be %u and %zd rather than casting to uint64_t?
> 
> 
> >
> >      /* Find which queue we are targeting */
> >      q = get_queue_from_screen(s, rxbuf_ptr, rxbufsize); @@ -992,9
> > +992,9 @@ static ssize_t gem_receive(NetClientState *nc, const uint8_t
> *buf, size_t size)
> >              return -1;
> >          }
> >
> > -        DB_PRINT("copy %u bytes to 0x%" PRIx64 "\n",
> > -                 MIN(bytes_to_copy, rxbufsize),
> > -                 rx_desc_get_buffer(s, s->rx_desc[q]));
> > +        DB_PRINT("copy %" PRIu32 " bytes to 0x%" PRIx64 "\n",
> > +                MIN(bytes_to_copy, rxbufsize),
> > +                rx_desc_get_buffer(s, s->rx_desc[q] + rxbuf_offset));
> 
> Looks like this is changing what we print (+ rxbuf_offset), was that
> intentional? (it was not mentioned in the commit message)
> 
> 
> >
> >          /* Copy packet data to emulated DMA buffer */
> >          address_space_write(&s->dma_as, rx_desc_get_buffer(s,
> > s->rx_desc[q]) + @@ -1160,8 +1160,8 @@ static void
> gem_transmit(CadenceGEMState *s)
> >               */
> >              if ((tx_desc_get_buffer(s, desc) == 0) ||
> >                  (tx_desc_get_length(desc) == 0)) {
> > -                DB_PRINT("Invalid TX descriptor @ 0x%x\n",
> > -                         (unsigned)packet_desc_addr);
> > +                DB_PRINT("Invalid TX descriptor @ 0x%" HWADDR_PRIx "\n",
> > +                         packet_desc_addr);
> >                  break;
> >              }
> >
> > --
> > 2.7.4
> >

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.