Re: [Qemu-ppc] [Qemu-devel] [PATCH 2/6] dbdma: add per-channel debugging enabled via DEBUG_DBDMA_CHANMASK

[Mark Cave-Ayland]

On 10/07/16 19:08, Mark Cave-Ayland wrote:

> By default large amounts of DBDMA debugging are produced when often it is just
> 1 or 2 channels that are of interest. Introduce DEBUG_DBDMA_CHANMASK to allow
> the developer to select the channels of interest at compile time, and then
> further add the extra channel information to each debug statement where
> possible.
> 
> Also clearly mark the start/end of DBDMA_run_bh to allow tracking the bottom
> half execution.
> 
> Signed-off-by: Mark Cave-Ayland <address@hidden>
> Acked-by: Benjamin Herrenschmidt <address@hidden>
> ---
>  hw/misc/macio/mac_dbdma.c |   75 
> +++++++++++++++++++++++++--------------------
>  1 file changed, 42 insertions(+), 33 deletions(-)
> 
> diff --git a/hw/misc/macio/mac_dbdma.c b/hw/misc/macio/mac_dbdma.c
> index b6639f4..8e4b208 100644
> --- a/hw/misc/macio/mac_dbdma.c
> +++ b/hw/misc/macio/mac_dbdma.c
> @@ -46,6 +46,7 @@
>  
>  /* debug DBDMA */
>  #define DEBUG_DBDMA 0
> +#define DEBUG_DBDMA_CHANMASK ((1ul << DBDMA_CHANNELS) - 1)

Someone flagged up that this doesn't work on 32-bit hosts because while
the channel is 0 to 31, the compiler shifts first and then subtracts
which gives warnings like this:

hw/misc/macio/mac_dbdma.c: In function ‘dbdma_cmdptr_load’:
hw/misc/macio/mac_dbdma.c:49:36: error: left shift count >= width of
type [-Werror=shift-count-overflow]
 #define DEBUG_DBDMA_CHANMASK ((1ul << DBDMA_CHANNELS) - 1)

The fix here is to change the above to:

#define DEBUG_DBDMA_CHANMASK ((1ull << DBDMA_CHANNELS) - 1)

Here the unsigned long long will ensure that the shift is valid before
the subtraction to get the final 32-bit wide mask.

David, are you able to fix this up in your branch?

>  #define DBDMA_DPRINTF(fmt, ...) do { \
>      if (DEBUG_DBDMA) { \
> @@ -53,6 +54,14 @@
>      } \
>  } while (0);
>  
> +#define DBDMA_DPRINTFCH(ch, fmt, ...) do { \
> +    if (DEBUG_DBDMA) { \
> +        if ((1ul << (ch)->channel) & DEBUG_DBDMA_CHANMASK) { \
> +            printf("DBDMA[%02x]: " fmt , (ch)->channel, ## __VA_ARGS__); \
> +        } \
> +    } \
> +} while (0);
> +

This part is still okay for a channel range 0 to 31.


ATB,

Mark.