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

[sashiko-bot@kernel.org]

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