Re: [PATCH v8 06/10] drm/xe/sysctrl: simplify mailbox header handling

[Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>]

Hi Mallesh,

s/simplify/Fix/ in the subject. On big-endian the byte order would be 
swapped twice since the mmio helpers internally handle endianness as 
well.

Please include the Fixes tag too

Fixes: 1f95f618182b ("drm/xe/xe_sysctrl: Add System Controller mailbox 
communication support")

On Mon, Sep 21, 2026 at 07:09:33PM +0530, Mallesh Koujalagi wrote:
>Use u32 for the mailbox header data field and remove unnecessary
>cpu_to_le32()/le32_to_cpu() conversions when encoding and decoding
>header bitfields.
>
>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>
>---
> drivers/gpu/drm/xe/xe_sysctrl_mailbox.c | 14 +++++++-------
> 1 file changed, 7 insertions(+), 7 deletions(-)
>
>diff --git a/drivers/gpu/drm/xe/xe_sysctrl_mailbox.c b/drivers/gpu/drm/xe/xe_sysctrl_mailbox.c
>index 50bbdab072de..750178a0974b 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;
>+	u32 data;
> } __packed;

nit: __packed may be dropped. I guess it only protects any future 
additions to this struct.

Thanks,
Umesh

>
> #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
>