[PATCH v02] mailbox: pcc: report errors for PCC clients
Adam Young
admiyo at amperemail.onmicrosoft.com
Tue Jun 2 11:44:35 PDT 2026
I am geting ready to resubmit this patch along with a series of other
PCC based changes that show how it is necessary
for MCTP-PCC.
On 5/19/26 09:23, Sudeep Holla wrote:
> On Mon, May 18, 2026 at 03:30:06PM -0400, Adam Young wrote:
>> The tx_done callback function has a return code (rc) parameter
>> that the tx_done callback can use to determine how to handle an error.
>> However the IRQ handler was not setting that value if there is an error.
>>
>> The following clients are affected:
>>
>> drivers/acpi/cppc_acpi.c
>> drivers/i2c/busses/i2c-xgene-slimpro.c
>> drivers/hwmon/xgene-hwmon.c
>> drivers/soc/hisilicon/kunpeng_hccs.c
>> drivers/devfreq/hisi_uncore_freq.c
>>
>> All of these only use the error code to report, so they
>> are expecting an error code to come thorugh, but they
>> do not modify behavior based on this code.
>>
>> In the case of an error code in the IRQ, the handler was returning
>> IRQ_NONE which is not correct: the IRQ handler was matched
>> to the IRQ. This mean that multiple error codes returned from
>> a PCC triggered interrupt would end up disabling the device.
>>
>> In addition, if the error code IRQ was coming from a Type4 Device that was
>> expecting an IRQ response, that device would then be hung.
>>
>> Fixes: c45ded7e1135 ("mailbox: pcc: Add support for PCCT extended PCC subspaces(type 3/4)")
>> Signed-off-by: Adam Young <admiyo at os.amperecomputing.com>
>>
>> ---
>> ---
>> drivers/mailbox/pcc.c | 9 +++++----
>> 1 file changed, 5 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/mailbox/pcc.c b/drivers/mailbox/pcc.c
>> index 636879ae1db7..16b9ce087b9e 100644
>> --- a/drivers/mailbox/pcc.c
>> +++ b/drivers/mailbox/pcc.c
>> @@ -314,6 +314,7 @@ static irqreturn_t pcc_mbox_irq(int irq, void *p)
>> {
>> struct pcc_chan_info *pchan;
>> struct mbox_chan *chan = p;
>> + int rc;
>>
>> pchan = chan->con_priv;
>>
>> @@ -327,8 +328,7 @@ 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;
>> + rc = pcc_mbox_error_check_and_clear(pchan);
> I think we may have to skip the check inside pcc_mbox_error_check_and_clear()
> for Type 4 channel as the spec expects OSPM to ignore it. It is a separate
> fix, just noting that here.
I will submit this as a pre-req patch.
>
>>
>> /*
>> * Clear this flag after updating interrupt ack register and just
>> @@ -337,8 +337,9 @@ static irqreturn_t pcc_mbox_irq(int irq, void *p)
>> * required to avoid any possible race in updatation of this flag.
>> */
>> pchan->chan_in_use = false;
>> - mbox_chan_received_data(chan, NULL);
>> - mbox_chan_txdone(chan, 0);
>> + if (!rc)
>> + mbox_chan_received_data(chan, NULL);
> Not sure if making this conditional is good as some platforms may expect
> it to move the state machine, I am not sure 100% just thinking aloud here.
That is currently not the case. Right now, and error code
short-circuits and further execution of the IRC handler. I think that
those drivers would need to be changes to have a tx_done callback instead.
>
More information about the linux-arm-kernel
mailing list