Re: [Qemu-devel] [PATCH for-next v3 2/5] tmp105: Add debug output

[Andreas Färber]

CC'ing some more people from the "debug output revamp" RFC discussion.

Am 11.02.2013 20:01, schrieb Andreas Färber:
> From: Andreas Färber <address@hidden>
> 
> Signed-off-by: Andreas Färber <address@hidden>
> ---
>  hw/tmp105.c |   27 +++++++++++++++++++++++++--
>  1 Datei geändert, 25 Zeilen hinzugefügt(+), 2 Zeilen entfernt(-)
> 
> diff --git a/hw/tmp105.c b/hw/tmp105.c
> index 3ad2d2f..5dafa37 100644
> --- a/hw/tmp105.c
> +++ b/hw/tmp105.c
> @@ -23,6 +23,18 @@
>  #include "tmp105.h"
>  #include "qapi/visitor.h"
>  
> +#undef DEBUG_TMP105
> +
> +static inline void GCC_FMT_ATTR(1, 2) DPRINTF(const char *fmt, ...)

Being an inline function, I think it would be better to name this
tmp105_dprintf() and alias via: #define DPRINTF tmp105_dprintf
Assuming we want an uppercase conditional-output statement in the first
place?

dprintf() as used in some files (sheepdog, sPAPR, SPICE,
target-{i386,ppc,s390x}/kvm.c) is already used by system headers on some
platforms, so I'd rather avoid it.

Any other comments or suggestions? I'm preparing to respin the above
series in a similar, less-invasive style.

Thanks,
Andreas

> +{
> +#ifdef DEBUG_TMP105
> +    va_list ap;
> +    va_start(ap, fmt);
> +    vfprintf(stderr, fmt, ap);
> +    va_end(ap);
> +#endif
> +}
> +
>  static void tmp105_interrupt_update(TMP105State *s)
>  {
>      qemu_set_irq(s->pin, s->alarm ^ ((~s->config >> 2) & 1));        /* POL 
> */
> @@ -115,10 +127,16 @@ static void tmp105_read(TMP105State *s)
>          s->buf[s->len ++] = ((uint16_t) s->limit[1]) >> 0;
>          break;
>      }
> +
> +    DPRINTF("%s: %02" PRIx8 " %02" PRIx8 "%02" PRIx8 "\n",
> +            __func__, s->pointer, s->buf[0], s->buf[1]);
>  }
>  
>  static void tmp105_write(TMP105State *s)
>  {
> +    DPRINTF("%s: %02" PRIx8 " %02" PRIx8 "%02" PRIx8 "\n",
> +            __func__, s->pointer, s->buf[0], s->buf[1]);
> +
>      switch (s->pointer & 3) {
>      case TMP105_REG_TEMPERATURE:
>          break;
> @@ -144,18 +162,23 @@ static void tmp105_write(TMP105State *s)
>  static int tmp105_rx(I2CSlave *i2c)
>  {
>      TMP105State *s = TMP105(i2c);
> +    int ret;
>  
>      if (s->len < 2) {
> -        return s->buf[s->len ++];
> +        ret = s->buf[s->len++];
>      } else {
> -        return 0xff;
> +        ret = 0xff;
>      }
> +    DPRINTF("%s: <- %02x\n", __func__, ret);
> +    return ret;
>  }
>  
>  static int tmp105_tx(I2CSlave *i2c, uint8_t data)
>  {
>      TMP105State *s = TMP105(i2c);
>  
> +    DPRINTF("%s: -> %02" PRIx8 "\n", __func__, data);
> +
>      if (s->len == 0) {
>          s->pointer = data;
>          s->len++;

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg