Hi Edgar,
Below comments will be taken care in V3.
Thanks,
Sai Pavan
> -----Original Message-----
> From: Edgar E. Iglesias <edgar.igles...@gmail.com>
> Sent: Monday, May 4, 2020 8:09 PM
> To: Sai Pavan Boddu <saip...@xilinx.com>
> Cc: Alistair Francis <alistair.fran...@wdc.com>; Peter Maydell
> <peter.mayd...@linaro.org>; Jason Wang <jasow...@redhat.com>; Markus
> Armbruster <arm...@redhat.com>; Philippe Mathieu-Daudé
> <phi...@redhat.com>; Tong Ho <to...@xilinx.com>; Ramon Fried
> <rfried....@gmail.com>; qemu-...@nongnu.org; qemu-
> de...@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.bo...@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
> >
