Thread (15 messages) 15 messages, 6 authors, 2024-07-15

Re: [PATCH v4 1/3] mctp pcc: Check before sending MCTP PCC response ACK

From: Simon Horman <horms@kernel.org>
Date: 2024-07-03 19:52:37
Also in: lkml

On Tue, Jul 02, 2024 at 06:58:43PM -0400, admiyo@os.amperecomputing.com wrote:
quoted hunk ↗ jump to hunk
From: Adam Young <redacted>

Type 4 PCC channels have an option to send back a response
to the platform when they are done processing the request.
The flag to indicate whether or not to respond is inside
the message body, and thus is not available to the pcc
mailbox.

In order to read the flag, this patch maps the shared
buffer to virtual memory.

Signed-off-by: Adam Young <redacted>
---
 drivers/mailbox/pcc.c | 31 +++++++++++++++++++++++--------
 include/acpi/pcc.h    |  8 ++++++++
 2 files changed, 31 insertions(+), 8 deletions(-)
diff --git a/drivers/mailbox/pcc.c b/drivers/mailbox/pcc.c
index 94885e411085..cad6b5bc4b04 100644
--- a/drivers/mailbox/pcc.c
+++ b/drivers/mailbox/pcc.c
@@ -107,6 +107,7 @@ struct pcc_chan_info {
 	struct pcc_chan_reg cmd_complete;
 	struct pcc_chan_reg cmd_update;
 	struct pcc_chan_reg error;
+	void __iomem *shmem_base_addr;
Hi Adam,

Please add an entry for shmem_base_addr to the Kernel doc for this
structure, which appears just above the structure.

Flagged by ./scripts/kernel-doc -none
quoted hunk ↗ jump to hunk
 	int plat_irq;
 	u8 type;
 	unsigned int plat_irq_flags;
@@ -269,6 +270,24 @@ static bool pcc_mbox_cmd_complete_check(struct pcc_chan_info *pchan)
 	return !!val;
 }
 
+static void check_and_ack(struct pcc_chan_info *pchan, struct mbox_chan *chan)
+{
+	struct pcc_extended_type_hdr pcc_hdr;
+
+	if (pchan->type != ACPI_PCCT_TYPE_EXT_PCC_SLAVE_SUBSPACE)
+		return;
+	memcpy_fromio(&pcc_hdr, pchan->shmem_base_addr,
+		      sizeof(struct pcc_extended_type_hdr));
+	/*
+	 * The PCC slave subspace channel needs to set the command complete bit
+	 * and ring doorbell after processing message.
+	 *
+	 * The PCC master subspace channel clears chan_in_use to free channel.
+	 */
+	if (!!le32_to_cpup(&pcc_hdr.flags) & PCC_ACK_FLAG_MASK)
Should this be:

	if (!!(le32_to_cpup(&pcc_hdr.flags) & PCC_ACK_FLAG_MASK))

In which case I think it can be more simply expressed as:

	if (le32_to_cpup(&pcc_hdr.flags) & PCC_ACK_FLAG_MASK)

Flagged by Smatch and Sparse.
+		pcc_send_data(chan, NULL);
+}
+
 /**
  * pcc_mbox_irq - PCC mailbox interrupt handler
  * @irq:	interrupt number
...
quoted hunk ↗ jump to hunk
diff --git a/include/acpi/pcc.h b/include/acpi/pcc.h
...
quoted hunk ↗ jump to hunk
@@ -31,6 +38,7 @@ struct pcc_mbox_chan {
 #define PCC_CMD_COMPLETION_NOTIFY	BIT(0)
 
 #define MAX_PCC_SUBSPACES	256
+#define PCC_ACK_FLAG_MASK       0x1
nit: For consistency please follow the white-space pattern of the preceding
     line: please use a tab rathe rthan spaces between ...MASK and 0x1.
 
 #ifdef CONFIG_PCC
 extern struct pcc_mbox_chan *
-- 
2.34.1
-- 
pw-bot: changes-requested
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help