Re: [PATCH v3 06/13] macfb: add trace events for reading and writing the control registers

[Laurent Vivier]

Le 07/10/2021 à 11:34, Mark Cave-Ayland a écrit :
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> Reviewed-by: Laurent Vivier <laurent@vivier.eu>
> ---
>  hw/display/macfb.c      | 8 +++++++-
>  hw/display/trace-events | 4 ++++
>  2 files changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/display/macfb.c b/hw/display/macfb.c
> index f88f5a6523..1128a51c98 100644
> --- a/hw/display/macfb.c
> +++ b/hw/display/macfb.c
> @@ -20,6 +20,7 @@
>  #include "qapi/error.h"
>  #include "hw/qdev-properties.h"
>  #include "migration/vmstate.h"
> +#include "trace.h"
>  
>  #define VIDEO_BASE 0x00001000
>  #define DAFB_BASE  0x00800000
> @@ -289,7 +290,10 @@ static uint64_t macfb_ctrl_read(void *opaque,
>                                  hwaddr addr,
>                                  unsigned int size)
>  {
> -    return 0;
> +    uint64_t val = 0;
> +
> +    trace_macfb_ctrl_read(addr, val, size);
> +    return val;
>  }
>  
>  static void macfb_ctrl_write(void *opaque,
> @@ -311,6 +315,8 @@ static void macfb_ctrl_write(void *opaque,
>          }
>          break;
>      }
> +
> +    trace_macfb_ctrl_write(addr, val, size);
>  }
>  
>  static const MemoryRegionOps macfb_ctrl_ops = {
> diff --git a/hw/display/trace-events b/hw/display/trace-events
> index f03f6655bc..6c460aaa4c 100644
> --- a/hw/display/trace-events
> +++ b/hw/display/trace-events
> @@ -167,3 +167,7 @@ sm501_disp_ctrl_read(uint32_t addr, uint32_t val) 
> "addr=0x%x, val=0x%x"
>  sm501_disp_ctrl_write(uint32_t addr, uint32_t val) "addr=0x%x, val=0x%x"
>  sm501_2d_engine_read(uint32_t addr, uint32_t val) "addr=0x%x, val=0x%x"
>  sm501_2d_engine_write(uint32_t addr, uint32_t val) "addr=0x%x, val=0x%x"
> +
> +# macfb.c
> +macfb_ctrl_read(uint64_t addr, uint64_t value, unsigned int size) "addr 
> 0x%"PRIx64 " value 0x%"PRIx64 " size %d"
> +macfb_ctrl_write(uint64_t addr, uint64_t value, unsigned int size) "addr 
> 0x%"PRIx64 " value 0x%"PRIx64 " size %d"

You changed the "int" to "unsigned int", you should change the "%d" to "%u"

Thanks,
Laurent