[PATCH v3 0/5] Add driver for 1Gbe network chips from MUCSE

[PATCH v3 0/5] Add driver for 1Gbe network chips from MUCSE

[Dong Yibo]

Hi maintainers,

This patch series is v3 to introduce support for MUCSE N500/N210 1Gbps
Ethernet controllers. I divide codes into multiple series, this is the
first one which only register netdev without true tx/rx functions.

The driver has been tested on the following platform:
   - Kernel version: 6.16.0
   - Intel Xeon Processor

Changelog:
v2 -> v3:
  [patch 1/5]:
  1. Fix rnpgbe_driver_name as static.
  2. Fix sorted-list for index.rst Kconfig Makefile.
  [patch 2/5]
  1. Using common funcs to initialize common parts across multiple chips.
  [patch 3/5]
  1. Fix no initialised error.
  [patch 4/5]:
  1. Fix invalid bitfield specifier for type restricted __le32
  
  Some common modifications:
  1. Remove extra parentheses for constant.
  2. Check code-spell error and comment.
  3. Remove no used code and define.
  4. Use const for all ops.
  5. Modify the code based on Brett Creeley, andrew, horms's feedbacks.

links:
v2: https://lore.kernel.org/netdev/20250721113238.18615-1-dong100@mucse.com/
v1: https://lore.kernel.org/netdev/20250703014859.210110-1-dong100@mucse.com/

Dong Yibo (5):
  net: rnpgbe: Add build support for rnpgbe
  net: rnpgbe: Add n500/n210 chip support
  net: rnpgbe: Add basic mbx ops support
  net: rnpgbe: Add basic mbx_fw support
  net: rnpgbe: Add register_netdev

 .../device_drivers/ethernet/index.rst         |   1 +
 .../device_drivers/ethernet/mucse/rnpgbe.rst  |  21 +
 MAINTAINERS                                   |   8 +
 drivers/net/ethernet/Kconfig                  |   1 +
 drivers/net/ethernet/Makefile                 |   1 +
 drivers/net/ethernet/mucse/Kconfig            |  34 ++
 drivers/net/ethernet/mucse/Makefile           |   7 +
 drivers/net/ethernet/mucse/rnpgbe/Makefile    |  11 +
 drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h    | 148 ++++++
 .../net/ethernet/mucse/rnpgbe/rnpgbe_chip.c   | 166 +++++++
 drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h |  15 +
 .../net/ethernet/mucse/rnpgbe/rnpgbe_main.c   | 352 ++++++++++++++
 .../net/ethernet/mucse/rnpgbe/rnpgbe_mbx.c    | 443 ++++++++++++++++++
 .../net/ethernet/mucse/rnpgbe/rnpgbe_mbx.h    |  31 ++
 .../net/ethernet/mucse/rnpgbe/rnpgbe_mbx_fw.c | 275 +++++++++++
 .../net/ethernet/mucse/rnpgbe/rnpgbe_mbx_fw.h | 201 ++++++++
 16 files changed, 1715 insertions(+)
 create mode 100644 Documentation/networking/device_drivers/ethernet/mucse/rnpgbe.rst
 create mode 100644 drivers/net/ethernet/mucse/Kconfig
 create mode 100644 drivers/net/ethernet/mucse/Makefile
 create mode 100644 drivers/net/ethernet/mucse/rnpgbe/Makefile
 create mode 100644 drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h
 create mode 100644 drivers/net/ethernet/mucse/rnpgbe/rnpgbe_chip.c
 create mode 100644 drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h
 create mode 100644 drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c
 create mode 100644 drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx.c
 create mode 100644 drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx.h
 create mode 100644 drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx_fw.c
 create mode 100644 drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx_fw.h

-- 
2.25.1

[PATCH v3 1/5] net: rnpgbe: Add build support for rnpgbe

[Dong Yibo]

Add build options and doc for mucse.
Initialize pci device access for MUCSE devices.

Signed-off-by: Dong Yibo <dong100@mucse.com>
---
 .../device_drivers/ethernet/index.rst         |   1 +
 .../device_drivers/ethernet/mucse/rnpgbe.rst  |  21 +++
 MAINTAINERS                                   |   8 +
 drivers/net/ethernet/Kconfig                  |   1 +
 drivers/net/ethernet/Makefile                 |   1 +
 drivers/net/ethernet/mucse/Kconfig            |  34 ++++
 drivers/net/ethernet/mucse/Makefile           |   7 +
 drivers/net/ethernet/mucse/rnpgbe/Makefile    |   8 +
 drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h    |  25 +++
 .../net/ethernet/mucse/rnpgbe/rnpgbe_main.c   | 161 ++++++++++++++++++
 10 files changed, 267 insertions(+)
 create mode 100644 Documentation/networking/device_drivers/ethernet/mucse/rnpgbe.rst
 create mode 100644 drivers/net/ethernet/mucse/Kconfig
 create mode 100644 drivers/net/ethernet/mucse/Makefile
 create mode 100644 drivers/net/ethernet/mucse/rnpgbe/Makefile
 create mode 100644 drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h
 create mode 100644 drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c

diff --git a/Documentation/networking/device_drivers/ethernet/index.rst b/Documentation/networking/device_drivers/ethernet/index.rst
index 40ac552641a3..c8abadbe15ee 100644
--- a/Documentation/networking/device_drivers/ethernet/index.rst
+++ b/Documentation/networking/device_drivers/ethernet/index.rst
@@ -47,6 +47,7 @@ Contents:
    mellanox/mlx5/index
    meta/fbnic
    microsoft/netvsc
+   mucse/rnpgbe
    neterion/s2io
    netronome/nfp
    pensando/ionic
diff --git a/Documentation/networking/device_drivers/ethernet/mucse/rnpgbe.rst b/Documentation/networking/device_drivers/ethernet/mucse/rnpgbe.rst
new file mode 100644
index 000000000000..7562fb6b8f61
--- /dev/null
+++ b/Documentation/networking/device_drivers/ethernet/mucse/rnpgbe.rst
@@ -0,0 +1,21 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+===========================================================
+Linux Base Driver for MUCSE(R) Gigabit PCI Express Adapters
+===========================================================
+
+MUCSE Gigabit Linux driver.
+Copyright (c) 2020 - 2025 MUCSE Co.,Ltd.
+
+Identifying Your Adapter
+========================
+The driver is compatible with devices based on the following:
+
+ * MUCSE(R) Ethernet Controller N500 series
+ * MUCSE(R) Ethernet Controller N210 series
+
+Support
+=======
+ If you have problems with the software or hardware, please contact our
+ customer support team via email at techsupport@mucse.com or check our
+ website at https://www.mucse.com/en/
diff --git a/MAINTAINERS b/MAINTAINERS
index bd62ad58a47f..80c1ae2ae26c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -17278,6 +17278,14 @@ T:	git git://linuxtv.org/media.git
 F:	Documentation/devicetree/bindings/media/i2c/aptina,mt9v111.yaml
 F:	drivers/media/i2c/mt9v111.c
 
+MUCSE ETHERNET DRIVER
+M:	Yibo Dong <dong100@mucse.com>
+L:	netdev@vger.kernel.org
+S:	Maintained
+W:	https://www.mucse.com/en/
+F:	Documentation/networking/device_drivers/ethernet/mucse/*
+F:	drivers/net/ethernet/mucse/*
+
 MULTIFUNCTION DEVICES (MFD)
 M:	Lee Jones <lee@kernel.org>
 S:	Maintained
diff --git a/drivers/net/ethernet/Kconfig b/drivers/net/ethernet/Kconfig
index f86d4557d8d7..167388f9c744 100644
--- a/drivers/net/ethernet/Kconfig
+++ b/drivers/net/ethernet/Kconfig
@@ -129,6 +129,7 @@ source "drivers/net/ethernet/microchip/Kconfig"
 source "drivers/net/ethernet/mscc/Kconfig"
 source "drivers/net/ethernet/microsoft/Kconfig"
 source "drivers/net/ethernet/moxa/Kconfig"
+source "drivers/net/ethernet/mucse/Kconfig"
 source "drivers/net/ethernet/myricom/Kconfig"
 
 config FEALNX
diff --git a/drivers/net/ethernet/Makefile b/drivers/net/ethernet/Makefile
index 67182339469a..1b8c4df3f594 100644
--- a/drivers/net/ethernet/Makefile
+++ b/drivers/net/ethernet/Makefile
@@ -65,6 +65,7 @@ obj-$(CONFIG_NET_VENDOR_MICREL) += micrel/
 obj-$(CONFIG_NET_VENDOR_MICROCHIP) += microchip/
 obj-$(CONFIG_NET_VENDOR_MICROSEMI) += mscc/
 obj-$(CONFIG_NET_VENDOR_MOXART) += moxa/
+obj-$(CONFIG_NET_VENDOR_MUCSE) += mucse/
 obj-$(CONFIG_NET_VENDOR_MYRI) += myricom/
 obj-$(CONFIG_FEALNX) += fealnx.o
 obj-$(CONFIG_NET_VENDOR_NATSEMI) += natsemi/
diff --git a/drivers/net/ethernet/mucse/Kconfig b/drivers/net/ethernet/mucse/Kconfig
new file mode 100644
index 000000000000..be0fdf268484
--- /dev/null
+++ b/drivers/net/ethernet/mucse/Kconfig
@@ -0,0 +1,34 @@
+# SPDX-License-Identifier: GPL-2.0-only
+#
+# Mucse network device configuration
+#
+
+config NET_VENDOR_MUCSE
+	bool "Mucse devices"
+	default y
+	help
+	  If you have a network (Ethernet) card from Mucse(R), say Y.
+
+	  Note that the answer to this question doesn't directly affect the
+	  kernel: saying N will just cause the configurator to skip all
+	  the questions about Mucse(R) cards. If you say Y, you will
+	  be asked for your specific card in the following questions.
+
+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.
+
+	  More specific information on configuring the driver is in
+	  <file:Documentation/networking/device_drivers/ethernet/mucse/rnpgbe.rst>.
+
+	  To compile this driver as a module, choose M here. The module
+	  will be called rnpgbe.
+
+endif # NET_VENDOR_MUCSE
+
diff --git a/drivers/net/ethernet/mucse/Makefile b/drivers/net/ethernet/mucse/Makefile
new file mode 100644
index 000000000000..835771a5c374
--- /dev/null
+++ b/drivers/net/ethernet/mucse/Makefile
@@ -0,0 +1,7 @@
+# SPDX-License-Identifier: GPL-2.0
+# Copyright(c) 2020 - 2025 MUCSE Corporation.
+#
+# Makefile for the MUCSE(R) 1GbE PCI Express ethernet driver
+#
+
+obj-$(CONFIG_MGBE) += rnpgbe/
diff --git a/drivers/net/ethernet/mucse/rnpgbe/Makefile b/drivers/net/ethernet/mucse/rnpgbe/Makefile
new file mode 100644
index 000000000000..9df536f0d04c
--- /dev/null
+++ b/drivers/net/ethernet/mucse/rnpgbe/Makefile
@@ -0,0 +1,8 @@
+# SPDX-License-Identifier: GPL-2.0
+# Copyright(c) 2020 - 2025 MUCSE Corporation.
+#
+# Makefile for the MUCSE(R) 1GbE PCI Express ethernet driver
+#
+
+obj-$(CONFIG_MGBE) += rnpgbe.o
+rnpgbe-objs := rnpgbe_main.o
diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h
new file mode 100644
index 000000000000..23c84454e7c7
--- /dev/null
+++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h
@@ -0,0 +1,25 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Copyright(c) 2020 - 2025 Mucse Corporation. */
+
+#ifndef _RNPGBE_H
+#define _RNPGBE_H
+
+enum rnpgbe_boards {
+	board_n500,
+	board_n210,
+	board_n210L,
+};
+
+struct mucse {
+	struct net_device *netdev;
+	struct pci_dev *pdev;
+	u16 bd_number;
+};
+
+/* Device IDs */
+#define PCI_VENDOR_ID_MUCSE 0x8848
+#define PCI_DEVICE_ID_N500_QUAD_PORT 0x8308
+#define PCI_DEVICE_ID_N500_DUAL_PORT 0x8318
+#define PCI_DEVICE_ID_N210 0x8208
+#define PCI_DEVICE_ID_N210L 0x820a
+#endif /* _RNPGBE_H */
diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c
new file mode 100644
index 000000000000..c2e099bd2639
--- /dev/null
+++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c
@@ -0,0 +1,161 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright(c) 2020 - 2025 Mucse Corporation. */
+
+#include <linux/types.h>
+#include <linux/module.h>
+#include <linux/pci.h>
+
+#include "rnpgbe.h"
+
+static const char rnpgbe_driver_name[] = "rnpgbe";
+
+/* rnpgbe_pci_tbl - PCI Device ID Table
+ *
+ * { PCI_DEVICE(Vendor ID, Device ID),
+ *   driver_data (used for different hw chip) }
+ */
+static struct pci_device_id rnpgbe_pci_tbl[] = {
+	{ PCI_DEVICE(PCI_VENDOR_ID_MUCSE, PCI_DEVICE_ID_N500_QUAD_PORT),
+	  .driver_data = board_n500},
+	{ PCI_DEVICE(PCI_VENDOR_ID_MUCSE, PCI_DEVICE_ID_N500_DUAL_PORT),
+	  .driver_data = board_n500},
+	{ PCI_DEVICE(PCI_VENDOR_ID_MUCSE, PCI_DEVICE_ID_N210),
+	  .driver_data = board_n210},
+	{ PCI_DEVICE(PCI_VENDOR_ID_MUCSE, PCI_DEVICE_ID_N210L),
+	  .driver_data = board_n210L},
+	/* required last entry */
+	{0, },
+};
+
+/**
+ * rnpgbe_probe - Device initialization routine
+ * @pdev: PCI device information struct
+ * @id: entry in rnpgbe_pci_tbl
+ *
+ * rnpgbe_probe initializes a PF adapter identified by a pci_dev
+ * structure.
+ *
+ * @return: 0 on success, negative on failure
+ **/
+static int rnpgbe_probe(struct pci_dev *pdev, const struct pci_device_id *id)
+{
+	int err;
+
+	err = pci_enable_device_mem(pdev);
+	if (err)
+		return err;
+
+	err = dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(56));
+	if (err) {
+		dev_err(&pdev->dev,
+			"No usable DMA configuration, aborting %d\n", err);
+		goto err_dma;
+	}
+
+	err = pci_request_mem_regions(pdev, rnpgbe_driver_name);
+	if (err) {
+		dev_err(&pdev->dev,
+			"pci_request_selected_regions failed 0x%x\n", err);
+		goto err_pci_req;
+	}
+
+	pci_set_master(pdev);
+	pci_save_state(pdev);
+
+	return 0;
+err_dma:
+err_pci_req:
+	pci_disable_device(pdev);
+	return err;
+}
+
+/**
+ * rnpgbe_remove - Device removal routine
+ * @pdev: PCI device information struct
+ *
+ * rnpgbe_remove is called by the PCI subsystem to alert the driver
+ * that it should release a PCI device.  This could be caused by a
+ * Hot-Plug event, or because the driver is going to be removed from
+ * memory.
+ **/
+static void rnpgbe_remove(struct pci_dev *pdev)
+{
+	pci_release_mem_regions(pdev);
+	pci_disable_device(pdev);
+}
+
+/**
+ * rnpgbe_dev_shutdown - Device shutdown routine
+ * @pdev: PCI device information struct
+ * @enable_wake: wakeup status
+ **/
+static void rnpgbe_dev_shutdown(struct pci_dev *pdev,
+				bool *enable_wake)
+{
+	*enable_wake = false;
+	pci_disable_device(pdev);
+}
+
+/**
+ * rnpgbe_shutdown - Device shutdown routine
+ * @pdev: PCI device information struct
+ *
+ * rnpgbe_shutdown is called by the PCI subsystem to alert the driver
+ * that os shutdown. Device should setup wakeup state here.
+ **/
+static void rnpgbe_shutdown(struct pci_dev *pdev)
+{
+	bool wake;
+
+	rnpgbe_dev_shutdown(pdev, &wake);
+
+	if (system_state == SYSTEM_POWER_OFF) {
+		pci_wake_from_d3(pdev, wake);
+		pci_set_power_state(pdev, PCI_D3hot);
+	}
+}
+
+static struct pci_driver rnpgbe_driver = {
+	.name = rnpgbe_driver_name,
+	.id_table = rnpgbe_pci_tbl,
+	.probe = rnpgbe_probe,
+	.remove = rnpgbe_remove,
+	.shutdown = rnpgbe_shutdown,
+};
+
+/**
+ * rnpgbe_init_module - Driver init routine
+ *
+ * rnpgbe_init_module is called when driver insmod
+ *
+ * @return: 0 on success, negative on failure
+ **/
+static int __init rnpgbe_init_module(void)
+{
+	int ret;
+
+	ret = pci_register_driver(&rnpgbe_driver);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+module_init(rnpgbe_init_module);
+
+/**
+ * rnpgbe_exit_module - Driver remove routine
+ *
+ * rnpgbe_exit_module is called when driver is removed
+ **/
+static void __exit rnpgbe_exit_module(void)
+{
+	pci_unregister_driver(&rnpgbe_driver);
+}
+
+module_exit(rnpgbe_exit_module);
+
+MODULE_DEVICE_TABLE(pci, rnpgbe_pci_tbl);
+MODULE_AUTHOR("Mucse Corporation, <techsupport@mucse.com>");
+MODULE_DESCRIPTION("Mucse(R) 1 Gigabit PCI Express Network Driver");
+MODULE_LICENSE("GPL");
-- 
2.25.1

[PATCH v3 2/5] net: rnpgbe: Add n500/n210 chip support

[Dong Yibo]

Initialize n500/n210 chip bar resource map and
dma, eth, mbx ... info for future use.

Signed-off-by: Dong Yibo <dong100@mucse.com>
---
 drivers/net/ethernet/mucse/rnpgbe/Makefile    |   3 +-
 drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h    |  60 +++++++++
 .../net/ethernet/mucse/rnpgbe/rnpgbe_chip.c   |  88 ++++++++++++++
 drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h |  12 ++
 .../net/ethernet/mucse/rnpgbe/rnpgbe_main.c   | 115 ++++++++++++++++++
 5 files changed, 277 insertions(+), 1 deletion(-)
 create mode 100644 drivers/net/ethernet/mucse/rnpgbe/rnpgbe_chip.c
 create mode 100644 drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h

diff --git a/drivers/net/ethernet/mucse/rnpgbe/Makefile b/drivers/net/ethernet/mucse/rnpgbe/Makefile
index 9df536f0d04c..42c359f459d9 100644
--- a/drivers/net/ethernet/mucse/rnpgbe/Makefile
+++ b/drivers/net/ethernet/mucse/rnpgbe/Makefile
@@ -5,4 +5,5 @@
 #
 
 obj-$(CONFIG_MGBE) += rnpgbe.o
-rnpgbe-objs := rnpgbe_main.o
+rnpgbe-objs := rnpgbe_main.o\
+	       rnpgbe_chip.o
diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h
index 23c84454e7c7..0dd3d3cb2a4d 100644
--- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h
+++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h
@@ -4,18 +4,78 @@
 #ifndef _RNPGBE_H
 #define _RNPGBE_H
 
+#include <linux/types.h>
+
+extern const struct rnpgbe_info rnpgbe_n500_info;
+extern const struct rnpgbe_info rnpgbe_n210_info;
+extern const struct rnpgbe_info rnpgbe_n210L_info;
+
 enum rnpgbe_boards {
 	board_n500,
 	board_n210,
 	board_n210L,
 };
 
+enum rnpgbe_hw_type {
+	rnpgbe_hw_n500 = 0,
+	rnpgbe_hw_n210,
+	rnpgbe_hw_n210L,
+	rnpgbe_hw_unknow
+};
+
+struct mucse_dma_info {
+	void __iomem *dma_base_addr;
+	void __iomem *dma_ring_addr;
+	void *back;
+	u32 dma_version;
+};
+
+struct mucse_eth_info {
+	void __iomem *eth_base_addr;
+	void *back;
+};
+
+struct mucse_mac_info {
+	void __iomem *mac_addr;
+	void *back;
+};
+
+struct mucse_mbx_info {
+	/* fw <--> pf mbx */
+	u32 fw_pf_shm_base;
+	u32 pf2fw_mbox_ctrl;
+	u32 pf2fw_mbox_mask;
+	u32 fw_pf_mbox_mask;
+	u32 fw2pf_mbox_vec;
+};
+
+struct mucse_hw {
+	void *back;
+	void __iomem *hw_addr;
+	void __iomem *ring_msix_base;
+	struct pci_dev *pdev;
+	enum rnpgbe_hw_type hw_type;
+	struct mucse_dma_info dma;
+	struct mucse_eth_info eth;
+	struct mucse_mac_info mac;
+	struct mucse_mbx_info mbx;
+	u32 driver_version;
+	u16 usecstocount;
+};
+
 struct mucse {
 	struct net_device *netdev;
 	struct pci_dev *pdev;
+	struct mucse_hw hw;
 	u16 bd_number;
 };
 
+struct rnpgbe_info {
+	int total_queue_pair_cnts;
+	enum rnpgbe_hw_type hw_type;
+	void (*init)(struct mucse_hw *hw);
+};
+
 /* Device IDs */
 #define PCI_VENDOR_ID_MUCSE 0x8848
 #define PCI_DEVICE_ID_N500_QUAD_PORT 0x8308
diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_chip.c b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_chip.c
new file mode 100644
index 000000000000..20ec67c9391e
--- /dev/null
+++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_chip.c
@@ -0,0 +1,88 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright(c) 2020 - 2025 Mucse Corporation. */
+
+#include "rnpgbe.h"
+#include "rnpgbe_hw.h"
+
+/**
+ * rnpgbe_init_common - Setup common attribute
+ * @hw: hw information structure
+ **/
+static void rnpgbe_init_common(struct mucse_hw *hw)
+{
+	struct mucse_dma_info *dma = &hw->dma;
+	struct mucse_eth_info *eth = &hw->eth;
+	struct mucse_mac_info *mac = &hw->mac;
+
+	dma->dma_base_addr = hw->hw_addr;
+	dma->dma_ring_addr = hw->hw_addr + RNPGBE_RING_BASE;
+	dma->back = hw;
+
+	eth->eth_base_addr = hw->hw_addr + RNPGBE_ETH_BASE;
+	eth->back = hw;
+
+	mac->mac_addr = hw->hw_addr + RNPGBE_MAC_BASE;
+	mac->back = hw;
+}
+
+/**
+ * rnpgbe_init_n500 - Setup n500 hw info
+ * @hw: hw information structure
+ *
+ * rnpgbe_init_n500 initializes all private
+ * structure, such as dma, eth, mac and mbx base on
+ * hw->addr for n500
+ **/
+static void rnpgbe_init_n500(struct mucse_hw *hw)
+{
+	struct mucse_mbx_info *mbx = &hw->mbx;
+
+	rnpgbe_init_common(hw);
+
+	mbx->fw2pf_mbox_vec = 0x28b00;
+	mbx->fw_pf_shm_base = 0x2d000;
+	mbx->pf2fw_mbox_ctrl = 0x2e000;
+	mbx->fw_pf_mbox_mask = 0x2e200;
+	hw->ring_msix_base = hw->hw_addr + 0x28700;
+	hw->usecstocount = 125;
+}
+
+/**
+ * rnpgbe_init_n210 - Setup n210 hw info
+ * @hw: hw information structure
+ *
+ * rnpgbe_init_n210 initializes all private
+ * structure, such as dma, eth, mac and mbx base on
+ * hw->addr for n210
+ **/
+static void rnpgbe_init_n210(struct mucse_hw *hw)
+{
+	struct mucse_mbx_info *mbx = &hw->mbx;
+
+	rnpgbe_init_common(hw);
+
+	mbx->fw2pf_mbox_vec = 0x29400;
+	mbx->fw_pf_shm_base = 0x2d900;
+	mbx->pf2fw_mbox_ctrl = 0x2e900;
+	mbx->fw_pf_mbox_mask = 0x2eb00;
+	hw->ring_msix_base = hw->hw_addr + 0x29000;
+	hw->usecstocount = 62;
+}
+
+const struct rnpgbe_info rnpgbe_n500_info = {
+	.total_queue_pair_cnts = RNPGBE_MAX_QUEUES,
+	.hw_type = rnpgbe_hw_n500,
+	.init = &rnpgbe_init_n500,
+};
+
+const struct rnpgbe_info rnpgbe_n210_info = {
+	.total_queue_pair_cnts = RNPGBE_MAX_QUEUES,
+	.hw_type = rnpgbe_hw_n210,
+	.init = &rnpgbe_init_n210,
+};
+
+const struct rnpgbe_info rnpgbe_n210L_info = {
+	.total_queue_pair_cnts = RNPGBE_MAX_QUEUES,
+	.hw_type = rnpgbe_hw_n210L,
+	.init = &rnpgbe_init_n210,
+};
diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h
new file mode 100644
index 000000000000..fc57258537cf
--- /dev/null
+++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h
@@ -0,0 +1,12 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Copyright(c) 2020 - 2025 Mucse Corporation. */
+
+#ifndef _RNPGBE_HW_H
+#define _RNPGBE_HW_H
+
+#define RNPGBE_RING_BASE 0x1000
+#define RNPGBE_MAC_BASE 0x20000
+#define RNPGBE_ETH_BASE 0x10000
+/**************** CHIP Resource ****************************/
+#define RNPGBE_MAX_QUEUES 8
+#endif /* _RNPGBE_HW_H */
diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c
index c2e099bd2639..c151995309f8 100644
--- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c
+++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c
@@ -4,10 +4,17 @@
 #include <linux/types.h>
 #include <linux/module.h>
 #include <linux/pci.h>
+#include <linux/netdevice.h>
+#include <linux/etherdevice.h>
 
 #include "rnpgbe.h"
 
 static const char rnpgbe_driver_name[] = "rnpgbe";
+static const struct rnpgbe_info *rnpgbe_info_tbl[] = {
+	[board_n500] = &rnpgbe_n500_info,
+	[board_n210] = &rnpgbe_n210_info,
+	[board_n210L] = &rnpgbe_n210L_info,
+};
 
 /* rnpgbe_pci_tbl - PCI Device ID Table
  *
@@ -27,6 +34,85 @@ static struct pci_device_id rnpgbe_pci_tbl[] = {
 	{0, },
 };
 
+/**
+ * rnpgbe_add_adapter - Add netdev for this pci_dev
+ * @pdev: PCI device information structure
+ * @info: chip info structure
+ *
+ * rnpgbe_add_adapter initializes a netdev for this pci_dev
+ * structure. Initializes Bar map, private structure, and a
+ * hardware reset occur.
+ *
+ * @return: 0 on success, negative on failure
+ **/
+static int rnpgbe_add_adapter(struct pci_dev *pdev,
+			      const struct rnpgbe_info *info)
+{
+	struct net_device *netdev;
+	void __iomem *hw_addr;
+	static int bd_number;
+	struct mucse *mucse;
+	struct mucse_hw *hw;
+	u32 dma_version = 0;
+	u32 queues;
+	int err;
+
+	queues = info->total_queue_pair_cnts;
+	netdev = alloc_etherdev_mq(sizeof(struct mucse), queues);
+	if (!netdev)
+		return -ENOMEM;
+
+	SET_NETDEV_DEV(netdev, &pdev->dev);
+	mucse = netdev_priv(netdev);
+	mucse->netdev = netdev;
+	mucse->pdev = pdev;
+	mucse->bd_number = bd_number++;
+	pci_set_drvdata(pdev, mucse);
+
+	hw = &mucse->hw;
+	hw->back = mucse;
+	hw->hw_type = info->hw_type;
+	hw->pdev = pdev;
+
+	switch (hw->hw_type) {
+	case rnpgbe_hw_n500:
+		hw_addr = devm_ioremap(&pdev->dev,
+				       pci_resource_start(pdev, 2),
+				       pci_resource_len(pdev, 2));
+		if (!hw_addr) {
+			err = -EIO;
+			goto err_free_net;
+		}
+
+		dma_version = readl(hw_addr);
+		break;
+	case rnpgbe_hw_n210:
+	case rnpgbe_hw_n210L:
+		hw_addr = devm_ioremap(&pdev->dev,
+				       pci_resource_start(pdev, 2),
+				       pci_resource_len(pdev, 2));
+		if (!hw_addr) {
+			err = -EIO;
+			goto err_free_net;
+		}
+
+		dma_version = readl(hw_addr);
+		break;
+	default:
+		err = -EIO;
+		goto err_free_net;
+	}
+	hw->hw_addr = hw_addr;
+	hw->dma.dma_version = dma_version;
+	hw->driver_version = 0x0002040f;
+	info->init(hw);
+	return 0;
+
+err_free_net:
+	free_netdev(netdev);
+	return err;
+}
+
 /**
  * rnpgbe_probe - Device initialization routine
  * @pdev: PCI device information struct
@@ -39,6 +125,7 @@ static struct pci_device_id rnpgbe_pci_tbl[] = {
  **/
 static int rnpgbe_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 {
+	const struct rnpgbe_info *info = rnpgbe_info_tbl[id->driver_data];
 	int err;
 
 	err = pci_enable_device_mem(pdev);
@@ -61,14 +148,37 @@ static int rnpgbe_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 
 	pci_set_master(pdev);
 	pci_save_state(pdev);
+	err = rnpgbe_add_adapter(pdev, info);
+	if (err)
+		goto err_regions;
 
 	return 0;
+err_regions:
+	pci_release_mem_regions(pdev);
 err_dma:
 err_pci_req:
 	pci_disable_device(pdev);
 	return err;
 }
 
+/**
+ * rnpgbe_rm_adapter - Remove netdev for this mucse structure
+ * @pdev: PCI device information struct
+ *
+ * rnpgbe_rm_adapter remove a netdev for this mucse structure
+ **/
+static void rnpgbe_rm_adapter(struct pci_dev *pdev)
+{
+	struct mucse *mucse = pci_get_drvdata(pdev);
+	struct net_device *netdev;
+
+	if (!mucse)
+		return;
+	netdev = mucse->netdev;
+	mucse->netdev = NULL;
+	free_netdev(netdev);
+}
+
 /**
  * rnpgbe_remove - Device removal routine
  * @pdev: PCI device information struct
@@ -80,6 +190,7 @@ static int rnpgbe_probe(struct pci_dev *pdev, const struct pci_device_id *id)
  **/
 static void rnpgbe_remove(struct pci_dev *pdev)
 {
+	rnpgbe_rm_adapter(pdev);
 	pci_release_mem_regions(pdev);
 	pci_disable_device(pdev);
 }
@@ -92,7 +203,11 @@ static void rnpgbe_remove(struct pci_dev *pdev)
 static void rnpgbe_dev_shutdown(struct pci_dev *pdev,
 				bool *enable_wake)
 {
+	struct mucse *mucse = pci_get_drvdata(pdev);
+	struct net_device *netdev = mucse->netdev;
+
 	*enable_wake = false;
+	netif_device_detach(netdev);
 	pci_disable_device(pdev);
 }
 
-- 
2.25.1

[PATCH v3 3/5] net: rnpgbe: Add basic mbx ops support

[Dong Yibo]

Initialize basic mbx function.

Signed-off-by: Dong Yibo <dong100@mucse.com>
---
 drivers/net/ethernet/mucse/rnpgbe/Makefile    |   3 +-
 drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h    |  37 ++
 .../net/ethernet/mucse/rnpgbe/rnpgbe_chip.c   |   5 +
 drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h |   2 +
 .../net/ethernet/mucse/rnpgbe/rnpgbe_mbx.c    | 443 ++++++++++++++++++
 .../net/ethernet/mucse/rnpgbe/rnpgbe_mbx.h    |  31 ++
 6 files changed, 520 insertions(+), 1 deletion(-)
 create mode 100644 drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx.c
 create mode 100644 drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx.h

diff --git a/drivers/net/ethernet/mucse/rnpgbe/Makefile b/drivers/net/ethernet/mucse/rnpgbe/Makefile
index 42c359f459d9..5fc878ada4b1 100644
--- a/drivers/net/ethernet/mucse/rnpgbe/Makefile
+++ b/drivers/net/ethernet/mucse/rnpgbe/Makefile
@@ -6,4 +6,5 @@
 
 obj-$(CONFIG_MGBE) += rnpgbe.o
 rnpgbe-objs := rnpgbe_main.o\
-	       rnpgbe_chip.o
+	       rnpgbe_chip.o\
+	       rnpgbe_mbx.o
diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h
index 0dd3d3cb2a4d..05830bb73d3e 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/mutex.h>
 
 extern const struct rnpgbe_info rnpgbe_n500_info;
 extern const struct rnpgbe_info rnpgbe_n210_info;
@@ -40,7 +41,43 @@ struct mucse_mac_info {
 	void *back;
 };
 
+struct mucse_hw;
+
+struct mucse_mbx_operations {
+	void (*init_params)(struct mucse_hw *hw);
+	int (*read)(struct mucse_hw *hw, u32 *msg,
+		    u16 size);
+	int (*write)(struct mucse_hw *hw, u32 *msg,
+		     u16 size);
+	int (*read_posted)(struct mucse_hw *hw, u32 *msg,
+			   u16 size);
+	int (*write_posted)(struct mucse_hw *hw, u32 *msg,
+			    u16 size);
+	int (*check_for_msg)(struct mucse_hw *hw);
+	int (*check_for_ack)(struct mucse_hw *hw);
+	void (*configure)(struct mucse_hw *hw, int num_vec,
+			  bool enable);
+};
+
+struct mucse_mbx_stats {
+	u32 msgs_tx;
+	u32 msgs_rx;
+	u32 acks;
+	u32 reqs;
+	u32 rsts;
+};
+
 struct mucse_mbx_info {
+	const struct mucse_mbx_operations *ops;
+	struct mucse_mbx_stats stats;
+	u32 timeout;
+	u32 usec_delay;
+	u16 size;
+	u16 fw_req;
+	u16 fw_ack;
+	/* lock for only one use mbx */
+	struct mutex lock;
+	bool irq_enabled;
 	/* fw <--> pf mbx */
 	u32 fw_pf_shm_base;
 	u32 pf2fw_mbox_ctrl;
diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_chip.c b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_chip.c
index 20ec67c9391e..16d0a76114b5 100644
--- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_chip.c
+++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_chip.c
@@ -1,8 +1,11 @@
 // SPDX-License-Identifier: GPL-2.0
 /* Copyright(c) 2020 - 2025 Mucse Corporation. */
 
+#include <linux/string.h>
+
 #include "rnpgbe.h"
 #include "rnpgbe_hw.h"
+#include "rnpgbe_mbx.h"
 
 /**
  * rnpgbe_init_common - Setup common attribute
@@ -23,6 +26,8 @@ static void rnpgbe_init_common(struct mucse_hw *hw)
 
 	mac->mac_addr = hw->hw_addr + RNPGBE_MAC_BASE;
 	mac->back = hw;
+
+	hw->mbx.ops = &mucse_mbx_ops_generic;
 }
 
 /**
diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h
index fc57258537cf..aee037e3219d 100644
--- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h
+++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h
@@ -7,6 +7,8 @@
 #define RNPGBE_RING_BASE 0x1000
 #define RNPGBE_MAC_BASE 0x20000
 #define RNPGBE_ETH_BASE 0x10000
+/**************** DMA Registers ****************************/
+#define RNPGBE_DMA_DUMY 0x000c
 /**************** CHIP Resource ****************************/
 #define RNPGBE_MAX_QUEUES 8
 #endif /* _RNPGBE_HW_H */
diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx.c b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx.c
new file mode 100644
index 000000000000..1195cf945ad1
--- /dev/null
+++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx.c
@@ -0,0 +1,443 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright(c) 2022 - 2025 Mucse Corporation. */
+
+#include <linux/pci.h>
+#include <linux/errno.h>
+#include <linux/delay.h>
+#include <linux/iopoll.h>
+
+#include "rnpgbe.h"
+#include "rnpgbe_mbx.h"
+#include "rnpgbe_hw.h"
+
+/**
+ * mucse_read_mbx - Reads a message from the mailbox
+ * @hw: pointer to the HW structure
+ * @msg: the message buffer
+ * @size: length of buffer
+ *
+ * @return: 0 on success, negative on failure
+ **/
+int mucse_read_mbx(struct mucse_hw *hw, u32 *msg, u16 size)
+{
+	struct mucse_mbx_info *mbx = &hw->mbx;
+
+	/* limit read size */
+	min(size, mbx->size);
+	return mbx->ops->read(hw, msg, size);
+}
+
+/**
+ * mucse_write_mbx - Write a message to the mailbox
+ * @hw: pointer to the HW structure
+ * @msg: the message buffer
+ * @size: length of buffer
+ *
+ * @return: 0 on success, negative on failure
+ **/
+int mucse_write_mbx(struct mucse_hw *hw, u32 *msg, u16 size)
+{
+	struct mucse_mbx_info *mbx = &hw->mbx;
+
+	return mbx->ops->write(hw, msg, size);
+}
+
+/**
+ * mucse_mbx_get_req - Read req from reg
+ * @hw: pointer to the HW structure
+ * @reg: register to read
+ *
+ * @return: the req value
+ **/
+static u16 mucse_mbx_get_req(struct mucse_hw *hw, int reg)
+{
+	return mbx_rd32(hw, reg) & GENMASK(15, 0);
+}
+
+/**
+ * mucse_mbx_get_ack - Read ack from reg
+ * @hw: pointer to the HW structure
+ * @reg: register to read
+ *
+ * @return: the ack value
+ **/
+static u16 mucse_mbx_get_ack(struct mucse_hw *hw, int reg)
+{
+	return (mbx_rd32(hw, reg) >> 16);
+}
+
+/**
+ * mucse_mbx_inc_pf_req - Increase req
+ * @hw: pointer to the HW structure
+ *
+ * mucse_mbx_inc_pf_req read pf_req from hw, then write
+ * new value back after increase
+ **/
+static void mucse_mbx_inc_pf_req(struct mucse_hw *hw)
+{
+	struct mucse_mbx_info *mbx = &hw->mbx;
+	u32 reg, v;
+	u16 req;
+
+	reg = PF2FW_COUNTER(mbx);
+	v = mbx_rd32(hw, reg);
+	req = (v & GENMASK(15, 0));
+	req++;
+	v &= GENMASK(31, 16);
+	v |= req;
+	mbx_wr32(hw, reg, v);
+	hw->mbx.stats.msgs_tx++;
+}
+
+/**
+ * mucse_mbx_inc_pf_ack - Increase ack
+ * @hw: pointer to the HW structure
+ *
+ * mucse_mbx_inc_pf_ack read pf_ack from hw, then write
+ * new value back after increase
+ **/
+static void mucse_mbx_inc_pf_ack(struct mucse_hw *hw)
+{
+	struct mucse_mbx_info *mbx = &hw->mbx;
+	u32 reg, v;
+	u16 ack;
+
+	reg = PF2FW_COUNTER(mbx);
+	v = mbx_rd32(hw, reg);
+	ack = (v >> 16) & GENMASK(15, 0);
+	ack++;
+	v &= GENMASK(15, 0);
+	v |= (ack << 16);
+	mbx_wr32(hw, reg, v);
+	hw->mbx.stats.msgs_rx++;
+}
+
+/**
+ * mucse_check_for_msg - Check to see if fw sent us mail
+ * @hw: pointer to the HW structure
+ *
+ * @return: 0 on success, negative on failure
+ **/
+int mucse_check_for_msg(struct mucse_hw *hw)
+{
+	struct mucse_mbx_info *mbx = &hw->mbx;
+
+	return mbx->ops->check_for_msg(hw);
+}
+
+/**
+ * mucse_check_for_ack - Check to see if fw sent us ACK
+ * @hw: pointer to the HW structure
+ *
+ * @return: 0 on success, negative on failure
+ **/
+int mucse_check_for_ack(struct mucse_hw *hw)
+{
+	struct mucse_mbx_info *mbx = &hw->mbx;
+
+	return mbx->ops->check_for_ack(hw);
+}
+
+/**
+ * mucse_poll_for_msg - Wait for message notification
+ * @hw: pointer to the HW structure
+ *
+ * @return: 0 on success, negative on failure
+ **/
+static int mucse_poll_for_msg(struct mucse_hw *hw)
+{
+	struct mucse_mbx_info *mbx = &hw->mbx;
+	int countdown = mbx->timeout;
+	int val;
+
+	return read_poll_timeout(mbx->ops->check_for_msg,
+				 val, val == 0, mbx->usec_delay,
+				 countdown * mbx->usec_delay,
+				 false, hw);
+}
+
+/**
+ * mucse_poll_for_ack - Wait for message acknowledgment
+ * @hw: pointer to the HW structure
+ *
+ * @return: 0 if it successfully received a message acknowledgment
+ **/
+static int mucse_poll_for_ack(struct mucse_hw *hw)
+{
+	struct mucse_mbx_info *mbx = &hw->mbx;
+	int countdown = mbx->timeout;
+	int val;
+
+	return read_poll_timeout(mbx->ops->check_for_ack,
+				 val, val == 0, mbx->usec_delay,
+				 countdown * mbx->usec_delay,
+				 false, hw);
+}
+
+/**
+ * mucse_read_posted_mbx - Wait for message 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.
+ **/
+static int mucse_read_posted_mbx(struct mucse_hw *hw, u32 *msg, u16 size)
+{
+	struct mucse_mbx_info *mbx = &hw->mbx;
+	int ret;
+
+	ret = mucse_poll_for_msg(hw);
+	if (ret)
+		return ret;
+
+	return mbx->ops->read(hw, msg, size);
+}
+
+/**
+ * mucse_write_posted_mbx - Write a message to the mailbox, wait for ack
+ * @hw: pointer to the HW structure
+ * @msg: the message buffer
+ * @size: length of buffer
+ *
+ * @return: 0 if it successfully copied message into the buffer and
+ * received an ack to that message within delay * timeout period
+ **/
+static int mucse_write_posted_mbx(struct mucse_hw *hw, u32 *msg, u16 size)
+{
+	struct mucse_mbx_info *mbx = &hw->mbx;
+	int ret;
+
+	ret = mbx->ops->write(hw, msg, size);
+	if (ret)
+		return ret;
+	return mucse_poll_for_ack(hw);
+}
+
+/**
+ * mucse_check_for_msg_pf - Check to see if the fw has sent mail
+ * @hw: pointer to the HW structure
+ *
+ * @return: 0 if the fw has set the Status bit or else
+ * -EIO
+ **/
+static int mucse_check_for_msg_pf(struct mucse_hw *hw)
+{
+	struct mucse_mbx_info *mbx = &hw->mbx;
+	u16 hw_req_count = 0;
+
+	hw_req_count = mucse_mbx_get_req(hw, FW2PF_COUNTER(mbx));
+	/* chip's register is reset to 0 when rc send reset
+	 * mbx command. This causes 'hw_req_count != hw->mbx.fw_req'
+	 * be TRUE before fw really reply. Driver must wait fw reset
+	 * done reply before using chip, we must check no-zero.
+	 **/
+	if (hw_req_count != 0 && hw_req_count != hw->mbx.fw_req) {
+		hw->mbx.stats.reqs++;
+		return 0;
+	}
+
+	return -EIO;
+}
+
+/**
+ * mucse_check_for_ack_pf - Check to see if the VF has ACKed
+ * @hw: pointer to the HW structure
+ *
+ * @return: 0 if the fw has set the Status bit or else
+ * -EIO
+ **/
+static int mucse_check_for_ack_pf(struct mucse_hw *hw)
+{
+	struct mucse_mbx_info *mbx = &hw->mbx;
+	u16 hw_fw_ack;
+
+	hw_fw_ack = mucse_mbx_get_ack(hw, FW2PF_COUNTER(mbx));
+	/* chip's register is reset to 0 when rc send reset
+	 * mbx command. This causes 'hw_fw_ack != hw->mbx.fw_ack'
+	 * be TRUE before fw really reply. Driver must wait fw reset
+	 * done reply before using chip, we must check no-zero.
+	 **/
+	if (hw_fw_ack != 0 && hw_fw_ack != hw->mbx.fw_ack) {
+		hw->mbx.stats.acks++;
+		return 0;
+	}
+
+	return -EIO;
+}
+
+/**
+ * mucse_obtain_mbx_lock_pf - Obtain mailbox lock
+ * @hw: pointer to the HW structure
+ *
+ * This function maybe used in an irq handler.
+ *
+ * @return: 0 if we obtained the mailbox lock
+ **/
+static int mucse_obtain_mbx_lock_pf(struct mucse_hw *hw)
+{
+	struct mucse_mbx_info *mbx = &hw->mbx;
+	int try_cnt = 5000;
+	u32 reg;
+
+	reg = PF2FW_MBOX_CTRL(mbx);
+	while (try_cnt-- > 0) {
+		mbx_wr32(hw, reg, MBOX_PF_HOLD);
+		/* force write back before check */
+		wmb();
+		if (mbx_rd32(hw, reg) & MBOX_PF_HOLD)
+			return 0;
+		udelay(100);
+	}
+	return -EIO;
+}
+
+/**
+ * mucse_write_mbx_pf - Place a message in the mailbox
+ * @hw: pointer to the HW structure
+ * @msg: the message buffer
+ * @size: length of buffer
+ *
+ * This function maybe used in an irq handler.
+ *
+ * @return: 0 if it successfully copied message into the buffer
+ **/
+static int mucse_write_mbx_pf(struct mucse_hw *hw, u32 *msg, u16 size)
+{
+	struct mucse_mbx_info *mbx = &hw->mbx;
+	u32 data_reg, ctrl_reg;
+	int ret;
+	u16 i;
+
+	data_reg = FW_PF_SHM_DATA(mbx);
+	ctrl_reg = PF2FW_MBOX_CTRL(mbx);
+	ret = mucse_obtain_mbx_lock_pf(hw);
+	if (ret)
+		return ret;
+
+	for (i = 0; i < size; i++)
+		mbx_wr32(hw, data_reg + i * 4, msg[i]);
+
+	/* flush msg and acks as we are overwriting the message buffer */
+	hw->mbx.fw_ack = mucse_mbx_get_ack(hw, FW2PF_COUNTER(mbx));
+	mucse_mbx_inc_pf_req(hw);
+	mbx_wr32(hw, ctrl_reg, MBOX_CTRL_REQ);
+
+	return 0;
+}
+
+/**
+ * mucse_read_mbx_pf - Read a message from the mailbox
+ * @hw: pointer to the HW structure
+ * @msg: the message buffer
+ * @size: length of buffer
+ *
+ * This function copies a message from the mailbox buffer to the caller's
+ * memory buffer. The presumption is that the caller knows that there was
+ * a message due to a fw request so no polling for message is needed.
+ *
+ * @return: 0 on success, negative on failure
+ **/
+static int mucse_read_mbx_pf(struct mucse_hw *hw, u32 *msg, u16 size)
+{
+	struct mucse_mbx_info *mbx = &hw->mbx;
+	u32 data_reg, ctrl_reg;
+	int ret;
+	u32 i;
+
+	data_reg = FW_PF_SHM_DATA(mbx);
+	ctrl_reg = PF2FW_MBOX_CTRL(mbx);
+
+	ret = mucse_obtain_mbx_lock_pf(hw);
+	if (ret)
+		return ret;
+	for (i = 0; i < size; i++)
+		msg[i] = mbx_rd32(hw, data_reg + 4 * i);
+	/* Hw need write data_reg at last */
+	mbx_wr32(hw, data_reg, 0);
+	hw->mbx.fw_req = mucse_mbx_get_req(hw, FW2PF_COUNTER(mbx));
+	mucse_mbx_inc_pf_ack(hw);
+	mbx_wr32(hw, ctrl_reg, 0);
+
+	return 0;
+}
+
+/**
+ * mucse_mbx_reset - Reset mbx info, sync info from regs
+ * @hw: pointer to the HW structure
+ *
+ * This function reset all mbx variables to default.
+ **/
+static void mucse_mbx_reset(struct mucse_hw *hw)
+{
+	struct mucse_mbx_info *mbx = &hw->mbx;
+	int v;
+
+	v = mbx_rd32(hw, FW2PF_COUNTER(mbx));
+	hw->mbx.fw_req = v & GENMASK(15, 0);
+	hw->mbx.fw_ack = (v >> 16) & GENMASK(15, 0);
+	mbx_wr32(hw, PF2FW_MBOX_CTRL(mbx), 0);
+	mbx_wr32(hw, FW_PF_MBOX_MASK(mbx), GENMASK(31, 16));
+}
+
+/**
+ * mucse_mbx_configure_pf - Configure mbx to use nr_vec interrupt
+ * @hw: pointer to the HW structure
+ * @nr_vec: vector number for mbx
+ * @enable: TRUE for enable, FALSE for disable
+ *
+ * This function configure mbx to use interrupt nr_vec.
+ **/
+static void mucse_mbx_configure_pf(struct mucse_hw *hw, int nr_vec,
+				   bool enable)
+{
+	struct mucse_mbx_info *mbx = &hw->mbx;
+	u32 v;
+
+	if (enable) {
+		v = mbx_rd32(hw, FW2PF_COUNTER(mbx));
+		hw->mbx.fw_req = v & GENMASK(15, 0);
+		hw->mbx.fw_ack = (v >> 16) & GENMASK(15, 0);
+		mbx_wr32(hw, PF2FW_MBOX_CTRL(mbx), 0);
+		mbx_wr32(hw, FW2PF_MBOX_VEC(mbx), nr_vec);
+		mbx_wr32(hw, FW_PF_MBOX_MASK(mbx), GENMASK(31, 16));
+	} else {
+		mbx_wr32(hw, FW_PF_MBOX_MASK(mbx), 0xfffffffe);
+		mbx_wr32(hw, PF2FW_MBOX_CTRL(mbx), 0);
+		mbx_wr32(hw, RNPGBE_DMA_DUMY, 0);
+	}
+}
+
+/**
+ * mucse_init_mbx_params_pf - Set initial values for pf mailbox
+ * @hw: pointer to the HW structure
+ *
+ * Initializes the hw->mbx struct to correct values for pf mailbox
+ */
+static void mucse_init_mbx_params_pf(struct mucse_hw *hw)
+{
+	struct mucse_mbx_info *mbx = &hw->mbx;
+
+	mbx->usec_delay = 100;
+	mbx->timeout = (4 * 1000 * 1000) / mbx->usec_delay;
+	mbx->stats.msgs_tx = 0;
+	mbx->stats.msgs_rx = 0;
+	mbx->stats.reqs = 0;
+	mbx->stats.acks = 0;
+	mbx->stats.rsts = 0;
+	mbx->size = MUCSE_MAILBOX_WORDS;
+	mutex_init(&mbx->lock);
+	mucse_mbx_reset(hw);
+}
+
+const struct mucse_mbx_operations mucse_mbx_ops_generic = {
+	.init_params = mucse_init_mbx_params_pf,
+	.read = mucse_read_mbx_pf,
+	.write = mucse_write_mbx_pf,
+	.read_posted = mucse_read_posted_mbx,
+	.write_posted = mucse_write_posted_mbx,
+	.check_for_msg = mucse_check_for_msg_pf,
+	.check_for_ack = mucse_check_for_ack_pf,
+	.configure = mucse_mbx_configure_pf,
+};
diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx.h b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx.h
new file mode 100644
index 000000000000..2d850a11c369
--- /dev/null
+++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx.h
@@ -0,0 +1,31 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Copyright(c) 2020 - 2025 Mucse Corporation. */
+
+#ifndef _RNPGBE_MBX_H
+#define _RNPGBE_MBX_H
+
+#include "rnpgbe.h"
+
+#define MUCSE_MAILBOX_WORDS 14
+#define MUCSE_FW_MAILBOX_WORDS MUCSE_MAILBOX_WORDS
+#define FW_PF_SHM(mbx) ((mbx)->fw_pf_shm_base)
+#define FW2PF_COUNTER(mbx) (FW_PF_SHM(mbx) + 0)
+#define PF2FW_COUNTER(mbx) (FW_PF_SHM(mbx) + 4)
+#define FW_PF_SHM_DATA(mbx) (FW_PF_SHM(mbx) + 8)
+#define FW2PF_MBOX_VEC(mbx) ((mbx)->fw2pf_mbox_vec)
+#define PF2FW_MBOX_CTRL(mbx) ((mbx)->pf2fw_mbox_ctrl)
+#define FW_PF_MBOX_MASK(mbx) ((mbx)->fw_pf_mbox_mask)
+#define MBOX_CTRL_REQ BIT(0)
+#define MBOX_PF_HOLD BIT(3)
+#define MBOX_IRQ_EN 0
+#define MBOX_IRQ_DISABLE 1
+#define mbx_rd32(hw, reg) readl((hw)->hw_addr + (reg))
+#define mbx_wr32(hw, reg, val) writel((val), (hw)->hw_addr + (reg))
+
+extern const struct mucse_mbx_operations mucse_mbx_ops_generic;
+
+int mucse_read_mbx(struct mucse_hw *hw, u32 *msg, u16 size);
+int mucse_write_mbx(struct mucse_hw *hw, u32 *msg, u16 size);
+int mucse_check_for_msg(struct mucse_hw *hw);
+int mucse_check_for_ack(struct mucse_hw *hw);
+#endif /* _RNPGBE_MBX_H */
-- 
2.25.1

[PATCH v3 4/5] net: rnpgbe: Add basic mbx_fw support

[Dong Yibo]

Initialize basic mbx_fw ops, such as get_capability, reset phy
and so on.

Signed-off-by: Dong Yibo <dong100@mucse.com>
---
 drivers/net/ethernet/mucse/rnpgbe/Makefile    |   3 +-
 drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h    |   4 +
 .../net/ethernet/mucse/rnpgbe/rnpgbe_mbx_fw.c | 275 ++++++++++++++++++
 .../net/ethernet/mucse/rnpgbe/rnpgbe_mbx_fw.h | 201 +++++++++++++
 4 files changed, 482 insertions(+), 1 deletion(-)
 create mode 100644 drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx_fw.c
 create mode 100644 drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx_fw.h

diff --git a/drivers/net/ethernet/mucse/rnpgbe/Makefile b/drivers/net/ethernet/mucse/rnpgbe/Makefile
index 5fc878ada4b1..de8bcb7772ab 100644
--- a/drivers/net/ethernet/mucse/rnpgbe/Makefile
+++ b/drivers/net/ethernet/mucse/rnpgbe/Makefile
@@ -7,4 +7,5 @@
 obj-$(CONFIG_MGBE) += rnpgbe.o
 rnpgbe-objs := rnpgbe_main.o\
 	       rnpgbe_chip.o\
-	       rnpgbe_mbx.o
+	       rnpgbe_mbx.o\
+	       rnpgbe_mbx_fw.o
diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h
index 05830bb73d3e..6cb14b79cbfe 100644
--- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h
+++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h
@@ -88,9 +88,13 @@ struct mucse_mbx_info {
 
 struct mucse_hw {
 	void *back;
+	u8 pfvfnum;
 	void __iomem *hw_addr;
 	void __iomem *ring_msix_base;
 	struct pci_dev *pdev;
+	u32 fw_version;
+	u32 axi_mhz;
+	u32 bd_uid;
 	enum rnpgbe_hw_type hw_type;
 	struct mucse_dma_info dma;
 	struct mucse_eth_info eth;
diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx_fw.c b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx_fw.c
new file mode 100644
index 000000000000..374eb9df1369
--- /dev/null
+++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx_fw.c
@@ -0,0 +1,275 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright(c) 2020 - 2025 Mucse Corporation. */
+
+#include <linux/pci.h>
+
+#include "rnpgbe.h"
+#include "rnpgbe_hw.h"
+#include "rnpgbe_mbx.h"
+#include "rnpgbe_mbx_fw.h"
+
+/**
+ * mucse_fw_send_cmd_wait - Send cmd req and wait for response
+ * @hw: pointer to the HW structure
+ * @req: pointer to the cmd req structure
+ * @reply: pointer to the fw reply structure
+ *
+ * mucse_fw_send_cmd_wait sends req to pf-fw mailbox and wait
+ * reply from fw.
+ *
+ * @return: 0 on success, negative on failure
+ **/
+static int mucse_fw_send_cmd_wait(struct mucse_hw *hw,
+				  struct mbx_fw_cmd_req *req,
+				  struct mbx_fw_cmd_reply *reply)
+{
+	int len = le16_to_cpu(req->datalen) + MBX_REQ_HDR_LEN;
+	int retry_cnt = 3;
+	int err;
+
+	err = mutex_lock_interruptible(&hw->mbx.lock);
+	if (err)
+		return err;
+	err = hw->mbx.ops->write_posted(hw, (u32 *)req,
+					L_WD(len));
+	if (err) {
+		mutex_unlock(&hw->mbx.lock);
+		return err;
+	}
+	do {
+		err = hw->mbx.ops->read_posted(hw, (u32 *)reply,
+					       L_WD(sizeof(*reply)));
+		if (err) {
+			mutex_unlock(&hw->mbx.lock);
+			return err;
+		}
+	} while (--retry_cnt >= 0 && reply->opcode != req->opcode);
+	mutex_unlock(&hw->mbx.lock);
+	if (retry_cnt < 0 || reply->error_code)
+		return -EIO;
+	return 0;
+}
+
+/**
+ * mucse_fw_get_capability - Get hw abilities from fw
+ * @hw: pointer to the HW structure
+ * @abil: pointer to the hw_abilities structure
+ *
+ * mucse_fw_get_capability tries to get hw abilities from
+ * hw.
+ *
+ * @return: 0 on success, negative on failure
+ **/
+static int mucse_fw_get_capability(struct mucse_hw *hw,
+				   struct hw_abilities *abil)
+{
+	struct mbx_fw_cmd_reply reply;
+	struct mbx_fw_cmd_req req;
+	int err;
+
+	memset(&req, 0, sizeof(req));
+	memset(&reply, 0, sizeof(reply));
+	build_phy_abalities_req(&req, &req);
+	err = mucse_fw_send_cmd_wait(hw, &req, &reply);
+	if (!err)
+		memcpy(abil, &reply.hw_abilities, sizeof(*abil));
+	return err;
+}
+
+/**
+ * mucse_mbx_get_capability - Get hw abilities from fw
+ * @hw: pointer to the HW structure
+ *
+ * mucse_mbx_get_capability tries to some capabities from
+ * hw. Many retrys will do if it is failed.
+ *
+ * @return: 0 on success, negative on failure
+ **/
+int mucse_mbx_get_capability(struct mucse_hw *hw)
+{
+	struct hw_abilities ability;
+	int try_cnt = 3;
+	int err;
+
+	memset(&ability, 0, sizeof(ability));
+	while (try_cnt--) {
+		err = mucse_fw_get_capability(hw, &ability);
+		if (err)
+			continue;
+		hw->pfvfnum = le16_to_cpu(ability.pfnum);
+		hw->fw_version = le32_to_cpu(ability.fw_version);
+		hw->axi_mhz = le32_to_cpu(ability.axi_mhz);
+		hw->bd_uid = le32_to_cpu(ability.bd_uid);
+		return 0;
+	}
+	return err;
+}
+
+/**
+ * mbx_cookie_zalloc - Alloc a cookie structure
+ * @priv_len: private length for this cookie
+ *
+ * @return: cookie structure on success
+ **/
+static struct mbx_req_cookie *mbx_cookie_zalloc(int priv_len)
+{
+	struct mbx_req_cookie *cookie;
+
+	cookie = kzalloc(struct_size(cookie, priv, priv_len), GFP_KERNEL);
+	if (cookie) {
+		cookie->timeout_jiffes = 30 * HZ;
+		cookie->magic = COOKIE_MAGIC;
+		cookie->priv_len = priv_len;
+	}
+	return cookie;
+}
+
+/**
+ * mucse_mbx_fw_post_req - Posts a mbx req to firmware and wait reply
+ * @hw: pointer to the HW structure
+ * @req: pointer to the cmd req structure
+ * @cookie: pointer to the req cookie
+ *
+ * mucse_mbx_fw_post_req posts a mbx req to firmware and wait for the
+ * reply. cookie->wait will be set in irq handler.
+ *
+ * @return: 0 on success, negative on failure
+ **/
+static int mucse_mbx_fw_post_req(struct mucse_hw *hw,
+				 struct mbx_fw_cmd_req *req,
+				 struct mbx_req_cookie *cookie)
+{
+	int len = le16_to_cpu(req->datalen) + MBX_REQ_HDR_LEN;
+	int err;
+
+	cookie->errcode = 0;
+	cookie->done = 0;
+	init_waitqueue_head(&cookie->wait);
+	err = mutex_lock_interruptible(&hw->mbx.lock);
+	if (err)
+		return err;
+	err = mucse_write_mbx(hw, (u32 *)req,
+			      L_WD(len));
+	if (err) {
+		mutex_unlock(&hw->mbx.lock);
+		return err;
+	}
+	do {
+		err = wait_event_interruptible_timeout(cookie->wait,
+						       cookie->done == 1,
+						       cookie->timeout_jiffes);
+	} while (err == -ERESTARTSYS);
+
+	mutex_unlock(&hw->mbx.lock);
+	if (!err)
+		err = -ETIME;
+	else
+		err = 0;
+	if (!err && cookie->errcode)
+		err = cookie->errcode;
+
+	return err;
+}
+
+/**
+ * mucse_mbx_ifinsmod - Echo driver insmod status to hw
+ * @hw: pointer to the HW structure
+ * @status: true for insmod, false for rmmod
+ *
+ * @return: 0 on success, negative on failure
+ **/
+int mucse_mbx_ifinsmod(struct mucse_hw *hw, int status)
+{
+	struct mbx_fw_cmd_reply reply;
+	struct mbx_fw_cmd_req req;
+	int len;
+	int err;
+
+	memset(&req, 0, sizeof(req));
+	memset(&reply, 0, sizeof(reply));
+	build_ifinsmod(&req, hw->driver_version, status);
+	len = le16_to_cpu(req.datalen) + MBX_REQ_HDR_LEN;
+	err = mutex_lock_interruptible(&hw->mbx.lock);
+	if (err)
+		return err;
+
+	if (status) {
+		err = hw->mbx.ops->write_posted(hw, (u32 *)&req,
+						L_WD(len));
+	} else {
+		err = hw->mbx.ops->write(hw, (u32 *)&req,
+					 L_WD(len));
+	}
+
+	mutex_unlock(&hw->mbx.lock);
+	return err;
+}
+
+/**
+ * mucse_mbx_fw_reset_phy - Posts a mbx req to reset hw
+ * @hw: pointer to the HW structure
+ *
+ * mucse_mbx_fw_reset_phy posts a mbx req to firmware to reset hw.
+ * It uses mucse_fw_send_cmd_wait if no irq, and mucse_mbx_fw_post_req
+ * if other irq is registered.
+ *
+ * @return: 0 on success, negative on failure
+ **/
+int mucse_mbx_fw_reset_phy(struct mucse_hw *hw)
+{
+	struct mbx_fw_cmd_reply reply;
+	struct mbx_fw_cmd_req req;
+	int ret;
+
+	memset(&req, 0, sizeof(req));
+	memset(&reply, 0, sizeof(reply));
+	if (hw->mbx.irq_enabled) {
+		struct mbx_req_cookie *cookie = mbx_cookie_zalloc(0);
+
+		if (!cookie)
+			return -ENOMEM;
+
+		build_reset_phy_req(&req, cookie);
+		ret = mucse_mbx_fw_post_req(hw, &req, cookie);
+		kfree(cookie);
+		return ret;
+	}
+
+	build_reset_phy_req(&req, &req);
+	return mucse_fw_send_cmd_wait(hw, &req, &reply);
+}
+
+/**
+ * mucse_fw_get_macaddr - Posts a mbx req to request macaddr
+ * @hw: pointer to the HW structure
+ * @pfvfnum: index of pf/vf num
+ * @mac_addr: pointer to store mac_addr
+ * @lane: lane index
+ *
+ * mucse_fw_get_macaddr posts a mbx req to firmware to get mac_addr.
+ * It uses mucse_fw_send_cmd_wait if no irq, and mucse_mbx_fw_post_req
+ * if other irq is registered.
+ *
+ * @return: 0 on success, negative on failure
+ **/
+int mucse_fw_get_macaddr(struct mucse_hw *hw, int pfvfnum,
+			 u8 *mac_addr,
+			 int lane)
+{
+	struct mbx_fw_cmd_reply reply;
+	struct mbx_fw_cmd_req req;
+	int err;
+
+	memset(&req, 0, sizeof(req));
+	memset(&reply, 0, sizeof(reply));
+	build_get_macaddress_req(&req, 1 << lane, pfvfnum, &req);
+	err = mucse_fw_send_cmd_wait(hw, &req, &reply);
+	if (err)
+		return err;
+
+	if ((1 << lane) & le32_to_cpu(reply.mac_addr.lanes))
+		memcpy(mac_addr, reply.mac_addr.addrs[lane].mac, 6);
+	else
+		return -ENODATA;
+	return 0;
+}
diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx_fw.h b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx_fw.h
new file mode 100644
index 000000000000..2e33073bfe9b
--- /dev/null
+++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx_fw.h
@@ -0,0 +1,201 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Copyright(c) 2020 - 2025 Mucse Corporation. */
+
+#ifndef _RNPGBE_MBX_FW_H
+#define _RNPGBE_MBX_FW_H
+
+#include <linux/types.h>
+#include <linux/errno.h>
+#include <linux/wait.h>
+
+#include "rnpgbe.h"
+
+#define MBX_REQ_HDR_LEN 24
+#define L_WD(x) ((x) / 4)
+#define M_DEFAULT_DUMY 0xa0000000
+#define M_DUMY_MASK 0x0f000f11
+#define EVT_LINK_UP 1
+
+struct mbx_fw_cmd_reply;
+typedef void (*cookie_cb)(struct mbx_fw_cmd_reply *reply, void *priv);
+
+struct mbx_req_cookie {
+	int magic;
+#define COOKIE_MAGIC 0xCE
+	cookie_cb cb;
+	int timeout_jiffes;
+	int errcode;
+	wait_queue_head_t wait;
+	int done;
+	int priv_len;
+	char priv[];
+};
+
+enum MUCSE_FW_CMD {
+	GET_PHY_ABALITY = 0x0601,
+	GET_MAC_ADDRES = 0x0602,
+	RESET_PHY = 0x0603,
+	DRIVER_INSMOD = 0x0803,
+};
+
+struct hw_abilities {
+	u8 link_stat;
+	u8 lane_mask;
+	__le32 speed;
+	__le16 phy_type;
+	__le16 nic_mode;
+	__le16 pfnum;
+	__le32 fw_version;
+	__le32 axi_mhz;
+	union {
+		u8 port_id[4];
+		__le32 port_ids;
+	};
+	__le32 bd_uid;
+	__le32 phy_id;
+	__le32 wol_status;
+	union {
+		__le32 ext_ability;
+		struct {
+			u32 valid : 1;
+			u32 wol_en : 1;
+			u32 pci_preset_runtime_en : 1;
+			u32 smbus_en : 1;
+			u32 ncsi_en : 1;
+			u32 rpu_en : 1;
+			u32 v2 : 1;
+			u32 pxe_en : 1;
+			u32 mctp_en : 1;
+			u32 yt8614 : 1;
+			u32 pci_ext_reset : 1;
+			u32 rpu_availble : 1;
+			u32 fw_lldp_ability : 1;
+			u32 lldp_enabled : 1;
+			u32 only_1g : 1;
+			u32 force_down_en: 1;
+		} e_host;
+	};
+} __packed;
+
+static inline void ability_update_host_endian(struct hw_abilities *abi)
+{
+	u32 host_val = le32_to_cpu(abi->ext_ability);
+
+	abi->e_host = *(typeof(abi->e_host) *)&host_val;
+}
+
+#define FLAGS_DD BIT(0)
+#define FLAGS_ERR BIT(2)
+
+/* Request is in little-endian format. Big-endian systems should be considered */
+struct mbx_fw_cmd_req {
+	__le16 flags;
+	__le16 opcode;
+	__le16 datalen;
+	__le16 ret_value;
+	union {
+		struct {
+			__le32 cookie_lo;
+			__le32 cookie_hi;
+		};
+
+		void *cookie;
+	};
+	__le32 reply_lo;
+	__le32 reply_hi;
+	union {
+		u8 data[32];
+		struct {
+			__le32 lane;
+			__le32 status;
+		} ifinsmod;
+		struct {
+			__le32 lane_mask;
+			__le32 pfvf_num;
+		} get_mac_addr;
+	};
+} __packed;
+
+struct mbx_fw_cmd_reply {
+	__le16 flags;
+	__le16 opcode;
+	__le16 error_code;
+	__le16 datalen;
+	union {
+		struct {
+			__le32 cookie_lo;
+			__le32 cookie_hi;
+		};
+		void *cookie;
+	};
+	union {
+		u8 data[40];
+		struct mac_addr {
+			__le32 lanes;
+			struct _addr {
+				/* for macaddr:01:02:03:04:05:06
+				 * mac-hi=0x01020304 mac-lo=0x05060000
+				 */
+				u8 mac[8];
+			} addrs[4];
+		} mac_addr;
+		struct hw_abilities hw_abilities;
+	};
+} __packed;
+
+static inline void build_phy_abalities_req(struct mbx_fw_cmd_req *req,
+					   void *cookie)
+{
+	req->flags = 0;
+	req->opcode = cpu_to_le16(GET_PHY_ABALITY);
+	req->datalen = 0;
+	req->reply_lo = 0;
+	req->reply_hi = 0;
+	req->cookie = cookie;
+}
+
+static inline void build_ifinsmod(struct mbx_fw_cmd_req *req,
+				  unsigned int lane,
+				  int status)
+{
+	req->flags = 0;
+	req->opcode = cpu_to_le16(DRIVER_INSMOD);
+	req->datalen = cpu_to_le16(sizeof(req->ifinsmod));
+	req->cookie = NULL;
+	req->reply_lo = 0;
+	req->reply_hi = 0;
+	req->ifinsmod.lane = cpu_to_le32(lane);
+	req->ifinsmod.status = cpu_to_le32(status);
+}
+
+static inline void build_reset_phy_req(struct mbx_fw_cmd_req *req,
+				       void *cookie)
+{
+	req->flags = 0;
+	req->opcode = cpu_to_le16(RESET_PHY);
+	req->datalen = 0;
+	req->reply_lo = 0;
+	req->reply_hi = 0;
+	req->cookie = cookie;
+}
+
+static inline void build_get_macaddress_req(struct mbx_fw_cmd_req *req,
+					    int lane_mask, int pfvfnum,
+					    void *cookie)
+{
+	req->flags = 0;
+	req->opcode = cpu_to_le16(GET_MAC_ADDRES);
+	req->datalen = cpu_to_le16(sizeof(req->get_mac_addr));
+	req->cookie = cookie;
+	req->reply_lo = 0;
+	req->reply_hi = 0;
+	req->get_mac_addr.lane_mask = cpu_to_le32(lane_mask);
+	req->get_mac_addr.pfvf_num = cpu_to_le32(pfvfnum);
+}
+
+int mucse_mbx_get_capability(struct mucse_hw *hw);
+int mucse_mbx_ifinsmod(struct mucse_hw *hw, int status);
+int mucse_mbx_fw_reset_phy(struct mucse_hw *hw);
+int mucse_fw_get_macaddr(struct mucse_hw *hw, int pfvfnum,
+			 u8 *mac_addr, int lane);
+#endif /* _RNPGBE_MBX_FW_H */
-- 
2.25.1

[PATCH v3 5/5] net: rnpgbe: Add register_netdev

[Dong Yibo]

Initialize get mac from hw, register the netdev.

Signed-off-by: Dong Yibo <dong100@mucse.com>
---
 drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h    | 22 ++++++
 .../net/ethernet/mucse/rnpgbe/rnpgbe_chip.c   | 73 ++++++++++++++++++
 drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h |  1 +
 .../net/ethernet/mucse/rnpgbe/rnpgbe_main.c   | 76 +++++++++++++++++++
 4 files changed, 172 insertions(+)

diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h
index 6cb14b79cbfe..644b8c85c29d 100644
--- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h
+++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h
@@ -6,6 +6,7 @@
 
 #include <linux/types.h>
 #include <linux/mutex.h>
+#include <linux/netdevice.h>
 
 extern const struct rnpgbe_info rnpgbe_n500_info;
 extern const struct rnpgbe_info rnpgbe_n210_info;
@@ -86,6 +87,18 @@ struct mucse_mbx_info {
 	u32 fw2pf_mbox_vec;
 };
 
+struct mucse_hw_operations {
+	int (*init_hw)(struct mucse_hw *hw);
+	int (*reset_hw)(struct mucse_hw *hw);
+	void (*start_hw)(struct mucse_hw *hw);
+	void (*init_rx_addrs)(struct mucse_hw *hw);
+	void (*driver_status)(struct mucse_hw *hw, bool enable, int mode);
+};
+
+enum {
+	mucse_driver_insmod,
+};
+
 struct mucse_hw {
 	void *back;
 	u8 pfvfnum;
@@ -96,12 +109,18 @@ struct mucse_hw {
 	u32 axi_mhz;
 	u32 bd_uid;
 	enum rnpgbe_hw_type hw_type;
+	const struct mucse_hw_operations *ops;
 	struct mucse_dma_info dma;
 	struct mucse_eth_info eth;
 	struct mucse_mac_info mac;
 	struct mucse_mbx_info mbx;
+	u32 flags;
+#define M_FLAGS_INIT_MAC_ADDRESS BIT(0)
 	u32 driver_version;
 	u16 usecstocount;
+	int lane;
+	u8 addr[ETH_ALEN];
+	u8 perm_addr[ETH_ALEN];
 };
 
 struct mucse {
@@ -123,4 +142,7 @@ struct rnpgbe_info {
 #define PCI_DEVICE_ID_N500_DUAL_PORT 0x8318
 #define PCI_DEVICE_ID_N210 0x8208
 #define PCI_DEVICE_ID_N210L 0x820a
+
+#define dma_wr32(dma, reg, val) writel((val), (dma)->dma_base_addr + (reg))
+#define dma_rd32(dma, reg) readl((dma)->dma_base_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 16d0a76114b5..3eaa0257f3bb 100644
--- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_chip.c
+++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_chip.c
@@ -2,10 +2,82 @@
 /* Copyright(c) 2020 - 2025 Mucse Corporation. */
 
 #include <linux/string.h>
+#include <linux/etherdevice.h>
 
 #include "rnpgbe.h"
 #include "rnpgbe_hw.h"
 #include "rnpgbe_mbx.h"
+#include "rnpgbe_mbx_fw.h"
+
+/**
+ * rnpgbe_get_permanent_mac - Get permanent mac
+ * @hw: hw information structure
+ * @mac_addr: pointer to store mac
+ *
+ * rnpgbe_get_permanent_mac tries to get mac from hw.
+ * It use eth_random_addr if failed.
+ **/
+static void rnpgbe_get_permanent_mac(struct mucse_hw *hw,
+				     u8 *mac_addr)
+{
+	if (mucse_fw_get_macaddr(hw, hw->pfvfnum, mac_addr, hw->lane)) {
+		eth_random_addr(mac_addr);
+	} else {
+		if (!is_valid_ether_addr(mac_addr))
+			eth_random_addr(mac_addr);
+	}
+
+	hw->flags |= M_FLAGS_INIT_MAC_ADDRESS;
+}
+
+/**
+ * rnpgbe_reset_hw_ops - Do a hardware reset
+ * @hw: hw information structure
+ *
+ * rnpgbe_reset_hw_ops calls fw to do a hardware
+ * reset, and cleans some regs to default.
+ *
+ * @return: 0 on success, negative on failure
+ **/
+static int rnpgbe_reset_hw_ops(struct mucse_hw *hw)
+{
+	struct mucse_dma_info *dma = &hw->dma;
+	int err;
+
+	dma_wr32(dma, RNPGBE_DMA_AXI_EN, 0);
+	err = mucse_mbx_fw_reset_phy(hw);
+	if (err)
+		return err;
+	/* Store the permanent mac address */
+	if (!(hw->flags & M_FLAGS_INIT_MAC_ADDRESS)) {
+		rnpgbe_get_permanent_mac(hw, hw->perm_addr);
+		memcpy(hw->addr, hw->perm_addr, ETH_ALEN);
+	}
+
+	return 0;
+}
+
+/**
+ * rnpgbe_driver_status_hw_ops - Echo driver status to hw
+ * @hw: hw information structure
+ * @enable: true or false status
+ * @mode: status mode
+ **/
+static void rnpgbe_driver_status_hw_ops(struct mucse_hw *hw,
+					bool enable,
+					int mode)
+{
+	switch (mode) {
+	case mucse_driver_insmod:
+		mucse_mbx_ifinsmod(hw, enable);
+		break;
+	}
+}
+
+static const struct mucse_hw_operations rnpgbe_hw_ops = {
+	.reset_hw = &rnpgbe_reset_hw_ops,
+	.driver_status = &rnpgbe_driver_status_hw_ops,
+};
 
 /**
  * rnpgbe_init_common - Setup common attribute
@@ -28,6 +100,7 @@ static void rnpgbe_init_common(struct mucse_hw *hw)
 	mac->back = hw;
 
 	hw->mbx.ops = &mucse_mbx_ops_generic;
+	hw->ops = &rnpgbe_hw_ops;
 }
 
 /**
diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h
index aee037e3219d..4e07328ccf82 100644
--- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h
+++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h
@@ -9,6 +9,7 @@
 #define RNPGBE_ETH_BASE 0x10000
 /**************** DMA Registers ****************************/
 #define RNPGBE_DMA_DUMY 0x000c
+#define RNPGBE_DMA_AXI_EN 0x0010
 /**************** CHIP Resource ****************************/
 #define RNPGBE_MAX_QUEUES 8
 #endif /* _RNPGBE_HW_H */
diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c
index c151995309f8..e0a08fa5b297 100644
--- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c
+++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c
@@ -8,6 +8,7 @@
 #include <linux/etherdevice.h>
 
 #include "rnpgbe.h"
+#include "rnpgbe_mbx_fw.h"
 
 static const char rnpgbe_driver_name[] = "rnpgbe";
 static const struct rnpgbe_info *rnpgbe_info_tbl[] = {
@@ -34,6 +35,54 @@ static struct pci_device_id rnpgbe_pci_tbl[] = {
 	{0, },
 };
 
+/**
+ * rnpgbe_open - Called when a network interface is made active
+ * @netdev: network interface device structure
+ *
+ * The open entry point is called when a network interface is made
+ * active by the system (IFF_UP).
+ *
+ * @return: 0 on success, negative value on failure
+ **/
+static int rnpgbe_open(struct net_device *netdev)
+{
+	return 0;
+}
+
+/**
+ * rnpgbe_close - Disables a network interface
+ * @netdev: network interface device structure
+ *
+ * The close entry point is called when an interface is de-activated
+ * by the OS.
+ *
+ * @return: 0, this is not allowed to fail
+ **/
+static int rnpgbe_close(struct net_device *netdev)
+{
+	return 0;
+}
+
+/**
+ * rnpgbe_xmit_frame - Send a skb to driver
+ * @skb: skb structure to be sent
+ * @netdev: network interface device structure
+ *
+ * @return: NETDEV_TX_OK or NETDEV_TX_BUSY
+ **/
+static netdev_tx_t rnpgbe_xmit_frame(struct sk_buff *skb,
+				     struct net_device *netdev)
+{
+		dev_kfree_skb_any(skb);
+		return NETDEV_TX_OK;
+}
+
+static const struct net_device_ops rnpgbe_netdev_ops = {
+	.ndo_open = rnpgbe_open,
+	.ndo_stop = rnpgbe_close,
+	.ndo_start_xmit = rnpgbe_xmit_frame,
+};
+
 /**
  * rnpgbe_add_adapter - Add netdev for this pci_dev
  * @pdev: PCI device information structure
@@ -106,6 +155,29 @@ static int rnpgbe_add_adapter(struct pci_dev *pdev,
 	hw->dma.dma_version = dma_version;
 	hw->driver_version = 0x0002040f;
 	info->init(hw);
+	hw->mbx.ops->init_params(hw);
+	/* echo fw driver insmod to control hw */
+	hw->ops->driver_status(hw, true, mucse_driver_insmod);
+	err = mucse_mbx_get_capability(hw);
+	if (err) {
+		dev_err(&pdev->dev,
+			"mucse_mbx_get_capability failed! %d\n",
+			err);
+		goto err_free_net;
+	}
+	netdev->netdev_ops = &rnpgbe_netdev_ops;
+	netdev->watchdog_timeo = 5 * HZ;
+	err = hw->ops->reset_hw(hw);
+	if (err) {
+		dev_err(&pdev->dev, "Hw reset failed %d\n", err);
+		goto err_free_net;
+	}
+	eth_hw_addr_set(netdev, hw->perm_addr);
+	memcpy(netdev->perm_addr, hw->perm_addr, netdev->addr_len);
+	ether_addr_copy(hw->addr, hw->perm_addr);
+	err = register_netdev(netdev);
+	if (err)
+		goto err_free_net;
 	return 0;
 
 err_free_net:
@@ -170,12 +242,16 @@ static int rnpgbe_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 static void rnpgbe_rm_adapter(struct pci_dev *pdev)
 {
 	struct mucse *mucse = pci_get_drvdata(pdev);
+	struct mucse_hw *hw = &mucse->hw;
 	struct net_device *netdev;
 
 	if (!mucse)
 		return;
 	netdev = mucse->netdev;
+	if (netdev->reg_state == NETREG_REGISTERED)
+		unregister_netdev(netdev);
 	mucse->netdev = NULL;
+	hw->ops->driver_status(hw, false, mucse_driver_insmod);
 	free_netdev(netdev);
 }
 
-- 
2.25.1

Re: [PATCH v3 5/5] net: rnpgbe: Add register_netdev

[Vadim Fedorenko]

On 12/08/2025 10:39, Dong Yibo wrote:
> Initialize get mac from hw, register the netdev.
> 
> Signed-off-by: Dong Yibo <dong100@mucse.com>
> ---
>   drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h    | 22 ++++++
>   .../net/ethernet/mucse/rnpgbe/rnpgbe_chip.c   | 73 ++++++++++++++++++
>   drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h |  1 +
>   .../net/ethernet/mucse/rnpgbe/rnpgbe_main.c   | 76 +++++++++++++++++++
>   4 files changed, 172 insertions(+)
> 
> diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h
> index 6cb14b79cbfe..644b8c85c29d 100644
> --- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h
> +++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h
> @@ -6,6 +6,7 @@
>   
>   #include <linux/types.h>
>   #include <linux/mutex.h>
> +#include <linux/netdevice.h>
>   
>   extern const struct rnpgbe_info rnpgbe_n500_info;
>   extern const struct rnpgbe_info rnpgbe_n210_info;
> @@ -86,6 +87,18 @@ struct mucse_mbx_info {
>   	u32 fw2pf_mbox_vec;
>   };
>   
> +struct mucse_hw_operations {
> +	int (*init_hw)(struct mucse_hw *hw);
> +	int (*reset_hw)(struct mucse_hw *hw);
> +	void (*start_hw)(struct mucse_hw *hw);
> +	void (*init_rx_addrs)(struct mucse_hw *hw);
> +	void (*driver_status)(struct mucse_hw *hw, bool enable, int mode);
> +};
> +
> +enum {
> +	mucse_driver_insmod,
> +};
> +
>   struct mucse_hw {
>   	void *back;
>   	u8 pfvfnum;
> @@ -96,12 +109,18 @@ struct mucse_hw {
>   	u32 axi_mhz;
>   	u32 bd_uid;
>   	enum rnpgbe_hw_type hw_type;
> +	const struct mucse_hw_operations *ops;
>   	struct mucse_dma_info dma;
>   	struct mucse_eth_info eth;
>   	struct mucse_mac_info mac;
>   	struct mucse_mbx_info mbx;
> +	u32 flags;
> +#define M_FLAGS_INIT_MAC_ADDRESS BIT(0)
>   	u32 driver_version;
>   	u16 usecstocount;
> +	int lane;
> +	u8 addr[ETH_ALEN];
> +	u8 perm_addr[ETH_ALEN];

why do you need both addresses if you have this info already in netdev?

>   };
>   
>   struct mucse {
> @@ -123,4 +142,7 @@ struct rnpgbe_info {
>   #define PCI_DEVICE_ID_N500_DUAL_PORT 0x8318
>   #define PCI_DEVICE_ID_N210 0x8208
>   #define PCI_DEVICE_ID_N210L 0x820a
> +
> +#define dma_wr32(dma, reg, val) writel((val), (dma)->dma_base_addr + (reg))
> +#define dma_rd32(dma, reg) readl((dma)->dma_base_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 16d0a76114b5..3eaa0257f3bb 100644
> --- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_chip.c
> +++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_chip.c
> @@ -2,10 +2,82 @@
>   /* Copyright(c) 2020 - 2025 Mucse Corporation. */
>   
>   #include <linux/string.h>
> +#include <linux/etherdevice.h>
>   
>   #include "rnpgbe.h"
>   #include "rnpgbe_hw.h"
>   #include "rnpgbe_mbx.h"
> +#include "rnpgbe_mbx_fw.h"
> +
> +/**
> + * rnpgbe_get_permanent_mac - Get permanent mac
> + * @hw: hw information structure
> + * @mac_addr: pointer to store mac
> + *
> + * rnpgbe_get_permanent_mac tries to get mac from hw.
> + * It use eth_random_addr if failed.
> + **/
> +static void rnpgbe_get_permanent_mac(struct mucse_hw *hw,
> +				     u8 *mac_addr)
> +{
> +	if (mucse_fw_get_macaddr(hw, hw->pfvfnum, mac_addr, hw->lane)) {
> +		eth_random_addr(mac_addr);
> +	} else {
> +		if (!is_valid_ether_addr(mac_addr))
> +			eth_random_addr(mac_addr);
> +	}

well, this can be done in one if() statement using logical "or"

> +
> +	hw->flags |= M_FLAGS_INIT_MAC_ADDRESS;
> +}
> +
> +/**
> + * rnpgbe_reset_hw_ops - Do a hardware reset
> + * @hw: hw information structure
> + *
> + * rnpgbe_reset_hw_ops calls fw to do a hardware
> + * reset, and cleans some regs to default.
> + *
> + * @return: 0 on success, negative on failure
> + **/
> +static int rnpgbe_reset_hw_ops(struct mucse_hw *hw)
> +{
> +	struct mucse_dma_info *dma = &hw->dma;
> +	int err;
> +
> +	dma_wr32(dma, RNPGBE_DMA_AXI_EN, 0);
> +	err = mucse_mbx_fw_reset_phy(hw);
> +	if (err)
> +		return err;
> +	/* Store the permanent mac address */
> +	if (!(hw->flags & M_FLAGS_INIT_MAC_ADDRESS)) {
> +		rnpgbe_get_permanent_mac(hw, hw->perm_addr);
> +		memcpy(hw->addr, hw->perm_addr, ETH_ALEN);
> +	}
> +
> +	return 0;
> +}
> +
> +/**
> + * rnpgbe_driver_status_hw_ops - Echo driver status to hw
> + * @hw: hw information structure
> + * @enable: true or false status
> + * @mode: status mode
> + **/
> +static void rnpgbe_driver_status_hw_ops(struct mucse_hw *hw,
> +					bool enable,
> +					int mode)
> +{
> +	switch (mode) {
> +	case mucse_driver_insmod:
> +		mucse_mbx_ifinsmod(hw, enable);
> +		break;
> +	}
> +}
> +
> +static const struct mucse_hw_operations rnpgbe_hw_ops = {
> +	.reset_hw = &rnpgbe_reset_hw_ops,
> +	.driver_status = &rnpgbe_driver_status_hw_ops,
> +};
>   
>   /**
>    * rnpgbe_init_common - Setup common attribute
> @@ -28,6 +100,7 @@ static void rnpgbe_init_common(struct mucse_hw *hw)
>   	mac->back = hw;
>   
>   	hw->mbx.ops = &mucse_mbx_ops_generic;
> +	hw->ops = &rnpgbe_hw_ops;
>   }
>   
>   /**
> diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h
> index aee037e3219d..4e07328ccf82 100644
> --- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h
> +++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h
> @@ -9,6 +9,7 @@
>   #define RNPGBE_ETH_BASE 0x10000
>   /**************** DMA Registers ****************************/
>   #define RNPGBE_DMA_DUMY 0x000c
> +#define RNPGBE_DMA_AXI_EN 0x0010
>   /**************** CHIP Resource ****************************/
>   #define RNPGBE_MAX_QUEUES 8
>   #endif /* _RNPGBE_HW_H */
> diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c
> index c151995309f8..e0a08fa5b297 100644
> --- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c
> +++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c
> @@ -8,6 +8,7 @@
>   #include <linux/etherdevice.h>
>   
>   #include "rnpgbe.h"
> +#include "rnpgbe_mbx_fw.h"
>   
>   static const char rnpgbe_driver_name[] = "rnpgbe";
>   static const struct rnpgbe_info *rnpgbe_info_tbl[] = {
> @@ -34,6 +35,54 @@ static struct pci_device_id rnpgbe_pci_tbl[] = {
>   	{0, },
>   };
>   
> +/**
> + * rnpgbe_open - Called when a network interface is made active
> + * @netdev: network interface device structure
> + *
> + * The open entry point is called when a network interface is made
> + * active by the system (IFF_UP).
> + *
> + * @return: 0 on success, negative value on failure
> + **/
> +static int rnpgbe_open(struct net_device *netdev)
> +{
> +	return 0;
> +}
> +
> +/**
> + * rnpgbe_close - Disables a network interface
> + * @netdev: network interface device structure
> + *
> + * The close entry point is called when an interface is de-activated
> + * by the OS.
> + *
> + * @return: 0, this is not allowed to fail
> + **/
> +static int rnpgbe_close(struct net_device *netdev)
> +{
> +	return 0;
> +}
> +
> +/**
> + * rnpgbe_xmit_frame - Send a skb to driver
> + * @skb: skb structure to be sent
> + * @netdev: network interface device structure
> + *
> + * @return: NETDEV_TX_OK or NETDEV_TX_BUSY
> + **/
> +static netdev_tx_t rnpgbe_xmit_frame(struct sk_buff *skb,
> +				     struct net_device *netdev)
> +{
> +		dev_kfree_skb_any(skb);
> +		return NETDEV_TX_OK;
> +}
> +
> +static const struct net_device_ops rnpgbe_netdev_ops = {
> +	.ndo_open = rnpgbe_open,
> +	.ndo_stop = rnpgbe_close,
> +	.ndo_start_xmit = rnpgbe_xmit_frame,
> +};
> +
>   /**
>    * rnpgbe_add_adapter - Add netdev for this pci_dev
>    * @pdev: PCI device information structure
> @@ -106,6 +155,29 @@ static int rnpgbe_add_adapter(struct pci_dev *pdev,
>   	hw->dma.dma_version = dma_version;
>   	hw->driver_version = 0x0002040f;
>   	info->init(hw);
> +	hw->mbx.ops->init_params(hw);
> +	/* echo fw driver insmod to control hw */
> +	hw->ops->driver_status(hw, true, mucse_driver_insmod);
> +	err = mucse_mbx_get_capability(hw);
> +	if (err) {
> +		dev_err(&pdev->dev,
> +			"mucse_mbx_get_capability failed! %d\n",
> +			err);
> +		goto err_free_net;
> +	}
> +	netdev->netdev_ops = &rnpgbe_netdev_ops;
> +	netdev->watchdog_timeo = 5 * HZ;
> +	err = hw->ops->reset_hw(hw);
> +	if (err) {
> +		dev_err(&pdev->dev, "Hw reset failed %d\n", err);
> +		goto err_free_net;
> +	}
> +	eth_hw_addr_set(netdev, hw->perm_addr);
> +	memcpy(netdev->perm_addr, hw->perm_addr, netdev->addr_len);

the comment from register_netdevice() says:

	/* If the device has permanent device address, driver should
	 * set dev_addr and also addr_assign_type should be set to
	 * NET_ADDR_PERM (default value).
	 */

dev_addr is set by eth_hw_addr_set, perm_addr will be set by
register_netdev(), no need to manually copy it.

> +	ether_addr_copy(hw->addr, hw->perm_addr);

your init() function has the same copy operation...

> +	err = register_netdev(netdev);
> +	if (err)
> +		goto err_free_net;
>   	return 0;
>   
>   err_free_net:
> @@ -170,12 +242,16 @@ static int rnpgbe_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>   static void rnpgbe_rm_adapter(struct pci_dev *pdev)
>   {
>   	struct mucse *mucse = pci_get_drvdata(pdev);
> +	struct mucse_hw *hw = &mucse->hw;
>   	struct net_device *netdev;
>   
>   	if (!mucse)
>   		return;
>   	netdev = mucse->netdev;
> +	if (netdev->reg_state == NETREG_REGISTERED)
> +		unregister_netdev(netdev);
>   	mucse->netdev = NULL;
> +	hw->ops->driver_status(hw, false, mucse_driver_insmod);
>   	free_netdev(netdev);
>   }
>   

Re: [PATCH v3 1/5] net: rnpgbe: Add build support for rnpgbe

[Vadim Fedorenko]

On 12/08/2025 10:39, Dong Yibo wrote:
> Add build options and doc for mucse.
> Initialize pci device access for MUCSE devices.
> 
> Signed-off-by: Dong Yibo <dong100@mucse.com>
> ---

[...]

> +/**
> + * rnpgbe_probe - Device initialization routine
> + * @pdev: PCI device information struct
> + * @id: entry in rnpgbe_pci_tbl
> + *
> + * rnpgbe_probe initializes a PF adapter identified by a pci_dev
> + * structure.
> + *
> + * @return: 0 on success, negative on failure
> + **/
> +static int rnpgbe_probe(struct pci_dev *pdev, const struct pci_device_id *id)
> +{
> +	int err;
> +
> +	err = pci_enable_device_mem(pdev);
> +	if (err)
> +		return err;
> +
> +	err = dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(56));
> +	if (err) {
> +		dev_err(&pdev->dev,
> +			"No usable DMA configuration, aborting %d\n", err);
> +		goto err_dma;
> +	}
> +
> +	err = pci_request_mem_regions(pdev, rnpgbe_driver_name);
> +	if (err) {
> +		dev_err(&pdev->dev,
> +			"pci_request_selected_regions failed 0x%x\n", err);
> +		goto err_pci_req;
> +	}
> +
> +	pci_set_master(pdev);
> +	pci_save_state(pdev);
> +
> +	return 0;
> +err_dma:
> +err_pci_req:
> +	pci_disable_device(pdev);
> +	return err;
> +}

Why do you need 2 different labels pointing to the very same line? The
code is not changed through patchset, I see no reasons to have it like
this

Re: [PATCH v3 2/5] net: rnpgbe: Add n500/n210 chip support

[Vadim Fedorenko]

> +struct mucse_dma_info {
> +	void __iomem *dma_base_addr;
> +	void __iomem *dma_ring_addr;
> +	void *back;

it might be better to keep the type of back pointer and give it
a bit more meaningful name ...

> +	u32 dma_version;
> +};
> +
> +struct mucse_eth_info {
> +	void __iomem *eth_base_addr;
> +	void *back;

.. here ...

> +};
> +
> +struct mucse_mac_info {
> +	void __iomem *mac_addr;
> +	void *back;

and here...

> +};
> +
> +struct mucse_mbx_info {
> +	/* fw <--> pf mbx */
> +	u32 fw_pf_shm_base;
> +	u32 pf2fw_mbox_ctrl;
> +	u32 pf2fw_mbox_mask;
> +	u32 fw_pf_mbox_mask;
> +	u32 fw2pf_mbox_vec;
> +};
> +
> +struct mucse_hw {
> +	void *back;

you can also use container_of() as all these structures are embedded and
simple pointer math can give you proper result.

> +	void __iomem *hw_addr;
> +	void __iomem *ring_msix_base;
> +	struct pci_dev *pdev;
> +	enum rnpgbe_hw_type hw_type;
> +	struct mucse_dma_info dma;
> +	struct mucse_eth_info eth;
> +	struct mucse_mac_info mac;
> +	struct mucse_mbx_info mbx;
> +	u32 driver_version;
> +	u16 usecstocount;
> +};
> +
>   struct mucse {
>   	struct net_device *netdev;
>   	struct pci_dev *pdev;
> +	struct mucse_hw hw;
>   	u16 bd_number;
>   };
>   

[...]

> +/**
> + * rnpgbe_add_adapter - Add netdev for this pci_dev
> + * @pdev: PCI device information structure
> + * @info: chip info structure
> + *
> + * rnpgbe_add_adapter initializes a netdev for this pci_dev
> + * structure. Initializes Bar map, private structure, and a
> + * hardware reset occur.
> + *
> + * @return: 0 on success, negative on failure
> + **/
> +static int rnpgbe_add_adapter(struct pci_dev *pdev,
> +			      const struct rnpgbe_info *info)
> +{
> +	struct net_device *netdev;
> +	void __iomem *hw_addr;
> +	static int bd_number;

it's not clear from the patchset why do you need this static variable...

> +	struct mucse *mucse;
> +	struct mucse_hw *hw;
> +	u32 dma_version = 0;
> +	u32 queues;
> +	int err;
> +
> +	queues = info->total_queue_pair_cnts;
> +	netdev = alloc_etherdev_mq(sizeof(struct mucse), queues);
> +	if (!netdev)
> +		return -ENOMEM;
> +
> +	SET_NETDEV_DEV(netdev, &pdev->dev);
> +	mucse = netdev_priv(netdev);
> +	mucse->netdev = netdev;
> +	mucse->pdev = pdev;
> +	mucse->bd_number = bd_number++;

... but this code is racy by design

> +	pci_set_drvdata(pdev, mucse);
> +
> +	hw = &mucse->hw;
> +	hw->back = mucse;
> +	hw->hw_type = info->hw_type;
> +	hw->pdev = pdev;
> +

Re: [PATCH v3 3/5] net: rnpgbe: Add basic mbx ops support

[Vadim Fedorenko]

> + * mucse_read_mbx - Reads a message from the mailbox
> + * @hw: pointer to the HW structure
> + * @msg: the message buffer
> + * @size: length of buffer
> + *
> + * @return: 0 on success, negative on failure
> + **/
> +int mucse_read_mbx(struct mucse_hw *hw, u32 *msg, u16 size)
> +{
> +	struct mucse_mbx_info *mbx = &hw->mbx;
> +
> +	/* limit read size */
> +	min(size, mbx->size);

you have to store the result of min() as it's lost now

Re: [PATCH v3 4/5] net: rnpgbe: Add basic mbx_fw support

[Vadim Fedorenko]

On 12/08/2025 10:39, Dong Yibo wrote:
> Initialize basic mbx_fw ops, such as get_capability, reset phy
> and so on.
> 
> Signed-off-by: Dong Yibo <dong100@mucse.com>
> +static int mucse_fw_send_cmd_wait(struct mucse_hw *hw,
> +				  struct mbx_fw_cmd_req *req,
> +				  struct mbx_fw_cmd_reply *reply)
> +{
> +	int len = le16_to_cpu(req->datalen) + MBX_REQ_HDR_LEN;
> +	int retry_cnt = 3;
> +	int err;
> +
> +	err = mutex_lock_interruptible(&hw->mbx.lock);
> +	if (err)
> +		return err;
> +	err = hw->mbx.ops->write_posted(hw, (u32 *)req,
> +					L_WD(len));
 > +	if (err) {> +		mutex_unlock(&hw->mbx.lock);
> +		return err;
> +	}

it might look a bit cleaner if you add error label and have unlock code
once in the end of the function...

> +	do {
> +		err = hw->mbx.ops->read_posted(hw, (u32 *)reply,
> +					       L_WD(sizeof(*reply)));
> +		if (err) {
> +			mutex_unlock(&hw->mbx.lock);
> +			return err;
> +		}
> +	} while (--retry_cnt >= 0 && reply->opcode != req->opcode);
> +	mutex_unlock(&hw->mbx.lock);
> +	if (retry_cnt < 0 || reply->error_code)
> +		return -EIO;
> +	return 0;
> +}
> +
> +/**
> + * mucse_fw_get_capability - Get hw abilities from fw
> + * @hw: pointer to the HW structure
> + * @abil: pointer to the hw_abilities structure
> + *
> + * mucse_fw_get_capability tries to get hw abilities from
> + * hw.
> + *
> + * @return: 0 on success, negative on failure
> + **/
> +static int mucse_fw_get_capability(struct mucse_hw *hw,
> +				   struct hw_abilities *abil)
> +{
> +	struct mbx_fw_cmd_reply reply;
> +	struct mbx_fw_cmd_req req;
> +	int err;
> +
> +	memset(&req, 0, sizeof(req));
> +	memset(&reply, 0, sizeof(reply));

probably

	struct mbx_fw_cmd_reply reply = {};
	struct mbx_fw_cmd_req req = {};

will look better. in the other functions as well..

Re: [PATCH v3 1/5] net: rnpgbe: Add build support for rnpgbe

[Anwar, Md Danish]

On 8/12/2025 3:09 PM, Dong Yibo wrote:
> Add build options and doc for mucse.
> Initialize pci device access for MUCSE devices.
> 
> Signed-off-by: Dong Yibo <dong100@mucse.com>
> ---
>  .../device_drivers/ethernet/index.rst         |   1 +
>  .../device_drivers/ethernet/mucse/rnpgbe.rst  |  21 +++
>  MAINTAINERS                                   |   8 +
>  drivers/net/ethernet/Kconfig                  |   1 +
>  drivers/net/ethernet/Makefile                 |   1 +
>  drivers/net/ethernet/mucse/Kconfig            |  34 ++++
>  drivers/net/ethernet/mucse/Makefile           |   7 +
>  drivers/net/ethernet/mucse/rnpgbe/Makefile    |   8 +
>  drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h    |  25 +++
>  .../net/ethernet/mucse/rnpgbe/rnpgbe_main.c   | 161 ++++++++++++++++++
>  10 files changed, 267 insertions(+)
>  create mode 100644 Documentation/networking/device_drivers/ethernet/mucse/rnpgbe.rst
>  create mode 100644 drivers/net/ethernet/mucse/Kconfig
>  create mode 100644 drivers/net/ethernet/mucse/Makefile
>  create mode 100644 drivers/net/ethernet/mucse/rnpgbe/Makefile
>  create mode 100644 drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h
>  create mode 100644 drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c

[ ... ]

> + **/
> +static int __init rnpgbe_init_module(void)
> +{
> +	int ret;
> +
> +	ret = pci_register_driver(&rnpgbe_driver);
> +	if (ret)
> +		return ret;
> +
> +	return 0;
> +}

Unnecessary code - can be simplified to just `return
pci_register_driver(&rnpgbe_driver);`

> +
> +module_init(rnpgbe_init_module);
> +
> +/**
> + * rnpgbe_exit_module - Driver remove routine
> + *
> + * rnpgbe_exit_module is called when driver is removed
> + **/
> +static void __exit rnpgbe_exit_module(void)
> +{
> +	pci_unregister_driver(&rnpgbe_driver);
> +}
> +
> +module_exit(rnpgbe_exit_module);
> +
> +MODULE_DEVICE_TABLE(pci, rnpgbe_pci_tbl);
> +MODULE_AUTHOR("Mucse Corporation, <techsupport@mucse.com>");
> +MODULE_DESCRIPTION("Mucse(R) 1 Gigabit PCI Express Network Driver");
> +MODULE_LICENSE("GPL");

-- 
Thanks and Regards,
Md Danish Anwar

Re: [PATCH v3 2/5] net: rnpgbe: Add n500/n210 chip support

[Anwar, Md Danish]

On 8/12/2025 3:09 PM, Dong Yibo wrote:
> Initialize n500/n210 chip bar resource map and
> dma, eth, mbx ... info for future use.
> 
> Signed-off-by: Dong Yibo <dong100@mucse.com>
> ---
>  drivers/net/ethernet/mucse/rnpgbe/Makefile    |   3 +-
>  drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h    |  60 +++++++++
>  .../net/ethernet/mucse/rnpgbe/rnpgbe_chip.c   |  88 ++++++++++++++
>  drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h |  12 ++
>  .../net/ethernet/mucse/rnpgbe/rnpgbe_main.c   | 115 ++++++++++++++++++
>  5 files changed, 277 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/net/ethernet/mucse/rnpgbe/rnpgbe_chip.c
>  create mode 100644 drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h
> 
> diff --git a/drivers/net/ethernet/mucse/rnpgbe/Makefile b/drivers/net/ethernet/mucse/rnpgbe/Makefile
> index 9df536f0d04c..42c359f459d9 100644
> --- a/drivers/net/ethernet/mucse/rnpgbe/Makefile
> +++ b/drivers/net/ethernet/mucse/rnpgbe/Makefile
> @@ -5,4 +5,5 @@
>  #
>  
>  obj-$(CONFIG_MGBE) += rnpgbe.o
> -rnpgbe-objs := rnpgbe_main.o
> +rnpgbe-objs := rnpgbe_main.o\
> +	       rnpgbe_chip.o
> diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h
> index 23c84454e7c7..0dd3d3cb2a4d 100644
> --- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h
> +++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h
> @@ -4,18 +4,78 @@
>  #ifndef _RNPGBE_H
>  #define _RNPGBE_H
>  
> +#include <linux/types.h>
> +
> +extern const struct rnpgbe_info rnpgbe_n500_info;
> +extern const struct rnpgbe_info rnpgbe_n210_info;
> +extern const struct rnpgbe_info rnpgbe_n210L_info;
> +
>  enum rnpgbe_boards {
>  	board_n500,
>  	board_n210,
>  	board_n210L,
>  };
>  
> +enum rnpgbe_hw_type {
> +	rnpgbe_hw_n500 = 0,
> +	rnpgbe_hw_n210,
> +	rnpgbe_hw_n210L,
> +	rnpgbe_hw_unknow
> +};


The enum value name should be "rnpgbe_hw_unknown" not "rnpgbe_hw_unknow"
(missing 'n').

> +
> +struct mucse_dma_info {
> +	void __iomem *dma_base_addr;
> +	void __iomem *dma_ring_addr;
> +	void *back;
> +	u32 dma_version;
> +};
> +
> +struct mucse_eth_info {
> +	void __iomem *eth_base_addr;
> +	void *back;
> +};
> +
> +struct mucse_mac_info {
> +	void __iomem *mac_addr;
> +	void *back;
> +};
> +
> +struct mucse_mbx_info {
> +	/* fw <--> pf mbx */
> +	u32 fw_pf_shm_base;
> +	u32 pf2fw_mbox_ctrl;
> +	u32 pf2fw_mbox_mask;
> +	u32 fw_pf_mbox_mask;
> +	u32 fw2pf_mbox_vec;
> +};
> +
> +struct mucse_hw {
> +	void *back;
> +	void __iomem *hw_addr;
> +	void __iomem *ring_msix_base;
> +	struct pci_dev *pdev;
> +	enum rnpgbe_hw_type hw_type;
> +	struct mucse_dma_info dma;
> +	struct mucse_eth_info eth;
> +	struct mucse_mac_info mac;
> +	struct mucse_mbx_info mbx;
> +	u32 driver_version;
> +	u16 usecstocount;
> +};
> +
>  struct mucse {
>  	struct net_device *netdev;
>  	struct pci_dev *pdev;
> +	struct mucse_hw hw;
>  	u16 bd_number;
>  };
>  
> +struct rnpgbe_info {
> +	int total_queue_pair_cnts;
> +	enum rnpgbe_hw_type hw_type;
> +	void (*init)(struct mucse_hw *hw);
> +};
> +
>  /* Device IDs */
>  #define PCI_VENDOR_ID_MUCSE 0x8848
>  #define PCI_DEVICE_ID_N500_QUAD_PORT 0x8308
> diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_chip.c b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_chip.c
> new file mode 100644
> index 000000000000..20ec67c9391e
> --- /dev/null
> +++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_chip.c
> @@ -0,0 +1,88 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright(c) 2020 - 2025 Mucse Corporation. */
> +
> +#include "rnpgbe.h"
> +#include "rnpgbe_hw.h"
> +
> +/**
> + * rnpgbe_init_common - Setup common attribute
> + * @hw: hw information structure
> + **/
> +static void rnpgbe_init_common(struct mucse_hw *hw)
> +{
> +	struct mucse_dma_info *dma = &hw->dma;
> +	struct mucse_eth_info *eth = &hw->eth;
> +	struct mucse_mac_info *mac = &hw->mac;
> +
> +	dma->dma_base_addr = hw->hw_addr;
> +	dma->dma_ring_addr = hw->hw_addr + RNPGBE_RING_BASE;
> +	dma->back = hw;
> +
> +	eth->eth_base_addr = hw->hw_addr + RNPGBE_ETH_BASE;
> +	eth->back = hw;
> +
> +	mac->mac_addr = hw->hw_addr + RNPGBE_MAC_BASE;
> +	mac->back = hw;
> +}
> +
> +/**
> + * rnpgbe_init_n500 - Setup n500 hw info
> + * @hw: hw information structure
> + *
> + * rnpgbe_init_n500 initializes all private
> + * structure, such as dma, eth, mac and mbx base on
> + * hw->addr for n500
> + **/
> +static void rnpgbe_init_n500(struct mucse_hw *hw)
> +{
> +	struct mucse_mbx_info *mbx = &hw->mbx;
> +
> +	rnpgbe_init_common(hw);
> +
> +	mbx->fw2pf_mbox_vec = 0x28b00;
> +	mbx->fw_pf_shm_base = 0x2d000;
> +	mbx->pf2fw_mbox_ctrl = 0x2e000;
> +	mbx->fw_pf_mbox_mask = 0x2e200;
> +	hw->ring_msix_base = hw->hw_addr + 0x28700;
> +	hw->usecstocount = 125;
> +}
> +
> +/**
> + * rnpgbe_init_n210 - Setup n210 hw info
> + * @hw: hw information structure
> + *
> + * rnpgbe_init_n210 initializes all private
> + * structure, such as dma, eth, mac and mbx base on
> + * hw->addr for n210
> + **/
> +static void rnpgbe_init_n210(struct mucse_hw *hw)
> +{
> +	struct mucse_mbx_info *mbx = &hw->mbx;
> +
> +	rnpgbe_init_common(hw);
> +
> +	mbx->fw2pf_mbox_vec = 0x29400;
> +	mbx->fw_pf_shm_base = 0x2d900;
> +	mbx->pf2fw_mbox_ctrl = 0x2e900;
> +	mbx->fw_pf_mbox_mask = 0x2eb00;
> +	hw->ring_msix_base = hw->hw_addr + 0x29000;
> +	hw->usecstocount = 62;
> +}

I don't see pf2fw_mbox_mask getting initialized anywhere. Is that not
needed?

> +
> +const struct rnpgbe_info rnpgbe_n500_info = {


-- 
Thanks and Regards,
Md Danish Anwar

Re: [PATCH v3 3/5] net: rnpgbe: Add basic mbx ops support

[Anwar, Md Danish]

On 8/12/2025 3:09 PM, Dong Yibo wrote:
> Initialize basic mbx function.
> 
> Signed-off-by: Dong Yibo <dong100@mucse.com>
> ---
>  drivers/net/ethernet/mucse/rnpgbe/Makefile    |   3 +-
>  drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h    |  37 ++
>  .../net/ethernet/mucse/rnpgbe/rnpgbe_chip.c   |   5 +
>  drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h |   2 +
>  .../net/ethernet/mucse/rnpgbe/rnpgbe_mbx.c    | 443 ++++++++++++++++++
>  .../net/ethernet/mucse/rnpgbe/rnpgbe_mbx.h    |  31 ++
>  6 files changed, 520 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx.c
>  create mode 100644 drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx.h
> 
> diff --git a/drivers/net/ethernet/mucse/rnpgbe/Makefile b/drivers/net/ethernet/mucse/rnpgbe/Makefile
> index 42c359f459d9..5fc878ada4b1 100644
> --- a/drivers/net/ethernet/mucse/rnpgbe/Makefile
> +++ b/drivers/net/ethernet/mucse/rnpgbe/Makefile
> @@ -6,4 +6,5 @@
>  
>  obj-$(CONFIG_MGBE) += rnpgbe.o
>  rnpgbe-objs := rnpgbe_main.o\
> -	       rnpgbe_chip.o
> +	       rnpgbe_chip.o\
> +	       rnpgbe_mbx.o
> diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h
> index 0dd3d3cb2a4d..05830bb73d3e 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/mutex.h>
>  
>  extern const struct rnpgbe_info rnpgbe_n500_info;
>  extern const struct rnpgbe_info rnpgbe_n210_info;
> @@ -40,7 +41,43 @@ struct mucse_mac_info {
>  	void *back;
>  };
>  
> +struct mucse_hw;
> +
> +struct mucse_mbx_operations {
> +	void (*init_params)(struct mucse_hw *hw);
> +	int (*read)(struct mucse_hw *hw, u32 *msg,
> +		    u16 size);
> +	int (*write)(struct mucse_hw *hw, u32 *msg,
> +		     u16 size);
> +	int (*read_posted)(struct mucse_hw *hw, u32 *msg,
> +			   u16 size);
> +	int (*write_posted)(struct mucse_hw *hw, u32 *msg,
> +			    u16 size);
> +	int (*check_for_msg)(struct mucse_hw *hw);
> +	int (*check_for_ack)(struct mucse_hw *hw);
> +	void (*configure)(struct mucse_hw *hw, int num_vec,
> +			  bool enable);
> +};
> +
> +struct mucse_mbx_stats {
> +	u32 msgs_tx;
> +	u32 msgs_rx;
> +	u32 acks;
> +	u32 reqs;
> +	u32 rsts;
> +};
> +
>  struct mucse_mbx_info {
> +	const struct mucse_mbx_operations *ops;
> +	struct mucse_mbx_stats stats;
> +	u32 timeout;
> +	u32 usec_delay;
> +	u16 size;
> +	u16 fw_req;
> +	u16 fw_ack;
> +	/* lock for only one use mbx */
> +	struct mutex lock;
> +	bool irq_enabled;
>  	/* fw <--> pf mbx */
>  	u32 fw_pf_shm_base;
>  	u32 pf2fw_mbox_ctrl;
> diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_chip.c b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_chip.c
> index 20ec67c9391e..16d0a76114b5 100644
> --- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_chip.c
> +++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_chip.c
> @@ -1,8 +1,11 @@
>  // SPDX-License-Identifier: GPL-2.0
>  /* Copyright(c) 2020 - 2025 Mucse Corporation. */
>  
> +#include <linux/string.h>
> +
>  #include "rnpgbe.h"
>  #include "rnpgbe_hw.h"
> +#include "rnpgbe_mbx.h"
>  
>  /**
>   * rnpgbe_init_common - Setup common attribute
> @@ -23,6 +26,8 @@ static void rnpgbe_init_common(struct mucse_hw *hw)
>  
>  	mac->mac_addr = hw->hw_addr + RNPGBE_MAC_BASE;
>  	mac->back = hw;
> +
> +	hw->mbx.ops = &mucse_mbx_ops_generic;
>  }
>  
>  /**
> diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h
> index fc57258537cf..aee037e3219d 100644
> --- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h
> +++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h
> @@ -7,6 +7,8 @@
>  #define RNPGBE_RING_BASE 0x1000
>  #define RNPGBE_MAC_BASE 0x20000
>  #define RNPGBE_ETH_BASE 0x10000
> +/**************** DMA Registers ****************************/
> +#define RNPGBE_DMA_DUMY 0x000c
>  /**************** CHIP Resource ****************************/
>  #define RNPGBE_MAX_QUEUES 8
>  #endif /* _RNPGBE_HW_H */
> diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx.c b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx.c
> new file mode 100644
> index 000000000000..1195cf945ad1
> --- /dev/null
> +++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx.c
> @@ -0,0 +1,443 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright(c) 2022 - 2025 Mucse Corporation. */
> +
> +#include <linux/pci.h>
> +#include <linux/errno.h>
> +#include <linux/delay.h>
> +#include <linux/iopoll.h>
> +
> +#include "rnpgbe.h"
> +#include "rnpgbe_mbx.h"
> +#include "rnpgbe_hw.h"
> +
> +/**
> + * mucse_read_mbx - Reads a message from the mailbox
> + * @hw: pointer to the HW structure
> + * @msg: the message buffer
> + * @size: length of buffer
> + *
> + * @return: 0 on success, negative on failure
> + **/
> +int mucse_read_mbx(struct mucse_hw *hw, u32 *msg, u16 size)
> +{
> +	struct mucse_mbx_info *mbx = &hw->mbx;
> +
> +	/* limit read size */
> +	min(size, mbx->size);
> +	return mbx->ops->read(hw, msg, size);
> +}

What's the purpose of min() here if you are anyways passing size to read()?

The min() call needs to be assigned to size, e.g.: size = min(size,
mbx->size);

> +
> +/**
> + * mucse_write_mbx - Write a message to the mailbox
> + * @hw: pointer to the HW structure
> + * @msg: the message buffer
> + * @size: length of buffer
> + *
> + * @return: 0 on success, negative on failure
> + **/

> +
> +/**
> + * mucse_mbx_reset - Reset mbx info, sync info from regs
> + * @hw: pointer to the HW structure
> + *
> + * This function reset all mbx variables to default.
> + **/
> +static void mucse_mbx_reset(struct mucse_hw *hw)
> +{
> +	struct mucse_mbx_info *mbx = &hw->mbx;
> +	int v;
> +

Variable 'v' should be declared as u32 to match the register read.

> +	v = mbx_rd32(hw, FW2PF_COUNTER(mbx));
> +	hw->mbx.fw_req = v & GENMASK(15, 0);
> +	hw->mbx.fw_ack = (v >> 16) & GENMASK(15, 0);
> +	mbx_wr32(hw, PF2FW_MBOX_CTRL(mbx), 0);
> +	mbx_wr32(hw, FW_PF_MBOX_MASK(mbx), GENMASK(31, 16));
> +}
> +
> +/**
> + * mucse_mbx_configure_pf - Configure mbx to use nr_vec interrupt
> + * @hw: pointer to the HW structure
> + * @nr_vec: vector number for mbx
> + * @enable: TRUE for enable, FALSE for disable
> + *
> + * This function configure mbx to use interrupt nr_vec.
> + **/
> +static void mucse_mbx_configure_pf(struct mucse_hw *hw, int nr_vec,
> +				   bool enable)
> +{
> +	struct mucse_mbx_info *mbx = &hw->mbx;
> +	u32 v;
> +
> +	if (enable) {
> +		v = mbx_rd32(hw, FW2PF_COUNTER(mbx));
> +		hw->mbx.fw_req = v & GENMASK(15, 0);
> +		hw->mbx.fw_ack = (v >> 16) & GENMASK(15, 0);
> +		mbx_wr32(hw, PF2FW_MBOX_CTRL(mbx), 0);
> +		mbx_wr32(hw, FW2PF_MBOX_VEC(mbx), nr_vec);
> +		mbx_wr32(hw, FW_PF_MBOX_MASK(mbx), GENMASK(31, 16));
> +	} else {
> +		mbx_wr32(hw, FW_PF_MBOX_MASK(mbx), 0xfffffffe);
> +		mbx_wr32(hw, PF2FW_MBOX_CTRL(mbx), 0);
> +		mbx_wr32(hw, RNPGBE_DMA_DUMY, 0);
> +	}
> +}
> +
> +/**
> + * mucse_init_mbx_params_pf - Set initial values for pf mailbox
> + * @hw: pointer to the HW structure
> + *
> + * Initializes the hw->mbx struct to correct values for pf mailbox
> + */
> +static void mucse_init_mbx_params_pf(struct mucse_hw *hw)
> +{
> +	struct mucse_mbx_info *mbx = &hw->mbx;
> +
> +	mbx->usec_delay = 100;
> +	mbx->timeout = (4 * 1000 * 1000) / mbx->usec_delay;

Use appropriate constants like USEC_PER_SEC instead of hardcoded values.

> +	mbx->stats.msgs_tx = 0;
> +	mbx->stats.msgs_rx = 0;


-- 
Thanks and Regards,
Md Danish Anwar

Re: [PATCH v3 1/5] net: rnpgbe: Add build support for rnpgbe

[Yibo Dong]

On Tue, Aug 12, 2025 at 04:37:52PM +0100, Vadim Fedorenko wrote:
> On 12/08/2025 10:39, Dong Yibo wrote:
> > Add build options and doc for mucse.
> > Initialize pci device access for MUCSE devices.
> > 
> > Signed-off-by: Dong Yibo <dong100@mucse.com>
> > ---
> 
> [...]
> 
> > +/**
> > + * rnpgbe_probe - Device initialization routine
> > + * @pdev: PCI device information struct
> > + * @id: entry in rnpgbe_pci_tbl
> > + *
> > + * rnpgbe_probe initializes a PF adapter identified by a pci_dev
> > + * structure.
> > + *
> > + * @return: 0 on success, negative on failure
> > + **/
> > +static int rnpgbe_probe(struct pci_dev *pdev, const struct pci_device_id *id)
> > +{
> > +	int err;
> > +
> > +	err = pci_enable_device_mem(pdev);
> > +	if (err)
> > +		return err;
> > +
> > +	err = dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(56));
> > +	if (err) {
> > +		dev_err(&pdev->dev,
> > +			"No usable DMA configuration, aborting %d\n", err);
> > +		goto err_dma;
> > +	}
> > +
> > +	err = pci_request_mem_regions(pdev, rnpgbe_driver_name);
> > +	if (err) {
> > +		dev_err(&pdev->dev,
> > +			"pci_request_selected_regions failed 0x%x\n", err);
> > +		goto err_pci_req;
> > +	}
> > +
> > +	pci_set_master(pdev);
> > +	pci_save_state(pdev);
> > +
> > +	return 0;
> > +err_dma:
> > +err_pci_req:
> > +	pci_disable_device(pdev);
> > +	return err;
> > +}
> 
> Why do you need 2 different labels pointing to the very same line? The
> code is not changed through patchset, I see no reasons to have it like
> this
> 
> 
> 
You are right, I should only 1 label here.

Thanks for your feedback.

Re: [PATCH v3 1/5] net: rnpgbe: Add build support for rnpgbe

[Yibo Dong]

On Tue, Aug 12, 2025 at 09:48:07PM +0530, Anwar, Md Danish wrote:
> On 8/12/2025 3:09 PM, Dong Yibo wrote:
> > Add build options and doc for mucse.
> > Initialize pci device access for MUCSE devices.
> > 
> > Signed-off-by: Dong Yibo <dong100@mucse.com>
> > ---
> >  .../device_drivers/ethernet/index.rst         |   1 +
> >  .../device_drivers/ethernet/mucse/rnpgbe.rst  |  21 +++
> >  MAINTAINERS                                   |   8 +
> >  drivers/net/ethernet/Kconfig                  |   1 +
> >  drivers/net/ethernet/Makefile                 |   1 +
> >  drivers/net/ethernet/mucse/Kconfig            |  34 ++++
> >  drivers/net/ethernet/mucse/Makefile           |   7 +
> >  drivers/net/ethernet/mucse/rnpgbe/Makefile    |   8 +
> >  drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h    |  25 +++
> >  .../net/ethernet/mucse/rnpgbe/rnpgbe_main.c   | 161 ++++++++++++++++++
> >  10 files changed, 267 insertions(+)
> >  create mode 100644 Documentation/networking/device_drivers/ethernet/mucse/rnpgbe.rst
> >  create mode 100644 drivers/net/ethernet/mucse/Kconfig
> >  create mode 100644 drivers/net/ethernet/mucse/Makefile
> >  create mode 100644 drivers/net/ethernet/mucse/rnpgbe/Makefile
> >  create mode 100644 drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h
> >  create mode 100644 drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c
> 
> [ ... ]
> 
> > + **/
> > +static int __init rnpgbe_init_module(void)
> > +{
> > +	int ret;
> > +
> > +	ret = pci_register_driver(&rnpgbe_driver);
> > +	if (ret)
> > +		return ret;
> > +
> > +	return 0;
> > +}
> 
> Unnecessary code - can be simplified to just `return
> pci_register_driver(&rnpgbe_driver);`
> 

Yes, but if I add some new codes which need some free after
pci_register_driver failed, the new patch will be like this:

-return pci_register_driver(&rnpgbe_driver);
+int ret:
+wq = create_singlethread_workqueue(rnpgbe_driver_name);
+ret = pci_register_driver(&rnpgbe_driver);
+if (ret) {
+	destroy_workqueue(wq);
+	return ret;
+}
+return 0;

Is this ok? Maybe not good to delete code for adding new feature?
This is what Andrew suggested not to do.

> > +
> > +module_init(rnpgbe_init_module);
> > +
> > +/**
> > + * rnpgbe_exit_module - Driver remove routine
> > + *
> > + * rnpgbe_exit_module is called when driver is removed
> > + **/
> > +static void __exit rnpgbe_exit_module(void)
> > +{
> > +	pci_unregister_driver(&rnpgbe_driver);
> > +}
> > +
> > +module_exit(rnpgbe_exit_module);
> > +
> > +MODULE_DEVICE_TABLE(pci, rnpgbe_pci_tbl);
> > +MODULE_AUTHOR("Mucse Corporation, <techsupport@mucse.com>");
> > +MODULE_DESCRIPTION("Mucse(R) 1 Gigabit PCI Express Network Driver");
> > +MODULE_LICENSE("GPL");
> 
> -- 
> Thanks and Regards,
> Md Danish Anwar
> 
> 

Thanks for your feedback.

Re: [PATCH v3 2/5] net: rnpgbe: Add n500/n210 chip support

[Yibo Dong]

On Tue, Aug 12, 2025 at 04:49:33PM +0100, Vadim Fedorenko wrote:
> > +struct mucse_dma_info {
> > +	void __iomem *dma_base_addr;
> > +	void __iomem *dma_ring_addr;
> > +	void *back;
> 
> it might be better to keep the type of back pointer and give it
> a bit more meaningful name ...
> 
> > +	u32 dma_version;
> > +};
> > +
> > +struct mucse_eth_info {
> > +	void __iomem *eth_base_addr;
> > +	void *back;
> 
> .. here ...
> 
> > +};
> > +
> > +struct mucse_mac_info {
> > +	void __iomem *mac_addr;
> > +	void *back;
> 
> and here...
> 
> > +};
> > +
> > +struct mucse_mbx_info {
> > +	/* fw <--> pf mbx */
> > +	u32 fw_pf_shm_base;
> > +	u32 pf2fw_mbox_ctrl;
> > +	u32 pf2fw_mbox_mask;
> > +	u32 fw_pf_mbox_mask;
> > +	u32 fw2pf_mbox_vec;
> > +};
> > +
> > +struct mucse_hw {
> > +	void *back;
> 
> you can also use container_of() as all these structures are embedded and
> simple pointer math can give you proper result.
> 

Got it, I will use container_of(), and remove the '*back' define.
Maybe eth to hw like this:
#define eth_to_hw(eth) container_of(eth, struct rnpgbe_hw, eth)
It is ok?

> > +	void __iomem *hw_addr;
> > +	void __iomem *ring_msix_base;
> > +	struct pci_dev *pdev;
> > +	enum rnpgbe_hw_type hw_type;
> > +	struct mucse_dma_info dma;
> > +	struct mucse_eth_info eth;
> > +	struct mucse_mac_info mac;
> > +	struct mucse_mbx_info mbx;
> > +	u32 driver_version;
> > +	u16 usecstocount;
> > +};
> > +
> >   struct mucse {
> >   	struct net_device *netdev;
> >   	struct pci_dev *pdev;
> > +	struct mucse_hw hw;
> >   	u16 bd_number;
> >   };
> 
> [...]
> 
> > +/**
> > + * rnpgbe_add_adapter - Add netdev for this pci_dev
> > + * @pdev: PCI device information structure
> > + * @info: chip info structure
> > + *
> > + * rnpgbe_add_adapter initializes a netdev for this pci_dev
> > + * structure. Initializes Bar map, private structure, and a
> > + * hardware reset occur.
> > + *
> > + * @return: 0 on success, negative on failure
> > + **/
> > +static int rnpgbe_add_adapter(struct pci_dev *pdev,
> > +			      const struct rnpgbe_info *info)
> > +{
> > +	struct net_device *netdev;
> > +	void __iomem *hw_addr;
> > +	static int bd_number;
> 
> it's not clear from the patchset why do you need this static variable...
> 

Ok, bd_number seems no usefull, I will remove it.

> > +	struct mucse *mucse;
> > +	struct mucse_hw *hw;
> > +	u32 dma_version = 0;
> > +	u32 queues;
> > +	int err;
> > +
> > +	queues = info->total_queue_pair_cnts;
> > +	netdev = alloc_etherdev_mq(sizeof(struct mucse), queues);
> > +	if (!netdev)
> > +		return -ENOMEM;
> > +
> > +	SET_NETDEV_DEV(netdev, &pdev->dev);
> > +	mucse = netdev_priv(netdev);
> > +	mucse->netdev = netdev;
> > +	mucse->pdev = pdev;
> > +	mucse->bd_number = bd_number++;
> 
> ... but this code is racy by design
> 
> > +	pci_set_drvdata(pdev, mucse);
> > +
> > +	hw = &mucse->hw;
> > +	hw->back = mucse;
> > +	hw->hw_type = info->hw_type;
> > +	hw->pdev = pdev;
> > +
> 

Thanks for your feedback.

Re: [PATCH v3 2/5] net: rnpgbe: Add n500/n210 chip support

[Yibo Dong]

On Tue, Aug 12, 2025 at 09:55:11PM +0530, Anwar, Md Danish wrote:
> On 8/12/2025 3:09 PM, Dong Yibo wrote:
> > Initialize n500/n210 chip bar resource map and
> > dma, eth, mbx ... info for future use.
> > 
> > Signed-off-by: Dong Yibo <dong100@mucse.com>
> > ---
> >  drivers/net/ethernet/mucse/rnpgbe/Makefile    |   3 +-
> >  drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h    |  60 +++++++++
> >  .../net/ethernet/mucse/rnpgbe/rnpgbe_chip.c   |  88 ++++++++++++++
> >  drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h |  12 ++
> >  .../net/ethernet/mucse/rnpgbe/rnpgbe_main.c   | 115 ++++++++++++++++++
> >  5 files changed, 277 insertions(+), 1 deletion(-)
> >  create mode 100644 drivers/net/ethernet/mucse/rnpgbe/rnpgbe_chip.c
> >  create mode 100644 drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h
> > 
> > diff --git a/drivers/net/ethernet/mucse/rnpgbe/Makefile b/drivers/net/ethernet/mucse/rnpgbe/Makefile
> > index 9df536f0d04c..42c359f459d9 100644
> > --- a/drivers/net/ethernet/mucse/rnpgbe/Makefile
> > +++ b/drivers/net/ethernet/mucse/rnpgbe/Makefile
> > @@ -5,4 +5,5 @@
> >  #
> >  
> >  obj-$(CONFIG_MGBE) += rnpgbe.o
> > -rnpgbe-objs := rnpgbe_main.o
> > +rnpgbe-objs := rnpgbe_main.o\
> > +	       rnpgbe_chip.o
> > diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h
> > index 23c84454e7c7..0dd3d3cb2a4d 100644
> > --- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h
> > +++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h
> > @@ -4,18 +4,78 @@
> >  #ifndef _RNPGBE_H
> >  #define _RNPGBE_H
> >  
> > +#include <linux/types.h>
> > +
> > +extern const struct rnpgbe_info rnpgbe_n500_info;
> > +extern const struct rnpgbe_info rnpgbe_n210_info;
> > +extern const struct rnpgbe_info rnpgbe_n210L_info;
> > +
> >  enum rnpgbe_boards {
> >  	board_n500,
> >  	board_n210,
> >  	board_n210L,
> >  };
> >  
> > +enum rnpgbe_hw_type {
> > +	rnpgbe_hw_n500 = 0,
> > +	rnpgbe_hw_n210,
> > +	rnpgbe_hw_n210L,
> > +	rnpgbe_hw_unknow
> > +};
> 
> 
> The enum value name should be "rnpgbe_hw_unknown" not "rnpgbe_hw_unknow"
> (missing 'n').
> 

Got it, I will fix this.

> > +
> > +struct mucse_dma_info {
> > +	void __iomem *dma_base_addr;
> > +	void __iomem *dma_ring_addr;
> > +	void *back;
> > +	u32 dma_version;
> > +};
> > +
> > +struct mucse_eth_info {
> > +	void __iomem *eth_base_addr;
> > +	void *back;
> > +};
> > +
> > +struct mucse_mac_info {
> > +	void __iomem *mac_addr;
> > +	void *back;
> > +};
> > +
> > +struct mucse_mbx_info {
> > +	/* fw <--> pf mbx */
> > +	u32 fw_pf_shm_base;
> > +	u32 pf2fw_mbox_ctrl;
> > +	u32 pf2fw_mbox_mask;
> > +	u32 fw_pf_mbox_mask;
> > +	u32 fw2pf_mbox_vec;
> > +};
> > +
> > +struct mucse_hw {
> > +	void *back;
> > +	void __iomem *hw_addr;
> > +	void __iomem *ring_msix_base;
> > +	struct pci_dev *pdev;
> > +	enum rnpgbe_hw_type hw_type;
> > +	struct mucse_dma_info dma;
> > +	struct mucse_eth_info eth;
> > +	struct mucse_mac_info mac;
> > +	struct mucse_mbx_info mbx;
> > +	u32 driver_version;
> > +	u16 usecstocount;
> > +};
> > +
> >  struct mucse {
> >  	struct net_device *netdev;
> >  	struct pci_dev *pdev;
> > +	struct mucse_hw hw;
> >  	u16 bd_number;
> >  };
> >  
> > +struct rnpgbe_info {
> > +	int total_queue_pair_cnts;
> > +	enum rnpgbe_hw_type hw_type;
> > +	void (*init)(struct mucse_hw *hw);
> > +};
> > +
> >  /* Device IDs */
> >  #define PCI_VENDOR_ID_MUCSE 0x8848
> >  #define PCI_DEVICE_ID_N500_QUAD_PORT 0x8308
> > diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_chip.c b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_chip.c
> > new file mode 100644
> > index 000000000000..20ec67c9391e
> > --- /dev/null
> > +++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_chip.c
> > @@ -0,0 +1,88 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/* Copyright(c) 2020 - 2025 Mucse Corporation. */
> > +
> > +#include "rnpgbe.h"
> > +#include "rnpgbe_hw.h"
> > +
> > +/**
> > + * rnpgbe_init_common - Setup common attribute
> > + * @hw: hw information structure
> > + **/
> > +static void rnpgbe_init_common(struct mucse_hw *hw)
> > +{
> > +	struct mucse_dma_info *dma = &hw->dma;
> > +	struct mucse_eth_info *eth = &hw->eth;
> > +	struct mucse_mac_info *mac = &hw->mac;
> > +
> > +	dma->dma_base_addr = hw->hw_addr;
> > +	dma->dma_ring_addr = hw->hw_addr + RNPGBE_RING_BASE;
> > +	dma->back = hw;
> > +
> > +	eth->eth_base_addr = hw->hw_addr + RNPGBE_ETH_BASE;
> > +	eth->back = hw;
> > +
> > +	mac->mac_addr = hw->hw_addr + RNPGBE_MAC_BASE;
> > +	mac->back = hw;
> > +}
> > +
> > +/**
> > + * rnpgbe_init_n500 - Setup n500 hw info
> > + * @hw: hw information structure
> > + *
> > + * rnpgbe_init_n500 initializes all private
> > + * structure, such as dma, eth, mac and mbx base on
> > + * hw->addr for n500
> > + **/
> > +static void rnpgbe_init_n500(struct mucse_hw *hw)
> > +{
> > +	struct mucse_mbx_info *mbx = &hw->mbx;
> > +
> > +	rnpgbe_init_common(hw);
> > +
> > +	mbx->fw2pf_mbox_vec = 0x28b00;
> > +	mbx->fw_pf_shm_base = 0x2d000;
> > +	mbx->pf2fw_mbox_ctrl = 0x2e000;
> > +	mbx->fw_pf_mbox_mask = 0x2e200;
> > +	hw->ring_msix_base = hw->hw_addr + 0x28700;
> > +	hw->usecstocount = 125;
> > +}
> > +
> > +/**
> > + * rnpgbe_init_n210 - Setup n210 hw info
> > + * @hw: hw information structure
> > + *
> > + * rnpgbe_init_n210 initializes all private
> > + * structure, such as dma, eth, mac and mbx base on
> > + * hw->addr for n210
> > + **/
> > +static void rnpgbe_init_n210(struct mucse_hw *hw)
> > +{
> > +	struct mucse_mbx_info *mbx = &hw->mbx;
> > +
> > +	rnpgbe_init_common(hw);
> > +
> > +	mbx->fw2pf_mbox_vec = 0x29400;
> > +	mbx->fw_pf_shm_base = 0x2d900;
> > +	mbx->pf2fw_mbox_ctrl = 0x2e900;
> > +	mbx->fw_pf_mbox_mask = 0x2eb00;
> > +	hw->ring_msix_base = hw->hw_addr + 0x29000;
> > +	hw->usecstocount = 62;
> > +}
> 
> I don't see pf2fw_mbox_mask getting initialized anywhere. Is that not
> needed?
> 

You are right, pf2fw_mbox_mask is not needed. I will delete it.

> > +
> > +const struct rnpgbe_info rnpgbe_n500_info = {
> 
> 
> -- 
> Thanks and Regards,
> Md Danish Anwar
> 
> 

Thanks for your feedback.

Re: [PATCH v3 3/5] net: rnpgbe: Add basic mbx ops support

[Yibo Dong]

On Tue, Aug 12, 2025 at 10:00:57PM +0530, Anwar, Md Danish wrote:
> On 8/12/2025 3:09 PM, Dong Yibo wrote:
> > Initialize basic mbx function.
> > 
> > Signed-off-by: Dong Yibo <dong100@mucse.com>
> > ---
> >  drivers/net/ethernet/mucse/rnpgbe/Makefile    |   3 +-
> >  drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h    |  37 ++
> >  .../net/ethernet/mucse/rnpgbe/rnpgbe_chip.c   |   5 +
> >  drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h |   2 +
> >  .../net/ethernet/mucse/rnpgbe/rnpgbe_mbx.c    | 443 ++++++++++++++++++
> >  .../net/ethernet/mucse/rnpgbe/rnpgbe_mbx.h    |  31 ++
> >  6 files changed, 520 insertions(+), 1 deletion(-)
> >  create mode 100644 drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx.c
> >  create mode 100644 drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx.h
> > 
> > diff --git a/drivers/net/ethernet/mucse/rnpgbe/Makefile b/drivers/net/ethernet/mucse/rnpgbe/Makefile
> > index 42c359f459d9..5fc878ada4b1 100644
> > --- a/drivers/net/ethernet/mucse/rnpgbe/Makefile
> > +++ b/drivers/net/ethernet/mucse/rnpgbe/Makefile
> > @@ -6,4 +6,5 @@
> >  
> >  obj-$(CONFIG_MGBE) += rnpgbe.o
> >  rnpgbe-objs := rnpgbe_main.o\
> > -	       rnpgbe_chip.o
> > +	       rnpgbe_chip.o\
> > +	       rnpgbe_mbx.o
> > diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h
> > index 0dd3d3cb2a4d..05830bb73d3e 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/mutex.h>
> >  
> >  extern const struct rnpgbe_info rnpgbe_n500_info;
> >  extern const struct rnpgbe_info rnpgbe_n210_info;
> > @@ -40,7 +41,43 @@ struct mucse_mac_info {
> >  	void *back;
> >  };
> >  
> > +struct mucse_hw;
> > +
> > +struct mucse_mbx_operations {
> > +	void (*init_params)(struct mucse_hw *hw);
> > +	int (*read)(struct mucse_hw *hw, u32 *msg,
> > +		    u16 size);
> > +	int (*write)(struct mucse_hw *hw, u32 *msg,
> > +		     u16 size);
> > +	int (*read_posted)(struct mucse_hw *hw, u32 *msg,
> > +			   u16 size);
> > +	int (*write_posted)(struct mucse_hw *hw, u32 *msg,
> > +			    u16 size);
> > +	int (*check_for_msg)(struct mucse_hw *hw);
> > +	int (*check_for_ack)(struct mucse_hw *hw);
> > +	void (*configure)(struct mucse_hw *hw, int num_vec,
> > +			  bool enable);
> > +};
> > +
> > +struct mucse_mbx_stats {
> > +	u32 msgs_tx;
> > +	u32 msgs_rx;
> > +	u32 acks;
> > +	u32 reqs;
> > +	u32 rsts;
> > +};
> > +
> >  struct mucse_mbx_info {
> > +	const struct mucse_mbx_operations *ops;
> > +	struct mucse_mbx_stats stats;
> > +	u32 timeout;
> > +	u32 usec_delay;
> > +	u16 size;
> > +	u16 fw_req;
> > +	u16 fw_ack;
> > +	/* lock for only one use mbx */
> > +	struct mutex lock;
> > +	bool irq_enabled;
> >  	/* fw <--> pf mbx */
> >  	u32 fw_pf_shm_base;
> >  	u32 pf2fw_mbox_ctrl;
> > diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_chip.c b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_chip.c
> > index 20ec67c9391e..16d0a76114b5 100644
> > --- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_chip.c
> > +++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_chip.c
> > @@ -1,8 +1,11 @@
> >  // SPDX-License-Identifier: GPL-2.0
> >  /* Copyright(c) 2020 - 2025 Mucse Corporation. */
> >  
> > +#include <linux/string.h>
> > +
> >  #include "rnpgbe.h"
> >  #include "rnpgbe_hw.h"
> > +#include "rnpgbe_mbx.h"
> >  
> >  /**
> >   * rnpgbe_init_common - Setup common attribute
> > @@ -23,6 +26,8 @@ static void rnpgbe_init_common(struct mucse_hw *hw)
> >  
> >  	mac->mac_addr = hw->hw_addr + RNPGBE_MAC_BASE;
> >  	mac->back = hw;
> > +
> > +	hw->mbx.ops = &mucse_mbx_ops_generic;
> >  }
> >  
> >  /**
> > diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h
> > index fc57258537cf..aee037e3219d 100644
> > --- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h
> > +++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h
> > @@ -7,6 +7,8 @@
> >  #define RNPGBE_RING_BASE 0x1000
> >  #define RNPGBE_MAC_BASE 0x20000
> >  #define RNPGBE_ETH_BASE 0x10000
> > +/**************** DMA Registers ****************************/
> > +#define RNPGBE_DMA_DUMY 0x000c
> >  /**************** CHIP Resource ****************************/
> >  #define RNPGBE_MAX_QUEUES 8
> >  #endif /* _RNPGBE_HW_H */
> > diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx.c b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx.c
> > new file mode 100644
> > index 000000000000..1195cf945ad1
> > --- /dev/null
> > +++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx.c
> > @@ -0,0 +1,443 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/* Copyright(c) 2022 - 2025 Mucse Corporation. */
> > +
> > +#include <linux/pci.h>
> > +#include <linux/errno.h>
> > +#include <linux/delay.h>
> > +#include <linux/iopoll.h>
> > +
> > +#include "rnpgbe.h"
> > +#include "rnpgbe_mbx.h"
> > +#include "rnpgbe_hw.h"
> > +
> > +/**
> > + * mucse_read_mbx - Reads a message from the mailbox
> > + * @hw: pointer to the HW structure
> > + * @msg: the message buffer
> > + * @size: length of buffer
> > + *
> > + * @return: 0 on success, negative on failure
> > + **/
> > +int mucse_read_mbx(struct mucse_hw *hw, u32 *msg, u16 size)
> > +{
> > +	struct mucse_mbx_info *mbx = &hw->mbx;
> > +
> > +	/* limit read size */
> > +	min(size, mbx->size);
> > +	return mbx->ops->read(hw, msg, size);
> > +}
> 
> What's the purpose of min() here if you are anyways passing size to read()?
> 
> The min() call needs to be assigned to size, e.g.: size = min(size,
> mbx->size);
> 

Yes, I will fix this error.

> > +
> > +/**
> > + * mucse_write_mbx - Write a message to the mailbox
> > + * @hw: pointer to the HW structure
> > + * @msg: the message buffer
> > + * @size: length of buffer
> > + *
> > + * @return: 0 on success, negative on failure
> > + **/
> 
> > +
> > +/**
> > + * mucse_mbx_reset - Reset mbx info, sync info from regs
> > + * @hw: pointer to the HW structure
> > + *
> > + * This function reset all mbx variables to default.
> > + **/
> > +static void mucse_mbx_reset(struct mucse_hw *hw)
> > +{
> > +	struct mucse_mbx_info *mbx = &hw->mbx;
> > +	int v;
> > +
> 
> Variable 'v' should be declared as u32 to match the register read.
> 

Got it, I will fix it.

> > +	v = mbx_rd32(hw, FW2PF_COUNTER(mbx));
> > +	hw->mbx.fw_req = v & GENMASK(15, 0);
> > +	hw->mbx.fw_ack = (v >> 16) & GENMASK(15, 0);
> > +	mbx_wr32(hw, PF2FW_MBOX_CTRL(mbx), 0);
> > +	mbx_wr32(hw, FW_PF_MBOX_MASK(mbx), GENMASK(31, 16));
> > +}
> > +
> > +/**
> > + * mucse_mbx_configure_pf - Configure mbx to use nr_vec interrupt
> > + * @hw: pointer to the HW structure
> > + * @nr_vec: vector number for mbx
> > + * @enable: TRUE for enable, FALSE for disable
> > + *
> > + * This function configure mbx to use interrupt nr_vec.
> > + **/
> > +static void mucse_mbx_configure_pf(struct mucse_hw *hw, int nr_vec,
> > +				   bool enable)
> > +{
> > +	struct mucse_mbx_info *mbx = &hw->mbx;
> > +	u32 v;
> > +
> > +	if (enable) {
> > +		v = mbx_rd32(hw, FW2PF_COUNTER(mbx));
> > +		hw->mbx.fw_req = v & GENMASK(15, 0);
> > +		hw->mbx.fw_ack = (v >> 16) & GENMASK(15, 0);
> > +		mbx_wr32(hw, PF2FW_MBOX_CTRL(mbx), 0);
> > +		mbx_wr32(hw, FW2PF_MBOX_VEC(mbx), nr_vec);
> > +		mbx_wr32(hw, FW_PF_MBOX_MASK(mbx), GENMASK(31, 16));
> > +	} else {
> > +		mbx_wr32(hw, FW_PF_MBOX_MASK(mbx), 0xfffffffe);
> > +		mbx_wr32(hw, PF2FW_MBOX_CTRL(mbx), 0);
> > +		mbx_wr32(hw, RNPGBE_DMA_DUMY, 0);
> > +	}
> > +}
> > +
> > +/**
> > + * mucse_init_mbx_params_pf - Set initial values for pf mailbox
> > + * @hw: pointer to the HW structure
> > + *
> > + * Initializes the hw->mbx struct to correct values for pf mailbox
> > + */
> > +static void mucse_init_mbx_params_pf(struct mucse_hw *hw)
> > +{
> > +	struct mucse_mbx_info *mbx = &hw->mbx;
> > +
> > +	mbx->usec_delay = 100;
> > +	mbx->timeout = (4 * 1000 * 1000) / mbx->usec_delay;
> 
> Use appropriate constants like USEC_PER_SEC instead of hardcoded values.
> 

Ok, I will update it.

> > +	mbx->stats.msgs_tx = 0;
> > +	mbx->stats.msgs_rx = 0;
> 
> 
> -- 
> Thanks and Regards,
> Md Danish Anwar
> 
> 

Thanks for your feedback.

Re: [PATCH v3 1/5] net: rnpgbe: Add build support for rnpgbe

[MD Danish Anwar]

On 13/08/25 12:14 pm, Yibo Dong wrote:
> On Tue, Aug 12, 2025 at 09:48:07PM +0530, Anwar, Md Danish wrote:
>> On 8/12/2025 3:09 PM, Dong Yibo wrote:
>>> Add build options and doc for mucse.
>>> Initialize pci device access for MUCSE devices.
>>>
>>> Signed-off-by: Dong Yibo <dong100@mucse.com>
>>> ---
>>>  .../device_drivers/ethernet/index.rst         |   1 +
>>>  .../device_drivers/ethernet/mucse/rnpgbe.rst  |  21 +++
>>>  MAINTAINERS                                   |   8 +
>>>  drivers/net/ethernet/Kconfig                  |   1 +
>>>  drivers/net/ethernet/Makefile                 |   1 +
>>>  drivers/net/ethernet/mucse/Kconfig            |  34 ++++
>>>  drivers/net/ethernet/mucse/Makefile           |   7 +
>>>  drivers/net/ethernet/mucse/rnpgbe/Makefile    |   8 +
>>>  drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h    |  25 +++
>>>  .../net/ethernet/mucse/rnpgbe/rnpgbe_main.c   | 161 ++++++++++++++++++
>>>  10 files changed, 267 insertions(+)
>>>  create mode 100644 Documentation/networking/device_drivers/ethernet/mucse/rnpgbe.rst
>>>  create mode 100644 drivers/net/ethernet/mucse/Kconfig
>>>  create mode 100644 drivers/net/ethernet/mucse/Makefile
>>>  create mode 100644 drivers/net/ethernet/mucse/rnpgbe/Makefile
>>>  create mode 100644 drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h
>>>  create mode 100644 drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c
>>
>> [ ... ]
>>
>>> + **/
>>> +static int __init rnpgbe_init_module(void)
>>> +{
>>> +	int ret;
>>> +
>>> +	ret = pci_register_driver(&rnpgbe_driver);
>>> +	if (ret)
>>> +		return ret;
>>> +
>>> +	return 0;
>>> +}
>>
>> Unnecessary code - can be simplified to just `return
>> pci_register_driver(&rnpgbe_driver);`
>>
> 
> Yes, but if I add some new codes which need some free after
> pci_register_driver failed, the new patch will be like this:
> 
> -return pci_register_driver(&rnpgbe_driver);
> +int ret:
> +wq = create_singlethread_workqueue(rnpgbe_driver_name);
> +ret = pci_register_driver(&rnpgbe_driver);
> +if (ret) {
> +	destroy_workqueue(wq);
> +	return ret;
> +}
> +return 0;
> 
> Is this ok? Maybe not good to delete code for adding new feature?
> This is what Andrew suggested not to do.
> 

In this patch series you are not modifying rnpgbe_init_module() again.
If you define a function as something in one patch and in later patches
you change it to something else - That is not encouraged, you should not
remove the code that you added in previous patches.

However here throughout your series you are not modifying this function.
Now the diff that you are showing, I don't know when you plan to post
that but as far as this series is concerned this diff is not part of the
series.

static int __init rnpgbe_init_module(void)
{
	int ret;

	ret = pci_register_driver(&rnpgbe_driver);
	if (ret)
		return ret;

	return 0;
}

This to me just seems unnecessary. You can just return
pci_register_driver() now and in future whenever you add other code you
can modify the function.

It would have  made sense for you to keep it as it is if some later
patch in your series would have modified it.

>>> +
>>> +module_init(rnpgbe_init_module);
>>> +
>>> +/**
>>> + * rnpgbe_exit_module - Driver remove routine
>>> + *
>>> + * rnpgbe_exit_module is called when driver is removed
>>> + **/
>>> +static void __exit rnpgbe_exit_module(void)
>>> +{
>>> +	pci_unregister_driver(&rnpgbe_driver);
>>> +}
>>> +
>>> +module_exit(rnpgbe_exit_module);
>>> +
>>> +MODULE_DEVICE_TABLE(pci, rnpgbe_pci_tbl);
>>> +MODULE_AUTHOR("Mucse Corporation, <techsupport@mucse.com>");
>>> +MODULE_DESCRIPTION("Mucse(R) 1 Gigabit PCI Express Network Driver");
>>> +MODULE_LICENSE("GPL");
>>
>> -- 
>> Thanks and Regards,
>> Md Danish Anwar
>>
>>
> 
> Thanks for your feedback.

-- 
Thanks and Regards,
Danish

Re: [PATCH v3 1/5] net: rnpgbe: Add build support for rnpgbe

[Yibo Dong]

On Wed, Aug 13, 2025 at 01:21:26PM +0530, MD Danish Anwar wrote:
> On 13/08/25 12:14 pm, Yibo Dong wrote:
> > On Tue, Aug 12, 2025 at 09:48:07PM +0530, Anwar, Md Danish wrote:
> >> On 8/12/2025 3:09 PM, Dong Yibo wrote:
> >>> Add build options and doc for mucse.
> >>> Initialize pci device access for MUCSE devices.
> >>>
> >>> Signed-off-by: Dong Yibo <dong100@mucse.com>
> >>> ---
> >>>  .../device_drivers/ethernet/index.rst         |   1 +
> >>>  .../device_drivers/ethernet/mucse/rnpgbe.rst  |  21 +++
> >>>  MAINTAINERS                                   |   8 +
> >>>  drivers/net/ethernet/Kconfig                  |   1 +
> >>>  drivers/net/ethernet/Makefile                 |   1 +
> >>>  drivers/net/ethernet/mucse/Kconfig            |  34 ++++
> >>>  drivers/net/ethernet/mucse/Makefile           |   7 +
> >>>  drivers/net/ethernet/mucse/rnpgbe/Makefile    |   8 +
> >>>  drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h    |  25 +++
> >>>  .../net/ethernet/mucse/rnpgbe/rnpgbe_main.c   | 161 ++++++++++++++++++
> >>>  10 files changed, 267 insertions(+)
> >>>  create mode 100644 Documentation/networking/device_drivers/ethernet/mucse/rnpgbe.rst
> >>>  create mode 100644 drivers/net/ethernet/mucse/Kconfig
> >>>  create mode 100644 drivers/net/ethernet/mucse/Makefile
> >>>  create mode 100644 drivers/net/ethernet/mucse/rnpgbe/Makefile
> >>>  create mode 100644 drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h
> >>>  create mode 100644 drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c
> >>
> >> [ ... ]
> >>
> >>> + **/
> >>> +static int __init rnpgbe_init_module(void)
> >>> +{
> >>> +	int ret;
> >>> +
> >>> +	ret = pci_register_driver(&rnpgbe_driver);
> >>> +	if (ret)
> >>> +		return ret;
> >>> +
> >>> +	return 0;
> >>> +}
> >>
> >> Unnecessary code - can be simplified to just `return
> >> pci_register_driver(&rnpgbe_driver);`
> >>
> > 
> > Yes, but if I add some new codes which need some free after
> > pci_register_driver failed, the new patch will be like this:
> > 
> > -return pci_register_driver(&rnpgbe_driver);
> > +int ret:
> > +wq = create_singlethread_workqueue(rnpgbe_driver_name);
> > +ret = pci_register_driver(&rnpgbe_driver);
> > +if (ret) {
> > +	destroy_workqueue(wq);
> > +	return ret;
> > +}
> > +return 0;
> > 
> > Is this ok? Maybe not good to delete code for adding new feature?
> > This is what Andrew suggested not to do.
> > 
> 
> In this patch series you are not modifying rnpgbe_init_module() again.
> If you define a function as something in one patch and in later patches
> you change it to something else - That is not encouraged, you should not
> remove the code that you added in previous patches.
> 
> However here throughout your series you are not modifying this function.
> Now the diff that you are showing, I don't know when you plan to post
> that but as far as this series is concerned this diff is not part of the
> series.
> 
> static int __init rnpgbe_init_module(void)
> {
> 	int ret;
> 
> 	ret = pci_register_driver(&rnpgbe_driver);
> 	if (ret)
> 		return ret;
> 
> 	return 0;
> }
> 
> This to me just seems unnecessary. You can just return
> pci_register_driver() now and in future whenever you add other code you
> can modify the function.
> 
> It would have  made sense for you to keep it as it is if some later
> patch in your series would have modified it.
> 

Ok, I got it, thanks. I will just return pci_register_driver().

> >>> +
> >>> +module_init(rnpgbe_init_module);
> >>> +
> >>> +/**
> >>> + * rnpgbe_exit_module - Driver remove routine
> >>> + *
> >>> + * rnpgbe_exit_module is called when driver is removed
> >>> + **/
> >>> +static void __exit rnpgbe_exit_module(void)
> >>> +{
> >>> +	pci_unregister_driver(&rnpgbe_driver);
> >>> +}
> >>> +
> >>> +module_exit(rnpgbe_exit_module);
> >>> +
> >>> +MODULE_DEVICE_TABLE(pci, rnpgbe_pci_tbl);
> >>> +MODULE_AUTHOR("Mucse Corporation, <techsupport@mucse.com>");
> >>> +MODULE_DESCRIPTION("Mucse(R) 1 Gigabit PCI Express Network Driver");
> >>> +MODULE_LICENSE("GPL");
> >>
> >> -- 
> >> Thanks and Regards,
> >> Md Danish Anwar
> >>
> >>
> > 
> > Thanks for your feedback.
> 
> -- 
> Thanks and Regards,
> Danish
> 
> 

Thanks for your feedback.

Re: [PATCH v3 4/5] net: rnpgbe: Add basic mbx_fw support

[Yibo Dong]

On Tue, Aug 12, 2025 at 05:14:15PM +0100, Vadim Fedorenko wrote:
> On 12/08/2025 10:39, Dong Yibo wrote:
> > Initialize basic mbx_fw ops, such as get_capability, reset phy
> > and so on.
> > 
> > Signed-off-by: Dong Yibo <dong100@mucse.com>
> > +static int mucse_fw_send_cmd_wait(struct mucse_hw *hw,
> > +				  struct mbx_fw_cmd_req *req,
> > +				  struct mbx_fw_cmd_reply *reply)
> > +{
> > +	int len = le16_to_cpu(req->datalen) + MBX_REQ_HDR_LEN;
> > +	int retry_cnt = 3;
> > +	int err;
> > +
> > +	err = mutex_lock_interruptible(&hw->mbx.lock);
> > +	if (err)
> > +		return err;
> > +	err = hw->mbx.ops->write_posted(hw, (u32 *)req,
> > +					L_WD(len));
> > +	if (err) {> +		mutex_unlock(&hw->mbx.lock);
> > +		return err;
> > +	}
> 
> it might look a bit cleaner if you add error label and have unlock code
> once in the end of the function...
> 

Ok, I will try to update this.

> > +	do {
> > +		err = hw->mbx.ops->read_posted(hw, (u32 *)reply,
> > +					       L_WD(sizeof(*reply)));
> > +		if (err) {
> > +			mutex_unlock(&hw->mbx.lock);
> > +			return err;
> > +		}
> > +	} while (--retry_cnt >= 0 && reply->opcode != req->opcode);
> > +	mutex_unlock(&hw->mbx.lock);
> > +	if (retry_cnt < 0 || reply->error_code)
> > +		return -EIO;
> > +	return 0;
> > +}
> > +
> > +/**
> > + * mucse_fw_get_capability - Get hw abilities from fw
> > + * @hw: pointer to the HW structure
> > + * @abil: pointer to the hw_abilities structure
> > + *
> > + * mucse_fw_get_capability tries to get hw abilities from
> > + * hw.
> > + *
> > + * @return: 0 on success, negative on failure
> > + **/
> > +static int mucse_fw_get_capability(struct mucse_hw *hw,
> > +				   struct hw_abilities *abil)
> > +{
> > +	struct mbx_fw_cmd_reply reply;
> > +	struct mbx_fw_cmd_req req;
> > +	int err;
> > +
> > +	memset(&req, 0, sizeof(req));
> > +	memset(&reply, 0, sizeof(reply));
> 
> probably
> 
> 	struct mbx_fw_cmd_reply reply = {};
> 	struct mbx_fw_cmd_req req = {};
> 
> will look better. in the other functions as well..
> 
> 
> 

Got it, I will update it.

Thanks for your feedback.

Re: [PATCH v3 4/5] net: rnpgbe: Add basic mbx_fw support

[MD Danish Anwar]

On 12/08/25 3:09 pm, Dong Yibo wrote:
> +/**
> + * mucse_fw_get_capability - Get hw abilities from fw
> + * @hw: pointer to the HW structure
> + * @abil: pointer to the hw_abilities structure
> + *
> + * mucse_fw_get_capability tries to get hw abilities from
> + * hw.
> + *
> + * @return: 0 on success, negative on failure
> + **/
> +static int mucse_fw_get_capability(struct mucse_hw *hw,
> +				   struct hw_abilities *abil)
> +{
> +	struct mbx_fw_cmd_reply reply;
> +	struct mbx_fw_cmd_req req;
> +	int err;
> +
> +	memset(&req, 0, sizeof(req));
> +	memset(&reply, 0, sizeof(reply));
> +	build_phy_abalities_req(&req, &req);

Typo in function name. You probably meant "build_phy_abilities_req".

> +	err = mucse_fw_send_cmd_wait(hw, &req, &reply);
> +	if (!err)
> +		memcpy(abil, &reply.hw_abilities, sizeof(*abil));
> +	return err;
> +}
> +
> +/**
> + * mucse_mbx_get_capability - Get hw abilities from fw
> + * @hw: pointer to the HW structure
> + *
> + * mucse_mbx_get_capability tries to some capabities from
> + * hw. Many retrys will do if it is failed.
> + *

Typo in comment: "tries to some capabities" should be "tries to get
capabilities"

> + * @return: 0 on success, negative on failure
> + **/
> +int mucse_mbx_get_capability(struct mucse_hw *hw)
> +{
> +	struct hw_abilities ability;
> +	int try_cnt = 3;
> +	int err;
> +
> +	memset(&ability, 0, sizeof(ability));
> +	while (try_cnt--) {
> +		err = mucse_fw_get_capability(hw, &ability);
> +		if (err)
> +			continue;
> +		hw->pfvfnum = le16_to_cpu(ability.pfnum);
> +		hw->fw_version = le32_to_cpu(ability.fw_version);
> +		hw->axi_mhz = le32_to_cpu(ability.axi_mhz);
> +		hw->bd_uid = le32_to_cpu(ability.bd_uid);
> +		return 0;
> +	}
> +	return err;
> +}


Missing initialization of err variable before the last return, which
could lead to undefined behavior if all attempts fail.

> +
> +/**
> + * mbx_cookie_zalloc - Alloc a cookie structure
> + * @priv_len: private length for this cookie
> + *


-- 
Thanks and Regards,
Danish

Re: [PATCH v3 5/5] net: rnpgbe: Add register_netdev

[MD Danish Anwar]

On 12/08/25 3:09 pm, Dong Yibo wrote:
> Initialize get mac from hw, register the netdev.
> 
> Signed-off-by: Dong Yibo <dong100@mucse.com>
> ---
>  drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h    | 22 ++++++
>  .../net/ethernet/mucse/rnpgbe/rnpgbe_chip.c   | 73 ++++++++++++++++++
>  drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h |  1 +
>  .../net/ethernet/mucse/rnpgbe/rnpgbe_main.c   | 76 +++++++++++++++++++
>  4 files changed, 172 insertions(+)
> 
> diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h
> index 6cb14b79cbfe..644b8c85c29d 100644
> --- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h
> +++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h
> @@ -6,6 +6,7 @@
>  
>  #include <linux/types.h>
>  #include <linux/mutex.h>
> +#include <linux/netdevice.h>
>  
>  extern const struct rnpgbe_info rnpgbe_n500_info;
>  extern const struct rnpgbe_info rnpgbe_n210_info;
> @@ -86,6 +87,18 @@ struct mucse_mbx_info {
>  	u32 fw2pf_mbox_vec;
>  };
>  
> +struct mucse_hw_operations {
> +	int (*init_hw)(struct mucse_hw *hw);
> +	int (*reset_hw)(struct mucse_hw *hw);
> +	void (*start_hw)(struct mucse_hw *hw);
> +	void (*init_rx_addrs)(struct mucse_hw *hw);
> +	void (*driver_status)(struct mucse_hw *hw, bool enable, int mode);
> +};

You define functions init_hw, start_hw, and init_rx_addrs in this
structure but they aren't implemented in this patch. Either implement
them or remove them if not needed yet.


> +
> +enum {
> +	mucse_driver_insmod,
> +};
> +
>  struct mucse_hw {
>  	void *back;
>  	u8 pfvfnum;
> @@ -96,12 +109,18 @@ struct mucse_hw {
>  	u32 axi_mhz;
>  	u32 bd_uid;
>  	enum rnpgbe_hw_type hw_type;
> +	const struct mucse_hw_operations *ops;
>  	struct mucse_dma_info dma;
>  	struct mucse_eth_info eth;
>  	struct mucse_mac_info mac;
>  	struct mucse_mbx_info mbx;
> +	u32 flags;
> +#define M_FLAGS_INIT_MAC_ADDRESS BIT(0)
>  	u32 driver_version;
>  	u16 usecstocount;
> +	int lane;
> +	u8 addr[ETH_ALEN];
> +	u8 perm_addr[ETH_ALEN];
>  };
>  
>  struct mucse {
> @@ -123,4 +142,7 @@ struct rnpgbe_info {
>  #define PCI_DEVICE_ID_N500_DUAL_PORT 0x8318
>  #define PCI_DEVICE_ID_N210 0x8208
>  #define PCI_DEVICE_ID_N210L 0x820a
> +
> +#define dma_wr32(dma, reg, val) writel((val), (dma)->dma_base_addr + (reg))
> +#define dma_rd32(dma, reg) readl((dma)->dma_base_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 16d0a76114b5..3eaa0257f3bb 100644
> --- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_chip.c
> +++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_chip.c
> @@ -2,10 +2,82 @@
>  /* Copyright(c) 2020 - 2025 Mucse Corporation. */
>  
>  #include <linux/string.h>
> +#include <linux/etherdevice.h>
>  
>  #include "rnpgbe.h"
>  #include "rnpgbe_hw.h"
>  #include "rnpgbe_mbx.h"
> +#include "rnpgbe_mbx_fw.h"
> +
> +/**
> + * rnpgbe_get_permanent_mac - Get permanent mac
> + * @hw: hw information structure
> + * @mac_addr: pointer to store mac
> + *
> + * rnpgbe_get_permanent_mac tries to get mac from hw.
> + * It use eth_random_addr if failed.
> + **/
> +static void rnpgbe_get_permanent_mac(struct mucse_hw *hw,
> +				     u8 *mac_addr)
> +{
> +	if (mucse_fw_get_macaddr(hw, hw->pfvfnum, mac_addr, hw->lane)) {
> +		eth_random_addr(mac_addr);
> +	} else {
> +		if (!is_valid_ether_addr(mac_addr))
> +			eth_random_addr(mac_addr);
> +	}
> +

The function should log a warning when falling back to a random MAC
address, especially in the second case where the hardware returned an
invalid MAC.

> +	hw->flags |= M_FLAGS_INIT_MAC_ADDRESS;
> +}
> +

> +/**
> + * rnpgbe_xmit_frame - Send a skb to driver
> + * @skb: skb structure to be sent
> + * @netdev: network interface device structure
> + *
> + * @return: NETDEV_TX_OK or NETDEV_TX_BUSY
> + **/
> +static netdev_tx_t rnpgbe_xmit_frame(struct sk_buff *skb,
> +				     struct net_device *netdev)
> +{
> +		dev_kfree_skb_any(skb);
> +		return NETDEV_TX_OK;
> +}

Extra indentation on these two lines. Also, the function just drops all
packets without any actual transmission. This should at least increment
the drop counter statistics.

> +
> +static const struct net_device_ops rnpgbe_netdev_ops = {
> +	.ndo_open = rnpgbe_open,
> +	.ndo_stop = rnpgbe_close,
> +	.ndo_start_xmit = rnpgbe_xmit_frame,
> +};


-- 
Thanks and Regards,
Danish

Re: [PATCH v3 5/5] net: rnpgbe: Add register_netdev

[Yibo Dong]

On Tue, Aug 12, 2025 at 04:32:00PM +0100, Vadim Fedorenko wrote:
> On 12/08/2025 10:39, Dong Yibo wrote:
> > Initialize get mac from hw, register the netdev.
> > 
> > Signed-off-by: Dong Yibo <dong100@mucse.com>
> > ---
> >   drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h    | 22 ++++++
> >   .../net/ethernet/mucse/rnpgbe/rnpgbe_chip.c   | 73 ++++++++++++++++++
> >   drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h |  1 +
> >   .../net/ethernet/mucse/rnpgbe/rnpgbe_main.c   | 76 +++++++++++++++++++
> >   4 files changed, 172 insertions(+)
> > 
> > diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h
> > index 6cb14b79cbfe..644b8c85c29d 100644
> > --- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h
> > +++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h
> > @@ -6,6 +6,7 @@
> >   #include <linux/types.h>
> >   #include <linux/mutex.h>
> > +#include <linux/netdevice.h>
> >   extern const struct rnpgbe_info rnpgbe_n500_info;
> >   extern const struct rnpgbe_info rnpgbe_n210_info;
> > @@ -86,6 +87,18 @@ struct mucse_mbx_info {
> >   	u32 fw2pf_mbox_vec;
> >   };
> > +struct mucse_hw_operations {
> > +	int (*init_hw)(struct mucse_hw *hw);
> > +	int (*reset_hw)(struct mucse_hw *hw);
> > +	void (*start_hw)(struct mucse_hw *hw);
> > +	void (*init_rx_addrs)(struct mucse_hw *hw);
> > +	void (*driver_status)(struct mucse_hw *hw, bool enable, int mode);
> > +};
> > +
> > +enum {
> > +	mucse_driver_insmod,
> > +};
> > +
> >   struct mucse_hw {
> >   	void *back;
> >   	u8 pfvfnum;
> > @@ -96,12 +109,18 @@ struct mucse_hw {
> >   	u32 axi_mhz;
> >   	u32 bd_uid;
> >   	enum rnpgbe_hw_type hw_type;
> > +	const struct mucse_hw_operations *ops;
> >   	struct mucse_dma_info dma;
> >   	struct mucse_eth_info eth;
> >   	struct mucse_mac_info mac;
> >   	struct mucse_mbx_info mbx;
> > +	u32 flags;
> > +#define M_FLAGS_INIT_MAC_ADDRESS BIT(0)
> >   	u32 driver_version;
> >   	u16 usecstocount;
> > +	int lane;
> > +	u8 addr[ETH_ALEN];
> > +	u8 perm_addr[ETH_ALEN];
> 
> why do you need both addresses if you have this info already in netdev?
> 

'perm_addr' is address from firmware (fixed, can't be changed by user).
'addr' is the current netdev address (It is Initialized the same with
'perm_addr', but can be changed by user)
Maybe I should add 'addr' in the patch which support ndo_set_mac_address?

> >   };
> >   struct mucse {
> > @@ -123,4 +142,7 @@ struct rnpgbe_info {
> >   #define PCI_DEVICE_ID_N500_DUAL_PORT 0x8318
> >   #define PCI_DEVICE_ID_N210 0x8208
> >   #define PCI_DEVICE_ID_N210L 0x820a
> > +
> > +#define dma_wr32(dma, reg, val) writel((val), (dma)->dma_base_addr + (reg))
> > +#define dma_rd32(dma, reg) readl((dma)->dma_base_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 16d0a76114b5..3eaa0257f3bb 100644
> > --- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_chip.c
> > +++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_chip.c
> > @@ -2,10 +2,82 @@
> >   /* Copyright(c) 2020 - 2025 Mucse Corporation. */
> >   #include <linux/string.h>
> > +#include <linux/etherdevice.h>
> >   #include "rnpgbe.h"
> >   #include "rnpgbe_hw.h"
> >   #include "rnpgbe_mbx.h"
> > +#include "rnpgbe_mbx_fw.h"
> > +
> > +/**
> > + * rnpgbe_get_permanent_mac - Get permanent mac
> > + * @hw: hw information structure
> > + * @mac_addr: pointer to store mac
> > + *
> > + * rnpgbe_get_permanent_mac tries to get mac from hw.
> > + * It use eth_random_addr if failed.
> > + **/
> > +static void rnpgbe_get_permanent_mac(struct mucse_hw *hw,
> > +				     u8 *mac_addr)
> > +{
> > +	if (mucse_fw_get_macaddr(hw, hw->pfvfnum, mac_addr, hw->lane)) {
> > +		eth_random_addr(mac_addr);
> > +	} else {
> > +		if (!is_valid_ether_addr(mac_addr))
> > +			eth_random_addr(mac_addr);
> > +	}
> 
> well, this can be done in one if() statement using logical "or"
> 

Got it, I will update it.

> > +
> > +	hw->flags |= M_FLAGS_INIT_MAC_ADDRESS;
> > +}
> > +
> > +/**
> > + * rnpgbe_reset_hw_ops - Do a hardware reset
> > + * @hw: hw information structure
> > + *
> > + * rnpgbe_reset_hw_ops calls fw to do a hardware
> > + * reset, and cleans some regs to default.
> > + *
> > + * @return: 0 on success, negative on failure
> > + **/
> > +static int rnpgbe_reset_hw_ops(struct mucse_hw *hw)
> > +{
> > +	struct mucse_dma_info *dma = &hw->dma;
> > +	int err;
> > +
> > +	dma_wr32(dma, RNPGBE_DMA_AXI_EN, 0);
> > +	err = mucse_mbx_fw_reset_phy(hw);
> > +	if (err)
> > +		return err;
> > +	/* Store the permanent mac address */
> > +	if (!(hw->flags & M_FLAGS_INIT_MAC_ADDRESS)) {
> > +		rnpgbe_get_permanent_mac(hw, hw->perm_addr);
> > +		memcpy(hw->addr, hw->perm_addr, ETH_ALEN);
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +/**
> > + * rnpgbe_driver_status_hw_ops - Echo driver status to hw
> > + * @hw: hw information structure
> > + * @enable: true or false status
> > + * @mode: status mode
> > + **/
> > +static void rnpgbe_driver_status_hw_ops(struct mucse_hw *hw,
> > +					bool enable,
> > +					int mode)
> > +{
> > +	switch (mode) {
> > +	case mucse_driver_insmod:
> > +		mucse_mbx_ifinsmod(hw, enable);
> > +		break;
> > +	}
> > +}
> > +
> > +static const struct mucse_hw_operations rnpgbe_hw_ops = {
> > +	.reset_hw = &rnpgbe_reset_hw_ops,
> > +	.driver_status = &rnpgbe_driver_status_hw_ops,
> > +};
> >   /**
> >    * rnpgbe_init_common - Setup common attribute
> > @@ -28,6 +100,7 @@ static void rnpgbe_init_common(struct mucse_hw *hw)
> >   	mac->back = hw;
> >   	hw->mbx.ops = &mucse_mbx_ops_generic;
> > +	hw->ops = &rnpgbe_hw_ops;
> >   }
> >   /**
> > diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h
> > index aee037e3219d..4e07328ccf82 100644
> > --- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h
> > +++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h
> > @@ -9,6 +9,7 @@
> >   #define RNPGBE_ETH_BASE 0x10000
> >   /**************** DMA Registers ****************************/
> >   #define RNPGBE_DMA_DUMY 0x000c
> > +#define RNPGBE_DMA_AXI_EN 0x0010
> >   /**************** CHIP Resource ****************************/
> >   #define RNPGBE_MAX_QUEUES 8
> >   #endif /* _RNPGBE_HW_H */
> > diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c
> > index c151995309f8..e0a08fa5b297 100644
> > --- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c
> > +++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c
> > @@ -8,6 +8,7 @@
> >   #include <linux/etherdevice.h>
> >   #include "rnpgbe.h"
> > +#include "rnpgbe_mbx_fw.h"
> >   static const char rnpgbe_driver_name[] = "rnpgbe";
> >   static const struct rnpgbe_info *rnpgbe_info_tbl[] = {
> > @@ -34,6 +35,54 @@ static struct pci_device_id rnpgbe_pci_tbl[] = {
> >   	{0, },
> >   };
> > +/**
> > + * rnpgbe_open - Called when a network interface is made active
> > + * @netdev: network interface device structure
> > + *
> > + * The open entry point is called when a network interface is made
> > + * active by the system (IFF_UP).
> > + *
> > + * @return: 0 on success, negative value on failure
> > + **/
> > +static int rnpgbe_open(struct net_device *netdev)
> > +{
> > +	return 0;
> > +}
> > +
> > +/**
> > + * rnpgbe_close - Disables a network interface
> > + * @netdev: network interface device structure
> > + *
> > + * The close entry point is called when an interface is de-activated
> > + * by the OS.
> > + *
> > + * @return: 0, this is not allowed to fail
> > + **/
> > +static int rnpgbe_close(struct net_device *netdev)
> > +{
> > +	return 0;
> > +}
> > +
> > +/**
> > + * rnpgbe_xmit_frame - Send a skb to driver
> > + * @skb: skb structure to be sent
> > + * @netdev: network interface device structure
> > + *
> > + * @return: NETDEV_TX_OK or NETDEV_TX_BUSY
> > + **/
> > +static netdev_tx_t rnpgbe_xmit_frame(struct sk_buff *skb,
> > +				     struct net_device *netdev)
> > +{
> > +		dev_kfree_skb_any(skb);
> > +		return NETDEV_TX_OK;
> > +}
> > +
> > +static const struct net_device_ops rnpgbe_netdev_ops = {
> > +	.ndo_open = rnpgbe_open,
> > +	.ndo_stop = rnpgbe_close,
> > +	.ndo_start_xmit = rnpgbe_xmit_frame,
> > +};
> > +
> >   /**
> >    * rnpgbe_add_adapter - Add netdev for this pci_dev
> >    * @pdev: PCI device information structure
> > @@ -106,6 +155,29 @@ static int rnpgbe_add_adapter(struct pci_dev *pdev,
> >   	hw->dma.dma_version = dma_version;
> >   	hw->driver_version = 0x0002040f;
> >   	info->init(hw);
> > +	hw->mbx.ops->init_params(hw);
> > +	/* echo fw driver insmod to control hw */
> > +	hw->ops->driver_status(hw, true, mucse_driver_insmod);
> > +	err = mucse_mbx_get_capability(hw);
> > +	if (err) {
> > +		dev_err(&pdev->dev,
> > +			"mucse_mbx_get_capability failed! %d\n",
> > +			err);
> > +		goto err_free_net;
> > +	}
> > +	netdev->netdev_ops = &rnpgbe_netdev_ops;
> > +	netdev->watchdog_timeo = 5 * HZ;
> > +	err = hw->ops->reset_hw(hw);
> > +	if (err) {
> > +		dev_err(&pdev->dev, "Hw reset failed %d\n", err);
> > +		goto err_free_net;
> > +	}
> > +	eth_hw_addr_set(netdev, hw->perm_addr);
> > +	memcpy(netdev->perm_addr, hw->perm_addr, netdev->addr_len);
> 
> the comment from register_netdevice() says:
> 
> 	/* If the device has permanent device address, driver should
> 	 * set dev_addr and also addr_assign_type should be set to
> 	 * NET_ADDR_PERM (default value).
> 	 */
> 
> dev_addr is set by eth_hw_addr_set, perm_addr will be set by
> register_netdev(), no need to manually copy it.
> 

Got it, I will remove memcpy here.

> > +	ether_addr_copy(hw->addr, hw->perm_addr);
> 
> your init() function has the same copy operation...
> 

I will remove it here.

> > +	err = register_netdev(netdev);
> > +	if (err)
> > +		goto err_free_net;
> >   	return 0;
> >   err_free_net:
> > @@ -170,12 +242,16 @@ static int rnpgbe_probe(struct pci_dev *pdev, const struct pci_device_id *id)
> >   static void rnpgbe_rm_adapter(struct pci_dev *pdev)
> >   {
> >   	struct mucse *mucse = pci_get_drvdata(pdev);
> > +	struct mucse_hw *hw = &mucse->hw;
> >   	struct net_device *netdev;
> >   	if (!mucse)
> >   		return;
> >   	netdev = mucse->netdev;
> > +	if (netdev->reg_state == NETREG_REGISTERED)
> > +		unregister_netdev(netdev);
> >   	mucse->netdev = NULL;
> > +	hw->ops->driver_status(hw, false, mucse_driver_insmod);
> >   	free_netdev(netdev);
> >   }
> 
> 

Thanks for your feedback.

Re: [PATCH v3 4/5] net: rnpgbe: Add basic mbx_fw support

[Yibo Dong]

On Wed, Aug 13, 2025 at 01:50:08PM +0530, MD Danish Anwar wrote:
> On 12/08/25 3:09 pm, Dong Yibo wrote:
> > +/**
> > + * mucse_fw_get_capability - Get hw abilities from fw
> > + * @hw: pointer to the HW structure
> > + * @abil: pointer to the hw_abilities structure
> > + *
> > + * mucse_fw_get_capability tries to get hw abilities from
> > + * hw.
> > + *
> > + * @return: 0 on success, negative on failure
> > + **/
> > +static int mucse_fw_get_capability(struct mucse_hw *hw,
> > +				   struct hw_abilities *abil)
> > +{
> > +	struct mbx_fw_cmd_reply reply;
> > +	struct mbx_fw_cmd_req req;
> > +	int err;
> > +
> > +	memset(&req, 0, sizeof(req));
> > +	memset(&reply, 0, sizeof(reply));
> > +	build_phy_abalities_req(&req, &req);
> 
> Typo in function name. You probably meant "build_phy_abilities_req".
> 

You are right, I will update it.

> > +	err = mucse_fw_send_cmd_wait(hw, &req, &reply);
> > +	if (!err)
> > +		memcpy(abil, &reply.hw_abilities, sizeof(*abil));
> > +	return err;
> > +}
> > +
> > +/**
> > + * mucse_mbx_get_capability - Get hw abilities from fw
> > + * @hw: pointer to the HW structure
> > + *
> > + * mucse_mbx_get_capability tries to some capabities from
> > + * hw. Many retrys will do if it is failed.
> > + *
> 
> Typo in comment: "tries to some capabities" should be "tries to get
> capabilities"
> 

Got it, I will fix it.

> > + * @return: 0 on success, negative on failure
> > + **/
> > +int mucse_mbx_get_capability(struct mucse_hw *hw)
> > +{
> > +	struct hw_abilities ability;
> > +	int try_cnt = 3;
> > +	int err;
> > +
> > +	memset(&ability, 0, sizeof(ability));
> > +	while (try_cnt--) {
> > +		err = mucse_fw_get_capability(hw, &ability);
> > +		if (err)
> > +			continue;
> > +		hw->pfvfnum = le16_to_cpu(ability.pfnum);
> > +		hw->fw_version = le32_to_cpu(ability.fw_version);
> > +		hw->axi_mhz = le32_to_cpu(ability.axi_mhz);
> > +		hw->bd_uid = le32_to_cpu(ability.bd_uid);
> > +		return 0;
> > +	}
> > +	return err;
> > +}
> 
> 
> Missing initialization of err variable before the last return, which
> could lead to undefined behavior if all attempts fail.
> 

Got it, I will init it by 'int err = -EIO'.

> > +
> > +/**
> > + * mbx_cookie_zalloc - Alloc a cookie structure
> > + * @priv_len: private length for this cookie
> > + *
> 
> 
> -- 
> Thanks and Regards,
> Danish
> 
> 

Thanks for your feedback.

Re: [PATCH v3 5/5] net: rnpgbe: Add register_netdev

[Yibo Dong]

On Wed, Aug 13, 2025 at 01:56:07PM +0530, MD Danish Anwar wrote:
> On 12/08/25 3:09 pm, Dong Yibo wrote:
> > Initialize get mac from hw, register the netdev.
> > 
> > Signed-off-by: Dong Yibo <dong100@mucse.com>
> > ---
> >  drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h    | 22 ++++++
> >  .../net/ethernet/mucse/rnpgbe/rnpgbe_chip.c   | 73 ++++++++++++++++++
> >  drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h |  1 +
> >  .../net/ethernet/mucse/rnpgbe/rnpgbe_main.c   | 76 +++++++++++++++++++
> >  4 files changed, 172 insertions(+)
> > 
> > diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h
> > index 6cb14b79cbfe..644b8c85c29d 100644
> > --- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h
> > +++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h
> > @@ -6,6 +6,7 @@
> >  
> >  #include <linux/types.h>
> >  #include <linux/mutex.h>
> > +#include <linux/netdevice.h>
> >  
> >  extern const struct rnpgbe_info rnpgbe_n500_info;
> >  extern const struct rnpgbe_info rnpgbe_n210_info;
> > @@ -86,6 +87,18 @@ struct mucse_mbx_info {
> >  	u32 fw2pf_mbox_vec;
> >  };
> >  
> > +struct mucse_hw_operations {
> > +	int (*init_hw)(struct mucse_hw *hw);
> > +	int (*reset_hw)(struct mucse_hw *hw);
> > +	void (*start_hw)(struct mucse_hw *hw);
> > +	void (*init_rx_addrs)(struct mucse_hw *hw);
> > +	void (*driver_status)(struct mucse_hw *hw, bool enable, int mode);
> > +};
> 
> You define functions init_hw, start_hw, and init_rx_addrs in this
> structure but they aren't implemented in this patch. Either implement
> them or remove them if not needed yet.
> 
> 

Got it, I will remove not implemented define.

> > +
> > +enum {
> > +	mucse_driver_insmod,
> > +};
> > +
> >  struct mucse_hw {
> >  	void *back;
> >  	u8 pfvfnum;
> > @@ -96,12 +109,18 @@ struct mucse_hw {
> >  	u32 axi_mhz;
> >  	u32 bd_uid;
> >  	enum rnpgbe_hw_type hw_type;
> > +	const struct mucse_hw_operations *ops;
> >  	struct mucse_dma_info dma;
> >  	struct mucse_eth_info eth;
> >  	struct mucse_mac_info mac;
> >  	struct mucse_mbx_info mbx;
> > +	u32 flags;
> > +#define M_FLAGS_INIT_MAC_ADDRESS BIT(0)
> >  	u32 driver_version;
> >  	u16 usecstocount;
> > +	int lane;
> > +	u8 addr[ETH_ALEN];
> > +	u8 perm_addr[ETH_ALEN];
> >  };
> >  
> >  struct mucse {
> > @@ -123,4 +142,7 @@ struct rnpgbe_info {
> >  #define PCI_DEVICE_ID_N500_DUAL_PORT 0x8318
> >  #define PCI_DEVICE_ID_N210 0x8208
> >  #define PCI_DEVICE_ID_N210L 0x820a
> > +
> > +#define dma_wr32(dma, reg, val) writel((val), (dma)->dma_base_addr + (reg))
> > +#define dma_rd32(dma, reg) readl((dma)->dma_base_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 16d0a76114b5..3eaa0257f3bb 100644
> > --- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_chip.c
> > +++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_chip.c
> > @@ -2,10 +2,82 @@
> >  /* Copyright(c) 2020 - 2025 Mucse Corporation. */
> >  
> >  #include <linux/string.h>
> > +#include <linux/etherdevice.h>
> >  
> >  #include "rnpgbe.h"
> >  #include "rnpgbe_hw.h"
> >  #include "rnpgbe_mbx.h"
> > +#include "rnpgbe_mbx_fw.h"
> > +
> > +/**
> > + * rnpgbe_get_permanent_mac - Get permanent mac
> > + * @hw: hw information structure
> > + * @mac_addr: pointer to store mac
> > + *
> > + * rnpgbe_get_permanent_mac tries to get mac from hw.
> > + * It use eth_random_addr if failed.
> > + **/
> > +static void rnpgbe_get_permanent_mac(struct mucse_hw *hw,
> > +				     u8 *mac_addr)
> > +{
> > +	if (mucse_fw_get_macaddr(hw, hw->pfvfnum, mac_addr, hw->lane)) {
> > +		eth_random_addr(mac_addr);
> > +	} else {
> > +		if (!is_valid_ether_addr(mac_addr))
> > +			eth_random_addr(mac_addr);
> > +	}
> > +
> 
> The function should log a warning when falling back to a random MAC
> address, especially in the second case where the hardware returned an
> invalid MAC.
> 

Got it, I will fix it.

> > +	hw->flags |= M_FLAGS_INIT_MAC_ADDRESS;
> > +}
> > +
> 
> > +/**
> > + * rnpgbe_xmit_frame - Send a skb to driver
> > + * @skb: skb structure to be sent
> > + * @netdev: network interface device structure
> > + *
> > + * @return: NETDEV_TX_OK or NETDEV_TX_BUSY
> > + **/
> > +static netdev_tx_t rnpgbe_xmit_frame(struct sk_buff *skb,
> > +				     struct net_device *netdev)
> > +{
> > +		dev_kfree_skb_any(skb);
> > +		return NETDEV_TX_OK;
> > +}
> 
> Extra indentation on these two lines. Also, the function just drops all
> packets without any actual transmission. This should at least increment
> the drop counter statistics.
> 

Got it, I will add 'netdev->stats.tx_dropped++;' here.

> > +
> > +static const struct net_device_ops rnpgbe_netdev_ops = {
> > +	.ndo_open = rnpgbe_open,
> > +	.ndo_stop = rnpgbe_close,
> > +	.ndo_start_xmit = rnpgbe_xmit_frame,
> > +};
> 
> 
> -- 
> Thanks and Regards,
> Danish
> 
> 

Thanks for your feedback.

Re: [PATCH v3 4/5] net: rnpgbe: Add basic mbx_fw support

[Yibo Dong]

On Tue, Aug 12, 2025 at 05:14:15PM +0100, Vadim Fedorenko wrote:
> On 12/08/2025 10:39, Dong Yibo wrote:
> > Initialize basic mbx_fw ops, such as get_capability, reset phy
> > and so on.
> > 
> > Signed-off-by: Dong Yibo <dong100@mucse.com>
> > +static int mucse_fw_send_cmd_wait(struct mucse_hw *hw,
> > +				  struct mbx_fw_cmd_req *req,
> > +				  struct mbx_fw_cmd_reply *reply)
> > +{
> > +	int len = le16_to_cpu(req->datalen) + MBX_REQ_HDR_LEN;
> > +	int retry_cnt = 3;
> > +	int err;
> > +
> > +	err = mutex_lock_interruptible(&hw->mbx.lock);
> > +	if (err)
> > +		return err;
> > +	err = hw->mbx.ops->write_posted(hw, (u32 *)req,
> > +					L_WD(len));
> > +	if (err) {> +		mutex_unlock(&hw->mbx.lock);
> > +		return err;
> > +	}
> 
> it might look a bit cleaner if you add error label and have unlock code
> once in the end of the function...
> 

If it is more cleaner bellow?

static int mucse_fw_send_cmd_wait(struct mucse_hw *hw,
                                  struct mbx_fw_cmd_req *req,
                                  struct mbx_fw_cmd_reply *reply)
{
        int len = le16_to_cpu(req->datalen) + MBX_REQ_HDR_LEN;
        int retry_cnt = 3;
        int err;

        err = mutex_lock_interruptible(&hw->mbx.lock);
        if (err)
                return err;
        err = hw->mbx.ops->write_posted(hw, (u32 *)req,
                                        L_WD(len));
        if (err)
                goto quit;
        do {
                err = hw->mbx.ops->read_posted(hw, (u32 *)reply,
                                               L_WD(sizeof(*reply)));
                if (err)
                        goto quit;
        } while (--retry_cnt >= 0 && reply->opcode != req->opcode);

        mutex_unlock(&hw->mbx.lock);
        if (retry_cnt < 0)
                return -ETIMEDOUT;
        if (reply->error_code)
                return -EIO;
        return 0;
quit:
        mutex_unlock(&hw->mbx.lock);
        return err;
}

Re: [PATCH v3 4/5] net: rnpgbe: Add basic mbx_fw support

[Geert Uytterhoeven]

Hi Yibo,

On Wed, 13 Aug 2025 at 11:52, Yibo Dong <dong100@mucse.com> wrote:
> On Tue, Aug 12, 2025 at 05:14:15PM +0100, Vadim Fedorenko wrote:
> > On 12/08/2025 10:39, Dong Yibo wrote:
> > > Initialize basic mbx_fw ops, such as get_capability, reset phy
> > > and so on.
> > >
> > > Signed-off-by: Dong Yibo <dong100@mucse.com>
> > > +static int mucse_fw_send_cmd_wait(struct mucse_hw *hw,
> > > +                             struct mbx_fw_cmd_req *req,
> > > +                             struct mbx_fw_cmd_reply *reply)
> > > +{
> > > +   int len = le16_to_cpu(req->datalen) + MBX_REQ_HDR_LEN;
> > > +   int retry_cnt = 3;
> > > +   int err;
> > > +
> > > +   err = mutex_lock_interruptible(&hw->mbx.lock);
> > > +   if (err)
> > > +           return err;
> > > +   err = hw->mbx.ops->write_posted(hw, (u32 *)req,
> > > +                                   L_WD(len));
> > > +   if (err) {> +           mutex_unlock(&hw->mbx.lock);
> > > +           return err;
> > > +   }
> >
> > it might look a bit cleaner if you add error label and have unlock code
> > once in the end of the function...
> >
>
> If it is more cleaner bellow?
>
> static int mucse_fw_send_cmd_wait(struct mucse_hw *hw,
>                                   struct mbx_fw_cmd_req *req,
>                                   struct mbx_fw_cmd_reply *reply)
> {
>         int len = le16_to_cpu(req->datalen) + MBX_REQ_HDR_LEN;
>         int retry_cnt = 3;
>         int err;
>
>         err = mutex_lock_interruptible(&hw->mbx.lock);
>         if (err)
>                 return err;
>         err = hw->mbx.ops->write_posted(hw, (u32 *)req,
>                                         L_WD(len));
>         if (err)
>                 goto quit;
>         do {
>                 err = hw->mbx.ops->read_posted(hw, (u32 *)reply,
>                                                L_WD(sizeof(*reply)));
>                 if (err)
>                         goto quit;
>         } while (--retry_cnt >= 0 && reply->opcode != req->opcode);
>
>         mutex_unlock(&hw->mbx.lock);
>         if (retry_cnt < 0)
>                 return -ETIMEDOUT;
>         if (reply->error_code)
>                 return -EIO;
>         return 0;
> quit:
>         mutex_unlock(&hw->mbx.lock);
>         return err;
> }

Or use scoped_cond_guard(mutex_intr, ...) { ... }?

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v3 5/5] net: rnpgbe: Add register_netdev

[Vadim Fedorenko]

On 13/08/2025 10:04, Yibo Dong wrote:
> On Tue, Aug 12, 2025 at 04:32:00PM +0100, Vadim Fedorenko wrote:
>> On 12/08/2025 10:39, Dong Yibo wrote:
>>> Initialize get mac from hw, register the netdev.
>>>
>>> Signed-off-by: Dong Yibo <dong100@mucse.com>
>>> ---
>>>    drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h    | 22 ++++++
>>>    .../net/ethernet/mucse/rnpgbe/rnpgbe_chip.c   | 73 ++++++++++++++++++
>>>    drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h |  1 +
>>>    .../net/ethernet/mucse/rnpgbe/rnpgbe_main.c   | 76 +++++++++++++++++++
>>>    4 files changed, 172 insertions(+)
>>>
>>> diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h
>>> index 6cb14b79cbfe..644b8c85c29d 100644
>>> --- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h
>>> +++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h
>>> @@ -6,6 +6,7 @@
>>>    #include <linux/types.h>
>>>    #include <linux/mutex.h>
>>> +#include <linux/netdevice.h>
>>>    extern const struct rnpgbe_info rnpgbe_n500_info;
>>>    extern const struct rnpgbe_info rnpgbe_n210_info;
>>> @@ -86,6 +87,18 @@ struct mucse_mbx_info {
>>>    	u32 fw2pf_mbox_vec;
>>>    };
>>> +struct mucse_hw_operations {
>>> +	int (*init_hw)(struct mucse_hw *hw);
>>> +	int (*reset_hw)(struct mucse_hw *hw);
>>> +	void (*start_hw)(struct mucse_hw *hw);
>>> +	void (*init_rx_addrs)(struct mucse_hw *hw);
>>> +	void (*driver_status)(struct mucse_hw *hw, bool enable, int mode);
>>> +};
>>> +
>>> +enum {
>>> +	mucse_driver_insmod,
>>> +};
>>> +
>>>    struct mucse_hw {
>>>    	void *back;
>>>    	u8 pfvfnum;
>>> @@ -96,12 +109,18 @@ struct mucse_hw {
>>>    	u32 axi_mhz;
>>>    	u32 bd_uid;
>>>    	enum rnpgbe_hw_type hw_type;
>>> +	const struct mucse_hw_operations *ops;
>>>    	struct mucse_dma_info dma;
>>>    	struct mucse_eth_info eth;
>>>    	struct mucse_mac_info mac;
>>>    	struct mucse_mbx_info mbx;
>>> +	u32 flags;
>>> +#define M_FLAGS_INIT_MAC_ADDRESS BIT(0)
>>>    	u32 driver_version;
>>>    	u16 usecstocount;
>>> +	int lane;
>>> +	u8 addr[ETH_ALEN];
>>> +	u8 perm_addr[ETH_ALEN];
>>
>> why do you need both addresses if you have this info already in netdev?
>>
> 
> 'perm_addr' is address from firmware (fixed, can't be changed by user).
> 'addr' is the current netdev address (It is Initialized the same with
> 'perm_addr', but can be changed by user)
> Maybe I should add 'addr' in the patch which support ndo_set_mac_address?

But why do you need 'addr' at all? Current netdev address can be
retrieved from netdev, why do you need to store it within driver's
structure?

Re: [PATCH v3 4/5] net: rnpgbe: Add basic mbx_fw support

[Vadim Fedorenko]

On 13/08/2025 10:52, Yibo Dong wrote:
> On Tue, Aug 12, 2025 at 05:14:15PM +0100, Vadim Fedorenko wrote:
>> On 12/08/2025 10:39, Dong Yibo wrote:
>>> Initialize basic mbx_fw ops, such as get_capability, reset phy
>>> and so on.
>>>
>>> Signed-off-by: Dong Yibo <dong100@mucse.com>
>>> +static int mucse_fw_send_cmd_wait(struct mucse_hw *hw,
>>> +				  struct mbx_fw_cmd_req *req,
>>> +				  struct mbx_fw_cmd_reply *reply)
>>> +{
>>> +	int len = le16_to_cpu(req->datalen) + MBX_REQ_HDR_LEN;
>>> +	int retry_cnt = 3;
>>> +	int err;
>>> +
>>> +	err = mutex_lock_interruptible(&hw->mbx.lock);
>>> +	if (err)
>>> +		return err;
>>> +	err = hw->mbx.ops->write_posted(hw, (u32 *)req,
>>> +					L_WD(len));
>>> +	if (err) {> +		mutex_unlock(&hw->mbx.lock);
>>> +		return err;
>>> +	}
>>
>> it might look a bit cleaner if you add error label and have unlock code
>> once in the end of the function...
>>
> 
> If it is more cleaner bellow?
> 
> static int mucse_fw_send_cmd_wait(struct mucse_hw *hw,
>                                    struct mbx_fw_cmd_req *req,
>                                    struct mbx_fw_cmd_reply *reply)
> {
>          int len = le16_to_cpu(req->datalen) + MBX_REQ_HDR_LEN;
>          int retry_cnt = 3;
>          int err;
> 
>          err = mutex_lock_interruptible(&hw->mbx.lock);
>          if (err)
>                  return err;
>          err = hw->mbx.ops->write_posted(hw, (u32 *)req,
>                                          L_WD(len));
>          if (err)
>                  goto quit;
>          do {
>                  err = hw->mbx.ops->read_posted(hw, (u32 *)reply,
>                                                 L_WD(sizeof(*reply)));
>                  if (err)
>                          goto quit;
>          } while (--retry_cnt >= 0 && reply->opcode != req->opcode);
> 
>          mutex_unlock(&hw->mbx.lock);
>          if (retry_cnt < 0)
>                  return -ETIMEDOUT;
>          if (reply->error_code)
>                  return -EIO;
>          return 0;
> quit:
>          mutex_unlock(&hw->mbx.lock);
>          return err;
> }
> 

Maybe:

           } while (--retry_cnt >= 0 && reply->opcode != req->opcode);

  quit:
           mutex_unlock(&hw->mbx.lock);
           if (!err && retry_cnt < 0)
                   return -ETIMEDOUT;
           if (!err && reply->error_code)
                   return -EIO;
           return err;


looks cleaner

Re: [PATCH v3 4/5] net: rnpgbe: Add basic mbx_fw support

[Jakub Kicinski]

On Wed, 13 Aug 2025 13:05:36 +0200 Geert Uytterhoeven wrote:
> Or use scoped_cond_guard(mutex_intr, ...) { ... }?

Hard no on that in networking.

Re: [PATCH v3 5/5] net: rnpgbe: Add register_netdev

[Yibo Dong]

On Wed, Aug 13, 2025 at 01:50:34PM +0100, Vadim Fedorenko wrote:
> On 13/08/2025 10:04, Yibo Dong wrote:
> > On Tue, Aug 12, 2025 at 04:32:00PM +0100, Vadim Fedorenko wrote:
> > > On 12/08/2025 10:39, Dong Yibo wrote:
> > > > Initialize get mac from hw, register the netdev.
> > > > 
> > > > Signed-off-by: Dong Yibo <dong100@mucse.com>
> > > > ---
> > > >    drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h    | 22 ++++++
> > > >    .../net/ethernet/mucse/rnpgbe/rnpgbe_chip.c   | 73 ++++++++++++++++++
> > > >    drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h |  1 +
> > > >    .../net/ethernet/mucse/rnpgbe/rnpgbe_main.c   | 76 +++++++++++++++++++
> > > >    4 files changed, 172 insertions(+)
> > > > 
> > > > diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h
> > > > index 6cb14b79cbfe..644b8c85c29d 100644
> > > > --- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h
> > > > +++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h
> > > > @@ -6,6 +6,7 @@
> > > >    #include <linux/types.h>
> > > >    #include <linux/mutex.h>
> > > > +#include <linux/netdevice.h>
> > > >    extern const struct rnpgbe_info rnpgbe_n500_info;
> > > >    extern const struct rnpgbe_info rnpgbe_n210_info;
> > > > @@ -86,6 +87,18 @@ struct mucse_mbx_info {
> > > >    	u32 fw2pf_mbox_vec;
> > > >    };
> > > > +struct mucse_hw_operations {
> > > > +	int (*init_hw)(struct mucse_hw *hw);
> > > > +	int (*reset_hw)(struct mucse_hw *hw);
> > > > +	void (*start_hw)(struct mucse_hw *hw);
> > > > +	void (*init_rx_addrs)(struct mucse_hw *hw);
> > > > +	void (*driver_status)(struct mucse_hw *hw, bool enable, int mode);
> > > > +};
> > > > +
> > > > +enum {
> > > > +	mucse_driver_insmod,
> > > > +};
> > > > +
> > > >    struct mucse_hw {
> > > >    	void *back;
> > > >    	u8 pfvfnum;
> > > > @@ -96,12 +109,18 @@ struct mucse_hw {
> > > >    	u32 axi_mhz;
> > > >    	u32 bd_uid;
> > > >    	enum rnpgbe_hw_type hw_type;
> > > > +	const struct mucse_hw_operations *ops;
> > > >    	struct mucse_dma_info dma;
> > > >    	struct mucse_eth_info eth;
> > > >    	struct mucse_mac_info mac;
> > > >    	struct mucse_mbx_info mbx;
> > > > +	u32 flags;
> > > > +#define M_FLAGS_INIT_MAC_ADDRESS BIT(0)
> > > >    	u32 driver_version;
> > > >    	u16 usecstocount;
> > > > +	int lane;
> > > > +	u8 addr[ETH_ALEN];
> > > > +	u8 perm_addr[ETH_ALEN];
> > > 
> > > why do you need both addresses if you have this info already in netdev?
> > > 
> > 
> > 'perm_addr' is address from firmware (fixed, can't be changed by user).
> > 'addr' is the current netdev address (It is Initialized the same with
> > 'perm_addr', but can be changed by user)
> > Maybe I should add 'addr' in the patch which support ndo_set_mac_address?
> 
> But why do you need 'addr' at all? Current netdev address can be
> retrieved from netdev, why do you need to store it within driver's
> structure?
> 
> 

Ok, I will remove addr and use netdev->dev_addr if driver use it.

Re: [PATCH v3 4/5] net: rnpgbe: Add basic mbx_fw support

[Yibo Dong]

On Wed, Aug 13, 2025 at 02:33:39PM +0100, Vadim Fedorenko wrote:
> On 13/08/2025 10:52, Yibo Dong wrote:
> > On Tue, Aug 12, 2025 at 05:14:15PM +0100, Vadim Fedorenko wrote:
> > > On 12/08/2025 10:39, Dong Yibo wrote:
> > > > Initialize basic mbx_fw ops, such as get_capability, reset phy
> > > > and so on.
> > > > 
> > > > Signed-off-by: Dong Yibo <dong100@mucse.com>
> > > > +static int mucse_fw_send_cmd_wait(struct mucse_hw *hw,
> > > > +				  struct mbx_fw_cmd_req *req,
> > > > +				  struct mbx_fw_cmd_reply *reply)
> > > > +{
> > > > +	int len = le16_to_cpu(req->datalen) + MBX_REQ_HDR_LEN;
> > > > +	int retry_cnt = 3;
> > > > +	int err;
> > > > +
> > > > +	err = mutex_lock_interruptible(&hw->mbx.lock);
> > > > +	if (err)
> > > > +		return err;
> > > > +	err = hw->mbx.ops->write_posted(hw, (u32 *)req,
> > > > +					L_WD(len));
> > > > +	if (err) {> +		mutex_unlock(&hw->mbx.lock);
> > > > +		return err;
> > > > +	}
> > > 
> > > it might look a bit cleaner if you add error label and have unlock code
> > > once in the end of the function...
> > > 
> > 
> > If it is more cleaner bellow?
> > 
> > static int mucse_fw_send_cmd_wait(struct mucse_hw *hw,
> >                                    struct mbx_fw_cmd_req *req,
> >                                    struct mbx_fw_cmd_reply *reply)
> > {
> >          int len = le16_to_cpu(req->datalen) + MBX_REQ_HDR_LEN;
> >          int retry_cnt = 3;
> >          int err;
> > 
> >          err = mutex_lock_interruptible(&hw->mbx.lock);
> >          if (err)
> >                  return err;
> >          err = hw->mbx.ops->write_posted(hw, (u32 *)req,
> >                                          L_WD(len));
> >          if (err)
> >                  goto quit;
> >          do {
> >                  err = hw->mbx.ops->read_posted(hw, (u32 *)reply,
> >                                                 L_WD(sizeof(*reply)));
> >                  if (err)
> >                          goto quit;
> >          } while (--retry_cnt >= 0 && reply->opcode != req->opcode);
> > 
> >          mutex_unlock(&hw->mbx.lock);
> >          if (retry_cnt < 0)
> >                  return -ETIMEDOUT;
> >          if (reply->error_code)
> >                  return -EIO;
> >          return 0;
> > quit:
> >          mutex_unlock(&hw->mbx.lock);
> >          return err;
> > }
> > 
> 
> Maybe:
> 
>           } while (--retry_cnt >= 0 && reply->opcode != req->opcode);
> 
>  quit:
>           mutex_unlock(&hw->mbx.lock);
>           if (!err && retry_cnt < 0)
>                   return -ETIMEDOUT;
>           if (!err && reply->error_code)
>                   return -EIO;
>           return err;
> 
> 
> looks cleaner
> 
> 

Got it, I will update this, thanks. 

Re: [PATCH v3 3/5] net: rnpgbe: Add basic mbx ops support

[Andrew Lunn]

> +int mucse_read_mbx(struct mucse_hw *hw, u32 *msg, u16 size)
> +{
> +	struct mucse_mbx_info *mbx = &hw->mbx;
> +
> +	/* limit read size */
> +	min(size, mbx->size);
> +	return mbx->ops->read(hw, msg, size);

As well as the obvious bug pointed out by others, isn't this condition
actually indicating a bug somewhere else? If size is bigger than
mbx->size, the caller is broken. You probably want a dev_err() here,
and return -EINVAL, so you get a hint something else is broken
somewhere.

	Andrew

Re: [PATCH v3 4/5] net: rnpgbe: Add basic mbx_fw support

[Andrew Lunn]

> If it is more cleaner bellow?
> 
> static int mucse_fw_send_cmd_wait(struct mucse_hw *hw,
>                                   struct mbx_fw_cmd_req *req,
>                                   struct mbx_fw_cmd_reply *reply)
> {
>         int len = le16_to_cpu(req->datalen) + MBX_REQ_HDR_LEN;
>         int retry_cnt = 3;
>         int err;
> 
>         err = mutex_lock_interruptible(&hw->mbx.lock);
>         if (err)
>                 return err;
>         err = hw->mbx.ops->write_posted(hw, (u32 *)req,
>                                         L_WD(len));
>         if (err)
>                 goto quit;
>         do {
>                 err = hw->mbx.ops->read_posted(hw, (u32 *)reply,
>                                                L_WD(sizeof(*reply)));
>                 if (err)
>                         goto quit;
>         } while (--retry_cnt >= 0 && reply->opcode != req->opcode);
> 
>         mutex_unlock(&hw->mbx.lock);
>         if (retry_cnt < 0)
>                 return -ETIMEDOUT;
>         if (reply->error_code)
>                 return -EIO;
>         return 0;
> quit:
>         mutex_unlock(&hw->mbx.lock);
>         return err;
> }

You might want a read a few other drivers in mailine. Look at the
naming. I doubt you will find many using "quit" for a label. "out" or
"unlock" is more popular.

When it comes to locks, it is better to have one lock statement and
one unlock statement. It then becomes easy to see all paths lead to
the unlock.

	Andrew

Re: [PATCH v3 3/5] net: rnpgbe: Add basic mbx ops support

[Yibo Dong]

On Fri, Aug 15, 2025 at 01:55:42AM +0200, Andrew Lunn wrote:
> > +int mucse_read_mbx(struct mucse_hw *hw, u32 *msg, u16 size)
> > +{
> > +	struct mucse_mbx_info *mbx = &hw->mbx;
> > +
> > +	/* limit read size */
> > +	min(size, mbx->size);
> > +	return mbx->ops->read(hw, msg, size);
> 
> As well as the obvious bug pointed out by others, isn't this condition
> actually indicating a bug somewhere else? If size is bigger than
> mbx->size, the caller is broken. You probably want a dev_err() here,
> and return -EINVAL, so you get a hint something else is broken
> somewhere.
> 
> 	Andrew
> 

Ok, the caller is broken when size is bigger than mbx->size. I will use
dev_err here in v5 since I had sent v4 before this mail.
By the way, how long should I wait before sending the new version? If it
is too frequent, it might cause reviewers to check old versions and miss
feedback, link what happened with this mail. And if it is too long, it
is easy to miss the 'open window'....

Thanks for your feedback.

Re: [PATCH v3 4/5] net: rnpgbe: Add basic mbx_fw support

[Yibo Dong]

On Fri, Aug 15, 2025 at 02:04:57AM +0200, Andrew Lunn wrote:
> > If it is more cleaner bellow?
> > 
> > static int mucse_fw_send_cmd_wait(struct mucse_hw *hw,
> >                                   struct mbx_fw_cmd_req *req,
> >                                   struct mbx_fw_cmd_reply *reply)
> > {
> >         int len = le16_to_cpu(req->datalen) + MBX_REQ_HDR_LEN;
> >         int retry_cnt = 3;
> >         int err;
> > 
> >         err = mutex_lock_interruptible(&hw->mbx.lock);
> >         if (err)
> >                 return err;
> >         err = hw->mbx.ops->write_posted(hw, (u32 *)req,
> >                                         L_WD(len));
> >         if (err)
> >                 goto quit;
> >         do {
> >                 err = hw->mbx.ops->read_posted(hw, (u32 *)reply,
> >                                                L_WD(sizeof(*reply)));
> >                 if (err)
> >                         goto quit;
> >         } while (--retry_cnt >= 0 && reply->opcode != req->opcode);
> > 
> >         mutex_unlock(&hw->mbx.lock);
> >         if (retry_cnt < 0)
> >                 return -ETIMEDOUT;
> >         if (reply->error_code)
> >                 return -EIO;
> >         return 0;
> > quit:
> >         mutex_unlock(&hw->mbx.lock);
> >         return err;
> > }
> 
> You might want a read a few other drivers in mailine. Look at the
> naming. I doubt you will find many using "quit" for a label. "out" or
> "unlock" is more popular.
> 
> When it comes to locks, it is better to have one lock statement and
> one unlock statement. It then becomes easy to see all paths lead to
> the unlock.
> 
> 	Andrew
> 

Got it, I will change label 'quit' to 'out'.
And I will try to keep 'one lock statement and one unlock statement'
principle in mind.

Thanks for your feedback.

Re: [PATCH v3 3/5] net: rnpgbe: Add basic mbx ops support

[Andrew Lunn]

> By the way, how long should I wait before sending the new version? If it
> is too frequent, it might cause reviewers to check old versions and miss
> feedback, link what happened with this mail. And if it is too long, it
> is easy to miss the 'open window'....

https://www.kernel.org/doc/html/latest/process/maintainer-netdev.html

Says at least 24 hours.

But i have been away from emails for a few days, so i was slower than
usual.

Most patches get reviewed in 3 work days. So i would probably not wait
much longer than that. But also wait for any discussion about a patch
to come to an end.

Also, different subsystems work are different speed. 3 days would be
way to fast for USB for example, that would be more like 2 weeks.

	Andrew