From: Sudeep Holla <sudeep.holla@kernel•org>
To: Adam Young <admiyo@os•amperecomputing.com>
Cc: Jassi Brar <jassisinghbrar@gmail•com>,
linux-kernel@vger•kernel.org,
Sudeep Holla <sudeep.holla@kernel•org>,
linux-hwmon@vger•kernel.org,
"Rafael J . Wysocki" <rafael@kernel•org>,
Len Brown <lenb@kernel•org>,
linux-acpi@vger•kernel.org, Andi Shyti <andi.shyti@kernel•org>,
Guenter Roeck <linux@roeck-us•net>,
Huisong Li <lihuisong@huawei•com>,
MyungJoo Ham <myungjoo.ham@samsung•com>,
Kyungmin Park <kyungmin.park@samsung•com>,
Chanwoo Choi <cw00.choi@samsung•com>,
linux-arm-kernel@lists•infradead.org
Subject: Re: [PATCH v02] mailbox: pcc: report errors for PCC clients
Date: Tue, 19 May 2026 14:23:59 +0100 [thread overview]
Message-ID: <20260519-inquisitive-teal-yak-56abd1@sudeepholla> (raw)
In-Reply-To: <20260518193006.27425-1-admiyo@os.amperecomputing.com>
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@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.
>
> /*
> * 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.
--
Regards,
Sudeep
next prev parent reply other threads:[~2026-05-19 13:24 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-05-18 19:30 [PATCH v02] mailbox: pcc: report errors for PCC clients Adam Young
2026-05-19 13:23 ` Sudeep Holla [this message]
2026-06-02 18:44 ` Adam Young
2026-06-03 15:15 ` Adam Young
2026-05-19 13:54 ` lihuisong (C)
2026-05-19 16:25 ` Sudeep Holla
2026-05-20 11:53 ` lihuisong (C)
2026-05-20 13:32 ` Sudeep Holla
2026-05-21 12:26 ` lihuisong (C)
2026-05-22 16:52 ` Adam Young
2026-05-26 3:53 ` lihuisong (C)
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260519-inquisitive-teal-yak-56abd1@sudeepholla \
--to=sudeep.holla@kernel$(echo .)org \
--cc=admiyo@os$(echo .)amperecomputing.com \
--cc=andi.shyti@kernel$(echo .)org \
--cc=cw00.choi@samsung$(echo .)com \
--cc=jassisinghbrar@gmail$(echo .)com \
--cc=kyungmin.park@samsung$(echo .)com \
--cc=lenb@kernel$(echo .)org \
--cc=lihuisong@huawei$(echo .)com \
--cc=linux-acpi@vger$(echo .)kernel.org \
--cc=linux-arm-kernel@lists$(echo .)infradead.org \
--cc=linux-hwmon@vger$(echo .)kernel.org \
--cc=linux-kernel@vger$(echo .)kernel.org \
--cc=linux@roeck-us$(echo .)net \
--cc=myungjoo.ham@samsung$(echo .)com \
--cc=rafael@kernel$(echo .)org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox