Re: [PATCH v3 3/5] drm/xe/xe_ras: Add helper to clear error counter

[Raag Jadav <raag.jadav@intel.com>]

On Tue, Apr 21, 2026 at 08:20:59PM +0530, Riana Tauro wrote:
> Add structures and helper function to clear error counter value.

...

> +/**
> + * xe_ras_clear_error_counter() - Clear error counter value
> + * @xe: xe device instance
> + * @severity: Error severity level to be cleared
> + * @error_id: Error component to be cleared
> + *
> + * This function clears the value of a specific RAS error counter based on
> + * the provided severity and component.
> + *
> + * Return: 0 on success, negative error code on failure.
> + */
> +int xe_ras_clear_error_counter(struct xe_device *xe, enum drm_xe_ras_error_severity severity,

Same comment as last patch.

> +			       u32 error_id)
> +{
> +	struct xe_ras_clear_counter_response response = {0};
> +	struct xe_ras_clear_counter_request request = {0};
> +	struct xe_sysctrl_mailbox_command command = {0};
> +	struct xe_ras_error_class *error_class = &request.error_class;
> +	enum xe_sysctrl_gfsp_cmd cmd = XE_SYSCTRL_CMD_CLEAR_COUNTER;
> +	size_t rlen;
> +	u32 status;

Shouldn't this be int? Or why not just reuse ret?

> +	int ret;
> +
> +	error_class->common.severity = drm_to_xe_ras_severity[severity];
> +	error_class->common.component = drm_to_xe_ras_component[error_id];
> +
> +	prepare_sysctrl_command(&command, cmd, &request, sizeof(request),
> +				&response, sizeof(response));
> +
> +	guard(xe_pm_runtime)(xe);
> +	ret = xe_sysctrl_send_command(&xe->sc, &command, &rlen);
> +	if (ret) {
> +		xe_err(xe, "sysctrl: cmd %d error ret %d\n", cmd, ret);
> +		return ret;
> +	}
> +
> +	if (rlen != sizeof(response)) {
> +		xe_err(xe, "sysctrl: cmd %d response size mismatch. Expected %zu, got %zu\n",
> +		       cmd, sizeof(response), rlen);
> +		return -EIO;
> +	}
> +
> +	status = xe_ras_status_to_errno[response.status];
> +	if (status) {
> +		xe_err(xe, "sysctrl: cmd %d failed %d\n", cmd, status);
> +		return status;

Hm, so I'm wondering if this is useful to end user? Note, anything exposed
to the user will be hard to change once in place.

> +	}
> +
> +	return 0;
> +}

...

> +++ b/drivers/gpu/drm/xe/xe_sysctrl_mailbox_types.h
> @@ -14,9 +14,11 @@
>   * enum xe_sysctrl_gfsp_cmd  - Sysctrl Command ID's for GFSP group
>   *
>   * @XE_SYSCTRL_CMD_GET_COUNTER: Get error counter value
> + * @XE_SYSCTRL_CMD_CLEAR_COUNTER: Clear error counter value
>   */
>  enum xe_sysctrl_gfsp_cmd  {
> -	XE_SYSCTRL_CMD_GET_COUNTER = 0x03
> +	XE_SYSCTRL_CMD_GET_COUNTER = 0x03,

Redundant churn. Please add comma in original patch.

> +	XE_SYSCTRL_CMD_CLEAR_COUNTER = 0x04

Also please align with tabs for readability.

Raag

>  };
>  
>  enum xe_sysctrl_group {
> -- 
> 2.47.1
>