Re: [PATCH v9 06/10] drm/xe/sysctrl: Fix mailbox header handling

[Rodrigo Vivi <rodrigo.vivi@intel.com>]

On Thu, Sep 24, 2026 at 11:39:07AM +0530, Mallesh Koujalagi wrote:
> The mailbox header was defined as __le32, which required
> cpu_to_le32() and le32_to_cpu() conversions whenever it was
> write or read.
> 
> These conversions are unnecessary because the header is always
> handled as a 32-bit value, not as raw bytes. It is written with
> xe_mmio_write32() and read with xe_mmio_read32(), which already
> handle any required byte-order conversion through the underlying
> writel()/readl() accessors.
> 
> As a result, the header value is already in CPU byte order when
> used by the driver. Keeping the extra conversions had no effect
> on little-endian systems and could cause an incorrect on
> big-endian systems.
> 
> So use u32 for the mailbox header and remove the unnecessary
> endianness conversions.
> 
> Fixes: 1f95f618182b ("drm/xe/xe_sysctrl: Add System Controller mailbox communication support")
> Suggested-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Signed-off-by: Mallesh Koujalagi <mallesh.koujalagi@intel.com>
> Tested-by: Anoop Vijay <anoop.c.vijay@intel.com>

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

> ---
> v9:
> - Update commit message. (Michal)
> - Use fix tag. (Umesh)
> - drop __packed.
> - s/simplify/Fix/
> ---
>  drivers/gpu/drm/xe/xe_sysctrl_mailbox.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_sysctrl_mailbox.c b/drivers/gpu/drm/xe/xe_sysctrl_mailbox.c
> index 2fb317175922..2e4247914bf9 100644
> --- a/drivers/gpu/drm/xe/xe_sysctrl_mailbox.c
> +++ b/drivers/gpu/drm/xe/xe_sysctrl_mailbox.c
> @@ -20,20 +20,20 @@
>  #include "xe_sysctrl_mailbox_types.h"
>  
>  struct xe_sysctrl_mailbox_msg_hdr {
> -	__le32 data;
> -} __packed;
> +	u32 data;
> +};
>  
>  #define XE_SYSCTRL_HDR_GROUP_ID(hdr) \
> -	FIELD_GET(SYSCTRL_HDR_GROUP_ID_MASK, le32_to_cpu((hdr)->data))
> +	FIELD_GET(SYSCTRL_HDR_GROUP_ID_MASK, (hdr)->data)
>  
>  #define XE_SYSCTRL_HDR_COMMAND(hdr) \
> -	FIELD_GET(SYSCTRL_HDR_COMMAND_MASK, le32_to_cpu((hdr)->data))
> +	FIELD_GET(SYSCTRL_HDR_COMMAND_MASK, (hdr)->data)
>  
>  #define XE_SYSCTRL_HDR_IS_RESPONSE(hdr) \
> -	FIELD_GET(SYSCTRL_HDR_IS_RESPONSE, le32_to_cpu((hdr)->data))
> +	FIELD_GET(SYSCTRL_HDR_IS_RESPONSE, (hdr)->data)
>  
>  #define XE_SYSCTRL_HDR_RESULT(hdr) \
> -	FIELD_GET(SYSCTRL_HDR_RESULT_MASK, le32_to_cpu((hdr)->data))
> +	FIELD_GET(SYSCTRL_HDR_RESULT_MASK, (hdr)->data)
>  
>  static int sysctrl_wait_bit_clear(struct xe_sysctrl *sc, u32 bit_mask,
>  				  unsigned int timeout_ms)
> @@ -112,8 +112,8 @@ static int sysctrl_prepare_command(struct xe_device *xe,
>  		return -ENOMEM;
>  
>  	hdr = (struct xe_sysctrl_mailbox_msg_hdr *)buffer;
> -	hdr->data = cpu_to_le32(FIELD_PREP(SYSCTRL_HDR_GROUP_ID_MASK, group_id) |
> -				     FIELD_PREP(SYSCTRL_HDR_COMMAND_MASK, command));
> +	hdr->data = FIELD_PREP(SYSCTRL_HDR_GROUP_ID_MASK, group_id) |
> +		    FIELD_PREP(SYSCTRL_HDR_COMMAND_MASK, command);
>  
>  	if (data_in && data_in_len)
>  		memcpy(buffer + sizeof(*hdr), data_in, data_in_len);
> -- 
> 2.48.1
>