[PATCH 0/6] mailbox: pcc: Refactor and improve initialisation and interrupt handling

[PATCH 0/6] mailbox: pcc: Refactor and improve initialisation and interrupt handling

[Sudeep Holla]

This series refines and stabilizes the PCC mailbox driver to improve
initialisation order, interrupt handling, and completion signaling.

It begins by reverting a previous patch that introduced redundant shared
buffer management, simplifying the driver and restoring consistency with the
mailbox core framework. It is essential to have a proper baseline for the
main changes in the series.

Subsequent patches add proper completion reporting, clarify completion mode
selection, and fix subtle sequencing and interrupt issues to ensure
predictable, robust operation across ACPI-based PCC implementations.

Specifically, the series:

1. Removes redundant shared buffer logic, reverting an earlier change that
   duplicated existing mailbox core features and caused maintenance overhead.

2. Adds ->last_tx_done() support to allow polling clients to verify
   transmission completion without relying on interrupts.

3. Explicitly configures completion modes (txdone_irq or txdone_poll) based
   on ACPI PCCT doorbell capability flag, ensuring the correct completion
   mechanism is chosen for each platform.

4. Marks transmit completion in the IRQ handler by invoking mbox_chan_txdone(),
   preventing timeouts and ensuring proper synchronization for interrupt-driven
   transfers.

5. Initializes the shared memory region (SHMEM) before binding clients,
   preventing race conditions where clients could access uninitialized memory.

6. Clears any pending responder interrupts before enabling IRQs, avoiding
   spurious or false interrupts during startup.

Together, these updates make the PCC mailbox driver cleaner, more reliable,
and fully aligned with the mailbox framework's expectations, improving
determinism and robustness across different hardware platforms.

Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
Sudeep Holla (6):
      Revert "mailbox/pcc: support mailbox management of the shared buffer"
      mailbox: pcc: Wire up ->last_tx_done() for PCC channels
      mailbox: pcc: Set txdone_irq/txdone_poll based on PCCT flags
      mailbox: pcc: Mark Tx as complete in PCC IRQ handler
      mailbox: pcc: Initialize SHMEM before binding the channel with the client
      mailbox: pcc: Clear any pending responder interrupts before enabling it

 drivers/mailbox/pcc.c | 118 ++++++++++----------------------------------------
 include/acpi/pcc.h    |  29 -------------
 2 files changed, 23 insertions(+), 124 deletions(-)
---
base-commit: 7ea30958b3054f5e488fa0b33c352723f7ab3a2a
change-id: 20251016-pcc_mb_updates-d9d428985400


-- 
Regards,
Sudeep

[PATCH 1/6] Revert "mailbox/pcc: support mailbox management of the shared buffer"

[Sudeep Holla]

This reverts commit 5378bdf6a611a32500fccf13d14156f219bb0c85.

Commit 5378bdf6a611 ("mailbox/pcc: support mailbox management of the shared buffer")
attempted to introduce generic helpers for managing the PCC shared memory,
but it largely duplicates functionality already provided by the mailbox
core and leaves gaps:

1. TX preparation: The mailbox framework already supports this via
  ->tx_prepare callback for mailbox clients. The patch adds
  pcc_write_to_buffer() and expects clients to toggle pchan->chan.manage_writes,
  but no drivers set manage_writes, so pcc_write_to_buffer() has no users.

2. RX handling: Data reception is already delivered through
   mbox_chan_received_data() and client ->rx_callback. The patch adds an
   optional pchan->chan.rx_alloc, which again has no users and duplicates
   the existing path.

3. Completion handling: While adding last_tx_done is directionally useful,
   the implementation only covers Type 3/4 and fails to handle the absence
   of a command_complete register, so it is incomplete for other types.

Given the duplication and incomplete coverage, revert this change. Any new
requirements should be addressed in focused follow-ups rather than bundling
multiple behavioral changes together.

Fixes: 5378bdf6a611 ("mailbox/pcc: support mailbox management of the shared buffer")
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 drivers/mailbox/pcc.c | 102 ++------------------------------------------------
 include/acpi/pcc.h    |  29 --------------
 2 files changed, 4 insertions(+), 127 deletions(-)

diff --git a/drivers/mailbox/pcc.c b/drivers/mailbox/pcc.c
index 0a00719b2482..f6714c233f5a 100644
--- a/drivers/mailbox/pcc.c
+++ b/drivers/mailbox/pcc.c
@@ -306,22 +306,6 @@ static void pcc_chan_acknowledge(struct pcc_chan_info *pchan)
 		pcc_chan_reg_read_modify_write(&pchan->db);
 }
 
-static void *write_response(struct pcc_chan_info *pchan)
-{
-	struct pcc_header pcc_header;
-	void *buffer;
-	int data_len;
-
-	memcpy_fromio(&pcc_header, pchan->chan.shmem,
-		      sizeof(pcc_header));
-	data_len = pcc_header.length - sizeof(u32) + sizeof(struct pcc_header);
-
-	buffer = pchan->chan.rx_alloc(pchan->chan.mchan->cl, data_len);
-	if (buffer != NULL)
-		memcpy_fromio(buffer, pchan->chan.shmem, data_len);
-	return buffer;
-}
-
 /**
  * pcc_mbox_irq - PCC mailbox interrupt handler
  * @irq:	interrupt number
@@ -333,8 +317,6 @@ static irqreturn_t pcc_mbox_irq(int irq, void *p)
 {
 	struct pcc_chan_info *pchan;
 	struct mbox_chan *chan = p;
-	struct pcc_header *pcc_header = chan->active_req;
-	void *handle = NULL;
 
 	pchan = chan->con_priv;
 
@@ -358,17 +340,7 @@ 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;
-
-	if (pchan->chan.rx_alloc)
-		handle = write_response(pchan);
-
-	if (chan->active_req) {
-		pcc_header = chan->active_req;
-		if (pcc_header->flags & PCC_CMD_COMPLETION_NOTIFY)
-			mbox_chan_txdone(chan, 0);
-	}
-
-	mbox_chan_received_data(chan, handle);
+	mbox_chan_received_data(chan, NULL);
 
 	pcc_chan_acknowledge(pchan);
 
@@ -412,24 +384,9 @@ pcc_mbox_request_channel(struct mbox_client *cl, int subspace_id)
 	pcc_mchan = &pchan->chan;
 	pcc_mchan->shmem = acpi_os_ioremap(pcc_mchan->shmem_base_addr,
 					   pcc_mchan->shmem_size);
-	if (!pcc_mchan->shmem)
-		goto err;
-
-	pcc_mchan->manage_writes = false;
-
-	/* This indicates that the channel is ready to accept messages.
-	 * This needs to happen after the channel has registered
-	 * its callback. There is no access point to do that in
-	 * the mailbox API. That implies that the mailbox client must
-	 * have set the allocate callback function prior to
-	 * sending any messages.
-	 */
-	if (pchan->type == ACPI_PCCT_TYPE_EXT_PCC_SLAVE_SUBSPACE)
-		pcc_chan_reg_read_modify_write(&pchan->cmd_update);
-
-	return pcc_mchan;
+	if (pcc_mchan->shmem)
+		return pcc_mchan;
 
-err:
 	mbox_free_channel(chan);
 	return ERR_PTR(-ENXIO);
 }
@@ -460,38 +417,8 @@ void pcc_mbox_free_channel(struct pcc_mbox_chan *pchan)
 }
 EXPORT_SYMBOL_GPL(pcc_mbox_free_channel);
 
-static int pcc_write_to_buffer(struct mbox_chan *chan, void *data)
-{
-	struct pcc_chan_info *pchan = chan->con_priv;
-	struct pcc_mbox_chan *pcc_mbox_chan = &pchan->chan;
-	struct pcc_header *pcc_header = data;
-
-	if (!pchan->chan.manage_writes)
-		return 0;
-
-	/* The PCC header length includes the command field
-	 * but not the other values from the header.
-	 */
-	int len = pcc_header->length - sizeof(u32) + sizeof(struct pcc_header);
-	u64 val;
-
-	pcc_chan_reg_read(&pchan->cmd_complete, &val);
-	if (!val) {
-		pr_info("%s pchan->cmd_complete not set", __func__);
-		return -1;
-	}
-	memcpy_toio(pcc_mbox_chan->shmem,  data, len);
-	return 0;
-}
-
-
 /**
- * pcc_send_data - Called from Mailbox Controller code. If
- *		pchan->chan.rx_alloc is set, then the command complete
- *		flag is checked and the data is written to the shared
- *		buffer io memory.
- *
- *		If pchan->chan.rx_alloc is not set, then it is used
+ * pcc_send_data - Called from Mailbox Controller code. Used
  *		here only to ring the channel doorbell. The PCC client
  *		specific read/write is done in the client driver in
  *		order to maintain atomicity over PCC channel once
@@ -507,37 +434,17 @@ static int pcc_send_data(struct mbox_chan *chan, void *data)
 	int ret;
 	struct pcc_chan_info *pchan = chan->con_priv;
 
-	ret = pcc_write_to_buffer(chan, data);
-	if (ret)
-		return ret;
-
 	ret = pcc_chan_reg_read_modify_write(&pchan->cmd_update);
 	if (ret)
 		return ret;
 
 	ret = pcc_chan_reg_read_modify_write(&pchan->db);
-
 	if (!ret && pchan->plat_irq > 0)
 		pchan->chan_in_use = true;
 
 	return ret;
 }
 
-
-static bool pcc_last_tx_done(struct mbox_chan *chan)
-{
-	struct pcc_chan_info *pchan = chan->con_priv;
-	u64 val;
-
-	pcc_chan_reg_read(&pchan->cmd_complete, &val);
-	if (!val)
-		return false;
-	else
-		return true;
-}
-
-
-
 /**
  * pcc_startup - Called from Mailbox Controller code. Used here
  *		to request the interrupt.
@@ -583,7 +490,6 @@ static const struct mbox_chan_ops pcc_chan_ops = {
 	.send_data = pcc_send_data,
 	.startup = pcc_startup,
 	.shutdown = pcc_shutdown,
-	.last_tx_done = pcc_last_tx_done,
 };
 
 /**
diff --git a/include/acpi/pcc.h b/include/acpi/pcc.h
index 9af3b502f839..840bfc95bae3 100644
--- a/include/acpi/pcc.h
+++ b/include/acpi/pcc.h
@@ -17,35 +17,6 @@ struct pcc_mbox_chan {
 	u32 latency;
 	u32 max_access_rate;
 	u16 min_turnaround_time;
-
-	/* Set to true to indicate that the mailbox should manage
-	 * writing the dat to the shared buffer. This differs from
-	 * the case where the drivesr are writing to the buffer and
-	 * using send_data only to  ring the doorbell.  If this flag
-	 * is set, then the void * data parameter of send_data must
-	 * point to a kernel-memory buffer formatted in accordance with
-	 * the PCC specification.
-	 *
-	 * The active buffer management will include reading the
-	 * notify_on_completion flag, and will then
-	 * call mbox_chan_txdone when the acknowledgment interrupt is
-	 * received.
-	 */
-	bool manage_writes;
-
-	/* Optional callback that allows the driver
-	 * to allocate the memory used for receiving
-	 * messages.  The return value is the location
-	 * inside the buffer where the mailbox should write the data.
-	 */
-	void *(*rx_alloc)(struct mbox_client *cl,  int size);
-};
-
-struct pcc_header {
-	u32 signature;
-	u32 flags;
-	u32 length;
-	u32 command;
 };
 
 /* Generic Communications Channel Shared Memory Region */

-- 
2.34.1

Re: [PATCH 1/6] Revert "mailbox/pcc: support mailbox management of the shared buffer"

[Adam Young]

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

On 10/16/25 15:08, Sudeep Holla wrote:
> This reverts commit 5378bdf6a611a32500fccf13d14156f219bb0c85.
>
> Commit 5378bdf6a611 ("mailbox/pcc: support mailbox management of the shared buffer")
> attempted to introduce generic helpers for managing the PCC shared memory,
> but it largely duplicates functionality already provided by the mailbox
> core and leaves gaps:
>
> 1. TX preparation: The mailbox framework already supports this via
>    ->tx_prepare callback for mailbox clients. The patch adds
>    pcc_write_to_buffer() and expects clients to toggle pchan->chan.manage_writes,
>    but no drivers set manage_writes, so pcc_write_to_buffer() has no users.
>
> 2. RX handling: Data reception is already delivered through
>     mbox_chan_received_data() and client ->rx_callback. The patch adds an
>     optional pchan->chan.rx_alloc, which again has no users and duplicates
>     the existing path.
>
> 3. Completion handling: While adding last_tx_done is directionally useful,
>     the implementation only covers Type 3/4 and fails to handle the absence
>     of a command_complete register, so it is incomplete for other types.
>
> Given the duplication and incomplete coverage, revert this change. Any new
> requirements should be addressed in focused follow-ups rather than bundling
> multiple behavioral changes together.
>
> Fixes: 5378bdf6a611 ("mailbox/pcc: support mailbox management of the shared buffer")
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> ---
>   drivers/mailbox/pcc.c | 102 ++------------------------------------------------
>   include/acpi/pcc.h    |  29 --------------
>   2 files changed, 4 insertions(+), 127 deletions(-)
>
> diff --git a/drivers/mailbox/pcc.c b/drivers/mailbox/pcc.c
> index 0a00719b2482..f6714c233f5a 100644
> --- a/drivers/mailbox/pcc.c
> +++ b/drivers/mailbox/pcc.c
> @@ -306,22 +306,6 @@ static void pcc_chan_acknowledge(struct pcc_chan_info *pchan)
>   		pcc_chan_reg_read_modify_write(&pchan->db);
>   }
>   
> -static void *write_response(struct pcc_chan_info *pchan)
> -{
> -	struct pcc_header pcc_header;
> -	void *buffer;
> -	int data_len;
> -
> -	memcpy_fromio(&pcc_header, pchan->chan.shmem,
> -		      sizeof(pcc_header));
> -	data_len = pcc_header.length - sizeof(u32) + sizeof(struct pcc_header);
> -
> -	buffer = pchan->chan.rx_alloc(pchan->chan.mchan->cl, data_len);
> -	if (buffer != NULL)
> -		memcpy_fromio(buffer, pchan->chan.shmem, data_len);
> -	return buffer;
> -}
> -
>   /**
>    * pcc_mbox_irq - PCC mailbox interrupt handler
>    * @irq:	interrupt number
> @@ -333,8 +317,6 @@ static irqreturn_t pcc_mbox_irq(int irq, void *p)
>   {
>   	struct pcc_chan_info *pchan;
>   	struct mbox_chan *chan = p;
> -	struct pcc_header *pcc_header = chan->active_req;
> -	void *handle = NULL;
>   
>   	pchan = chan->con_priv;
>   
> @@ -358,17 +340,7 @@ 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;
> -
> -	if (pchan->chan.rx_alloc)
> -		handle = write_response(pchan);
> -
> -	if (chan->active_req) {
> -		pcc_header = chan->active_req;
> -		if (pcc_header->flags & PCC_CMD_COMPLETION_NOTIFY)
> -			mbox_chan_txdone(chan, 0);
> -	}
> -
> -	mbox_chan_received_data(chan, handle);
> +	mbox_chan_received_data(chan, NULL);
>   
>   	pcc_chan_acknowledge(pchan);
>   
> @@ -412,24 +384,9 @@ pcc_mbox_request_channel(struct mbox_client *cl, int subspace_id)
>   	pcc_mchan = &pchan->chan;
>   	pcc_mchan->shmem = acpi_os_ioremap(pcc_mchan->shmem_base_addr,
>   					   pcc_mchan->shmem_size);
> -	if (!pcc_mchan->shmem)
> -		goto err;
> -
> -	pcc_mchan->manage_writes = false;
> -
> -	/* This indicates that the channel is ready to accept messages.
> -	 * This needs to happen after the channel has registered
> -	 * its callback. There is no access point to do that in
> -	 * the mailbox API. That implies that the mailbox client must
> -	 * have set the allocate callback function prior to
> -	 * sending any messages.
> -	 */
> -	if (pchan->type == ACPI_PCCT_TYPE_EXT_PCC_SLAVE_SUBSPACE)
> -		pcc_chan_reg_read_modify_write(&pchan->cmd_update);
> -
> -	return pcc_mchan;
> +	if (pcc_mchan->shmem)
> +		return pcc_mchan;
>   
> -err:
>   	mbox_free_channel(chan);
>   	return ERR_PTR(-ENXIO);
>   }
> @@ -460,38 +417,8 @@ void pcc_mbox_free_channel(struct pcc_mbox_chan *pchan)
>   }
>   EXPORT_SYMBOL_GPL(pcc_mbox_free_channel);
>   
> -static int pcc_write_to_buffer(struct mbox_chan *chan, void *data)
> -{
> -	struct pcc_chan_info *pchan = chan->con_priv;
> -	struct pcc_mbox_chan *pcc_mbox_chan = &pchan->chan;
> -	struct pcc_header *pcc_header = data;
> -
> -	if (!pchan->chan.manage_writes)
> -		return 0;
> -
> -	/* The PCC header length includes the command field
> -	 * but not the other values from the header.
> -	 */
> -	int len = pcc_header->length - sizeof(u32) + sizeof(struct pcc_header);
> -	u64 val;
> -
> -	pcc_chan_reg_read(&pchan->cmd_complete, &val);
> -	if (!val) {
> -		pr_info("%s pchan->cmd_complete not set", __func__);
> -		return -1;
> -	}
> -	memcpy_toio(pcc_mbox_chan->shmem,  data, len);
> -	return 0;
> -}
> -
> -
>   /**
> - * pcc_send_data - Called from Mailbox Controller code. If
> - *		pchan->chan.rx_alloc is set, then the command complete
> - *		flag is checked and the data is written to the shared
> - *		buffer io memory.
> - *
> - *		If pchan->chan.rx_alloc is not set, then it is used
> + * pcc_send_data - Called from Mailbox Controller code. Used
>    *		here only to ring the channel doorbell. The PCC client
>    *		specific read/write is done in the client driver in
>    *		order to maintain atomicity over PCC channel once
> @@ -507,37 +434,17 @@ static int pcc_send_data(struct mbox_chan *chan, void *data)
>   	int ret;
>   	struct pcc_chan_info *pchan = chan->con_priv;
>   
> -	ret = pcc_write_to_buffer(chan, data);
> -	if (ret)
> -		return ret;
> -
>   	ret = pcc_chan_reg_read_modify_write(&pchan->cmd_update);
>   	if (ret)
>   		return ret;
>   
>   	ret = pcc_chan_reg_read_modify_write(&pchan->db);
> -
>   	if (!ret && pchan->plat_irq > 0)
>   		pchan->chan_in_use = true;
>   
>   	return ret;
>   }
>   
> -
> -static bool pcc_last_tx_done(struct mbox_chan *chan)
> -{
> -	struct pcc_chan_info *pchan = chan->con_priv;
> -	u64 val;
> -
> -	pcc_chan_reg_read(&pchan->cmd_complete, &val);
> -	if (!val)
> -		return false;
> -	else
> -		return true;
> -}
> -
> -
> -
>   /**
>    * pcc_startup - Called from Mailbox Controller code. Used here
>    *		to request the interrupt.
> @@ -583,7 +490,6 @@ static const struct mbox_chan_ops pcc_chan_ops = {
>   	.send_data = pcc_send_data,
>   	.startup = pcc_startup,
>   	.shutdown = pcc_shutdown,
> -	.last_tx_done = pcc_last_tx_done,
>   };
>   
>   /**
> diff --git a/include/acpi/pcc.h b/include/acpi/pcc.h
> index 9af3b502f839..840bfc95bae3 100644
> --- a/include/acpi/pcc.h
> +++ b/include/acpi/pcc.h
> @@ -17,35 +17,6 @@ struct pcc_mbox_chan {
>   	u32 latency;
>   	u32 max_access_rate;
>   	u16 min_turnaround_time;
> -
> -	/* Set to true to indicate that the mailbox should manage
> -	 * writing the dat to the shared buffer. This differs from
> -	 * the case where the drivesr are writing to the buffer and
> -	 * using send_data only to  ring the doorbell.  If this flag
> -	 * is set, then the void * data parameter of send_data must
> -	 * point to a kernel-memory buffer formatted in accordance with
> -	 * the PCC specification.
> -	 *
> -	 * The active buffer management will include reading the
> -	 * notify_on_completion flag, and will then
> -	 * call mbox_chan_txdone when the acknowledgment interrupt is
> -	 * received.
> -	 */
> -	bool manage_writes;
> -
> -	/* Optional callback that allows the driver
> -	 * to allocate the memory used for receiving
> -	 * messages.  The return value is the location
> -	 * inside the buffer where the mailbox should write the data.
> -	 */
> -	void *(*rx_alloc)(struct mbox_client *cl,  int size);
> -};
> -
> -struct pcc_header {
> -	u32 signature;
> -	u32 flags;
> -	u32 length;
> -	u32 command;
>   };
>   
>   /* Generic Communications Channel Shared Memory Region */
>

Re: [PATCH 1/6] Revert "mailbox/pcc: support mailbox management of the shared buffer"

[lihuisong (C)]

在 2025/10/17 3:08, Sudeep Holla 写道:
> This reverts commit 5378bdf6a611a32500fccf13d14156f219bb0c85.
>
> Commit 5378bdf6a611 ("mailbox/pcc: support mailbox management of the shared buffer")
> attempted to introduce generic helpers for managing the PCC shared memory,
> but it largely duplicates functionality already provided by the mailbox
> core and leaves gaps:
>
> 1. TX preparation: The mailbox framework already supports this via
>    ->tx_prepare callback for mailbox clients. The patch adds
>    pcc_write_to_buffer() and expects clients to toggle pchan->chan.manage_writes,
>    but no drivers set manage_writes, so pcc_write_to_buffer() has no users.
>
> 2. RX handling: Data reception is already delivered through
>     mbox_chan_received_data() and client ->rx_callback. The patch adds an
>     optional pchan->chan.rx_alloc, which again has no users and duplicates
>     the existing path.
>
> 3. Completion handling: While adding last_tx_done is directionally useful,
>     the implementation only covers Type 3/4 and fails to handle the absence
>     of a command_complete register, so it is incomplete for other types.
>
> Given the duplication and incomplete coverage, revert this change. Any new
> requirements should be addressed in focused follow-ups rather than bundling
> multiple behavioral changes together.
>
> Fixes: 5378bdf6a611 ("mailbox/pcc: support mailbox management of the shared buffer")
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
Yeah, there are some duplications. I remember I also mentioned before.
Acked-by: lihuisong@huawei.com

[PATCH 2/6] mailbox: pcc: Wire up ->last_tx_done() for PCC channels

[Sudeep Holla]

Some PCC users poll for completion between transfers and benefit from
the knowledge of previous Tx completion check through the mailbox
framework's ->last_tx_done() op.

Hook up the last_tx_done callback in the PCC mailbox driver so the mailbox
framework can correctly query the completion status of the last transmitted
message. This aligns PCC with other controllers that already implement such
last_tx_done status query.

No functional change unless callers use ->last_tx_done(). Normal Tx and
IRQ paths are unchanged. This change just improves synchronization and
avoids unnecessary timeouts for non-interrupt driven channels by ensuring
correct completion detection for PCC channels that don’t rely on interrupts.

Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 drivers/mailbox/pcc.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/mailbox/pcc.c b/drivers/mailbox/pcc.c
index f6714c233f5a..2b690c268cf0 100644
--- a/drivers/mailbox/pcc.c
+++ b/drivers/mailbox/pcc.c
@@ -445,6 +445,13 @@ static int pcc_send_data(struct mbox_chan *chan, void *data)
 	return ret;
 }
 
+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);
+}
+
 /**
  * pcc_startup - Called from Mailbox Controller code. Used here
  *		to request the interrupt.
@@ -490,6 +497,7 @@ static const struct mbox_chan_ops pcc_chan_ops = {
 	.send_data = pcc_send_data,
 	.startup = pcc_startup,
 	.shutdown = pcc_shutdown,
+	.last_tx_done = pcc_last_tx_done,
 };
 
 /**

-- 
2.34.1

Re: [PATCH 2/6] mailbox: pcc: Wire up ->last_tx_done() for PCC channels

[Adam Young]

Tested-by: Adam Young <admiyo@os.amperecompuing.com>

On 10/16/25 15:08, Sudeep Holla wrote:
> Some PCC users poll for completion between transfers and benefit from
> the knowledge of previous Tx completion check through the mailbox
> framework's ->last_tx_done() op.
>
> Hook up the last_tx_done callback in the PCC mailbox driver so the mailbox
> framework can correctly query the completion status of the last transmitted
> message. This aligns PCC with other controllers that already implement such
> last_tx_done status query.
>
> No functional change unless callers use ->last_tx_done(). Normal Tx and
> IRQ paths are unchanged. This change just improves synchronization and
> avoids unnecessary timeouts for non-interrupt driven channels by ensuring
> correct completion detection for PCC channels that don’t rely on interrupts.
>
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> ---
>   drivers/mailbox/pcc.c | 8 ++++++++
>   1 file changed, 8 insertions(+)
>
> diff --git a/drivers/mailbox/pcc.c b/drivers/mailbox/pcc.c
> index f6714c233f5a..2b690c268cf0 100644
> --- a/drivers/mailbox/pcc.c
> +++ b/drivers/mailbox/pcc.c
> @@ -445,6 +445,13 @@ static int pcc_send_data(struct mbox_chan *chan, void *data)
>   	return ret;
>   }
>   
> +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);
> +}
> +
>   /**
>    * pcc_startup - Called from Mailbox Controller code. Used here
>    *		to request the interrupt.
> @@ -490,6 +497,7 @@ static const struct mbox_chan_ops pcc_chan_ops = {
>   	.send_data = pcc_send_data,
>   	.startup = pcc_startup,
>   	.shutdown = pcc_shutdown,
> +	.last_tx_done = pcc_last_tx_done,
>   };
>   
>   /**
>

Re: [PATCH 2/6] mailbox: pcc: Wire up ->last_tx_done() for PCC channels

[Adam Young]

Correction.  I need to slow down.

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

On 10/17/25 12:48, Adam Young wrote:
> Tested-by: Adam Young <admiyo@os.amperecompuing.com>
>
> On 10/16/25 15:08, Sudeep Holla wrote:
>> Some PCC users poll for completion between transfers and benefit from
>> the knowledge of previous Tx completion check through the mailbox
>> framework's ->last_tx_done() op.
>>
>> Hook up the last_tx_done callback in the PCC mailbox driver so the 
>> mailbox
>> framework can correctly query the completion status of the last 
>> transmitted
>> message. This aligns PCC with other controllers that already 
>> implement such
>> last_tx_done status query.
>>
>> No functional change unless callers use ->last_tx_done(). Normal Tx and
>> IRQ paths are unchanged. This change just improves synchronization and
>> avoids unnecessary timeouts for non-interrupt driven channels by 
>> ensuring
>> correct completion detection for PCC channels that don’t rely on 
>> interrupts.
>>
>> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
>> ---
>>   drivers/mailbox/pcc.c | 8 ++++++++
>>   1 file changed, 8 insertions(+)
>>
>> diff --git a/drivers/mailbox/pcc.c b/drivers/mailbox/pcc.c
>> index f6714c233f5a..2b690c268cf0 100644
>> --- a/drivers/mailbox/pcc.c
>> +++ b/drivers/mailbox/pcc.c
>> @@ -445,6 +445,13 @@ static int pcc_send_data(struct mbox_chan *chan, 
>> void *data)
>>       return ret;
>>   }
>>   +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);
>> +}
>> +
>>   /**
>>    * pcc_startup - Called from Mailbox Controller code. Used here
>>    *        to request the interrupt.
>> @@ -490,6 +497,7 @@ static const struct mbox_chan_ops pcc_chan_ops = {
>>       .send_data = pcc_send_data,
>>       .startup = pcc_startup,
>>       .shutdown = pcc_shutdown,
>> +    .last_tx_done = pcc_last_tx_done,
>>   };
>>     /**
>>

Re: [PATCH 2/6] mailbox: pcc: Wire up ->last_tx_done() for PCC channels

[lihuisong (C)]

在 2025/10/17 3:08, Sudeep Holla 写道:
> Some PCC users poll for completion between transfers and benefit from
> the knowledge of previous Tx completion check through the mailbox
> framework's ->last_tx_done() op.
>
> Hook up the last_tx_done callback in the PCC mailbox driver so the mailbox
> framework can correctly query the completion status of the last transmitted
> message. This aligns PCC with other controllers that already implement such
> last_tx_done status query.
>
> No functional change unless callers use ->last_tx_done(). Normal Tx and
> IRQ paths are unchanged. This change just improves synchronization and
> avoids unnecessary timeouts for non-interrupt driven channels by ensuring
> correct completion detection for PCC channels that don’t rely on interrupts.
>
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> ---
>   drivers/mailbox/pcc.c | 8 ++++++++
>   1 file changed, 8 insertions(+)
>
> diff --git a/drivers/mailbox/pcc.c b/drivers/mailbox/pcc.c
> index f6714c233f5a..2b690c268cf0 100644
> --- a/drivers/mailbox/pcc.c
> +++ b/drivers/mailbox/pcc.c
> @@ -445,6 +445,13 @@ static int pcc_send_data(struct mbox_chan *chan, void *data)
>   	return ret;
>   }
>   
> +static bool pcc_last_tx_done(struct mbox_chan *chan)
> +{
> +	struct pcc_chan_info *pchan = chan->con_priv;
> +
The last_tx_done() is used on the condition that txdone_poll  is true 
and txdone_irq is false.
So how about add WARN_ON() for this in this API?
> +	return pcc_mbox_cmd_complete_check(pchan);
The pcc_mbox_cmd_complete_check() works on type 3/4.
I'm not sure if we need to add some comments or do some other something 
for this.
> +}
> +
>   /**
>    * pcc_startup - Called from Mailbox Controller code. Used here
>    *		to request the interrupt.
> @@ -490,6 +497,7 @@ static const struct mbox_chan_ops pcc_chan_ops = {
>   	.send_data = pcc_send_data,
>   	.startup = pcc_startup,
>   	.shutdown = pcc_shutdown,
> +	.last_tx_done = pcc_last_tx_done,
>   };
>   
>   /**
>

[PATCH 3/6] mailbox: pcc: Set txdone_irq/txdone_poll based on PCCT flags

[Sudeep Holla]

The PCC controller currently enables txdone via IRQ if the PCCT exposes
platform capability to generate command completion interrupt, but it
leaves txdone_poll unchanged. Make the behaviour explicit:

  - If ACPI_PCCT_DOORBELL is present, use txdone_irq and disable polling.
  - Otherwise, disable txdone_irq and fall back to txdone_poll.

Configure the PCC mailbox to use interrupt-based completion for PCC types
that signal completion via IRQ using TXDONE_BY_IRQ, and fall back to
polling for others using TXDONE_BY_POLL.

This ensures the PCC driver uses the appropriate completion mechanism
according to the PCCT table definition and makes the completion mode
unambiguous avoiding mixed signalling when the platform lacks a doorbell
flag set.

Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 drivers/mailbox/pcc.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/mailbox/pcc.c b/drivers/mailbox/pcc.c
index 2b690c268cf0..327e022973db 100644
--- a/drivers/mailbox/pcc.c
+++ b/drivers/mailbox/pcc.c
@@ -791,8 +791,13 @@ static int pcc_mbox_probe(struct platform_device *pdev)
 		(unsigned long) pcct_tbl + sizeof(struct acpi_table_pcct));
 
 	acpi_pcct_tbl = (struct acpi_table_pcct *) pcct_tbl;
-	if (acpi_pcct_tbl->flags & ACPI_PCCT_DOORBELL)
+	if (acpi_pcct_tbl->flags & ACPI_PCCT_DOORBELL) {
 		pcc_mbox_ctrl->txdone_irq = true;
+		pcc_mbox_ctrl->txdone_poll = false;
+	} else {
+		pcc_mbox_ctrl->txdone_irq = false;
+		pcc_mbox_ctrl->txdone_poll = true;
+	}
 
 	for (i = 0; i < count; i++) {
 		struct pcc_chan_info *pchan = chan_info + i;

-- 
2.34.1

Re: [PATCH 3/6] mailbox: pcc: Set txdone_irq/txdone_poll based on PCCT flags

[Adam Young]

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

On 10/16/25 15:08, Sudeep Holla wrote:
> The PCC controller currently enables txdone via IRQ if the PCCT exposes
> platform capability to generate command completion interrupt, but it
> leaves txdone_poll unchanged. Make the behaviour explicit:
>
>    - If ACPI_PCCT_DOORBELL is present, use txdone_irq and disable polling.
>    - Otherwise, disable txdone_irq and fall back to txdone_poll.
>
> Configure the PCC mailbox to use interrupt-based completion for PCC types
> that signal completion via IRQ using TXDONE_BY_IRQ, and fall back to
> polling for others using TXDONE_BY_POLL.
>
> This ensures the PCC driver uses the appropriate completion mechanism
> according to the PCCT table definition and makes the completion mode
> unambiguous avoiding mixed signalling when the platform lacks a doorbell
> flag set.
>
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> ---
>   drivers/mailbox/pcc.c | 7 ++++++-
>   1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mailbox/pcc.c b/drivers/mailbox/pcc.c
> index 2b690c268cf0..327e022973db 100644
> --- a/drivers/mailbox/pcc.c
> +++ b/drivers/mailbox/pcc.c
> @@ -791,8 +791,13 @@ static int pcc_mbox_probe(struct platform_device *pdev)
>   		(unsigned long) pcct_tbl + sizeof(struct acpi_table_pcct));
>   
>   	acpi_pcct_tbl = (struct acpi_table_pcct *) pcct_tbl;
> -	if (acpi_pcct_tbl->flags & ACPI_PCCT_DOORBELL)
> +	if (acpi_pcct_tbl->flags & ACPI_PCCT_DOORBELL) {
>   		pcc_mbox_ctrl->txdone_irq = true;
> +		pcc_mbox_ctrl->txdone_poll = false;
> +	} else {
> +		pcc_mbox_ctrl->txdone_irq = false;
> +		pcc_mbox_ctrl->txdone_poll = true;
> +	}
>   
>   	for (i = 0; i < count; i++) {
>   		struct pcc_chan_info *pchan = chan_info + i;
>

Re: [PATCH 3/6] mailbox: pcc: Set txdone_irq/txdone_poll based on PCCT flags

[lihuisong (C)]

在 2025/10/17 3:08, Sudeep Holla 写道:
> The PCC controller currently enables txdone via IRQ if the PCCT exposes
> platform capability to generate command completion interrupt, but it
> leaves txdone_poll unchanged. Make the behaviour explicit:
>
>    - If ACPI_PCCT_DOORBELL is present, use txdone_irq and disable polling.
>    - Otherwise, disable txdone_irq and fall back to txdone_poll.
>
> Configure the PCC mailbox to use interrupt-based completion for PCC types
> that signal completion via IRQ using TXDONE_BY_IRQ, and fall back to
> polling for others using TXDONE_BY_POLL.
>
> This ensures the PCC driver uses the appropriate completion mechanism
> according to the PCCT table definition and makes the completion mode
> unambiguous avoiding mixed signalling when the platform lacks a doorbell
> flag set.
>
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> ---
LGTM,
Acked-by: lihuisong@huawei.com
>

[PATCH 4/6] mailbox: pcc: Mark Tx as complete in PCC IRQ handler

[Sudeep Holla]

The PCC IRQ handler clears channel-in-use and notifies clients with
mbox_chan_received_data(), but it does not explicitly mark the
transmit as complete. In IRQ completion mode this could leave Tx complete
waiters hanging or lead to generic timeouts in the mailbox core.

Invoke mbox_chan_txdone() in the IRQ path once the platform has
acknowledged the transfer so the core can wake any waiters and update
state accordingly.

Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 drivers/mailbox/pcc.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/mailbox/pcc.c b/drivers/mailbox/pcc.c
index 327e022973db..33bd2d05704b 100644
--- a/drivers/mailbox/pcc.c
+++ b/drivers/mailbox/pcc.c
@@ -341,6 +341,7 @@ static irqreturn_t pcc_mbox_irq(int irq, void *p)
 	 */
 	pchan->chan_in_use = false;
 	mbox_chan_received_data(chan, NULL);
+	mbox_chan_txdone(chan, 0);
 
 	pcc_chan_acknowledge(pchan);
 

-- 
2.34.1

Re: [PATCH 4/6] mailbox: pcc: Mark Tx as complete in PCC IRQ handler

[Adam Young]

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

On 10/16/25 15:08, Sudeep Holla wrote:
> The PCC IRQ handler clears channel-in-use and notifies clients with
> mbox_chan_received_data(), but it does not explicitly mark the
> transmit as complete. In IRQ completion mode this could leave Tx complete
> waiters hanging or lead to generic timeouts in the mailbox core.
>
> Invoke mbox_chan_txdone() in the IRQ path once the platform has
> acknowledged the transfer so the core can wake any waiters and update
> state accordingly.
>
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> ---
>   drivers/mailbox/pcc.c | 1 +
>   1 file changed, 1 insertion(+)
>
> diff --git a/drivers/mailbox/pcc.c b/drivers/mailbox/pcc.c
> index 327e022973db..33bd2d05704b 100644
> --- a/drivers/mailbox/pcc.c
> +++ b/drivers/mailbox/pcc.c
> @@ -341,6 +341,7 @@ static irqreturn_t pcc_mbox_irq(int irq, void *p)
>   	 */
>   	pchan->chan_in_use = false;
>   	mbox_chan_received_data(chan, NULL);
> +	mbox_chan_txdone(chan, 0);
>   
>   	pcc_chan_acknowledge(pchan);
>   
>

Re: [PATCH 4/6] mailbox: pcc: Mark Tx as complete in PCC IRQ handler

[lihuisong (C)]

在 2025/10/17 3:08, Sudeep Holla 写道:
> The PCC IRQ handler clears channel-in-use and notifies clients with
> mbox_chan_received_data(), but it does not explicitly mark the
> transmit as complete. In IRQ completion mode this could leave Tx complete
> waiters hanging or lead to generic timeouts in the mailbox core.
>
> Invoke mbox_chan_txdone() in the IRQ path once the platform has
> acknowledged the transfer so the core can wake any waiters and update
> state accordingly.
>
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> ---
>   drivers/mailbox/pcc.c | 1 +
>   1 file changed, 1 insertion(+)
>
> diff --git a/drivers/mailbox/pcc.c b/drivers/mailbox/pcc.c
> index 327e022973db..33bd2d05704b 100644
> --- a/drivers/mailbox/pcc.c
> +++ b/drivers/mailbox/pcc.c
> @@ -341,6 +341,7 @@ static irqreturn_t pcc_mbox_irq(int irq, void *p)
>   	 */
>   	pchan->chan_in_use = false;
>   	mbox_chan_received_data(chan, NULL);
> +	mbox_chan_txdone(chan, 0);
Normally, this interface is called by mbox client.
So, IMO, this added code is not ok for mbox client.
AFAIS, this code should be for type4, right?
If so, the mbox client of type 4 channel has  responsibility to call 
this interface.
>   
>   	pcc_chan_acknowledge(pchan);
>   
>

[PATCH 5/6] mailbox: pcc: Initialize SHMEM before binding the channel with the client

[Sudeep Holla]

The PCC channel's shared memory region must be set up before the
mailbox controller binds the channel with the client, as the binding
process may trigger client operations like startup() that may rely on
SHMEM being initialized.

Reorder the setup sequence to ensure the shared memory is ready before
binding. Initialize and map the PCC shared memory (SHMEM) prior to
calling mbox_bind_client() so that clients never observe an uninitialized
or NULL SHMEM during bind-time callbacks or early use in startup().

This makes the PCC mailbox channel bring-up order consistent and
eliminates a race between SHMEM setup and client binding.

This will be needed in channel startup to clear/acknowledge any pending
interrupts before enabling them.

Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 drivers/mailbox/pcc.c | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/drivers/mailbox/pcc.c b/drivers/mailbox/pcc.c
index 33bd2d05704b..2829ec51b47f 100644
--- a/drivers/mailbox/pcc.c
+++ b/drivers/mailbox/pcc.c
@@ -378,18 +378,20 @@ pcc_mbox_request_channel(struct mbox_client *cl, int subspace_id)
 		return ERR_PTR(-EBUSY);
 	}
 
-	rc = mbox_bind_client(chan, cl);
-	if (rc)
-		return ERR_PTR(rc);
-
 	pcc_mchan = &pchan->chan;
 	pcc_mchan->shmem = acpi_os_ioremap(pcc_mchan->shmem_base_addr,
 					   pcc_mchan->shmem_size);
-	if (pcc_mchan->shmem)
-		return pcc_mchan;
+	if (!pcc_mchan->shmem)
+		return ERR_PTR(-ENXIO);
 
-	mbox_free_channel(chan);
-	return ERR_PTR(-ENXIO);
+	rc = mbox_bind_client(chan, cl);
+	if (rc) {
+		iounmap(pcc_mchan->shmem);
+		pcc_mchan->shmem = NULL;
+		return ERR_PTR(rc);
+	}
+
+	return pcc_mchan;
 }
 EXPORT_SYMBOL_GPL(pcc_mbox_request_channel);
 

-- 
2.34.1

Re: [PATCH 5/6] mailbox: pcc: Initialize SHMEM before binding the channel with the client

[Adam Young]

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

On 10/16/25 15:08, Sudeep Holla wrote:
> The PCC channel's shared memory region must be set up before the
> mailbox controller binds the channel with the client, as the binding
> process may trigger client operations like startup() that may rely on
> SHMEM being initialized.
>
> Reorder the setup sequence to ensure the shared memory is ready before
> binding. Initialize and map the PCC shared memory (SHMEM) prior to
> calling mbox_bind_client() so that clients never observe an uninitialized
> or NULL SHMEM during bind-time callbacks or early use in startup().
>
> This makes the PCC mailbox channel bring-up order consistent and
> eliminates a race between SHMEM setup and client binding.
>
> This will be needed in channel startup to clear/acknowledge any pending
> interrupts before enabling them.
>
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> ---
>   drivers/mailbox/pcc.c | 18 ++++++++++--------
>   1 file changed, 10 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/mailbox/pcc.c b/drivers/mailbox/pcc.c
> index 33bd2d05704b..2829ec51b47f 100644
> --- a/drivers/mailbox/pcc.c
> +++ b/drivers/mailbox/pcc.c
> @@ -378,18 +378,20 @@ pcc_mbox_request_channel(struct mbox_client *cl, int subspace_id)
>   		return ERR_PTR(-EBUSY);
>   	}
>   
> -	rc = mbox_bind_client(chan, cl);
> -	if (rc)
> -		return ERR_PTR(rc);
> -
>   	pcc_mchan = &pchan->chan;
>   	pcc_mchan->shmem = acpi_os_ioremap(pcc_mchan->shmem_base_addr,
>   					   pcc_mchan->shmem_size);
> -	if (pcc_mchan->shmem)
> -		return pcc_mchan;
> +	if (!pcc_mchan->shmem)
> +		return ERR_PTR(-ENXIO);
>   
> -	mbox_free_channel(chan);
> -	return ERR_PTR(-ENXIO);
> +	rc = mbox_bind_client(chan, cl);
> +	if (rc) {
> +		iounmap(pcc_mchan->shmem);
> +		pcc_mchan->shmem = NULL;
> +		return ERR_PTR(rc);
> +	}
> +
> +	return pcc_mchan;
>   }
>   EXPORT_SYMBOL_GPL(pcc_mbox_request_channel);
>   
>

Re: [PATCH 5/6] mailbox: pcc: Initialize SHMEM before binding the channel with the client

[lihuisong (C)]

在 2025/10/17 3:08, Sudeep Holla 写道:
> The PCC channel's shared memory region must be set up before the
> mailbox controller binds the channel with the client, as the binding
> process may trigger client operations like startup() that may rely on
> SHMEM being initialized.
>
> Reorder the setup sequence to ensure the shared memory is ready before
> binding. Initialize and map the PCC shared memory (SHMEM) prior to
> calling mbox_bind_client() so that clients never observe an uninitialized
> or NULL SHMEM during bind-time callbacks or early use in startup().
>
> This makes the PCC mailbox channel bring-up order consistent and
> eliminates a race between SHMEM setup and client binding.

I don't think this race exists. The above reason is enough.

This patch should be for patch 6/6, right?

>
> This will be needed in channel startup to clear/acknowledge any pending
> interrupts before enabling them.
>
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
Acked-by: lihuisong@huawei.com
> ---
>   drivers/mailbox/pcc.c | 18 ++++++++++--------
>   1 file changed, 10 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/mailbox/pcc.c b/drivers/mailbox/pcc.c
> index 33bd2d05704b..2829ec51b47f 100644
> --- a/drivers/mailbox/pcc.c
> +++ b/drivers/mailbox/pcc.c
> @@ -378,18 +378,20 @@ pcc_mbox_request_channel(struct mbox_client *cl, int subspace_id)

>

[PATCH 6/6] mailbox: pcc: Clear any pending responder interrupts before enabling it

[Sudeep Holla]

Some platforms may leave a responder interrupt pending from earlier
transactions. If a PCC responder channel has a pending interrupt when
the controller starts up, enabling the IRQ line without first clearing
the condition can lead to a spurious interrupt which could disrupt other
transmissions if the IRQ is shared.

Explicitly clear any pending responder interrupt before enabling the IRQ
to ensure a clean start. Acknowledge the responder channel via
pcc_chan_acknowledge() in startup before requesting/enablement of the
IRQ. This ensures a clean baseline for the first transfer/receiption
of the notification/response.

Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 drivers/mailbox/pcc.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/mailbox/pcc.c b/drivers/mailbox/pcc.c
index 2829ec51b47f..418007020439 100644
--- a/drivers/mailbox/pcc.c
+++ b/drivers/mailbox/pcc.c
@@ -468,6 +468,12 @@ static int pcc_startup(struct mbox_chan *chan)
 	unsigned long irqflags;
 	int rc;
 
+	/*
+	 * Clear and acknowledge any pending interrupts on responder channel
+	 * before enabling the interrupt
+	 */
+	pcc_chan_acknowledge(pchan);
+
 	if (pchan->plat_irq > 0) {
 		irqflags = pcc_chan_plat_irq_can_be_shared(pchan) ?
 						IRQF_SHARED | IRQF_ONESHOT : 0;

-- 
2.34.1

Re: [PATCH 6/6] mailbox: pcc: Clear any pending responder interrupts before enabling it

[Adam Young]

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

On 10/16/25 15:08, Sudeep Holla wrote:
> Some platforms may leave a responder interrupt pending from earlier
> transactions. If a PCC responder channel has a pending interrupt when
> the controller starts up, enabling the IRQ line without first clearing
> the condition can lead to a spurious interrupt which could disrupt other
> transmissions if the IRQ is shared.
>
> Explicitly clear any pending responder interrupt before enabling the IRQ
> to ensure a clean start. Acknowledge the responder channel via
> pcc_chan_acknowledge() in startup before requesting/enablement of the
> IRQ. This ensures a clean baseline for the first transfer/receiption
> of the notification/response.
>
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> ---
>   drivers/mailbox/pcc.c | 6 ++++++
>   1 file changed, 6 insertions(+)
>
> diff --git a/drivers/mailbox/pcc.c b/drivers/mailbox/pcc.c
> index 2829ec51b47f..418007020439 100644
> --- a/drivers/mailbox/pcc.c
> +++ b/drivers/mailbox/pcc.c
> @@ -468,6 +468,12 @@ static int pcc_startup(struct mbox_chan *chan)
>   	unsigned long irqflags;
>   	int rc;
>   
> +	/*
> +	 * Clear and acknowledge any pending interrupts on responder channel
> +	 * before enabling the interrupt
> +	 */
> +	pcc_chan_acknowledge(pchan);
> +
>   	if (pchan->plat_irq > 0) {
>   		irqflags = pcc_chan_plat_irq_can_be_shared(pchan) ?
>   						IRQF_SHARED | IRQF_ONESHOT : 0;
>

Re: [PATCH 0/6] mailbox: pcc: Refactor and improve initialisation and interrupt handling

[Adam Young]

Tested-by: Adam Young <admiyo@os.amperecompuitng.com>

On 10/16/25 15:08, Sudeep Holla wrote:
> This series refines and stabilizes the PCC mailbox driver to improve
> initialisation order, interrupt handling, and completion signaling.
>
> It begins by reverting a previous patch that introduced redundant shared
> buffer management, simplifying the driver and restoring consistency with the
> mailbox core framework. It is essential to have a proper baseline for the
> main changes in the series.
>
> Subsequent patches add proper completion reporting, clarify completion mode
> selection, and fix subtle sequencing and interrupt issues to ensure
> predictable, robust operation across ACPI-based PCC implementations.
>
> Specifically, the series:
>
> 1. Removes redundant shared buffer logic, reverting an earlier change that
>     duplicated existing mailbox core features and caused maintenance overhead.
>
> 2. Adds ->last_tx_done() support to allow polling clients to verify
>     transmission completion without relying on interrupts.
>
> 3. Explicitly configures completion modes (txdone_irq or txdone_poll) based
>     on ACPI PCCT doorbell capability flag, ensuring the correct completion
>     mechanism is chosen for each platform.
>
> 4. Marks transmit completion in the IRQ handler by invoking mbox_chan_txdone(),
>     preventing timeouts and ensuring proper synchronization for interrupt-driven
>     transfers.
>
> 5. Initializes the shared memory region (SHMEM) before binding clients,
>     preventing race conditions where clients could access uninitialized memory.
>
> 6. Clears any pending responder interrupts before enabling IRQs, avoiding
>     spurious or false interrupts during startup.
>
> Together, these updates make the PCC mailbox driver cleaner, more reliable,
> and fully aligned with the mailbox framework's expectations, improving
> determinism and robustness across different hardware platforms.
>
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> ---
> Sudeep Holla (6):
>        Revert "mailbox/pcc: support mailbox management of the shared buffer"
>        mailbox: pcc: Wire up ->last_tx_done() for PCC channels
>        mailbox: pcc: Set txdone_irq/txdone_poll based on PCCT flags
>        mailbox: pcc: Mark Tx as complete in PCC IRQ handler
>        mailbox: pcc: Initialize SHMEM before binding the channel with the client
>        mailbox: pcc: Clear any pending responder interrupts before enabling it
>
>   drivers/mailbox/pcc.c | 118 ++++++++++----------------------------------------
>   include/acpi/pcc.h    |  29 -------------
>   2 files changed, 23 insertions(+), 124 deletions(-)
> ---
> base-commit: 7ea30958b3054f5e488fa0b33c352723f7ab3a2a
> change-id: 20251016-pcc_mb_updates-d9d428985400
>
>

Re: [PATCH 0/6] mailbox: pcc: Refactor and improve initialisation and interrupt handling

[Sudeep Holla]

Hi Jassi,

On Thu, Oct 16, 2025 at 08:08:14PM +0100, Sudeep Holla wrote:
> This series refines and stabilizes the PCC mailbox driver to improve
> initialisation order, interrupt handling, and completion signaling.
> 
Are you happy to pull these patches directly from the list or do you
prefer me to send you pull request or do you want me to direct this via
ACPI/Rafael's tree. Please advice.

> It begins by reverting a previous patch that introduced redundant shared
> buffer management, simplifying the driver and restoring consistency with the
> mailbox core framework. It is essential to have a proper baseline for the
> main changes in the series.
> 
> Subsequent patches add proper completion reporting, clarify completion mode
> selection, and fix subtle sequencing and interrupt issues to ensure
> predictable, robust operation across ACPI-based PCC implementations.
> 
> Specifically, the series:
> 
> 1. Removes redundant shared buffer logic, reverting an earlier change that
>    duplicated existing mailbox core features and caused maintenance overhead.
> 
> 2. Adds ->last_tx_done() support to allow polling clients to verify
>    transmission completion without relying on interrupts.
> 
> 3. Explicitly configures completion modes (txdone_irq or txdone_poll) based
>    on ACPI PCCT doorbell capability flag, ensuring the correct completion
>    mechanism is chosen for each platform.
> 
> 4. Marks transmit completion in the IRQ handler by invoking mbox_chan_txdone(),
>    preventing timeouts and ensuring proper synchronization for interrupt-driven
>    transfers.
> 
> 5. Initializes the shared memory region (SHMEM) before binding clients,
>    preventing race conditions where clients could access uninitialized memory.
> 
> 6. Clears any pending responder interrupts before enabling IRQs, avoiding
>    spurious or false interrupts during startup.
> 
> Together, these updates make the PCC mailbox driver cleaner, more reliable,
> and fully aligned with the mailbox framework's expectations, improving
> determinism and robustness across different hardware platforms.
> 
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> ---
> Sudeep Holla (6):
>       Revert "mailbox/pcc: support mailbox management of the shared buffer"
>       mailbox: pcc: Wire up ->last_tx_done() for PCC channels
>       mailbox: pcc: Set txdone_irq/txdone_poll based on PCCT flags
>       mailbox: pcc: Mark Tx as complete in PCC IRQ handler
>       mailbox: pcc: Initialize SHMEM before binding the channel with the client
>       mailbox: pcc: Clear any pending responder interrupts before enabling it
> 
>  drivers/mailbox/pcc.c | 118 ++++++++++----------------------------------------
>  include/acpi/pcc.h    |  29 -------------
>  2 files changed, 23 insertions(+), 124 deletions(-)
> ---
> base-commit: 7ea30958b3054f5e488fa0b33c352723f7ab3a2a
> change-id: 20251016-pcc_mb_updates-d9d428985400
> 
> 
> -- 
> Regards,
> Sudeep
> 

-- 
Regards,
Sudeep

Re: [PATCH 0/6] mailbox: pcc: Refactor and improve initialisation and interrupt handling

[Sudeep Holla]

On Thu, Nov 27, 2025 at 02:40:56PM +0000, Sudeep Holla wrote:
> Hi Jassi,
> 
> On Thu, Oct 16, 2025 at 08:08:14PM +0100, Sudeep Holla wrote:
> > This series refines and stabilizes the PCC mailbox driver to improve
> > initialisation order, interrupt handling, and completion signaling.
> > 
> Are you happy to pull these patches directly from the list or do you
> prefer me to send you pull request or do you want me to direct this via
> ACPI/Rafael's tree. Please advice.
> 

Hi Jassi,

Sorry for the nag. I did see these patches in -next as well as your
v6.19 merge window pull request which didn't make it to Linus tree.
However I don't see it -next any longer. Please advice if you want
anything from my side so that this can be merged for v6.20/v7.0

-- 
Regards,
Sudeep

Re: [PATCH 0/6] mailbox: pcc: Refactor and improve initialisation and interrupt handling

[Adam Young]

On 1/12/26 11:55, Sudeep Holla wrote:
> On Thu, Nov 27, 2025 at 02:40:56PM +0000, Sudeep Holla wrote:
>> Hi Jassi,
>>
>> On Thu, Oct 16, 2025 at 08:08:14PM +0100, Sudeep Holla wrote:
>>> This series refines and stabilizes the PCC mailbox driver to improve
>>> initialisation order, interrupt handling, and completion signaling.
>>>
>> Are you happy to pull these patches directly from the list or do you
>> prefer me to send you pull request or do you want me to direct this via
>> ACPI/Rafael's tree. Please advice.
>>
> Hi Jassi,
>
> Sorry for the nag. I did see these patches in -next as well as your
> v6.19 merge window pull request which didn't make it to Linus tree.
> However I don't see it -next any longer. Please advice if you want
> anything from my side so that this can be merged for v6.20/v7.0
>

I thought you had an approach you wanted to implement for the functions 
that provided access to the Mailbox internals: you wanted to do them 
inline but hadn't gotten to them yet.  Is that still the case?  I will 
resubmit mine as is with -next if that is acceptable.

Re: [PATCH 0/6] mailbox: pcc: Refactor and improve initialisation and interrupt handling

[Adam Young]

On 1/26/26 12:07, Adam Young wrote:
>
>
> On 1/12/26 11:55, Sudeep Holla wrote:
>> On Thu, Nov 27, 2025 at 02:40:56PM +0000, Sudeep Holla wrote:
>>> Hi Jassi,
>>>
>>> On Thu, Oct 16, 2025 at 08:08:14PM +0100, Sudeep Holla wrote:
>>>> This series refines and stabilizes the PCC mailbox driver to improve
>>>> initialisation order, interrupt handling, and completion signaling.
>>>>
>>> Are you happy to pull these patches directly from the list or do you
>>> prefer me to send you pull request or do you want me to direct this via
>>> ACPI/Rafael's tree. Please advice.
>>>
>> Hi Jassi,
>>
>> Sorry for the nag. I did see these patches in -next as well as your
>> v6.19 merge window pull request which didn't make it to Linus tree.
>> However I don't see it -next any longer. Please advice if you want
>> anything from my side so that this can be merged for v6.20/v7.0
>>
>
> I thought you had an approach you wanted to implement for the 
> functions that provided access to the Mailbox internals: you wanted to 
> do them inline but hadn't gotten to them yet.  Is that still the 
> case?  I will resubmit mine as is with -next if that is acceptable.
>
Apologies, I realize now that this was about the previous set of 
patches, and not the ones that Sudeep and I were discussing that depend 
on them.

Re: [PATCH 0/6] mailbox: pcc: Refactor and improve initialisation and interrupt handling

[Sudeep Holla]

On Mon, Jan 26, 2026 at 12:07:26PM -0500, Adam Young wrote:
> 
> 
> On 1/12/26 11:55, Sudeep Holla wrote:
> > On Thu, Nov 27, 2025 at 02:40:56PM +0000, Sudeep Holla wrote:
> > > Hi Jassi,
> > > 
> > > On Thu, Oct 16, 2025 at 08:08:14PM +0100, Sudeep Holla wrote:
> > > > This series refines and stabilizes the PCC mailbox driver to improve
> > > > initialisation order, interrupt handling, and completion signaling.
> > > > 
> > > Are you happy to pull these patches directly from the list or do you
> > > prefer me to send you pull request or do you want me to direct this via
> > > ACPI/Rafael's tree. Please advice.
> > > 
> > Hi Jassi,
> > 
> > Sorry for the nag. I did see these patches in -next as well as your
> > v6.19 merge window pull request which didn't make it to Linus tree.
> > However I don't see it -next any longer. Please advice if you want
> > anything from my side so that this can be merged for v6.20/v7.0
> > 
> 
> I thought you had an approach you wanted to implement for the functions that
> provided access to the Mailbox internals: you wanted to do them inline but
> hadn't gotten to them yet.  Is that still the case?  I will resubmit mine as
> is with -next if that is acceptable.
> 

Honestly, it has been a while and I have lost the context. Please post what
you have or thinking of on top of linux-next or jassi's -next and we can start
the discussion fresh.

-- 
Regards,
Sudeep

Re: [PATCH 0/6] mailbox: pcc: Refactor and improve initialisation and interrupt handling

[Adam Young]

I was just looking at it.  I think reposting might cause too much churn, 
when the  crux of the matter is what to do with these three functions I 
added:

extern struct pcc_mbox_chan *
pcc_mbox_request_channel(struct mbox_client *cl, int subspace_id);
extern void pcc_mbox_free_channel(struct pcc_mbox_chan *chan);
extern
int pcc_mbox_write_to_buffer(struct pcc_mbox_chan *pchan, int len, void 
*data);
extern
int pcc_mbox_query_bytes_available(struct pcc_mbox_chan *pchan);
extern
int pcc_mbox_read_from_buffer(struct pcc_mbox_chan *pchan, int len,
                               void *data);

In my last patch I added them as EXPORT_SYMBOL_GPL.  You mentioned you 
wanted them as inline.

We can do that, but it does mean further exposing the ACPI header file.  
THe simplest is  pcc_mbox_query_bytes_available which I have posted 
below.  As you can see, it needs the struct 
acpi_pcct_ext_pcc_shared_memory  which comes from 
include/acpi/actbl2.h.  If you are OK with adding that to 
include/acpi/pcc.h  We can inline the functions in there.

These three functions are requied as a result of the direct access to 
the shared memory buffer.


int pcc_mbox_query_bytes_available(struct pcc_mbox_chan *pchan)
{
         struct acpi_pcct_ext_pcc_shared_memory pcc_header;
         struct pcc_chan_info *pinfo = pchan->mchan->con_priv;
         int data_len;
         u64 val;

         pcc_chan_reg_read(&pinfo->cmd_complete, &val);
         if (val) {
                 pr_info("%s Buffer not enabled for reading", __func__);
                 return -1;
         }
         memcpy_fromio(&pcc_header, pchan->shmem,
                       sizeof(pcc_header));
         data_len = pcc_header.length - sizeof(u32) + sizeof(pcc_header);
         return data_len;
}




On 1/27/26 04:29, Sudeep Holla wrote:
> On Mon, Jan 26, 2026 at 12:07:26PM -0500, Adam Young wrote:
>>
>> On 1/12/26 11:55, Sudeep Holla wrote:
>>> On Thu, Nov 27, 2025 at 02:40:56PM +0000, Sudeep Holla wrote:
>>>> Hi Jassi,
>>>>
>>>> On Thu, Oct 16, 2025 at 08:08:14PM +0100, Sudeep Holla wrote:
>>>>> This series refines and stabilizes the PCC mailbox driver to improve
>>>>> initialisation order, interrupt handling, and completion signaling.
>>>>>
>>>> Are you happy to pull these patches directly from the list or do you
>>>> prefer me to send you pull request or do you want me to direct this via
>>>> ACPI/Rafael's tree. Please advice.
>>>>
>>> Hi Jassi,
>>>
>>> Sorry for the nag. I did see these patches in -next as well as your
>>> v6.19 merge window pull request which didn't make it to Linus tree.
>>> However I don't see it -next any longer. Please advice if you want
>>> anything from my side so that this can be merged for v6.20/v7.0
>>>
>> I thought you had an approach you wanted to implement for the functions that
>> provided access to the Mailbox internals: you wanted to do them inline but
>> hadn't gotten to them yet.  Is that still the case?  I will resubmit mine as
>> is with -next if that is acceptable.
>>
> Honestly, it has been a while and I have lost the context. Please post what
> you have or thinking of on top of linux-next or jassi's -next and we can start
> the discussion fresh.
>

Re: [PATCH 0/6] mailbox: pcc: Refactor and improve initialisation and interrupt handling

[Adam Young]

On 2/4/26 16:40, Adam Young wrote:
> I was just looking at it.  I think reposting might cause too much 
> churn, when the  crux of the matter is what to do with these three 
> functions I added:
>
> extern struct pcc_mbox_chan *
> pcc_mbox_request_channel(struct mbox_client *cl, int subspace_id);
> extern void pcc_mbox_free_channel(struct pcc_mbox_chan *chan);
> extern
> int pcc_mbox_write_to_buffer(struct pcc_mbox_chan *pchan, int len, 
> void *data);
> extern
> int pcc_mbox_query_bytes_available(struct pcc_mbox_chan *pchan);
> extern
> int pcc_mbox_read_from_buffer(struct pcc_mbox_chan *pchan, int len,
>                               void *data);
>
> In my last patch I added them as EXPORT_SYMBOL_GPL.  You mentioned you 
> wanted them as inline.
>
> We can do that, but it does mean further exposing the ACPI header 
> file.  THe simplest is  pcc_mbox_query_bytes_available which I have 
> posted below.  As you can see, it needs the struct 
> acpi_pcct_ext_pcc_shared_memory  which comes from 
> include/acpi/actbl2.h.  If you are OK with adding that to 
> include/acpi/pcc.h  We can inline the functions in there.

Actually, I just looked, and the more significant issue is that they 
need access to struct pcc_chan_info, which is inside of mailbox/pcc.c, 
and thus not available to client code.  While we could move this 
structure to the header, I think the better approach is what I 
originally posted, with the 3 additional functions exported as symbols.

I can resubmit it this way, if you agree.

>
> These three functions are requied as a result of the direct access to 
> the shared memory buffer.
>
>
> int pcc_mbox_query_bytes_available(struct pcc_mbox_chan *pchan)
> {
>         struct acpi_pcct_ext_pcc_shared_memory pcc_header;
>         struct pcc_chan_info *pinfo = pchan->mchan->con_priv;
>         int data_len;
>         u64 val;
>
>         pcc_chan_reg_read(&pinfo->cmd_complete, &val);
>         if (val) {
>                 pr_info("%s Buffer not enabled for reading", __func__);
>                 return -1;
>         }
>         memcpy_fromio(&pcc_header, pchan->shmem,
>                       sizeof(pcc_header));
>         data_len = pcc_header.length - sizeof(u32) + sizeof(pcc_header);
>         return data_len;
> }
>
>
>
>
> On 1/27/26 04:29, Sudeep Holla wrote:
>> On Mon, Jan 26, 2026 at 12:07:26PM -0500, Adam Young wrote:
>>>
>>> On 1/12/26 11:55, Sudeep Holla wrote:
>>>> On Thu, Nov 27, 2025 at 02:40:56PM +0000, Sudeep Holla wrote:
>>>>> Hi Jassi,
>>>>>
>>>>> On Thu, Oct 16, 2025 at 08:08:14PM +0100, Sudeep Holla wrote:
>>>>>> This series refines and stabilizes the PCC mailbox driver to improve
>>>>>> initialisation order, interrupt handling, and completion signaling.
>>>>>>
>>>>> Are you happy to pull these patches directly from the list or do you
>>>>> prefer me to send you pull request or do you want me to direct 
>>>>> this via
>>>>> ACPI/Rafael's tree. Please advice.
>>>>>
>>>> Hi Jassi,
>>>>
>>>> Sorry for the nag. I did see these patches in -next as well as your
>>>> v6.19 merge window pull request which didn't make it to Linus tree.
>>>> However I don't see it -next any longer. Please advice if you want
>>>> anything from my side so that this can be merged for v6.20/v7.0
>>>>
>>> I thought you had an approach you wanted to implement for the 
>>> functions that
>>> provided access to the Mailbox internals: you wanted to do them 
>>> inline but
>>> hadn't gotten to them yet.  Is that still the case?  I will resubmit 
>>> mine as
>>> is with -next if that is acceptable.
>>>
>> Honestly, it has been a while and I have lost the context. Please 
>> post what
>> you have or thinking of on top of linux-next or jassi's -next and we 
>> can start
>> the discussion fresh.
>>

Re: [PATCH 0/6] mailbox: pcc: Refactor and improve initialisation and interrupt handling

[Adam Young]

On 1/27/26 04:29, Sudeep Holla wrote:
> On Mon, Jan 26, 2026 at 12:07:26PM -0500, Adam Young wrote:
>>
>> On 1/12/26 11:55, Sudeep Holla wrote:
>>> On Thu, Nov 27, 2025 at 02:40:56PM +0000, Sudeep Holla wrote:
>>>> Hi Jassi,
>>>>
>>>> On Thu, Oct 16, 2025 at 08:08:14PM +0100, Sudeep Holla wrote:
>>>>> This series refines and stabilizes the PCC mailbox driver to improve
>>>>> initialisation order, interrupt handling, and completion signaling.
>>>>>
>>>> Are you happy to pull these patches directly from the list or do you
>>>> prefer me to send you pull request or do you want me to direct this via
>>>> ACPI/Rafael's tree. Please advice.
>>>>
>>> Hi Jassi,
>>>
>>> Sorry for the nag. I did see these patches in -next as well as your
>>> v6.19 merge window pull request which didn't make it to Linus tree.
>>> However I don't see it -next any longer. Please advice if you want
>>> anything from my side so that this can be merged for v6.20/v7.0
>>>
>> I thought you had an approach you wanted to implement for the functions that
>> provided access to the Mailbox internals: you wanted to do them inline but
>> hadn't gotten to them yet.  Is that still the case?  I will resubmit mine as
>> is with -next if that is acceptable.
>>
> Honestly, it has been a while and I have lost the context. Please post what
> you have or thinking of on top of linux-next or jassi's -next and we can start
> the discussion fresh.

The updated patch is posted.  It has a title of

[net-next v31 1/2] mailbox: pcc: functions for reading and writing PCC 
extended data