diff mbox series

[3/3] mctp pcc: RFC Check before sending MCTP PCC response ACK

Message ID 20240513173546.679061-4-admiyo@os.amperecomputing.com (mailing list archive)
State Changes Requested
Delegated to: Netdev Maintainers
Headers show
Series MCTP over PCC | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Guessed tree name to be net-next, async
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 941 this patch: 941
netdev/build_tools success Errors and warnings before: 0 this patch: 0
netdev/cc_maintainers warning 2 maintainers not CCed: acpica-devel@lists.linux.dev linux-acpi@vger.kernel.org
netdev/build_clang success Errors and warnings before: 941 this patch: 941
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 952 this patch: 952
netdev/checkpatch warning CHECK: Unnecessary parentheses around 'pchan->type == ACPI_PCCT_TYPE_EXT_PCC_SLAVE_SUBSPACE' WARNING: line length of 85 exceeds 80 columns
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

admiyo@os.amperecomputing.com May 13, 2024, 5:35 p.m. UTC 
From: Adam Young <admiyo@os.amperecomputing.com>

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.  Since only one message can be processed at once per
channel, the value of this flag is checked during message processing
and passed back via the channels global structure.

Ideally, the mailbox callback function would return a value
indicating whether the message requires an ACK, but that
would be a change to the mailbox API.  That would involve
some change to all of the mailbox based drivers.

Signed-off-by: Adam Young <admiyo@os.amperecomputing.com>
---
 drivers/mailbox/pcc.c       | 5 ++++-
 drivers/net/mctp/mctp-pcc.c | 4 ++++
 include/acpi/pcc.h          | 1 +
 3 files changed, 9 insertions(+), 1 deletion(-)

Comments

Andrew Lunn May 13, 2024, 8:26 p.m. UTC | #1 
On Mon, May 13, 2024 at 01:35:46PM -0400, admiyo@os.amperecomputing.com wrote:
> From: Adam Young <admiyo@os.amperecomputing.com>
> 
> 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.  Since only one message can be processed at once per
> channel, the value of this flag is checked during message processing
> and passed back via the channels global structure.
> 
> Ideally, the mailbox callback function would return a value
> indicating whether the message requires an ACK, but that
> would be a change to the mailbox API.  That would involve
> some change to all of the mailbox based drivers.

How many mailbox drivers are there?

Generally, taking the path of least resistance will cost more in the
long run. It is better to do it properly from the start.

    Andrew
diff mbox series

Patch

diff --git a/drivers/mailbox/pcc.c b/drivers/mailbox/pcc.c
index 94885e411085..774727b89693 100644
--- a/drivers/mailbox/pcc.c
+++ b/drivers/mailbox/pcc.c
@@ -280,6 +280,7 @@  static irqreturn_t pcc_mbox_irq(int irq, void *p)
 {
 	struct pcc_chan_info *pchan;
 	struct mbox_chan *chan = p;
+	struct pcc_mbox_chan *pmchan;
 	u64 val;
 	int ret;
 
@@ -304,6 +305,8 @@  static irqreturn_t pcc_mbox_irq(int irq, void *p)
 	if (pcc_chan_reg_read_modify_write(&pchan->plat_irq_ack))
 		return IRQ_NONE;
 
+	pmchan = &pchan->chan;
+	pmchan->ack_rx = true;  //TODO default to False
 	mbox_chan_received_data(chan, NULL);
 
 	/*
@@ -312,7 +315,7 @@  static irqreturn_t pcc_mbox_irq(int irq, void *p)
 	 *
 	 * The PCC master subspace channel clears chan_in_use to free channel.
 	 */
-	if (pchan->type == ACPI_PCCT_TYPE_EXT_PCC_SLAVE_SUBSPACE)
+	if ((pchan->type == ACPI_PCCT_TYPE_EXT_PCC_SLAVE_SUBSPACE) && pmchan->ack_rx)
 		pcc_send_data(chan, NULL);
 	pchan->chan_in_use = false;
 
diff --git a/drivers/net/mctp/mctp-pcc.c b/drivers/net/mctp/mctp-pcc.c
index 7242eedd2759..3df45b86ea03 100644
--- a/drivers/net/mctp/mctp-pcc.c
+++ b/drivers/net/mctp/mctp-pcc.c
@@ -96,6 +96,7 @@  static void mctp_pcc_client_rx_callback(struct mbox_client *c, void *)
 	unsigned long buf_ptr_val;
 	struct mctp_pcc_ndev *mctp_pcc_dev = container_of(c, struct mctp_pcc_ndev, inbox_client);
 	void *skb_buf;
+	u32 flags;
 
 	mpp = (struct mctp_pcc_packet *)mctp_pcc_dev->pcc_comm_inbox_addr;
 	buf_ptr_val = (unsigned long)mpp;
@@ -115,6 +116,9 @@  static void mctp_pcc_client_rx_callback(struct mbox_client *c, void *)
 	cb->halen = 0;
 	skb->dev =  mctp_pcc_dev->mdev.dev;
 	netif_rx(skb);
+
+	flags = readl(&mpp->pcc_header.flags);
+	mctp_pcc_dev->in_chan->ack_rx = (flags & 1) > 0;
 }
 
 static netdev_tx_t mctp_pcc_tx(struct sk_buff *skb, struct net_device *ndev)
diff --git a/include/acpi/pcc.h b/include/acpi/pcc.h
index 9b373d172a77..297913378c2b 100644
--- a/include/acpi/pcc.h
+++ b/include/acpi/pcc.h
@@ -16,6 +16,7 @@  struct pcc_mbox_chan {
 	u32 latency;
 	u32 max_access_rate;
 	u16 min_turnaround_time;
+	bool ack_rx;
 };
 
 /* Generic Communications Channel Shared Memory Region */