From nobody Wed Oct 1 22:37:09 2025 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by smtp.subspace.kernel.org (Postfix) with ESMTP id 166882046BA; Fri, 26 Sep 2025 15:33:15 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=217.140.110.172 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1758900798; cv=none; b=Df48VgmAnCE3a21AvfvXqkkuPExpwirnzuXJWw3Ril4cliSXn+FxcOhmGRyUWrIGGszNOPhKbLXJcDXDYoYsvGadAHBHEzpVm7l4q24hxVLfwT7aGyM/rs6C389+wf5V0pzVfnLvKpeDEzpzrR5Hog+nt73RiMFOn8a9qZaTeqs= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1758900798; c=relaxed/simple; bh=ni3wiqzdcrWJ5hA14fX69yFGyXTIV7lbBZKPSRcswf0=; h=From:To:Cc:Subject:Date:Message-Id:MIME-Version; b=dsnMgefqfb3N3QSW+JMCHwtIVq8kUrs00D5X3pRqtUT1seI+FGDIqIY71LaYwYDImIPtQTl5ZSb0O7lB2qXi2eG8+AYq+GesThwjaJalp8AUiipl05l7QPPytk3wWeJDPnm7MpFgkFan6hUzWIBxacJXWKDdRUsjKEMqD9fIsjo= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=fail (p=none dis=none) header.from=arm.com; spf=none smtp.mailfrom=foss.arm.com; arc=none smtp.client-ip=217.140.110.172 Authentication-Results: smtp.subspace.kernel.org; dmarc=fail (p=none dis=none) header.from=arm.com Authentication-Results: smtp.subspace.kernel.org; spf=none smtp.mailfrom=foss.arm.com Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 177F7168F; Fri, 26 Sep 2025 08:33:07 -0700 (PDT) Received: from usa.arm.com (e133711.arm.com [10.1.196.55]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPA id 2C11B3F66E; Fri, 26 Sep 2025 08:33:14 -0700 (PDT) From: Sudeep Holla To: Jassi Brar , Adam Young Cc: Sudeep Holla , netdev@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH] Revert "mailbox/pcc: support mailbox management of the shared buffer" Date: Fri, 26 Sep 2025 16:33:11 +0100 Message-Id: <20250926153311.2202648-1-sudeep.holla@arm.com> X-Mailer: git-send-email 2.34.1 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8" 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_wr= ites, 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. Signed-off-by: Sudeep Holla --- 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); } =20 -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 =3D pcc_header.length - sizeof(u32) + sizeof(struct pcc_header); - - buffer =3D pchan->chan.rx_alloc(pchan->chan.mchan->cl, data_len); - if (buffer !=3D 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 =3D p; - struct pcc_header *pcc_header =3D chan->active_req; - void *handle =3D NULL; =20 pchan =3D chan->con_priv; =20 @@ -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 =3D false; - - if (pchan->chan.rx_alloc) - handle =3D write_response(pchan); - - if (chan->active_req) { - pcc_header =3D 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); =20 pcc_chan_acknowledge(pchan); =20 @@ -412,24 +384,9 @@ pcc_mbox_request_channel(struct mbox_client *cl, int s= ubspace_id) pcc_mchan =3D &pchan->chan; pcc_mchan->shmem =3D acpi_os_ioremap(pcc_mchan->shmem_base_addr, pcc_mchan->shmem_size); - if (!pcc_mchan->shmem) - goto err; - - pcc_mchan->manage_writes =3D 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 =3D=3D 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; =20 -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); =20 -static int pcc_write_to_buffer(struct mbox_chan *chan, void *data) -{ - struct pcc_chan_info *pchan =3D chan->con_priv; - struct pcc_mbox_chan *pcc_mbox_chan =3D &pchan->chan; - struct pcc_header *pcc_header =3D 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 =3D 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 =3D chan->con_priv; =20 - ret =3D pcc_write_to_buffer(chan, data); - if (ret) - return ret; - ret =3D pcc_chan_reg_read_modify_write(&pchan->cmd_update); if (ret) return ret; =20 ret =3D pcc_chan_reg_read_modify_write(&pchan->db); - if (!ret && pchan->plat_irq > 0) pchan->chan_in_use =3D true; =20 return ret; } =20 - -static bool pcc_last_tx_done(struct mbox_chan *chan) -{ - struct pcc_chan_info *pchan =3D 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 =3D { .send_data =3D pcc_send_data, .startup =3D pcc_startup, .shutdown =3D pcc_shutdown, - .last_tx_done =3D pcc_last_tx_done, }; =20 /** 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; }; =20 /* Generic Communications Channel Shared Memory Region */ --=20 2.34.1