Re: [PATCH v45 3/7] mailbox/pcc.c: report errors for PCC clients

On 7/22/26 05:07, Sudeep Holla wrote:

On Tue, Jul 21, 2026 at 01:52:52PM -0400, Adam Young wrote:

quoted

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.

I recall reviewing something similar and may have had comments, although I
cannot locate them at the moment, thanks to your random patch inclusion
exclusion scheme in this MCTP over PCC.

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

Could you please avoid continuing the MCTP-over-PCC series in its current
form? In the previous v44 revisions, PCC changes appeared to be removed in
some versions and replaced with unrelated changes in others, which has made
the series difficult to follow and review.

Please post PCC changes that are independent of MCTP as a separate series.
Where the MCTP implementation depends directly on those PCC changes, they may
be posted together. However, the current series appears to include several
unrelated cleanups described as part of the MCTP-over-PCC work.

To keep the review manageable, please separate those changes going forward. I
may need to NACK future revisions if unrelated PCC cleanups continue to be
bundled into this series. I will be away and would like to review any PCC
code, so your patience will be much appreciated.

Everything here is related to, and required by, MCTP over PCC.  I had 
posted some of the PCC patches in stand alone changes, but got feedback 
from the network  reviewers that MCTP over PCC could not go in until 
these were fixed.  MCTP over PCC seems to be one of the first drivers, 
if not the first to make use of the Type 4 interface, and getting fixes 
in place to make that work is prerequisite.

So, apologies for the earlier separate of patches.  I am trying to 
follow the guidelines for getting the patch into the kernel, but this 
spans two very different subsystems with different maintainers and 
guidelines. This is one of the reasons this patch series is at version 
45, and why I am still working on this 2+ years after the initial 
submission.

I am going to keep all of this work together.  It is the only way I can 
be sure that all of the work is reviewed end to end, and maintain any 
semblance of order in managing the patches.

The inclusion of AI in the review process has brought a sereis of new 
issues to light.  Before the kernel started producing automated AI 
reviews, the set of reviewers had reduced their concerns to fixes within 
the MCTP Driver.  That expanded earlier this year to numerous changes 
required in the PCC mailbox.  Some of those changes will have the added 
benefit of fixing corresponding bugs that have not been triggered 
elsewhere in the PCC drivers.  Her's a brief synopsis:

shmem map/unmap was added to the PCC layer for the MCTP Driver over a 
year ago.  It was based on a change I suggested for MCTP driver 
start/stop.  These operations are much more likely to be called in a 
network device that will likely be brought up and down on a running 
server, than most HW devices that are only started at machine bringup 
and stopped when the machine turns down.

The changes to  ignore errors on type 4 channels was based on your 
feedback on a different patch. MCTP over PCC will depend on the Type 4 
device functioning correctly, to include error reporting.

In the message Re: [PATCH v02] mailbox: pcc: report errors for PCC clients

The discussion  was:

quoted

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 think that should be in this patch, for correctness.  It is a small 
enough change.  I'll update.

Actually, it is a fix in its own right, and can be merged regardless of 
this patch, so:
https://lore.kernel.org/lkml/20260604163306.160017-1-admiyo@os.amperecomputing.com/ (local) 


Which got no feedback.


The query channel patch is required for MCTP.  While no other driver 
will need it yet, it does prevent a false-start situation that may help 
other drivers.

I did order the patches to post a couple fixes to PCC after the MCTP 
driver, as they are fixes that affect all drivers, under the general 
idea that general fixes for a subsystem should not hold up bugs in the 
subsystem.  Those are synchronize-IRQ-before-releasing-shared-memory and 
wrap-pchan-chan_in_use-in-READ-WRITE_ONCE.  Again, these were exposed by 
AI code review during the MCTP process, and me splitting them out 
elsewhere would really make it hard for me to track.  A network 
maintainer could argue that the sync IRQ fix really should go in before 
the MCTP driver.

So, while I do appreciate the additional load this puts on you, I think 
you will agree that this set of changes needs to be kept together for 
the larger reviewer community to be able to see.  It is hard  to make 
things easy for everyone.  I appreciate your patience and diligence in 
the review process. I hope you can get to these reviews before you 
disappear.