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

[Breno Leitao <leitao@debian.org>]

Hello Sudeep,

On Tue, Oct 27, 2026 at 04:47:39PM +0000, Sudeep Holla wrote:
> On Tue, Oct 27, 2026 at 08:19:14AM -0700, Breno Leitao wrote:
> > Clients that wait for the rx callback still time out on the failed
> > transfer, but the channel works again afterwards.
> > 
> 
> Nice, finally good to see this driver and various error paths being tested
> on real platforms. Thanks for finding the issues and the fix.

Cool, this will definitely get more eyes now on.

Michael deserves the credit for finding this and proposing this fix,
honestly. He is looking at all of this.

> > @@ -333,8 +334,20 @@ 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) {
> > +		/*
> > +		 * No other interrupt arrives for this transfer, so complete
> > +		 * it here or the channel stalls. Skip the rx callback, as the
> > +		 * shared memory holds no valid response.
> > +		 */
> > +		dev_err_ratelimited(chan->mbox->dev,
> > +				    "PCC subspace %td: transfer error %d\n",
> > +				    pchan - chan_info, ret);
> > +		WRITE_ONCE(pchan->chan_in_use, false);
> > +		mbox_chan_txdone(chan, -EIO);
> > +		return IRQ_HANDLED;
> 
> Since we are returning IRQ_HANDLED, can we merge this path with the code
> below ?
> 
> 	if (ret)
> 		dev_err_...
> 
> 	..
> 	if(!ret)
> 		mbox_chan_received_data();
> 
> That should work right ? The reason I prefer this way is the comment for
> WRITE_ONCE() applies above as well. Instead we can reuse the path. Let me
> know what you think.

Ack. Let me update and respin.

Thanks for the feedback,
--breno