[ v01] mailbox: pcc: report errors for PCC clients

[ v01] mailbox: pcc: report errors for PCC clients

[Adam Young]

The tx_done callback function has a return code (rc) parameter
that the tx_done callback can use to determine how to handle an error.
However the IRQ handler was not setting that value if there is an error.

The following clients are affected:

drivers/acpi/cppc_acpi.c
drivers/i2c/busses/i2c-xgene-slimpro.c
drivers/hwmon/xgene-hwmon.c
drivers/soc/hisilicon/kunpeng_hccs.c
drivers/devfreq/hisi_uncore_freq.c

All of these only use the error code to report, so they
are expecting an error code to come thorugh, but they
do not modify behavior based on this code.

Fixes: c45ded7e1135 ("mailbox: pcc: Add support for PCCT extended PCC subspaces(type 3/4)")
Signed-off-by: Adam Young <admiyo@os.amperecomputing.com>

---
---
 drivers/mailbox/pcc.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/mailbox/pcc.c b/drivers/mailbox/pcc.c
index 636879ae1db7..25650446cd2b 100644
--- a/drivers/mailbox/pcc.c
+++ b/drivers/mailbox/pcc.c
@@ -314,6 +314,7 @@ static irqreturn_t pcc_mbox_irq(int irq, void *p)
 {
 	struct pcc_chan_info *pchan;
 	struct mbox_chan *chan = p;
+	int rc;
 
 	pchan = chan->con_priv;
 
@@ -327,8 +328,11 @@ static irqreturn_t pcc_mbox_irq(int irq, void *p)
 	if (!pcc_mbox_cmd_complete_check(pchan))
 		return IRQ_NONE;
 
-	if (pcc_mbox_error_check_and_clear(pchan))
+	rc = pcc_mbox_error_check_and_clear(pchan);
+	if (rc) {
+		mbox_chan_txdone(chan, rc);
 		return IRQ_NONE;
+	}
 
 	/*
 	 * Clear this flag after updating interrupt ack register and just
-- 
2.43.0

Re: [ v01] mailbox: pcc: report errors for PCC clients

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] The error path fails to clear the `pchan->chan_in_use` flag, leading to state machine corruption and spurious client callbacks.
- [High] The error path fails to acknowledge Slave Subspace notifications, which can permanently hang the channel.
- [High] The interrupt handler incorrectly returns `IRQ_NONE` after successfully handling a mailbox error condition, risking the disablement of the shared IRQ line.
--

commit 8be35a18843a43d39fee5986fb5339b3f4e55c68
Author: Adam Young <admiyo@os.amperecomputing.com>

mailbox: pcc: report errors for PCC clients

This commit updates the PCC mailbox interrupt handler to pass the error code
to the tx_done callback via mbox_chan_txdone() when a channel error is
detected. This allows several clients to accurately report these errors.

> diff --git a/drivers/mailbox/pcc.c b/drivers/mailbox/pcc.c
> --- a/drivers/mailbox/pcc.c
> +++ b/drivers/mailbox/pcc.c
[ ... ]
> @@ -327,8 +328,11 @@ static irqreturn_t pcc_mbox_irq(int irq, void *p)
>  	if (!pcc_mbox_cmd_complete_check(pchan))
>  		return IRQ_NONE;
>  
> -	if (pcc_mbox_error_check_and_clear(pchan))
> +	rc = pcc_mbox_error_check_and_clear(pchan);
> +	if (rc) {
> +		mbox_chan_txdone(chan, rc);
>  		return IRQ_NONE;

Since the early return is taken when an error occurs, does this code need to
clear pchan->chan_in_use? If it remains true while the channel is idle, could
the next interrupt on a shared IRQ line bypass the !pchan->chan_in_use filter,
incorrectly evaluate cmd_complete as true, and spuriously call
mbox_chan_received_data() with phantom data?

Also, for PCC Slave Subspaces, the platform sends asynchronous notifications
that must be acknowledged. By returning early, this path skips calling
pcc_chan_acknowledge(pchan). Can this leave the command unacknowledged and
hang the Slave Subspace if the platform stops sending notifications?

Finally, since pcc_mbox_error_check_and_clear() clears the hardware error
register, it confirms the device legitimately generated the interrupt. Does
returning IRQ_NONE here risk having the kernel's spurious interrupt detector
eventually disable the shared IRQ line if this error path is triggered
repeatedly?

> +	}
>  
>  	/*
>  	 * Clear this flag after updating interrupt ack register and just

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260515221745.714736-1-admiyo@os.amperecomputing.com?part=1