Re: [PATCH v7 7/9] drm/xe/sysctrl: Improve firmware response error logging

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Passing restricted `__le32` fields to the `%x` format specifier causes Sparse warnings and prints byte-swapped values on big-endian architectures.
--

commit 76127f4976105bb1328499efbd66f71de155b1ba
Author: Mallesh Koujalagi <mallesh.koujalagi@intel.com>

drm/xe/sysctrl: Improve firmware response error logging

This commit updates the error logging in sysctrl_receive_frames() to use
the xe_log_err() helper. It also includes additional details such as the
response header values when a firmware response mismatch occurs.

> diff --git a/drivers/gpu/drm/xe/xe_sysctrl_mailbox.c b/drivers/gpu/drm/xe/xe_sysctrl_mailbox.c
> index cdd929ebed4a6..94ed2d378b89a 100644
> --- a/drivers/gpu/drm/xe/xe_sysctrl_mailbox.c
> +++ b/drivers/gpu/drm/xe/xe_sysctrl_mailbox.c
> @@ -233,13 +233,15 @@ static int sysctrl_receive_frames(struct xe_sysctrl *sc,
>  	if (!XE_SYSCTRL_HDR_IS_RESPONSE(hdr) ||
>  	    XE_SYSCTRL_HDR_GROUP_ID(hdr) != XE_SYSCTRL_HDR_GROUP_ID(req) ||
>  	    XE_SYSCTRL_HDR_COMMAND(hdr) != XE_SYSCTRL_HDR_COMMAND(req)) {
> -		xe_err(xe, "sysctrl: Response header mismatch\n");
> +		xe_log_err(xe, SYSCTRL, -EPROTO,
> +			   "Response header mismatch: %#x != %#x\n",
> +			   hdr->data, req->data);

[Severity: Low]
Does this code cause Sparse warnings and print byte-swapped values on big-endian
architectures?

In sysctrl_receive_frames() in xe_sysctrl_mailbox.c, hdr->data and req->data
appear to be restricted __le32 types (based on their use with le32_to_cpu() in
the XE_SYSCTRL_HDR_* macros). Passing them directly to the %#x format specifier
without endianness conversion or casting will cause Sparse to emit warnings
about type mismatches ("incorrect type in argument ... expected unsigned int,
got restricted __le32").

On big-endian systems, they will be interpreted as native integers by the
format string parser, printing byte-swapped values.

While the v7 changelog notes that le32_to_cpu() was explicitly removed, a
(__force u32) cast might still be needed if the raw value is intended to be
printed without triggering Sparse warnings.

>  		return -EPROTO;
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260907105313.2208003-11-mallesh.koujalagi@intel.com?part=7