| Message ID | 1588601168-27576-2-git-send-email-sai.pavan.boddu@xilinx.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Cadence GEM Fixes | expand |
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 >
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 > >
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; }
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(-)