Re: [PATCH trivial v2] hw/timer/imx_epit: Replace DPRINTF with trace events

On 17/6/26 13:59, Wang Yuxin wrote:
> Clean up the codebase by removing the outdated DEBUG_IMX_EPIT
> and DPRINTF macros, replacing them with modern QEMU trace events.
> 
> This also removes an empty and meaningless DPRINTF("\n") in the
> imx_epit_realize function.
> 
> Signed-off-by: jw_mac <[email protected]>
> Signed-off-by: Wang Yuxin <[email protected]>
> ---
>   hw/timer/imx_epit.c   | 25 +++++--------------------
>   hw/timer/trace-events |  8 ++++++++
>   2 files changed, 13 insertions(+), 20 deletions(-)


> @@ -328,8 +316,7 @@ static void imx_epit_write(void *opaque, hwaddr offset, uint64_t value,
>   {
>       IMXEPITState *s = IMX_EPIT(opaque);
>   
> -    DPRINTF("(%s, value = 0x%08x)\n", imx_epit_reg_name(offset >> 2),
> -            (uint32_t)value);
> +    trace_imx_epit_write(imx_epit_reg_name(offset >> 2), (uint32_t)value);

We could simply pass an uint64_t to the trace formatter.

Anyway faithful conversion, so:
Reviewed-by: Philippe Mathieu-Daudé <[email protected]>

>   
>       switch (offset >> 2) {
>       case 0: /* CR */
> @@ -362,7 +349,7 @@ static void imx_epit_cmp(void *opaque)
>       /* The cmp ptimer can't be running when the peripheral is disabled */
>       assert(s->cr & CR_EN);
>   
> -    DPRINTF("sr was %d\n", s->sr);
> +    trace_imx_epit_cmp(s->sr);
>       /* Set interrupt status bit SR.OCIF and update the interrupt state */
>       s->sr |= SR_OCIF;
>       imx_epit_update_int(s);
> @@ -399,8 +386,6 @@ static void imx_epit_realize(DeviceState *dev, Error **errp)
>       IMXEPITState *s = IMX_EPIT(dev);
>       SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
>   
> -    DPRINTF("\n");
> -
>       sysbus_init_irq(sbd, &s->irq);
>       memory_region_init_io(&s->iomem, OBJECT(s), &imx_epit_ops, s, TYPE_IMX_EPIT,
>                             0x00001000);
> diff --git a/hw/timer/trace-events b/hw/timer/trace-events
> index ac5afe84e8..3c282800c6 100644
> --- a/hw/timer/trace-events
> +++ b/hw/timer/trace-events
> @@ -122,3 +122,11 @@ hpet_ram_write_tn_cmp(uint8_t reg_off) "hpet_ram_writel HPET_TN_CMP + %" PRIu8
>   hpet_ram_write_invalid_tn_cmp(void) "invalid HPET_TN_CMP + 4 write"
>   hpet_ram_write_invalid(void) "invalid hpet_ram_writel"
>   hpet_ram_write_counter_write_while_enabled(void) "Writing counter while HPET enabled!"
> +
> +# imx_epit.c
> +imx_epit_get_freq(uint32_t freq) "ptimer frequency is %u"
> +imx_epit_read(const char *name, uint32_t value) "(%s) = 0x%08x"
> +imx_epit_write(const char *name, uint32_t value) "(%s, value = 0x%08x)"
> +imx_epit_cmp(uint32_t sr) "sr was %d"
> +
> +