Re: [PATCH for-5.2 3/4] hw/net/can/ctucan_core: Handle big-endian hosts

["Philippe Mathieu-Daudé" <f4bug@amsat.org>]

On 11/6/20 6:11 PM, Peter Maydell wrote:
> The ctucan driver defines types for its registers which are a union
> of a uint32_t with a struct with bitfields for the individual
> fields within that register. This is a bad idea, because bitfields
> aren't portable. The ctu_can_fd_regs.h header works around the
> most glaring of the portability issues by defining the
> fields in two different orders depending on the setting of the
> __LITTLE_ENDIAN_BITFIELD define. However, in ctucan_core.h this
> is unconditionally set to 1, which is wrong for big-endian hosts.
> 
> Set it only if HOST_WORDS_BIGENDIAN is not set. There is no need
> for a "have we defined it already" guard, because the only place
> that should set it is ctucan_core.h, which has the usual
> double-inclusion guard.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> Ideally all that bitfield-using code would be rewritten to use
> extract32 and deposit32 instead, IMHO.
> ---
>  hw/net/can/ctucan_core.h | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/hw/net/can/ctucan_core.h b/hw/net/can/ctucan_core.h
> index f21cb1c5ec3..bbc09ae0678 100644
> --- a/hw/net/can/ctucan_core.h
> +++ b/hw/net/can/ctucan_core.h
> @@ -31,8 +31,7 @@
>  #include "exec/hwaddr.h"
>  #include "net/can_emu.h"
>  
> -
> -#ifndef __LITTLE_ENDIAN_BITFIELD
> +#ifndef HOST_WORDS_BIGENDIAN
>  #define __LITTLE_ENDIAN_BITFIELD 1
>  #endif

Alternatively s/#ifdef __LITTLE_ENDIAN_BITFIELD/#ifndef
HOST_WORDS_BIGENDIAN/ the codebase and remove this here...

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>