Re: [PATCH 3/4] drm/xe/pcode: Report pcode errors using SIGID

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Passing a negated error code (`-ret`) to `xe_log_err()` in `xe_pcode_ready()` creates a positive value, causing the internal `IS_ERR()` check to fail and silently drop the error code from the log output.
- [Low] Missing newline (`\n`) in the format string for `xe_log_err()` in `xe_pcode_request()`.
--

--- Patch [3]: [PATCH 3/4] drm/xe/pcode: Report pcode errors using SIGID ---
commit d093d9ae2e2b45793c1c50dd19b274f80a9f8387
Author: Umesh Nerlige Ramappa <[email protected]>

drm/xe/pcode: Report pcode errors using SIGID

This commit replaces direct drm_err() logging calls with the xe_log_err()
wrapper in the pcode component. This updates the driver to report pcode 
errors using the SIGID mechanism.

> diff --git a/drivers/gpu/drm/xe/xe_pcode.c b/drivers/gpu/drm/xe/xe_pcode.c
> index e9125341481b7..b35c1a77effd1 100644
> --- a/drivers/gpu/drm/xe/xe_pcode.c
> +++ b/drivers/gpu/drm/xe/xe_pcode.c

[ ... ]

> @@ -225,6 +224,9 @@ int xe_pcode_request(struct xe_tile *tile, u32 mbox, u32 request,
>  				true, 50 * 1000, true);
>  	preempt_enable();
>  
> +	if (!status && ret)
> +		xe_log_err(tile, PCODE, ret, "Request timed out on retry");

[Severity: Low]
This isn't a bug, but should this format string end with a newline character
(\n) to prevent subsequent kernel log messages from being appended to the
same line?

> +
>  out:
>  	mutex_unlock(&tile->pcode.lock);
>  	return status ? status : ret;

[ ... ]

> @@ -319,8 +321,8 @@ int xe_pcode_ready(struct xe_device *xe, bool locked)
>  		mutex_unlock(&tile->pcode.lock);
>  
>  	if (ret)
> -		drm_err(&xe->drm,
> -			"PCODE initialization timedout after: 3 min\n");
> +		xe_log_err(xe, PCODE, -ret,
> +			"Initialization timed out after: %d min\n", timeout_us / 60000000);

[Severity: Medium]
The xe_log_err() macro wraps the passed error code in ERR_PTR(). If ret is
already a negative error code (such as -ETIMEDOUT), does passing -ret create
a positive value that will fail the internal IS_ERR() check inside the
logging framework and silently drop the error code?

>  
>  	return ret;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3