* [PATCH net-next v9 1/4] net: rnpgbe: Add interrupt handling
2026-08-14 11:13 [PATCH net-next v9 0/4] net: rnpgbe: Add TX/RX and link status support Dong Yibo
@ 2026-08-14 11:13 ` Dong Yibo
2026-08-18 10:59 ` Simon Horman
2026-08-14 11:13 ` [PATCH net-next v9 2/4] net: rnpgbe: Add basic TX packet transmission support Dong Yibo
` (2 subsequent siblings)
3 siblings, 1 reply; 9+ messages in thread
From: Dong Yibo @ 2026-08-14 11:13 UTC (permalink / raw)
To: andrew+netdev, davem, edumazet, kuba, pabeni, vadim.fedorenko,
u.kleine-koenig
Cc: netdev, linux-kernel, dong100, yaojun
Add interrupt support for the RNPGBE driver.
Set up and tear down MSI-X/MSI vectors and NAPI, and process mailbox
events from a workqueue so that mailbox polling stays out of hard-IRQ
context. Queue mailbox work on system_percpu_wq.
Signed-off-by: Dong Yibo <dong100@mucse.com>
---
drivers/net/ethernet/mucse/rnpgbe/Makefile | 3 +-
drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h | 55 ++
.../net/ethernet/mucse/rnpgbe/rnpgbe_chip.c | 4 +
drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h | 2 +
.../net/ethernet/mucse/rnpgbe/rnpgbe_lib.c | 660 ++++++++++++++++++
.../net/ethernet/mucse/rnpgbe/rnpgbe_lib.h | 34 +
.../net/ethernet/mucse/rnpgbe/rnpgbe_main.c | 49 +-
.../net/ethernet/mucse/rnpgbe/rnpgbe_mbx.c | 12 +-
.../net/ethernet/mucse/rnpgbe/rnpgbe_mbx_fw.c | 8 +
.../net/ethernet/mucse/rnpgbe/rnpgbe_mbx_fw.h | 1 +
10 files changed, 819 insertions(+), 9 deletions(-)
create mode 100644 drivers/net/ethernet/mucse/rnpgbe/rnpgbe_lib.c
create mode 100644 drivers/net/ethernet/mucse/rnpgbe/rnpgbe_lib.h
diff --git a/drivers/net/ethernet/mucse/rnpgbe/Makefile b/drivers/net/ethernet/mucse/rnpgbe/Makefile
index de8bcb7772ab..17574cad392a 100644
--- a/drivers/net/ethernet/mucse/rnpgbe/Makefile
+++ b/drivers/net/ethernet/mucse/rnpgbe/Makefile
@@ -8,4 +8,5 @@ obj-$(CONFIG_MGBE) += rnpgbe.o
rnpgbe-objs := rnpgbe_main.o\
rnpgbe_chip.o\
rnpgbe_mbx.o\
- rnpgbe_mbx_fw.o
+ rnpgbe_mbx_fw.o\
+ rnpgbe_lib.o
diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h
index 5b024f9f7e17..77304196c2b6 100644
--- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h
+++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h
@@ -6,6 +6,11 @@
#include <linux/types.h>
#include <linux/mutex.h>
+#include <linux/netdevice.h>
+#include <linux/if.h>
+#include <linux/workqueue.h>
+
+#include "rnpgbe_hw.h"
enum rnpgbe_boards {
board_n500,
@@ -35,21 +40,69 @@ enum {
struct mucse_hw {
void __iomem *hw_addr;
+ void __iomem *ring_msix_base;
struct pci_dev *pdev;
struct mucse_mbx_info mbx;
int port;
u8 pfvfnum;
};
+struct mucse_ring {
+ struct mucse_ring *next;
+ struct mucse_q_vector *q_vector;
+ void __iomem *ring_addr;
+ void __iomem *irq_mask;
+ void __iomem *trig;
+ u8 queue_index;
+ /* hw ring idx */
+ u8 rnpgbe_queue_idx;
+} ____cacheline_internodealigned_in_smp;
+
+struct mucse_ring_container {
+ struct mucse_ring *ring;
+ u16 count;
+};
+
+struct mucse_q_vector {
+ struct mucse *mucse;
+ /* hardware interrupt vector number */
+ int hw_vector;
+ struct mucse_ring_container rx, tx;
+ struct napi_struct napi;
+ char name[IFNAMSIZ + 18];
+ /* for dynamic allocation of rings associated with this q_vector */
+ struct mucse_ring ring[] ____cacheline_internodealigned_in_smp;
+};
+
struct mucse_stats {
u64 tx_dropped;
};
+#define MAX_Q_VECTORS 8
+
+enum mucse_state_t {
+ __MUCSE_DOWN,
+};
+
struct mucse {
struct net_device *netdev;
struct pci_dev *pdev;
struct mucse_hw hw;
struct mucse_stats stats;
+#define M_FLAG_MSIX_SINGLE_EN BIT(0)
+#define M_FLAG_MSIX_EN BIT(1)
+ u32 flags;
+ struct mucse_ring *tx_ring[RNPGBE_MAX_QUEUES]
+ ____cacheline_aligned_in_smp;
+ struct mucse_ring *rx_ring[RNPGBE_MAX_QUEUES]
+ ____cacheline_aligned_in_smp;
+ struct mucse_q_vector *q_vector[MAX_Q_VECTORS];
+ int num_tx_queues;
+ int num_q_vectors;
+ int num_rx_queues;
+ char mbx_name[32];
+ unsigned long state;
+ struct work_struct mbx_work;
};
int rnpgbe_get_permanent_mac(struct mucse_hw *hw, u8 *perm_addr);
@@ -68,4 +121,6 @@ int rnpgbe_init_hw(struct mucse_hw *hw, int board_type);
#define mucse_hw_wr32(hw, reg, val) \
writel((val), (hw)->hw_addr + (reg))
+#define mucse_hw_rd32(hw, reg) \
+ readl((hw)->hw_addr + (reg))
#endif /* _RNPGBE_H */
diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_chip.c b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_chip.c
index ebc7b3750157..921cc325a991 100644
--- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_chip.c
+++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_chip.c
@@ -89,6 +89,8 @@ static void rnpgbe_init_n500(struct mucse_hw *hw)
{
struct mucse_mbx_info *mbx = &hw->mbx;
+ hw->ring_msix_base = hw->hw_addr + MUCSE_N500_RING_MSIX_BASE;
+
mbx->fwpf_ctrl_base = MUCSE_N500_FWPF_CTRL_BASE;
mbx->fwpf_shm_base = MUCSE_N500_FWPF_SHM_BASE;
}
@@ -104,6 +106,8 @@ static void rnpgbe_init_n210(struct mucse_hw *hw)
{
struct mucse_mbx_info *mbx = &hw->mbx;
+ hw->ring_msix_base = hw->hw_addr + MUCSE_N210_RING_MSIX_BASE;
+
mbx->fwpf_ctrl_base = MUCSE_N210_FWPF_CTRL_BASE;
mbx->fwpf_shm_base = MUCSE_N210_FWPF_SHM_BASE;
}
diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h
index e77e6bc3d3e3..0dce78e4a91b 100644
--- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h
+++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h
@@ -6,10 +6,12 @@
#define MUCSE_N500_FWPF_CTRL_BASE 0x28b00
#define MUCSE_N500_FWPF_SHM_BASE 0x2d000
+#define MUCSE_N500_RING_MSIX_BASE 0x28700
#define MUCSE_GBE_PFFW_MBX_CTRL_OFFSET 0x5500
#define MUCSE_GBE_FWPF_MBX_MASK_OFFSET 0x5700
#define MUCSE_N210_FWPF_CTRL_BASE 0x29400
#define MUCSE_N210_FWPF_SHM_BASE 0x2d900
+#define MUCSE_N210_RING_MSIX_BASE 0x29000
#define RNPGBE_DMA_AXI_EN 0x0010
diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_lib.c b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_lib.c
new file mode 100644
index 000000000000..c661290d561b
--- /dev/null
+++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_lib.c
@@ -0,0 +1,660 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright(c) 2020 - 2025 Mucse Corporation. */
+
+#include <linux/pci.h>
+#include <linux/netdevice.h>
+
+#include "rnpgbe_lib.h"
+#include "rnpgbe.h"
+#include "rnpgbe_mbx_fw.h"
+
+static void rnpgbe_mbx_work(struct work_struct *work)
+{
+ struct mucse *mucse = container_of(work, struct mucse, mbx_work);
+
+ mucse_fw_irq_handler(&mucse->hw);
+}
+
+/**
+ * rnpgbe_msix_other - Other irq handler
+ * @irq: interrupt number
+ * @data: private data
+ *
+ * Return: IRQ_HANDLED
+ **/
+static irqreturn_t rnpgbe_msix_other(int irq, void *data)
+{
+ struct mucse *mucse = (struct mucse *)data;
+
+ queue_work(system_percpu_wq, &mucse->mbx_work);
+
+ return IRQ_HANDLED;
+}
+
+static void rnpgbe_irq_disable_queues(struct mucse_q_vector *q_vector)
+{
+ struct mucse_ring *ring;
+
+ /*
+ * TX/RX pairs share interrupt control registers; update them through
+ * the TX ring list.
+ */
+ mucse_for_each_ring(ring, q_vector->tx) {
+ writel(INT_VALID, ring->trig);
+ writel((RX_INT_MASK | TX_INT_MASK), ring->irq_mask);
+ }
+
+ /* flush posted writes to ensure hardware sees the mask */
+ if (q_vector->tx.ring)
+ readl(q_vector->tx.ring->irq_mask);
+}
+
+/**
+ * rnpgbe_int_single - MSI-X single-vector/MSI interrupt handler
+ * @irq: interrupt number
+ * @data: private data
+ *
+ * Return: IRQ_HANDLED
+ **/
+static irqreturn_t rnpgbe_int_single(int irq, void *data)
+{
+ struct mucse *mucse = (struct mucse *)data;
+ struct mucse_q_vector *q_vector;
+
+ queue_work(system_percpu_wq, &mucse->mbx_work);
+
+ if (test_bit(__MUCSE_DOWN, &mucse->state))
+ return IRQ_HANDLED;
+
+ q_vector = mucse->q_vector[0];
+ rnpgbe_irq_disable_queues(q_vector);
+ napi_schedule_irqoff(&q_vector->napi);
+
+ return IRQ_HANDLED;
+}
+
+static void rnpgbe_irq_enable_queues(struct mucse_q_vector *q_vector)
+{
+ struct mucse_ring *ring;
+
+ /*
+ * TX/RX pairs share interrupt control registers; update them through
+ * the TX ring list.
+ */
+ mucse_for_each_ring(ring, q_vector->tx) {
+ writel(0, ring->irq_mask);
+
+ /* Re-trigger hw to re-check events lost while masked. */
+ writel(INT_VALID | TX_INT_MASK | RX_INT_MASK, ring->trig);
+ }
+}
+
+/**
+ * rnpgbe_poll - NAPI Rx polling callback
+ * @napi: structure for representing this polling device
+ * @budget: how many packets driver is allowed to clean
+ *
+ * This function is the NAPI poll callback for all interrupt modes.
+ *
+ * Return: work done in this call
+ **/
+static int rnpgbe_poll(struct napi_struct *napi, int budget)
+{
+ struct mucse_q_vector *q_vector =
+ container_of(napi, struct mucse_q_vector, napi);
+ int work_done = 0;
+
+ /* Exit if we are called by netpoll */
+ if (unlikely(!budget))
+ return 0;
+
+ if (likely(napi_complete_done(napi, work_done)))
+ rnpgbe_irq_enable_queues(q_vector);
+
+ return work_done;
+}
+
+/**
+ * rnpgbe_request_mbx_irq - Register mbx routine
+ * @mucse: pointer to private structure
+ *
+ * In MSIX mode, register a dedicated handler for vector 0 (mailbox)
+ * In MSI/MSI-X_SINGLE mode, mailbox is multiplexed through
+ * data tx/rx handler.
+ *
+ * Return: 0 on success, negative on failure
+ **/
+int rnpgbe_request_mbx_irq(struct mucse *mucse)
+{
+ struct pci_dev *pdev = mucse->pdev;
+ int err = 0;
+
+ snprintf(mucse->mbx_name, sizeof(mucse->mbx_name),
+ "rnpgbe-mbx:%s", pci_name(pdev));
+ INIT_WORK(&mucse->mbx_work, rnpgbe_mbx_work);
+
+ if (mucse->flags & M_FLAG_MSIX_EN) {
+ err = request_irq(pci_irq_vector(pdev, 0),
+ rnpgbe_msix_other, 0, mucse->mbx_name,
+ mucse);
+ } else {
+ err = request_irq(pci_irq_vector(pdev, 0),
+ rnpgbe_int_single, 0, mucse->mbx_name,
+ mucse);
+ }
+
+ return err;
+}
+
+/**
+ * rnpgbe_free_mbx_irq - Remove mbx routine
+ * @mucse: pointer to private structure
+ **/
+void rnpgbe_free_mbx_irq(struct mucse *mucse)
+{
+ struct pci_dev *pdev = mucse->pdev;
+
+ free_irq(pci_irq_vector(pdev, 0), mucse);
+ cancel_work_sync(&mucse->mbx_work);
+}
+
+/**
+ * rnpgbe_set_num_queues - Allocate queues for device, feature dependent
+ * @mucse: pointer to private structure
+ *
+ * Determine tx/rx queue counts
+ **/
+static void rnpgbe_set_num_queues(struct mucse *mucse)
+{
+ /* start from 1 queue */
+ mucse->num_tx_queues = 1;
+ mucse->num_rx_queues = 1;
+}
+
+/**
+ * rnpgbe_set_interrupt_capability - Set MSI-X or MSI if supported
+ * @mucse: pointer to private structure
+ *
+ * Attempt to configure the interrupts using the best available
+ * capabilities of the hardware.
+ *
+ * Return: 0 on success, negative on failure
+ **/
+static int rnpgbe_set_interrupt_capability(struct mucse *mucse)
+{
+ int v_budget;
+
+ v_budget = min3(mucse->num_tx_queues, mucse->num_rx_queues,
+ MAX_Q_VECTORS);
+ v_budget = min_t(int, v_budget, num_online_cpus());
+ /* add one vector for mbx */
+ v_budget += 1;
+
+ /* Hardware limitation: only 1 MSI vector is supported even
+ * if multiple messages are requested. MSI mode falls back
+ * to single vector automatically.
+ */
+ v_budget = pci_alloc_irq_vectors(mucse->pdev, 1, v_budget,
+ PCI_IRQ_MSI | PCI_IRQ_MSIX);
+ if (v_budget < 0)
+ return v_budget;
+
+ if (mucse->pdev->msix_enabled) {
+ /* q_vector not include mbx */
+ if (v_budget > 1) {
+ mucse->flags |= M_FLAG_MSIX_EN;
+ mucse->num_q_vectors = v_budget - 1;
+ } else {
+ mucse->flags |= M_FLAG_MSIX_SINGLE_EN;
+ mucse->num_q_vectors = 1;
+ }
+ } else {
+ /* Hardware supports only one MSI interrupt. */
+ mucse->num_q_vectors = 1;
+ }
+
+ return 0;
+}
+
+/**
+ * mucse_add_ring - Add ring to ring container
+ * @ring: ring to be added
+ * @head: ring container
+ **/
+static void mucse_add_ring(struct mucse_ring *ring,
+ struct mucse_ring_container *head)
+{
+ ring->next = head->ring;
+ head->ring = ring;
+ head->count++;
+}
+
+/**
+ * rnpgbe_alloc_q_vector - Allocate memory for a single interrupt vector
+ * @mucse: pointer to private structure
+ * @eth_queue_idx: queue_index idx for this q_vector
+ * @vector_idx: q_vector array index
+ * @r_idx: starting hardware ring index
+ * @r_count: number of TX/RX ring pairs
+ * @step: ring step
+ *
+ * Return: 0 on success. If allocation fails we return -ENOMEM.
+ **/
+static int rnpgbe_alloc_q_vector(struct mucse *mucse,
+ int eth_queue_idx, int vector_idx, int r_idx,
+ int r_count, int step)
+{
+ int rxr_idx = r_idx, txr_idx = r_idx;
+ struct mucse_hw *hw = &mucse->hw;
+ struct mucse_q_vector *q_vector;
+ int txr_count, rxr_count, idx;
+ struct mucse_ring *ring;
+ int ring_count;
+
+ /*
+ * TX and RX rings are always allocated as pairs with the same hardware
+ * ring index. Interrupt control is shared by the pair, so the TX ring
+ * list is used to update the common registers.
+ */
+ txr_count = r_count;
+ rxr_count = r_count;
+ ring_count = txr_count + rxr_count;
+
+ q_vector = kzalloc_flex(*q_vector, ring, ring_count);
+ if (!q_vector)
+ return -ENOMEM;
+
+ netif_napi_add(mucse->netdev, &q_vector->napi, rnpgbe_poll);
+ /* tie q_vector and mucse together */
+ mucse->q_vector[vector_idx] = q_vector;
+ q_vector->mucse = mucse;
+ q_vector->hw_vector = vector_idx;
+ /* if mbx use separate irq, we should add 1 */
+ if (mucse->flags & M_FLAG_MSIX_EN)
+ q_vector->hw_vector++;
+
+ ring = q_vector->ring;
+
+ for (idx = 0; idx < txr_count; idx++) {
+ mucse_add_ring(ring, &q_vector->tx);
+ ring->queue_index = eth_queue_idx + idx;
+ ring->rnpgbe_queue_idx = txr_idx;
+ ring->ring_addr = hw->hw_addr + RING_OFFSET(txr_idx);
+ ring->irq_mask = ring->ring_addr + RNPGBE_DMA_INT_MASK;
+ ring->trig = ring->ring_addr + RNPGBE_DMA_INT_TRIG;
+ ring->q_vector = q_vector;
+ mucse->tx_ring[ring->queue_index] = ring;
+ txr_idx += step;
+ ring++;
+ }
+
+ for (idx = 0; idx < rxr_count; idx++) {
+ mucse_add_ring(ring, &q_vector->rx);
+ ring->queue_index = eth_queue_idx + idx;
+ ring->rnpgbe_queue_idx = rxr_idx;
+ ring->ring_addr = hw->hw_addr + RING_OFFSET(rxr_idx);
+ ring->irq_mask = ring->ring_addr + RNPGBE_DMA_INT_MASK;
+ ring->trig = ring->ring_addr + RNPGBE_DMA_INT_TRIG;
+ ring->q_vector = q_vector;
+ mucse->rx_ring[ring->queue_index] = ring;
+ rxr_idx += step;
+ ring++;
+ }
+
+ return 0;
+}
+
+/**
+ * rnpgbe_free_q_vector - Free memory allocated for specific interrupt vector
+ * @mucse: pointer to private structure
+ * @vector_idx: q_vector array index
+ *
+ * This function frees the memory allocated to the q_vector. In addition if
+ * NAPI is enabled it will delete any references to the NAPI struct prior
+ * to freeing the q_vector.
+ **/
+static void rnpgbe_free_q_vector(struct mucse *mucse, int vector_idx)
+{
+ struct mucse_q_vector *q_vector = mucse->q_vector[vector_idx];
+ struct mucse_ring *ring;
+
+ mucse_for_each_ring(ring, q_vector->tx)
+ mucse->tx_ring[ring->queue_index] = NULL;
+ mucse_for_each_ring(ring, q_vector->rx)
+ mucse->rx_ring[ring->queue_index] = NULL;
+ mucse->q_vector[vector_idx] = NULL;
+ netif_napi_del(&q_vector->napi);
+ kfree(q_vector);
+}
+
+/**
+ * rnpgbe_alloc_q_vectors - Allocate memory for interrupt vectors
+ * @mucse: pointer to private structure
+ *
+ * Return: 0 on success, or -ENOMEM on allocation failure.
+ **/
+static int rnpgbe_alloc_q_vectors(struct mucse *mucse)
+{
+ int err, ring_cnt, v_remaing = mucse->num_q_vectors;
+ int r_remaing = min_t(int, mucse->num_tx_queues,
+ mucse->num_rx_queues);
+ int q_vector_nums = 0;
+ int eth_queue_idx = 0;
+ int vector_idx = 0;
+ int ring_step = 1;
+ int ring_idx = 0;
+
+ for (; r_remaing > 0 && v_remaing > 0; v_remaing--) {
+ ring_cnt = DIV_ROUND_UP(r_remaing, v_remaing);
+ err = rnpgbe_alloc_q_vector(mucse, eth_queue_idx,
+ vector_idx, ring_idx, ring_cnt,
+ ring_step);
+ if (err)
+ goto err_free_q_vector;
+ ring_idx += ring_step * ring_cnt;
+ eth_queue_idx += ring_cnt;
+ r_remaing -= ring_cnt;
+ q_vector_nums++;
+ vector_idx++;
+ }
+ /* Fix the real used q_vectors_nums */
+ mucse->num_q_vectors = q_vector_nums;
+ mucse->num_tx_queues = eth_queue_idx;
+ mucse->num_rx_queues = eth_queue_idx;
+
+ return 0;
+
+err_free_q_vector:
+ mucse->num_tx_queues = 0;
+ mucse->num_rx_queues = 0;
+ mucse->num_q_vectors = 0;
+
+ while (vector_idx--)
+ rnpgbe_free_q_vector(mucse, vector_idx);
+
+ return err;
+}
+
+/**
+ * rnpgbe_reset_interrupt_capability - Reset irq capability setup
+ * @mucse: pointer to private structure
+ **/
+static void rnpgbe_reset_interrupt_capability(struct mucse *mucse)
+{
+ pci_free_irq_vectors(mucse->pdev);
+ mucse->flags &= ~(M_FLAG_MSIX_EN |
+ M_FLAG_MSIX_SINGLE_EN);
+}
+
+/**
+ * rnpgbe_init_interrupt_scheme - Determine proper interrupt scheme
+ * @mucse: pointer to private structure
+ *
+ * We determine which interrupt scheme to use based on...
+ * - Hardware queue count
+ * - cpu count
+ * - interrupt mode (MSI and legacy modes use one vector)
+ *
+ * Return: 0 on success, negative on failure
+ **/
+int rnpgbe_init_interrupt_scheme(struct mucse *mucse)
+{
+ int err;
+
+ rnpgbe_set_num_queues(mucse);
+
+ err = rnpgbe_set_interrupt_capability(mucse);
+ if (err)
+ return err;
+
+ err = rnpgbe_alloc_q_vectors(mucse);
+ if (err) {
+ rnpgbe_reset_interrupt_capability(mucse);
+ return err;
+ }
+
+ return 0;
+}
+
+/**
+ * rnpgbe_free_q_vectors - Free memory allocated for interrupt vectors
+ * @mucse: pointer to private structure
+ *
+ * This function frees the memory allocated to the q_vectors. In addition if
+ * NAPI is enabled it will delete any references to the NAPI struct prior
+ * to freeing the q_vector.
+ **/
+static void rnpgbe_free_q_vectors(struct mucse *mucse)
+{
+ int vector_idx = mucse->num_q_vectors;
+
+ mucse->num_rx_queues = 0;
+ mucse->num_tx_queues = 0;
+ mucse->num_q_vectors = 0;
+
+ while (vector_idx--)
+ rnpgbe_free_q_vector(mucse, vector_idx);
+}
+
+/**
+ * rnpgbe_clear_interrupt_scheme - Clear the current interrupt scheme settings
+ * @mucse: pointer to private structure
+ *
+ * Clear interrupt specific resources and reset the structure
+ **/
+void rnpgbe_clear_interrupt_scheme(struct mucse *mucse)
+{
+ mucse->num_tx_queues = 0;
+ mucse->num_rx_queues = 0;
+ rnpgbe_free_q_vectors(mucse);
+ rnpgbe_reset_interrupt_capability(mucse);
+}
+
+/**
+ * rnpgbe_msix_clean_rings - MSI-x interrupt handler for ring irq
+ * @irq: interrupt number
+ * @data: private data
+ *
+ * rnpgbe_msix_clean_rings handle irq from ring, start napi
+ * Return: IRQ_HANDLED
+ **/
+static irqreturn_t rnpgbe_msix_clean_rings(int irq, void *data)
+{
+ struct mucse_q_vector *q_vector = (struct mucse_q_vector *)data;
+
+ rnpgbe_irq_disable_queues(q_vector);
+ napi_schedule_irqoff(&q_vector->napi);
+
+ return IRQ_HANDLED;
+}
+
+/**
+ * rnpgbe_request_irq - Initialize interrupts
+ * @mucse: pointer to private structure
+ *
+ * Attempts to configure interrupts using the best available
+ * capabilities of the hardware and kernel.
+ *
+ * Return: 0 on success, negative value on failure
+ **/
+int rnpgbe_request_irq(struct mucse *mucse)
+{
+ struct net_device *netdev = mucse->netdev;
+ struct pci_dev *pdev = mucse->pdev;
+ struct mucse_q_vector *q_vector;
+ int err, i;
+
+ if (mucse->flags & M_FLAG_MSIX_EN) {
+ for (i = 0; i < mucse->num_q_vectors; i++) {
+ q_vector = mucse->q_vector[i];
+
+ snprintf(q_vector->name, sizeof(q_vector->name),
+ "%s-%s-%d", netdev->name, "TxRx", i);
+
+ err = request_irq(pci_irq_vector(pdev, i + 1),
+ rnpgbe_msix_clean_rings, 0,
+ q_vector->name,
+ q_vector);
+ if (err) {
+ dev_err(&pdev->dev, "MSI-X req err %d: %d\n",
+ i + 1, err);
+ goto err_free_irqs;
+ }
+ }
+ }
+
+ return 0;
+err_free_irqs:
+ while (i--) {
+ q_vector = mucse->q_vector[i];
+ synchronize_irq(pci_irq_vector(pdev, i + 1));
+ free_irq(pci_irq_vector(pdev, i + 1), q_vector);
+ }
+
+ return err;
+}
+
+/**
+ * rnpgbe_free_irq - Free interrupts
+ * @mucse: pointer to private structure
+ *
+ * Attempts to free interrupts according initialized type.
+ **/
+void rnpgbe_free_irq(struct mucse *mucse)
+{
+ struct pci_dev *pdev = mucse->pdev;
+ struct mucse_q_vector *q_vector;
+
+ if (mucse->flags & M_FLAG_MSIX_EN) {
+ for (int i = 0; i < mucse->num_q_vectors; i++) {
+ q_vector = mucse->q_vector[i];
+ if (!q_vector)
+ continue;
+
+ free_irq(pci_irq_vector(pdev, i + 1), q_vector);
+ }
+ }
+}
+
+/**
+ * rnpgbe_set_ring_vector - Set the ring_vector registers,
+ * mapping interrupt causes to vectors
+ * @mucse: pointer to private structure
+ * @queue: queue to map the corresponding interrupt to
+ * @vector: the vector num to map to the corresponding queue
+ *
+ */
+static void rnpgbe_set_ring_vector(struct mucse *mucse,
+ u8 queue, u8 vector)
+{
+ struct mucse_hw *hw = &mucse->hw;
+ u32 data;
+
+ data = hw->pfvfnum << 24;
+ data |= (vector << 8);
+ data |= vector;
+ writel(data, hw->ring_msix_base + RING_VECTOR(queue));
+}
+
+/**
+ * rnpgbe_configure_msi - Configure MSI hardware
+ * @mucse: pointer to private structure
+ *
+ * rnpgbe_configure_msi sets up the hardware to properly generate MSI
+ * interrupts.
+ **/
+static void rnpgbe_configure_msi(struct mucse *mucse)
+{
+ struct mucse_q_vector *q_vector = mucse->q_vector[0];
+ struct mucse_ring *ring;
+
+ /* TX/RX pairs share the vector register; update it through TX rings. */
+ mucse_for_each_ring(ring, q_vector->tx)
+ rnpgbe_set_ring_vector(mucse, ring->rnpgbe_queue_idx, 0);
+}
+
+/**
+ * rnpgbe_configure_msix - Configure MSI-X hardware
+ * @mucse: pointer to private structure
+ *
+ * rnpgbe_configure_msix sets up the hardware to properly generate MSI-X
+ * interrupts.
+ **/
+static void rnpgbe_configure_msix(struct mucse *mucse)
+{
+ struct mucse_q_vector *q_vector;
+
+ for (int i = 0; i < mucse->num_q_vectors; i++) {
+ struct mucse_ring *ring;
+
+ q_vector = mucse->q_vector[i];
+ /* TX/RX pairs share the vector register; update it through TX rings. */
+ mucse_for_each_ring(ring, q_vector->tx) {
+ rnpgbe_set_ring_vector(mucse, ring->rnpgbe_queue_idx,
+ q_vector->hw_vector);
+ }
+ }
+}
+
+static void rnpgbe_irq_enable(struct mucse *mucse)
+{
+ for (int i = 0; i < mucse->num_q_vectors; i++)
+ rnpgbe_irq_enable_queues(mucse->q_vector[i]);
+}
+
+/**
+ * rnpgbe_irq_disable - Mask off interrupt generation on the NIC
+ * @mucse: board private structure
+ **/
+void rnpgbe_irq_disable(struct mucse *mucse)
+{
+ struct pci_dev *pdev = mucse->pdev;
+
+ if (mucse->flags & M_FLAG_MSIX_EN) {
+ for (int i = 0; i < mucse->num_q_vectors; i++) {
+ rnpgbe_irq_disable_queues(mucse->q_vector[i]);
+ synchronize_irq(pci_irq_vector(pdev, i + 1));
+ }
+ } else {
+ rnpgbe_irq_disable_queues(mucse->q_vector[0]);
+ synchronize_irq(pci_irq_vector(pdev, 0));
+ }
+}
+
+static void rnpgbe_napi_enable_all(struct mucse *mucse)
+{
+ for (int i = 0; i < mucse->num_q_vectors; i++)
+ napi_enable(&mucse->q_vector[i]->napi);
+}
+
+static void rnpgbe_napi_disable_all(struct mucse *mucse)
+{
+ for (int i = 0; i < mucse->num_q_vectors; i++)
+ napi_disable(&mucse->q_vector[i]->napi);
+}
+
+bool rnpgbe_down(struct mucse *mucse)
+{
+ if (test_and_set_bit(__MUCSE_DOWN, &mucse->state))
+ return false;
+
+ rnpgbe_napi_disable_all(mucse);
+ rnpgbe_irq_disable(mucse);
+
+ return true;
+}
+
+/**
+ * rnpgbe_up_complete - Final step for port up
+ * @mucse: pointer to private structure
+ **/
+void rnpgbe_up_complete(struct mucse *mucse)
+{
+ if (mucse->flags & (M_FLAG_MSIX_EN | M_FLAG_MSIX_SINGLE_EN))
+ rnpgbe_configure_msix(mucse);
+ else
+ rnpgbe_configure_msi(mucse);
+ rnpgbe_napi_enable_all(mucse);
+ clear_bit(__MUCSE_DOWN, &mucse->state);
+ rnpgbe_irq_enable(mucse);
+}
diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_lib.h b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_lib.h
new file mode 100644
index 000000000000..c03f7aad2c08
--- /dev/null
+++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_lib.h
@@ -0,0 +1,34 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Copyright(c) 2020 - 2025 Mucse Corporation. */
+
+#ifndef _RNPGBE_LIB_H
+#define _RNPGBE_LIB_H
+
+struct mucse;
+struct mucse_hw;
+
+#define RING_OFFSET(n) (0x1000 + 0x100 * (n))
+#define RNPGBE_DMA_INT_MASK 0x24
+#define TX_INT_MASK BIT(1)
+#define RX_INT_MASK BIT(0)
+#define INT_VALID (BIT(16) | BIT(17))
+#define RNPGBE_DMA_INT_TRIG 0x2c /* lost-interrupt recovery trigger */
+/* | 31:24 | .... | 15:8 | 7:0 | */
+/* | pfvfnum | | tx vector | rx vector | */
+#define RING_VECTOR(n) (0x04 * (n))
+
+#define mucse_for_each_ring(pos, head)\
+ for (typeof((head).ring) __pos = (head).ring;\
+ __pos ? ({ pos = __pos; 1; }) : 0;\
+ __pos = __pos->next)
+
+int rnpgbe_init_interrupt_scheme(struct mucse *mucse);
+void rnpgbe_clear_interrupt_scheme(struct mucse *mucse);
+int rnpgbe_request_mbx_irq(struct mucse *mucse);
+void rnpgbe_free_mbx_irq(struct mucse *mucse);
+int rnpgbe_request_irq(struct mucse *mucse);
+void rnpgbe_free_irq(struct mucse *mucse);
+void rnpgbe_irq_disable(struct mucse *mucse);
+bool rnpgbe_down(struct mucse *mucse);
+void rnpgbe_up_complete(struct mucse *mucse);
+#endif
diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c
index 70a2b0082ba8..c3296a3dde0b 100644
--- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c
+++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c
@@ -7,6 +7,7 @@
#include "rnpgbe.h"
#include "rnpgbe_hw.h"
+#include "rnpgbe_lib.h"
#include "rnpgbe_mbx_fw.h"
static const char rnpgbe_driver_name[] = "rnpgbe";
@@ -32,11 +33,28 @@ static struct pci_device_id rnpgbe_pci_tbl[] = {
* The open entry point is called when a network interface is made
* active by the system (IFF_UP).
*
- * Return: 0
+ * Return: 0 on success, negative value on failure
**/
static int rnpgbe_open(struct net_device *netdev)
{
+ struct mucse *mucse = netdev_priv(netdev);
+ int err;
+
+ err = rnpgbe_request_irq(mucse);
+ if (err)
+ return err;
+
+ err = netif_set_real_num_queues(netdev, mucse->num_tx_queues,
+ mucse->num_rx_queues);
+ if (err)
+ goto err_free_irqs;
+
+ rnpgbe_up_complete(mucse);
+
return 0;
+err_free_irqs:
+ rnpgbe_free_irq(mucse);
+ return err;
}
/**
@@ -50,6 +68,13 @@ static int rnpgbe_open(struct net_device *netdev)
**/
static int rnpgbe_close(struct net_device *netdev)
{
+ struct mucse *mucse = netdev_priv(netdev);
+
+ if (!rnpgbe_down(mucse))
+ return 0;
+
+ rnpgbe_free_irq(mucse);
+
return 0;
}
@@ -106,6 +131,7 @@ static int rnpgbe_add_adapter(struct pci_dev *pdev,
mucse = netdev_priv(netdev);
mucse->netdev = netdev;
mucse->pdev = pdev;
+ set_bit(__MUCSE_DOWN, &mucse->state);
pci_set_drvdata(pdev, mucse);
hw = &mucse->hw;
@@ -166,11 +192,28 @@ static int rnpgbe_add_adapter(struct pci_dev *pdev,
goto err_powerdown;
}
+ err = rnpgbe_init_interrupt_scheme(mucse);
+ if (err) {
+ dev_err(&pdev->dev, "init interrupt failed %d\n", err);
+ goto err_powerdown;
+ }
+
+ err = rnpgbe_request_mbx_irq(mucse);
+ if (err) {
+ dev_err(&pdev->dev, "register mbx irq failed %d\n", err);
+ goto err_clear_interrupt;
+ }
+
err = register_netdev(netdev);
if (err)
- goto err_powerdown;
+ goto err_remove_mbx;
return 0;
+
+err_remove_mbx:
+ rnpgbe_free_mbx_irq(mucse);
+err_clear_interrupt:
+ rnpgbe_clear_interrupt_scheme(mucse);
err_powerdown:
/* notify powerdown only powerup ok */
if (!err_notify) {
@@ -253,9 +296,11 @@ static void rnpgbe_rm_adapter(struct pci_dev *pdev)
return;
netdev = mucse->netdev;
unregister_netdev(netdev);
+ rnpgbe_free_mbx_irq(mucse);
err = rnpgbe_send_notify(hw, false, mucse_fw_powerup);
if (err)
dev_warn(&pdev->dev, "Send powerdown to hw failed %d\n", err);
+ rnpgbe_clear_interrupt_scheme(mucse);
free_netdev(netdev);
}
diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx.c b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx.c
index c46408698263..2310f01df1f5 100644
--- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx.c
+++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx.c
@@ -84,7 +84,7 @@ static u32 mucse_mbx_get_lock_pf(struct mucse_hw *hw)
* @hw: pointer to the HW structure
*
* Pair with mucse_release_mbx_lock_pf()
- * This function maybe used in an irq handler.
+ * All mailbox access runs in process context.
*
* Return: 0 on success, negative errno on failure
**/
@@ -93,11 +93,11 @@ static int mucse_obtain_mbx_lock_pf(struct mucse_hw *hw)
struct mucse_mbx_info *mbx = &hw->mbx;
u32 val;
- return read_poll_timeout_atomic(mucse_mbx_get_lock_pf,
- val, val & MUCSE_MBX_PFU,
- mbx->delay_us,
- mbx->timeout_us,
- false, hw);
+ return read_poll_timeout(mucse_mbx_get_lock_pf,
+ val, val & MUCSE_MBX_PFU,
+ mbx->delay_us,
+ mbx->timeout_us,
+ false, hw);
}
/**
diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx_fw.c b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx_fw.c
index 5ba74997beac..754016f6b4f0 100644
--- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx_fw.c
+++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx_fw.c
@@ -197,3 +197,11 @@ int mucse_mbx_get_macaddr(struct mucse_hw *hw, int pfvfnum,
return 0;
}
+
+/**
+ * mucse_fw_irq_handler - Try to handle a req from hw
+ * @hw: pointer to the HW structure
+ **/
+void mucse_fw_irq_handler(struct mucse_hw *hw)
+{
+}
diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx_fw.h b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx_fw.h
index fe996aeffc4d..a6bf5de55aa3 100644
--- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx_fw.h
+++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx_fw.h
@@ -99,4 +99,5 @@ int mucse_mbx_powerup(struct mucse_hw *hw, bool is_powerup);
int mucse_mbx_reset_hw(struct mucse_hw *hw);
int mucse_mbx_get_macaddr(struct mucse_hw *hw, int pfvfnum,
u8 *mac_addr, int port);
+void mucse_fw_irq_handler(struct mucse_hw *hw);
#endif /* _RNPGBE_MBX_FW_H */
--
2.25.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH net-next v9 1/4] net: rnpgbe: Add interrupt handling
2026-08-14 11:13 ` [PATCH net-next v9 1/4] net: rnpgbe: Add interrupt handling Dong Yibo
@ 2026-08-18 10:59 ` Simon Horman
0 siblings, 0 replies; 9+ messages in thread
From: Simon Horman @ 2026-08-18 10:59 UTC (permalink / raw)
To: dong100
Cc: Simon Horman, andrew+netdev, davem, edumazet, kuba, pabeni,
vadim.fedorenko, u.kleine-koenig, netdev, linux-kernel, yaojun
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260814111317.1741087-2-dong100@mucse.com?part=1
---
--- Patch [1]: [PATCH net-next v9 1/4] net: rnpgbe: Add interrupt handling ---
commit 77ae10f5dc17ffe22f37ef80f20221f43aaa66ab
Author: Dong Yibo <dong100@mucse.com>
net: rnpgbe: Add interrupt handling
This adds the MSI-X/MSI vector and NAPI setup and teardown for the rnpgbe
driver, plus a vector 0 mailbox handler that defers work to a work item on
system_percpu_wq. rnpgbe_open()/rnpgbe_close() are wired up to request and
release the per-queue interrupts and to bring the q-vectors up and down.
A few questions below.
> diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_lib.c b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_lib.c
> new file mode 100644
> index 0000000000000..c661290d561bd
> --- /dev/null
> +++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_lib.c
> @@ -0,0 +1,660 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright(c) 2020 - 2025 Mucse Corporation. */
> +
> +#include <linux/pci.h>
> +#include <linux/netdevice.h>
> +
> +#include "rnpgbe_lib.h"
> +#include "rnpgbe.h"
> +#include "rnpgbe_mbx_fw.h"
> +
> +static void rnpgbe_mbx_work(struct work_struct *work)
> +{
> + struct mucse *mucse = container_of(work, struct mucse, mbx_work);
> +
> + mucse_fw_irq_handler(&mucse->hw);
> +}
> +
> +/**
> + * rnpgbe_msix_other - Other irq handler
> + * @irq: interrupt number
> + * @data: private data
> + *
> + * Return: IRQ_HANDLED
> + **/
> +static irqreturn_t rnpgbe_msix_other(int irq, void *data)
> +{
> + struct mucse *mucse = (struct mucse *)data;
> +
> + queue_work(system_percpu_wq, &mucse->mbx_work);
> +
> + return IRQ_HANDLED;
> +}
[Severity: Medium]
[Finding: gpt-5-6-sol-7-16]
[Sources: gpt-5-6-sol, opus-5]
Should this handler mask the fw->pf mailbox source, or otherwise retire the
request, before returning IRQ_HANDLED?
The request half of the mailbox mask is enabled once in mucse_mbx_reset():
mbx_ctrl_wr32(mbx, MUCSE_MBX_FWPF_MASK(mbx), GENMASK_U32(31, 16));
and nothing else in the driver masks it again. The only code that clears
the request condition is mucse_read_mbx_pf():
hw->mbx.fw_req = mucse_mbx_get_fwreq(mbx);
mucse_mbx_inc_pf_ack(hw);
At this commit the deferred work reaches only the empty
mucse_fw_irq_handler(), so fw_req != pf_ack when the hard IRQ returns and
the source is still unmasked.
If the fw->pf doorbell assertion is level-held until the ack, as is usual
for a req/ack counter doorbell, does this vector keep being re-asserted
until the core disables it as spurious?
The ack side is added later in the series by "net: rnpgbe: Add link status
handling support", where mucse_fw_irq_handler() drains events via
mucse_check_and_read_mbx(). The un-acked window in the hard IRQ handler
still covers the workqueue latency plus hw->mbx.lock contention plus
mbx->timeout_us of mailbox polling after that patch.
> +
> +static void rnpgbe_irq_disable_queues(struct mucse_q_vector *q_vector)
[ ... ]
> +/**
> + * rnpgbe_int_single - MSI-X single-vector/MSI interrupt handler
> + * @irq: interrupt number
> + * @data: private data
> + *
> + * Return: IRQ_HANDLED
> + **/
> +static irqreturn_t rnpgbe_int_single(int irq, void *data)
> +{
> + struct mucse *mucse = (struct mucse *)data;
> + struct mucse_q_vector *q_vector;
> +
> + queue_work(system_percpu_wq, &mucse->mbx_work);
> +
> + if (test_bit(__MUCSE_DOWN, &mucse->state))
> + return IRQ_HANDLED;
> +
> + q_vector = mucse->q_vector[0];
> + rnpgbe_irq_disable_queues(q_vector);
> + napi_schedule_irqoff(&q_vector->napi);
> +
> + return IRQ_HANDLED;
> +}
[ ... ]
> +/**
> + * rnpgbe_poll - NAPI Rx polling callback
> + * @napi: structure for representing this polling device
> + * @budget: how many packets driver is allowed to clean
> + *
> + * This function is the NAPI poll callback for all interrupt modes.
> + *
> + * Return: work done in this call
> + **/
> +static int rnpgbe_poll(struct napi_struct *napi, int budget)
> +{
> + struct mucse_q_vector *q_vector =
> + container_of(napi, struct mucse_q_vector, napi);
> + int work_done = 0;
> +
> + /* Exit if we are called by netpoll */
> + if (unlikely(!budget))
> + return 0;
> +
> + if (likely(napi_complete_done(napi, work_done)))
> + rnpgbe_irq_enable_queues(q_vector);
> +
> + return work_done;
> +}
[Severity: Low]
[Finding: gpt-5-6-sol-2-8]
[Sources: gpt-5-6-sol, opus-5]
This isn't a bug, but the kernel-doc here describes @budget as "how many
packets driver is allowed to clean" and "Return: work done in this call",
while the body touches no ring and work_done is hard-coded to 0.
The actual cleaning is added later in the series by the TX and RX patches,
where rnpgbe_poll() calls rnpgbe_clean_tx_irq() and rnpgbe_clean_rx_irq().
Could the kernel-doc match the code at this step?
> +
> +/**
> + * rnpgbe_request_mbx_irq - Register mbx routine
> + * @mucse: pointer to private structure
> + *
> + * In MSIX mode, register a dedicated handler for vector 0 (mailbox)
> + * In MSI/MSI-X_SINGLE mode, mailbox is multiplexed through
> + * data tx/rx handler.
> + *
> + * Return: 0 on success, negative on failure
> + **/
> +int rnpgbe_request_mbx_irq(struct mucse *mucse)
> +{
> + struct pci_dev *pdev = mucse->pdev;
> + int err = 0;
> +
> + snprintf(mucse->mbx_name, sizeof(mucse->mbx_name),
> + "rnpgbe-mbx:%s", pci_name(pdev));
> + INIT_WORK(&mucse->mbx_work, rnpgbe_mbx_work);
> +
> + if (mucse->flags & M_FLAG_MSIX_EN) {
> + err = request_irq(pci_irq_vector(pdev, 0),
> + rnpgbe_msix_other, 0, mucse->mbx_name,
> + mucse);
> + } else {
> + err = request_irq(pci_irq_vector(pdev, 0),
> + rnpgbe_int_single, 0, mucse->mbx_name,
> + mucse);
> + }
> +
> + return err;
> +}
> +
> +/**
> + * rnpgbe_free_mbx_irq - Remove mbx routine
> + * @mucse: pointer to private structure
> + **/
> +void rnpgbe_free_mbx_irq(struct mucse *mucse)
> +{
> + struct pci_dev *pdev = mucse->pdev;
> +
> + free_irq(pci_irq_vector(pdev, 0), mucse);
> + cancel_work_sync(&mucse->mbx_work);
> +}
[ ... ]
> +static int rnpgbe_set_interrupt_capability(struct mucse *mucse)
> +{
[ ... ]
> + v_budget = pci_alloc_irq_vectors(mucse->pdev, 1, v_budget,
> + PCI_IRQ_MSI | PCI_IRQ_MSIX);
> + if (v_budget < 0)
> + return v_budget;
[ ... ]
> +/**
> + * rnpgbe_init_interrupt_scheme - Determine proper interrupt scheme
> + * @mucse: pointer to private structure
> + *
> + * We determine which interrupt scheme to use based on...
> + * - Hardware queue count
> + * - cpu count
> + * - interrupt mode (MSI and legacy modes use one vector)
[Severity: Low]
[Finding: main-2-6]
[Sources: opus-5]
Is there a legacy INTx mode in this driver for this comment to refer to?
rnpgbe_set_interrupt_capability() requests only PCI_IRQ_MSI | PCI_IRQ_MSIX,
and both request_irq() calls in rnpgbe_request_mbx_irq() pass flags 0 rather
than IRQF_SHARED, which would not work on a shared legacy line. If neither
MSI nor MSI-X is available, probe simply fails.
This wording is still present at the end of the series.
> + *
> + * Return: 0 on success, negative on failure
> + **/
[ ... ]
> diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c
> index 70a2b0082ba86..c3296a3dde0bb 100644
> --- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c
> +++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c
[ ... ]
> @@ -50,6 +68,13 @@ static int rnpgbe_open(struct net_device *netdev)
> **/
> static int rnpgbe_close(struct net_device *netdev)
> {
> + struct mucse *mucse = netdev_priv(netdev);
> +
> + if (!rnpgbe_down(mucse))
> + return 0;
> +
> + rnpgbe_free_irq(mucse);
> +
> return 0;
> }
[Severity: Medium]
[Finding: main-4-10]
[Sources: opus-5]
rnpgbe_close() -> rnpgbe_down() + rnpgbe_free_irq() masks and frees only the
per-queue MSI-X vectors, and the shutdown path relies on it:
rnpgbe_shutdown()
rnpgbe_dev_shutdown()
netif_device_detach(netdev);
if (netif_running(netdev))
rnpgbe_close(netdev);
pci_disable_device(pdev);
So the vector 0 handler registered by rnpgbe_request_mbx_irq()
(rnpgbe_msix_other(), or rnpgbe_int_single() in MSI / MSI-X-single mode) is
never freed and mucse->mbx_work is never cancel_work_sync()'d on shutdown -
rnpgbe_free_mbx_irq() is reached only from rnpgbe_rm_adapter() and the probe
error path.
pci_disable_device() clears PCI_COMMAND_MASTER but does not disable MSI/MSI-X
or MMIO decoding. Can the device still raise the mailbox interrupt after
.shutdown returns, with the handler queueing mbx_work that then performs
mailbox MMIO on a device the driver has declared dead? This is visible
across kexec, where the machine keeps running until machine_kexec().
Should rnpgbe_dev_shutdown() call rnpgbe_free_mbx_irq() and
rnpgbe_clear_interrupt_scheme() so the shutdown path is symmetric with
rnpgbe_rm_adapter()? The same asymmetry is still present at the end of the
series.
[ ... ]
> diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx_fw.c b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx_fw.c
> index 5ba74997beacf..754016f6b4f00 100644
> --- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx_fw.c
> +++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx_fw.c
> @@ -197,3 +197,11 @@ int mucse_mbx_get_macaddr(struct mucse_hw *hw, int pfvfnum,
>
> return 0;
> }
> +
> +/**
> + * mucse_fw_irq_handler - Try to handle a req from hw
> + * @hw: pointer to the HW structure
> + **/
> +void mucse_fw_irq_handler(struct mucse_hw *hw)
> +{
> +}
[Severity: Low]
[Finding: gpt-5-6-sol-1-2]
[Sources: gpt-5-6-sol, opus-5]
The commit message says the patch "process[es] mailbox events from a
workqueue" and this kernel-doc says the function will "Try to handle a req
from hw", but the body is empty.
The full path
rnpgbe_msix_other()/rnpgbe_int_single()
queue_work(system_percpu_wq, &mucse->mbx_work)
rnpgbe_mbx_work()
mucse_fw_irq_handler()
reads no mailbox register, decodes no event, and never performs the retire
sequence that mucse_read_mbx_pf() defines:
hw->mbx.fw_req = mucse_mbx_get_fwreq(mbx);
mucse_mbx_inc_pf_ack(hw);
At this commit a firmware request that arrives stays pending, so a later
driver-initiated transaction (mucse_fw_send_cmd_wait_resp() ->
mucse_poll_and_read_mbx()) can consume it as a bogus reply and spend one of
its three retries on the opcode mismatch.
Could the commit message and this kernel-doc say that this patch adds only
the interrupt and workqueue plumbing? The handler body is filled in later
in the series by "net: rnpgbe: Add link status handling support".
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH net-next v9 2/4] net: rnpgbe: Add basic TX packet transmission support
2026-08-14 11:13 [PATCH net-next v9 0/4] net: rnpgbe: Add TX/RX and link status support Dong Yibo
2026-08-14 11:13 ` [PATCH net-next v9 1/4] net: rnpgbe: Add interrupt handling Dong Yibo
@ 2026-08-14 11:13 ` Dong Yibo
2026-08-18 11:03 ` Simon Horman
2026-08-14 11:13 ` [PATCH net-next v9 3/4] net: rnpgbe: Add RX packet reception support Dong Yibo
2026-08-14 11:13 ` [PATCH net-next v9 4/4] net: rnpgbe: Add link status handling support Dong Yibo
3 siblings, 1 reply; 9+ messages in thread
From: Dong Yibo @ 2026-08-14 11:13 UTC (permalink / raw)
To: andrew+netdev, davem, edumazet, kuba, pabeni, vadim.fedorenko,
u.kleine-koenig
Cc: netdev, linux-kernel, dong100, yaojun
Add the basic TX path for the RNPGBE driver.
Introduce the TX descriptor structure and buffer management, the
rnpgbe_xmit_frame_ring() transmit function, TX ring allocation and
teardown, and TX completion handling in rnpgbe_clean_tx_irq(). Track TX
packet and byte statistics, and report dropped and tx_dropped counts
through ndo_get_stats64().
Require the hardware's 56-bit streaming and coherent DMA mask. Add
cycles_per_us to struct mucse_hw to scale the TX interrupt timer, and
enable NETIF_F_SG plus NETIF_F_HIGHDMA when the DMA mask allows it.
Signed-off-by: Dong Yibo <dong100@mucse.com>
---
drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h | 79 ++-
.../net/ethernet/mucse/rnpgbe/rnpgbe_chip.c | 4 +
drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h | 7 +
.../net/ethernet/mucse/rnpgbe/rnpgbe_lib.c | 667 +++++++++++++++++-
.../net/ethernet/mucse/rnpgbe/rnpgbe_lib.h | 29 +
.../net/ethernet/mucse/rnpgbe/rnpgbe_main.c | 52 +-
6 files changed, 825 insertions(+), 13 deletions(-)
diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h
index 77304196c2b6..49fcbd2e3740 100644
--- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h
+++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h
@@ -44,20 +44,85 @@ struct mucse_hw {
struct pci_dev *pdev;
struct mucse_mbx_info mbx;
int port;
+ u16 cycles_per_us;
u8 pfvfnum;
};
+struct rnpgbe_tx_desc {
+ __le64 pkt_addr; /* Packet buffer address */
+ union {
+ __le64 vlan_cmd_bsz;
+ struct {
+ __le32 blen_mac_ip_len;
+ __le32 vlan_cmd; /* vlan & cmd status */
+ };
+ };
+#define M_TXD_CMD_RS 0x040000 /* Report Status */
+#define M_TXD_STAT_DD 0x020000 /* Descriptor Done */
+#define M_TXD_CMD_EOP 0x010000 /* End of Packet */
+};
+
+#define M_TX_DESC(R, i) (&(((struct rnpgbe_tx_desc *)((R)->desc))[i]))
+
+struct mucse_tx_buffer {
+ struct rnpgbe_tx_desc *next_to_watch;
+ struct sk_buff *skb;
+ unsigned int bytecount;
+ unsigned short gso_segs;
+ DEFINE_DMA_UNMAP_ADDR(dma);
+ DEFINE_DMA_UNMAP_LEN(len);
+ bool mapped_as_page; /* true if dma was mapped with dma_map_page */
+};
+
+struct mucse_queue_stats {
+ u64 packets;
+ u64 bytes;
+ atomic64_t dropped;
+};
+
struct mucse_ring {
struct mucse_ring *next;
struct mucse_q_vector *q_vector;
+ struct net_device *netdev;
+ struct device *dev;
+ void *desc;
+ struct mucse_tx_buffer *tx_buffer_info;
void __iomem *ring_addr;
+ void __iomem *tail;
void __iomem *irq_mask;
void __iomem *trig;
u8 queue_index;
/* hw ring idx */
u8 rnpgbe_queue_idx;
+ u8 pfvfnum;
+ u16 count;
+ u16 next_to_use;
+ u16 next_to_clean;
+ dma_addr_t dma;
+ unsigned int size;
+ struct mucse_queue_stats stats;
+ struct u64_stats_sync syncp;
} ____cacheline_internodealigned_in_smp;
+static inline u16 mucse_desc_unused(struct mucse_ring *ring)
+{
+ u16 ntc = ring->next_to_clean;
+ u16 ntu = ring->next_to_use;
+
+ return ((ntc > ntu) ? 0 : ring->count) + ntc - ntu - 1;
+}
+
+static inline __le64 build_ctob(u32 vlan_cmd, u32 mac_ip_len, u32 size)
+{
+ return cpu_to_le64(((u64)vlan_cmd << 32) | ((u64)mac_ip_len << 16) |
+ ((u64)size));
+}
+
+static inline struct netdev_queue *txring_txq(const struct mucse_ring *ring)
+{
+ return netdev_get_tx_queue(ring->netdev, ring->queue_index);
+}
+
struct mucse_ring_container {
struct mucse_ring *ring;
u16 count;
@@ -74,12 +139,11 @@ struct mucse_q_vector {
struct mucse_ring ring[] ____cacheline_internodealigned_in_smp;
};
-struct mucse_stats {
- u64 tx_dropped;
-};
-
#define MAX_Q_VECTORS 8
+#define M_DEFAULT_TXD 512
+#define M_DEFAULT_TX_WORK 256
+
enum mucse_state_t {
__MUCSE_DOWN,
};
@@ -88,7 +152,6 @@ struct mucse {
struct net_device *netdev;
struct pci_dev *pdev;
struct mucse_hw hw;
- struct mucse_stats stats;
#define M_FLAG_MSIX_SINGLE_EN BIT(0)
#define M_FLAG_MSIX_EN BIT(1)
u32 flags;
@@ -97,6 +160,8 @@ struct mucse {
struct mucse_ring *rx_ring[RNPGBE_MAX_QUEUES]
____cacheline_aligned_in_smp;
struct mucse_q_vector *q_vector[MAX_Q_VECTORS];
+ int tx_ring_item_count;
+ int tx_work_limit;
int num_tx_queues;
int num_q_vectors;
int num_rx_queues;
@@ -123,4 +188,8 @@ int rnpgbe_init_hw(struct mucse_hw *hw, int board_type);
writel((val), (hw)->hw_addr + (reg))
#define mucse_hw_rd32(hw, reg) \
readl((hw)->hw_addr + (reg))
+#define mucse_ring_wr32(ring, reg, val) \
+ writel((val), (ring)->ring_addr + (reg))
+#define mucse_ring_rd32(ring, reg) \
+ readl((ring)->ring_addr + (reg))
#endif /* _RNPGBE_H */
diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_chip.c b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_chip.c
index 921cc325a991..291e77d573fe 100644
--- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_chip.c
+++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_chip.c
@@ -93,6 +93,8 @@ static void rnpgbe_init_n500(struct mucse_hw *hw)
mbx->fwpf_ctrl_base = MUCSE_N500_FWPF_CTRL_BASE;
mbx->fwpf_shm_base = MUCSE_N500_FWPF_SHM_BASE;
+
+ hw->cycles_per_us = M_DEFAULT_N500_MHZ;
}
/**
@@ -110,6 +112,8 @@ static void rnpgbe_init_n210(struct mucse_hw *hw)
mbx->fwpf_ctrl_base = MUCSE_N210_FWPF_CTRL_BASE;
mbx->fwpf_shm_base = MUCSE_N210_FWPF_SHM_BASE;
+
+ hw->cycles_per_us = M_DEFAULT_N210_MHZ;
}
/**
diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h
index 0dce78e4a91b..6dc29ebe6fa7 100644
--- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h
+++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h
@@ -7,13 +7,20 @@
#define MUCSE_N500_FWPF_CTRL_BASE 0x28b00
#define MUCSE_N500_FWPF_SHM_BASE 0x2d000
#define MUCSE_N500_RING_MSIX_BASE 0x28700
+#define M_DEFAULT_N500_MHZ 125
#define MUCSE_GBE_PFFW_MBX_CTRL_OFFSET 0x5500
#define MUCSE_GBE_FWPF_MBX_MASK_OFFSET 0x5700
#define MUCSE_N210_FWPF_CTRL_BASE 0x29400
#define MUCSE_N210_FWPF_SHM_BASE 0x2d900
#define MUCSE_N210_RING_MSIX_BASE 0x29000
+#define M_DEFAULT_N210_MHZ 62
+#define RNPGBE_DMA_STATUS 0x0008
+#define TX_AXI_RW_EN 0xc
+/* DMA_STATUS_REG[23:20]: tx_wr, tx_rd, rx_wr, rx_rd done status. */
+#define RNPGBE_DMA_TX_STATUS GENMASK_U32(23, 22)
#define RNPGBE_DMA_AXI_EN 0x0010
+#define RNPGBE_TX_MIN_PKT_LEN 33
#define RNPGBE_MAX_QUEUES 8
#endif /* _RNPGBE_HW_H */
diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_lib.c b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_lib.c
index c661290d561b..8700c48669ec 100644
--- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_lib.c
+++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_lib.c
@@ -3,6 +3,9 @@
#include <linux/pci.h>
#include <linux/netdevice.h>
+#include <linux/iopoll.h>
+#include <linux/vmalloc.h>
+#include <net/netdev_queues.h>
#include "rnpgbe_lib.h"
#include "rnpgbe.h"
@@ -43,7 +46,6 @@ static void rnpgbe_irq_disable_queues(struct mucse_q_vector *q_vector)
writel(INT_VALID, ring->trig);
writel((RX_INT_MASK | TX_INT_MASK), ring->irq_mask);
}
-
/* flush posted writes to ensure hardware sees the mask */
if (q_vector->tx.ring)
readl(q_vector->tx.ring->irq_mask);
@@ -89,6 +91,114 @@ static void rnpgbe_irq_enable_queues(struct mucse_q_vector *q_vector)
}
}
+/**
+ * rnpgbe_clean_tx_irq - Reclaim resources after transmit completes
+ * @q_vector: structure containing interrupt and ring information
+ * @tx_ring: tx ring to clean
+ * @napi_budget: Used to determine if we are in netpoll
+ *
+ * Return: true if cleaning completed within the budget, otherwise false
+ **/
+static bool rnpgbe_clean_tx_irq(struct mucse_q_vector *q_vector,
+ struct mucse_ring *tx_ring,
+ int napi_budget)
+{
+ int budget = q_vector->mucse->tx_work_limit;
+ u64 total_bytes = 0, total_packets = 0;
+ struct mucse *mucse = q_vector->mucse;
+ struct mucse_tx_buffer *tx_buffer;
+ struct rnpgbe_tx_desc *tx_desc;
+ int i = tx_ring->next_to_clean;
+
+ tx_buffer = &tx_ring->tx_buffer_info[i];
+ tx_desc = M_TX_DESC(tx_ring, i);
+ i -= tx_ring->count;
+
+ do {
+ struct rnpgbe_tx_desc *eop_desc = tx_buffer->next_to_watch;
+
+ /* if next_to_watch is not set then there is no work pending */
+ if (!eop_desc)
+ break;
+
+ /* prevent any other reads prior to eop_desc */
+ dma_rmb();
+
+ /* if eop DD is not set pending work has not been completed */
+ if (!(eop_desc->vlan_cmd & cpu_to_le32(M_TXD_STAT_DD)))
+ break;
+ /* clear next_to_watch to prevent false hangs */
+ tx_buffer->next_to_watch = NULL;
+ total_bytes += tx_buffer->bytecount;
+ total_packets += tx_buffer->gso_segs;
+ napi_consume_skb(tx_buffer->skb, napi_budget);
+ if (tx_buffer->mapped_as_page) {
+ dma_unmap_page(tx_ring->dev,
+ dma_unmap_addr(tx_buffer, dma),
+ dma_unmap_len(tx_buffer, len),
+ DMA_TO_DEVICE);
+ } else {
+ dma_unmap_single(tx_ring->dev,
+ dma_unmap_addr(tx_buffer, dma),
+ dma_unmap_len(tx_buffer, len),
+ DMA_TO_DEVICE);
+ }
+ tx_buffer->skb = NULL;
+ dma_unmap_len_set(tx_buffer, len, 0);
+
+ /* unmap remaining buffers */
+ while (tx_desc != eop_desc) {
+ tx_buffer++;
+ tx_desc++;
+ i++;
+ if (unlikely(!i)) {
+ i -= tx_ring->count;
+ tx_buffer = tx_ring->tx_buffer_info;
+ tx_desc = M_TX_DESC(tx_ring, 0);
+ }
+
+ /* unmap any remaining paged data */
+ if (dma_unmap_len(tx_buffer, len)) {
+ dma_unmap_page(tx_ring->dev,
+ dma_unmap_addr(tx_buffer, dma),
+ dma_unmap_len(tx_buffer, len),
+ DMA_TO_DEVICE);
+ dma_unmap_len_set(tx_buffer, len, 0);
+ }
+ }
+
+ /* move us one more past the eop_desc for start of next pkt */
+ tx_buffer++;
+ tx_desc++;
+ i++;
+ if (unlikely(!i)) {
+ i -= tx_ring->count;
+ tx_buffer = tx_ring->tx_buffer_info;
+ tx_desc = M_TX_DESC(tx_ring, 0);
+ }
+
+ prefetch(tx_desc);
+ budget--;
+ } while (likely(budget > 0));
+
+ i += tx_ring->count;
+ tx_ring->next_to_clean = i;
+ u64_stats_update_begin(&tx_ring->syncp);
+ tx_ring->stats.bytes += total_bytes;
+ tx_ring->stats.packets += total_packets;
+ u64_stats_update_end(&tx_ring->syncp);
+
+#define TX_WAKE_THRESHOLD (DESC_NEEDED * 2)
+ __netif_txq_completed_wake(txring_txq(tx_ring),
+ total_packets, total_bytes,
+ mucse_desc_unused(tx_ring),
+ TX_WAKE_THRESHOLD,
+ !netif_carrier_ok(tx_ring->netdev) ||
+ test_bit(__MUCSE_DOWN, &mucse->state));
+
+ return !!budget;
+}
+
/**
* rnpgbe_poll - NAPI Rx polling callback
* @napi: structure for representing this polling device
@@ -102,12 +212,22 @@ static int rnpgbe_poll(struct napi_struct *napi, int budget)
{
struct mucse_q_vector *q_vector =
container_of(napi, struct mucse_q_vector, napi);
+ bool clean_complete = true;
+ struct mucse_ring *ring;
int work_done = 0;
+ mucse_for_each_ring(ring, q_vector->tx) {
+ if (!rnpgbe_clean_tx_irq(q_vector, ring, budget))
+ clean_complete = false;
+ }
+
/* Exit if we are called by netpoll */
if (unlikely(!budget))
return 0;
+ if (!clean_complete)
+ return budget;
+
if (likely(napi_complete_done(napi, work_done)))
rnpgbe_irq_enable_queues(q_vector);
@@ -276,13 +396,18 @@ static int rnpgbe_alloc_q_vector(struct mucse *mucse,
ring = q_vector->ring;
for (idx = 0; idx < txr_count; idx++) {
+ ring->dev = &mucse->pdev->dev;
mucse_add_ring(ring, &q_vector->tx);
+ ring->count = mucse->tx_ring_item_count;
+ ring->netdev = mucse->netdev;
ring->queue_index = eth_queue_idx + idx;
ring->rnpgbe_queue_idx = txr_idx;
ring->ring_addr = hw->hw_addr + RING_OFFSET(txr_idx);
ring->irq_mask = ring->ring_addr + RNPGBE_DMA_INT_MASK;
ring->trig = ring->ring_addr + RNPGBE_DMA_INT_TRIG;
ring->q_vector = q_vector;
+ ring->pfvfnum = hw->pfvfnum;
+ u64_stats_init(&ring->syncp);
mucse->tx_ring[ring->queue_index] = ring;
txr_idx += step;
ring++;
@@ -633,13 +758,144 @@ static void rnpgbe_napi_disable_all(struct mucse *mucse)
napi_disable(&mucse->q_vector[i]->napi);
}
+static void rnpgbe_stop_tx_ring(struct mucse_ring *tx_ring)
+{
+ if (!tx_ring->tx_buffer_info)
+ return;
+
+ /* Stop hw. No new descriptors are fetched after TX_START=0.
+ * DMA for descriptors fetched before the stop may still be in flight.
+ */
+ mucse_ring_wr32(tx_ring, RNPGBE_TX_START, 0);
+ /* Flush posted write to ensure hardware sees TX_START=0 */
+ (void)mucse_ring_rd32(tx_ring, RNPGBE_TX_START);
+}
+
+static void rnpgbe_wait_tx_dma_idle(struct mucse *mucse)
+{
+ struct mucse_hw *hw = &mucse->hw;
+ u32 dma_status;
+ int err;
+
+ err = readl_poll_timeout(hw->hw_addr + RNPGBE_DMA_STATUS,
+ dma_status,
+ (dma_status & RNPGBE_DMA_TX_STATUS) ==
+ RNPGBE_DMA_TX_STATUS,
+ 10, 100000);
+ if (err)
+ dev_warn(&mucse->pdev->dev,
+ "Timed out waiting for TX DMA to quiesce, status %#x\n",
+ dma_status);
+}
+
+/**
+ * rnpgbe_clean_tx_ring - Free Tx Buffers
+ * @tx_ring: ring to be cleaned
+ **/
+static void rnpgbe_clean_tx_ring(struct mucse_ring *tx_ring)
+{
+ struct mucse_tx_buffer *tx_buffer;
+ u16 i = tx_ring->next_to_clean;
+ unsigned long size;
+
+ /* ring already cleared, nothing to do */
+ if (!tx_ring->tx_buffer_info)
+ return;
+
+ tx_buffer = &tx_ring->tx_buffer_info[i];
+
+ while (i != tx_ring->next_to_use) {
+ struct rnpgbe_tx_desc *eop_desc, *tx_desc;
+
+ dev_kfree_skb_any(tx_buffer->skb);
+ /* unmap skb header data */
+ if (dma_unmap_len(tx_buffer, len)) {
+ if (tx_buffer->mapped_as_page) {
+ dma_unmap_page(tx_ring->dev,
+ dma_unmap_addr(tx_buffer, dma),
+ dma_unmap_len(tx_buffer, len),
+ DMA_TO_DEVICE);
+ } else {
+ dma_unmap_single(tx_ring->dev,
+ dma_unmap_addr(tx_buffer, dma),
+ dma_unmap_len(tx_buffer, len),
+ DMA_TO_DEVICE);
+ }
+ }
+ eop_desc = tx_buffer->next_to_watch;
+ tx_desc = M_TX_DESC(tx_ring, i);
+ /* unmap remaining buffers */
+ while (tx_desc != eop_desc) {
+ tx_buffer++;
+ tx_desc++;
+ i++;
+ if (unlikely(i == tx_ring->count)) {
+ i = 0;
+ tx_buffer = tx_ring->tx_buffer_info;
+ tx_desc = M_TX_DESC(tx_ring, 0);
+ }
+
+ /* unmap any remaining paged data */
+ if (dma_unmap_len(tx_buffer, len))
+ dma_unmap_page(tx_ring->dev,
+ dma_unmap_addr(tx_buffer, dma),
+ dma_unmap_len(tx_buffer, len),
+ DMA_TO_DEVICE);
+ }
+ /* move us one more past the eop_desc for start of next pkt */
+ tx_buffer++;
+ i++;
+ if (unlikely(i == tx_ring->count)) {
+ i = 0;
+ tx_buffer = tx_ring->tx_buffer_info;
+ }
+ }
+
+ netdev_tx_reset_queue(txring_txq(tx_ring));
+ size = sizeof(struct mucse_tx_buffer) * tx_ring->count;
+ memset(tx_ring->tx_buffer_info, 0, size);
+ /* Zero out the descriptor ring */
+ memset(tx_ring->desc, 0, tx_ring->size);
+ tx_ring->next_to_use = 0;
+ tx_ring->next_to_clean = 0;
+}
+
+/**
+ * rnpgbe_clean_all_tx_rings - Stop TX DMA and free Tx buffers for all queues
+ * @mucse: board private structure
+ **/
+static void rnpgbe_clean_all_tx_rings(struct mucse *mucse)
+{
+ struct mucse_hw *hw = &mucse->hw;
+ u32 dma_axi_ctl;
+
+ for (int i = 0; i < mucse->num_tx_queues; i++)
+ rnpgbe_stop_tx_ring(mucse->tx_ring[i]);
+
+ if (mucse->num_tx_queues)
+ rnpgbe_wait_tx_dma_idle(mucse);
+
+ dma_axi_ctl = mucse_hw_rd32(hw, RNPGBE_DMA_AXI_EN);
+ dma_axi_ctl &= ~TX_AXI_RW_EN;
+ mucse_hw_wr32(hw, RNPGBE_DMA_AXI_EN, dma_axi_ctl);
+ /* Flush the posted write before releasing the DMA mappings. */
+ (void)mucse_hw_rd32(hw, RNPGBE_DMA_AXI_EN);
+
+ for (int i = 0; i < mucse->num_tx_queues; i++)
+ rnpgbe_clean_tx_ring(mucse->tx_ring[i]);
+}
+
bool rnpgbe_down(struct mucse *mucse)
{
+ struct net_device *netdev = mucse->netdev;
+
if (test_and_set_bit(__MUCSE_DOWN, &mucse->state))
return false;
+ netif_tx_disable(netdev);
rnpgbe_napi_disable_all(mucse);
rnpgbe_irq_disable(mucse);
+ rnpgbe_clean_all_tx_rings(mucse);
return true;
}
@@ -650,11 +906,420 @@ bool rnpgbe_down(struct mucse *mucse)
**/
void rnpgbe_up_complete(struct mucse *mucse)
{
+ struct net_device *netdev = mucse->netdev;
+
if (mucse->flags & (M_FLAG_MSIX_EN | M_FLAG_MSIX_SINGLE_EN))
rnpgbe_configure_msix(mucse);
else
rnpgbe_configure_msi(mucse);
+
rnpgbe_napi_enable_all(mucse);
clear_bit(__MUCSE_DOWN, &mucse->state);
rnpgbe_irq_enable(mucse);
+ netif_tx_start_all_queues(netdev);
+}
+
+/**
+ * rnpgbe_free_tx_resources - Free Tx Resources per Queue
+ * @tx_ring: tx descriptor ring for a specific queue
+ *
+ * Free all transmit software resources
+ **/
+static void rnpgbe_free_tx_resources(struct mucse_ring *tx_ring)
+{
+ vfree(tx_ring->tx_buffer_info);
+ tx_ring->tx_buffer_info = NULL;
+ /* if not set, then don't free */
+ if (!tx_ring->desc)
+ return;
+
+ dma_free_coherent(tx_ring->dev, tx_ring->size, tx_ring->desc,
+ tx_ring->dma);
+ tx_ring->desc = NULL;
+}
+
+/**
+ * rnpgbe_setup_tx_resources - allocate Tx resources (Descriptors)
+ * @tx_ring: tx descriptor ring (for a specific queue) to setup
+ * @mucse: pointer to private structure
+ *
+ * Return: 0 on success, negative on failure
+ **/
+static int rnpgbe_setup_tx_resources(struct mucse_ring *tx_ring,
+ struct mucse *mucse)
+{
+ struct device *dev = tx_ring->dev;
+ int size;
+
+ size = sizeof(struct mucse_tx_buffer) * tx_ring->count;
+
+ tx_ring->tx_buffer_info = vzalloc(size);
+ if (!tx_ring->tx_buffer_info)
+ goto err_return;
+ /* round up to nearest 4K */
+ tx_ring->size = tx_ring->count * sizeof(struct rnpgbe_tx_desc);
+ tx_ring->size = ALIGN(tx_ring->size, 4096);
+ tx_ring->desc = dma_alloc_coherent(dev, tx_ring->size, &tx_ring->dma,
+ GFP_KERNEL);
+ if (!tx_ring->desc)
+ goto err_free_buffer;
+
+ tx_ring->next_to_use = 0;
+ tx_ring->next_to_clean = 0;
+
+ return 0;
+
+err_free_buffer:
+ vfree(tx_ring->tx_buffer_info);
+err_return:
+ tx_ring->tx_buffer_info = NULL;
+ return -ENOMEM;
+}
+
+/**
+ * rnpgbe_configure_tx_ring - Configure Tx ring after Reset
+ * @mucse: pointer to private structure
+ * @ring: structure containing ring specific data
+ *
+ * Configure the Tx descriptor ring after a reset.
+ **/
+static void rnpgbe_configure_tx_ring(struct mucse *mucse,
+ struct mucse_ring *ring)
+{
+ struct mucse_hw *hw = &mucse->hw;
+
+ mucse_ring_wr32(ring, RNPGBE_TX_BASE_ADDR_LO, (u32)ring->dma);
+ mucse_ring_wr32(ring, RNPGBE_TX_BASE_ADDR_HI,
+ (u32)(((u64)ring->dma) >> 32) | (hw->pfvfnum << 24));
+ mucse_ring_wr32(ring, RNPGBE_TX_LEN, ring->count);
+ ring->next_to_clean = mucse_ring_rd32(ring, RNPGBE_TX_HEAD) %
+ ring->count;
+ ring->next_to_use = ring->next_to_clean;
+ ring->tail = ring->ring_addr + RNPGBE_TX_TAIL;
+ writel(ring->next_to_use, ring->tail);
+ mucse_ring_wr32(ring, RNPGBE_TX_FETCH_CTRL, M_DEFAULT_TX_FETCH);
+ mucse_ring_wr32(ring, RNPGBE_TX_INT_TIMER,
+ M_DEFAULT_INT_TIMER * hw->cycles_per_us);
+ mucse_ring_wr32(ring, RNPGBE_TX_INT_PKTCNT, M_DEFAULT_INT_PKTCNT);
+}
+
+/**
+ * rnpgbe_configure_tx - Configure Transmit Unit after Reset
+ * @mucse: pointer to private structure
+ *
+ * Configure the Tx DMA after a reset.
+ **/
+void rnpgbe_configure_tx(struct mucse *mucse)
+{
+ struct mucse_hw *hw = &mucse->hw;
+ u32 i, dma_axi_ctl;
+
+ for (i = 0; i < mucse->num_tx_queues; i++)
+ rnpgbe_stop_tx_ring(mucse->tx_ring[i]);
+ if (mucse->num_tx_queues)
+ rnpgbe_wait_tx_dma_idle(mucse);
+
+ dma_axi_ctl = mucse_hw_rd32(hw, RNPGBE_DMA_AXI_EN);
+ dma_axi_ctl &= ~TX_AXI_RW_EN;
+ mucse_hw_wr32(hw, RNPGBE_DMA_AXI_EN, dma_axi_ctl);
+ /* Flush the posted write before programming the ring registers. */
+ (void)mucse_hw_rd32(hw, RNPGBE_DMA_AXI_EN);
+
+ for (i = 0; i < mucse->num_tx_queues; i++)
+ rnpgbe_configure_tx_ring(mucse, mucse->tx_ring[i]);
+
+ /* Ensure all ring configuration is visible before enabling TX DMA. */
+ wmb();
+ dma_axi_ctl |= TX_AXI_RW_EN;
+ mucse_hw_wr32(hw, RNPGBE_DMA_AXI_EN, dma_axi_ctl);
+ /* Ensure TX_AXI_RW_EN is visible before starting the rings. */
+ (void)mucse_hw_rd32(hw, RNPGBE_DMA_AXI_EN);
+
+ for (i = 0; i < mucse->num_tx_queues; i++)
+ mucse_ring_wr32(mucse->tx_ring[i], RNPGBE_TX_START, 1);
+}
+
+/**
+ * rnpgbe_setup_all_tx_resources - allocate all queues Tx resources
+ * @mucse: pointer to private structure
+ *
+ * Allocate memory for tx_ring.
+ *
+ * Return: 0 on success, negative on failure
+ **/
+int rnpgbe_setup_all_tx_resources(struct mucse *mucse)
+{
+ int i, err = 0;
+
+ for (i = 0; i < mucse->num_tx_queues; i++) {
+ err = rnpgbe_setup_tx_resources(mucse->tx_ring[i], mucse);
+ if (!err)
+ continue;
+
+ goto err_free_res;
+ }
+
+ return 0;
+err_free_res:
+ while (i--)
+ rnpgbe_free_tx_resources(mucse->tx_ring[i]);
+ return err;
+}
+
+/**
+ * rnpgbe_free_all_tx_resources - Free Tx Resources for All Queues
+ * @mucse: pointer to private structure
+ *
+ * Free all transmit software resources
+ **/
+void rnpgbe_free_all_tx_resources(struct mucse *mucse)
+{
+ for (int i = 0; i < (mucse->num_tx_queues); i++)
+ rnpgbe_free_tx_resources(mucse->tx_ring[i]);
+}
+
+static int rnpgbe_tx_map(struct mucse_ring *tx_ring,
+ struct mucse_tx_buffer *first, u32 mac_ip_len,
+ u32 tx_flags)
+{
+ struct mucse_tx_buffer *tx_buffer;
+ struct rnpgbe_tx_desc *tx_desc;
+ struct skb_shared_info *shinfo;
+ unsigned int data_len, size;
+ struct sk_buff *skb;
+ skb_frag_t *frag;
+ dma_addr_t dma;
+ int nr_frags;
+ int frag_idx;
+ /* Hardware requires this in the upper 8 bits of
+ * the descriptor address
+ */
+ u64 fun_id;
+ u16 i;
+
+ skb = first->skb;
+ shinfo = skb_shinfo(skb);
+ fun_id = (u64)tx_ring->pfvfnum << 56;
+ nr_frags = shinfo->nr_frags;
+ i = tx_ring->next_to_use;
+ frag_idx = 0;
+ tx_desc = M_TX_DESC(tx_ring, i);
+ size = skb_headlen(skb);
+ data_len = skb->data_len;
+
+ if (size) {
+ dma = dma_map_single(tx_ring->dev, skb->data, size,
+ DMA_TO_DEVICE);
+ first->mapped_as_page = false;
+ } else if (data_len) {
+ while (frag_idx < nr_frags &&
+ !skb_frag_size(&shinfo->frags[frag_idx]))
+ frag_idx++;
+
+ if (frag_idx == nr_frags)
+ goto err_unmap;
+
+ frag = &shinfo->frags[frag_idx];
+ size = skb_frag_size(frag);
+
+ dma = skb_frag_dma_map(tx_ring->dev, frag, 0,
+ size, DMA_TO_DEVICE);
+ first->mapped_as_page = true;
+ data_len -= size;
+ frag_idx++;
+ } else {
+ goto err_unmap;
+ }
+
+ tx_buffer = first;
+
+ dma_unmap_len_set(tx_buffer, len, 0);
+ dma_unmap_addr_set(tx_buffer, dma, 0);
+
+ for (;; frag_idx++) {
+ if (dma_mapping_error(tx_ring->dev, dma))
+ goto err_unmap;
+
+ /* record length, and DMA address */
+ dma_unmap_len_set(tx_buffer, len, size);
+ dma_unmap_addr_set(tx_buffer, dma, dma);
+
+ tx_desc->pkt_addr = cpu_to_le64(dma | fun_id);
+
+ while (unlikely(size > M_MAX_DATA_PER_TXD)) {
+ tx_desc->vlan_cmd_bsz = build_ctob(tx_flags,
+ mac_ip_len,
+ M_MAX_DATA_PER_TXD);
+ i++;
+ tx_desc++;
+ if (i == tx_ring->count) {
+ tx_desc = M_TX_DESC(tx_ring, 0);
+ i = 0;
+ }
+
+ tx_buffer = &tx_ring->tx_buffer_info[i];
+ dma_unmap_len_set(tx_buffer, len, 0);
+ dma += M_MAX_DATA_PER_TXD;
+ size -= M_MAX_DATA_PER_TXD;
+ tx_desc->pkt_addr = cpu_to_le64(dma | fun_id);
+ }
+
+ if (likely(!data_len))
+ break;
+ tx_desc->vlan_cmd_bsz = build_ctob(tx_flags, mac_ip_len, size);
+ i++;
+ tx_desc++;
+ if (i == tx_ring->count) {
+ tx_desc = M_TX_DESC(tx_ring, 0);
+ i = 0;
+ }
+
+ while (frag_idx < nr_frags &&
+ !skb_frag_size(&shinfo->frags[frag_idx]))
+ frag_idx++;
+
+ if (frag_idx == nr_frags)
+ goto err_unmap;
+
+ frag = &shinfo->frags[frag_idx];
+ size = skb_frag_size(frag);
+ data_len -= size;
+ dma = skb_frag_dma_map(tx_ring->dev, frag, 0, size,
+ DMA_TO_DEVICE);
+ tx_buffer = &tx_ring->tx_buffer_info[i];
+ tx_buffer->mapped_as_page = true;
+ }
+
+ /* write last descriptor with RS and EOP bits */
+ tx_desc->vlan_cmd_bsz = build_ctob(tx_flags | M_TXD_CMD_EOP |
+ M_TXD_CMD_RS,
+ mac_ip_len, size);
+
+ /* Force memory writes to complete before letting h/w know there
+ * are new descriptors to fetch. (Only applicable for weak-ordered
+ * memory model archs, such as IA-64).
+ *
+ * We also need this memory barrier to make certain all of the
+ * status bits have been updated before next_to_watch is written.
+ */
+ dma_wmb();
+ /* set next_to_watch value indicating a packet is present */
+ first->next_to_watch = tx_desc;
+ i++;
+ if (i == tx_ring->count)
+ i = 0;
+ tx_ring->next_to_use = i;
+ skb_tx_timestamp(skb);
+ netdev_tx_sent_queue(txring_txq(tx_ring), first->bytecount);
+ /* notify HW of packet */
+ writel(i, tx_ring->tail);
+
+ return 0;
+err_unmap:
+ for (;;) {
+ tx_buffer = &tx_ring->tx_buffer_info[i];
+ if (dma_unmap_len(tx_buffer, len)) {
+ if (tx_buffer->mapped_as_page) {
+ dma_unmap_page(tx_ring->dev,
+ dma_unmap_addr(tx_buffer, dma),
+ dma_unmap_len(tx_buffer, len),
+ DMA_TO_DEVICE);
+ } else {
+ dma_unmap_single(tx_ring->dev,
+ dma_unmap_addr(tx_buffer, dma),
+ dma_unmap_len(tx_buffer, len),
+ DMA_TO_DEVICE);
+ }
+ }
+ dma_unmap_len_set(tx_buffer, len, 0);
+ dma_unmap_addr_set(tx_buffer, dma, 0);
+ if (tx_buffer == first)
+ break;
+ if (i == 0)
+ i += tx_ring->count;
+ i--;
+ }
+ dev_kfree_skb_any(first->skb);
+ first->skb = NULL;
+ tx_ring->next_to_use = i;
+
+ return -ENOMEM;
+}
+
+netdev_tx_t rnpgbe_xmit_frame_ring(struct sk_buff *skb,
+ struct mucse_ring *tx_ring)
+{
+ u16 count;
+ /* hw requires non-zero mac_ip_len even without offload;
+ * Will be updated per-packet when offload is added
+ */
+ u32 mac_ip_len = M_DEFAULT_MAC_IP_LEN;
+ struct mucse_tx_buffer *first;
+ u32 tx_flags = 0;
+ unsigned short f;
+
+ count = TXD_USE_COUNT(skb_headlen(skb));
+ for (f = 0; f < skb_shinfo(skb)->nr_frags; f++) {
+ skb_frag_t *frag_temp = &skb_shinfo(skb)->frags[f];
+
+ count += TXD_USE_COUNT(skb_frag_size(frag_temp));
+ }
+
+ if (!netif_txq_maybe_stop(txring_txq(tx_ring),
+ mucse_desc_unused(tx_ring),
+ count + RESV_DESC_NEEDED,
+ count + RESV_DESC_NEEDED))
+ return NETDEV_TX_BUSY;
+
+ /* record the location of the first descriptor for this packet */
+ first = &tx_ring->tx_buffer_info[tx_ring->next_to_use];
+ first->skb = skb;
+ first->bytecount = skb->len;
+ first->gso_segs = 1;
+
+ if (rnpgbe_tx_map(tx_ring, first, mac_ip_len, tx_flags)) {
+ atomic64_inc(&tx_ring->stats.dropped);
+
+ goto out;
+ }
+
+ /* NETDEV_TX_BUSY is expensive. So stop advancing the TX queue. */
+ netif_txq_maybe_stop(txring_txq(tx_ring),
+ mucse_desc_unused(tx_ring),
+ DESC_NEEDED, DESC_NEEDED);
+out:
+ return NETDEV_TX_OK;
+}
+
+/**
+ * rnpgbe_get_stats64 - Get stats for this netdev
+ * @netdev: network interface device structure
+ * @stats: stats data
+ **/
+void rnpgbe_get_stats64(struct net_device *netdev,
+ struct rtnl_link_stats64 *stats)
+{
+ struct mucse *mucse = netdev_priv(netdev);
+ int i;
+
+ rcu_read_lock();
+ for (i = 0; i < mucse->num_tx_queues; i++) {
+ struct mucse_ring *ring = READ_ONCE(mucse->tx_ring[i]);
+ u64 bytes, packets, dropped;
+ unsigned int start;
+
+ if (ring) {
+ do {
+ start = u64_stats_fetch_begin(&ring->syncp);
+ packets = ring->stats.packets;
+ bytes = ring->stats.bytes;
+ } while (u64_stats_fetch_retry(&ring->syncp, start));
+ dropped = atomic64_read(&ring->stats.dropped);
+
+ stats->tx_packets += packets;
+ stats->tx_dropped += dropped;
+ stats->tx_bytes += bytes;
+ }
+ }
+ rcu_read_unlock();
}
diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_lib.h b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_lib.h
index c03f7aad2c08..0ac4514b07c7 100644
--- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_lib.h
+++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_lib.h
@@ -6,17 +6,39 @@
struct mucse;
struct mucse_hw;
+struct mucse_ring;
#define RING_OFFSET(n) (0x1000 + 0x100 * (n))
+#define RNPGBE_TX_START 0x18
#define RNPGBE_DMA_INT_MASK 0x24
#define TX_INT_MASK BIT(1)
#define RX_INT_MASK BIT(0)
#define INT_VALID (BIT(16) | BIT(17))
#define RNPGBE_DMA_INT_TRIG 0x2c /* lost-interrupt recovery trigger */
+#define RNPGBE_TX_BASE_ADDR_HI 0x60
+#define RNPGBE_TX_BASE_ADDR_LO 0x64
+#define RNPGBE_TX_LEN 0x68
+#define RNPGBE_TX_HEAD 0x6c
+#define RNPGBE_TX_TAIL 0x70
+#define M_DEFAULT_TX_FETCH 0x80008
+#define RNPGBE_TX_FETCH_CTRL 0x74
+#define M_DEFAULT_INT_TIMER 100
+#define RNPGBE_TX_INT_TIMER 0x78
+#define M_DEFAULT_INT_PKTCNT 48
+#define RNPGBE_TX_INT_PKTCNT 0x7c
/* | 31:24 | .... | 15:8 | 7:0 | */
/* | pfvfnum | | tx vector | rx vector | */
#define RING_VECTOR(n) (0x04 * (n))
+#define M_MAX_TXD_PWR 12
+#define M_MAX_DATA_PER_TXD (0x1 << M_MAX_TXD_PWR)
+#define TXD_USE_COUNT(S) DIV_ROUND_UP((S), M_MAX_DATA_PER_TXD)
+#define DESC_NEEDED (MAX_SKB_FRAGS + 4)
+/* 2 desc gap to keep tail from touching head */
+/* 1 desc for context descriptor */
+#define RESV_DESC_NEEDED 3
+/* Hardware requires this to be nonzero */
+#define M_DEFAULT_MAC_IP_LEN 20
#define mucse_for_each_ring(pos, head)\
for (typeof((head).ring) __pos = (head).ring;\
__pos ? ({ pos = __pos; 1; }) : 0;\
@@ -31,4 +53,11 @@ void rnpgbe_free_irq(struct mucse *mucse);
void rnpgbe_irq_disable(struct mucse *mucse);
bool rnpgbe_down(struct mucse *mucse);
void rnpgbe_up_complete(struct mucse *mucse);
+void rnpgbe_configure_tx(struct mucse *mucse);
+int rnpgbe_setup_all_tx_resources(struct mucse *mucse);
+void rnpgbe_free_all_tx_resources(struct mucse *mucse);
+netdev_tx_t rnpgbe_xmit_frame_ring(struct sk_buff *skb,
+ struct mucse_ring *tx_ring);
+void rnpgbe_get_stats64(struct net_device *netdev,
+ struct rtnl_link_stats64 *stats);
#endif
diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c
index c3296a3dde0b..24f4cf27a307 100644
--- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c
+++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c
@@ -2,6 +2,7 @@
/* Copyright(c) 2020 - 2025 Mucse Corporation. */
#include <linux/pci.h>
+#include <linux/skbuff.h>
#include <net/rtnetlink.h>
#include <linux/etherdevice.h>
@@ -26,6 +27,17 @@ static struct pci_device_id rnpgbe_pci_tbl[] = {
{ },
};
+/**
+ * rnpgbe_configure - Configure the hardware
+ * @mucse: pointer to private structure
+ *
+ * rnpgbe_configure configure mac, tx, rx regs to hw
+ **/
+static void rnpgbe_configure(struct mucse *mucse)
+{
+ rnpgbe_configure_tx(mucse);
+}
+
/**
* rnpgbe_open - Called when a network interface is made active
* @netdev: network interface device structure
@@ -49,6 +61,11 @@ static int rnpgbe_open(struct net_device *netdev)
if (err)
goto err_free_irqs;
+ err = rnpgbe_setup_all_tx_resources(mucse);
+ if (err)
+ goto err_free_irqs;
+
+ rnpgbe_configure(mucse);
rnpgbe_up_complete(mucse);
return 0;
@@ -74,6 +91,7 @@ static int rnpgbe_close(struct net_device *netdev)
return 0;
rnpgbe_free_irq(mucse);
+ rnpgbe_free_all_tx_resources(mucse);
return 0;
}
@@ -83,25 +101,37 @@ static int rnpgbe_close(struct net_device *netdev)
* @skb: skb structure to be sent
* @netdev: network interface device structure
*
- * Return: NETDEV_TX_OK
+ * Return: NETDEV_TX_OK or NETDEV_TX_BUSY when insufficient descriptors
**/
static netdev_tx_t rnpgbe_xmit_frame(struct sk_buff *skb,
struct net_device *netdev)
{
struct mucse *mucse = netdev_priv(netdev);
+ struct mucse_ring *tx_ring;
- dev_kfree_skb_any(skb);
- mucse->stats.tx_dropped++;
+ tx_ring = mucse->tx_ring[skb_get_queue_mapping(skb)];
- return NETDEV_TX_OK;
+ if (unlikely(skb_put_padto(skb, RNPGBE_TX_MIN_PKT_LEN))) {
+ atomic64_inc(&tx_ring->stats.dropped);
+ return NETDEV_TX_OK;
+ }
+
+ return rnpgbe_xmit_frame_ring(skb, tx_ring);
}
static const struct net_device_ops rnpgbe_netdev_ops = {
.ndo_open = rnpgbe_open,
.ndo_stop = rnpgbe_close,
.ndo_start_xmit = rnpgbe_xmit_frame,
+ .ndo_get_stats64 = rnpgbe_get_stats64,
};
+static void rnpgbe_sw_init(struct mucse *mucse)
+{
+ mucse->tx_ring_item_count = M_DEFAULT_TXD;
+ mucse->tx_work_limit = M_DEFAULT_TX_WORK;
+}
+
/**
* rnpgbe_add_adapter - Add netdev for this pci_dev
* @pdev: PCI device information structure
@@ -175,6 +205,7 @@ static int rnpgbe_add_adapter(struct pci_dev *pdev,
}
netdev->netdev_ops = &rnpgbe_netdev_ops;
+ rnpgbe_sw_init(mucse);
err = rnpgbe_reset_hw(hw);
if (err) {
dev_err(&pdev->dev, "Hw reset failed %d\n", err);
@@ -204,6 +235,13 @@ static int rnpgbe_add_adapter(struct pci_dev *pdev,
goto err_clear_interrupt;
}
+ netdev->features |= NETIF_F_SG;
+ netdev->hw_features |= NETIF_F_SG;
+ if (dma_get_mask(&pdev->dev) > DMA_BIT_MASK(32)) {
+ netdev->features |= NETIF_F_HIGHDMA;
+ netdev->hw_features |= NETIF_F_HIGHDMA;
+ }
+
err = register_netdev(netdev);
if (err)
goto err_remove_mbx;
@@ -246,10 +284,10 @@ static int rnpgbe_probe(struct pci_dev *pdev, const struct pci_device_id *id)
if (err)
return err;
- err = dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(56));
+ err = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(56));
if (err) {
- dev_err(&pdev->dev,
- "No usable DMA configuration, aborting %d\n", err);
+ dev_err(&pdev->dev, "No usable DMA configuration, aborting %d\n",
+ err);
goto err_disable_dev;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH net-next v9 2/4] net: rnpgbe: Add basic TX packet transmission support
2026-08-14 11:13 ` [PATCH net-next v9 2/4] net: rnpgbe: Add basic TX packet transmission support Dong Yibo
@ 2026-08-18 11:03 ` Simon Horman
0 siblings, 0 replies; 9+ messages in thread
From: Simon Horman @ 2026-08-18 11:03 UTC (permalink / raw)
To: Dong Yibo
Cc: andrew+netdev, davem, edumazet, kuba, pabeni, vadim.fedorenko,
u.kleine-koenig, netdev, linux-kernel, yaojun
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260814111317.1741087-2-dong100@mucse.com?part=2
---
--- Patch [2]: [PATCH net-next v9 2/4] net: rnpgbe: Add basic TX packet transmission support ---
commit 8bff6db2aa4a278636a66ac5e08b9b7d42fd23a7
Author: Dong Yibo <dong100@mucse.com>
net: rnpgbe: Add basic TX packet transmission support
This adds the TX path for rnpgbe: the TX descriptor layout and buffer
management, rnpgbe_xmit_frame_ring(), TX ring setup/teardown, and TX
completion handling in rnpgbe_clean_tx_irq(). It also requires the 56-bit
DMA mask and enables NETIF_F_SG plus NETIF_F_HIGHDMA.
A few questions about the TX stop/wake logic and the TX DMA quiesce
handling below.
> diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h
> index 0dce78e4a91ba..6dc29ebe6fa70 100644
> --- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h
> +++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h
> @@ -7,13 +7,20 @@
> +#define RNPGBE_DMA_STATUS 0x0008
> +#define TX_AXI_RW_EN 0xc
> +/* DMA_STATUS_REG[23:20]: tx_wr, tx_rd, rx_wr, rx_rd done status. */
> +#define RNPGBE_DMA_TX_STATUS GENMASK_U32(23, 22)
> #define RNPGBE_DMA_AXI_EN 0x0010
> +#define RNPGBE_TX_MIN_PKT_LEN 33
[ ... ]
> diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_lib.c b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_lib.c
> index c661290d561bd..8700c48669ecb 100644
> --- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_lib.c
> +++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_lib.c
> @@ -89,6 +91,114 @@ static void rnpgbe_irq_enable_queues(struct mucse_q_vector *q_vector)
> +static bool rnpgbe_clean_tx_irq(struct mucse_q_vector *q_vector,
> + struct mucse_ring *tx_ring,
> + int napi_budget)
> +{
[ ... ]
> + i += tx_ring->count;
> + tx_ring->next_to_clean = i;
> + u64_stats_update_begin(&tx_ring->syncp);
> + tx_ring->stats.bytes += total_bytes;
> + tx_ring->stats.packets += total_packets;
> + u64_stats_update_end(&tx_ring->syncp);
> +
> +#define TX_WAKE_THRESHOLD (DESC_NEEDED * 2)
> + __netif_txq_completed_wake(txring_txq(tx_ring),
> + total_packets, total_bytes,
> + mucse_desc_unused(tx_ring),
> + TX_WAKE_THRESHOLD,
> + !netif_carrier_ok(tx_ring->netdev) ||
> + test_bit(__MUCSE_DOWN, &mucse->state));
[Severity: High]
[Finding: main-1-1]
[Sources: opus-5]
Can the TX queue end up stopped forever here?
rnpgbe_clean_tx_irq() is the only place in the driver that can clear
__QUEUE_STATE_DRV_XOFF after rnpgbe_xmit_frame_ring() stops the queue,
and the down_cond argument suppresses the wake whenever the carrier is
reported down:
!netif_carrier_ok(tx_ring->netdev) ||
test_bit(__MUCSE_DOWN, &mucse->state)
Since link state is polled by the service task, netif_carrier_off() can
lag the real link event. If the hardware finishes the queued descriptors
during that window, this call drains the ring but returns without waking,
and with an empty ring there is no further TX interrupt to re-evaluate
the condition.
What re-enables the queue after that? netif_tx_start_all_queues() is
called only from rnpgbe_up_complete() on ifup, and the runtime carrier-on
handler added later in the series only calls netif_carrier_on() without
touching the queues. dev_activate() does not clear DRV_XOFF either.
rnpgbe_netdev_ops also registers no .ndo_tx_timeout, and
netdev_watchdog_up() bails out early in that case:
net/sched/sch_generic.c:netdev_watchdog_up() {
if (!dev->netdev_ops->ndo_tx_timeout)
return;
}
so dev_watchdog() never fires and a genuine hardware TX stall (DD never
set, no TX interrupt) is equally invisible. Would it make sense to add
.ndo_tx_timeout, and to wake the queues from the carrier-on path?
> +
> + return !!budget;
> +}
[ ... ]
> @@ -633,13 +758,144 @@ static void rnpgbe_napi_disable_all(struct mucse *mucse)
> +static void rnpgbe_stop_tx_ring(struct mucse_ring *tx_ring)
> +{
> + if (!tx_ring->tx_buffer_info)
> + return;
> +
> + /* Stop hw. No new descriptors are fetched after TX_START=0.
> + * DMA for descriptors fetched before the stop may still be in flight.
> + */
> + mucse_ring_wr32(tx_ring, RNPGBE_TX_START, 0);
> + /* Flush posted write to ensure hardware sees TX_START=0 */
> + (void)mucse_ring_rd32(tx_ring, RNPGBE_TX_START);
> +}
> +
> +static void rnpgbe_wait_tx_dma_idle(struct mucse *mucse)
> +{
> + struct mucse_hw *hw = &mucse->hw;
> + u32 dma_status;
> + int err;
> +
> + err = readl_poll_timeout(hw->hw_addr + RNPGBE_DMA_STATUS,
> + dma_status,
> + (dma_status & RNPGBE_DMA_TX_STATUS) ==
> + RNPGBE_DMA_TX_STATUS,
> + 10, 100000);
> + if (err)
> + dev_warn(&mucse->pdev->dev,
> + "Timed out waiting for TX DMA to quiesce, status %#x\n",
> + dma_status);
> +}
[Severity: Medium]
[Finding: gpt-5-6-sol-4-6]
[Sources: gpt-5-6-sol, opus-5]
Should the timeout here be propagated to the callers?
rnpgbe_wait_tx_dma_idle() returns void, so a timeout is only logged and
both callers continue as if the engine had confirmed idle. On the ifdown
side that means rnpgbe_clean_all_tx_rings() clears TX_AXI_RW_EN and then
rnpgbe_clean_tx_ring() calls dma_unmap_single()/dma_unmap_page() on every
in-flight TX buffer, and rnpgbe_close() -> rnpgbe_free_all_tx_resources()
-> rnpgbe_free_tx_resources() then dma_free_coherent()s the descriptor
ring.
The comment in rnpgbe_stop_tx_ring() states that DMA for descriptors
fetched before the stop may still be in flight, which is what this poll
is guarding. If the poll times out, can the device still be reading the
mapped buffers or writing back into the descriptor ring after they are
released?
The same unchecked wait sits at the head of rnpgbe_configure_tx(), where
RNPGBE_TX_BASE_ADDR_LO/HI, RNPGBE_TX_LEN and TAIL are reprogrammed and
TX_START is set to 1 again. Is a device reset or an error return needed
when the engine never reports idle?
> +
> +/**
> + * rnpgbe_clean_tx_ring - Free Tx Buffers
> + * @tx_ring: ring to be cleaned
> + **/
> +static void rnpgbe_clean_tx_ring(struct mucse_ring *tx_ring)
> +{
[ ... ]
> +static void rnpgbe_clean_all_tx_rings(struct mucse *mucse)
> +{
> + struct mucse_hw *hw = &mucse->hw;
> + u32 dma_axi_ctl;
> +
> + for (int i = 0; i < mucse->num_tx_queues; i++)
> + rnpgbe_stop_tx_ring(mucse->tx_ring[i]);
> +
> + if (mucse->num_tx_queues)
> + rnpgbe_wait_tx_dma_idle(mucse);
> +
> + dma_axi_ctl = mucse_hw_rd32(hw, RNPGBE_DMA_AXI_EN);
> + dma_axi_ctl &= ~TX_AXI_RW_EN;
> + mucse_hw_wr32(hw, RNPGBE_DMA_AXI_EN, dma_axi_ctl);
> + /* Flush the posted write before releasing the DMA mappings. */
> + (void)mucse_hw_rd32(hw, RNPGBE_DMA_AXI_EN);
> +
> + for (int i = 0; i < mucse->num_tx_queues; i++)
> + rnpgbe_clean_tx_ring(mucse->tx_ring[i]);
> +}
[ ... ]
> @@ -650,11 +906,420 @@ bool rnpgbe_down(struct mucse *mucse)
> +static void rnpgbe_configure_tx_ring(struct mucse *mucse,
> + struct mucse_ring *ring)
> +{
> + struct mucse_hw *hw = &mucse->hw;
> +
> + mucse_ring_wr32(ring, RNPGBE_TX_BASE_ADDR_LO, (u32)ring->dma);
> + mucse_ring_wr32(ring, RNPGBE_TX_BASE_ADDR_HI,
> + (u32)(((u64)ring->dma) >> 32) | (hw->pfvfnum << 24));
Simon says: I think we can skip the issue below.
[Severity: Low]
[Finding: main-2-3]
[Sources: opus-5]
This isn't a bug, but hw->pfvfnum is a u8 here and the shift is done on
the int-promoted value, while rnpgbe_tx_map() casts first for the same
field:
fun_id = (u64)tx_ring->pfvfnum << 56;
With the kernel's -fno-strict-overflow the produced bit pattern is the
one the register wants in either form, so this is only a consistency
point; an explicit (u32) cast would match the other encoding site.
[ ... ]
> +netdev_tx_t rnpgbe_xmit_frame_ring(struct sk_buff *skb,
> + struct mucse_ring *tx_ring)
> +{
[ ... ]
> + if (!netif_txq_maybe_stop(txring_txq(tx_ring),
> + mucse_desc_unused(tx_ring),
> + count + RESV_DESC_NEEDED,
> + count + RESV_DESC_NEEDED))
> + return NETDEV_TX_BUSY;
[ ... ]
> diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c
> index c3296a3dde0bb..24f4cf27a307b 100644
> --- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c
> +++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c
> @@ -83,25 +101,37 @@ static int rnpgbe_close(struct net_device *netdev)
> static const struct net_device_ops rnpgbe_netdev_ops = {
> .ndo_open = rnpgbe_open,
> .ndo_stop = rnpgbe_close,
> .ndo_start_xmit = rnpgbe_xmit_frame,
> + .ndo_get_stats64 = rnpgbe_get_stats64,
> };
[ ... ]
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH net-next v9 3/4] net: rnpgbe: Add RX packet reception support
2026-08-14 11:13 [PATCH net-next v9 0/4] net: rnpgbe: Add TX/RX and link status support Dong Yibo
2026-08-14 11:13 ` [PATCH net-next v9 1/4] net: rnpgbe: Add interrupt handling Dong Yibo
2026-08-14 11:13 ` [PATCH net-next v9 2/4] net: rnpgbe: Add basic TX packet transmission support Dong Yibo
@ 2026-08-14 11:13 ` Dong Yibo
2026-08-18 11:15 ` Simon Horman
2026-08-14 11:13 ` [PATCH net-next v9 4/4] net: rnpgbe: Add link status handling support Dong Yibo
3 siblings, 1 reply; 9+ messages in thread
From: Dong Yibo @ 2026-08-14 11:13 UTC (permalink / raw)
To: andrew+netdev, davem, edumazet, kuba, pabeni, vadim.fedorenko,
u.kleine-koenig
Cc: netdev, linux-kernel, dong100, yaojun
Add basic RX packet reception for the RNPGBE driver.
Introduce the RX descriptor structure (union rnpgbe_rx_desc) with the
hardware write-back format, and manage RX buffers with page_pool using a
single page per descriptor without page splitting. Implement the NAPI
poll callback, RX ring setup and teardown, and packet assembly from page
buffers, along with RX statistics.
Keep a 16-descriptor gap in the fixed 512-entry RX ring so at most 496
descriptors are posted to hardware. Retry failed page allocations with
one timer per q_vector that schedules NAPI and is cancelled before RX
cleanup and q_vector teardown. Publish the RX tail after every refill
attempt, even when the first allocation fails, and count dropped
complete packets against the NAPI budget rather than the RX success
statistics. Quiesce RX DMA and cancel the refill timers before disabling
NAPI, and do not re-enable queue interrupts while the device is down.
Signed-off-by: Dong Yibo <dong100@mucse.com>
---
drivers/net/ethernet/mucse/Kconfig | 1 +
drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h | 55 +-
drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h | 2 +
.../net/ethernet/mucse/rnpgbe/rnpgbe_lib.c | 758 +++++++++++++++++-
.../net/ethernet/mucse/rnpgbe/rnpgbe_lib.h | 25 +-
.../net/ethernet/mucse/rnpgbe/rnpgbe_main.c | 9 +
6 files changed, 843 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/mucse/Kconfig b/drivers/net/ethernet/mucse/Kconfig
index 0b3e853d625f..be0fdf268484 100644
--- a/drivers/net/ethernet/mucse/Kconfig
+++ b/drivers/net/ethernet/mucse/Kconfig
@@ -19,6 +19,7 @@ if NET_VENDOR_MUCSE
config MGBE
tristate "Mucse(R) 1GbE PCI Express adapters support"
depends on PCI
+ select PAGE_POOL
help
This driver supports Mucse(R) 1GbE PCI Express family of
adapters.
diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h
index 49fcbd2e3740..1d3e4078f16b 100644
--- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h
+++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h
@@ -7,6 +7,7 @@
#include <linux/types.h>
#include <linux/mutex.h>
#include <linux/netdevice.h>
+#include <linux/timer.h>
#include <linux/if.h>
#include <linux/workqueue.h>
@@ -62,7 +63,32 @@ struct rnpgbe_tx_desc {
#define M_TXD_CMD_EOP 0x010000 /* End of Packet */
};
+union rnpgbe_rx_desc {
+ struct {
+ __le64 pkt_addr; /* Packet buffer address */
+ __le64 resv_cmd; /* cmd status */
+ };
+ struct {
+ __le32 rss_hash; /* RSS HASH */
+ __le16 mark; /* mark info */
+ __le16 rev1;
+ __le16 len; /* Packet length */
+ __le16 padding_len;
+ __le16 vlan; /* VLAN tag */
+ __le16 cmd; /* cmd status */
+#define M_RXD_STAT_DD BIT(1) /* Descriptor Done */
+#define M_RXD_STAT_EOP BIT(0) /* End of Packet */
+ } wb;
+};
+
#define M_TX_DESC(R, i) (&(((struct rnpgbe_tx_desc *)((R)->desc))[i]))
+#define M_RX_DESC(R, i) (&(((union rnpgbe_rx_desc *)((R)->desc))[i]))
+
+static inline __le16 rnpgbe_test_staterr(union rnpgbe_rx_desc *rx_desc,
+ const u16 stat_err_bits)
+{
+ return rx_desc->wb.cmd & cpu_to_le16(stat_err_bits);
+}
struct mucse_tx_buffer {
struct rnpgbe_tx_desc *next_to_watch;
@@ -80,13 +106,24 @@ struct mucse_queue_stats {
atomic64_t dropped;
};
+struct mucse_rx_buffer {
+ struct sk_buff *skb;
+ dma_addr_t dma;
+ struct page *page;
+ u32 page_offset;
+};
+
struct mucse_ring {
struct mucse_ring *next;
struct mucse_q_vector *q_vector;
struct net_device *netdev;
struct device *dev;
+ struct page_pool *page_pool;
void *desc;
- struct mucse_tx_buffer *tx_buffer_info;
+ union {
+ struct mucse_tx_buffer *tx_buffer_info;
+ struct mucse_rx_buffer *rx_buffer_info;
+ };
void __iomem *ring_addr;
void __iomem *tail;
void __iomem *irq_mask;
@@ -102,6 +139,7 @@ struct mucse_ring {
unsigned int size;
struct mucse_queue_stats stats;
struct u64_stats_sync syncp;
+ bool drop_status;
} ____cacheline_internodealigned_in_smp;
static inline u16 mucse_desc_unused(struct mucse_ring *ring)
@@ -112,6 +150,18 @@ static inline u16 mucse_desc_unused(struct mucse_ring *ring)
return ((ntc > ntu) ? 0 : ring->count) + ntc - ntu - 1;
}
+static inline u16 mucse_desc_unused_rx(struct mucse_ring *ring)
+{
+ u16 ntc = ring->next_to_clean;
+ u16 ntu = ring->next_to_use;
+
+ /* Keep M_RX_BUFFER_WRITE descriptors unused so the ring is not filled
+ * completely. Refill is attempted once at least this many descriptors
+ * are available.
+ */
+ return ((ntc > ntu) ? 0 : ring->count) + ntc - ntu - 16;
+}
+
static inline __le64 build_ctob(u32 vlan_cmd, u32 mac_ip_len, u32 size)
{
return cpu_to_le64(((u64)vlan_cmd << 32) | ((u64)mac_ip_len << 16) |
@@ -134,6 +184,7 @@ struct mucse_q_vector {
int hw_vector;
struct mucse_ring_container rx, tx;
struct napi_struct napi;
+ struct timer_list rx_alloc_timer;
char name[IFNAMSIZ + 18];
/* for dynamic allocation of rings associated with this q_vector */
struct mucse_ring ring[] ____cacheline_internodealigned_in_smp;
@@ -142,6 +193,7 @@ struct mucse_q_vector {
#define MAX_Q_VECTORS 8
#define M_DEFAULT_TXD 512
+#define M_DEFAULT_RXD 512
#define M_DEFAULT_TX_WORK 256
enum mucse_state_t {
@@ -164,6 +216,7 @@ struct mucse {
int tx_work_limit;
int num_tx_queues;
int num_q_vectors;
+ int rx_ring_item_count;
int num_rx_queues;
char mbx_name[32];
unsigned long state;
diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h
index 6dc29ebe6fa7..1d87edfba3d7 100644
--- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h
+++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h
@@ -19,6 +19,8 @@
#define TX_AXI_RW_EN 0xc
/* DMA_STATUS_REG[23:20]: tx_wr, tx_rd, rx_wr, rx_rd done status. */
#define RNPGBE_DMA_TX_STATUS GENMASK_U32(23, 22)
+#define RNPGBE_DMA_RX_STATUS GENMASK_U32(21, 20)
+#define RX_AXI_RW_EN 0x03
#define RNPGBE_DMA_AXI_EN 0x0010
#define RNPGBE_TX_MIN_PKT_LEN 33
diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_lib.c b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_lib.c
index 8700c48669ec..9199a48e75d4 100644
--- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_lib.c
+++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_lib.c
@@ -4,8 +4,10 @@
#include <linux/pci.h>
#include <linux/netdevice.h>
#include <linux/iopoll.h>
+#include <linux/etherdevice.h>
#include <linux/vmalloc.h>
#include <net/netdev_queues.h>
+#include <net/page_pool/helpers.h>
#include "rnpgbe_lib.h"
#include "rnpgbe.h"
@@ -199,8 +201,412 @@ static bool rnpgbe_clean_tx_irq(struct mucse_q_vector *q_vector,
return !!budget;
}
+static bool mucse_alloc_mapped_page(struct mucse_ring *rx_ring,
+ struct mucse_rx_buffer *bi)
+{
+ struct page *page = bi->page;
+ dma_addr_t dma;
+
+ if (page) {
+ /* Buffer is being reused without going back through the
+ * page_pool. Do dma_sync for hw use.
+ */
+ dma_sync_single_range_for_device(rx_ring->dev, bi->dma,
+ bi->page_offset,
+ PAGE_SIZE - bi->page_offset,
+ DMA_FROM_DEVICE);
+ return true;
+ }
+
+ page = page_pool_dev_alloc_pages(rx_ring->page_pool);
+ if (unlikely(!page))
+ return false;
+ dma = page_pool_get_dma_addr(page);
+
+ bi->dma = dma;
+ bi->page = page;
+ bi->page_offset = RNPGBE_SKB_PAD;
+
+ return true;
+}
+
+static void mucse_update_rx_tail(struct mucse_ring *rx_ring,
+ u32 val)
+{
+ rx_ring->next_to_use = val;
+ writel(val, rx_ring->tail);
+}
+
+/**
+ * rnpgbe_alloc_rx_buffers - Replace used receive buffers
+ * @rx_ring: ring to place buffers on
+ * @cleaned_count: number of buffers to replace
+ *
+ * Return: true if alloc failed
+ **/
+static bool rnpgbe_alloc_rx_buffers(struct mucse_ring *rx_ring,
+ u16 cleaned_count)
+{
+ u64 fun_id = ((u64)(rx_ring->pfvfnum) << 56);
+ union rnpgbe_rx_desc *rx_desc;
+ u16 i = rx_ring->next_to_use;
+ struct mucse_rx_buffer *bi;
+ bool err = false;
+ u64 addr;
+ /* nothing to do */
+ if (!cleaned_count)
+ return err;
+
+ rx_desc = M_RX_DESC(rx_ring, i);
+ bi = &rx_ring->rx_buffer_info[i];
+ i -= rx_ring->count;
+
+ do {
+ if (!mucse_alloc_mapped_page(rx_ring, bi)) {
+ err = true;
+ break;
+ }
+
+ addr = (u64)(bi->dma + bi->page_offset);
+ rx_desc->pkt_addr = cpu_to_le64(addr | fun_id);
+ /* clean dd */
+ rx_desc->resv_cmd = 0;
+ rx_desc++;
+ bi++;
+ i++;
+ if (unlikely(!i)) {
+ rx_desc = M_RX_DESC(rx_ring, 0);
+ bi = rx_ring->rx_buffer_info;
+ i -= rx_ring->count;
+ }
+ cleaned_count--;
+ } while (cleaned_count);
+
+ i += rx_ring->count;
+
+ /* Publish the tail even when allocation stopped early, so a stale
+ * tail from a previous session cannot point hardware at a
+ * reallocated ring.
+ */
+ dma_wmb();
+ mucse_update_rx_tail(rx_ring, i);
+
+ return err;
+}
+
+/**
+ * rnpgbe_get_buffer - Get the rx_buffer to be used
+ * @rx_ring: pointer to rx ring
+ * @skb: pointer skb for this packet
+ * @size: data size in this desc
+ *
+ * Return: rx_buffer.
+ **/
+static struct mucse_rx_buffer *rnpgbe_get_buffer(struct mucse_ring *rx_ring,
+ struct sk_buff **skb,
+ const unsigned int size)
+{
+ struct mucse_rx_buffer *rx_buffer;
+
+ rx_buffer = &rx_ring->rx_buffer_info[rx_ring->next_to_clean];
+ *skb = rx_buffer->skb;
+ prefetchw(page_address(rx_buffer->page) + rx_buffer->page_offset);
+ /* we are reusing so sync this buffer for CPU use */
+ dma_sync_single_range_for_cpu(rx_ring->dev, rx_buffer->dma,
+ rx_buffer->page_offset, size,
+ DMA_FROM_DEVICE);
+
+ return rx_buffer;
+}
+
+/**
+ * rnpgbe_add_rx_frag - Add non-linear data to the skb
+ * @rx_buffer: pointer to rx_buffer
+ * @skb: pointer skb for this packet
+ * @size: data size in this desc
+ **/
+static void rnpgbe_add_rx_frag(struct mucse_rx_buffer *rx_buffer,
+ struct sk_buff *skb,
+ unsigned int size)
+{
+ unsigned int truesize = PAGE_SIZE;
+
+ skb_add_rx_frag(skb, skb_shinfo(skb)->nr_frags, rx_buffer->page,
+ rx_buffer->page_offset, size, truesize);
+}
+
+/**
+ * rnpgbe_build_skb - Try to build a skb based on rx_buffer
+ * @rx_buffer: pointer to rx_buffer
+ * @size: data size in this desc
+ *
+ * Return: skb for this rx_buffer
+ **/
+static struct sk_buff *rnpgbe_build_skb(struct mucse_rx_buffer *rx_buffer,
+ unsigned int size)
+{
+ void *va = page_address(rx_buffer->page) + rx_buffer->page_offset;
+ unsigned int truesize = PAGE_SIZE;
+ struct sk_buff *skb;
+
+ net_prefetch(va);
+ /* build an skb around the page buffer */
+ skb = build_skb(va - RNPGBE_SKB_PAD, truesize);
+ if (unlikely(!skb))
+ return NULL;
+ /* update pointers within the skb to store the data */
+ skb_reserve(skb, RNPGBE_SKB_PAD);
+ __skb_put(skb, size);
+ skb_mark_for_recycle(skb);
+
+ return skb;
+}
+
+/**
+ * rnpgbe_is_non_eop - Process handling of non-EOP buffers
+ * @rx_ring: rx ring being processed
+ * @rx_desc: rx descriptor for current buffer
+ * @skb: current socket buffer containing buffer in progress
+ *
+ * This function updates next to clean. If the buffer is an EOP buffer
+ * this function exits returning false, otherwise it will place the
+ * sk_buff in the next buffer to be chained and return true indicating
+ * that this is in fact a non-EOP buffer.
+ *
+ * Return: true for not end of packet
+ **/
+static bool rnpgbe_is_non_eop(struct mucse_ring *rx_ring,
+ union rnpgbe_rx_desc *rx_desc,
+ struct sk_buff *skb)
+{
+ u32 ntc = rx_ring->next_to_clean + 1;
+
+ /* fetch, update, and store next to clean */
+ ntc = (ntc < rx_ring->count) ? ntc : 0;
+ rx_ring->next_to_clean = ntc;
+ prefetch(M_RX_DESC(rx_ring, ntc));
+ /* if we are the last buffer then there is nothing else to do */
+ if (likely(rnpgbe_test_staterr(rx_desc, M_RXD_STAT_EOP)))
+ return false;
+ if (skb_shinfo(skb)->nr_frags < MAX_SKB_FRAGS) {
+ /* place skb in next buffer to be received */
+ rx_ring->rx_buffer_info[ntc].skb = skb;
+ } else {
+ atomic64_inc(&rx_ring->stats.dropped);
+ /* too much frags, force free */
+ dev_kfree_skb_any(skb);
+ rx_ring->drop_status = true;
+ }
+ /* we should clean it since we used all info in it */
+ rx_desc->wb.cmd = 0;
+
+ return true;
+}
+
/**
- * rnpgbe_poll - NAPI Rx polling callback
+ * rnpgbe_cleanup_headers - Correct corrupted or empty headers
+ * @skb: current socket buffer containing buffer in progress
+ *
+ * Return: true if an error was encountered and skb was freed.
+ **/
+static bool rnpgbe_cleanup_headers(struct sk_buff *skb)
+{
+ /* if eth_skb_pad returns an error the skb was freed */
+ if (eth_skb_pad(skb))
+ return true;
+
+ return false;
+}
+
+/**
+ * rnpgbe_process_skb_fields - Set the RX queue and protocol fields
+ * @rx_ring: RX descriptor ring containing the queue information
+ * @skb: skb currently being received
+ *
+ * Records the RX queue that received the skb and sets its protocol from
+ * the Ethernet header.
+ **/
+static void rnpgbe_process_skb_fields(struct mucse_ring *rx_ring,
+ struct sk_buff *skb)
+{
+ struct net_device *dev = rx_ring->netdev;
+
+ skb_record_rx_queue(skb, rx_ring->queue_index);
+ skb->protocol = eth_type_trans(skb, dev);
+}
+
+/**
+ * rnpgbe_rx_alloc_retry - Retry RX buffer allocation
+ * @timer: RX allocation retry timer
+ *
+ * Schedules NAPI after RX buffer allocation fails during polling or
+ * initial ring configuration.
+ **/
+static void rnpgbe_rx_alloc_retry(struct timer_list *timer)
+{
+ struct mucse_q_vector *q_vector =
+ timer_container_of(q_vector, timer, rx_alloc_timer);
+
+ napi_schedule(&q_vector->napi);
+}
+
+/**
+ * rnpgbe_clean_rx_irq - Clean completed descriptors from Rx ring
+ * @q_vector: structure containing interrupt and ring information
+ * @rx_ring: rx descriptor ring to transact packets on
+ * @budget: total limit on number of packets to process
+ *
+ * rnpgbe_clean_rx_irq tries to check dd in desc, handle this desc
+ * if dd is set which means data is write-back by hw
+ *
+ * Return: amount of work completed.
+ **/
+static int rnpgbe_clean_rx_irq(struct mucse_q_vector *q_vector,
+ struct mucse_ring *rx_ring,
+ int budget)
+{
+ unsigned int max_size = SKB_WITH_OVERHEAD(PAGE_SIZE) - RNPGBE_SKB_PAD;
+ unsigned int total_rx_bytes = 0, total_rx_packets = 0;
+ u16 cleaned_count = mucse_desc_unused_rx(rx_ring);
+ unsigned int work_done = 0;
+
+ while (likely(work_done < budget)) {
+ struct mucse_rx_buffer *rx_buffer;
+ union rnpgbe_rx_desc *rx_desc;
+ struct sk_buff *skb;
+ unsigned int size;
+
+ if (cleaned_count >= M_RX_BUFFER_WRITE) {
+ if (rnpgbe_alloc_rx_buffers(rx_ring, cleaned_count)) {
+ mod_timer(&q_vector->rx_alloc_timer,
+ jiffies + msecs_to_jiffies(500));
+ cleaned_count = mucse_desc_unused_rx(rx_ring);
+ } else {
+ cleaned_count = 0;
+ }
+ }
+ rx_desc = M_RX_DESC(rx_ring, rx_ring->next_to_clean);
+
+ if (!rnpgbe_test_staterr(rx_desc, M_RXD_STAT_DD))
+ break;
+
+ /* This memory barrier is needed to keep us from reading
+ * any other fields out of the rx_desc until we know the
+ * descriptor has been written back
+ */
+ dma_rmb();
+ /* Hardware enforces: minimum 33-bytes descriptor(no 1-13 byte
+ * size), multi-descriptors only for jumbo frames > 1536 bytes
+ * (controlled by M_DEFAULT_SG=96, each descriptor no more than
+ * 1536 bytes). Small packets use single descriptor.
+ */
+ size = le16_to_cpu(rx_desc->wb.len);
+
+ if (unlikely(rx_ring->drop_status)) {
+ cleaned_count++;
+ /* drop data until eop */
+ if (rnpgbe_test_staterr(rx_desc, M_RXD_STAT_EOP)) {
+ rx_ring->drop_status = false;
+ work_done++;
+ }
+
+ rx_desc->wb.cmd = 0;
+ rx_ring->next_to_clean++;
+ if (rx_ring->next_to_clean >= rx_ring->count)
+ rx_ring->next_to_clean = 0;
+ continue;
+ }
+
+ if (unlikely(!size || size > max_size)) {
+ struct mucse_rx_buffer *err_rx_buffer;
+ u16 idx = rx_ring->next_to_clean;
+
+ cleaned_count++;
+ atomic64_inc(&rx_ring->stats.dropped);
+
+ /* Free the partial skb from a previous non-EOP
+ * descriptor before advancing next_to_clean.
+ */
+ err_rx_buffer = &rx_ring->rx_buffer_info[idx];
+ if (unlikely(err_rx_buffer->skb)) {
+ dev_kfree_skb_any(err_rx_buffer->skb);
+ err_rx_buffer->skb = NULL;
+ }
+
+ /* drop data until eop */
+ if (rnpgbe_test_staterr(rx_desc, M_RXD_STAT_EOP))
+ work_done++;
+ else
+ rx_ring->drop_status = true;
+
+ rx_desc->wb.cmd = 0;
+ rx_ring->next_to_clean++;
+ if (rx_ring->next_to_clean >= rx_ring->count)
+ rx_ring->next_to_clean = 0;
+ continue;
+ }
+
+ rx_buffer = rnpgbe_get_buffer(rx_ring, &skb, size);
+
+ if (skb)
+ rnpgbe_add_rx_frag(rx_buffer, skb, size);
+ else
+ skb = rnpgbe_build_skb(rx_buffer, size);
+
+ if (!skb) {
+ cleaned_count++;
+
+ /* drop until eop if multiple descriptors */
+ if (rnpgbe_test_staterr(rx_desc, M_RXD_STAT_EOP))
+ work_done++;
+ else
+ rx_ring->drop_status = true;
+
+ rx_desc->wb.cmd = 0;
+ rx_ring->next_to_clean++;
+ atomic64_inc(&rx_ring->stats.dropped);
+ if (rx_ring->next_to_clean >= rx_ring->count)
+ rx_ring->next_to_clean = 0;
+
+ continue;
+ }
+
+ rx_buffer->page = NULL;
+ rx_buffer->skb = NULL;
+ cleaned_count++;
+
+ if (rnpgbe_is_non_eop(rx_ring, rx_desc, skb))
+ continue;
+
+ /* verify the packet layout is correct */
+ if (rnpgbe_cleanup_headers(skb)) {
+ /* we should clean it since we used all info in it */
+ atomic64_inc(&rx_ring->stats.dropped);
+ rx_desc->wb.cmd = 0;
+ work_done++;
+ continue;
+ }
+
+ /* probably a little skewed due to removing CRC */
+ total_rx_bytes += skb->len;
+ rnpgbe_process_skb_fields(rx_ring, skb);
+ rx_desc->wb.cmd = 0;
+ napi_gro_receive(&q_vector->napi, skb);
+ /* Update packet statistics and NAPI work accounting. */
+ total_rx_packets++;
+ work_done++;
+ }
+
+ u64_stats_update_begin(&rx_ring->syncp);
+ rx_ring->stats.packets += total_rx_packets;
+ rx_ring->stats.bytes += total_rx_bytes;
+ u64_stats_update_end(&rx_ring->syncp);
+
+ return work_done;
+}
+
+/**
+ * rnpgbe_poll - NAPI polling callback
* @napi: structure for representing this polling device
* @budget: how many packets driver is allowed to clean
*
@@ -214,6 +620,7 @@ static int rnpgbe_poll(struct napi_struct *napi, int budget)
container_of(napi, struct mucse_q_vector, napi);
bool clean_complete = true;
struct mucse_ring *ring;
+ int per_ring_budget;
int work_done = 0;
mucse_for_each_ring(ring, q_vector->tx) {
@@ -225,11 +632,27 @@ static int rnpgbe_poll(struct napi_struct *napi, int budget)
if (unlikely(!budget))
return 0;
+ if (q_vector->rx.count > 1)
+ per_ring_budget = max(budget / q_vector->rx.count, 1);
+ else
+ per_ring_budget = budget;
+
+ mucse_for_each_ring(ring, q_vector->rx) {
+ int cleaned = 0;
+
+ cleaned = rnpgbe_clean_rx_irq(q_vector, ring, per_ring_budget);
+ work_done += cleaned;
+ if (cleaned >= per_ring_budget)
+ clean_complete = false;
+ }
+
if (!clean_complete)
return budget;
- if (likely(napi_complete_done(napi, work_done)))
- rnpgbe_irq_enable_queues(q_vector);
+ if (likely(napi_complete_done(napi, work_done))) {
+ if (!test_bit(__MUCSE_DOWN, &q_vector->mucse->state))
+ rnpgbe_irq_enable_queues(q_vector);
+ }
return work_done;
}
@@ -388,6 +811,7 @@ static int rnpgbe_alloc_q_vector(struct mucse *mucse,
/* tie q_vector and mucse together */
mucse->q_vector[vector_idx] = q_vector;
q_vector->mucse = mucse;
+ timer_setup(&q_vector->rx_alloc_timer, rnpgbe_rx_alloc_retry, 0);
q_vector->hw_vector = vector_idx;
/* if mbx use separate irq, we should add 1 */
if (mucse->flags & M_FLAG_MSIX_EN)
@@ -414,13 +838,18 @@ static int rnpgbe_alloc_q_vector(struct mucse *mucse,
}
for (idx = 0; idx < rxr_count; idx++) {
+ ring->dev = &mucse->pdev->dev;
mucse_add_ring(ring, &q_vector->rx);
+ ring->count = mucse->rx_ring_item_count;
+ ring->netdev = mucse->netdev;
ring->queue_index = eth_queue_idx + idx;
ring->rnpgbe_queue_idx = rxr_idx;
ring->ring_addr = hw->hw_addr + RING_OFFSET(rxr_idx);
ring->irq_mask = ring->ring_addr + RNPGBE_DMA_INT_MASK;
ring->trig = ring->ring_addr + RNPGBE_DMA_INT_TRIG;
ring->q_vector = q_vector;
+ ring->pfvfnum = hw->pfvfnum;
+ u64_stats_init(&ring->syncp);
mucse->rx_ring[ring->queue_index] = ring;
rxr_idx += step;
ring++;
@@ -443,6 +872,8 @@ static void rnpgbe_free_q_vector(struct mucse *mucse, int vector_idx)
struct mucse_q_vector *q_vector = mucse->q_vector[vector_idx];
struct mucse_ring *ring;
+ timer_delete_sync(&q_vector->rx_alloc_timer);
+
mucse_for_each_ring(ring, q_vector->tx)
mucse->tx_ring[ring->queue_index] = NULL;
mucse_for_each_ring(ring, q_vector->rx)
@@ -788,6 +1219,23 @@ static void rnpgbe_wait_tx_dma_idle(struct mucse *mucse)
dma_status);
}
+static void rnpgbe_wait_rx_dma_idle(struct mucse *mucse)
+{
+ struct mucse_hw *hw = &mucse->hw;
+ u32 dma_status;
+ int err;
+
+ err = readl_poll_timeout(hw->hw_addr + RNPGBE_DMA_STATUS,
+ dma_status,
+ (dma_status & RNPGBE_DMA_RX_STATUS) ==
+ RNPGBE_DMA_RX_STATUS,
+ 10, 100000);
+ if (err)
+ dev_warn(&mucse->pdev->dev,
+ "Timed out waiting for RX DMA to quiesce, status %#x\n",
+ dma_status);
+}
+
/**
* rnpgbe_clean_tx_ring - Free Tx Buffers
* @tx_ring: ring to be cleaned
@@ -864,7 +1312,7 @@ static void rnpgbe_clean_tx_ring(struct mucse_ring *tx_ring)
* rnpgbe_clean_all_tx_rings - Stop TX DMA and free Tx buffers for all queues
* @mucse: board private structure
**/
-static void rnpgbe_clean_all_tx_rings(struct mucse *mucse)
+void rnpgbe_clean_all_tx_rings(struct mucse *mucse)
{
struct mucse_hw *hw = &mucse->hw;
u32 dma_axi_ctl;
@@ -885,6 +1333,55 @@ static void rnpgbe_clean_all_tx_rings(struct mucse *mucse)
rnpgbe_clean_tx_ring(mucse->tx_ring[i]);
}
+static void rnpgbe_stop_rx_ring(struct mucse_ring *rx_ring)
+{
+ /* Stop hw. hardware design guarantees:
+ * - No new descriptors will be fetched after RX_START=0
+ * - No DMA will be initiated for already-fetched descriptors
+ */
+ mucse_ring_wr32(rx_ring, RNPGBE_RX_START, 0);
+ /* Flush posted write to ensure hardware sees RX_START=0 */
+ (void)mucse_ring_rd32(rx_ring, RNPGBE_RX_START);
+}
+
+/**
+ * rnpgbe_stop_all_rx_rings - Stop RX DMA for all queues
+ * @mucse: board private structure
+ **/
+static void rnpgbe_stop_all_rx_rings(struct mucse *mucse)
+{
+ struct mucse_hw *hw = &mucse->hw;
+ u32 dma_axi_ctl;
+
+ for (int i = 0; i < mucse->num_rx_queues; i++)
+ rnpgbe_stop_rx_ring(mucse->rx_ring[i]);
+
+ if (mucse->num_rx_queues)
+ rnpgbe_wait_rx_dma_idle(mucse);
+
+ dma_axi_ctl = mucse_hw_rd32(hw, RNPGBE_DMA_AXI_EN);
+ dma_axi_ctl &= ~RX_AXI_RW_EN;
+ mucse_hw_wr32(hw, RNPGBE_DMA_AXI_EN, dma_axi_ctl);
+ /* Flush the posted write before releasing the DMA mappings. */
+ (void)mucse_hw_rd32(hw, RNPGBE_DMA_AXI_EN);
+}
+
+/**
+ * rnpgbe_clean_all_rx_rings - Free Rx buffers for all queues
+ * @mucse: board private structure
+ **/
+static void rnpgbe_clean_all_rx_rings(struct mucse *mucse)
+{
+ for (int i = 0; i < mucse->num_rx_queues; i++)
+ rnpgbe_clean_rx_ring(mucse->rx_ring[i]);
+}
+
+static void rnpgbe_cancel_rx_retry_timers(struct mucse *mucse)
+{
+ for (int i = 0; i < mucse->num_q_vectors; i++)
+ timer_delete_sync(&mucse->q_vector[i]->rx_alloc_timer);
+}
+
bool rnpgbe_down(struct mucse *mucse)
{
struct net_device *netdev = mucse->netdev;
@@ -893,9 +1390,12 @@ bool rnpgbe_down(struct mucse *mucse)
return false;
netif_tx_disable(netdev);
- rnpgbe_napi_disable_all(mucse);
+ rnpgbe_cancel_rx_retry_timers(mucse);
rnpgbe_irq_disable(mucse);
+ rnpgbe_stop_all_rx_rings(mucse);
+ rnpgbe_napi_disable_all(mucse);
rnpgbe_clean_all_tx_rings(mucse);
+ rnpgbe_clean_all_rx_rings(mucse);
return true;
}
@@ -917,6 +1417,8 @@ void rnpgbe_up_complete(struct mucse *mucse)
clear_bit(__MUCSE_DOWN, &mucse->state);
rnpgbe_irq_enable(mucse);
netif_tx_start_all_queues(netdev);
+ for (int i = 0; i < mucse->num_rx_queues; i++)
+ mucse_ring_wr32(mucse->rx_ring[i], RNPGBE_RX_START, 1);
}
/**
@@ -1321,5 +1823,251 @@ void rnpgbe_get_stats64(struct net_device *netdev,
stats->tx_bytes += bytes;
}
}
+
+ for (i = 0; i < mucse->num_rx_queues; i++) {
+ struct mucse_ring *ring = READ_ONCE(mucse->rx_ring[i]);
+ u64 bytes, packets, dropped;
+ unsigned int start;
+
+ if (ring) {
+ do {
+ start = u64_stats_fetch_begin(&ring->syncp);
+ packets = ring->stats.packets;
+ bytes = ring->stats.bytes;
+ } while (u64_stats_fetch_retry(&ring->syncp, start));
+ dropped = atomic64_read(&ring->stats.dropped);
+
+ stats->rx_packets += packets;
+ stats->rx_dropped += dropped;
+ stats->rx_bytes += bytes;
+ }
+ }
rcu_read_unlock();
}
+
+static int mucse_alloc_page_pool(struct mucse_ring *rx_ring)
+{
+ int ret = 0;
+
+ struct page_pool_params pp_params = {
+ .flags = PP_FLAG_DMA_MAP | PP_FLAG_DMA_SYNC_DEV,
+ .order = 0,
+ .pool_size = rx_ring->count,
+ .nid = dev_to_node(rx_ring->dev),
+ .dev = rx_ring->dev,
+ .dma_dir = DMA_FROM_DEVICE,
+ .offset = 0,
+ .max_len = PAGE_SIZE,
+ };
+
+ rx_ring->page_pool = page_pool_create(&pp_params);
+ if (IS_ERR(rx_ring->page_pool)) {
+ ret = PTR_ERR(rx_ring->page_pool);
+ rx_ring->page_pool = NULL;
+ }
+
+ return ret;
+}
+
+/**
+ * rnpgbe_setup_rx_resources - allocate Rx resources (Descriptors)
+ * @rx_ring: rx descriptor ring (for a specific queue) to setup
+ * @mucse: pointer to private structure
+ *
+ * Return: 0 on success, negative on failure
+ **/
+static int rnpgbe_setup_rx_resources(struct mucse_ring *rx_ring,
+ struct mucse *mucse)
+{
+ struct device *dev = rx_ring->dev;
+ int size;
+
+ size = sizeof(struct mucse_rx_buffer) * rx_ring->count;
+
+ rx_ring->rx_buffer_info = vzalloc(size);
+
+ if (!rx_ring->rx_buffer_info)
+ goto err_return;
+ /* Round up to nearest 4K */
+ rx_ring->size = rx_ring->count * sizeof(union rnpgbe_rx_desc);
+ rx_ring->size = ALIGN(rx_ring->size, 4096);
+ rx_ring->desc = dma_alloc_coherent(dev, rx_ring->size, &rx_ring->dma,
+ GFP_KERNEL);
+ if (!rx_ring->desc)
+ goto err_free_buffer;
+
+ rx_ring->next_to_clean = 0;
+ rx_ring->next_to_use = 0;
+
+ if (mucse_alloc_page_pool(rx_ring))
+ goto err_free_desc;
+
+ return 0;
+err_free_desc:
+ dma_free_coherent(dev, rx_ring->size, rx_ring->desc,
+ rx_ring->dma);
+ rx_ring->desc = NULL;
+err_free_buffer:
+ vfree(rx_ring->rx_buffer_info);
+err_return:
+ rx_ring->rx_buffer_info = NULL;
+ return -ENOMEM;
+}
+
+/**
+ * rnpgbe_clean_rx_ring - Free Rx Buffers per Queue
+ * @rx_ring: ring to free buffers from
+ **/
+void rnpgbe_clean_rx_ring(struct mucse_ring *rx_ring)
+{
+ struct mucse_rx_buffer *rx_buffer;
+ u16 i;
+
+ /* ring already cleared, nothing to do */
+ if (!rx_ring->rx_buffer_info)
+ return;
+ /* Free all the Rx ring sk_buffs */
+ for (i = 0; i < rx_ring->count; i++) {
+ rx_buffer = &rx_ring->rx_buffer_info[i];
+
+ if (rx_buffer->skb) {
+ struct sk_buff *skb = rx_buffer->skb;
+
+ dev_kfree_skb(skb);
+ rx_buffer->skb = NULL;
+ }
+
+ if (rx_buffer->page) {
+ page_pool_put_full_page(rx_ring->page_pool,
+ rx_buffer->page, false);
+ rx_buffer->page = NULL;
+ }
+ }
+
+ rx_ring->next_to_clean = 0;
+ rx_ring->next_to_use = 0;
+}
+
+/**
+ * rnpgbe_free_rx_resources - Free Rx Resources
+ * @rx_ring: ring to clean the resources from
+ *
+ * Free all receive software resources
+ **/
+static void rnpgbe_free_rx_resources(struct mucse_ring *rx_ring)
+{
+ vfree(rx_ring->rx_buffer_info);
+ rx_ring->rx_buffer_info = NULL;
+ /* if not set, then don't free */
+ if (!rx_ring->desc)
+ return;
+
+ dma_free_coherent(rx_ring->dev, rx_ring->size, rx_ring->desc,
+ rx_ring->dma);
+ rx_ring->desc = NULL;
+ if (rx_ring->page_pool) {
+ page_pool_destroy(rx_ring->page_pool);
+ rx_ring->page_pool = NULL;
+ }
+}
+
+/**
+ * rnpgbe_setup_all_rx_resources - allocate all queues Rx resources
+ * @mucse: pointer to private structure
+ *
+ * Return: 0 on success, negative on failure
+ **/
+int rnpgbe_setup_all_rx_resources(struct mucse *mucse)
+{
+ int i, err = 0;
+
+ for (i = 0; i < mucse->num_rx_queues; i++) {
+ err = rnpgbe_setup_rx_resources(mucse->rx_ring[i], mucse);
+ if (!err)
+ continue;
+
+ goto err_setup_rx;
+ }
+
+ return 0;
+err_setup_rx:
+ while (i--)
+ rnpgbe_free_rx_resources(mucse->rx_ring[i]);
+ return err;
+}
+
+/**
+ * rnpgbe_free_all_rx_resources - Free Rx Resources for All Queues
+ * @mucse: pointer to private structure
+ *
+ * Free all receive software resources
+ **/
+void rnpgbe_free_all_rx_resources(struct mucse *mucse)
+{
+ int i;
+
+ for (i = 0; i < mucse->num_rx_queues; i++)
+ rnpgbe_free_rx_resources(mucse->rx_ring[i]);
+}
+
+/**
+ * rnpgbe_configure_rx_ring - Configure Rx ring info to hw
+ * @mucse: pointer to private structure
+ * @ring: structure containing ring specific data
+ *
+ * Configure the Rx descriptor ring after a reset.
+ **/
+static void rnpgbe_configure_rx_ring(struct mucse *mucse,
+ struct mucse_ring *ring)
+{
+ struct mucse_hw *hw = &mucse->hw;
+
+ /* Stop hw. hardware design guarantees:
+ * - No new descriptors will be fetched after RX_START=0
+ * - No DMA will be initiated for already-fetched descriptors
+ */
+ mucse_ring_wr32(ring, RNPGBE_RX_START, 0);
+ /* Flush posted write to ensure hardware sees RX_START=0 */
+ (void)mucse_ring_rd32(ring, RNPGBE_RX_START);
+ /* Wait for in-flight DMA to quiesce */
+ usleep_range(300, 500);
+ /* Set the descriptor registers */
+ mucse_ring_wr32(ring, RNPGBE_RX_BASE_ADDR_LO, (u32)ring->dma);
+ mucse_ring_wr32(ring, RNPGBE_RX_BASE_ADDR_HI,
+ (u32)((u64)ring->dma >> 32) | (hw->pfvfnum << 24));
+ mucse_ring_wr32(ring, RNPGBE_RX_LEN, ring->count);
+ ring->tail = ring->ring_addr + RNPGBE_RX_TAIL;
+ ring->next_to_clean = mucse_ring_rd32(ring, RNPGBE_RX_HEAD) %
+ ring->count;
+ ring->next_to_use = ring->next_to_clean;
+ ring->drop_status = false;
+ mucse_ring_wr32(ring, RNPGBE_RX_SG_LEN, M_DEFAULT_SG);
+ mucse_ring_wr32(ring, RNPGBE_RX_FETCH, M_DEFAULT_RX_FETCH);
+ mucse_ring_wr32(ring, RNPGBE_RX_TIMEOUT_TH, 0);
+ mucse_ring_wr32(ring, RNPGBE_RX_INT_TIMER,
+ M_DEFAULT_INT_TIMER_R * hw->cycles_per_us);
+ mucse_ring_wr32(ring, RNPGBE_RX_INT_PKTCNT, M_DEFAULT_RX_INT_PKTCNT);
+ if (rnpgbe_alloc_rx_buffers(ring, mucse_desc_unused_rx(ring))) {
+ mod_timer(&ring->q_vector->rx_alloc_timer,
+ jiffies + msecs_to_jiffies(500));
+ }
+}
+
+/**
+ * rnpgbe_configure_rx - Configure Receive Unit after Reset
+ * @mucse: pointer to private structure
+ *
+ * Configure the Rx unit after a reset.
+ **/
+void rnpgbe_configure_rx(struct mucse *mucse)
+{
+ struct mucse_hw *hw = &mucse->hw;
+ u32 dma_axi_ctl;
+
+ for (int i = 0; i < mucse->num_rx_queues; i++)
+ rnpgbe_configure_rx_ring(mucse, mucse->rx_ring[i]);
+
+ dma_axi_ctl = mucse_hw_rd32(hw, RNPGBE_DMA_AXI_EN);
+ dma_axi_ctl |= RX_AXI_RW_EN;
+ mucse_hw_wr32(hw, RNPGBE_DMA_AXI_EN, dma_axi_ctl);
+}
diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_lib.h b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_lib.h
index 0ac4514b07c7..f3f45281d86e 100644
--- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_lib.h
+++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_lib.h
@@ -9,12 +9,28 @@ struct mucse_hw;
struct mucse_ring;
#define RING_OFFSET(n) (0x1000 + 0x100 * (n))
+#define RNPGBE_RX_START 0x10
#define RNPGBE_TX_START 0x18
#define RNPGBE_DMA_INT_MASK 0x24
#define TX_INT_MASK BIT(1)
#define RX_INT_MASK BIT(0)
#define INT_VALID (BIT(16) | BIT(17))
#define RNPGBE_DMA_INT_TRIG 0x2c /* lost-interrupt recovery trigger */
+#define RNPGBE_RX_BASE_ADDR_HI 0x30
+#define RNPGBE_RX_BASE_ADDR_LO 0x34
+#define RNPGBE_RX_LEN 0x38
+#define RNPGBE_RX_HEAD 0x3c
+#define RNPGBE_RX_TAIL 0x40
+#define M_DEFAULT_RX_FETCH 0x100020
+#define RNPGBE_RX_FETCH 0x44
+#define M_DEFAULT_INT_TIMER_R 30
+#define RNPGBE_RX_INT_TIMER 0x48
+#define M_DEFAULT_RX_INT_PKTCNT 64
+#define RNPGBE_RX_INT_PKTCNT 0x4c
+#define RNPGBE_RX_ARB_DEF_LVL 0x50
+#define RNPGBE_RX_TIMEOUT_TH 0x54
+#define M_DEFAULT_SG 96 /* unit 16b, 1536 bytes */
+#define RNPGBE_RX_SG_LEN 0x58
#define RNPGBE_TX_BASE_ADDR_HI 0x60
#define RNPGBE_TX_BASE_ADDR_LO 0x64
#define RNPGBE_TX_LEN 0x68
@@ -37,13 +53,15 @@ struct mucse_ring;
/* 2 desc gap to keep tail from touching head */
/* 1 desc for context descriptor */
#define RESV_DESC_NEEDED 3
+#define RNPGBE_SKB_PAD (NET_SKB_PAD + NET_IP_ALIGN)
+#define M_RX_BUFFER_WRITE 16
+
/* Hardware requires this to be nonzero */
#define M_DEFAULT_MAC_IP_LEN 20
#define mucse_for_each_ring(pos, head)\
for (typeof((head).ring) __pos = (head).ring;\
__pos ? ({ pos = __pos; 1; }) : 0;\
__pos = __pos->next)
-
int rnpgbe_init_interrupt_scheme(struct mucse *mucse);
void rnpgbe_clear_interrupt_scheme(struct mucse *mucse);
int rnpgbe_request_mbx_irq(struct mucse *mucse);
@@ -54,10 +72,15 @@ void rnpgbe_irq_disable(struct mucse *mucse);
bool rnpgbe_down(struct mucse *mucse);
void rnpgbe_up_complete(struct mucse *mucse);
void rnpgbe_configure_tx(struct mucse *mucse);
+void rnpgbe_configure_rx(struct mucse *mucse);
+void rnpgbe_clean_all_tx_rings(struct mucse *mucse);
int rnpgbe_setup_all_tx_resources(struct mucse *mucse);
void rnpgbe_free_all_tx_resources(struct mucse *mucse);
netdev_tx_t rnpgbe_xmit_frame_ring(struct sk_buff *skb,
struct mucse_ring *tx_ring);
void rnpgbe_get_stats64(struct net_device *netdev,
struct rtnl_link_stats64 *stats);
+void rnpgbe_clean_rx_ring(struct mucse_ring *rx_ring);
+int rnpgbe_setup_all_rx_resources(struct mucse *mucse);
+void rnpgbe_free_all_rx_resources(struct mucse *mucse);
#endif
diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c
index 24f4cf27a307..0ff8afe628e3 100644
--- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c
+++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c
@@ -36,6 +36,7 @@ static struct pci_device_id rnpgbe_pci_tbl[] = {
static void rnpgbe_configure(struct mucse *mucse)
{
rnpgbe_configure_tx(mucse);
+ rnpgbe_configure_rx(mucse);
}
/**
@@ -64,11 +65,17 @@ static int rnpgbe_open(struct net_device *netdev)
err = rnpgbe_setup_all_tx_resources(mucse);
if (err)
goto err_free_irqs;
+ err = rnpgbe_setup_all_rx_resources(mucse);
+ if (err)
+ goto err_free_tx;
rnpgbe_configure(mucse);
rnpgbe_up_complete(mucse);
return 0;
+err_free_tx:
+ rnpgbe_clean_all_tx_rings(mucse);
+ rnpgbe_free_all_tx_resources(mucse);
err_free_irqs:
rnpgbe_free_irq(mucse);
return err;
@@ -92,6 +99,7 @@ static int rnpgbe_close(struct net_device *netdev)
rnpgbe_free_irq(mucse);
rnpgbe_free_all_tx_resources(mucse);
+ rnpgbe_free_all_rx_resources(mucse);
return 0;
}
@@ -129,6 +137,7 @@ static const struct net_device_ops rnpgbe_netdev_ops = {
static void rnpgbe_sw_init(struct mucse *mucse)
{
mucse->tx_ring_item_count = M_DEFAULT_TXD;
+ mucse->rx_ring_item_count = M_DEFAULT_RXD;
mucse->tx_work_limit = M_DEFAULT_TX_WORK;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH net-next v9 3/4] net: rnpgbe: Add RX packet reception support
2026-08-14 11:13 ` [PATCH net-next v9 3/4] net: rnpgbe: Add RX packet reception support Dong Yibo
@ 2026-08-18 11:15 ` Simon Horman
0 siblings, 0 replies; 9+ messages in thread
From: Simon Horman @ 2026-08-18 11:15 UTC (permalink / raw)
To: Dong Yibo
Cc: andrew+netdev, davem, edumazet, kuba, pabeni, vadim.fedorenko,
u.kleine-koenig, netdev, linux-kernel, yaojun
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260814111317.1741087-2-dong100@mucse.com?part=3
---
--- Patch [3]: [PATCH net-next v9 3/4] net: rnpgbe: Add RX packet reception support ---
commit a40163a8c61d2ade85f38149bd7ae36c6f8c266b
Author: Dong Yibo <dong100@mucse.com>
net: rnpgbe: Add RX packet reception support
This adds the RX path for rnpgbe: the hardware write-back RX descriptor
format, page_pool backed RX buffers with one page per descriptor, the NAPI
poll callback, RX ring setup/teardown and RX statistics. A per q_vector
timer retries failed page allocations by scheduling NAPI.
A few questions on the new code below.
> diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h
> index 49fcbd2e37409..1d3e4078f16b1 100644
> --- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h
> +++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h
[ ... ]
> @@ -112,6 +150,18 @@ static inline u16 mucse_desc_unused(struct mucse_ring *ring)
> return ((ntc > ntu) ? 0 : ring->count) + ntc - ntu - 1;
> }
>
> +static inline u16 mucse_desc_unused_rx(struct mucse_ring *ring)
> +{
> + u16 ntc = ring->next_to_clean;
> + u16 ntu = ring->next_to_use;
> +
> + /* Keep M_RX_BUFFER_WRITE descriptors unused so the ring is not filled
> + * completely. Refill is attempted once at least this many descriptors
> + * are available.
> + */
> + return ((ntc > ntu) ? 0 : ring->count) + ntc - ntu - 16;
> +}
[Severity: Low]
[Finding: main-1-1]
[Sources: opus-5]
The comment in mucse_desc_unused_rx() says the reserved gap is
M_RX_BUFFER_WRITE descriptors, but the expression subtracts a literal 16.
M_RX_BUFFER_WRITE is defined independently in rnpgbe_lib.h and is what
rnpgbe_clean_rx_irq() uses as the refill threshold, and rnpgbe.h does not
include rnpgbe_lib.h, so the helper cannot reference the macro as written.
Both values are 16 today, so the accounting is consistent, but if
M_RX_BUFFER_WRITE is later changed only the refill trigger moves while the
reserved gap stays at 16. And if the reserve were ever made smaller than
the number of posted descriptors, this u16 expression would wrap to roughly
65500 and rnpgbe_alloc_rx_buffers() would walk well past the ring.
Would it make sense to have the helper use the same constant rather than
open-coding 16?
> diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_lib.c b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_lib.c
> index 8700c48669ecb..9199a48e75d42 100644
> --- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_lib.c
> +++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_lib.c
[ ... ]
> @@ -199,8 +201,412 @@ static bool rnpgbe_clean_tx_irq(struct mucse_q_vector *q_vector,
[ ... ]
> +static int rnpgbe_clean_rx_irq(struct mucse_q_vector *q_vector,
> + struct mucse_ring *rx_ring,
> + int budget)
> +{
> + unsigned int max_size = SKB_WITH_OVERHEAD(PAGE_SIZE) - RNPGBE_SKB_PAD;
> + unsigned int total_rx_bytes = 0, total_rx_packets = 0;
> + u16 cleaned_count = mucse_desc_unused_rx(rx_ring);
> + unsigned int work_done = 0;
> +
> + while (likely(work_done < budget)) {
> + struct mucse_rx_buffer *rx_buffer;
> + union rnpgbe_rx_desc *rx_desc;
> + struct sk_buff *skb;
> + unsigned int size;
> +
> + if (cleaned_count >= M_RX_BUFFER_WRITE) {
> + if (rnpgbe_alloc_rx_buffers(rx_ring, cleaned_count)) {
> + mod_timer(&q_vector->rx_alloc_timer,
> + jiffies + msecs_to_jiffies(500));
> + cleaned_count = mucse_desc_unused_rx(rx_ring);
> + } else {
> + cleaned_count = 0;
> + }
> + }
[ ... ]
> @@ -893,9 +1390,12 @@ bool rnpgbe_down(struct mucse *mucse)
> return false;
>
> netif_tx_disable(netdev);
> - rnpgbe_napi_disable_all(mucse);
> + rnpgbe_cancel_rx_retry_timers(mucse);
> rnpgbe_irq_disable(mucse);
> + rnpgbe_stop_all_rx_rings(mucse);
> + rnpgbe_napi_disable_all(mucse);
> rnpgbe_clean_all_tx_rings(mucse);
> + rnpgbe_clean_all_rx_rings(mucse);
>
> return true;
> }
[Severity: Medium]
[Finding: gpt-5-6-sol-5-12]
[Sources: gpt-5-6-sol, opus-5]
Can the rx_alloc_timer still be armed after rnpgbe_cancel_rx_retry_timers()
returns here? The timers are cancelled before rnpgbe_napi_disable_all(),
but NAPI is the only context that arms them:
rnpgbe_clean_rx_irq()
if (rnpgbe_alloc_rx_buffers(rx_ring, cleaned_count)) {
mod_timer(&q_vector->rx_alloc_timer,
jiffies + msecs_to_jiffies(500));
timer_delete_sync() only guarantees the timer is neither pending nor running
when it returns, so a poll still in flight on another CPU can re-arm it:
CPU0 CPU1 (NAPI softirq)
rnpgbe_down()
rnpgbe_cancel_rx_retry_timers()
timer_delete_sync()
rnpgbe_clean_rx_irq()
mod_timer(&q_vector->rx_alloc_timer)
rnpgbe_napi_disable_all()
rnpgbe_clean_all_rx_rings()
rnpgbe_close() then calls rnpgbe_free_all_rx_resources(), which vfree()s
rx_buffer_info, dma_free_coherent()s the descriptor ring and calls
page_pool_destroy(), while that timer is still pending.
Today the damage looks bounded: rnpgbe_rx_alloc_retry() only calls
napi_schedule(), rnpgbe_free_q_vector() does its own timer_delete_sync()
before kfree(q_vector), and after napi_disable() napi_schedule_prep() just
sets NAPIF_STATE_MISSED, which produces one spurious poll after the next
napi_enable(). That still leaves a timer armed over freed ring state, and
it contradicts the commit message wording that the timer "is cancelled
before RX cleanup and q_vector teardown".
Would cancelling the timers after rnpgbe_napi_disable_all() (or cancelling
them a second time there) close this window?
[ ... ]
> @@ -1321,5 +1823,251 @@ void rnpgbe_get_stats64(struct net_device *netdev,
[ ... ]
> +static int mucse_alloc_page_pool(struct mucse_ring *rx_ring)
> +{
> + int ret = 0;
> +
> + struct page_pool_params pp_params = {
> + .flags = PP_FLAG_DMA_MAP | PP_FLAG_DMA_SYNC_DEV,
> + .order = 0,
> + .pool_size = rx_ring->count,
> + .nid = dev_to_node(rx_ring->dev),
> + .dev = rx_ring->dev,
> + .dma_dir = DMA_FROM_DEVICE,
> + .offset = 0,
> + .max_len = PAGE_SIZE,
> + };
> +
> + rx_ring->page_pool = page_pool_create(&pp_params);
[Severity: Low]
[Finding: gpt-5-6-sol-2-7]
[Sources: gpt-5-6-sol]
This isn't a bug, but was leaving .netdev, .queue_idx and .napi unset
intentional? All three are known at this point: rx_ring->netdev,
rx_ring->queue_index and &rx_ring->q_vector->napi.
page_pool_list() in net/core/page_pool_user.c only links a pool into the
netdev list when slow.netdev is set:
if (pool->slow.netdev) {
hlist_add_head(&pool->user.list,
&pool->slow.netdev->page_pools);
so these per-ring pools are not visible per interface or per queue through
the netdev page-pool netlink interface. With .napi unset, pages returned
via skb_mark_for_recycle() also cannot use the NAPI-local recycle fast path
and always go through the ptr_ring.
> diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_lib.h b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_lib.h
> index 0ac4514b07c74..f3f45281d86e4 100644
> --- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_lib.h
> +++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_lib.h
> @@ -9,12 +9,28 @@ struct mucse_hw;
> struct mucse_ring;
>
> #define RING_OFFSET(n) (0x1000 + 0x100 * (n))
> +#define RNPGBE_RX_START 0x10
> #define RNPGBE_TX_START 0x18
> #define RNPGBE_DMA_INT_MASK 0x24
> #define TX_INT_MASK BIT(1)
> #define RX_INT_MASK BIT(0)
> #define INT_VALID (BIT(16) | BIT(17))
> #define RNPGBE_DMA_INT_TRIG 0x2c /* lost-interrupt recovery trigger */
> +#define RNPGBE_RX_BASE_ADDR_HI 0x30
> +#define RNPGBE_RX_BASE_ADDR_LO 0x34
> +#define RNPGBE_RX_LEN 0x38
> +#define RNPGBE_RX_HEAD 0x3c
> +#define RNPGBE_RX_TAIL 0x40
> +#define M_DEFAULT_RX_FETCH 0x100020
> +#define RNPGBE_RX_FETCH 0x44
> +#define M_DEFAULT_INT_TIMER_R 30
> +#define RNPGBE_RX_INT_TIMER 0x48
> +#define M_DEFAULT_RX_INT_PKTCNT 64
> +#define RNPGBE_RX_INT_PKTCNT 0x4c
> +#define RNPGBE_RX_ARB_DEF_LVL 0x50
^^^^
Simon says: I think the concerns below are highly subjective.
I think you can use your own best judgement here.
[Severity: Low]
[Finding: main-1-2]
[Sources: opus-5]
RNPGBE_RX_ARB_DEF_LVL has no users anywhere in the driver, while every
other RX register macro added here is programmed by
rnpgbe_configure_rx_ring(). Is the RX arbitration default level a
configuration step that was missed, or can this define be dropped?
Related, rnpgbe_clean_rx_ring() is declared in this header but its only
caller is rnpgbe_clean_all_rx_rings() in the same file. The TX side is the
other way round: rnpgbe_clean_tx_ring() is static and only the all-rings
wrapper is exported. Should rnpgbe_clean_rx_ring() be static as well?
> +#define RNPGBE_RX_TIMEOUT_TH 0x54
> +#define M_DEFAULT_SG 96 /* unit 16b, 1536 bytes */
> +#define RNPGBE_RX_SG_LEN 0x58
> #define RNPGBE_TX_BASE_ADDR_HI 0x60
> #define RNPGBE_TX_BASE_ADDR_LO 0x64
> #define RNPGBE_TX_LEN 0x68
> @@ -37,13 +53,15 @@ struct mucse_ring;
> /* 2 desc gap to keep tail from touching head */
> /* 1 desc for context descriptor */
> #define RESV_DESC_NEEDED 3
> +#define RNPGBE_SKB_PAD (NET_SKB_PAD + NET_IP_ALIGN)
> +#define M_RX_BUFFER_WRITE 16
> +
[ ... ]
> @@ -54,10 +72,15 @@ void rnpgbe_irq_disable(struct mucse *mucse);
> bool rnpgbe_down(struct mucse *mucse);
> void rnpgbe_up_complete(struct mucse *mucse);
> void rnpgbe_configure_tx(struct mucse *mucse);
> +void rnpgbe_configure_rx(struct mucse *mucse);
> +void rnpgbe_clean_all_tx_rings(struct mucse *mucse);
> int rnpgbe_setup_all_tx_resources(struct mucse *mucse);
> void rnpgbe_free_all_tx_resources(struct mucse *mucse);
> netdev_tx_t rnpgbe_xmit_frame_ring(struct sk_buff *skb,
> struct mucse_ring *tx_ring);
> void rnpgbe_get_stats64(struct net_device *netdev,
> struct rtnl_link_stats64 *stats);
> +void rnpgbe_clean_rx_ring(struct mucse_ring *rx_ring);
> +int rnpgbe_setup_all_rx_resources(struct mucse *mucse);
> +void rnpgbe_free_all_rx_resources(struct mucse *mucse);
> #endif
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH net-next v9 4/4] net: rnpgbe: Add link status handling support
2026-08-14 11:13 [PATCH net-next v9 0/4] net: rnpgbe: Add TX/RX and link status support Dong Yibo
` (2 preceding siblings ...)
2026-08-14 11:13 ` [PATCH net-next v9 3/4] net: rnpgbe: Add RX packet reception support Dong Yibo
@ 2026-08-14 11:13 ` Dong Yibo
2026-08-18 12:04 ` Simon Horman
3 siblings, 1 reply; 9+ messages in thread
From: Dong Yibo @ 2026-08-14 11:13 UTC (permalink / raw)
To: andrew+netdev, davem, edumazet, kuba, pabeni, vadim.fedorenko,
u.kleine-koenig
Cc: netdev, linux-kernel, dong100, yaojun
Add link status handling for the RNPGBE driver.
Introduce link state data structures for speed, duplex and link state,
and process firmware link events delivered over the mailbox, since
firmware controls the link and phylink is not used. Add a service task
to handle pending events and manage carrier status with
netif_carrier_on()/netif_carrier_off().
Notify firmware when the interface is brought up and after the data path
is quiesced during teardown, and fail interface open if the firmware
port-up or link-report setup fails. Validate firmware link events before
updating the cached link state. Enable the GMAC receiver in receive-all
mode because packet filtering is done by the chip-level filter, and
document the driver-to-firmware link-state snapshot and reset semantics.
Signed-off-by: Dong Yibo <dong100@mucse.com>
---
drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h | 13 +-
.../net/ethernet/mucse/rnpgbe/rnpgbe_chip.c | 37 +++-
drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h | 19 ++
.../net/ethernet/mucse/rnpgbe/rnpgbe_lib.c | 182 +++++++++++++++-
.../net/ethernet/mucse/rnpgbe/rnpgbe_lib.h | 3 +-
.../net/ethernet/mucse/rnpgbe/rnpgbe_main.c | 16 +-
.../net/ethernet/mucse/rnpgbe/rnpgbe_mbx.c | 20 ++
.../net/ethernet/mucse/rnpgbe/rnpgbe_mbx.h | 1 +
.../net/ethernet/mucse/rnpgbe/rnpgbe_mbx_fw.c | 200 +++++++++++++++++-
.../net/ethernet/mucse/rnpgbe/rnpgbe_mbx_fw.h | 50 +++++
10 files changed, 533 insertions(+), 8 deletions(-)
diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h
index 1d3e4078f16b..482c13d987b1 100644
--- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h
+++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h
@@ -5,6 +5,7 @@
#define _RNPGBE_H
#include <linux/types.h>
+#include <linux/atomic.h>
#include <linux/mutex.h>
#include <linux/netdevice.h>
#include <linux/timer.h>
@@ -32,11 +33,10 @@ struct mucse_mbx_info {
u32 fwpf_ctrl_base;
};
-/* Enum for firmware notification modes,
- * more modes (e.g., portup, link_report) will be added in future
- **/
enum {
mucse_fw_powerup,
+ mucse_fw_portup,
+ mucse_fw_link_report_en,
};
struct mucse_hw {
@@ -45,8 +45,11 @@ struct mucse_hw {
struct pci_dev *pdev;
struct mucse_mbx_info mbx;
int port;
+ int speed;
+ bool link;
u16 cycles_per_us;
u8 pfvfnum;
+ u8 duplex;
};
struct rnpgbe_tx_desc {
@@ -220,7 +223,10 @@ struct mucse {
int num_rx_queues;
char mbx_name[32];
unsigned long state;
+ atomic_t link_pending;
struct work_struct mbx_work;
+ struct delayed_work serv_task;
+ spinlock_t link_lock; /* spinlock for link update */
};
int rnpgbe_get_permanent_mac(struct mucse_hw *hw, u8 *perm_addr);
@@ -229,6 +235,7 @@ int rnpgbe_send_notify(struct mucse_hw *hw,
bool enable,
int mode);
int rnpgbe_init_hw(struct mucse_hw *hw, int board_type);
+void rnpgbe_set_link(struct mucse_hw *hw, bool linkup);
/* Device IDs */
#define PCI_VENDOR_ID_MUCSE 0x8848
diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_chip.c b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_chip.c
index 291e77d573fe..55b4abed82bd 100644
--- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_chip.c
+++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_chip.c
@@ -66,11 +66,17 @@ int rnpgbe_send_notify(struct mucse_hw *hw,
int mode)
{
int err;
- /* Keep switch struct to support more modes in the future */
+
switch (mode) {
case mucse_fw_powerup:
err = mucse_mbx_powerup(hw, enable);
break;
+ case mucse_fw_portup:
+ err = mucse_mbx_phyup(hw, enable);
+ break;
+ case mucse_fw_link_report_en:
+ err = mucse_mbx_link_report(hw, enable);
+ break;
default:
err = -EINVAL;
}
@@ -149,3 +155,32 @@ int rnpgbe_init_hw(struct mucse_hw *hw, int board_type)
return 0;
}
+
+/**
+ * rnpgbe_set_link - Set the hardware link state
+ * @hw: hw information structure
+ * @linkup: link on or not
+ *
+ * rnpgbe_set_link setup link status
+ *
+ **/
+void rnpgbe_set_link(struct mucse_hw *hw, bool linkup)
+{
+ u32 value = mucse_hw_rd32(hw, GMAC_CONTROL);
+
+ if (linkup)
+ value |= GMAC_CONTROL_RE;
+ else
+ value &= ~GMAC_CONTROL_RE;
+
+ mucse_hw_wr32(hw, GMAC_CONTROL, value);
+
+ /* Keep the GMAC in receive-all mode while the link is up. The
+ * chip-level filter does the actual address filtering, but there
+ * is no ndo_set_rx_mode yet to configure it.
+ */
+ if (linkup)
+ mucse_hw_wr32(hw, GMAC_FRAME_FILTER, GMAC_RX_ALL);
+ else
+ mucse_hw_wr32(hw, GMAC_FRAME_FILTER, 0);
+}
diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h
index 1d87edfba3d7..9e640e8d6703 100644
--- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h
+++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h
@@ -21,8 +21,27 @@
#define RNPGBE_DMA_TX_STATUS GENMASK_U32(23, 22)
#define RNPGBE_DMA_RX_STATUS GENMASK_U32(21, 20)
#define RX_AXI_RW_EN 0x03
+/* RNPGBE_LINK_ST is a driver-owned link-state snapshot consumed by firmware.
+ * M_DEFAULT_ST replaces the complete snapshot, causing firmware
+ * to report the current link state again when it differs from this default.
+ */
+#define M_ST_MASK (GENMASK_U32(31, 24) | \
+ GENMASK_U32(11, 8) | BIT(6) | \
+ BIT(4) | BIT(0))
+/* Set the driver-state marker; all other driver status fields start clear. */
+#define M_DEFAULT_ST 0xa0000000
+/* Driver-reported fields: 25:24 pause, 11:8 speed, 6 LLDP, 4 duplex,
+ * and 0 link up/down. M_DEFAULT_ST resets these fields so firmware reports
+ * the current hardware state again.
+ */
+#define RNPGBE_LINK_ST 0x000c
#define RNPGBE_DMA_AXI_EN 0x0010
#define RNPGBE_TX_MIN_PKT_LEN 33
+#define MUCSE_GMAC_OFF(_n) (0x20000 + (_n))
+#define GMAC_CONTROL_RE 0x00000004
+#define GMAC_CONTROL MUCSE_GMAC_OFF(0)
+#define GMAC_RX_ALL (BIT(31) | BIT(0))
+#define GMAC_FRAME_FILTER MUCSE_GMAC_OFF(0x4)
#define RNPGBE_MAX_QUEUES 8
#endif /* _RNPGBE_HW_H */
diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_lib.c b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_lib.c
index 9199a48e75d4..fa5ea4dd7a1c 100644
--- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_lib.c
+++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_lib.c
@@ -1385,15 +1385,43 @@ static void rnpgbe_cancel_rx_retry_timers(struct mucse *mucse)
bool rnpgbe_down(struct mucse *mucse)
{
struct net_device *netdev = mucse->netdev;
+ struct mucse_hw *hw = &mucse->hw;
+ unsigned long flags;
+ int err;
if (test_and_set_bit(__MUCSE_DOWN, &mucse->state))
return false;
+ cancel_delayed_work_sync(&mucse->serv_task);
+
+ spin_lock_irqsave(&mucse->link_lock, flags);
+ WRITE_ONCE(hw->link, false);
+ WRITE_ONCE(hw->speed, 0);
+ WRITE_ONCE(hw->duplex, 0);
+ atomic_set(&mucse->link_pending, 0);
+ spin_unlock_irqrestore(&mucse->link_lock, flags);
+ rnpgbe_set_link(hw, false);
+
+ netif_carrier_off(netdev);
netif_tx_disable(netdev);
rnpgbe_cancel_rx_retry_timers(mucse);
rnpgbe_irq_disable(mucse);
rnpgbe_stop_all_rx_rings(mucse);
rnpgbe_napi_disable_all(mucse);
+
+ err = rnpgbe_send_notify(hw, false, mucse_fw_link_report_en);
+ if (err) {
+ dev_warn(&hw->pdev->dev, "Send link report to hw failed %d\n",
+ err);
+ dev_warn(&hw->pdev->dev, "Fw will still report link event\n");
+ }
+
+ err = rnpgbe_send_notify(hw, false, mucse_fw_portup);
+ if (err) {
+ dev_warn(&hw->pdev->dev, "Send port down to hw failed %d\n",
+ err);
+ dev_warn(&hw->pdev->dev, "Port is not truly down\n");
+ }
rnpgbe_clean_all_tx_rings(mucse);
rnpgbe_clean_all_rx_rings(mucse);
@@ -1403,10 +1431,15 @@ bool rnpgbe_down(struct mucse *mucse)
/**
* rnpgbe_up_complete - Final step for port up
* @mucse: pointer to private structure
+ *
+ * Return: 0 on success, negative errno if firmware setup fails
**/
-void rnpgbe_up_complete(struct mucse *mucse)
+int rnpgbe_up_complete(struct mucse *mucse)
{
struct net_device *netdev = mucse->netdev;
+ struct mucse_hw *hw = &mucse->hw;
+ unsigned long flags;
+ int err;
if (mucse->flags & (M_FLAG_MSIX_EN | M_FLAG_MSIX_SINGLE_EN))
rnpgbe_configure_msix(mucse);
@@ -1414,11 +1447,50 @@ void rnpgbe_up_complete(struct mucse *mucse)
rnpgbe_configure_msi(mucse);
rnpgbe_napi_enable_all(mucse);
+ /* Clear stale link state and tell firmware the driver hasn't
+ * seen any link yet. Firmware will re-assert LINK_CHANGE_EVT
+ * when link_report_en is sent below since RNPGBE_LINK_ST
+ * no longer matches the actual (possibly up) link state.
+ */
+ spin_lock_irqsave(&mucse->link_lock, flags);
+ WRITE_ONCE(hw->link, false);
+ WRITE_ONCE(hw->speed, 0);
+ WRITE_ONCE(hw->duplex, 0);
+ atomic_set(&mucse->link_pending, 0);
+ /* echo fw driver now in default state */
+ mucse_hw_wr32(hw, RNPGBE_LINK_ST, M_DEFAULT_ST);
+ /* Keep the default snapshot and DOWN state transition atomic to
+ * mailbox link-event handling.
+ */
clear_bit(__MUCSE_DOWN, &mucse->state);
+ spin_unlock_irqrestore(&mucse->link_lock, flags);
rnpgbe_irq_enable(mucse);
netif_tx_start_all_queues(netdev);
for (int i = 0; i < mucse->num_rx_queues; i++)
mucse_ring_wr32(mucse->rx_ring[i], RNPGBE_RX_START, 1);
+
+ err = rnpgbe_send_notify(hw, true, mucse_fw_portup);
+ if (err) {
+ dev_err(&hw->pdev->dev,
+ "Failed to notify firmware that port is up: %d\n", err);
+ return err;
+ }
+ /* Firmware checks RNPGBE_LINK_ST (driver's last-received link state)
+ * and only asserts LINK_CHANGE_EVT when it differs from the actual link
+ * state AND link_report_en is true.
+ */
+ err = rnpgbe_send_notify(hw, true, mucse_fw_link_report_en);
+ if (err) {
+ dev_err(&hw->pdev->dev,
+ "Failed to enable firmware link reporting: %d\n",
+ err);
+ return err;
+ }
+
+ queue_delayed_work(system_percpu_wq, &mucse->serv_task,
+ msecs_to_jiffies(500));
+
+ return 0;
}
/**
@@ -2071,3 +2143,111 @@ void rnpgbe_configure_rx(struct mucse *mucse)
dma_axi_ctl |= RX_AXI_RW_EN;
mucse_hw_wr32(hw, RNPGBE_DMA_AXI_EN, dma_axi_ctl);
}
+
+/**
+ * rnpgbe_process_link_event - Consume a pending link event
+ * @mucse: pointer to the device private structure
+ * @link: link status snapshot
+ * @speed: link speed snapshot
+ * @duplex: link duplex snapshot
+ *
+ * Return: true if a link event was consumed, false otherwise
+ **/
+static bool rnpgbe_process_link_event(struct mucse *mucse, bool *link,
+ int *speed, u8 *duplex)
+{
+ struct mucse_hw *hw = &mucse->hw;
+ unsigned long flags;
+
+ if (!atomic_xchg(&mucse->link_pending, 0))
+ return false;
+
+ spin_lock_irqsave(&mucse->link_lock, flags);
+ *link = hw->link;
+ *speed = hw->speed;
+ *duplex = hw->duplex;
+ spin_unlock_irqrestore(&mucse->link_lock, flags);
+
+ return true;
+}
+
+/**
+ * rnpgbe_link_is_up - Update netif_carrier status and
+ * print link up message
+ * @mucse: pointer to the device private structure
+ * @speed: link speed snapshot
+ * @duplex: link duplex snapshot
+ **/
+static void rnpgbe_link_is_up(struct mucse *mucse, int speed, u8 duplex)
+{
+ struct net_device *netdev = mucse->netdev;
+ struct mucse_hw *hw = &mucse->hw;
+
+ /* Only continue if link was previously down */
+ if (netif_carrier_ok(netdev))
+ return;
+
+ netdev_info(netdev, "NIC Link is Up %d Mbps, %s Duplex\n",
+ speed, duplex ? "Full" : "Half");
+ rnpgbe_set_link(hw, true);
+ netif_carrier_on(netdev);
+}
+
+/**
+ * rnpgbe_link_is_down - Update netif_carrier status and
+ * print link down message
+ * @mucse: pointer to the private structure
+ **/
+static void rnpgbe_link_is_down(struct mucse *mucse)
+{
+ struct net_device *netdev = mucse->netdev;
+ struct mucse_hw *hw = &mucse->hw;
+
+ /* Only continue if link was up previously */
+ if (!netif_carrier_ok(netdev))
+ return;
+ netdev_info(netdev, "NIC Link is Down\n");
+ rnpgbe_set_link(hw, false);
+ netif_carrier_off(netdev);
+}
+
+/**
+ * rnpgbe_process_link_subtask - Process a link-state update
+ * @mucse: pointer to the device private structure
+ **/
+static void rnpgbe_process_link_subtask(struct mucse *mucse)
+{
+ bool link;
+ int speed;
+ u8 duplex;
+
+ /* if interface is down do nothing */
+ if (test_bit(__MUCSE_DOWN, &mucse->state))
+ return;
+
+ if (!rnpgbe_process_link_event(mucse, &link, &speed, &duplex))
+ return;
+
+ if (link)
+ rnpgbe_link_is_up(mucse, speed, duplex);
+ else
+ rnpgbe_link_is_down(mucse);
+}
+
+/**
+ * rnpgbe_service_task - Manages and runs subtasks
+ * @work: pointer to work_struct containing our data
+ **/
+void rnpgbe_service_task(struct work_struct *work)
+{
+ struct mucse *mucse = container_of(work, struct mucse, serv_task.work);
+
+ if (test_bit(__MUCSE_DOWN, &mucse->state))
+ return;
+
+ rnpgbe_process_link_subtask(mucse);
+
+ if (!test_bit(__MUCSE_DOWN, &mucse->state))
+ queue_delayed_work(system_percpu_wq, &mucse->serv_task,
+ msecs_to_jiffies(500));
+}
diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_lib.h b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_lib.h
index f3f45281d86e..5b93c1183c52 100644
--- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_lib.h
+++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_lib.h
@@ -70,7 +70,7 @@ int rnpgbe_request_irq(struct mucse *mucse);
void rnpgbe_free_irq(struct mucse *mucse);
void rnpgbe_irq_disable(struct mucse *mucse);
bool rnpgbe_down(struct mucse *mucse);
-void rnpgbe_up_complete(struct mucse *mucse);
+int rnpgbe_up_complete(struct mucse *mucse);
void rnpgbe_configure_tx(struct mucse *mucse);
void rnpgbe_configure_rx(struct mucse *mucse);
void rnpgbe_clean_all_tx_rings(struct mucse *mucse);
@@ -83,4 +83,5 @@ void rnpgbe_get_stats64(struct net_device *netdev,
void rnpgbe_clean_rx_ring(struct mucse_ring *rx_ring);
int rnpgbe_setup_all_rx_resources(struct mucse *mucse);
void rnpgbe_free_all_rx_resources(struct mucse *mucse);
+void rnpgbe_service_task(struct work_struct *work);
#endif
diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c
index 0ff8afe628e3..fcad8a16ef26 100644
--- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c
+++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c
@@ -53,6 +53,7 @@ static int rnpgbe_open(struct net_device *netdev)
struct mucse *mucse = netdev_priv(netdev);
int err;
+ netif_carrier_off(netdev);
err = rnpgbe_request_irq(mucse);
if (err)
return err;
@@ -70,9 +71,16 @@ static int rnpgbe_open(struct net_device *netdev)
goto err_free_tx;
rnpgbe_configure(mucse);
- rnpgbe_up_complete(mucse);
+ err = rnpgbe_up_complete(mucse);
+ if (err)
+ goto err_down;
return 0;
+err_down:
+ rnpgbe_down(mucse);
+ rnpgbe_free_all_rx_resources(mucse);
+ rnpgbe_free_all_tx_resources(mucse);
+ goto err_free_irqs;
err_free_tx:
rnpgbe_clean_all_tx_rings(mucse);
rnpgbe_free_all_tx_resources(mucse);
@@ -190,6 +198,7 @@ static int rnpgbe_add_adapter(struct pci_dev *pdev,
dev_err(&pdev->dev, "Init hw err %d\n", err);
goto err_free_net;
}
+
/* Step 1: Send power-up notification to firmware (no response expected)
* This informs firmware to initialize hardware power state, but
* firmware only acknowledges receipt without returning data. Must be
@@ -232,6 +241,10 @@ static int rnpgbe_add_adapter(struct pci_dev *pdev,
goto err_powerdown;
}
+ INIT_DELAYED_WORK(&mucse->serv_task, rnpgbe_service_task);
+ spin_lock_init(&mucse->link_lock);
+ atomic_set(&mucse->link_pending, 0);
+
err = rnpgbe_init_interrupt_scheme(mucse);
if (err) {
dev_err(&pdev->dev, "init interrupt failed %d\n", err);
@@ -251,6 +264,7 @@ static int rnpgbe_add_adapter(struct pci_dev *pdev,
netdev->hw_features |= NETIF_F_HIGHDMA;
}
+ netif_carrier_off(netdev);
err = register_netdev(netdev);
if (err)
goto err_remove_mbx;
diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx.c b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx.c
index 2310f01df1f5..185465d4e0dc 100644
--- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx.c
+++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx.c
@@ -252,6 +252,26 @@ int mucse_poll_and_read_mbx(struct mucse_hw *hw, __le32 *msg, u16 size)
return mucse_read_mbx_pf(hw, msg, size);
}
+/**
+ * mucse_check_and_read_mbx - check if there is notification and receive message
+ * @hw: pointer to the HW structure
+ * @msg: the message buffer
+ * @size: length of buffer
+ *
+ * Return: 0 if it successfully received a message notification and
+ * copied it into the receive buffer, negative errno on failure
+ **/
+int mucse_check_and_read_mbx(struct mucse_hw *hw, __le32 *msg, u16 size)
+{
+ int err;
+
+ err = mucse_check_for_msg_pf(hw);
+ if (err)
+ return err;
+
+ return mucse_read_mbx_pf(hw, msg, size);
+}
+
/**
* mucse_mbx_get_fwack - Read fw ack from reg
* @mbx: pointer to the MBX structure
diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx.h b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx.h
index 75b88b18b04d..3af008a67fb1 100644
--- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx.h
+++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx.h
@@ -18,4 +18,5 @@ int mucse_write_and_wait_ack_mbx(struct mucse_hw *hw,
const __le32 *msg, u16 size);
void mucse_init_mbx_params_pf(struct mucse_hw *hw);
int mucse_poll_and_read_mbx(struct mucse_hw *hw, __le32 *msg, u16 size);
+int mucse_check_and_read_mbx(struct mucse_hw *hw, __le32 *msg, u16 size);
#endif /* _RNPGBE_MBX_H */
diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx_fw.c b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx_fw.c
index 754016f6b4f0..441724f8603e 100644
--- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx_fw.c
+++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx_fw.c
@@ -3,6 +3,7 @@
#include <linux/if_ether.h>
#include <linux/bitfield.h>
+#include <linux/ethtool.h>
#include "rnpgbe.h"
#include "rnpgbe_mbx.h"
@@ -27,10 +28,14 @@ static int mucse_fw_send_cmd_wait_resp(struct mucse_hw *hw,
int retry_cnt = 3;
int err;
+ BUILD_BUG_ON(sizeof(struct mbx_fw_cmd_reply) != 56);
+
mutex_lock(&hw->mbx.lock);
+
err = mucse_write_and_wait_ack_mbx(hw, req->dwords, len);
if (err)
goto out;
+
do {
err = mucse_poll_and_read_mbx(hw, reply->dwords,
sizeof(reply->r));
@@ -199,9 +204,202 @@ int mucse_mbx_get_macaddr(struct mucse_hw *hw, int pfvfnum,
}
/**
- * mucse_fw_irq_handler - Try to handle a req from hw
+ * mucse_mbx_phyup - Request that firmware bring the PHY up or down
+ * @hw: pointer to the HW structure
+ * @is_phyup: true for up, false for down
+ *
+ * mucse_mbx_phyup echo fw to change phy status
+ *
+ * Return: 0 on success, negative errno on failure
+ **/
+int mucse_mbx_phyup(struct mucse_hw *hw, bool is_phyup)
+{
+ union mbx_fw_cmd_req_u req = {
+ .r = {
+ .datalen = cpu_to_le16(sizeof(req.r.phy_status) +
+ MUCSE_MBX_REQ_HDR_LEN),
+ .opcode = cpu_to_le16(SET_PHY_UP),
+ .phy_status = {
+ .port_mask = cpu_to_le32(BIT(hw->port)),
+ .status = cpu_to_le32(is_phyup ? 1 : 0),
+ },
+ },
+ };
+ int len, err;
+
+ len = le16_to_cpu(req.r.datalen);
+ mutex_lock(&hw->mbx.lock);
+ err = mucse_write_and_wait_ack_mbx(hw, req.dwords, len);
+ mutex_unlock(&hw->mbx.lock);
+
+ return err;
+}
+
+/**
+ * mucse_mbx_link_report - Configure firmware link-change event reporting
+ * @hw: pointer to the HW structure
+ * @is_report: true for report, false for no
+ *
+ * mucse_mbx_link_report echo fw to change event report state
+ *
+ * Return: 0 on success, negative errno on failure
+ **/
+int mucse_mbx_link_report(struct mucse_hw *hw, bool is_report)
+{
+ union mbx_fw_cmd_req_u req = {
+ .r = {
+ .datalen = cpu_to_le16(sizeof(req.r.report_status) +
+ MUCSE_MBX_REQ_HDR_LEN),
+ .opcode = cpu_to_le16(LINK_REPORT_EN),
+ .report_status = {
+ .port_mask = cpu_to_le16(BIT(hw->port)),
+ .status = cpu_to_le16(is_report ? 1 : 0),
+ },
+ },
+ };
+ int len, err;
+
+ len = le16_to_cpu(req.r.datalen);
+ mutex_lock(&hw->mbx.lock);
+ err = mucse_write_and_wait_ack_mbx(hw, req.dwords, len);
+ mutex_unlock(&hw->mbx.lock);
+
+ return err;
+}
+
+/**
+ * mucse_update_link_status_reg - update driver speed inf to reg
+ * @hw: pointer to the HW structure
+ * @req: pointer to req data
+ *
+ * Update the driver's link-state snapshot exported to firmware. Firmware
+ * sends a new event when this snapshot differs from the hardware state.
+ * The default snapshot clears the driver-reported fields;
+ * a valid event then repopulates them, including the LLDP status in bit 6.
+ *
+ **/
+static void mucse_update_link_status_reg(struct mucse_hw *hw,
+ struct mbx_fw_cmd_req *req)
+{
+ u16 status = le16_to_cpu(req->link_stat.st.status);
+ u16 speed = le16_to_cpu(req->link_stat.st.speed);
+ u32 value;
+
+ value = mucse_hw_rd32(hw, RNPGBE_LINK_ST);
+ value &= ~M_ST_MASK;
+ value |= M_DEFAULT_ST;
+
+ if (le16_to_cpu(req->link_stat.port_status)) {
+ value |= BIT(0);
+ switch (speed) {
+ case 10:
+ value |= (mucse_speed_10 << 8);
+ break;
+ case 100:
+ value |= (mucse_speed_100 << 8);
+ break;
+ case 1000:
+ value |= (mucse_speed_1000 << 8);
+ break;
+ default:
+ break;
+ }
+
+ value |= FIELD_PREP(BIT(4),
+ !!(req->link_stat.st.flags & DUPLEX_BIT));
+ value |= FIELD_PREP(GENMASK_U32(25, 24),
+ status & GENMASK(1, 0));
+ } else {
+ value &= ~BIT(0);
+ }
+
+ if (status & ST_STATUS_LLDP_STATUS_MASK)
+ value |= BIT(6);
+ else
+ value &= ~BIT(6);
+
+ mucse_hw_wr32(hw, RNPGBE_LINK_ST, value);
+}
+
+/**
+ * mucse_mbx_fw_req_handler - Handle fw req
+ * @hw: pointer to the HW structure
+ * @req: pointer to req data
+ *
+ * mucse_mbx_fw_req_handler handler fw req, such as a link event req.
+ **/
+static void mucse_mbx_fw_req_handler(struct mucse_hw *hw,
+ struct mbx_fw_cmd_req *req)
+{
+ struct mucse *mucse = container_of(hw, struct mucse, hw);
+ u32 magic = le32_to_cpu(req->link_stat.port_magic);
+ unsigned long flags;
+
+ if (le16_to_cpu(req->opcode) == LINK_CHANGE_EVT) {
+ spin_lock_irqsave(&mucse->link_lock, flags);
+ if (magic != ST_VALID_MAGIC) {
+ /* Do not let an invalid event change the cached link state.
+ * Reset the driver snapshot so firmware reports it again.
+ */
+ mucse_hw_wr32(hw, RNPGBE_LINK_ST, M_DEFAULT_ST);
+ spin_unlock_irqrestore(&mucse->link_lock, flags);
+ return;
+ }
+
+ if (test_bit(__MUCSE_DOWN, &mucse->state)) {
+ mucse_update_link_status_reg(hw, req);
+ spin_unlock_irqrestore(&mucse->link_lock, flags);
+ return;
+ }
+
+ if (le16_to_cpu(req->link_stat.port_status))
+ WRITE_ONCE(hw->link, true);
+ else
+ WRITE_ONCE(hw->link, false);
+
+ WRITE_ONCE(hw->speed, le16_to_cpu(req->link_stat.st.speed));
+ WRITE_ONCE(hw->duplex, req->link_stat.st.flags & DUPLEX_BIT);
+ /* update regs to notify link info is received */
+ mucse_update_link_status_reg(hw, req);
+ atomic_set_release(&mucse->link_pending, 1);
+ spin_unlock_irqrestore(&mucse->link_lock, flags);
+ }
+}
+
+/**
+ * mucse_fw_handle_event - Handle one pending firmware event
+ * @hw: pointer to the hardware structure
+ *
+ * Return: true if an event was handled, false otherwise
+ **/
+static bool mucse_fw_handle_event(struct mucse_hw *hw)
+{
+ union mbx_fw_cmd_u msg = {};
+ int err;
+
+ /* try to check and read fw req */
+ mutex_lock(&hw->mbx.lock);
+ err = mucse_check_and_read_mbx(hw, msg.dwords, sizeof(msg));
+ mutex_unlock(&hw->mbx.lock);
+ if (err)
+ return false;
+
+ mucse_mbx_fw_req_handler(hw, &msg.req);
+
+ return true;
+}
+
+/**
+ * mucse_fw_irq_handler - Drain pending firmware mailbox events
* @hw: pointer to the HW structure
+ *
+ * The only asynchronous event currently handled is LINK_CHANGE_EVT.
+ * Firmware rate-limits link-change notifications to a minimum interval
+ * of 500 ms.
**/
void mucse_fw_irq_handler(struct mucse_hw *hw)
{
+ /* Drain events coalesced while the mailbox work item was pending. */
+ while (mucse_fw_handle_event(hw))
+ continue;
}
diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx_fw.h b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx_fw.h
index a6bf5de55aa3..0e6972b7c3c0 100644
--- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx_fw.h
+++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx_fw.h
@@ -14,6 +14,9 @@ enum MUCSE_FW_CMD {
GET_HW_INFO = 0x0601,
GET_MAC_ADDRESS = 0x0602,
RESET_HW = 0x0603,
+ LINK_CHANGE_EVT = 0x0608,
+ LINK_REPORT_EN = 0x0613,
+ SET_PHY_UP = 0x0800,
POWER_UP = 0x0803,
};
@@ -36,6 +39,16 @@ struct mucse_hw_info {
__le32 ext_info;
} __packed;
+#define ST_STATUS_LLDP_STATUS_MASK BIT(12)
+
+#define DUPLEX_BIT BIT(0)
+struct st_status {
+ u8 phyid;
+ u8 flags;
+ __le16 speed;
+ __le16 status;
+} __packed;
+
struct mbx_fw_cmd_req {
__le16 flags;
__le16 opcode;
@@ -55,10 +68,30 @@ struct mbx_fw_cmd_req {
__le32 port_mask;
__le32 pfvf_num;
} get_mac_addr;
+ struct {
+ __le32 port_mask;
+ __le32 status;
+ } phy_status;
+ struct {
+ /* LINK_REPORT_EN uses its own 16-bit payload layout:
+ * status precedes port_mask.
+ */
+ __le16 status;
+ __le16 port_mask;
+ } report_status;
+ struct {
+ __le16 changed_lanes;
+ __le16 port_status;
+ __le32 port_magic;
+#define ST_VALID_MAGIC 0xa4a6a8a9
+ struct st_status st;
+ } link_stat;
};
} __packed;
struct mbx_fw_cmd_reply {
+#define FLAGS_REPLY BIT(0)
+#define FLAGS_ERR BIT(2)
__le16 flags;
__le16 opcode;
__le16 error_code;
@@ -94,10 +127,27 @@ union mbx_fw_cmd_reply_u {
__le32 dwords[sizeof(struct mbx_fw_cmd_reply) / sizeof(__le32)];
};
+union mbx_fw_cmd_u {
+ struct mbx_fw_cmd_req req;
+ struct mbx_fw_cmd_reply reply;
+ __le32 dwords[sizeof(struct mbx_fw_cmd_reply) / sizeof(__le32)];
+};
+
+/* Firmware encoding for RNPGBE_LINK_ST[11:8]. Valid link events report
+ * only these three speeds.
+ */
+enum mucse_speed {
+ mucse_speed_10 = 0,
+ mucse_speed_100 = 1,
+ mucse_speed_1000 = 2,
+};
+
int mucse_mbx_sync_fw(struct mucse_hw *hw);
int mucse_mbx_powerup(struct mucse_hw *hw, bool is_powerup);
int mucse_mbx_reset_hw(struct mucse_hw *hw);
int mucse_mbx_get_macaddr(struct mucse_hw *hw, int pfvfnum,
u8 *mac_addr, int port);
+int mucse_mbx_phyup(struct mucse_hw *hw, bool is_phyup);
+int mucse_mbx_link_report(struct mucse_hw *hw, bool is_report);
void mucse_fw_irq_handler(struct mucse_hw *hw);
#endif /* _RNPGBE_MBX_FW_H */
--
2.25.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH net-next v9 4/4] net: rnpgbe: Add link status handling support
2026-08-14 11:13 ` [PATCH net-next v9 4/4] net: rnpgbe: Add link status handling support Dong Yibo
@ 2026-08-18 12:04 ` Simon Horman
0 siblings, 0 replies; 9+ messages in thread
From: Simon Horman @ 2026-08-18 12:04 UTC (permalink / raw)
To: Dong Yibo
Cc: andrew+netdev, davem, edumazet, kuba, pabeni, vadim.fedorenko,
u.kleine-koenig, netdev, linux-kernel, yaojun
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260814111317.1741087-2-dong100@mucse.com?part=4
---
--- Patch [4]: [PATCH net-next v9 4/4] net: rnpgbe: Add link status handling support ---
commit b6af113d772d4dd7e20d76a3f509b4e2063a20db
Author: Dong Yibo <dong100@mucse.com>
net: rnpgbe: Add link status handling support
This adds firmware-driven link status handling to the rnpgbe driver: link
state fields in struct mucse_hw, LINK_CHANGE_EVT parsing over the mailbox,
and a 500 ms delayed service task that flips the carrier. It also notifies
firmware of port up/down, makes rnpgbe_up_complete() return an error so
ndo_open can fail, and enables the GMAC receiver in receive-all mode.
A few questions below.
> diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_chip.c b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_chip.c
> index 291e77d573fe6..55b4abed82bd7 100644
> --- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_chip.c
> +++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_chip.c
[ ... ]
> @@ -149,3 +155,32 @@ int rnpgbe_init_hw(struct mucse_hw *hw, int board_type)
>
> return 0;
> }
> +
> +/**
> + * rnpgbe_set_link - Set the hardware link state
> + * @hw: hw information structure
> + * @linkup: link on or not
> + *
> + * rnpgbe_set_link setup link status
> + *
> + **/
> +void rnpgbe_set_link(struct mucse_hw *hw, bool linkup)
> +{
> + u32 value = mucse_hw_rd32(hw, GMAC_CONTROL);
> +
> + if (linkup)
> + value |= GMAC_CONTROL_RE;
> + else
> + value &= ~GMAC_CONTROL_RE;
> +
> + mucse_hw_wr32(hw, GMAC_CONTROL, value);
[Severity: Low]
[Finding: main-7-40]
[Sources: opus-5]
The enable path sets GMAC_CONTROL_RE here and only programs
GMAC_FRAME_FILTER afterwards, while the disable path does the two writes in
the opposite order.
Between these two writes the receiver is running with the frame filter
value left behind by the previous link-down path:
mucse_hw_wr32(hw, GMAC_FRAME_FILTER, 0);
which is exact-address matching, and no GMAC unicast address register is
ever programmed by this driver.
Are frames arriving in that window matched against reset address
registers? Would writing the filter before enabling the receiver be
more consistent with the disable path?
> +
> + /* Keep the GMAC in receive-all mode while the link is up. The
> + * chip-level filter does the actual address filtering, but there
> + * is no ndo_set_rx_mode yet to configure it.
> + */
> + if (linkup)
> + mucse_hw_wr32(hw, GMAC_FRAME_FILTER, GMAC_RX_ALL);
> + else
> + mucse_hw_wr32(hw, GMAC_FRAME_FILTER, 0);
> +}
[Severity: Medium]
[Finding: main-1-0]
[Sources: opus-5, old-prompts, opus-4-7]
GMAC_RX_ALL is defined as BIT(31) | BIT(0), which on this DesignWare-style
GMAC is receive-all plus promiscuous. Every link-up therefore leaves the
MAC accepting every frame on the wire for the lifetime of the link, and
dev->promiscuity / IFF_PROMISC are never updated, so userspace still sees a
non-promiscuous interface.
The comment and the commit message both say the filtering happens
elsewhere:
Enable the GMAC receiver in receive-all mode because packet filtering
is done by the chip-level filter
A grep over drivers/net/ethernet/mucse/ shows the GMAC_CONTROL and
GMAC_FRAME_FILTER writes in rnpgbe_set_link() are the only MAC or filter
register writes in the whole driver, and rnpgbe_netdev_ops has no
ndo_set_rx_mode:
static const struct net_device_ops rnpgbe_netdev_ops = {
.ndo_open = rnpgbe_open,
.ndo_stop = rnpgbe_close,
.ndo_start_xmit = rnpgbe_xmit_frame,
.ndo_get_stats64 = rnpgbe_get_stats64,
};
Which code programs that chip-level filter? As it stands, foreign unicast
and all multicast frames are DMA'd into the rx rings and discarded in
software, and local taps observe other hosts' traffic while ip link
reports the interface as non-promiscuous.
> diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h
> index 1d87edfba3d75..9e640e8d67036 100644
> --- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h
> +++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h
> @@ -21,8 +21,27 @@
> #define RNPGBE_DMA_TX_STATUS GENMASK_U32(23, 22)
> #define RNPGBE_DMA_RX_STATUS GENMASK_U32(21, 20)
> #define RX_AXI_RW_EN 0x03
> +/* RNPGBE_LINK_ST is a driver-owned link-state snapshot consumed by firmware.
> + * M_DEFAULT_ST replaces the complete snapshot, causing firmware
> + * to report the current link state again when it differs from this default.
> + */
> +#define M_ST_MASK (GENMASK_U32(31, 24) | \
> + GENMASK_U32(11, 8) | BIT(6) | \
> + BIT(4) | BIT(0))
> +/* Set the driver-state marker; all other driver status fields start clear. */
> +#define M_DEFAULT_ST 0xa0000000
[Severity: Medium]
[Finding: main-1-1]
[Sources: opus-5, old-prompts]
Who owns the bits of RNPGBE_LINK_ST outside M_ST_MASK, that is bits 23:12,
7, 5 and 3:1?
mucse_update_link_status_reg() treats them as state that must be preserved:
value = mucse_hw_rd32(hw, RNPGBE_LINK_ST);
value &= ~M_ST_MASK;
value |= M_DEFAULT_ST;
while rnpgbe_up_complete() and the invalid-magic path in
mucse_mbx_fw_req_handler() blind-write the whole word:
mucse_hw_wr32(hw, RNPGBE_LINK_ST, M_DEFAULT_ST);
so exactly the bits the read-modify-write takes care to keep are zeroed.
Note the register sits at 0x000c, between RNPGBE_DMA_STATUS and
RNPGBE_DMA_AXI_EN.
If the whole word is driver-owned, is the masked read-modify-write reading
anything meaningful? If it is not, do the two full-word writes destroy
state the driver cannot restore?
> +/* Driver-reported fields: 25:24 pause, 11:8 speed, 6 LLDP, 4 duplex,
> + * and 0 link up/down. M_DEFAULT_ST resets these fields so firmware reports
> + * the current hardware state again.
> + */
> +#define RNPGBE_LINK_ST 0x000c
> #define RNPGBE_DMA_AXI_EN 0x0010
> #define RNPGBE_TX_MIN_PKT_LEN 33
>
> +#define MUCSE_GMAC_OFF(_n) (0x20000 + (_n))
> +#define GMAC_CONTROL_RE 0x00000004
> +#define GMAC_CONTROL MUCSE_GMAC_OFF(0)
> +#define GMAC_RX_ALL (BIT(31) | BIT(0))
> +#define GMAC_FRAME_FILTER MUCSE_GMAC_OFF(0x4)
> #define RNPGBE_MAX_QUEUES 8
> #endif /* _RNPGBE_HW_H */
> diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_lib.c b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_lib.c
> index 9199a48e75d42..fa5ea4dd7a1c6 100644
> --- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_lib.c
> +++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_lib.c
[ ... ]
> @@ -1414,11 +1447,50 @@ void rnpgbe_up_complete(struct mucse *mucse)
> rnpgbe_configure_msi(mucse);
>
> rnpgbe_napi_enable_all(mucse);
> + /* Clear stale link state and tell firmware the driver hasn't
> + * seen any link yet. Firmware will re-assert LINK_CHANGE_EVT
> + * when link_report_en is sent below since RNPGBE_LINK_ST
> + * no longer matches the actual (possibly up) link state.
> + */
> + spin_lock_irqsave(&mucse->link_lock, flags);
> + WRITE_ONCE(hw->link, false);
> + WRITE_ONCE(hw->speed, 0);
> + WRITE_ONCE(hw->duplex, 0);
> + atomic_set(&mucse->link_pending, 0);
> + /* echo fw driver now in default state */
> + mucse_hw_wr32(hw, RNPGBE_LINK_ST, M_DEFAULT_ST);
[ ... ]
> + queue_delayed_work(system_percpu_wq, &mucse->serv_task,
> + msecs_to_jiffies(500));
> +
> + return 0;
> }
[ ... ]
> +/**
> + * rnpgbe_service_task - Manages and runs subtasks
> + * @work: pointer to work_struct containing our data
> + **/
> +void rnpgbe_service_task(struct work_struct *work)
> +{
> + struct mucse *mucse = container_of(work, struct mucse, serv_task.work);
> +
> + if (test_bit(__MUCSE_DOWN, &mucse->state))
> + return;
> +
> + rnpgbe_process_link_subtask(mucse);
> +
> + if (!test_bit(__MUCSE_DOWN, &mucse->state))
> + queue_delayed_work(system_percpu_wq, &mucse->serv_task,
> + msecs_to_jiffies(500));
> +}
[Severity: Low]
[Finding: main-1-4]
[Sources: opus-5, old-prompts]
Could the carrier update be done directly from the mailbox work item
instead of being polled?
The link event is already parsed in sleepable process context:
rnpgbe_msix_other()
queue_work(system_percpu_wq, &mucse->mbx_work)
rnpgbe_mbx_work()
mucse_fw_irq_handler()
mucse_mbx_fw_req_handler()
atomic_set_release(&mucse->link_pending, 1);
but the handler only sets link_pending, and this delayed work polls the
flag every 500 ms. That adds up to 500 ms of carrier-update latency per
event, including the first link report since rnpgbe_up_complete() arms the
task with msecs_to_jiffies(500), plus two workqueue wakeups per second for
as long as the interface is up.
Queueing serv_task with zero delay from mucse_mbx_fw_req_handler() would
avoid both.
> diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx_fw.c b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx_fw.c
> index 754016f6b4f00..441724f8603eb 100644
> --- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx_fw.c
> +++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx_fw.c
> @@ -3,6 +3,7 @@
>
> #include <linux/if_ether.h>
> #include <linux/bitfield.h>
> +#include <linux/ethtool.h>
>
> #include "rnpgbe.h"
> #include "rnpgbe_mbx.h"
> @@ -27,10 +28,14 @@ static int mucse_fw_send_cmd_wait_resp(struct mucse_hw *hw,
> int retry_cnt = 3;
> int err;
>
> + BUILD_BUG_ON(sizeof(struct mbx_fw_cmd_reply) != 56);
> +
> mutex_lock(&hw->mbx.lock);
> +
> err = mucse_write_and_wait_ack_mbx(hw, req->dwords, len);
> if (err)
> goto out;
> +
[Severity: Low]
[Finding: main-1-3]
[Sources: opus-5, old-prompts]
These changes are not described by the commit message and are unrelated to
link handling: the BUILD_BUG_ON plus blank-line churn inside the
pre-existing mucse_fw_send_cmd_wait_resp(), and a whitespace-only hunk in
rnpgbe_add_adapter().
The new #include <linux/ethtool.h> also has no user in this file: grep
finds no ethtool symbol here, DUPLEX_BIT is driver-local, and the speeds
are plain literals.
The new FLAGS_REPLY and FLAGS_ERR macros and the reply arm of
union mbx_fw_cmd_u have no consumer either; mucse_fw_handle_event()
demultiplexes on the opcode field alone and never checks the
reply/error discriminator. Could these be dropped or moved to the patch
that uses them?
> do {
> err = mucse_poll_and_read_mbx(hw, reply->dwords,
> sizeof(reply->r));
[ ... ]
> +static void mucse_update_link_status_reg(struct mucse_hw *hw,
> + struct mbx_fw_cmd_req *req)
> +{
> + u16 status = le16_to_cpu(req->link_stat.st.status);
> + u16 speed = le16_to_cpu(req->link_stat.st.speed);
> + u32 value;
> +
> + value = mucse_hw_rd32(hw, RNPGBE_LINK_ST);
> + value &= ~M_ST_MASK;
> + value |= M_DEFAULT_ST;
> +
> + if (le16_to_cpu(req->link_stat.port_status)) {
> + value |= BIT(0);
> + switch (speed) {
> + case 10:
> + value |= (mucse_speed_10 << 8);
> + break;
> + case 100:
> + value |= (mucse_speed_100 << 8);
> + break;
> + case 1000:
> + value |= (mucse_speed_1000 << 8);
> + break;
> + default:
> + break;
> + }
[Severity: Medium]
[Finding: gpt-5-6-sol-1-6]
[Sources: gpt-5-6-sol, opus-5, old-prompts]
What happens for a speed that is not 10, 100 or 1000?
The default arm leaves bits 11:8 at 0, and mucse_speed_10 is also 0:
enum mucse_speed {
mucse_speed_10 = 0,
mucse_speed_100 = 1,
mucse_speed_1000 = 2,
};
so an unsupported speed is encoded as 10 Mbps in the snapshot, while
mucse_mbx_fw_req_handler() caches the raw value:
WRITE_ONCE(hw->speed, le16_to_cpu(req->link_stat.st.speed));
and rnpgbe_link_is_up() prints it and enables carrier plus the GMAC
receiver.
The commit message says:
Validate firmware link events before updating the cached link state.
but the only check applied is port_magic against ST_VALID_MAGIC; speed is
not range checked. Should an out-of-range speed be rejected instead, given
that the snapshot then reports a state the hardware never has and firmware
can keep re-asserting LINK_CHANGE_EVT?
> +
> + value |= FIELD_PREP(BIT(4),
> + !!(req->link_stat.st.flags & DUPLEX_BIT));
> + value |= FIELD_PREP(GENMASK_U32(25, 24),
> + status & GENMASK(1, 0));
> + } else {
> + value &= ~BIT(0);
> + }
> +
> + if (status & ST_STATUS_LLDP_STATUS_MASK)
> + value |= BIT(6);
> + else
> + value &= ~BIT(6);
> +
> + mucse_hw_wr32(hw, RNPGBE_LINK_ST, value);
> +}
[ ... ]
> +static void mucse_mbx_fw_req_handler(struct mucse_hw *hw,
> + struct mbx_fw_cmd_req *req)
> +{
> + struct mucse *mucse = container_of(hw, struct mucse, hw);
> + u32 magic = le32_to_cpu(req->link_stat.port_magic);
> + unsigned long flags;
> +
> + if (le16_to_cpu(req->opcode) == LINK_CHANGE_EVT) {
> + spin_lock_irqsave(&mucse->link_lock, flags);
> + if (magic != ST_VALID_MAGIC) {
> + /* Do not let an invalid event change the cached link state.
> + * Reset the driver snapshot so firmware reports it again.
> + */
> + mucse_hw_wr32(hw, RNPGBE_LINK_ST, M_DEFAULT_ST);
> + spin_unlock_irqrestore(&mucse->link_lock, flags);
> + return;
> + }
[Severity: Medium]
[Finding: gpt-5-6-sol-3-22]
[Sources: gpt-5-6-sol]
Can this path leave the carrier reported as up after the link is gone?
If an invalid-magic event arrives while the link is going down, the cached
state is kept and the snapshot is reset to M_DEFAULT_ST. But a legitimate
link-down snapshot with LLDP clear resolves to exactly M_DEFAULT_ST too:
mucse_update_link_status_reg() clears BIT(0), encodes no speed, duplex or
pause, and clears bit 6.
After the reset the snapshot already matches the real down state, so per
the handshake documented in this patch firmware has no mismatch left to
re-report from. link_pending is never set, rnpgbe_process_link_subtask()
never calls rnpgbe_link_is_down(), and the interface keeps carrier on with
GMAC_CONTROL_RE set until it is cycled by hand.
> +
> + if (test_bit(__MUCSE_DOWN, &mucse->state)) {
> + mucse_update_link_status_reg(hw, req);
> + spin_unlock_irqrestore(&mucse->link_lock, flags);
> + return;
> + }
> +
> + if (le16_to_cpu(req->link_stat.port_status))
> + WRITE_ONCE(hw->link, true);
> + else
> + WRITE_ONCE(hw->link, false);
[Severity: Medium]
[Finding: main-7-37]
[Sources: opus-5, old-prompts]
Should port_status be tested with BIT(hw->port) rather than for plain
non-zero?
Every other port field in this mailbox protocol is a per-port bitmask. The
requests added by this patch encode:
.port_mask = cpu_to_le32(BIT(hw->port)),
and the GET_MAC_ADDRESS reply is decoded in mucse_mbx_get_macaddr() as:
if (le32_to_cpu(reply.r.mac_addr.ports) & BIT(port))
If port_status is likewise a bitmap of ports whose link is up, which its
pairing with changed_lanes suggests, then on multi-port parts such as
RNPGBE_DEVICE_ID_N500_QUAD_PORT a link-up on another port would make this
port declare carrier up, set GMAC_CONTROL_RE plus receive-all filtering,
and advertise link up in the firmware snapshot while its own PHY is down.
The same unmasked test is used in mucse_update_link_status_reg().
> +
> + WRITE_ONCE(hw->speed, le16_to_cpu(req->link_stat.st.speed));
> + WRITE_ONCE(hw->duplex, req->link_stat.st.flags & DUPLEX_BIT);
> + /* update regs to notify link info is received */
> + mucse_update_link_status_reg(hw, req);
> + atomic_set_release(&mucse->link_pending, 1);
> + spin_unlock_irqrestore(&mucse->link_lock, flags);
> + }
> +}
> +
> +/**
> + * mucse_fw_handle_event - Handle one pending firmware event
> + * @hw: pointer to the hardware structure
> + *
> + * Return: true if an event was handled, false otherwise
> + **/
> +static bool mucse_fw_handle_event(struct mucse_hw *hw)
> +{
> + union mbx_fw_cmd_u msg = {};
> + int err;
> +
> + /* try to check and read fw req */
> + mutex_lock(&hw->mbx.lock);
> + err = mucse_check_and_read_mbx(hw, msg.dwords, sizeof(msg));
> + mutex_unlock(&hw->mbx.lock);
> + if (err)
> + return false;
> +
> + mucse_mbx_fw_req_handler(hw, &msg.req);
> +
> + return true;
> +}
> +
> +/**
> + * mucse_fw_irq_handler - Drain pending firmware mailbox events
> * @hw: pointer to the HW structure
> + *
> + * The only asynchronous event currently handled is LINK_CHANGE_EVT.
> + * Firmware rate-limits link-change notifications to a minimum interval
> + * of 500 ms.
> **/
> void mucse_fw_irq_handler(struct mucse_hw *hw)
> {
> + /* Drain events coalesced while the mailbox work item was pending. */
> + while (mucse_fw_handle_event(hw))
> + continue;
> }
[Severity: Medium]
[Finding: gpt-5-6-sol-6-34]
[Sources: gpt-5-6-sol, opus-5, old-prompts]
Is this loop bounded by anything other than firmware behaviour?
The exit condition is entirely device controlled. mucse_check_for_msg_pf()
succeeds whenever the firmware-written counter differs from the cached
copy:
if (fw_req == 0 || fw_req == hw->mbx.fw_req)
return -EIO;
and mucse_read_mbx_pf() re-caches it from the same register, so as long as
firmware keeps advancing the counter the work item never returns.
mucse_fw_handle_event() also returns true for any successful read,
including messages with an unrecognised opcode.
That blocks the teardown path, which waits for this work:
rnpgbe_free_mbx_irq()
free_irq(pci_irq_vector(pdev, 0), mucse);
cancel_work_sync(&mucse->mbx_work);
so a device that keeps posting messages would hang rmmod or PCI remove and
keep one workqueue worker spinning on the mailbox lock. The reply path
bounds itself with retry_cnt = 3; should this drain get a similar budget?
The 500 ms firmware rate limit is only stated in the comment, not enforced
by the driver.
Simon says: I believe that the issue below is a false-positive as
the use of __MUCSE_DOWN ensures correct behaviour.
[Severity: Low]
[Finding: gpt-5-6-sol-5-30]
[Sources: gpt-5-6-sol, opus-5]
Now that mucse_fw_irq_handler() is no longer empty, can it run against an
already disabled device on the shutdown path?
rnpgbe_dev_shutdown() quiesces only the netdev and then disables the
device:
rtnl_lock();
netif_device_detach(netdev);
if (netif_running(netdev))
rnpgbe_close(netdev);
rtnl_unlock();
pci_disable_device(pdev);
The mailbox IRQ stays registered and mbx_work is never cancelled there;
rnpgbe_down() only cancels serv_task. A queued or running mbx_work would
then do mailbox MMIO reads and the RNPGBE_LINK_ST write after
pci_disable_device(), and feed whatever the reads return into
mucse_mbx_fw_req_handler(). rnpgbe_free_mbx_irq() already has the
free_irq() plus cancel_work_sync() ordering the shutdown path is missing.
> diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx_fw.h b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx_fw.h
> index a6bf5de55aa39..0e6972b7c3c02 100644
> --- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx_fw.h
> +++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx_fw.h
[ ... ]
> struct mbx_fw_cmd_reply {
> +#define FLAGS_REPLY BIT(0)
> +#define FLAGS_ERR BIT(2)
> __le16 flags;
> __le16 opcode;
> __le16 error_code;
> @@ -94,10 +127,27 @@ union mbx_fw_cmd_reply_u {
> __le32 dwords[sizeof(struct mbx_fw_cmd_reply) / sizeof(__le32)];
> };
>
> +union mbx_fw_cmd_u {
> + struct mbx_fw_cmd_req req;
> + struct mbx_fw_cmd_reply reply;
> + __le32 dwords[sizeof(struct mbx_fw_cmd_reply) / sizeof(__le32)];
> +};
> +
> +/* Firmware encoding for RNPGBE_LINK_ST[11:8]. Valid link events report
> + * only these three speeds.
> + */
> +enum mucse_speed {
> + mucse_speed_10 = 0,
> + mucse_speed_100 = 1,
> + mucse_speed_1000 = 2,
> +};
[ ... ]
^ permalink raw reply [flat|nested] 9+ messages in thread