[PATCH v2] mailbox: pcc: Complete the transfer when the platform flags an error

[PATCH v2] mailbox: pcc: Complete the transfer when the platform flags an error

[Breno Leitao]

A single error reported by the platform stops all later traffic on a
PCC channel whose client relies on the controller to complete
transfers, such as the SCMI PCC transport. Every later request on the
channel times out.

When the platform completes a command and sets a bit in the Error
Status register, pcc_mbox_irq() acks the platform interrupt, clears the
error and returns IRQ_NONE without calling mbox_chan_txdone().

No other interrupt arrives for that transfer, so the mailbox core keeps
it as the active request and holds back every later message.

The chan_in_use flag also stays set, so on a shared interrupt line the
next interrupt raised for another subspace passes the checks in
pcc_mbox_irq(), since Command Complete is still set, and runs the
success path on the failed transfer.

Log the error and fall through to the completion code, which clears
chan_in_use before calling mbox_chan_txdone(). Skip
mbox_chan_received_data() on error, as the shared memory holds no valid
response, and complete the transfer with -EIO rather than the helper's
return value, which is a positive acpi_status when the register read
fails.

Falling through also runs pcc_chan_acknowledge() after an error. It
only acts on responder subspaces, where the platform notification is
now acknowledged but not passed to the client.

Clients that wait for the rx callback still time out on the failed
transfer, but the channel works again afterwards.

Fixes: 9c753f7c953c ("mailbox: pcc: Mark Tx as complete in PCC IRQ handler")
Cc: stable@vger.kernel.org
Suggested-by: Michael van der Westhuizen <rmikey@meta.com>
Signed-off-by: Breno Leitao <leitao@debian.org>
---
Changes in v2:
- Fall through to the completion code on error (Sudeep)
- pcc_chan_acknowledge() now also runs after an error, which only
  affects responder subspaces
- Link to v1: https://patch.msgid.link/20261027-pcc_error_txdone-v1-1-2f8cfbb36d90@debian.org
---
 drivers/mailbox/pcc.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/mailbox/pcc.c b/drivers/mailbox/pcc.c
index 14dc4a187bfc67..0d36aa511a9d02 100644
--- a/drivers/mailbox/pcc.c
+++ b/drivers/mailbox/pcc.c
@@ -315,6 +315,7 @@ static irqreturn_t pcc_mbox_irq(int irq, void *p)
 {
 	struct pcc_chan_info *pchan;
 	struct mbox_chan *chan = p;
+	int ret;
 
 	pchan = chan->con_priv;
 
@@ -333,8 +334,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))
-		return IRQ_NONE;
+	ret = pcc_mbox_error_check_and_clear(pchan);
+	if (ret)
+		dev_err_ratelimited(chan->mbox->dev,
+				    "PCC subspace %td: transfer error %d\n",
+				    pchan - chan_info, ret);
 
 	/*
 	 * Clear this flag after updating the interrupt ack register and before
@@ -343,8 +347,9 @@ static irqreturn_t pcc_mbox_irq(int irq, void *p)
 	 * the lockless update observed by the send and interrupt paths.
 	 */
 	WRITE_ONCE(pchan->chan_in_use, false);
-	mbox_chan_received_data(chan, NULL);
-	mbox_chan_txdone(chan, 0);
+	if (!ret)
+		mbox_chan_received_data(chan, NULL);
+	mbox_chan_txdone(chan, ret ? -EIO : 0);
 
 	pcc_chan_acknowledge(pchan);
 

---
base-commit: f1ac44c32cba88ad65dcecd4a279661d14e93651
change-id: 20261027-pcc_error_txdone-94f48c610666

Best regards,
--  
Breno Leitao <leitao@debian.org>

Re: [PATCH v2] mailbox: pcc: Complete the transfer when the platform flags an error

[Sudeep Holla]

On Wed, Oct 28, 2026 at 06:35:30AM -0700, Breno Leitao wrote:
> A single error reported by the platform stops all later traffic on a
> PCC channel whose client relies on the controller to complete
> transfers, such as the SCMI PCC transport. Every later request on the
> channel times out.
> 
> When the platform completes a command and sets a bit in the Error
> Status register, pcc_mbox_irq() acks the platform interrupt, clears the
> error and returns IRQ_NONE without calling mbox_chan_txdone().
> 
> No other interrupt arrives for that transfer, so the mailbox core keeps
> it as the active request and holds back every later message.
> 
> The chan_in_use flag also stays set, so on a shared interrupt line the
> next interrupt raised for another subspace passes the checks in
> pcc_mbox_irq(), since Command Complete is still set, and runs the
> success path on the failed transfer.
> 
> Log the error and fall through to the completion code, which clears
> chan_in_use before calling mbox_chan_txdone(). Skip
> mbox_chan_received_data() on error, as the shared memory holds no valid
> response, and complete the transfer with -EIO rather than the helper's
> return value, which is a positive acpi_status when the register read
> fails.
> 
> Falling through also runs pcc_chan_acknowledge() after an error. It
> only acts on responder subspaces, where the platform notification is
> now acknowledged but not passed to the client.
> 
> Clients that wait for the rx callback still time out on the failed
> transfer, but the channel works again afterwards.
> 

Reviewed-by: Sudeep Holla <sudeep.holla@kernel.org>

> Fixes: 9c753f7c953c ("mailbox: pcc: Mark Tx as complete in PCC IRQ handler")
> Cc: stable@vger.kernel.org
> Suggested-by: Michael van der Westhuizen <rmikey@meta.com>
> Signed-off-by: Breno Leitao <leitao@debian.org>
> ---
> Changes in v2:
> - Fall through to the completion code on error (Sudeep)
> - pcc_chan_acknowledge() now also runs after an error, which only
>   affects responder subspaces
> - Link to v1: https://patch.msgid.link/20261027-pcc_error_txdone-v1-1-2f8cfbb36d90@debian.org
> ---
>  drivers/mailbox/pcc.c | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/mailbox/pcc.c b/drivers/mailbox/pcc.c
> index 14dc4a187bfc67..0d36aa511a9d02 100644
> --- a/drivers/mailbox/pcc.c
> +++ b/drivers/mailbox/pcc.c
> @@ -315,6 +315,7 @@ static irqreturn_t pcc_mbox_irq(int irq, void *p)
>  {
>  	struct pcc_chan_info *pchan;
>  	struct mbox_chan *chan = p;
> +	int ret;
>  
>  	pchan = chan->con_priv;
>  
> @@ -333,8 +334,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))
> -		return IRQ_NONE;
> +	ret = pcc_mbox_error_check_and_clear(pchan);
> +	if (ret)
> +		dev_err_ratelimited(chan->mbox->dev,
> +				    "PCC subspace %td: transfer error %d\n",
> +				    pchan - chan_info, ret);
>  
>  	/*
>  	 * Clear this flag after updating the interrupt ack register and before
> @@ -343,8 +347,9 @@ static irqreturn_t pcc_mbox_irq(int irq, void *p)
>  	 * the lockless update observed by the send and interrupt paths.
>  	 */
>  	WRITE_ONCE(pchan->chan_in_use, false);
> -	mbox_chan_received_data(chan, NULL);
> -	mbox_chan_txdone(chan, 0);
> +	if (!ret)
> +		mbox_chan_received_data(chan, NULL);
> +	mbox_chan_txdone(chan, ret ? -EIO : 0);

This works as well, you can ignore my last suggestion.

-- 
Regards,
Sudeep