Sorry, thought I was clear in  ACKing it.  Yes, please revert.

On 10/16/25 08:50, Sudeep Holla wrote:

On Fri, Sep 26, 2025 at 04:33:11PM +0100, Sudeep Holla wrote:

quoted

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.

The discussion on this revert stopped and I am not sure if we agreed to
revert it or not.

quoted

Signed-off-by: Sudeep Holla <redacted>
---
  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);

The above change in the original patch has introduced race on my platform
where the mbox_chan_txdone() kicks the Tx while the response to the
command is read in the below mbox_chan_received_data().

So I am going to repost/resend this revert as a fix now.