Skip to contentSkip to conversation

eth: fbnic: Update IPC mailbox support

PATCH Applied Mohsin Bashir 3 participants 9 messages Jan 15 – Jan 21

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

v2 cover eth: fbnic: Update IPC mailbox support

Update IPC mailbox support for fbnic to cater for several changes.
---
Changelog:
V2:
- Update P5 to handle regressions with the use of fbnic_mbx_wait_for_cmpl()

V1: https://lore.kernel.org/netdev/[email protected]

Mohsin Bashir (5):
eth: fbnic: Use GFP_KERNEL to allocting mbx pages
eth: fbnic: Allocate all pages for RX mailbox
eth: fbnic: Reuse RX mailbox pages
eth: fbnic: Remove retry support
eth: fbnic: Update RX mbox timeout value

 .../net/ethernet/meta/fbnic/fbnic_devlink.c   |  8 ++--
 .../net/ethernet/meta/fbnic/fbnic_ethtool.c   |  2 +-
 drivers/net/ethernet/meta/fbnic/fbnic_fw.c    | 42 ++++++++++++-------
 drivers/net/ethernet/meta/fbnic/fbnic_fw.h    |  9 ++++
 drivers/net/ethernet/meta/fbnic/fbnic_mac.c   | 24 +++--------
 5 files changed, 45 insertions(+), 40 deletions(-)
Signature · 1 line
2.47.3
#1 lore raw

PATCH v2 1/5 eth: fbnic: Use GFP_KERNEL to allocting mbx pages

Replace GFP_ATOMIC with GFP_KERNEL for mailbox RX page allocation. Since interrupt handler is threaded GFP_KERNEL is a safe option to reduce allocation failures.

Also remove __GFP_NOWARN so the kernel reports a warning on allocation failure to aid debugging.

---
 drivers/net/ethernet/meta/fbnic/fbnic_fw.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)
drivers/net/ethernet/meta/fbnic/fbnic_fw.c +1 -2
index d8d9b6cfde82..3dfd3f2442ff 100644
@@ -205,8 +205,7 @@ static int fbnic_mbx_alloc_rx_msgs(struct fbnic_dev *fbd)
 	while (!err && count--) {
 		struct fbnic_tlv_msg *msg;
 
-		msg = (struct fbnic_tlv_msg *)__get_free_page(GFP_ATOMIC |
-							      __GFP_NOWARN);
+		msg = (struct fbnic_tlv_msg *)__get_free_page(GFP_KERNEL);
 		if (!msg) {
 			err = -ENOMEM;
 			break;
Signature · 1 line
2.47.3
#2 lore raw

PATCH v2 2/5 eth: fbnic: Allocate all pages for RX mailbox

In reply to Mohsin Bashir

Now that memory is allocated with GFP_KERNEL, allocation failures should be extremely rare. Ensure the FW communication ring is always fully populated with free pages, and hard fail initialization otherwise. This enables simplifications in next patches.

---
 drivers/net/ethernet/meta/fbnic/fbnic_fw.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)
drivers/net/ethernet/meta/fbnic/fbnic_fw.c +9 -5
index 3dfd3f2442ff..09252b3e03ca 100644
@@ -415,7 +415,7 @@ static int fbnic_fw_xmit_simple_msg(struct fbnic_dev *fbd, u32 msg_type)
 	return err;
 }
 
-static void fbnic_mbx_init_desc_ring(struct fbnic_dev *fbd, int mbx_idx)
+static int fbnic_mbx_init_desc_ring(struct fbnic_dev *fbd, int mbx_idx)
 {
 	struct fbnic_fw_mbx *mbx = &fbd->mbx[mbx_idx];
 
@@ -428,14 +428,15 @@ static void fbnic_mbx_init_desc_ring(struct fbnic_dev *fbd, int mbx_idx)
 		     FBNIC_PUL_OB_TLP_HDR_AW_CFG_BME);
 
 		/* Make sure we have a page for the FW to write to */
-		fbnic_mbx_alloc_rx_msgs(fbd);
-		break;
+		return fbnic_mbx_alloc_rx_msgs(fbd);
 	case FBNIC_IPC_MBX_TX_IDX:
 		/* Enable DMA reads from the device */
 		wr32(fbd, FBNIC_PUL_OB_TLP_HDR_AR_CFG,
 		     FBNIC_PUL_OB_TLP_HDR_AR_CFG_BME);
 		break;
 	}
+
+	return 0;
 }
 
 static bool fbnic_mbx_event(struct fbnic_dev *fbd)
@@ -1683,8 +1684,11 @@ int fbnic_mbx_poll_tx_ready(struct fbnic_dev *fbd)
 	} while (!fbnic_mbx_event(fbd));
 
 	/* FW has shown signs of life. Enable DMA and start Tx/Rx */
-	for (i = 0; i < FBNIC_IPC_MBX_INDICES; i++)
-		fbnic_mbx_init_desc_ring(fbd, i);
+	for (i = 0; i < FBNIC_IPC_MBX_INDICES; i++) {
+		err = fbnic_mbx_init_desc_ring(fbd, i);
+		if (err)
+			goto clean_mbx;
+	}
 
 	/* Request an update from the firmware. This should overwrite
 	 * mgmt.version once we get the actual version from the firmware
Signature · 1 line
2.47.3
#3 lore raw

PATCH v2 3/5 eth: fbnic: Reuse RX mailbox pages

In reply to Mohsin Bashir

Currently, the RX mailbox frees and reallocates a page for each received message. Since FW Rx messages are processed synchronously, and nothing hold these pages (unlike skbs which we hand over to the stack), reuse the pages and put them back on the Rx ring. Now that we ensure the ring is always fully populated we don't have to worry about filling it up after partial population during init, either. Update fbnic_mbx_process_rx_msgs() to recycle pages after message processing.

---
 drivers/net/ethernet/meta/fbnic/fbnic_fw.c | 25 ++++++++++++++--------
 1 file changed, 16 insertions(+), 9 deletions(-)
drivers/net/ethernet/meta/fbnic/fbnic_fw.c +16 -9
index 09252b3e03ca..66c9412f4057 100644
@@ -1592,7 +1592,7 @@ static const struct fbnic_tlv_parser fbnic_fw_tlv_parser[] = {
 static void fbnic_mbx_process_rx_msgs(struct fbnic_dev *fbd)
 {
 	struct fbnic_fw_mbx *rx_mbx = &fbd->mbx[FBNIC_IPC_MBX_RX_IDX];
-	u8 head = rx_mbx->head;
+	u8 head = rx_mbx->head, tail = rx_mbx->tail;
 	u64 desc, length;
 
 	while (head != rx_mbx->tail) {
@@ -1603,8 +1603,8 @@ static void fbnic_mbx_process_rx_msgs(struct fbnic_dev *fbd)
 		if (!(desc & FBNIC_IPC_MBX_DESC_FW_CMPL))
 			break;
 
-		dma_unmap_single(fbd->dev, rx_mbx->buf_info[head].addr,
-				 PAGE_SIZE, DMA_FROM_DEVICE);
+		dma_sync_single_for_cpu(fbd->dev, rx_mbx->buf_info[head].addr,
+					FBNIC_RX_PAGE_SIZE, DMA_FROM_DEVICE);
 
 		msg = rx_mbx->buf_info[head].msg;
 
@@ -1637,19 +1637,26 @@ static void fbnic_mbx_process_rx_msgs(struct fbnic_dev *fbd)
 
 		dev_dbg(fbd->dev, "Parsed msg type %d\n", msg->hdr.type);
 next_page:
+		fw_wr32(fbd, FBNIC_IPC_MBX(FBNIC_IPC_MBX_RX_IDX, head), 0);
 
-		free_page((unsigned long)rx_mbx->buf_info[head].msg);
+		rx_mbx->buf_info[tail] = rx_mbx->buf_info[head];
 		rx_mbx->buf_info[head].msg = NULL;
+		rx_mbx->buf_info[head].addr = 0;
 
-		head++;
-		head %= FBNIC_IPC_MBX_DESC_LEN;
+		__fbnic_mbx_wr_desc(fbd, FBNIC_IPC_MBX_RX_IDX, tail,
+				    FIELD_PREP(FBNIC_IPC_MBX_DESC_LEN_MASK,
+					       FBNIC_RX_PAGE_SIZE) |
+				    (rx_mbx->buf_info[tail].addr &
+				     FBNIC_IPC_MBX_DESC_ADDR_MASK) |
+				    FBNIC_IPC_MBX_DESC_HOST_CMPL);
+
+		head = (head + 1) & (FBNIC_IPC_MBX_DESC_LEN - 1);
+		tail = (tail + 1) & (FBNIC_IPC_MBX_DESC_LEN - 1);
 	}
 
 	/* Record head for next interrupt */
 	rx_mbx->head = head;
-
-	/* Make sure we have at least one page for the FW to write to */
-	fbnic_mbx_alloc_rx_msgs(fbd);
+	rx_mbx->tail = tail;
 }
 
 void fbnic_mbx_poll(struct fbnic_dev *fbd)
Signature · 1 line
2.47.3
#4 lore raw

PATCH v2 4/5 eth: fbnic: Remove retry support

In reply to Mohsin Bashir

The driver retries sensor read requests from firmware, but this is unnecessary. A functioning firmware should respond to each request within the timeout period. Remove the retry logic and set the timeout to the sum of all retry timeouts.

---
 drivers/net/ethernet/meta/fbnic/fbnic_mac.c | 24 +++++----------------
 1 file changed, 5 insertions(+), 19 deletions(-)
drivers/net/ethernet/meta/fbnic/fbnic_mac.c +5 -19
index fc7abea4ef5b..9d0e4b2cc9ac 100644
@@ -835,7 +835,7 @@ static int fbnic_mac_get_sensor_asic(struct fbnic_dev *fbd, int id,
 				     long *val)
 {
 	struct fbnic_fw_completion *fw_cmpl;
-	int err = 0, retries = 5;
+	int err = 0;
 	s32 *sensor;
 
 	fw_cmpl = fbnic_fw_alloc_cmpl(FBNIC_TLV_MSG_ID_TSENE_READ_RESP);
@@ -862,24 +862,10 @@ static int fbnic_mac_get_sensor_asic(struct fbnic_dev *fbd, int id,
 		goto exit_free;
 	}
 
-	/* Allow 2 seconds for reply, resend and try up to 5 times */
-	while (!wait_for_completion_timeout(&fw_cmpl->done, 2 * HZ)) {
-		retries--;
-
-		if (retries == 0) {
-			dev_err(fbd->dev,
-				"Timed out waiting for TSENE read\n");
-			err = -ETIMEDOUT;
-			goto exit_cleanup;
-		}
-
-		err = fbnic_fw_xmit_tsene_read_msg(fbd, NULL);
-		if (err) {
-			dev_err(fbd->dev,
-				"Failed to transmit TSENE read msg, err %d\n",
-				err);
-			goto exit_cleanup;
-		}
+	if (!wait_for_completion_timeout(&fw_cmpl->done, 10 * HZ)) {
+		dev_err(fbd->dev, "Timed out waiting for TSENE read\n");
+		err = -ETIMEDOUT;
+		goto exit_cleanup;
 	}
 
 	/* Handle error returned by firmware */
Signature · 1 line
2.47.3
#5 lore raw

PATCH v2 5/5 eth: fbnic: Update RX mbox timeout value

In reply to Mohsin Bashir

While waiting for completions on read requests, driver is using different timeout values for different messages. Make use of a single timeout value.

Introduce a wrapper function to handle the wait, which also simplify maintaining the 80 char line limit.

---
Changelog:
V2:
- Update return type of fbnic_mbx_wait_for_cmpl() to unsigned long to match with the return type of wait_for_completion_timeout()
- Fix inverted fbnic_mbx_wait_for_cmpl() check in fbnic_flash_start() and fbnic_fw_reporter_dump()

---
 drivers/net/ethernet/meta/fbnic/fbnic_devlink.c | 8 ++++----
 drivers/net/ethernet/meta/fbnic/fbnic_ethtool.c | 2 +-
 drivers/net/ethernet/meta/fbnic/fbnic_fw.h      | 9 +++++++++
 3 files changed, 14 insertions(+), 5 deletions(-)
drivers/net/ethernet/meta/fbnic/fbnic_devlink.c +4 -4
index b62b1d5b1453..f1c992f5fe94 100644
@@ -178,7 +178,7 @@ fbnic_flash_start(struct fbnic_dev *fbd, struct pldmfw_component *component)
 		goto cmpl_free;
 
 	/* Wait for firmware to ack firmware upgrade start */
-	if (wait_for_completion_timeout(&cmpl->done, 10 * HZ))
+	if (fbnic_mbx_wait_for_cmpl(cmpl))
 		err = cmpl->result;
 	else
 		err = -ETIMEDOUT;
@@ -252,7 +252,7 @@ fbnic_flash_component(struct pldmfw *context,
 		goto err_no_msg;
 
 	while (offset < size) {
-		if (!wait_for_completion_timeout(&cmpl->done, 15 * HZ)) {
+		if (!fbnic_mbx_wait_for_cmpl(cmpl)) {
 			err = -ETIMEDOUT;
 			break;
 		}
@@ -390,7 +390,7 @@ static int fbnic_fw_reporter_dump(struct devlink_health_reporter *reporter,
 				   "Failed to transmit core dump info msg");
 		goto cmpl_free;
 	}
-	if (!wait_for_completion_timeout(&fw_cmpl->done, 2 * HZ)) {
+	if (!fbnic_mbx_wait_for_cmpl(fw_cmpl)) {
 		NL_SET_ERR_MSG_MOD(extack,
 				   "Timed out waiting on core dump info");
 		err = -ETIMEDOUT;
@@ -447,7 +447,7 @@ static int fbnic_fw_reporter_dump(struct devlink_health_reporter *reporter,
 				goto cmpl_cleanup;
 		}
 
-		if (wait_for_completion_timeout(&fw_cmpl->done, 2 * HZ)) {
+		if (fbnic_mbx_wait_for_cmpl(fw_cmpl)) {
 			reinit_completion(&fw_cmpl->done);
 		} else {
 			NL_SET_ERR_MSG_FMT_MOD(extack,
drivers/net/ethernet/meta/fbnic/fbnic_ethtool.c +1 -1
index 693ebdf38705..61b8005a0db5 100644
@@ -1671,7 +1671,7 @@ fbnic_get_module_eeprom_by_page(struct net_device *netdev,
 		goto exit_free;
 	}
 
-	if (!wait_for_completion_timeout(&fw_cmpl->done, 2 * HZ)) {
+	if (!fbnic_mbx_wait_for_cmpl(fw_cmpl)) {
 		err = -ETIMEDOUT;
 		NL_SET_ERR_MSG_MOD(extack,
 				   "Timed out waiting for firmware response");
drivers/net/ethernet/meta/fbnic/fbnic_fw.h +9 -0
index 1ecd777aaada..b40f68187ad5 100644
@@ -4,6 +4,7 @@
 #ifndef _FBNIC_FW_H_
 #define _FBNIC_FW_H_
 
+#include <linux/completion.h>
 #include <linux/if_ether.h>
 #include <linux/types.h>
 
@@ -36,6 +37,7 @@ struct fbnic_fw_mbx {
  *                       + INDEX_SZ))
  */
 #define FBNIC_FW_MAX_LOG_HISTORY		14
+#define FBNIC_MBX_RX_TO_SEC			10
 
 struct fbnic_fw_ver {
 	u32 version;
@@ -129,6 +131,13 @@ struct fbnic_fw_completion *__fbnic_fw_alloc_cmpl(u32 msg_type,
 struct fbnic_fw_completion *fbnic_fw_alloc_cmpl(u32 msg_type);
 void fbnic_fw_put_cmpl(struct fbnic_fw_completion *cmpl_data);
 
+static inline unsigned long
+fbnic_mbx_wait_for_cmpl(struct fbnic_fw_completion *cmpl)
+{
+	return wait_for_completion_timeout(&cmpl->done,
+					   FBNIC_MBX_RX_TO_SEC * HZ);
+}
+
 #define fbnic_mk_full_fw_ver_str(_rev_id, _delim, _commit, _str, _str_sz) \
 do {									\
 	const u32 __rev_id = _rev_id;					\
Signature · 1 line
2.47.3
maintainer
#6 lore raw

Re: [PATCH net-next V2 5/5] eth: fbnic: Update RX mbox timeout value

drivers/net/ethernet/meta/fbnic/fbnic_devlink.c fbnic_flash_component(struct pldmfw *context, · line 255

Quoted 9 lines · Mohsin Bashir
drivers/net/ethernet/meta/fbnic/fbnic_devlink.c +0 -1
index b62b1d5b1453..f1c992f5fe94 100644
@@ -252,7 +252,7 @@ fbnic_flash_component(struct pldmfw *context,
 		goto err_no_msg;
 
 	while (offset < size) {
-		if (!wait_for_completion_timeout(&cmpl->done, 15 * HZ)) {

flash completion timeout was 15 secs

Quoted 13 lines
drivers/net/ethernet/meta/fbnic/fbnic_fw.h +1 -0
index 1ecd777aaada..b40f68187ad5 100644
@@ -36,6 +37,7 @@ struct fbnic_fw_mbx {
  *                       + INDEX_SZ))
  */
 #define FBNIC_FW_MAX_LOG_HISTORY		14
+#define FBNIC_MBX_RX_TO_SEC			10
 
 struct fbnic_fw_ver {
 	u32 version;

drivers/net/ethernet/meta/fbnic/fbnic_fw.h struct fbnic_fw_completion *__fbnic_fw_alloc_cmpl(u32 msg_type, · line 138

Quoted 8 lines
drivers/net/ethernet/meta/fbnic/fbnic_fw.h +5 -0
@@ -129,6 +131,13 @@ struct fbnic_fw_completion *__fbnic_fw_alloc_cmpl(u32 msg_type,
 struct fbnic_fw_completion *fbnic_fw_alloc_cmpl(u32 msg_type);
 void fbnic_fw_put_cmpl(struct fbnic_fw_completion *cmpl_data);
 
+static inline unsigned long
+fbnic_mbx_wait_for_cmpl(struct fbnic_fw_completion *cmpl)
+{
+	return wait_for_completion_timeout(&cmpl->done,
+					   FBNIC_MBX_RX_TO_SEC * HZ);

Timeout for all events is now 10 seconds. Is that intentional? Could that cause unexpected timeout on flash update?

/P

#7 lore raw

Re: [PATCH net-next V2 5/5] eth: fbnic: Update RX mbox timeout value

Quoted 3 lines

Timeout for all events is now 10 seconds. Is that intentional? Could that cause unexpected timeout on flash update?

Hi Paolo,

Yes, we intentionally set it to 10 sec.

Quoted 2 lines

/P

#8 lore raw
In reply to Mohsin Bashir
This series was applied to netdev/net-next.git (main) by Jakub Kicinski <[email protected]>:

Hello:

This series was applied to netdev/net-next.git (main) by Jakub Kicinski <[email protected]>:

Quoted 8 lines · On Wed, 14 Jan 2026 16:33:48 -0800 you wrote:

Update IPC mailbox support for fbnic to cater for several changes.
---
Changelog:
V2:
- Update P5 to handle regressions with the use of fbnic_mbx_wait_for_cmpl()

[...]

Here is the summary with links:
- [net-next,V2,1/5] eth: fbnic: Use GFP_KERNEL to allocting mbx pages

    https://git.kernel.org/netdev/net-next/c/bafae5de41c2
  - [net-next,V2,2/5] eth: fbnic: Allocate all pages for RX mailbox
    https://git.kernel.org/netdev/net-next/c/0c86b52b1a94
  - [net-next,V2,3/5] eth: fbnic: Reuse RX mailbox pages
    https://git.kernel.org/netdev/net-next/c/301ae0d5391a
  - [net-next,V2,4/5] eth: fbnic: Remove retry support
    https://git.kernel.org/netdev/net-next/c/320ee20a0b54
  - [net-next,V2,5/5] eth: fbnic: Update RX mbox timeout value
    https://git.kernel.org/netdev/net-next/c/5d18a4d04309

You are awesome, thank you!

Signature · 2 lines
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html