Re: [PATCH 1/3] mailbox: pcc: Notify clients on polled completion

[Adam Young <adam@younglogic.com>]

On 7/23/26 15:45, Sudeep Holla wrote:
> On Thu, Jul 23, 2026 at 12:03:45PM -0400, Adam Young wrote:
>> On 7/17/26 03:56, Sudeep Holla wrote:
>>> PCC channels without a platform interrupt rely on the mailbox
>>> polling path to detect command completion.
>>>
>>> That path currently only reports transmit completion to the mailbox
>>> core, so clients that wait for their receive callback do not get
>>> notified when the command completes.
>>>
>>> Call mbox_chan_received_data() when polling observes completion on a
>>> channel without a platform IRQ, matching the interrupt-driven
>>> completion path.
>>>
>>> Reported-by: Cristian Marussi <cristian.marussi@arm.com>
>>> Acked-by: Huisong Li <lihuisong@huawei.com>
>>> Signed-off-by: Sudeep Holla <sudeep.holla@kernel.org>
>>> ---
>>>    drivers/mailbox/pcc.c | 10 +++++++++-
>>>    1 file changed, 9 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/mailbox/pcc.c b/drivers/mailbox/pcc.c
>>> index 636879ae1db7..d96b8b54e77e 100644
>>> --- a/drivers/mailbox/pcc.c
>>> +++ b/drivers/mailbox/pcc.c
>>> @@ -449,7 +449,15 @@ static bool pcc_last_tx_done(struct mbox_chan *chan)
>>>    {
>>>    	struct pcc_chan_info *pchan = chan->con_priv;
>>> -	return pcc_mbox_cmd_complete_check(pchan);
>>> +	if (!(chan->txdone_method & MBOX_TXDONE_BY_POLL))
>>> +		return false;
>>> +
>>> +	if (!pcc_mbox_cmd_complete_check(pchan))
>>> +		return false;
>>> +
>>> +	mbox_chan_received_data(chan, NULL);
>>> +
>>> +	return true;
>>>    }
>>>    /**
>> My code doesn't poll, so I cannot really claim to have tested it, but it
>> does not break the existing IRQ based driver mechanism.
>>
> Thanks for testing IRQ mode.
>
>> I can claim to have read through the code and confirm that it is comparable
>> to what happens on the IRQ case:   It clears the cmd_complete field and
>> calls mbox_chan_received_data.
> Thanks for taking a look and reviewing it.
>
>> However, it does not clear pchan->chan_in_use
>> = false; (Which should now be a WRITE_ONCE) and I would like to confirm that
>> is not an oversight before providing a reviewed-by tag.
>>
> IIRC I think we set the flag only when irq > 0, no ?

That is right.  So, this code is good.

Reviewed-by: Adam Young <admiyo@os.amperecomputing.com>




>