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

On 8/20/2026 12:16 PM, Mallesh Koujalagi wrote:
> Make sysctrl_wait_bit_clear() return an error code rather than a bool.
> and update callers to use xe_log_err() with the propagated error code.
> 
> Signed-off-by: Mallesh Koujalagi <[email protected]>

one nit below, with that

Reviewed-by: Michal Wajdeczko <[email protected]>
> ---
>  drivers/gpu/drm/xe/xe_sysctrl_mailbox.c | 26 ++++++++++++-------------
>  1 file changed, 13 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_sysctrl_mailbox.c b/drivers/gpu/drm/xe/xe_sysctrl_mailbox.c
> index e13eebaac1d0..ef847f0a8f2c 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"
>  #include "xe_mmio.h"
>  #include "xe_pm.h"
>  #include "xe_printk.h"
> @@ -34,15 +35,11 @@ struct xe_sysctrl_mailbox_msg_hdr {
>  #define XE_SYSCTRL_HDR_RESULT(hdr) \
>  	FIELD_GET(SYSCTRL_HDR_RESULT_MASK, le32_to_cpu((hdr)->data))
>  
> -static bool sysctrl_wait_bit_clear(struct xe_sysctrl *sc, u32 bit_mask,
> -				   unsigned int timeout_ms)
> +static int sysctrl_wait_bit_clear(struct xe_sysctrl *sc, u32 bit_mask,
> +				  unsigned int timeout_ms)
>  {
> -	int ret;
> -
> -	ret = xe_mmio_wait32_not(sc->mmio, SYSCTRL_MB_CTRL, bit_mask, bit_mask,
> +	return xe_mmio_wait32_not(sc->mmio, SYSCTRL_MB_CTRL, bit_mask, bit_mask,
>  				 timeout_ms * 1000, NULL, false);

is this correctly aligned?

> -
> -	return ret == 0;
>  }
>  
>  static bool sysctrl_wait_bit_set(struct xe_sysctrl *sc, u32 bit_mask,
> @@ -145,12 +142,14 @@ static int sysctrl_send_frames(struct xe_sysctrl *sc,
>  	struct xe_device *xe = sc_to_xe(sc);
>  	u32 ctrl_reg, total_frames, frame;
>  	size_t bytes_sent, frame_size;
> +	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");
> +		return ret;
>  	}
>  
>  	sc->phase_bit ^= 1;
> @@ -173,10 +172,11 @@ 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);
> +		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);
>  			sc->phase_bit = 0;
> -			return -ETIMEDOUT;
> +			return ret;
>  		}
>  
>  		bytes_sent += frame_size;