eth: fbnic: Update IPC mailbox support
Subsystems (3)
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
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
@@ -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
PATCH v2 2/5 eth: fbnic: Allocate all pages for RX mailbox
In reply to Mohsin BashirNow 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
@@ -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
PATCH v2 3/5 eth: fbnic: Reuse RX mailbox pages
In reply to Mohsin BashirCurrently, 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
@@ -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
PATCH v2 4/5 eth: fbnic: Remove retry support
In reply to Mohsin BashirThe 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
@@ -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
PATCH v2 5/5 eth: fbnic: Update RX mbox timeout value
In reply to Mohsin BashirWhile 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
@@ -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
@@ -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
@@ -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 10struct 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
Paolo Abeni
maintainerRe: [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@@ -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@@ -36,6 +37,7 @@ struct fbnic_fw_mbx {* + INDEX_SZ)) */ #define FBNIC_FW_MAX_LOG_HISTORY 14 +#define FBNIC_MBX_RX_TO_SEC 10struct 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
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
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 offbnic_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