Re: [PATCH v6 1/9] drm/xe/sysctrl: Return error codes from sysctrl_wait_bit_clear()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The patch introduces a call to `xe_log_err()` and includes `xe_log.h`, but neither the header nor the macro exist in the source tree, causing an immediate build failure.
--

commit 2a9731ca81fbb37bb5eeef908043568e8555c104
Author: Mallesh Koujalagi <mallesh.koujalagi@intel.com>

drm/xe/sysctrl: Return error codes from sysctrl_wait_bit_clear()

This commit updates sysctrl_wait_bit_clear() to return an error code instead
of a boolean. It also modifies the callers to log failures using xe_log_err().

> diff --git a/drivers/gpu/drm/xe/xe_sysctrl_mailbox.c b/drivers/gpu/drm/xe/xe_sysctrl_mailbox.c
> index 72baf1aa4b3ad..c36cac8c62887 100644
> --- a/drivers/gpu/drm/xe/xe_sysctrl_mailbox.c
> +++ b/drivers/gpu/drm/xe/xe_sysctrl_mailbox.c
> @@ -11,6 +11,7 @@
>  
>  #include "regs/xe_sysctrl_regs.h"
>  #include "xe_device.h"
> +#include "xe_log.h"

[Severity: Low]
Does this header exist in the repository? It appears to be missing, which
causes a build failure.

[ ... ]

> @@ -146,12 +143,14 @@ static int sysctrl_send_frames(struct xe_sysctrl *sc,
>  	u32 ctrl_reg, total_frames, frame;
>  	size_t bytes_sent, frame_size;
>  	bool phase;
> +	int ret;
>  
>  	total_frames = DIV_ROUND_UP(cmd_size, XE_SYSCTRL_MB_FRAME_SIZE);
>  
> -	if (!sysctrl_wait_bit_clear(sc, SYSCTRL_MB_CTRL_RUN_BUSY, timeout_ms)) {
> -		xe_err(xe, "sysctrl: Mailbox busy\n");
> -		return -EBUSY;
> +	ret = sysctrl_wait_bit_clear(sc, SYSCTRL_MB_CTRL_RUN_BUSY, timeout_ms);
> +	if (ret) {
> +		xe_log_err(xe, SYSCTRL, ret, "Mailbox busy\n");

[Severity: Low]
Is xe_log_err() defined anywhere in the tree? This introduces a call to an
undefined macro, resulting in an implicit declaration error during compilation
in sysctrl_send_frames().

[ ... ]

> @@ -174,9 +173,10 @@ static int sysctrl_send_frames(struct xe_sysctrl *sc,
>  
>  		xe_mmio_write32(sc->mmio, SYSCTRL_MB_CTRL, ctrl_reg);
>  
> -		if (!sysctrl_wait_bit_clear(sc, SYSCTRL_MB_CTRL_RUN_BUSY, timeout_ms)) {
> -			xe_err(xe, "sysctrl: Frame %u acknowledgment timeout\n", frame);
> -			return -ETIMEDOUT;
> +		ret = sysctrl_wait_bit_clear(sc, SYSCTRL_MB_CTRL_RUN_BUSY, timeout_ms);
> +		if (ret) {
> +			xe_log_err(xe, SYSCTRL, ret, "Frame %u acknowledgment timeout\n", frame);

[Severity: Low]
This is another instance of using the undefined xe_log_err() macro, leading
to a compilation failure.

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