Re: [PATCH v23 1/2] ACPI:RAS2: Add driver for the ACPI RAS2 feature table

Hi Shiju

On Sun, Jul 26, 2026 at 10:28:11PM +0100, [email protected] wrote:
> From: Shiju Jose <[email protected]>
> 
> ACPI 6.5 Specification, section 5.2.21, defined RAS2 feature table (RAS2).
> Driver adds support for RAS2 feature table, which provides interfaces for
> platform RAS features, e.g., for HW-based memory scrubbing, and logical to
> PA translation service. RAS2 uses PCC channel subspace for communicating
> with the ACPI compliant HW platform.
> 
> Co-developed-by: A Somasundaram <[email protected]>
> Signed-off-by: A Somasundaram <[email protected]>
> Co-developed-by: Jonathan Cameron <[email protected]>
> Signed-off-by: Jonathan Cameron <[email protected]>
> Tested-by: Daniel Ferguson <[email protected]>
> Reviewed-by: Ashok Raj <[email protected]>
> Signed-off-by: Shiju Jose <[email protected]>
> ---
>  drivers/acpi/Kconfig  |  11 +
>  drivers/acpi/Makefile |   1 +
>  drivers/acpi/bus.c    |   3 +
>  drivers/acpi/ras2.c   | 509 ++++++++++++++++++++++++++++++++++++++++++
>  include/acpi/ras2.h   |  57 +++++
>  5 files changed, 581 insertions(+)
>  create mode 100644 drivers/acpi/ras2.c
>  create mode 100644 include/acpi/ras2.h
> 
> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig

[snip]

> +int ras2_send_pcc_cmd(struct ras2_mem_ctx *ras2_ctx, u16 cmd)
> +{
> +	struct acpi_ras2_shmem __iomem *gen_comm_base;
> +	struct mbox_chan *pcc_channel;
> +	struct ras2_sspcc *sspcc;
> +	s64 time_delta;
> +	u16 val;
> +	int rc;
> +

[snip]

> +
> +	/* Ring doorbell */
> +	rc = mbox_send_message(pcc_channel, &cmd);
> +	/*
> +	 * mbox_send_message() returns a non-negative integer for successful submission
> +	 * and a negative value on failure.
> +	 */
> +	if (rc < 0) {
> +		dev_warn(ras2_ctx->dev,
> +			 "Error sending PCC mbox message command: 0x%x, rc:%d\n", cmd, rc);
> +		/* Restore CMD COMPLETE bit on error */
> +		val = ioread16(&gen_comm_base->status);
> +		val |= PCC_STATUS_CMD_COMPLETE;
> +		iowrite16(val, &gen_comm_base->status);
> +		return rc;
> +	} else {
> +		rc = 0;
> +	}

Minor: 

Sorry, I missed in prior review.

Can the else after return be removed?
just set rc = 0 should be sufficient.

Cheers,
Ashok