* [PATCH net-next v10 1/5] net: rnpgbe: Add build support for rnpgbe
2025-09-03 2:54 [PATCH net-next v10 0/5] Add driver for 1Gbe network chips from MUCSE Dong Yibo
@ 2025-09-03 2:54 ` Dong Yibo
2025-09-03 2:54 ` [PATCH net-next v10 2/5] net: rnpgbe: Add n500/n210 chip support Dong Yibo
` (3 subsequent siblings)
4 siblings, 0 replies; 22+ messages in thread
From: Dong Yibo @ 2025-09-03 2:54 UTC (permalink / raw)
To: andrew+netdev, davem, edumazet, kuba, pabeni, horms, corbet,
gur.stavi, maddy, mpe, danishanwar, lee, gongfan1, lorenzo,
geert+renesas, Parthiban.Veerasooran, lukas.bulwahn,
alexanderduyck, richardcochran, kees, gustavoars, rdunlap,
vadim.fedorenko
Cc: netdev, linux-doc, linux-kernel, linux-hardening, dong100,
Andrew Lunn
Add build options and doc for mucse.
Initialize pci device access for MUCSE devices.
Signed-off-by: Dong Yibo <dong100@mucse.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
---
.../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 | 24 ++++
.../net/ethernet/mucse/rnpgbe/rnpgbe_main.c | 126 ++++++++++++++++++
10 files changed, 231 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 0b0a3eef6aae..41ff2152b7aa 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 6cad6225381a..ca4e7f96d880 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -17305,6 +17305,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..675173fa05f7
--- /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) network device drivers
+#
+
+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..64b2c093bc6e
--- /dev/null
+++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h
@@ -0,0 +1,24 @@
+/* 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;
+};
+
+/* 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..98905607e74b
--- /dev/null
+++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c
@@ -0,0 +1,126 @@
+// 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 errno 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_disable_dev;
+ }
+
+ err = pci_request_mem_regions(pdev, rnpgbe_driver_name);
+ if (err) {
+ dev_err(&pdev->dev,
+ "pci_request_selected_regions failed %d\n", err);
+ goto err_disable_dev;
+ }
+
+ pci_set_master(pdev);
+ err = pci_save_state(pdev);
+ if (err) {
+ dev_err(&pdev->dev, "pci_save_state failed %d\n", err);
+ goto err_free_regions;
+ }
+
+ return 0;
+err_free_regions:
+ pci_release_mem_regions(pdev);
+err_disable_dev:
+ 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
+ **/
+static void rnpgbe_dev_shutdown(struct pci_dev *pdev)
+{
+ 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)
+{
+ rnpgbe_dev_shutdown(pdev);
+}
+
+static struct pci_driver rnpgbe_driver = {
+ .name = rnpgbe_driver_name,
+ .id_table = rnpgbe_pci_tbl,
+ .probe = rnpgbe_probe,
+ .remove = rnpgbe_remove,
+ .shutdown = rnpgbe_shutdown,
+};
+
+module_pci_driver(rnpgbe_driver);
+
+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
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH net-next v10 2/5] net: rnpgbe: Add n500/n210 chip support
2025-09-03 2:54 [PATCH net-next v10 0/5] Add driver for 1Gbe network chips from MUCSE Dong Yibo
2025-09-03 2:54 ` [PATCH net-next v10 1/5] net: rnpgbe: Add build support for rnpgbe Dong Yibo
@ 2025-09-03 2:54 ` Dong Yibo
2025-09-03 2:54 ` [PATCH net-next v10 3/5] net: rnpgbe: Add basic mbx ops support Dong Yibo
` (2 subsequent siblings)
4 siblings, 0 replies; 22+ messages in thread
From: Dong Yibo @ 2025-09-03 2:54 UTC (permalink / raw)
To: andrew+netdev, davem, edumazet, kuba, pabeni, horms, corbet,
gur.stavi, maddy, mpe, danishanwar, lee, gongfan1, lorenzo,
geert+renesas, Parthiban.Veerasooran, lukas.bulwahn,
alexanderduyck, richardcochran, kees, gustavoars, rdunlap,
vadim.fedorenko
Cc: netdev, linux-doc, linux-kernel, linux-hardening, dong100
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 | 34 ++++++++
.../net/ethernet/mucse/rnpgbe/rnpgbe_chip.c | 68 +++++++++++++++
drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h | 16 ++++
.../net/ethernet/mucse/rnpgbe/rnpgbe_main.c | 85 +++++++++++++++++++
5 files changed, 205 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 64b2c093bc6e..9a86e67d6395 100644
--- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h
+++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h
@@ -4,15 +4,49 @@
#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_unknown
+};
+
+struct mucse_mbx_info {
+ /* fw <--> pf mbx */
+ u32 fw_pf_shm_base;
+ u32 pf2fw_mbox_ctrl;
+ u32 fw_pf_mbox_mask;
+ u32 fw2pf_mbox_vec;
+};
+
+struct mucse_hw {
+ void __iomem *hw_addr;
+ struct pci_dev *pdev;
+ enum rnpgbe_hw_type hw_type;
+ struct mucse_mbx_info mbx;
+};
+
struct mucse {
struct net_device *netdev;
struct pci_dev *pdev;
+ struct mucse_hw hw;
+};
+
+struct rnpgbe_info {
+ enum rnpgbe_hw_type hw_type;
+ void (*init)(struct mucse_hw *hw);
};
/* Device IDs */
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..179621ea09f3
--- /dev/null
+++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_chip.c
@@ -0,0 +1,68 @@
+// 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_mbx_info *mbx = &hw->mbx;
+
+ mbx->pf2fw_mbox_ctrl = GBE_PF2FW_MBX_MASK_OFFSET;
+ mbx->fw_pf_mbox_mask = GBE_FWPF_MBX_MASK;
+}
+
+/**
+ * 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->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 = N500_FW2PF_MBX_VEC_OFFSET;
+ mbx->fw_pf_shm_base = N500_FWPF_SHM_BASE_OFFSET;
+}
+
+/**
+ * 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->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 = N210_FW2PF_MBX_VEC_OFFSET;
+ mbx->fw_pf_shm_base = N210_FWPF_SHM_BASE_OFFSET;
+}
+
+const struct rnpgbe_info rnpgbe_n500_info = {
+ .hw_type = rnpgbe_hw_n500,
+ .init = &rnpgbe_init_n500,
+};
+
+const struct rnpgbe_info rnpgbe_n210_info = {
+ .hw_type = rnpgbe_hw_n210,
+ .init = &rnpgbe_init_n210,
+};
+
+const struct rnpgbe_info rnpgbe_n210L_info = {
+ .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..746dca78f1df
--- /dev/null
+++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h
@@ -0,0 +1,16 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Copyright(c) 2020 - 2025 Mucse Corporation. */
+
+#ifndef _RNPGBE_HW_H
+#define _RNPGBE_HW_H
+
+/**************** MBX Resource ****************************/
+#define N500_FW2PF_MBX_VEC_OFFSET 0x28b00
+#define N500_FWPF_SHM_BASE_OFFSET 0x2d000
+#define GBE_PF2FW_MBX_MASK_OFFSET 0x5500
+#define GBE_FWPF_MBX_MASK 0x5700
+#define N210_FW2PF_MBX_VEC_OFFSET 0x29400
+#define N210_FWPF_SHM_BASE_OFFSET 0x2d900
+/**************** 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 98905607e74b..25b7119d6ecb 100644
--- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c
+++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c
@@ -4,10 +4,18 @@
#include <linux/types.h>
#include <linux/module.h>
#include <linux/pci.h>
+#include <linux/netdevice.h>
+#include <linux/etherdevice.h>
#include "rnpgbe.h"
+#include "rnpgbe_hw.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 +35,56 @@ 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 errno on failure
+ **/
+static int rnpgbe_add_adapter(struct pci_dev *pdev,
+ const struct rnpgbe_info *info)
+{
+ struct net_device *netdev;
+ void __iomem *hw_addr;
+ struct mucse *mucse;
+ struct mucse_hw *hw;
+ int err;
+
+ netdev = alloc_etherdev_mq(sizeof(struct mucse), RNPGBE_MAX_QUEUES);
+ if (!netdev)
+ return -ENOMEM;
+
+ SET_NETDEV_DEV(netdev, &pdev->dev);
+ mucse = netdev_priv(netdev);
+ mucse->netdev = netdev;
+ mucse->pdev = pdev;
+ pci_set_drvdata(pdev, mucse);
+
+ hw = &mucse->hw;
+ hw->hw_type = info->hw_type;
+ hw->pdev = pdev;
+ 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;
+ }
+
+ hw->hw_addr = hw_addr;
+ 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 +97,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);
@@ -65,6 +124,9 @@ static int rnpgbe_probe(struct pci_dev *pdev, const struct pci_device_id *id)
dev_err(&pdev->dev, "pci_save_state failed %d\n", err);
goto err_free_regions;
}
+ err = rnpgbe_add_adapter(pdev, info);
+ if (err)
+ goto err_free_regions;
return 0;
err_free_regions:
@@ -74,6 +136,24 @@ static int rnpgbe_probe(struct pci_dev *pdev, const struct pci_device_id *id)
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
@@ -85,6 +165,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);
}
@@ -95,6 +176,10 @@ static void rnpgbe_remove(struct pci_dev *pdev)
**/
static void rnpgbe_dev_shutdown(struct pci_dev *pdev)
{
+ struct mucse *mucse = pci_get_drvdata(pdev);
+ struct net_device *netdev = mucse->netdev;
+
+ netif_device_detach(netdev);
pci_disable_device(pdev);
}
--
2.25.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH net-next v10 3/5] net: rnpgbe: Add basic mbx ops support
2025-09-03 2:54 [PATCH net-next v10 0/5] Add driver for 1Gbe network chips from MUCSE Dong Yibo
2025-09-03 2:54 ` [PATCH net-next v10 1/5] net: rnpgbe: Add build support for rnpgbe Dong Yibo
2025-09-03 2:54 ` [PATCH net-next v10 2/5] net: rnpgbe: Add n500/n210 chip support Dong Yibo
@ 2025-09-03 2:54 ` Dong Yibo
2025-09-03 10:53 ` Vadim Fedorenko
` (2 more replies)
2025-09-03 2:54 ` [PATCH net-next v10 4/5] net: rnpgbe: Add basic mbx_fw support Dong Yibo
2025-09-03 2:54 ` [PATCH net-next v10 5/5] net: rnpgbe: Add register_netdev Dong Yibo
4 siblings, 3 replies; 22+ messages in thread
From: Dong Yibo @ 2025-09-03 2:54 UTC (permalink / raw)
To: andrew+netdev, davem, edumazet, kuba, pabeni, horms, corbet,
gur.stavi, maddy, mpe, danishanwar, lee, gongfan1, lorenzo,
geert+renesas, Parthiban.Veerasooran, lukas.bulwahn,
alexanderduyck, richardcochran, kees, gustavoars, rdunlap,
vadim.fedorenko
Cc: netdev, linux-doc, linux-kernel, linux-hardening, dong100
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 | 16 +
.../net/ethernet/mucse/rnpgbe/rnpgbe_chip.c | 3 +
.../net/ethernet/mucse/rnpgbe/rnpgbe_mbx.c | 393 ++++++++++++++++++
.../net/ethernet/mucse/rnpgbe/rnpgbe_mbx.h | 25 ++
5 files changed, 439 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 9a86e67d6395..7999bb99306b 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;
@@ -23,7 +24,22 @@ enum rnpgbe_hw_type {
rnpgbe_hw_unknown
};
+struct mucse_mbx_stats {
+ u32 msgs_tx;
+ u32 msgs_rx;
+ u32 acks;
+ u32 reqs;
+};
+
struct mucse_mbx_info {
+ 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;
/* 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 179621ea09f3..f38daef752a3 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
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..856cd4c8ab6f
--- /dev/null
+++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx.c
@@ -0,0 +1,393 @@
+// 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"
+
+/**
+ * mbx_data_rd32 - Reads reg with base mbx->fw_pf_shm_base
+ * @mbx: pointer to the MBX structure
+ * @reg: register offset
+ *
+ * Return: register value
+ **/
+static u32 mbx_data_rd32(struct mucse_mbx_info *mbx, u32 reg)
+{
+ struct mucse_hw *hw = container_of(mbx, struct mucse_hw, mbx);
+
+ return readl(hw->hw_addr + mbx->fw_pf_shm_base + reg);
+}
+
+/**
+ * mbx_data_wr32 - Writes value to reg with base mbx->fw_pf_shm_base
+ * @mbx: pointer to the MBX structure
+ * @reg: register offset
+ * @value: value to be written
+ *
+ **/
+static void mbx_data_wr32(struct mucse_mbx_info *mbx, u32 reg, u32 value)
+{
+ struct mucse_hw *hw = container_of(mbx, struct mucse_hw, mbx);
+
+ writel(value, hw->hw_addr + mbx->fw_pf_shm_base + reg);
+}
+
+/**
+ * mbx_ctrl_rd32 - Reads reg with base mbx->fw2pf_mbox_vec
+ * @mbx: pointer to the MBX structure
+ * @reg: register offset
+ *
+ * Return: register value
+ **/
+static u32 mbx_ctrl_rd32(struct mucse_mbx_info *mbx, u32 reg)
+{
+ struct mucse_hw *hw = container_of(mbx, struct mucse_hw, mbx);
+
+ return readl(hw->hw_addr + mbx->fw2pf_mbox_vec + reg);
+}
+
+/**
+ * mbx_ctrl_wr32 - Writes value to reg with base mbx->fw2pf_mbox_vec
+ * @mbx: pointer to the MBX structure
+ * @reg: register offset
+ * @value: value to be written
+ *
+ **/
+static void mbx_ctrl_wr32(struct mucse_mbx_info *mbx, u32 reg, u32 value)
+{
+ struct mucse_hw *hw = container_of(mbx, struct mucse_hw, mbx);
+
+ writel(value, hw->hw_addr + mbx->fw2pf_mbox_vec + reg);
+}
+
+/**
+ * mucse_mbx_get_fwreq - Read fw req from reg
+ * @mbx: pointer to the mbx structure
+ *
+ * Return: the fwreq value
+ **/
+static u16 mucse_mbx_get_fwreq(struct mucse_mbx_info *mbx)
+{
+ return mbx_data_rd32(mbx, MBX_FW2PF_COUNTER) & GENMASK_U32(15, 0);
+}
+
+/**
+ * mucse_mbx_get_fwack - Read fw ack from reg
+ * @mbx: pointer to the MBX structure
+ *
+ * Return: the fwack value
+ **/
+static u16 mucse_mbx_get_fwack(struct mucse_mbx_info *mbx)
+{
+ return (mbx_data_rd32(mbx, MBX_FW2PF_COUNTER) >> 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;
+ u16 req;
+ u32 v;
+
+ v = mbx_data_rd32(mbx, MBX_PF2FW_COUNTER);
+ req = (v & GENMASK_U32(15, 0));
+ req++;
+ v &= GENMASK_U32(31, 16);
+ v |= req;
+ mbx_data_wr32(mbx, MBX_PF2FW_COUNTER, 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;
+ u16 ack;
+ u32 v;
+
+ v = mbx_data_rd32(mbx, MBX_PF2FW_COUNTER);
+ ack = (v >> 16) & GENMASK_U32(15, 0);
+ ack++;
+ v &= GENMASK_U32(15, 0);
+ v |= (ack << 16);
+ mbx_data_wr32(mbx, MBX_PF2FW_COUNTER, v);
+ hw->mbx.stats.msgs_rx++;
+}
+
+/**
+ * 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;
+
+ hw_req_count = mucse_mbx_get_fwreq(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_poll_for_msg - Wait for message notification
+ * @hw: pointer to the HW structure
+ *
+ * Return: 0 on success, negative errno 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(mucse_check_for_msg_pf,
+ val, val == 0, mbx->usec_delay,
+ countdown * mbx->usec_delay,
+ false, hw);
+}
+
+/**
+ * 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_fwack(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_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(mucse_check_for_ack_pf,
+ val, val == 0, mbx->usec_delay,
+ countdown * mbx->usec_delay,
+ false, hw);
+}
+
+/**
+ * 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 or else -EIO
+ **/
+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_ctrl_wr32(mbx, reg, MBOX_PF_HOLD);
+ /* force write back before check */
+ wmb();
+ if (mbx_ctrl_rd32(mbx, reg) & MBOX_PF_HOLD)
+ return 0;
+ udelay(100);
+ }
+ return -EIO;
+}
+
+/**
+ * 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 errno on failure
+ **/
+static int mucse_read_mbx_pf(struct mucse_hw *hw, u32 *msg, u16 size)
+{
+ struct mucse_mbx_info *mbx = &hw->mbx;
+ int size_inwords = size / 4;
+ u32 ctrl_reg;
+ int ret;
+ int i;
+
+ ctrl_reg = PF2FW_MBOX_CTRL(mbx);
+
+ ret = mucse_obtain_mbx_lock_pf(hw);
+ if (ret)
+ return ret;
+ for (i = 0; i < size_inwords; i++)
+ msg[i] = mbx_data_rd32(mbx, MBX_FW_PF_SHM_DATA + 4 * i);
+ /* Hw need write data_reg at last */
+ mbx_data_wr32(mbx, MBX_FW_PF_SHM_DATA, 0);
+ hw->mbx.fw_req = mucse_mbx_get_fwreq(mbx);
+ mucse_mbx_inc_pf_ack(hw);
+ mbx_ctrl_wr32(mbx, ctrl_reg, 0);
+
+ return 0;
+}
+
+/**
+ * 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
+ **/
+int mucse_read_posted_mbx(struct mucse_hw *hw, u32 *msg, u16 size)
+{
+ int ret;
+
+ ret = mucse_poll_for_msg(hw);
+ if (ret)
+ return ret;
+
+ return mucse_read_mbx_pf(hw, msg, size);
+}
+
+/**
+ * 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;
+ u32 v;
+
+ v = mbx_data_rd32(mbx, MBX_FW2PF_COUNTER);
+ hw->mbx.fw_req = v & GENMASK_U32(15, 0);
+ hw->mbx.fw_ack = (v >> 16) & GENMASK_U32(15, 0);
+ mbx_ctrl_wr32(mbx, PF2FW_MBOX_CTRL(mbx), 0);
+ mbx_ctrl_wr32(mbx, FW_PF_MBOX_MASK(mbx), GENMASK_U32(31, 16));
+}
+
+/**
+ * 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
+ */
+void mucse_init_mbx_params_pf(struct mucse_hw *hw)
+{
+ struct mucse_mbx_info *mbx = &hw->mbx;
+
+ mbx->usec_delay = 100;
+ mbx->timeout = (4 * USEC_PER_SEC) / mbx->usec_delay;
+ mbx->stats.msgs_tx = 0;
+ mbx->stats.msgs_rx = 0;
+ mbx->stats.reqs = 0;
+ mbx->stats.acks = 0;
+ mbx->size = MUCSE_MAILBOX_BYTES;
+ mutex_init(&mbx->lock);
+ mucse_mbx_reset(hw);
+}
+
+/**
+ * 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
+ **/
+int mucse_write_mbx_pf(struct mucse_hw *hw, u32 *msg, u16 size)
+{
+ struct mucse_mbx_info *mbx = &hw->mbx;
+ int size_inwords = size / 4;
+ u32 ctrl_reg;
+ int ret;
+ int i;
+
+ ctrl_reg = PF2FW_MBOX_CTRL(mbx);
+ ret = mucse_obtain_mbx_lock_pf(hw);
+ if (ret)
+ return ret;
+
+ for (i = 0; i < size_inwords; i++)
+ mbx_data_wr32(mbx, MBX_FW_PF_SHM_DATA + i * 4, msg[i]);
+
+ /* flush msg and acks as we are overwriting the message buffer */
+ hw->mbx.fw_ack = mucse_mbx_get_fwack(mbx);
+ mucse_mbx_inc_pf_req(hw);
+ mbx_ctrl_wr32(mbx, ctrl_reg, MBOX_CTRL_REQ);
+
+ return 0;
+}
+
+/**
+ * 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
+ **/
+int mucse_write_posted_mbx(struct mucse_hw *hw, u32 *msg, u16 size)
+{
+ int ret;
+
+ ret = mucse_write_mbx_pf(hw, msg, size);
+ if (ret)
+ return ret;
+ return mucse_poll_for_ack(hw);
+}
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..110c1ee025ba
--- /dev/null
+++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx.h
@@ -0,0 +1,25 @@
+/* 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_BYTES 56
+#define MBX_FW2PF_COUNTER 0
+#define MBX_PF2FW_COUNTER 4
+#define MBX_FW_PF_SHM_DATA 8
+#define FW2PF_MBOX_VEC 0
+#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
+
+int mucse_write_mbx_pf(struct mucse_hw *hw, u32 *msg, u16 size);
+int mucse_write_posted_mbx(struct mucse_hw *hw, u32 *msg, u16 size);
+void mucse_init_mbx_params_pf(struct mucse_hw *hw);
+int mucse_read_posted_mbx(struct mucse_hw *hw, u32 *msg, u16 size);
+#endif /* _RNPGBE_MBX_H */
--
2.25.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH net-next v10 3/5] net: rnpgbe: Add basic mbx ops support
2025-09-03 2:54 ` [PATCH net-next v10 3/5] net: rnpgbe: Add basic mbx ops support Dong Yibo
@ 2025-09-03 10:53 ` Vadim Fedorenko
2025-09-03 11:12 ` Yibo Dong
2025-09-03 16:44 ` Simon Horman
2025-09-03 22:24 ` Andrew Lunn
2 siblings, 1 reply; 22+ messages in thread
From: Vadim Fedorenko @ 2025-09-03 10:53 UTC (permalink / raw)
To: Dong Yibo, andrew+netdev, davem, edumazet, kuba, pabeni, horms,
corbet, gur.stavi, maddy, mpe, danishanwar, lee, gongfan1,
lorenzo, geert+renesas, Parthiban.Veerasooran, lukas.bulwahn,
alexanderduyck, richardcochran, kees, gustavoars, rdunlap
Cc: netdev, linux-doc, linux-kernel, linux-hardening
On 03/09/2025 03:54, 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 | 16 +
> .../net/ethernet/mucse/rnpgbe/rnpgbe_chip.c | 3 +
> .../net/ethernet/mucse/rnpgbe/rnpgbe_mbx.c | 393 ++++++++++++++++++
> .../net/ethernet/mucse/rnpgbe/rnpgbe_mbx.h | 25 ++
> 5 files changed, 439 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/rnpgbe_chip.c b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_chip.c
> index 179621ea09f3..f38daef752a3 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>
I don't see a reason to have string.h included into rnpgbe_chip.c
> +
> #include "rnpgbe.h"
> #include "rnpgbe_hw.h"
> +#include "rnpgbe_mbx.h"
I believe this part has to be done in the previous patch.
Please, be sure that the code can compile after every patch in the
patchset.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH net-next v10 3/5] net: rnpgbe: Add basic mbx ops support
2025-09-03 10:53 ` Vadim Fedorenko
@ 2025-09-03 11:12 ` Yibo Dong
2025-09-03 12:43 ` Vadim Fedorenko
0 siblings, 1 reply; 22+ messages in thread
From: Yibo Dong @ 2025-09-03 11:12 UTC (permalink / raw)
To: Vadim Fedorenko
Cc: andrew+netdev, davem, edumazet, kuba, pabeni, horms, corbet,
gur.stavi, maddy, mpe, danishanwar, lee, gongfan1, lorenzo,
geert+renesas, Parthiban.Veerasooran, lukas.bulwahn,
alexanderduyck, richardcochran, kees, gustavoars, rdunlap, netdev,
linux-doc, linux-kernel, linux-hardening
On Wed, Sep 03, 2025 at 11:53:17AM +0100, Vadim Fedorenko wrote:
> On 03/09/2025 03:54, 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 | 16 +
> > .../net/ethernet/mucse/rnpgbe/rnpgbe_chip.c | 3 +
> > .../net/ethernet/mucse/rnpgbe/rnpgbe_mbx.c | 393 ++++++++++++++++++
> > .../net/ethernet/mucse/rnpgbe/rnpgbe_mbx.h | 25 ++
> > 5 files changed, 439 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/rnpgbe_chip.c b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_chip.c
> > index 179621ea09f3..f38daef752a3 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>
>
> I don't see a reason to have string.h included into rnpgbe_chip.c
>
You are right, I should add it when it is used.
> > +
> > #include "rnpgbe.h"
> > #include "rnpgbe_hw.h"
> > +#include "rnpgbe_mbx.h"
>
> I believe this part has to be done in the previous patch.
> Please, be sure that the code can compile after every patch in the
> patchset.
>
You mean 'include "rnpgbe_mbx.h"'? But 'rnpgbe_mbx.h' is added in this patch.
I had compiled every patch before submission for this series. And as you
remind, I will keep check this in the future.
Thanks for your feedback.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH net-next v10 3/5] net: rnpgbe: Add basic mbx ops support
2025-09-03 11:12 ` Yibo Dong
@ 2025-09-03 12:43 ` Vadim Fedorenko
2025-09-04 2:08 ` Yibo Dong
0 siblings, 1 reply; 22+ messages in thread
From: Vadim Fedorenko @ 2025-09-03 12:43 UTC (permalink / raw)
To: Yibo Dong
Cc: andrew+netdev, davem, edumazet, kuba, pabeni, horms, corbet,
gur.stavi, maddy, mpe, danishanwar, lee, gongfan1, lorenzo,
geert+renesas, Parthiban.Veerasooran, lukas.bulwahn,
alexanderduyck, richardcochran, kees, gustavoars, rdunlap, netdev,
linux-doc, linux-kernel, linux-hardening
On 03/09/2025 12:12, Yibo Dong wrote:
> On Wed, Sep 03, 2025 at 11:53:17AM +0100, Vadim Fedorenko wrote:
>> On 03/09/2025 03:54, 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 | 16 +
>>> .../net/ethernet/mucse/rnpgbe/rnpgbe_chip.c | 3 +
>>> .../net/ethernet/mucse/rnpgbe/rnpgbe_mbx.c | 393 ++++++++++++++++++
>>> .../net/ethernet/mucse/rnpgbe/rnpgbe_mbx.h | 25 ++
>>> 5 files changed, 439 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/rnpgbe_chip.c b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_chip.c
>>> index 179621ea09f3..f38daef752a3 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>
>>
>> I don't see a reason to have string.h included into rnpgbe_chip.c
>>
>
> You are right, I should add it when it is used.
>
>>> +
>>> #include "rnpgbe.h"
>>> #include "rnpgbe_hw.h"
>>> +#include "rnpgbe_mbx.h"
>>
>> I believe this part has to be done in the previous patch.
>> Please, be sure that the code can compile after every patch in the
>> patchset.
>>
>
> You mean 'include "rnpgbe_mbx.h"'? But 'rnpgbe_mbx.h' is added in this patch.
Ok, so what's in rnpgbe_chip.c which needs rnpgbe_mbx.h to be included?
If the change is introduced later in patch 5, the move this include to
it as well.
> I had compiled every patch before submission for this series. And as you
> remind, I will keep check this in the future.
>
> Thanks for your feedback.
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH net-next v10 3/5] net: rnpgbe: Add basic mbx ops support
2025-09-03 12:43 ` Vadim Fedorenko
@ 2025-09-04 2:08 ` Yibo Dong
0 siblings, 0 replies; 22+ messages in thread
From: Yibo Dong @ 2025-09-04 2:08 UTC (permalink / raw)
To: Vadim Fedorenko
Cc: andrew+netdev, davem, edumazet, kuba, pabeni, horms, corbet,
gur.stavi, maddy, mpe, danishanwar, lee, gongfan1, lorenzo,
geert+renesas, Parthiban.Veerasooran, lukas.bulwahn,
alexanderduyck, richardcochran, kees, gustavoars, rdunlap, netdev,
linux-doc, linux-kernel, linux-hardening
On Wed, Sep 03, 2025 at 01:43:02PM +0100, Vadim Fedorenko wrote:
> On 03/09/2025 12:12, Yibo Dong wrote:
> > On Wed, Sep 03, 2025 at 11:53:17AM +0100, Vadim Fedorenko wrote:
> > > On 03/09/2025 03:54, 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 | 16 +
> > > > .../net/ethernet/mucse/rnpgbe/rnpgbe_chip.c | 3 +
> > > > .../net/ethernet/mucse/rnpgbe/rnpgbe_mbx.c | 393 ++++++++++++++++++
> > > > .../net/ethernet/mucse/rnpgbe/rnpgbe_mbx.h | 25 ++
> > > > 5 files changed, 439 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/rnpgbe_chip.c b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_chip.c
> > > > index 179621ea09f3..f38daef752a3 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>
> > >
> > > I don't see a reason to have string.h included into rnpgbe_chip.c
> > >
> >
> > You are right, I should add it when it is used.
> >
> > > > +
> > > > #include "rnpgbe.h"
> > > > #include "rnpgbe_hw.h"
> > > > +#include "rnpgbe_mbx.h"
> > >
> > > I believe this part has to be done in the previous patch.
> > > Please, be sure that the code can compile after every patch in the
> > > patchset.
> > >
> >
> > You mean 'include "rnpgbe_mbx.h"'? But 'rnpgbe_mbx.h' is added in this patch.
>
> Ok, so what's in rnpgbe_chip.c which needs rnpgbe_mbx.h to be included?
> If the change is introduced later in patch 5, the move this include to
> it as well.
>
You are right, I will move it to patch 5.
> > I had compiled every patch before submission for this series. And as you
> > remind, I will keep check this in the future.
> >
> > Thanks for your feedback.
> >
>
>
Thanks for your feedback.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH net-next v10 3/5] net: rnpgbe: Add basic mbx ops support
2025-09-03 2:54 ` [PATCH net-next v10 3/5] net: rnpgbe: Add basic mbx ops support Dong Yibo
2025-09-03 10:53 ` Vadim Fedorenko
@ 2025-09-03 16:44 ` Simon Horman
2025-09-04 2:38 ` Yibo Dong
2025-09-03 22:24 ` Andrew Lunn
2 siblings, 1 reply; 22+ messages in thread
From: Simon Horman @ 2025-09-03 16:44 UTC (permalink / raw)
To: Dong Yibo
Cc: andrew+netdev, davem, edumazet, kuba, pabeni, corbet, gur.stavi,
maddy, mpe, danishanwar, lee, gongfan1, lorenzo, geert+renesas,
Parthiban.Veerasooran, lukas.bulwahn, alexanderduyck,
richardcochran, kees, gustavoars, rdunlap, vadim.fedorenko,
netdev, linux-doc, linux-kernel, linux-hardening
On Wed, Sep 03, 2025 at 10:54:28AM +0800, Dong Yibo wrote:
> Initialize basic mbx function.
>
> Signed-off-by: Dong Yibo <dong100@mucse.com>
...
> +/**
> + * 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;
> + u16 req;
> + u32 v;
> +
> + v = mbx_data_rd32(mbx, MBX_PF2FW_COUNTER);
> + req = (v & GENMASK_U32(15, 0));
nit1: Unnecessary parentheses
nit2: I would have used FIELD_GET here in conjunction with something like.
#define MBX_PF2FW_COUNTER_MASK GENMASK_U32(15, 0)
> + req++;
> + v &= GENMASK_U32(31, 16);
> + v |= req;
And using FIELD_PREP is probably more succinct here.
Likewise in the following function
> + mbx_data_wr32(mbx, MBX_PF2FW_COUNTER, 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;
> + u16 ack;
> + u32 v;
> +
> + v = mbx_data_rd32(mbx, MBX_PF2FW_COUNTER);
> + ack = (v >> 16) & GENMASK_U32(15, 0);
> + ack++;
> + v &= GENMASK_U32(15, 0);
> + v |= (ack << 16);
> + mbx_data_wr32(mbx, MBX_PF2FW_COUNTER, v);
> + hw->mbx.stats.msgs_rx++;
> +}
...
> +/**
> + * 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;
> + u32 v;
> +
> + v = mbx_data_rd32(mbx, MBX_FW2PF_COUNTER);
> + hw->mbx.fw_req = v & GENMASK_U32(15, 0);
> + hw->mbx.fw_ack = (v >> 16) & GENMASK_U32(15, 0);
I'd use FIELD_GET here too.
> + mbx_ctrl_wr32(mbx, PF2FW_MBOX_CTRL(mbx), 0);
> + mbx_ctrl_wr32(mbx, FW_PF_MBOX_MASK(mbx), GENMASK_U32(31, 16));
> +}
...
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH net-next v10 3/5] net: rnpgbe: Add basic mbx ops support
2025-09-03 16:44 ` Simon Horman
@ 2025-09-04 2:38 ` Yibo Dong
0 siblings, 0 replies; 22+ messages in thread
From: Yibo Dong @ 2025-09-04 2:38 UTC (permalink / raw)
To: Simon Horman
Cc: andrew+netdev, davem, edumazet, kuba, pabeni, corbet, gur.stavi,
maddy, mpe, danishanwar, lee, gongfan1, lorenzo, geert+renesas,
Parthiban.Veerasooran, lukas.bulwahn, alexanderduyck,
richardcochran, kees, gustavoars, rdunlap, vadim.fedorenko,
netdev, linux-doc, linux-kernel, linux-hardening
On Wed, Sep 03, 2025 at 05:44:31PM +0100, Simon Horman wrote:
> On Wed, Sep 03, 2025 at 10:54:28AM +0800, Dong Yibo wrote:
> > Initialize basic mbx function.
> >
> > Signed-off-by: Dong Yibo <dong100@mucse.com>
>
> ...
>
> > +/**
> > + * 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;
> > + u16 req;
> > + u32 v;
> > +
> > + v = mbx_data_rd32(mbx, MBX_PF2FW_COUNTER);
> > + req = (v & GENMASK_U32(15, 0));
>
> nit1: Unnecessary parentheses
> nit2: I would have used FIELD_GET here in conjunction with something like.
>
> #define MBX_PF2FW_COUNTER_MASK GENMASK_U32(15, 0)
>
Got it, maybe update it like this?
req = FIELD_GET(GENMASK_U32(15, 0), v);
> > + req++;
> > + v &= GENMASK_U32(31, 16);
> > + v |= req;
>
> And using FIELD_PREP is probably more succinct here.
>
> Likewise in the following function
>
Got it, maybe update it like this?
req++;
v = (v & ~GENMASK_U32(15, 0)) | FIELD_PREP(GENMASK_U32(15, 0), req);
> > + mbx_data_wr32(mbx, MBX_PF2FW_COUNTER, 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;
> > + u16 ack;
> > + u32 v;
> > +
> > + v = mbx_data_rd32(mbx, MBX_PF2FW_COUNTER);
> > + ack = (v >> 16) & GENMASK_U32(15, 0);
> > + ack++;
> > + v &= GENMASK_U32(15, 0);
> > + v |= (ack << 16);
> > + mbx_data_wr32(mbx, MBX_PF2FW_COUNTER, v);
> > + hw->mbx.stats.msgs_rx++;
> > +}
>
> ...
>
Maybe like this ?
...
v = mbx_data_rd32(mbx, MBX_PF2FW_COUNTER);
ack = FIELD_GET(GENMASK_U32(31, 16), v);
ack++;
v = (v & GENMASK_U32(15, 0)) | FIELD_PREP(GENMASK_U32(31, 16), ack);
mbx_data_wr32(mbx, MBX_PF2FW_COUNTER, v);
...
> > +/**
> > + * 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;
> > + u32 v;
> > +
> > + v = mbx_data_rd32(mbx, MBX_FW2PF_COUNTER);
> > + hw->mbx.fw_req = v & GENMASK_U32(15, 0);
> > + hw->mbx.fw_ack = (v >> 16) & GENMASK_U32(15, 0);
>
> I'd use FIELD_GET here too.
>
Maybe like this?
hw->mbx.fw_req = FIELD_GET(GENMASK_U32(15, 0), v);
hw->mbx.fw_ack = FIELD_GET(GENMASK_U32(31, 16), v);
> > + mbx_ctrl_wr32(mbx, PF2FW_MBOX_CTRL(mbx), 0);
> > + mbx_ctrl_wr32(mbx, FW_PF_MBOX_MASK(mbx), GENMASK_U32(31, 16));
> > +}
>
> ...
>
Thanks for your feedback.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH net-next v10 3/5] net: rnpgbe: Add basic mbx ops support
2025-09-03 2:54 ` [PATCH net-next v10 3/5] net: rnpgbe: Add basic mbx ops support Dong Yibo
2025-09-03 10:53 ` Vadim Fedorenko
2025-09-03 16:44 ` Simon Horman
@ 2025-09-03 22:24 ` Andrew Lunn
2025-09-04 3:19 ` Yibo Dong
2 siblings, 1 reply; 22+ messages in thread
From: Andrew Lunn @ 2025-09-03 22:24 UTC (permalink / raw)
To: Dong Yibo
Cc: andrew+netdev, davem, edumazet, kuba, pabeni, horms, corbet,
gur.stavi, maddy, mpe, danishanwar, lee, gongfan1, lorenzo,
geert+renesas, Parthiban.Veerasooran, lukas.bulwahn,
alexanderduyck, richardcochran, kees, gustavoars, rdunlap,
vadim.fedorenko, netdev, linux-doc, linux-kernel, linux-hardening
> struct mucse_mbx_info {
> + 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;
> /* fw <--> pf mbx */
> u32 fw_pf_shm_base;
> u32 pf2fw_mbox_ctrl;
> +/**
> + * 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 or else -EIO
> + **/
> +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_ctrl_wr32(mbx, reg, MBOX_PF_HOLD);
> + /* force write back before check */
> + wmb();
> + if (mbx_ctrl_rd32(mbx, reg) & MBOX_PF_HOLD)
> + return 0;
> + udelay(100);
> + }
> + return -EIO;
> +}
If there is a function which obtains a lock, there is normally a
function which releases a lock. But i don't see it.
> +void mucse_init_mbx_params_pf(struct mucse_hw *hw)
> +{
> + struct mucse_mbx_info *mbx = &hw->mbx;
> +
> + mbx->usec_delay = 100;
> + mbx->timeout = (4 * USEC_PER_SEC) / mbx->usec_delay;
> + mbx->stats.msgs_tx = 0;
> + mbx->stats.msgs_rx = 0;
> + mbx->stats.reqs = 0;
> + mbx->stats.acks = 0;
> + mbx->size = MUCSE_MAILBOX_BYTES;
> + mutex_init(&mbx->lock);
And this mutex never seems to be used anywhere. What is it supposed to
be protecting?
Andrew
---
pw-bot: cr
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH net-next v10 3/5] net: rnpgbe: Add basic mbx ops support
2025-09-03 22:24 ` Andrew Lunn
@ 2025-09-04 3:19 ` Yibo Dong
2025-09-04 12:05 ` Andrew Lunn
0 siblings, 1 reply; 22+ messages in thread
From: Yibo Dong @ 2025-09-04 3:19 UTC (permalink / raw)
To: Andrew Lunn
Cc: andrew+netdev, davem, edumazet, kuba, pabeni, horms, corbet,
gur.stavi, maddy, mpe, danishanwar, lee, gongfan1, lorenzo,
geert+renesas, Parthiban.Veerasooran, lukas.bulwahn,
alexanderduyck, richardcochran, kees, gustavoars, rdunlap,
vadim.fedorenko, netdev, linux-doc, linux-kernel, linux-hardening
On Thu, Sep 04, 2025 at 12:24:17AM +0200, Andrew Lunn wrote:
> > struct mucse_mbx_info {
> > + 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;
> > /* fw <--> pf mbx */
> > u32 fw_pf_shm_base;
> > u32 pf2fw_mbox_ctrl;
>
> > +/**
> > + * 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 or else -EIO
> > + **/
> > +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_ctrl_wr32(mbx, reg, MBOX_PF_HOLD);
> > + /* force write back before check */
> > + wmb();
> > + if (mbx_ctrl_rd32(mbx, reg) & MBOX_PF_HOLD)
> > + return 0;
> > + udelay(100);
> > + }
> > + return -EIO;
> > +}
>
> If there is a function which obtains a lock, there is normally a
> function which releases a lock. But i don't see it.
>
The lock is relased when send MBOX_CTRL_REQ in mucse_write_mbx_pf:
mbx_ctrl_wr32(mbx, ctrl_reg, MBOX_CTRL_REQ);
Set MBOX_PF_HOLD(bit3) to hold the lock, clear bit3 to release, and set
MBOX_CTRL_REQ(bit0) to send the req. req and lock are different bits in
one register. So we send the req along with releasing lock (set bit0 and
clear bit3).
Maybe I should add comment like this?
/* send the req along with releasing the lock */
mbx_ctrl_wr32(mbx, ctrl_reg, MBOX_CTRL_REQ);
> > +void mucse_init_mbx_params_pf(struct mucse_hw *hw)
> > +{
> > + struct mucse_mbx_info *mbx = &hw->mbx;
> > +
> > + mbx->usec_delay = 100;
> > + mbx->timeout = (4 * USEC_PER_SEC) / mbx->usec_delay;
> > + mbx->stats.msgs_tx = 0;
> > + mbx->stats.msgs_rx = 0;
> > + mbx->stats.reqs = 0;
> > + mbx->stats.acks = 0;
> > + mbx->size = MUCSE_MAILBOX_BYTES;
> > + mutex_init(&mbx->lock);
>
> And this mutex never seems to be used anywhere. What is it supposed to
> be protecting?
>
mbx->lock is used in patch5, to ensure that only one uses mbx.
I will move it to patch5.
> Andrew
>
> ---
> pw-bot: cr
>
Thanks for your feedback.
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH net-next v10 3/5] net: rnpgbe: Add basic mbx ops support
2025-09-04 3:19 ` Yibo Dong
@ 2025-09-04 12:05 ` Andrew Lunn
2025-09-05 1:16 ` Yibo Dong
0 siblings, 1 reply; 22+ messages in thread
From: Andrew Lunn @ 2025-09-04 12:05 UTC (permalink / raw)
To: Yibo Dong
Cc: andrew+netdev, davem, edumazet, kuba, pabeni, horms, corbet,
gur.stavi, maddy, mpe, danishanwar, lee, gongfan1, lorenzo,
geert+renesas, Parthiban.Veerasooran, lukas.bulwahn,
alexanderduyck, richardcochran, kees, gustavoars, rdunlap,
vadim.fedorenko, netdev, linux-doc, linux-kernel, linux-hardening
On Thu, Sep 04, 2025 at 11:19:48AM +0800, Yibo Dong wrote:
> On Thu, Sep 04, 2025 at 12:24:17AM +0200, Andrew Lunn wrote:
> > > struct mucse_mbx_info {
> > > + 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;
> > > /* fw <--> pf mbx */
> > > u32 fw_pf_shm_base;
> > > u32 pf2fw_mbox_ctrl;
> >
> > > +/**
> > > + * 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 or else -EIO
> > > + **/
> > > +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_ctrl_wr32(mbx, reg, MBOX_PF_HOLD);
> > > + /* force write back before check */
> > > + wmb();
> > > + if (mbx_ctrl_rd32(mbx, reg) & MBOX_PF_HOLD)
> > > + return 0;
> > > + udelay(100);
> > > + }
> > > + return -EIO;
> > > +}
> >
> > If there is a function which obtains a lock, there is normally a
> > function which releases a lock. But i don't see it.
> >
>
> The lock is relased when send MBOX_CTRL_REQ in mucse_write_mbx_pf:
>
> mbx_ctrl_wr32(mbx, ctrl_reg, MBOX_CTRL_REQ);
>
> Set MBOX_PF_HOLD(bit3) to hold the lock, clear bit3 to release, and set
> MBOX_CTRL_REQ(bit0) to send the req. req and lock are different bits in
> one register. So we send the req along with releasing lock (set bit0 and
> clear bit3).
> Maybe I should add comment like this?
>
> /* send the req along with releasing the lock */
> mbx_ctrl_wr32(mbx, ctrl_reg, MBOX_CTRL_REQ);
As i said, functions like this come in pairs. obtain/release,
lock/unlock. When reading code, you want to be able to see both of the
pair in a function, to know the unlock is not missing. The kernel even
has tools which will validate all paths through a function releasing
locks. Often error paths get this wrong.
So please make this a function, give it a name which makes it obvious
it is the opposite of mucse_obtain_mbx_lock_pf().
Andrew
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH net-next v10 3/5] net: rnpgbe: Add basic mbx ops support
2025-09-04 12:05 ` Andrew Lunn
@ 2025-09-05 1:16 ` Yibo Dong
0 siblings, 0 replies; 22+ messages in thread
From: Yibo Dong @ 2025-09-05 1:16 UTC (permalink / raw)
To: Andrew Lunn
Cc: andrew+netdev, davem, edumazet, kuba, pabeni, horms, corbet,
gur.stavi, maddy, mpe, danishanwar, lee, gongfan1, lorenzo,
geert+renesas, Parthiban.Veerasooran, lukas.bulwahn,
alexanderduyck, richardcochran, kees, gustavoars, rdunlap,
vadim.fedorenko, netdev, linux-doc, linux-kernel, linux-hardening
On Thu, Sep 04, 2025 at 02:05:57PM +0200, Andrew Lunn wrote:
> On Thu, Sep 04, 2025 at 11:19:48AM +0800, Yibo Dong wrote:
> > On Thu, Sep 04, 2025 at 12:24:17AM +0200, Andrew Lunn wrote:
> > > > struct mucse_mbx_info {
> > > > + 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;
> > > > /* fw <--> pf mbx */
> > > > u32 fw_pf_shm_base;
> > > > u32 pf2fw_mbox_ctrl;
> > >
> > > > +/**
> > > > + * 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 or else -EIO
> > > > + **/
> > > > +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_ctrl_wr32(mbx, reg, MBOX_PF_HOLD);
> > > > + /* force write back before check */
> > > > + wmb();
> > > > + if (mbx_ctrl_rd32(mbx, reg) & MBOX_PF_HOLD)
> > > > + return 0;
> > > > + udelay(100);
> > > > + }
> > > > + return -EIO;
> > > > +}
> > >
> > > If there is a function which obtains a lock, there is normally a
> > > function which releases a lock. But i don't see it.
> > >
> >
> > The lock is relased when send MBOX_CTRL_REQ in mucse_write_mbx_pf:
> >
> > mbx_ctrl_wr32(mbx, ctrl_reg, MBOX_CTRL_REQ);
> >
> > Set MBOX_PF_HOLD(bit3) to hold the lock, clear bit3 to release, and set
> > MBOX_CTRL_REQ(bit0) to send the req. req and lock are different bits in
> > one register. So we send the req along with releasing lock (set bit0 and
> > clear bit3).
> > Maybe I should add comment like this?
> >
> > /* send the req along with releasing the lock */
> > mbx_ctrl_wr32(mbx, ctrl_reg, MBOX_CTRL_REQ);
>
> As i said, functions like this come in pairs. obtain/release,
> lock/unlock. When reading code, you want to be able to see both of the
> pair in a function, to know the unlock is not missing. The kernel even
> has tools which will validate all paths through a function releasing
> locks. Often error paths get this wrong.
>
> So please make this a function, give it a name which makes it obvious
> it is the opposite of mucse_obtain_mbx_lock_pf().
>
> Andrew
>
Got it, I will update this.
Thanks for your feedback.
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH net-next v10 4/5] net: rnpgbe: Add basic mbx_fw support
2025-09-03 2:54 [PATCH net-next v10 0/5] Add driver for 1Gbe network chips from MUCSE Dong Yibo
` (2 preceding siblings ...)
2025-09-03 2:54 ` [PATCH net-next v10 3/5] net: rnpgbe: Add basic mbx ops support Dong Yibo
@ 2025-09-03 2:54 ` Dong Yibo
2025-09-03 22:45 ` Andrew Lunn
2025-09-03 2:54 ` [PATCH net-next v10 5/5] net: rnpgbe: Add register_netdev Dong Yibo
4 siblings, 1 reply; 22+ messages in thread
From: Dong Yibo @ 2025-09-03 2:54 UTC (permalink / raw)
To: andrew+netdev, davem, edumazet, kuba, pabeni, horms, corbet,
gur.stavi, maddy, mpe, danishanwar, lee, gongfan1, lorenzo,
geert+renesas, Parthiban.Veerasooran, lukas.bulwahn,
alexanderduyck, richardcochran, kees, gustavoars, rdunlap,
vadim.fedorenko
Cc: netdev, linux-doc, linux-kernel, linux-hardening, dong100
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 | 1 +
.../net/ethernet/mucse/rnpgbe/rnpgbe_mbx_fw.c | 250 ++++++++++++++++++
.../net/ethernet/mucse/rnpgbe/rnpgbe_mbx_fw.h | 126 +++++++++
4 files changed, 379 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 7999bb99306b..4d2cca59fb23 100644
--- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h
+++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h
@@ -51,6 +51,7 @@ struct mucse_hw {
void __iomem *hw_addr;
struct pci_dev *pdev;
enum rnpgbe_hw_type hw_type;
+ u8 pfvfnum;
struct mucse_mbx_info mbx;
};
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..30612c577c99
--- /dev/null
+++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx_fw.c
@@ -0,0 +1,250 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright(c) 2020 - 2025 Mucse Corporation. */
+
+#include <linux/pci.h>
+#include <linux/if_ether.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 errno 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);
+ int retry_cnt = 3;
+ int err;
+
+ mutex_lock(&hw->mbx.lock);
+ err = mucse_write_posted_mbx(hw, (u32 *)req, len);
+ if (err)
+ goto out;
+ do {
+ err = mucse_read_posted_mbx(hw, (u32 *)reply,
+ sizeof(*reply));
+ if (err)
+ goto out;
+ /* mucse_write_posted_mbx return 0 means fw has
+ * received request, wait for the expect opcode
+ * reply with 'retry_cnt' times.
+ */
+ } while (--retry_cnt >= 0 && reply->opcode != req->opcode);
+out:
+ mutex_unlock(&hw->mbx.lock);
+ if (!err && retry_cnt < 0)
+ return -ETIMEDOUT;
+ if (!err && reply->error_code)
+ return -EIO;
+
+ return err;
+}
+
+/**
+ * build_get_hw_info_req - build req with GET_HW_INFO opcode
+ * @req: pointer to the cmd req structure
+ **/
+static void build_get_hw_info_req(struct mbx_fw_cmd_req *req)
+{
+ req->flags = 0;
+ req->opcode = cpu_to_le16(GET_HW_INFO);
+ req->datalen = cpu_to_le16(MBX_REQ_HDR_LEN);
+ req->reply_lo = 0;
+ req->reply_hi = 0;
+}
+
+/**
+ * mucse_mbx_get_info - Get hw info from fw
+ * @hw: pointer to the HW structure
+ *
+ * mucse_mbx_get_info tries to get hw info from hw.
+ *
+ * Return: 0 on success, negative errno on failure
+ **/
+static int mucse_mbx_get_info(struct mucse_hw *hw)
+{
+ struct mbx_fw_cmd_reply reply = {};
+ struct mbx_fw_cmd_req req = {};
+ struct hw_info info = {};
+ int err;
+
+ build_get_hw_info_req(&req);
+ err = mucse_fw_send_cmd_wait(hw, &req, &reply);
+ if (!err) {
+ memcpy(&info, &reply.hw_info, sizeof(struct hw_info));
+ hw->pfvfnum = le16_to_cpu(info.pfnum) & GENMASK_U16(7, 0);
+ }
+
+ return err;
+}
+
+/**
+ * mucse_mbx_sync_fw - Try to sync with fw
+ * @hw: pointer to the HW structure
+ *
+ * mucse_mbx_sync_fw tries to sync with fw. It is only called in
+ * probe. Nothing (register network) todo if failed.
+ * Try more times to do sync.
+ *
+ * Return: 0 on success, negative errno on failure
+ **/
+int mucse_mbx_sync_fw(struct mucse_hw *hw)
+{
+ int try_cnt = 3;
+ int err;
+
+ do {
+ err = mucse_mbx_get_info(hw);
+ if (err == -ETIMEDOUT)
+ continue;
+ break;
+ } while (try_cnt--);
+
+ return err;
+}
+
+/**
+ * build_powerup - build req with powerup opcode
+ * @req: pointer to the cmd req structure
+ * @is_powerup: true for powerup, false for powerdown
+ **/
+static void build_powerup(struct mbx_fw_cmd_req *req,
+ bool is_powerup)
+{
+ req->flags = 0;
+ req->opcode = cpu_to_le16(POWER_UP);
+ req->datalen = cpu_to_le16(sizeof(req->powerup) +
+ MBX_REQ_HDR_LEN);
+ req->reply_lo = 0;
+ req->reply_hi = 0;
+#define FIXED_VERSION 0xFFFFFFFF
+ req->powerup.version = cpu_to_le32(FIXED_VERSION);
+ if (is_powerup)
+ req->powerup.status = cpu_to_le32(1);
+ else
+ req->powerup.status = cpu_to_le32(0);
+}
+
+/**
+ * mucse_mbx_powerup - Echo fw to powerup
+ * @hw: pointer to the HW structure
+ * @is_powerup: true for powerup, false for powerdown
+ *
+ * mucse_mbx_powerup echo fw to change working frequency
+ * to normal after received true, and reduce working frequency
+ * if false.
+ *
+ * Return: 0 on success, negative errno on failure
+ **/
+int mucse_mbx_powerup(struct mucse_hw *hw, bool is_powerup)
+{
+ struct mbx_fw_cmd_req req = {};
+ int len;
+ int err;
+
+ build_powerup(&req, is_powerup);
+ len = le16_to_cpu(req.datalen);
+ mutex_lock(&hw->mbx.lock);
+
+ if (is_powerup) {
+ err = mucse_write_posted_mbx(hw, (u32 *)&req,
+ len);
+ } else {
+ err = mucse_write_mbx_pf(hw, (u32 *)&req,
+ len);
+ }
+
+ mutex_unlock(&hw->mbx.lock);
+ return err;
+}
+
+/**
+ * build_reset_hw_req - build req with RESET_HW opcode
+ * @req: pointer to the cmd req structure
+ **/
+static void build_reset_hw_req(struct mbx_fw_cmd_req *req)
+{
+ req->flags = 0;
+ req->opcode = cpu_to_le16(RESET_HW);
+ req->datalen = cpu_to_le16(MBX_REQ_HDR_LEN);
+ req->reply_lo = 0;
+ req->reply_hi = 0;
+}
+
+/**
+ * mucse_mbx_reset_hw - Posts a mbx req to reset hw
+ * @hw: pointer to the HW structure
+ *
+ * mucse_mbx_reset_hw posts a mbx req to firmware to reset hw.
+ *
+ * Return: 0 on success, negative errno on failure
+ **/
+int mucse_mbx_reset_hw(struct mucse_hw *hw)
+{
+ struct mbx_fw_cmd_reply reply = {};
+ struct mbx_fw_cmd_req req = {};
+
+ build_reset_hw_req(&req);
+ return mucse_fw_send_cmd_wait(hw, &req, &reply);
+}
+
+/**
+ * build_get_macaddress_req - build req with get_mac opcode
+ * @req: pointer to the cmd req structure
+ * @port_mask: port valid for this cmd
+ * @pfvfnum: pfvfnum for this cmd
+ **/
+static void build_get_macaddress_req(struct mbx_fw_cmd_req *req,
+ int port_mask, int pfvfnum)
+{
+ req->flags = 0;
+ req->opcode = cpu_to_le16(GET_MAC_ADDRESS);
+ req->datalen = cpu_to_le16(sizeof(req->get_mac_addr) +
+ MBX_REQ_HDR_LEN);
+ req->reply_lo = 0;
+ req->reply_hi = 0;
+ req->get_mac_addr.port_mask = cpu_to_le32(port_mask);
+ req->get_mac_addr.pfvf_num = cpu_to_le32(pfvfnum);
+}
+
+/**
+ * mucse_mbx_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
+ * @port: port index
+ *
+ * mucse_mbx_get_macaddr posts a mbx req to firmware to get mac_addr.
+ *
+ * Return: 0 on success, negative errno on failure
+ **/
+int mucse_mbx_get_macaddr(struct mucse_hw *hw, int pfvfnum,
+ u8 *mac_addr,
+ int port)
+{
+ struct mbx_fw_cmd_reply reply = {};
+ struct mbx_fw_cmd_req req = {};
+ int err;
+
+ build_get_macaddress_req(&req, BIT(port), pfvfnum);
+ err = mucse_fw_send_cmd_wait(hw, &req, &reply);
+ if (err)
+ return err;
+ if (le32_to_cpu(reply.mac_addr.ports) & BIT(port))
+ memcpy(mac_addr, reply.mac_addr.addrs[port].mac, ETH_ALEN);
+ 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..d2dcad82f775
--- /dev/null
+++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx_fw.h
@@ -0,0 +1,126 @@
+/* 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
+
+enum MUCSE_FW_CMD {
+ GET_HW_INFO = 0x0601,
+ GET_MAC_ADDRESS = 0x0602,
+ RESET_HW = 0x0603,
+ POWER_UP = 0x0803,
+};
+
+struct hw_info {
+ u8 link_stat;
+ u8 port_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_info;
+ 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;
+
+/* FW stores extended information in 'ext_info' as a 32-bit
+ * little-endian value. To make these flags easily accessible in the
+ * kernel (via named 'bitfields' instead of raw bitmask operations),
+ * we use the union's 'e_host' struct, which provides named bits
+ * (e.g., 'wol_en', 'smbus_en')
+ */
+static inline void info_update_host_endian(struct hw_info *info)
+{
+ u32 host_val = le32_to_cpu(info->ext_info);
+
+ info->e_host = *(typeof(info->e_host) *)&host_val;
+}
+
+#define FLAGS_DD BIT(0)
+#define FLAGS_ERR BIT(2)
+
+struct mbx_fw_cmd_req {
+ __le16 flags;
+ __le16 opcode;
+ __le16 datalen;
+ __le16 ret_value;
+ __le32 cookie_lo;
+ __le32 cookie_hi;
+ __le32 reply_lo;
+ __le32 reply_hi;
+ union {
+ u8 data[32];
+ struct {
+ __le32 version;
+ __le32 status;
+ } powerup;
+ struct {
+ __le32 port_mask;
+ __le32 pfvf_num;
+ } get_mac_addr;
+ };
+} __packed;
+
+struct mbx_fw_cmd_reply {
+ __le16 flags;
+ __le16 opcode;
+ __le16 error_code;
+ __le16 datalen;
+ __le32 cookie_lo;
+ __le32 cookie_hi;
+ union {
+ u8 data[40];
+ struct mac_addr {
+ __le32 ports;
+ 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_info hw_info;
+ };
+} __packed;
+
+int mucse_mbx_sync_fw(struct mucse_hw *hw);
+int mucse_mbx_powerup(struct mucse_hw *hw, bool is_powerup);
+int mucse_mbx_reset_hw(struct mucse_hw *hw);
+int mucse_mbx_get_macaddr(struct mucse_hw *hw, int pfvfnum,
+ u8 *mac_addr, int port);
+#endif /* _RNPGBE_MBX_FW_H */
--
2.25.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH net-next v10 4/5] net: rnpgbe: Add basic mbx_fw support
2025-09-03 2:54 ` [PATCH net-next v10 4/5] net: rnpgbe: Add basic mbx_fw support Dong Yibo
@ 2025-09-03 22:45 ` Andrew Lunn
2025-09-04 2:57 ` Yibo Dong
0 siblings, 1 reply; 22+ messages in thread
From: Andrew Lunn @ 2025-09-03 22:45 UTC (permalink / raw)
To: Dong Yibo
Cc: andrew+netdev, davem, edumazet, kuba, pabeni, horms, corbet,
gur.stavi, maddy, mpe, danishanwar, lee, gongfan1, lorenzo,
geert+renesas, Parthiban.Veerasooran, lukas.bulwahn,
alexanderduyck, richardcochran, kees, gustavoars, rdunlap,
vadim.fedorenko, netdev, linux-doc, linux-kernel, linux-hardening
> +/**
> + * mucse_mbx_powerup - Echo fw to powerup
> + * @hw: pointer to the HW structure
> + * @is_powerup: true for powerup, false for powerdown
> + *
> + * mucse_mbx_powerup echo fw to change working frequency
> + * to normal after received true, and reduce working frequency
> + * if false.
> + *
> + * Return: 0 on success, negative errno on failure
> + **/
> +int mucse_mbx_powerup(struct mucse_hw *hw, bool is_powerup)
> +{
> + struct mbx_fw_cmd_req req = {};
> + int len;
> + int err;
> +
> + build_powerup(&req, is_powerup);
> + len = le16_to_cpu(req.datalen);
> + mutex_lock(&hw->mbx.lock);
> +
> + if (is_powerup) {
> + err = mucse_write_posted_mbx(hw, (u32 *)&req,
> + len);
> + } else {
> + err = mucse_write_mbx_pf(hw, (u32 *)&req,
> + len);
> + }
It looks odd that this is asymmetric. Why is a different low level
function used between power up and power down?
> +int mucse_mbx_reset_hw(struct mucse_hw *hw)
> +{
> + struct mbx_fw_cmd_reply reply = {};
> + struct mbx_fw_cmd_req req = {};
> +
> + build_reset_hw_req(&req);
> + return mucse_fw_send_cmd_wait(hw, &req, &reply);
> +}
And this uses a third low level API different to power up and power
down?
Andrew
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH net-next v10 4/5] net: rnpgbe: Add basic mbx_fw support
2025-09-03 22:45 ` Andrew Lunn
@ 2025-09-04 2:57 ` Yibo Dong
0 siblings, 0 replies; 22+ messages in thread
From: Yibo Dong @ 2025-09-04 2:57 UTC (permalink / raw)
To: Andrew Lunn
Cc: andrew+netdev, davem, edumazet, kuba, pabeni, horms, corbet,
gur.stavi, maddy, mpe, danishanwar, lee, gongfan1, lorenzo,
geert+renesas, Parthiban.Veerasooran, lukas.bulwahn,
alexanderduyck, richardcochran, kees, gustavoars, rdunlap,
vadim.fedorenko, netdev, linux-doc, linux-kernel, linux-hardening
On Thu, Sep 04, 2025 at 12:45:43AM +0200, Andrew Lunn wrote:
> > +/**
> > + * mucse_mbx_powerup - Echo fw to powerup
> > + * @hw: pointer to the HW structure
> > + * @is_powerup: true for powerup, false for powerdown
> > + *
> > + * mucse_mbx_powerup echo fw to change working frequency
> > + * to normal after received true, and reduce working frequency
> > + * if false.
> > + *
> > + * Return: 0 on success, negative errno on failure
> > + **/
> > +int mucse_mbx_powerup(struct mucse_hw *hw, bool is_powerup)
> > +{
> > + struct mbx_fw_cmd_req req = {};
> > + int len;
> > + int err;
> > +
> > + build_powerup(&req, is_powerup);
> > + len = le16_to_cpu(req.datalen);
> > + mutex_lock(&hw->mbx.lock);
> > +
> > + if (is_powerup) {
> > + err = mucse_write_posted_mbx(hw, (u32 *)&req,
> > + len);
> > + } else {
> > + err = mucse_write_mbx_pf(hw, (u32 *)&req,
> > + len);
> > + }
>
> It looks odd that this is asymmetric. Why is a different low level
> function used between power up and power down?
>
> > +int mucse_mbx_reset_hw(struct mucse_hw *hw)
> > +{
> > + struct mbx_fw_cmd_reply reply = {};
> > + struct mbx_fw_cmd_req req = {};
> > +
> > + build_reset_hw_req(&req);
> > + return mucse_fw_send_cmd_wait(hw, &req, &reply);
> > +}
>
> And this uses a third low level API different to power up and power
> down?
>
> Andrew
>
There are 3 APIs, they has some different ....
1. mucse_write_mbx_pf just sends mbx.
/**
* mucse_write_mbx_pf - Place a message in the mailbox
* @hw: pointer to the HW structure
* @msg: the message buffer
* @size: length of buffer
*
*/
2. mucse_write_posted_mbx sends mbx and wait fw read out.
/**
* 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
*/
powerdown is called when driver is removed, nothing to do with fw later,
no need to wait fw's ack.
Of course, it can use mucse_write_posted_mbx to wait when
powerdown if you want me to do it?
3. mucse_fw_send_cmd_wait sends mbx, wait fw read out, and wait for
response.
/**
* 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
*/
Thanks for feedback.
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH net-next v10 5/5] net: rnpgbe: Add register_netdev
2025-09-03 2:54 [PATCH net-next v10 0/5] Add driver for 1Gbe network chips from MUCSE Dong Yibo
` (3 preceding siblings ...)
2025-09-03 2:54 ` [PATCH net-next v10 4/5] net: rnpgbe: Add basic mbx_fw support Dong Yibo
@ 2025-09-03 2:54 ` Dong Yibo
2025-09-03 22:53 ` Andrew Lunn
4 siblings, 1 reply; 22+ messages in thread
From: Dong Yibo @ 2025-09-03 2:54 UTC (permalink / raw)
To: andrew+netdev, davem, edumazet, kuba, pabeni, horms, corbet,
gur.stavi, maddy, mpe, danishanwar, lee, gongfan1, lorenzo,
geert+renesas, Parthiban.Veerasooran, lukas.bulwahn,
alexanderduyck, richardcochran, kees, gustavoars, rdunlap,
vadim.fedorenko
Cc: netdev, linux-doc, linux-kernel, linux-hardening, dong100
Initialize get mac from hw, register the netdev.
Signed-off-by: Dong Yibo <dong100@mucse.com>
---
drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h | 24 +++++
.../net/ethernet/mucse/rnpgbe/rnpgbe_chip.c | 86 ++++++++++++++++++
drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h | 2 +
.../net/ethernet/mucse/rnpgbe/rnpgbe_main.c | 89 +++++++++++++++++++
4 files changed, 201 insertions(+)
diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h
index 4d2cca59fb23..92bd3ba76c72 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;
@@ -24,6 +25,10 @@ enum rnpgbe_hw_type {
rnpgbe_hw_unknown
};
+struct mucse_dma_info {
+ void __iomem *dma_base_addr;
+};
+
struct mucse_mbx_stats {
u32 msgs_tx;
u32 msgs_rx;
@@ -47,12 +52,28 @@ struct mucse_mbx_info {
u32 fw2pf_mbox_vec;
};
+struct mucse_hw;
+
+struct mucse_hw_operations {
+ int (*reset_hw)(struct mucse_hw *hw);
+ int (*get_perm_mac)(struct mucse_hw *hw);
+ int (*echo_fw_status)(struct mucse_hw *hw, bool enable, int mode);
+};
+
+enum {
+ mucse_fw_powerup,
+};
+
struct mucse_hw {
void __iomem *hw_addr;
struct pci_dev *pdev;
enum rnpgbe_hw_type hw_type;
u8 pfvfnum;
+ const struct mucse_hw_operations *ops;
+ struct mucse_dma_info dma;
struct mucse_mbx_info mbx;
+ int port;
+ u8 perm_addr[ETH_ALEN];
};
struct mucse {
@@ -72,4 +93,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 rnpgbe_dma_wr32(dma, reg, val) \
+ writel((val), (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 f38daef752a3..1c70653545f2 100644
--- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_chip.c
+++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_chip.c
@@ -1,11 +1,91 @@
// SPDX-License-Identifier: GPL-2.0
/* Copyright(c) 2020 - 2025 Mucse Corporation. */
+#include <linux/pci.h>
#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
+ *
+ * rnpgbe_get_permanent_mac tries to get mac from hw.
+ * It use eth_random_addr if failed.
+ *
+ * Return: 0 on success, negative errno on failure
+ **/
+static int rnpgbe_get_permanent_mac(struct mucse_hw *hw)
+{
+ struct device *dev = &hw->pdev->dev;
+ u8 *mac_addr = hw->perm_addr;
+ int err;
+
+ err = mucse_mbx_get_macaddr(hw, hw->pfvfnum, mac_addr, hw->port);
+ if (err) {
+ dev_err(dev, "Failed to get MAC from FW %d\n", err);
+ return err;
+ }
+
+ if (!is_valid_ether_addr(mac_addr)) {
+ dev_err(dev, "Failed to get valid MAC from FW\n");
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+/**
+ * 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 errno on failure
+ **/
+static int rnpgbe_reset_hw_ops(struct mucse_hw *hw)
+{
+ struct mucse_dma_info *dma = &hw->dma;
+
+ rnpgbe_dma_wr32(dma, RNPGBE_DMA_AXI_EN, 0);
+ return mucse_mbx_reset_hw(hw);
+}
+
+/**
+ * rnpgbe_echo_fw_status_hw_ops - Echo fw status
+ * @hw: hw information structure
+ * @enable: true or false status
+ * @mode: status mode
+ *
+ * Return: 0 on success, negative errno on failure
+ **/
+static int rnpgbe_echo_fw_status_hw_ops(struct mucse_hw *hw,
+ bool enable,
+ int mode)
+{
+ int err;
+
+ switch (mode) {
+ case mucse_fw_powerup:
+ err = mucse_mbx_powerup(hw, enable);
+ break;
+ default:
+ err = -EINVAL;
+ }
+
+ return err;
+}
+
+static const struct mucse_hw_operations rnpgbe_hw_ops = {
+ .reset_hw = &rnpgbe_reset_hw_ops,
+ .get_perm_mac = &rnpgbe_get_permanent_mac,
+ .echo_fw_status = &rnpgbe_echo_fw_status_hw_ops,
+};
/**
* rnpgbe_init_common - Setup common attribute
@@ -13,10 +93,16 @@
**/
static void rnpgbe_init_common(struct mucse_hw *hw)
{
+ struct mucse_dma_info *dma = &hw->dma;
struct mucse_mbx_info *mbx = &hw->mbx;
+ dma->dma_base_addr = hw->hw_addr;
+
mbx->pf2fw_mbox_ctrl = GBE_PF2FW_MBX_MASK_OFFSET;
mbx->fw_pf_mbox_mask = GBE_FWPF_MBX_MASK;
+
+ hw->ops = &rnpgbe_hw_ops;
+ hw->port = 0;
}
/**
diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h
index 746dca78f1df..0ab2c328c9e9 100644
--- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h
+++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h
@@ -11,6 +11,8 @@
#define GBE_FWPF_MBX_MASK 0x5700
#define N210_FW2PF_MBX_VEC_OFFSET 0x29400
#define N210_FWPF_SHM_BASE_OFFSET 0x2d900
+/**************** DMA Registers ****************************/
+#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 25b7119d6ecb..4562aeba4a24 100644
--- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c
+++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c
@@ -9,6 +9,8 @@
#include "rnpgbe.h"
#include "rnpgbe_hw.h"
+#include "rnpgbe_mbx.h"
+#include "rnpgbe_mbx_fw.h"
static const char rnpgbe_driver_name[] = "rnpgbe";
static const struct rnpgbe_info *rnpgbe_info_tbl[] = {
@@ -35,6 +37,55 @@ 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);
+ netdev->stats.tx_dropped++;
+ 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
@@ -78,6 +129,38 @@ static int rnpgbe_add_adapter(struct pci_dev *pdev,
hw->hw_addr = hw_addr;
info->init(hw);
+ mucse_init_mbx_params_pf(hw);
+ err = hw->ops->echo_fw_status(hw, true, mucse_fw_powerup);
+ if (err) {
+ dev_warn(&pdev->dev, "Send powerup to hw failed %d\n", err);
+ dev_warn(&pdev->dev, "Maybe low performance\n");
+ }
+
+ err = mucse_mbx_sync_fw(hw);
+ if (err) {
+ dev_err(&pdev->dev, "Sync fw 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;
+ }
+ err = hw->ops->get_perm_mac(hw);
+ if (err == -EINVAL) {
+ dev_warn(&pdev->dev, "Try to use random MAC\n");
+ eth_random_addr(hw->perm_addr);
+ } else if (err) {
+ dev_err(&pdev->dev, "get perm_addr failed %d\n", err);
+ goto err_free_net;
+ }
+ eth_hw_addr_set(netdev, hw->perm_addr);
+ err = register_netdev(netdev);
+ if (err)
+ goto err_free_net;
+
return 0;
err_free_net:
@@ -145,12 +228,18 @@ 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;
+ int err;
if (!mucse)
return;
netdev = mucse->netdev;
+ unregister_netdev(netdev);
mucse->netdev = NULL;
+ err = hw->ops->echo_fw_status(hw, false, mucse_fw_powerup);
+ if (err)
+ dev_warn(&pdev->dev, "Send powerdown to hw failed %d\n", err);
free_netdev(netdev);
}
--
2.25.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH net-next v10 5/5] net: rnpgbe: Add register_netdev
2025-09-03 2:54 ` [PATCH net-next v10 5/5] net: rnpgbe: Add register_netdev Dong Yibo
@ 2025-09-03 22:53 ` Andrew Lunn
2025-09-04 3:06 ` Yibo Dong
0 siblings, 1 reply; 22+ messages in thread
From: Andrew Lunn @ 2025-09-03 22:53 UTC (permalink / raw)
To: Dong Yibo
Cc: andrew+netdev, davem, edumazet, kuba, pabeni, horms, corbet,
gur.stavi, maddy, mpe, danishanwar, lee, gongfan1, lorenzo,
geert+renesas, Parthiban.Veerasooran, lukas.bulwahn,
alexanderduyck, richardcochran, kees, gustavoars, rdunlap,
vadim.fedorenko, netdev, linux-doc, linux-kernel, linux-hardening
> * rnpgbe_add_adapter - Add netdev for this pci_dev
> * @pdev: PCI device information structure
> @@ -78,6 +129,38 @@ static int rnpgbe_add_adapter(struct pci_dev *pdev,
>
> hw->hw_addr = hw_addr;
> info->init(hw);
> + mucse_init_mbx_params_pf(hw);
> + err = hw->ops->echo_fw_status(hw, true, mucse_fw_powerup);
> + if (err) {
> + dev_warn(&pdev->dev, "Send powerup to hw failed %d\n", err);
> + dev_warn(&pdev->dev, "Maybe low performance\n");
> + }
> +
> + err = mucse_mbx_sync_fw(hw);
> + if (err) {
> + dev_err(&pdev->dev, "Sync fw failed! %d\n", err);
> + goto err_free_net;
> + }
The order here seems odd. Don't you want to synchronise the mbox
before you power up? If your are out of sync, the power up could fail,
and you keep in lower power mode?
> + 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;
> + }
> + err = hw->ops->get_perm_mac(hw);
> + if (err == -EINVAL) {
> + dev_warn(&pdev->dev, "Try to use random MAC\n");
> + eth_random_addr(hw->perm_addr);
eth_random_addr() cannot fail. So you don't try to use a random MAC
address, you are using a random MAC address/
Andrew
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH net-next v10 5/5] net: rnpgbe: Add register_netdev
2025-09-03 22:53 ` Andrew Lunn
@ 2025-09-04 3:06 ` Yibo Dong
2025-09-04 12:35 ` Andrew Lunn
0 siblings, 1 reply; 22+ messages in thread
From: Yibo Dong @ 2025-09-04 3:06 UTC (permalink / raw)
To: Andrew Lunn
Cc: andrew+netdev, davem, edumazet, kuba, pabeni, horms, corbet,
gur.stavi, maddy, mpe, danishanwar, lee, gongfan1, lorenzo,
geert+renesas, Parthiban.Veerasooran, lukas.bulwahn,
alexanderduyck, richardcochran, kees, gustavoars, rdunlap,
vadim.fedorenko, netdev, linux-doc, linux-kernel, linux-hardening
On Thu, Sep 04, 2025 at 12:53:27AM +0200, Andrew Lunn wrote:
> > * rnpgbe_add_adapter - Add netdev for this pci_dev
> > * @pdev: PCI device information structure
> > @@ -78,6 +129,38 @@ static int rnpgbe_add_adapter(struct pci_dev *pdev,
> >
> > hw->hw_addr = hw_addr;
> > info->init(hw);
> > + mucse_init_mbx_params_pf(hw);
> > + err = hw->ops->echo_fw_status(hw, true, mucse_fw_powerup);
> > + if (err) {
> > + dev_warn(&pdev->dev, "Send powerup to hw failed %d\n", err);
> > + dev_warn(&pdev->dev, "Maybe low performance\n");
> > + }
> > +
> > + err = mucse_mbx_sync_fw(hw);
> > + if (err) {
> > + dev_err(&pdev->dev, "Sync fw failed! %d\n", err);
> > + goto err_free_net;
> > + }
>
> The order here seems odd. Don't you want to synchronise the mbox
> before you power up? If your are out of sync, the power up could fail,
> and you keep in lower power mode?
>
As I explained before, powerup sends mbx and wait fw read out, but
without response data from fw. mucse_mbx_sync_fw sends mbx and wait for
the corect response from fw, after mucse_mbx_sync_fw, driver->fw
request and fw->driver response will be both ok.
> > + 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;
> > + }
> > + err = hw->ops->get_perm_mac(hw);
> > + if (err == -EINVAL) {
> > + dev_warn(&pdev->dev, "Try to use random MAC\n");
> > + eth_random_addr(hw->perm_addr);
>
> eth_random_addr() cannot fail. So you don't try to use a random MAC
> address, you are using a random MAC address/
>
> Andrew
>
Maybe update it like this?
if (err == -EINVAL) {
dev_warn(&pdev->dev, "Using a random MAC\n");
....
Thanks for your feedback.
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH net-next v10 5/5] net: rnpgbe: Add register_netdev
2025-09-04 3:06 ` Yibo Dong
@ 2025-09-04 12:35 ` Andrew Lunn
2025-09-05 1:27 ` Yibo Dong
0 siblings, 1 reply; 22+ messages in thread
From: Andrew Lunn @ 2025-09-04 12:35 UTC (permalink / raw)
To: Yibo Dong
Cc: andrew+netdev, davem, edumazet, kuba, pabeni, horms, corbet,
gur.stavi, maddy, mpe, danishanwar, lee, gongfan1, lorenzo,
geert+renesas, Parthiban.Veerasooran, lukas.bulwahn,
alexanderduyck, richardcochran, kees, gustavoars, rdunlap,
vadim.fedorenko, netdev, linux-doc, linux-kernel, linux-hardening
On Thu, Sep 04, 2025 at 11:06:21AM +0800, Yibo Dong wrote:
> On Thu, Sep 04, 2025 at 12:53:27AM +0200, Andrew Lunn wrote:
> > > * rnpgbe_add_adapter - Add netdev for this pci_dev
> > > * @pdev: PCI device information structure
> > > @@ -78,6 +129,38 @@ static int rnpgbe_add_adapter(struct pci_dev *pdev,
> > >
> > > hw->hw_addr = hw_addr;
> > > info->init(hw);
> > > + mucse_init_mbx_params_pf(hw);
> > > + err = hw->ops->echo_fw_status(hw, true, mucse_fw_powerup);
> > > + if (err) {
> > > + dev_warn(&pdev->dev, "Send powerup to hw failed %d\n", err);
> > > + dev_warn(&pdev->dev, "Maybe low performance\n");
> > > + }
> > > +
> > > + err = mucse_mbx_sync_fw(hw);
> > > + if (err) {
> > > + dev_err(&pdev->dev, "Sync fw failed! %d\n", err);
> > > + goto err_free_net;
> > > + }
> >
> > The order here seems odd. Don't you want to synchronise the mbox
> > before you power up? If your are out of sync, the power up could fail,
> > and you keep in lower power mode?
> >
>
> As I explained before, powerup sends mbx and wait fw read out, but
> without response data from fw. mucse_mbx_sync_fw sends mbx and wait for
> the corect response from fw, after mucse_mbx_sync_fw, driver->fw
> request and fw->driver response will be both ok.
Because this is logically the wrong order, this deserves a comment.
You choice of function names for the lower level functions also does
not help. It is not so easy to look at the function used to know if it
is a request/response to the firmware, or just a request without a
response.
Andrew
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH net-next v10 5/5] net: rnpgbe: Add register_netdev
2025-09-04 12:35 ` Andrew Lunn
@ 2025-09-05 1:27 ` Yibo Dong
0 siblings, 0 replies; 22+ messages in thread
From: Yibo Dong @ 2025-09-05 1:27 UTC (permalink / raw)
To: Andrew Lunn
Cc: andrew+netdev, davem, edumazet, kuba, pabeni, horms, corbet,
gur.stavi, maddy, mpe, danishanwar, lee, gongfan1, lorenzo,
geert+renesas, Parthiban.Veerasooran, lukas.bulwahn,
alexanderduyck, richardcochran, kees, gustavoars, rdunlap,
vadim.fedorenko, netdev, linux-doc, linux-kernel, linux-hardening
On Thu, Sep 04, 2025 at 02:35:06PM +0200, Andrew Lunn wrote:
> On Thu, Sep 04, 2025 at 11:06:21AM +0800, Yibo Dong wrote:
> > On Thu, Sep 04, 2025 at 12:53:27AM +0200, Andrew Lunn wrote:
> > > > * rnpgbe_add_adapter - Add netdev for this pci_dev
> > > > * @pdev: PCI device information structure
> > > > @@ -78,6 +129,38 @@ static int rnpgbe_add_adapter(struct pci_dev *pdev,
> > > >
> > > > hw->hw_addr = hw_addr;
> > > > info->init(hw);
> > > > + mucse_init_mbx_params_pf(hw);
> > > > + err = hw->ops->echo_fw_status(hw, true, mucse_fw_powerup);
> > > > + if (err) {
> > > > + dev_warn(&pdev->dev, "Send powerup to hw failed %d\n", err);
> > > > + dev_warn(&pdev->dev, "Maybe low performance\n");
> > > > + }
> > > > +
> > > > + err = mucse_mbx_sync_fw(hw);
> > > > + if (err) {
> > > > + dev_err(&pdev->dev, "Sync fw failed! %d\n", err);
> > > > + goto err_free_net;
> > > > + }
> > >
> > > The order here seems odd. Don't you want to synchronise the mbox
> > > before you power up? If your are out of sync, the power up could fail,
> > > and you keep in lower power mode?
> > >
> >
> > As I explained before, powerup sends mbx and wait fw read out, but
> > without response data from fw. mucse_mbx_sync_fw sends mbx and wait for
> > the corect response from fw, after mucse_mbx_sync_fw, driver->fw
> > request and fw->driver response will be both ok.
>
> Because this is logically the wrong order, this deserves a comment.
>
> You choice of function names for the lower level functions also does
> not help. It is not so easy to look at the function used to know if it
> is a request/response to the firmware, or just a request without a
> response.
>
> Andrew
>
Got it, I will add comment.
...
/*
* Step 1: Send power-up notification to firmware (no response expected)
* This informs firmware to initialize hardware power state, but firmware
* only acknowledges receipt without returning data. Must be done before
* synchronization as firmware may be in low-power idle state initially.
*/
mucse_init_mbx_params_pf(hw);
err = hw->ops->mbx_send_notify(hw, true, mucse_fw_powerup);
if (err) {
dev_warn(&pdev->dev, "Failed to send power-up notification to firmware: %d\n", err);
dev_warn(&pdev->dev, "Performance may be limited\n");
}
/*
* Step 2: Synchronize mailbox communication with firmware (requires response)
* After power-up, confirm firmware is ready to process requests with responses.
* This ensures subsequent request/response interactions work reliably.
*/
err = mucse_mbx_sync_fw(hw);
...
And lower lever function names will be consided to be renamed.
Thanks for your feedback.
^ permalink raw reply [flat|nested] 22+ messages in thread