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@xxxxxxxxxx>

> Fixes: 9c753f7c953c ("mailbox: pcc: Mark Tx as complete in PCC IRQ handler")
> Cc: stable@xxxxxxxxxxxxxxx
> Suggested-by: Michael van der Westhuizen <rmikey@xxxxxxxx>
> Signed-off-by: Breno Leitao <leitao@xxxxxxxxxx>
> ---
> 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@xxxxxxxxxx
> ---
>  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