[PATCH v02] mailbox: pcc: report errors for PCC clients

[Sudeep Holla]

On Wed, May 20, 2026 at 07:53:45PM +0800, lihuisong (C) wrote:
> 
> On 5/20/2026 12:25 AM, Sudeep Holla wrote:
> > On Tue, May 19, 2026 at 09:54:47PM +0800, lihuisong (C) wrote:

[...]

> > > @Sudeep, I have always had doubts about the addition of this line of code in
> > > the
> > >   commit 9c753f7c953c (mailbox: pcc: Mark Tx as complete in PCC IRQ handler).
> > > The patch seems to avoid the timeouts in the mailbox core according to its
> > > commit log.
> > > Regardless of whether the command succeeds or fails, each mbox client
> > > driver, like cppc_acpi/acpi_pcc，kunpeng_hccs and so on, is responsible to
> > > call mbox_chan_txdone() to tell mailbox core.
> > Few controller drivers do have mbox_chan_txdone(), so Tx complete is detected
> Which controller driver?

git grep mbox_chan_txdone drivers/mailbox/

> > by PCC, so not sure why you think this is not the right place to do. The irq
> Because many mbox client drivers call mbox_chan_txdone() after running
> rx_callback() in mbox_chan_received_data().

OK, but why can't the controller hide that for the clients ? What am I missing?

> These drivers doesn't set chan->cl->tx_block to true.
> It seems that the client driver having tx_block need to set
> chan->tx_complete in tx_tick().
> Do you add this code for them?

I don't quite follow you.

> > is to indicate the completion. I am confused as why you think otherwise.
> > It is defined in include/linux/mailbox_controller.h for the same reason.
> > 
> > The client drivers can you mbox_client_txdone() if they wish to as defined
> > in include/linux/mailbox_client.h
> mbox_client_txdone() is used in the case that txdone_method is
> MBOX_TXDONE_BY_ACK.

Yes and agreed.

> And mbox clinte driver using IRQ method need to use mbox_chan_txdone().

Client doesn't handle IRQ its always controller driver and client must have
no business to do that IMO.

> It seems that all the current client drivers are used in this way.
> These interface internal would verify chan->txdone_method.
> 

Yes, sounds wrong to me.

drivers/acpi/acpi_pcc.c
drivers/acpi/cppc_acpi.c
drivers/hwmon/xgene-hwmon.c
drivers/i2c/busses/i2c-xgene-slimpro.c
drivers/soc/hisilicon/kunpeng_hccs.c

It is very clear from the code in mailbox.c, mbox_client_txdone() is for
the client drivers and mbox_chan_txdone() is for the controller. We need
to fix the above list but I need to check if there is anything I am missing
to understand first. Please let me know.

> In addition, I find that you also modify the txdone_irq/poll in the commit
> 3349f800609e (mailbox: pcc: Set txdone_irq/txdone_poll based on PCCT flags).
> The txdone_method will change from MBOX_TXDONE_BY_ACK to MBOX_TXDONE_BY_POLL
> on the platform using poll mode.
> This may lead to the original mbox client driver printing exceptions in
> mbox_client_txdone.
> I haven't observed it based on the latest code yet, it's just code analysis.

Right, I do remember seeing something and wonder if I moved to
mbox_chan_txdone() in drivers/acpi/acpi_pcc.c for that reason. But if the
expectations I have mentioned are correct, then we need to fix the framework
to avoid throwing that warnings.

-- 
Regards,
Sudeep