* [PATCH net-next v12 0/5] Add driver for 1Gbe network chips from MUCSE
@ 2025-09-16 11:29 Dong Yibo
2025-09-16 11:29 ` [PATCH net-next v12 1/5] net: rnpgbe: Add build support for rnpgbe Dong Yibo
` (4 more replies)
0 siblings, 5 replies; 21+ messages in thread
From: Dong Yibo @ 2025-09-16 11:29 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, joerg
Cc: netdev, linux-doc, linux-kernel, linux-hardening, dong100
Hi maintainers,
This patch series adds support for MUCSE RNPGBE 1Gbps PCIe Ethernet controllers
(N500/N210 series), including build infrastructure, hardware initialization,
mailbox (MBX) communication with firmware, and basic netdev registration
(Can show mac witch is got from firmware, and tx/rx will be added later).
Series breakdown (5 patches):
01/05: net: ethernet/mucse: Add build support for rnpgbe
- Kconfig/Makefile for MUCSE vendor, basic PCI probe (no netdev)
02/05: net: ethernet/mucse: Add N500/N210 chip support
- netdev allocation, BAR mapping
03/05: net: ethernet/mucse: Add basic MBX ops for PF-FW communication
- base read/write, write with poll ack, poll and read data
04/05: net: ethernet/mucse: Add FW commands (sync, reset, MAC query)
- FW sync retry logic, MAC address retrieval, reset hw with
base mbx ops in patch4
05/05: net: ethernet/mucse: Complete netdev registration
- HW reset, MAC setup, netdev_ops registration
Changelog:
v11 -> v12:
[patch 3/5]:
1. fix switch code style.
2. rename mbx struct define 'timeout_cnt to timeout_us',
'usec_delay to delay_us'.
[patch 4/5]:
1. update function mucse_mbx_sync_fw.
[patch 5/5]:
1. update commit for rnpgbe_xmit_frame.
2. update define position for 'struct mucse_hw_operations'.
3. remove struct dma.
links:
v11: https://lore.kernel.org/netdev/20250909120906.1781444-1-dong100@mucse.com/
v10: https://lore.kernel.org/netdev/20250903025430.864836-1-dong100@mucse.com/
v9 : https://lore.kernel.org/netdev/20250828025547.568563-1-dong100@mucse.com/
v8 : https://lore.kernel.org/netdev/20250827034509.501980-1-dong100@mucse.com/
v7 : https://lore.kernel.org/netdev/20250822023453.1910972-1-dong100@mucse.com
v6 : https://lore.kernel.org/netdev/20250820092154.1643120-1-dong100@mucse.com/
v5 : https://lore.kernel.org/netdev/20250818112856.1446278-1-dong100@mucse.com/
v4 : https://lore.kernel.org/netdev/20250814073855.1060601-1-dong100@mucse.com/
v3 : https://lore.kernel.org/netdev/20250812093937.882045-1-dong100@mucse.com/
v2 : https://lore.kernel.org/netdev/20250721113238.18615-1-dong100@mucse.com/
v1 : https://lore.kernel.org/netdev/20250703014859.210110-1-dong100@mucse.com/
Dong Yibo (5):
net: rnpgbe: Add build support for rnpgbe
net: rnpgbe: Add n500/n210 chip support with BAR2 mapping
net: rnpgbe: Add basic mbx ops support
net: rnpgbe: Add basic mbx_fw support
net: rnpgbe: Add register_netdev
.../device_drivers/ethernet/index.rst | 1 +
.../device_drivers/ethernet/mucse/rnpgbe.rst | 21 +
MAINTAINERS | 8 +
drivers/net/ethernet/Kconfig | 1 +
drivers/net/ethernet/Makefile | 1 +
drivers/net/ethernet/mucse/Kconfig | 34 ++
drivers/net/ethernet/mucse/Makefile | 7 +
drivers/net/ethernet/mucse/rnpgbe/Makefile | 11 +
drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h | 75 ++++
.../net/ethernet/mucse/rnpgbe/rnpgbe_chip.c | 150 +++++++
drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h | 17 +
.../net/ethernet/mucse/rnpgbe/rnpgbe_main.c | 311 +++++++++++++
.../net/ethernet/mucse/rnpgbe/rnpgbe_mbx.c | 421 ++++++++++++++++++
.../net/ethernet/mucse/rnpgbe/rnpgbe_mbx.h | 20 +
.../net/ethernet/mucse/rnpgbe/rnpgbe_mbx_fw.c | 246 ++++++++++
.../net/ethernet/mucse/rnpgbe/rnpgbe_mbx_fw.h | 121 +++++
16 files changed, 1445 insertions(+)
create mode 100644 Documentation/networking/device_drivers/ethernet/mucse/rnpgbe.rst
create mode 100644 drivers/net/ethernet/mucse/Kconfig
create mode 100644 drivers/net/ethernet/mucse/Makefile
create mode 100644 drivers/net/ethernet/mucse/rnpgbe/Makefile
create mode 100644 drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h
create mode 100644 drivers/net/ethernet/mucse/rnpgbe/rnpgbe_chip.c
create mode 100644 drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h
create mode 100644 drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c
create mode 100644 drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx.c
create mode 100644 drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx.h
create mode 100644 drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx_fw.c
create mode 100644 drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx_fw.h
--
2.25.1
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH net-next v12 1/5] net: rnpgbe: Add build support for rnpgbe
2025-09-16 11:29 [PATCH net-next v12 0/5] Add driver for 1Gbe network chips from MUCSE Dong Yibo
@ 2025-09-16 11:29 ` Dong Yibo
2025-09-16 19:00 ` Jörg Sommer
2025-09-17 7:01 ` MD Danish Anwar
2025-09-16 11:29 ` [PATCH net-next v12 2/5] net: rnpgbe: Add n500/n210 chip support with BAR2 mapping Dong Yibo
` (3 subsequent siblings)
4 siblings, 2 replies; 21+ messages in thread
From: Dong Yibo @ 2025-09-16 11:29 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, joerg
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>
Reviewed-by: Vadim Fedorenko <vadim.fedorenko@linux.dev>
---
.../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 | 18 +++
.../net/ethernet/mucse/rnpgbe/rnpgbe_main.c | 124 ++++++++++++++++++
10 files changed, 223 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 47bc35743f22..6c1ed4d493eb 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -17304,6 +17304,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..3c37fe2534a8
--- /dev/null
+++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h
@@ -0,0 +1,18 @@
+/* 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
+};
+
+/* 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..60bbc806f17b
--- /dev/null
+++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c
@@ -0,0 +1,124 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright(c) 2020 - 2025 Mucse Corporation. */
+
+#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_n210},
+ /* 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] 21+ messages in thread
* [PATCH net-next v12 2/5] net: rnpgbe: Add n500/n210 chip support with BAR2 mapping
2025-09-16 11:29 [PATCH net-next v12 0/5] Add driver for 1Gbe network chips from MUCSE Dong Yibo
2025-09-16 11:29 ` [PATCH net-next v12 1/5] net: rnpgbe: Add build support for rnpgbe Dong Yibo
@ 2025-09-16 11:29 ` Dong Yibo
2025-09-16 19:20 ` Jörg Sommer
2025-09-17 7:02 ` MD Danish Anwar
2025-09-16 11:29 ` [PATCH net-next v12 3/5] net: rnpgbe: Add basic mbx ops support Dong Yibo
` (2 subsequent siblings)
4 siblings, 2 replies; 21+ messages in thread
From: Dong Yibo @ 2025-09-16 11:29 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, joerg
Cc: netdev, linux-doc, linux-kernel, linux-hardening, dong100
Add hardware initialization foundation for MUCSE 1Gbe controller,
including:
1. Map PCI BAR2 as hardware register base;
2. Bind PCI device to driver private data (struct mucse) and
initialize hardware context (struct mucse_hw);
3. Reserve board-specific init framework via rnpgbe_init_hw.
Signed-off-by: Dong Yibo <dong100@mucse.com>
Reviewed-by: Vadim Fedorenko <vadim.fedorenko@linux.dev>
---
drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h | 10 +++
drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h | 8 ++
.../net/ethernet/mucse/rnpgbe/rnpgbe_main.c | 79 +++++++++++++++++++
3 files changed, 97 insertions(+)
create mode 100644 drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h
diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h
index 3c37fe2534a8..3b122dd508ce 100644
--- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h
+++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h
@@ -9,6 +9,16 @@ enum rnpgbe_boards {
board_n210
};
+struct mucse_hw {
+ void __iomem *hw_addr;
+};
+
+struct mucse {
+ struct net_device *netdev;
+ struct pci_dev *pdev;
+ struct mucse_hw hw;
+};
+
/* Device IDs */
#define PCI_VENDOR_ID_MUCSE 0x8848
#define PCI_DEVICE_ID_N500_QUAD_PORT 0x8308
diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h
new file mode 100644
index 000000000000..3a779806e8be
--- /dev/null
+++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h
@@ -0,0 +1,8 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Copyright(c) 2020 - 2025 Mucse Corporation. */
+
+#ifndef _RNPGBE_HW_H
+#define _RNPGBE_HW_H
+
+#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 60bbc806f17b..0afe39621661 100644
--- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c
+++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c
@@ -2,8 +2,11 @@
/* Copyright(c) 2020 - 2025 Mucse Corporation. */
#include <linux/pci.h>
+#include <net/rtnetlink.h>
+#include <linux/etherdevice.h>
#include "rnpgbe.h"
+#include "rnpgbe_hw.h"
static const char rnpgbe_driver_name[] = "rnpgbe";
@@ -25,6 +28,54 @@ static struct pci_device_id rnpgbe_pci_tbl[] = {
{0, },
};
+/**
+ * rnpgbe_add_adapter - Add netdev for this pci_dev
+ * @pdev: PCI device information structure
+ * @board_type: board type
+ *
+ * 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,
+ int board_type)
+{
+ 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_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;
+
+ return 0;
+
+err_free_net:
+ free_netdev(netdev);
+ return err;
+}
+
/**
* rnpgbe_probe - Device initialization routine
* @pdev: PCI device information struct
@@ -37,6 +88,7 @@ static struct pci_device_id rnpgbe_pci_tbl[] = {
**/
static int rnpgbe_probe(struct pci_dev *pdev, const struct pci_device_id *id)
{
+ int board_type = id->driver_data;
int err;
err = pci_enable_device_mem(pdev);
@@ -63,6 +115,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, board_type);
+ if (err)
+ goto err_free_regions;
return 0;
err_free_regions:
@@ -72,6 +127,23 @@ 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;
+ free_netdev(netdev);
+}
+
/**
* rnpgbe_remove - Device removal routine
* @pdev: PCI device information struct
@@ -83,6 +155,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);
}
@@ -93,6 +166,12 @@ 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;
+
+ rtnl_lock();
+ netif_device_detach(netdev);
+ rtnl_unlock();
pci_disable_device(pdev);
}
--
2.25.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH net-next v12 3/5] net: rnpgbe: Add basic mbx ops support
2025-09-16 11:29 [PATCH net-next v12 0/5] Add driver for 1Gbe network chips from MUCSE Dong Yibo
2025-09-16 11:29 ` [PATCH net-next v12 1/5] net: rnpgbe: Add build support for rnpgbe Dong Yibo
2025-09-16 11:29 ` [PATCH net-next v12 2/5] net: rnpgbe: Add n500/n210 chip support with BAR2 mapping Dong Yibo
@ 2025-09-16 11:29 ` Dong Yibo
2025-09-16 18:42 ` Jörg Sommer
2025-09-16 11:29 ` [PATCH net-next v12 4/5] net: rnpgbe: Add basic mbx_fw support Dong Yibo
2025-09-16 11:29 ` [PATCH net-next v12 5/5] net: rnpgbe: Add register_netdev Dong Yibo
4 siblings, 1 reply; 21+ messages in thread
From: Dong Yibo @ 2025-09-16 11:29 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, joerg
Cc: netdev, linux-doc, linux-kernel, linux-hardening, dong100
Add fundamental mailbox (MBX) communication operations between PF
(Physical Function) and firmware for n500/n210 chips
Signed-off-by: Dong Yibo <dong100@mucse.com>
---
drivers/net/ethernet/mucse/rnpgbe/Makefile | 4 +-
drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h | 25 ++
.../net/ethernet/mucse/rnpgbe/rnpgbe_chip.c | 70 +++
drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h | 7 +
.../net/ethernet/mucse/rnpgbe/rnpgbe_main.c | 5 +
.../net/ethernet/mucse/rnpgbe/rnpgbe_mbx.c | 420 ++++++++++++++++++
.../net/ethernet/mucse/rnpgbe/rnpgbe_mbx.h | 20 +
7 files changed, 550 insertions(+), 1 deletion(-)
create mode 100644 drivers/net/ethernet/mucse/rnpgbe/rnpgbe_chip.c
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 9df536f0d04c..5fc878ada4b1 100644
--- a/drivers/net/ethernet/mucse/rnpgbe/Makefile
+++ b/drivers/net/ethernet/mucse/rnpgbe/Makefile
@@ -5,4 +5,6 @@
#
obj-$(CONFIG_MGBE) += rnpgbe.o
-rnpgbe-objs := rnpgbe_main.o
+rnpgbe-objs := rnpgbe_main.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 3b122dd508ce..7450bfc5ee98 100644
--- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h
+++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h
@@ -4,13 +4,36 @@
#ifndef _RNPGBE_H
#define _RNPGBE_H
+#include <linux/types.h>
+
enum rnpgbe_boards {
board_n500,
board_n210
};
+struct mucse_mbx_stats {
+ u32 msgs_tx; /* Number of messages sent from PF to fw */
+ u32 msgs_rx; /* Number of messages received from fw to PF */
+ u32 acks; /* Number of ACKs received from firmware */
+ u32 reqs; /* Number of requests sent to firmware */
+};
+
+struct mucse_mbx_info {
+ struct mucse_mbx_stats stats;
+ u32 timeout_us;
+ u32 delay_us;
+ u16 fw_req;
+ u16 fw_ack;
+ /* fw <--> pf mbx */
+ u32 fwpf_shm_base;
+ u32 pf2fw_mbx_ctrl;
+ u32 fwpf_mbx_mask;
+ u32 fwpf_ctrl_base;
+};
+
struct mucse_hw {
void __iomem *hw_addr;
+ struct mucse_mbx_info mbx;
};
struct mucse {
@@ -19,6 +42,8 @@ struct mucse {
struct mucse_hw hw;
};
+int rnpgbe_init_hw(struct mucse_hw *hw, int board_type);
+
/* Device IDs */
#define PCI_VENDOR_ID_MUCSE 0x8848
#define PCI_DEVICE_ID_N500_QUAD_PORT 0x8308
diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_chip.c b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_chip.c
new file mode 100644
index 000000000000..86f1c75796b0
--- /dev/null
+++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_chip.c
@@ -0,0 +1,70 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright(c) 2020 - 2025 Mucse Corporation. */
+
+#include <linux/errno.h>
+
+#include "rnpgbe.h"
+#include "rnpgbe_hw.h"
+#include "rnpgbe_mbx.h"
+
+/**
+ * rnpgbe_init_n500 - Setup n500 hw info
+ * @hw: hw information structure
+ *
+ * rnpgbe_init_n500 initializes all private
+ * structure for n500
+ **/
+static void rnpgbe_init_n500(struct mucse_hw *hw)
+{
+ struct mucse_mbx_info *mbx = &hw->mbx;
+
+ mbx->fwpf_ctrl_base = MUCSE_N500_FWPF_CTRL_BASE;
+ mbx->fwpf_shm_base = MUCSE_N500_FWPF_SHM_BASE;
+}
+
+/**
+ * rnpgbe_init_n210 - Setup n210 hw info
+ * @hw: hw information structure
+ *
+ * rnpgbe_init_n210 initializes all private
+ * structure for n210
+ **/
+static void rnpgbe_init_n210(struct mucse_hw *hw)
+{
+ struct mucse_mbx_info *mbx = &hw->mbx;
+
+ mbx->fwpf_ctrl_base = MUCSE_N210_FWPF_CTRL_BASE;
+ mbx->fwpf_shm_base = MUCSE_N210_FWPF_SHM_BASE;
+}
+
+/**
+ * rnpgbe_init_hw - Setup hw info according to board_type
+ * @hw: hw information structure
+ * @board_type: board type
+ *
+ * rnpgbe_init_hw initializes all hw data
+ *
+ * Return: 0 on success, negative errno on failure
+ **/
+int rnpgbe_init_hw(struct mucse_hw *hw, int board_type)
+{
+ struct mucse_mbx_info *mbx = &hw->mbx;
+
+ mbx->pf2fw_mbx_ctrl = MUCSE_GBE_PFFW_MBX_CTRL_OFFSET;
+ mbx->fwpf_mbx_mask = MUCSE_GBE_FWPF_MBX_MASK_OFFSET;
+
+ switch (board_type) {
+ case board_n500:
+ rnpgbe_init_n500(hw);
+ break;
+ case board_n210:
+ rnpgbe_init_n210(hw);
+ break;
+ default:
+ return -EINVAL;
+ }
+ /* init_params with mbx base */
+ mucse_init_mbx_params_pf(hw);
+
+ return 0;
+}
diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h
index 3a779806e8be..aad4cb2f4164 100644
--- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h
+++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h
@@ -4,5 +4,12 @@
#ifndef _RNPGBE_HW_H
#define _RNPGBE_HW_H
+#define MUCSE_N500_FWPF_CTRL_BASE 0x28b00
+#define MUCSE_N500_FWPF_SHM_BASE 0x2d000
+#define MUCSE_GBE_PFFW_MBX_CTRL_OFFSET 0x5500
+#define MUCSE_GBE_FWPF_MBX_MASK_OFFSET 0x5700
+#define MUCSE_N210_FWPF_CTRL_BASE 0x29400
+#define MUCSE_N210_FWPF_SHM_BASE 0x2d900
+
#define 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 0afe39621661..c6cfb54f7c59 100644
--- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c
+++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c
@@ -68,6 +68,11 @@ static int rnpgbe_add_adapter(struct pci_dev *pdev,
}
hw->hw_addr = hw_addr;
+ err = rnpgbe_init_hw(hw, board_type);
+ if (err) {
+ dev_err(&pdev->dev, "Init hw err %d\n", err);
+ goto err_free_net;
+ }
return 0;
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..7501a55c3134
--- /dev/null
+++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx.c
@@ -0,0 +1,420 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright(c) 2022 - 2025 Mucse Corporation. */
+
+#include <linux/errno.h>
+#include <linux/bitfield.h>
+#include <linux/iopoll.h>
+
+#include "rnpgbe_mbx.h"
+
+/**
+ * mbx_data_rd32 - Reads reg with base mbx->fwpf_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->fwpf_shm_base + reg);
+}
+
+/**
+ * mbx_data_wr32 - Writes value to reg with base mbx->fwpf_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->fwpf_shm_base + reg);
+}
+
+/**
+ * mbx_ctrl_rd32 - Reads reg with base mbx->fwpf_ctrl_base
+ * @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->fwpf_ctrl_base + reg);
+}
+
+/**
+ * mbx_ctrl_wr32 - Writes value to reg with base mbx->fwpf_ctrl_base
+ * @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->fwpf_ctrl_base + reg);
+}
+
+/**
+ * mucse_mbx_get_lock_pf - Write ctrl and read back lock status
+ * @hw: pointer to the HW structure
+ *
+ * Return: register value after write
+ **/
+static u32 mucse_mbx_get_lock_pf(struct mucse_hw *hw)
+{
+ struct mucse_mbx_info *mbx = &hw->mbx;
+ u32 reg = MUCSE_MBX_PF2FW_CTRL(mbx);
+
+ mbx_ctrl_wr32(mbx, reg, MUCSE_MBX_PFU);
+
+ return mbx_ctrl_rd32(mbx, reg);
+}
+
+/**
+ * mucse_obtain_mbx_lock_pf - Obtain mailbox lock
+ * @hw: pointer to the HW structure
+ *
+ * Pair with mucse_release_mbx_lock_pf()
+ * This function maybe used in an irq handler.
+ *
+ * Return: 0 on success, negative errno on failure
+ **/
+static int mucse_obtain_mbx_lock_pf(struct mucse_hw *hw)
+{
+ struct mucse_mbx_info *mbx = &hw->mbx;
+ u32 val;
+
+ return read_poll_timeout_atomic(mucse_mbx_get_lock_pf,
+ val, val & MUCSE_MBX_PFU,
+ mbx->delay_us,
+ mbx->timeout_us,
+ false, hw);
+}
+
+/**
+ * mucse_release_mbx_lock_pf - Release mailbox lock
+ * @hw: pointer to the HW structure
+ * @req: send a request or not
+ *
+ * Pair with mucse_obtain_mbx_lock_pf():
+ * - Releases the mailbox lock by clearing MUCSE_MBX_PFU bit
+ * - Simultaneously sends the request by setting MUCSE_MBX_REQ bit
+ * if req is true
+ * (Both bits are in the same mailbox control register,
+ * so operations are combined)
+ **/
+static void mucse_release_mbx_lock_pf(struct mucse_hw *hw, bool req)
+{
+ struct mucse_mbx_info *mbx = &hw->mbx;
+ u32 reg = MUCSE_MBX_PF2FW_CTRL(mbx);
+
+ if (req)
+ mbx_ctrl_wr32(mbx, reg, MUCSE_MBX_REQ);
+ else
+ mbx_ctrl_wr32(mbx, reg, 0);
+}
+
+/**
+ * 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)
+{
+ u32 val = mbx_data_rd32(mbx, MUCSE_MBX_FW2PF_CNT);
+
+ return FIELD_GET(GENMASK_U32(15, 0), val);
+}
+
+/**
+ * mucse_mbx_inc_pf_ack - Increase ack
+ * @hw: pointer to the HW structure
+ *
+ * mucse_mbx_inc_pf_ack reads pf_ack from hw, then writes
+ * 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 val;
+
+ val = mbx_data_rd32(mbx, MUCSE_MBX_PF2FW_CNT);
+ ack = FIELD_GET(GENMASK_U32(31, 16), val);
+ ack++;
+ val &= ~GENMASK_U32(31, 16);
+ val |= FIELD_PREP(GENMASK_U32(31, 16), ack);
+ mbx_data_wr32(mbx, MUCSE_MBX_PF2FW_CNT, val);
+ hw->mbx.stats.msgs_rx++;
+}
+
+/**
+ * mucse_read_mbx_pf - Read a message from the mailbox
+ * @hw: pointer to the HW structure
+ * @msg: the message buffer
+ * @size: length of buffer
+ *
+ * mucse_read_mbx_pf copies a message from the mbx 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_in_words = size / 4;
+ int ret;
+ int i;
+
+ ret = mucse_obtain_mbx_lock_pf(hw);
+ if (ret)
+ return ret;
+
+ for (i = 0; i < size_in_words; i++)
+ msg[i] = mbx_data_rd32(mbx, MUCSE_MBX_FWPF_SHM + 4 * i);
+ /* Hw needs write data_reg at last */
+ mbx_data_wr32(mbx, MUCSE_MBX_FWPF_SHM, 0);
+ /* flush reqs as we have read this request data */
+ hw->mbx.fw_req = mucse_mbx_get_fwreq(mbx);
+ mucse_mbx_inc_pf_ack(hw);
+ mucse_release_mbx_lock_pf(hw, false);
+
+ return 0;
+}
+
+/**
+ * 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 fw_req;
+
+ fw_req = mucse_mbx_get_fwreq(mbx);
+ /* chip's register is reset to 0 when rc send reset
+ * mbx command. This causes 'fw_req != 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 (fw_req != 0 && fw_req != 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 val;
+
+ return read_poll_timeout(mucse_check_for_msg_pf,
+ val, !val, mbx->delay_us,
+ mbx->timeout_us,
+ false, hw);
+}
+
+/**
+ * mucse_poll_and_read_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, negative errno on failure
+ **/
+int mucse_poll_and_read_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_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)
+{
+ u32 val = mbx_data_rd32(mbx, MUCSE_MBX_FW2PF_CNT);
+
+ return FIELD_GET(GENMASK_U32(31, 16), val);
+}
+
+/**
+ * mucse_mbx_inc_pf_req - Increase req
+ * @hw: pointer to the HW structure
+ *
+ * mucse_mbx_inc_pf_req reads pf_req from hw, then writes
+ * 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 val;
+
+ val = mbx_data_rd32(mbx, MUCSE_MBX_PF2FW_CNT);
+ req = FIELD_GET(GENMASK_U32(15, 0), val);
+ req++;
+ val &= ~GENMASK_U32(15, 0);
+ val |= FIELD_PREP(GENMASK_U32(15, 0), req);
+ mbx_data_wr32(mbx, MUCSE_MBX_PF2FW_CNT, val);
+ hw->mbx.stats.msgs_tx++;
+}
+
+/**
+ * mucse_write_mbx_pf - Place a message in the mailbox
+ * @hw: pointer to the HW structure
+ * @msg: the message buffer
+ * @size: length of buffer
+ *
+ * Return: 0 if it successfully copied message into the buffer
+ **/
+static int mucse_write_mbx_pf(struct mucse_hw *hw, u32 *msg, u16 size)
+{
+ struct mucse_mbx_info *mbx = &hw->mbx;
+ int size_in_words = size / 4;
+ int ret;
+ int i;
+
+ ret = mucse_obtain_mbx_lock_pf(hw);
+ if (ret)
+ return ret;
+
+ for (i = 0; i < size_in_words; i++)
+ mbx_data_wr32(mbx, MUCSE_MBX_FWPF_SHM + i * 4, msg[i]);
+
+ /* flush acks as we are overwriting the message buffer */
+ hw->mbx.fw_ack = mucse_mbx_get_fwack(mbx);
+ mucse_mbx_inc_pf_req(hw);
+ mucse_release_mbx_lock_pf(hw, true);
+
+ return 0;
+}
+
+/**
+ * mucse_check_for_ack_pf - Check to see if the fw 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 fw_ack;
+
+ fw_ack = mucse_mbx_get_fwack(mbx);
+ /* chip's register is reset to 0 when rc send reset
+ * mbx command. This causes '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 (fw_ack != 0 && 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,
+ * else negative errno
+ **/
+static int mucse_poll_for_ack(struct mucse_hw *hw)
+{
+ struct mucse_mbx_info *mbx = &hw->mbx;
+ int val;
+
+ return read_poll_timeout(mucse_check_for_ack_pf,
+ val, !val, mbx->delay_us,
+ mbx->timeout_us,
+ false, hw);
+}
+
+/**
+ * mucse_write_and_wait_ack_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_cnt period
+ **/
+int mucse_write_and_wait_ack_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);
+}
+
+/**
+ * mucse_mbx_reset - Reset mbx info, sync info from regs
+ * @hw: pointer to the HW structure
+ *
+ * mucse_mbx_reset resets all mbx variables to default.
+ **/
+static void mucse_mbx_reset(struct mucse_hw *hw)
+{
+ struct mucse_mbx_info *mbx = &hw->mbx;
+ u32 val;
+
+ val = mbx_data_rd32(mbx, MUCSE_MBX_FW2PF_CNT);
+ hw->mbx.fw_req = FIELD_GET(GENMASK_U32(15, 0), val);
+ hw->mbx.fw_ack = FIELD_GET(GENMASK_U32(31, 16), val);
+ mbx_ctrl_wr32(mbx, MUCSE_MBX_PF2FW_CTRL(mbx), 0);
+ mbx_ctrl_wr32(mbx, MUCSE_MBX_FWPF_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->delay_us = 100;
+ mbx->timeout_us = 4 * USEC_PER_SEC;
+ mbx->stats.msgs_tx = 0;
+ mbx->stats.msgs_rx = 0;
+ mbx->stats.reqs = 0;
+ mbx->stats.acks = 0;
+ mucse_mbx_reset(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..eac99f13b6ff
--- /dev/null
+++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx.h
@@ -0,0 +1,20 @@
+/* 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_MBX_FW2PF_CNT 0
+#define MUCSE_MBX_PF2FW_CNT 4
+#define MUCSE_MBX_FWPF_SHM 8
+#define MUCSE_MBX_PF2FW_CTRL(mbx) ((mbx)->pf2fw_mbx_ctrl)
+#define MUCSE_MBX_FWPF_MASK(mbx) ((mbx)->fwpf_mbx_mask)
+#define MUCSE_MBX_REQ BIT(0) /* Request a req to mailbox */
+#define MUCSE_MBX_PFU BIT(3) /* PF owns the mailbox buffer */
+
+int mucse_write_and_wait_ack_mbx(struct mucse_hw *hw, u32 *msg, u16 size);
+void mucse_init_mbx_params_pf(struct mucse_hw *hw);
+int mucse_poll_and_read_mbx(struct mucse_hw *hw, u32 *msg, u16 size);
+#endif /* _RNPGBE_MBX_H */
--
2.25.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH net-next v12 4/5] net: rnpgbe: Add basic mbx_fw support
2025-09-16 11:29 [PATCH net-next v12 0/5] Add driver for 1Gbe network chips from MUCSE Dong Yibo
` (2 preceding siblings ...)
2025-09-16 11:29 ` [PATCH net-next v12 3/5] net: rnpgbe: Add basic mbx ops support Dong Yibo
@ 2025-09-16 11:29 ` Dong Yibo
2025-09-17 10:45 ` Vadim Fedorenko
2025-09-16 11:29 ` [PATCH net-next v12 5/5] net: rnpgbe: Add register_netdev Dong Yibo
4 siblings, 1 reply; 21+ messages in thread
From: Dong Yibo @ 2025-09-16 11:29 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, joerg
Cc: netdev, linux-doc, linux-kernel, linux-hardening, dong100
Add fundamental firmware (FW) communication operations via PF-FW
mailbox, including:
- FW sync (via HW info query with retries)
- HW reset (post FW command to reset hardware)
- MAC address retrieval (request FW for port-specific MAC)
- Power management (powerup/powerdown notification to FW)
Signed-off-by: Dong Yibo <dong100@mucse.com>
---
drivers/net/ethernet/mucse/rnpgbe/Makefile | 3 +-
drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h | 4 +
.../net/ethernet/mucse/rnpgbe/rnpgbe_mbx.c | 1 +
.../net/ethernet/mucse/rnpgbe/rnpgbe_mbx_fw.c | 246 ++++++++++++++++++
.../net/ethernet/mucse/rnpgbe/rnpgbe_mbx_fw.h | 121 +++++++++
5 files changed, 374 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 7450bfc5ee98..41b580f2168f 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>
enum rnpgbe_boards {
board_n500,
@@ -24,6 +25,8 @@ struct mucse_mbx_info {
u32 delay_us;
u16 fw_req;
u16 fw_ack;
+ /* lock for only one use mbx */
+ struct mutex lock;
/* fw <--> pf mbx */
u32 fwpf_shm_base;
u32 pf2fw_mbx_ctrl;
@@ -34,6 +37,7 @@ struct mucse_mbx_info {
struct mucse_hw {
void __iomem *hw_addr;
struct mucse_mbx_info mbx;
+ u8 pfvfnum;
};
struct mucse {
diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx.c b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx.c
index 7501a55c3134..8998fee2a615 100644
--- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx.c
+++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx.c
@@ -416,5 +416,6 @@ void mucse_init_mbx_params_pf(struct mucse_hw *hw)
mbx->stats.msgs_rx = 0;
mbx->stats.reqs = 0;
mbx->stats.acks = 0;
+ mutex_init(&mbx->lock);
mucse_mbx_reset(hw);
}
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..0612d39869fc
--- /dev/null
+++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx_fw.c
@@ -0,0 +1,246 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright(c) 2020 - 2025 Mucse Corporation. */
+
+#include <linux/if_ether.h>
+#include <linux/bitfield.h>
+
+#include "rnpgbe.h"
+#include "rnpgbe_mbx.h"
+#include "rnpgbe_mbx_fw.h"
+
+/**
+ * mucse_fw_send_cmd_wait_resp - 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_resp 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_resp(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_and_wait_ack_mbx(hw, (u32 *)req, len);
+ if (err)
+ goto out;
+ do {
+ err = mucse_poll_and_read_mbx(hw, (u32 *)reply,
+ sizeof(*reply));
+ if (err)
+ goto out;
+ /* mucse_write_and_wait_ack_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(MUCSE_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 mucse_hw_info info = {};
+ int err;
+
+ build_get_hw_info_req(&req);
+
+ err = mucse_fw_send_cmd_wait_resp(hw, &req, &reply);
+ if (!err) {
+ memcpy(&info, &reply.hw_info, sizeof(struct mucse_hw_info));
+ mucse_hw_info_update_host_endian(&info);
+ hw->pfvfnum = FIELD_GET(GENMASK_U16(7, 0),
+ le16_to_cpu(info.pfnum));
+ }
+
+ 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);
+ } while (err == -ETIMEDOUT && 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) +
+ MUCSE_MBX_REQ_HDR_LEN);
+ req->reply_lo = 0;
+ req->reply_hi = 0;
+ /* fw needs this to reply correct cmd */
+ req->powerup.version = cpu_to_le32(GENMASK_U32(31, 0));
+ 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);
+ err = mucse_write_and_wait_ack_mbx(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(MUCSE_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.
+ * We use mucse_fw_send_cmd_wait_resp to wait hw reset ok.
+ *
+ * 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_resp(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) +
+ MUCSE_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_resp(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..9dee6029e630
--- /dev/null
+++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx_fw.h
@@ -0,0 +1,121 @@
+/* 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 "rnpgbe.h"
+
+#define MUCSE_MBX_REQ_HDR_LEN 24
+
+enum MUCSE_FW_CMD {
+ GET_HW_INFO = 0x0601,
+ GET_MAC_ADDRESS = 0x0602,
+ RESET_HW = 0x0603,
+ POWER_UP = 0x0803,
+};
+
+struct mucse_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 mucse_hw_info_update_host_endian(struct mucse_hw_info *info)
+{
+ u32 host_val = le32_to_cpu(info->ext_info);
+
+ memcpy(&info->e_host, &host_val, sizeof(info->e_host));
+}
+
+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 mucse_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] 21+ messages in thread
* [PATCH net-next v12 5/5] net: rnpgbe: Add register_netdev
2025-09-16 11:29 [PATCH net-next v12 0/5] Add driver for 1Gbe network chips from MUCSE Dong Yibo
` (3 preceding siblings ...)
2025-09-16 11:29 ` [PATCH net-next v12 4/5] net: rnpgbe: Add basic mbx_fw support Dong Yibo
@ 2025-09-16 11:29 ` Dong Yibo
2025-09-16 15:19 ` Vadim Fedorenko
2025-09-17 7:09 ` MD Danish Anwar
4 siblings, 2 replies; 21+ messages in thread
From: Dong Yibo @ 2025-09-16 11:29 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, joerg
Cc: netdev, linux-doc, linux-kernel, linux-hardening, dong100
Complete the network device (netdev) registration flow for Mucse Gbe
Ethernet chips, including:
1. Hardware state initialization:
- Send powerup notification to firmware (via echo_fw_status)
- Sync with firmware
- Reset hardware
2. MAC address handling:
- Retrieve permanent MAC from firmware (via mucse_mbx_get_macaddr)
- Fallback to random valid MAC (eth_random_addr) if not valid mac
from Fw
Signed-off-by: Dong Yibo <dong100@mucse.com>
---
drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h | 18 +++
.../net/ethernet/mucse/rnpgbe/rnpgbe_chip.c | 80 ++++++++++++++
drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h | 2 +
.../net/ethernet/mucse/rnpgbe/rnpgbe_main.c | 103 ++++++++++++++++++
4 files changed, 203 insertions(+)
diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h
index 41b580f2168f..4c4b2f13cb4a 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>
enum rnpgbe_boards {
board_n500,
@@ -34,12 +35,26 @@ struct mucse_mbx_info {
u32 fwpf_ctrl_base;
};
+enum {
+ mucse_fw_powerup,
+};
+
struct mucse_hw {
void __iomem *hw_addr;
+ struct pci_dev *pdev;
+ const struct mucse_hw_operations *ops;
struct mucse_mbx_info mbx;
+ int port;
+ u8 perm_addr[ETH_ALEN];
u8 pfvfnum;
};
+struct mucse_hw_operations {
+ int (*reset_hw)(struct mucse_hw *hw);
+ int (*get_perm_mac)(struct mucse_hw *hw);
+ int (*mbx_send_notify)(struct mucse_hw *hw, bool enable, int mode);
+};
+
struct mucse {
struct net_device *netdev;
struct pci_dev *pdev;
@@ -54,4 +69,7 @@ int rnpgbe_init_hw(struct mucse_hw *hw, int board_type);
#define PCI_DEVICE_ID_N500_DUAL_PORT 0x8318
#define PCI_DEVICE_ID_N210 0x8208
#define PCI_DEVICE_ID_N210L 0x820a
+
+#define mucse_hw_wr32(hw, reg, val) \
+ writel((val), (hw)->hw_addr + (reg))
#endif /* _RNPGBE_H */
diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_chip.c b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_chip.c
index 86f1c75796b0..667e372387a2 100644
--- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_chip.c
+++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_chip.c
@@ -1,11 +1,88 @@
// SPDX-License-Identifier: GPL-2.0
/* Copyright(c) 2020 - 2025 Mucse Corporation. */
+#include <linux/pci.h>
#include <linux/errno.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
+ *
+ * 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 - Do a hardware reset
+ * @hw: hw information structure
+ *
+ * rnpgbe_reset 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(struct mucse_hw *hw)
+{
+ mucse_hw_wr32(hw, RNPGBE_DMA_AXI_EN, 0);
+ return mucse_mbx_reset_hw(hw);
+}
+
+/**
+ * rnpgbe_mbx_send_notify - 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_mbx_send_notify(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,
+ .get_perm_mac = rnpgbe_get_permanent_mac,
+ .mbx_send_notify = rnpgbe_mbx_send_notify,
+};
/**
* rnpgbe_init_n500 - Setup n500 hw info
@@ -50,6 +127,9 @@ int rnpgbe_init_hw(struct mucse_hw *hw, int board_type)
{
struct mucse_mbx_info *mbx = &hw->mbx;
+ hw->ops = &rnpgbe_hw_ops;
+ hw->port = 0;
+
mbx->pf2fw_mbx_ctrl = MUCSE_GBE_PFFW_MBX_CTRL_OFFSET;
mbx->fwpf_mbx_mask = MUCSE_GBE_FWPF_MBX_MASK_OFFSET;
diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h
index aad4cb2f4164..8d0f6352a251 100644
--- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h
+++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h
@@ -11,5 +11,7 @@
#define MUCSE_N210_FWPF_CTRL_BASE 0x29400
#define MUCSE_N210_FWPF_SHM_BASE 0x2d900
+#define RNPGBE_DMA_AXI_EN 0x0010
+
#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 c6cfb54f7c59..d8b9e7e3b01c 100644
--- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c
+++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c
@@ -7,6 +7,7 @@
#include "rnpgbe.h"
#include "rnpgbe_hw.h"
+#include "rnpgbe_mbx_fw.h"
static const char rnpgbe_driver_name[] = "rnpgbe";
@@ -28,6 +29,56 @@ 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
+ **/
+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
+ **/
+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
@@ -68,11 +119,55 @@ static int rnpgbe_add_adapter(struct pci_dev *pdev,
}
hw->hw_addr = hw_addr;
+ hw->pdev = pdev;
+
err = rnpgbe_init_hw(hw, board_type);
if (err) {
dev_err(&pdev->dev, "Init hw err %d\n", err);
goto err_free_net;
}
+ /* Step 1: Send power-up notification to firmware (no response expected)
+ * This informs firmware to initialize hardware power state, but
+ * firmware only acknowledges receipt without returning data. Must be
+ * done before synchronization as firmware may be in low-power idle
+ * state initially.
+ */
+ err = hw->ops->mbx_send_notify(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");
+ }
+ /* 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);
+ if (err) {
+ dev_err(&pdev->dev, "Sync fw failed! %d\n", err);
+ goto err_free_net;
+ }
+
+ netdev->netdev_ops = &rnpgbe_netdev_ops;
+ 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, "Using 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;
@@ -141,11 +236,17 @@ 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);
+ err = hw->ops->mbx_send_notify(hw, false, mucse_fw_powerup);
+ if (err)
+ dev_warn(&pdev->dev, "Send powerdown to hw failed %d\n", err);
free_netdev(netdev);
}
@@ -176,6 +277,8 @@ static void rnpgbe_dev_shutdown(struct pci_dev *pdev)
rtnl_lock();
netif_device_detach(netdev);
+ if (netif_running(netdev))
+ rnpgbe_close(netdev);
rtnl_unlock();
pci_disable_device(pdev);
}
--
2.25.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH net-next v12 5/5] net: rnpgbe: Add register_netdev
2025-09-16 11:29 ` [PATCH net-next v12 5/5] net: rnpgbe: Add register_netdev Dong Yibo
@ 2025-09-16 15:19 ` Vadim Fedorenko
2025-09-17 7:09 ` MD Danish Anwar
1 sibling, 0 replies; 21+ messages in thread
From: Vadim Fedorenko @ 2025-09-16 15:19 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, joerg
Cc: netdev, linux-doc, linux-kernel, linux-hardening
On 16/09/2025 12:29, Dong Yibo wrote:
> Complete the network device (netdev) registration flow for Mucse Gbe
> Ethernet chips, including:
> 1. Hardware state initialization:
> - Send powerup notification to firmware (via echo_fw_status)
> - Sync with firmware
> - Reset hardware
> 2. MAC address handling:
> - Retrieve permanent MAC from firmware (via mucse_mbx_get_macaddr)
> - Fallback to random valid MAC (eth_random_addr) if not valid mac
> from Fw
>
> Signed-off-by: Dong Yibo <dong100@mucse.com>
Reviewed-by: Vadim Fedorenko <vadim.fedorenko@linux.dev>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH net-next v12 3/5] net: rnpgbe: Add basic mbx ops support
2025-09-16 11:29 ` [PATCH net-next v12 3/5] net: rnpgbe: Add basic mbx ops support Dong Yibo
@ 2025-09-16 18:42 ` Jörg Sommer
2025-09-16 19:33 ` Andrew Lunn
2025-09-17 2:55 ` Yibo Dong
0 siblings, 2 replies; 21+ messages in thread
From: Jörg Sommer @ 2025-09-16 18:42 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
[-- Attachment #1: Type: text/plain, Size: 21412 bytes --]
Hi,
only some minor suggestions.
Dong Yibo schrieb am Di 16. Sep, 19:29 (+0800):
> Add fundamental mailbox (MBX) communication operations between PF
> (Physical Function) and firmware for n500/n210 chips
>
> Signed-off-by: Dong Yibo <dong100@mucse.com>
> ---
> drivers/net/ethernet/mucse/rnpgbe/Makefile | 4 +-
> drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h | 25 ++
> .../net/ethernet/mucse/rnpgbe/rnpgbe_chip.c | 70 +++
> drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h | 7 +
> .../net/ethernet/mucse/rnpgbe/rnpgbe_main.c | 5 +
> .../net/ethernet/mucse/rnpgbe/rnpgbe_mbx.c | 420 ++++++++++++++++++
> .../net/ethernet/mucse/rnpgbe/rnpgbe_mbx.h | 20 +
> 7 files changed, 550 insertions(+), 1 deletion(-)
> create mode 100644 drivers/net/ethernet/mucse/rnpgbe/rnpgbe_chip.c
> 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 9df536f0d04c..5fc878ada4b1 100644
> --- a/drivers/net/ethernet/mucse/rnpgbe/Makefile
> +++ b/drivers/net/ethernet/mucse/rnpgbe/Makefile
> @@ -5,4 +5,6 @@
> #
>
> obj-$(CONFIG_MGBE) += rnpgbe.o
> -rnpgbe-objs := rnpgbe_main.o
> +rnpgbe-objs := rnpgbe_main.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 3b122dd508ce..7450bfc5ee98 100644
> --- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h
> +++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h
> @@ -4,13 +4,36 @@
> #ifndef _RNPGBE_H
> #define _RNPGBE_H
>
> +#include <linux/types.h>
> +
> enum rnpgbe_boards {
> board_n500,
> board_n210
> };
>
> +struct mucse_mbx_stats {
> + u32 msgs_tx; /* Number of messages sent from PF to fw */
> + u32 msgs_rx; /* Number of messages received from fw to PF */
> + u32 acks; /* Number of ACKs received from firmware */
> + u32 reqs; /* Number of requests sent to firmware */
> +};
> +
> +struct mucse_mbx_info {
> + struct mucse_mbx_stats stats;
> + u32 timeout_us;
> + u32 delay_us;
> + u16 fw_req;
> + u16 fw_ack;
> + /* fw <--> pf mbx */
> + u32 fwpf_shm_base;
> + u32 pf2fw_mbx_ctrl;
> + u32 fwpf_mbx_mask;
> + u32 fwpf_ctrl_base;
> +};
> +
> struct mucse_hw {
> void __iomem *hw_addr;
> + struct mucse_mbx_info mbx;
> };
>
> struct mucse {
> @@ -19,6 +42,8 @@ struct mucse {
> struct mucse_hw hw;
> };
>
> +int rnpgbe_init_hw(struct mucse_hw *hw, int board_type);
> +
> /* Device IDs */
> #define PCI_VENDOR_ID_MUCSE 0x8848
> #define PCI_DEVICE_ID_N500_QUAD_PORT 0x8308
> diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_chip.c b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_chip.c
> new file mode 100644
> index 000000000000..86f1c75796b0
> --- /dev/null
> +++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_chip.c
> @@ -0,0 +1,70 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright(c) 2020 - 2025 Mucse Corporation. */
> +
> +#include <linux/errno.h>
> +
> +#include "rnpgbe.h"
> +#include "rnpgbe_hw.h"
> +#include "rnpgbe_mbx.h"
> +
> +/**
> + * rnpgbe_init_n500 - Setup n500 hw info
> + * @hw: hw information structure
> + *
> + * rnpgbe_init_n500 initializes all private
> + * structure for n500
> + **/
> +static void rnpgbe_init_n500(struct mucse_hw *hw)
> +{
> + struct mucse_mbx_info *mbx = &hw->mbx;
> +
> + mbx->fwpf_ctrl_base = MUCSE_N500_FWPF_CTRL_BASE;
> + mbx->fwpf_shm_base = MUCSE_N500_FWPF_SHM_BASE;
> +}
> +
> +/**
> + * rnpgbe_init_n210 - Setup n210 hw info
> + * @hw: hw information structure
> + *
> + * rnpgbe_init_n210 initializes all private
> + * structure for n210
> + **/
> +static void rnpgbe_init_n210(struct mucse_hw *hw)
> +{
> + struct mucse_mbx_info *mbx = &hw->mbx;
> +
> + mbx->fwpf_ctrl_base = MUCSE_N210_FWPF_CTRL_BASE;
> + mbx->fwpf_shm_base = MUCSE_N210_FWPF_SHM_BASE;
> +}
> +
> +/**
> + * rnpgbe_init_hw - Setup hw info according to board_type
> + * @hw: hw information structure
> + * @board_type: board type
> + *
> + * rnpgbe_init_hw initializes all hw data
> + *
> + * Return: 0 on success, negative errno on failure
Maybe say that -EINVAL is the only error returned.
> + **/
> +int rnpgbe_init_hw(struct mucse_hw *hw, int board_type)
> +{
> + struct mucse_mbx_info *mbx = &hw->mbx;
> +
> + mbx->pf2fw_mbx_ctrl = MUCSE_GBE_PFFW_MBX_CTRL_OFFSET;
> + mbx->fwpf_mbx_mask = MUCSE_GBE_FWPF_MBX_MASK_OFFSET;
> +
> + switch (board_type) {
> + case board_n500:
> + rnpgbe_init_n500(hw);
> + break;
> + case board_n210:
> + rnpgbe_init_n210(hw);
> + break;
> + default:
> + return -EINVAL;
> + }
> + /* init_params with mbx base */
> + mucse_init_mbx_params_pf(hw);
> +
> + return 0;
> +}
> diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h
> index 3a779806e8be..aad4cb2f4164 100644
> --- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h
> +++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h
> @@ -4,5 +4,12 @@
> #ifndef _RNPGBE_HW_H
> #define _RNPGBE_HW_H
>
> +#define MUCSE_N500_FWPF_CTRL_BASE 0x28b00
> +#define MUCSE_N500_FWPF_SHM_BASE 0x2d000
> +#define MUCSE_GBE_PFFW_MBX_CTRL_OFFSET 0x5500
> +#define MUCSE_GBE_FWPF_MBX_MASK_OFFSET 0x5700
> +#define MUCSE_N210_FWPF_CTRL_BASE 0x29400
> +#define MUCSE_N210_FWPF_SHM_BASE 0x2d900
> +
> #define 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 0afe39621661..c6cfb54f7c59 100644
> --- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c
> +++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c
> @@ -68,6 +68,11 @@ static int rnpgbe_add_adapter(struct pci_dev *pdev,
> }
>
> hw->hw_addr = hw_addr;
> + err = rnpgbe_init_hw(hw, board_type);
> + if (err) {
> + dev_err(&pdev->dev, "Init hw err %d\n", err);
> + goto err_free_net;
> + }
>
> return 0;
>
> 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..7501a55c3134
> --- /dev/null
> +++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx.c
> @@ -0,0 +1,420 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright(c) 2022 - 2025 Mucse Corporation. */
> +
> +#include <linux/errno.h>
> +#include <linux/bitfield.h>
> +#include <linux/iopoll.h>
> +
> +#include "rnpgbe_mbx.h"
> +
> +/**
> + * mbx_data_rd32 - Reads reg with base mbx->fwpf_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->fwpf_shm_base + reg);
> +}
> +
> +/**
> + * mbx_data_wr32 - Writes value to reg with base mbx->fwpf_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->fwpf_shm_base + reg);
> +}
> +
> +/**
> + * mbx_ctrl_rd32 - Reads reg with base mbx->fwpf_ctrl_base
> + * @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->fwpf_ctrl_base + reg);
> +}
> +
> +/**
> + * mbx_ctrl_wr32 - Writes value to reg with base mbx->fwpf_ctrl_base
> + * @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->fwpf_ctrl_base + reg);
> +}
> +
> +/**
> + * mucse_mbx_get_lock_pf - Write ctrl and read back lock status
> + * @hw: pointer to the HW structure
> + *
> + * Return: register value after write
> + **/
> +static u32 mucse_mbx_get_lock_pf(struct mucse_hw *hw)
> +{
> + struct mucse_mbx_info *mbx = &hw->mbx;
> + u32 reg = MUCSE_MBX_PF2FW_CTRL(mbx);
> +
> + mbx_ctrl_wr32(mbx, reg, MUCSE_MBX_PFU);
> +
> + return mbx_ctrl_rd32(mbx, reg);
> +}
> +
> +/**
> + * mucse_obtain_mbx_lock_pf - Obtain mailbox lock
> + * @hw: pointer to the HW structure
> + *
> + * Pair with mucse_release_mbx_lock_pf()
> + * This function maybe used in an irq handler.
> + *
> + * Return: 0 on success, negative errno on failure
> + **/
> +static int mucse_obtain_mbx_lock_pf(struct mucse_hw *hw)
> +{
> + struct mucse_mbx_info *mbx = &hw->mbx;
> + u32 val;
> +
> + return read_poll_timeout_atomic(mucse_mbx_get_lock_pf,
> + val, val & MUCSE_MBX_PFU,
> + mbx->delay_us,
> + mbx->timeout_us,
> + false, hw);
> +}
> +
> +/**
> + * mucse_release_mbx_lock_pf - Release mailbox lock
> + * @hw: pointer to the HW structure
> + * @req: send a request or not
> + *
> + * Pair with mucse_obtain_mbx_lock_pf():
> + * - Releases the mailbox lock by clearing MUCSE_MBX_PFU bit
> + * - Simultaneously sends the request by setting MUCSE_MBX_REQ bit
> + * if req is true
> + * (Both bits are in the same mailbox control register,
> + * so operations are combined)
> + **/
> +static void mucse_release_mbx_lock_pf(struct mucse_hw *hw, bool req)
> +{
> + struct mucse_mbx_info *mbx = &hw->mbx;
> + u32 reg = MUCSE_MBX_PF2FW_CTRL(mbx);
> +
> + if (req)
> + mbx_ctrl_wr32(mbx, reg, MUCSE_MBX_REQ);
> + else
> + mbx_ctrl_wr32(mbx, reg, 0);
Maybe combine this to:
mbx_ctrl_wr32(mbx, reg, req ? MUCSE_MBX_REQ : 0);
or also inlining reg:
mbx_ctrl_wr32(mbx, MUCSE_MBX_PF2FW_CTRL(mbx), req ? MUCSE_MBX_REQ : 0);
> +}
> +
> +/**
> + * 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)
> +{
> + u32 val = mbx_data_rd32(mbx, MUCSE_MBX_FW2PF_CNT);
> +
> + return FIELD_GET(GENMASK_U32(15, 0), val);
> +}
> +
> +/**
> + * mucse_mbx_inc_pf_ack - Increase ack
> + * @hw: pointer to the HW structure
> + *
> + * mucse_mbx_inc_pf_ack reads pf_ack from hw, then writes
> + * 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 val;
> +
> + val = mbx_data_rd32(mbx, MUCSE_MBX_PF2FW_CNT);
> + ack = FIELD_GET(GENMASK_U32(31, 16), val);
> + ack++;
> + val &= ~GENMASK_U32(31, 16);
> + val |= FIELD_PREP(GENMASK_U32(31, 16), ack);
> + mbx_data_wr32(mbx, MUCSE_MBX_PF2FW_CNT, val);
> + hw->mbx.stats.msgs_rx++;
> +}
> +
> +/**
> + * mucse_read_mbx_pf - Read a message from the mailbox
> + * @hw: pointer to the HW structure
> + * @msg: the message buffer
> + * @size: length of buffer
> + *
> + * mucse_read_mbx_pf copies a message from the mbx 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_in_words = size / 4;
* Make this `const int` or I'm in favour of inlining, because it's used only
once.
* Maybe `size / sizeof(*msg)` makes it more clearly where the 4 is coming
from.
> + int ret;
> + int i;
Because this variable is only used in the for-loop, I would narrow its scope
and write `for (int i = 0;`
> +
> + ret = mucse_obtain_mbx_lock_pf(hw);
> + if (ret)
> + return ret;
> +
> + for (i = 0; i < size_in_words; i++)
> + msg[i] = mbx_data_rd32(mbx, MUCSE_MBX_FWPF_SHM + 4 * i);
> + /* Hw needs write data_reg at last */
> + mbx_data_wr32(mbx, MUCSE_MBX_FWPF_SHM, 0);
> + /* flush reqs as we have read this request data */
> + hw->mbx.fw_req = mucse_mbx_get_fwreq(mbx);
> + mucse_mbx_inc_pf_ack(hw);
> + mucse_release_mbx_lock_pf(hw, false);
> +
> + return 0;
> +}
> +
> +/**
> + * 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 fw_req;
> +
> + fw_req = mucse_mbx_get_fwreq(mbx);
> + /* chip's register is reset to 0 when rc send reset
> + * mbx command. This causes 'fw_req != 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 I get this right, fw_req == 0 means that the request was send, but
nothing returned so far. Would not be -EAGAIN a better return value in this
case?
And fw_req == hw->mbx.fw_req means that we've got the old fw_req again which
means there was an error (-EIO).
> + if (fw_req != 0 && fw_req != hw->mbx.fw_req) {
> + hw->mbx.stats.reqs++;
> + return 0;
> + }
> +
> + return -EIO;
Only a suggestion: Might it be clearer to flip the cases and handle the if
as error case and continue with the success case?
if (fw_req == 0 || fw_req == hw->mbx.fw_req)
return -EIO;
hw->mbx.stats.reqs++;
return 0;
> +}
> +
> +/**
> + * 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 val;
> +
> + return read_poll_timeout(mucse_check_for_msg_pf,
> + val, !val, mbx->delay_us,
> + mbx->timeout_us,
> + false, hw);
> +}
> +
> +/**
> + * mucse_poll_and_read_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, negative errno on failure
> + **/
> +int mucse_poll_and_read_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_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)
> +{
> + u32 val = mbx_data_rd32(mbx, MUCSE_MBX_FW2PF_CNT);
> +
> + return FIELD_GET(GENMASK_U32(31, 16), val);
> +}
> +
> +/**
> + * mucse_mbx_inc_pf_req - Increase req
> + * @hw: pointer to the HW structure
> + *
> + * mucse_mbx_inc_pf_req reads pf_req from hw, then writes
> + * 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 val;
> +
> + val = mbx_data_rd32(mbx, MUCSE_MBX_PF2FW_CNT);
> + req = FIELD_GET(GENMASK_U32(15, 0), val);
Why not assign the values in the declaration like done with mbx?
> + req++;
> + val &= ~GENMASK_U32(15, 0);
> + val |= FIELD_PREP(GENMASK_U32(15, 0), req);
> + mbx_data_wr32(mbx, MUCSE_MBX_PF2FW_CNT, val);
> + hw->mbx.stats.msgs_tx++;
> +}
> +
> +/**
> + * mucse_write_mbx_pf - Place a message in the mailbox
> + * @hw: pointer to the HW structure
> + * @msg: the message buffer
> + * @size: length of buffer
> + *
> + * Return: 0 if it successfully copied message into the buffer
... "otherwise the error from obtaining the lock" or something better.
> + **/
> +static int mucse_write_mbx_pf(struct mucse_hw *hw, u32 *msg, u16 size)
> +{
> + struct mucse_mbx_info *mbx = &hw->mbx;
> + int size_in_words = size / 4;
> + int ret;
> + int i;
> +
> + ret = mucse_obtain_mbx_lock_pf(hw);
> + if (ret)
> + return ret;
> +
> + for (i = 0; i < size_in_words; i++)
> + mbx_data_wr32(mbx, MUCSE_MBX_FWPF_SHM + i * 4, msg[i]);
> +
> + /* flush acks as we are overwriting the message buffer */
> + hw->mbx.fw_ack = mucse_mbx_get_fwack(mbx);
> + mucse_mbx_inc_pf_req(hw);
> + mucse_release_mbx_lock_pf(hw, true);
> +
> + return 0;
> +}
> +
> +/**
> + * mucse_check_for_ack_pf - Check to see if the fw 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 fw_ack;
> +
> + fw_ack = mucse_mbx_get_fwack(mbx);
> + /* chip's register is reset to 0 when rc send reset
> + * mbx command. This causes '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 (fw_ack != 0 && 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,
> + * else negative errno
> + **/
> +static int mucse_poll_for_ack(struct mucse_hw *hw)
> +{
> + struct mucse_mbx_info *mbx = &hw->mbx;
> + int val;
> +
> + return read_poll_timeout(mucse_check_for_ack_pf,
> + val, !val, mbx->delay_us,
> + mbx->timeout_us,
> + false, hw);
> +}
> +
> +/**
> + * mucse_write_and_wait_ack_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_cnt period
> + **/
> +int mucse_write_and_wait_ack_mbx(struct mucse_hw *hw, u32 *msg, u16 size)
> +{
> + int ret;
> +
> + ret = mucse_write_mbx_pf(hw, msg, size);
If I see it right, this function is the only user of mucse_write_mbx_pf.
> + if (ret)
> + return ret;
> + return mucse_poll_for_ack(hw);
The same here?
> +}
> +
> +/**
> + * mucse_mbx_reset - Reset mbx info, sync info from regs
> + * @hw: pointer to the HW structure
> + *
> + * mucse_mbx_reset resets all mbx variables to default.
> + **/
> +static void mucse_mbx_reset(struct mucse_hw *hw)
> +{
> + struct mucse_mbx_info *mbx = &hw->mbx;
> + u32 val;
> +
> + val = mbx_data_rd32(mbx, MUCSE_MBX_FW2PF_CNT);
Because val is assigned only once, you could make it `const u32 val = ...`
> + hw->mbx.fw_req = FIELD_GET(GENMASK_U32(15, 0), val);
> + hw->mbx.fw_ack = FIELD_GET(GENMASK_U32(31, 16), val);
> + mbx_ctrl_wr32(mbx, MUCSE_MBX_PF2FW_CTRL(mbx), 0);
> + mbx_ctrl_wr32(mbx, MUCSE_MBX_FWPF_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->delay_us = 100;
> + mbx->timeout_us = 4 * USEC_PER_SEC;
> + mbx->stats.msgs_tx = 0;
> + mbx->stats.msgs_rx = 0;
> + mbx->stats.reqs = 0;
> + mbx->stats.acks = 0;
> + mucse_mbx_reset(hw);
Is mucse_init_mbx_params_pf the only user of mucse_mbx_reset? If so, the
function should inlined.
> +}
> 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..eac99f13b6ff
> --- /dev/null
> +++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx.h
> @@ -0,0 +1,20 @@
> +/* 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_MBX_FW2PF_CNT 0
> +#define MUCSE_MBX_PF2FW_CNT 4
> +#define MUCSE_MBX_FWPF_SHM 8
Are these constants used by other c files or should they be private to
rnpgbe_mbx.c?
> +#define MUCSE_MBX_PF2FW_CTRL(mbx) ((mbx)->pf2fw_mbx_ctrl)
> +#define MUCSE_MBX_FWPF_MASK(mbx) ((mbx)->fwpf_mbx_mask)
> +#define MUCSE_MBX_REQ BIT(0) /* Request a req to mailbox */
> +#define MUCSE_MBX_PFU BIT(3) /* PF owns the mailbox buffer */
> +
> +int mucse_write_and_wait_ack_mbx(struct mucse_hw *hw, u32 *msg, u16 size);
> +void mucse_init_mbx_params_pf(struct mucse_hw *hw);
> +int mucse_poll_and_read_mbx(struct mucse_hw *hw, u32 *msg, u16 size);
> +#endif /* _RNPGBE_MBX_H */
> --
> 2.25.1
>
>
--
“Perl—the only language that looks the same
before and after RSA encryption.” (Keith Bostic)
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH net-next v12 1/5] net: rnpgbe: Add build support for rnpgbe
2025-09-16 11:29 ` [PATCH net-next v12 1/5] net: rnpgbe: Add build support for rnpgbe Dong Yibo
@ 2025-09-16 19:00 ` Jörg Sommer
2025-09-17 1:37 ` Yibo Dong
2025-09-17 7:01 ` MD Danish Anwar
1 sibling, 1 reply; 21+ messages in thread
From: Jörg Sommer @ 2025-09-16 19:00 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,
Andrew Lunn
[-- Attachment #1: Type: text/plain, Size: 4423 bytes --]
Dong Yibo schrieb am Di 16. Sep, 19:29 (+0800):
> 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..60bbc806f17b
> --- /dev/null
> +++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c
> @@ -0,0 +1,124 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright(c) 2020 - 2025 Mucse Corporation. */
> +
> +#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_n210},
Should there be a space before }?
> + /* 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;
In rnpgbe_mbx.c you use `int ret` for this pattern. I think you should unify
this. But I'm more in favour of `err` than `ret`.
> +
> + 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);
Is this the only user of rnpgbe_dev_shutdown?
> +}
> +
> +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
>
>
--
Als deutscher Tourist im Ausland steht man vor der Frage, ob man sich
anständig benehmen muss oder ob schon deutsche Touristen dagewesen sind.
(Kurt Tucholsky)
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH net-next v12 2/5] net: rnpgbe: Add n500/n210 chip support with BAR2 mapping
2025-09-16 11:29 ` [PATCH net-next v12 2/5] net: rnpgbe: Add n500/n210 chip support with BAR2 mapping Dong Yibo
@ 2025-09-16 19:20 ` Jörg Sommer
2025-09-17 7:02 ` MD Danish Anwar
1 sibling, 0 replies; 21+ messages in thread
From: Jörg Sommer @ 2025-09-16 19:20 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
[-- Attachment #1: Type: text/plain, Size: 2861 bytes --]
Dong Yibo schrieb am Di 16. Sep, 19:29 (+0800):
> diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c
> index 60bbc806f17b..0afe39621661 100644
> --- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c
> +++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c
> @@ -2,8 +2,11 @@
> /* Copyright(c) 2020 - 2025 Mucse Corporation. */
>
> #include <linux/pci.h>
> +#include <net/rtnetlink.h>
> +#include <linux/etherdevice.h>
>
> #include "rnpgbe.h"
> +#include "rnpgbe_hw.h"
>
> static const char rnpgbe_driver_name[] = "rnpgbe";
>
> @@ -25,6 +28,54 @@ static struct pci_device_id rnpgbe_pci_tbl[] = {
> {0, },
> };
>
> +/**
> + * rnpgbe_add_adapter - Add netdev for this pci_dev
> + * @pdev: PCI device information structure
> + * @board_type: board type
> + *
> + * 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,
> + int board_type)
> +{
> + 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_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;
> +
> + return 0;
> +
> +err_free_net:
> + free_netdev(netdev);
> + return err;
> +}
> +
> /**
> * rnpgbe_probe - Device initialization routine
> * @pdev: PCI device information struct
> @@ -37,6 +88,7 @@ static struct pci_device_id rnpgbe_pci_tbl[] = {
> **/
> static int rnpgbe_probe(struct pci_dev *pdev, const struct pci_device_id *id)
> {
> + int board_type = id->driver_data;
> int err;
>
> err = pci_enable_device_mem(pdev);
> @@ -63,6 +115,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, board_type);
Would an empty line before this assignment make the code more readable?
--
Professor: ‚Gott‘, unverständliches und mythisches Wesen, das sich einmal
pro Woche im Kreis der Sterblichen manifestiert um Weisheit auf Folien
unter das Volk zu bringen. (Dschungelbuch 11, FSU Jena)
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH net-next v12 3/5] net: rnpgbe: Add basic mbx ops support
2025-09-16 18:42 ` Jörg Sommer
@ 2025-09-16 19:33 ` Andrew Lunn
2025-09-17 2:55 ` Yibo Dong
1 sibling, 0 replies; 21+ messages in thread
From: Andrew Lunn @ 2025-09-16 19:33 UTC (permalink / raw)
To: Jörg Sommer
Cc: 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,
vadim.fedorenko, netdev, linux-doc, linux-kernel, linux-hardening
> > + if (fw_req != 0 && fw_req != hw->mbx.fw_req) {
> > + hw->mbx.stats.reqs++;
> > + return 0;
> > + }
> > +
> > + return -EIO;
>
> Only a suggestion: Might it be clearer to flip the cases and handle the if
> as error case and continue with the success case?
>
> if (fw_req == 0 || fw_req == hw->mbx.fw_req)
> return -EIO;
>
> hw->mbx.stats.reqs++;
> return 0;
That would by the usual pattern in the kernel. It is good to follow
usual patterns.
> > +static void mucse_mbx_inc_pf_req(struct mucse_hw *hw)
> > +{
> > + struct mucse_mbx_info *mbx = &hw->mbx;
> > + u16 req;
> > + u32 val;
> > +
> > + val = mbx_data_rd32(mbx, MUCSE_MBX_PF2FW_CNT);
> > + req = FIELD_GET(GENMASK_U32(15, 0), val);
>
> Why not assign the values in the declaration like done with mbx?
Reverse Christmas tree.
struct mucse_mbx_info *mbx = &hw->mbx;
u32 val = mbx_data_rd32(mbx, MUCSE_MBX_PF2FW_CNT);
u16 req = FIELD_GET(GENMASK_U32(15, 0), val);
This is not reverse christmas tree. Sometimes you need to put
assignments into the body of the function.
Andrew
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH net-next v12 1/5] net: rnpgbe: Add build support for rnpgbe
2025-09-16 19:00 ` Jörg Sommer
@ 2025-09-17 1:37 ` Yibo Dong
0 siblings, 0 replies; 21+ messages in thread
From: Yibo Dong @ 2025-09-17 1:37 UTC (permalink / raw)
To: Jörg Sommer
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,
Andrew Lunn
On Tue, Sep 16, 2025 at 09:00:23PM +0200, Jörg Sommer wrote:
> Dong Yibo schrieb am Di 16. Sep, 19:29 (+0800):
> > 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..60bbc806f17b
> > --- /dev/null
> > +++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c
> > @@ -0,0 +1,124 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/* Copyright(c) 2020 - 2025 Mucse Corporation. */
> > +
> > +#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_n210},
>
> Should there be a space before }?
>
ixgbe_main.c has sapce before }, but no sapce after {.
ngbe_mainc. no sapce before }, but space after {.
mlx5/core/main.c has space both.
It seems not the same....
I will add sapce before }.
> > + /* 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;
>
> In rnpgbe_mbx.c you use `int ret` for this pattern. I think you should unify
> this. But I'm more in favour of `err` than `ret`.
>
I see, I will use err in rnpgbe_mbx.c
> > +
> > + 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);
>
> Is this the only user of rnpgbe_dev_shutdown?
>
No, it maybe called by suspend callback in the future.
Device relative code will be added in rnpgbe_dev_shutdown, and power state
code in rnpgbe_shutdown. This is the same like other driver did
(ixgbe_main.c)
> > +}
> > +
> > +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
> >
> >
>
> --
> Als deutscher Tourist im Ausland steht man vor der Frage, ob man sich
> anständig benehmen muss oder ob schon deutsche Touristen dagewesen sind.
> (Kurt Tucholsky)
Thanks for your feedback.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH net-next v12 3/5] net: rnpgbe: Add basic mbx ops support
2025-09-16 18:42 ` Jörg Sommer
2025-09-16 19:33 ` Andrew Lunn
@ 2025-09-17 2:55 ` Yibo Dong
1 sibling, 0 replies; 21+ messages in thread
From: Yibo Dong @ 2025-09-17 2:55 UTC (permalink / raw)
To: Jörg Sommer
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 Tue, Sep 16, 2025 at 08:42:44PM +0200, Jörg Sommer wrote:
> Hi,
>
> only some minor suggestions.
>
> Dong Yibo schrieb am Di 16. Sep, 19:29 (+0800):
> > Add fundamental mailbox (MBX) communication operations between PF
> > (Physical Function) and firmware for n500/n210 chips
> >
> > Signed-off-by: Dong Yibo <dong100@mucse.com>
> > ---
> > drivers/net/ethernet/mucse/rnpgbe/Makefile | 4 +-
> > drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h | 25 ++
> > .../net/ethernet/mucse/rnpgbe/rnpgbe_chip.c | 70 +++
> > drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h | 7 +
> > .../net/ethernet/mucse/rnpgbe/rnpgbe_main.c | 5 +
> > .../net/ethernet/mucse/rnpgbe/rnpgbe_mbx.c | 420 ++++++++++++++++++
> > .../net/ethernet/mucse/rnpgbe/rnpgbe_mbx.h | 20 +
> > 7 files changed, 550 insertions(+), 1 deletion(-)
> > create mode 100644 drivers/net/ethernet/mucse/rnpgbe/rnpgbe_chip.c
> > 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 9df536f0d04c..5fc878ada4b1 100644
> > --- a/drivers/net/ethernet/mucse/rnpgbe/Makefile
> > +++ b/drivers/net/ethernet/mucse/rnpgbe/Makefile
> > @@ -5,4 +5,6 @@
> > #
> >
> > obj-$(CONFIG_MGBE) += rnpgbe.o
> > -rnpgbe-objs := rnpgbe_main.o
> > +rnpgbe-objs := rnpgbe_main.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 3b122dd508ce..7450bfc5ee98 100644
> > --- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h
> > +++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h
> > @@ -4,13 +4,36 @@
> > #ifndef _RNPGBE_H
> > #define _RNPGBE_H
> >
> > +#include <linux/types.h>
> > +
> > enum rnpgbe_boards {
> > board_n500,
> > board_n210
> > };
> >
> > +struct mucse_mbx_stats {
> > + u32 msgs_tx; /* Number of messages sent from PF to fw */
> > + u32 msgs_rx; /* Number of messages received from fw to PF */
> > + u32 acks; /* Number of ACKs received from firmware */
> > + u32 reqs; /* Number of requests sent to firmware */
> > +};
> > +
> > +struct mucse_mbx_info {
> > + struct mucse_mbx_stats stats;
> > + u32 timeout_us;
> > + u32 delay_us;
> > + u16 fw_req;
> > + u16 fw_ack;
> > + /* fw <--> pf mbx */
> > + u32 fwpf_shm_base;
> > + u32 pf2fw_mbx_ctrl;
> > + u32 fwpf_mbx_mask;
> > + u32 fwpf_ctrl_base;
> > +};
> > +
> > struct mucse_hw {
> > void __iomem *hw_addr;
> > + struct mucse_mbx_info mbx;
> > };
> >
> > struct mucse {
> > @@ -19,6 +42,8 @@ struct mucse {
> > struct mucse_hw hw;
> > };
> >
> > +int rnpgbe_init_hw(struct mucse_hw *hw, int board_type);
> > +
> > /* Device IDs */
> > #define PCI_VENDOR_ID_MUCSE 0x8848
> > #define PCI_DEVICE_ID_N500_QUAD_PORT 0x8308
> > diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_chip.c b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_chip.c
> > new file mode 100644
> > index 000000000000..86f1c75796b0
> > --- /dev/null
> > +++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_chip.c
> > @@ -0,0 +1,70 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/* Copyright(c) 2020 - 2025 Mucse Corporation. */
> > +
> > +#include <linux/errno.h>
> > +
> > +#include "rnpgbe.h"
> > +#include "rnpgbe_hw.h"
> > +#include "rnpgbe_mbx.h"
> > +
> > +/**
> > + * rnpgbe_init_n500 - Setup n500 hw info
> > + * @hw: hw information structure
> > + *
> > + * rnpgbe_init_n500 initializes all private
> > + * structure for n500
> > + **/
> > +static void rnpgbe_init_n500(struct mucse_hw *hw)
> > +{
> > + struct mucse_mbx_info *mbx = &hw->mbx;
> > +
> > + mbx->fwpf_ctrl_base = MUCSE_N500_FWPF_CTRL_BASE;
> > + mbx->fwpf_shm_base = MUCSE_N500_FWPF_SHM_BASE;
> > +}
> > +
> > +/**
> > + * rnpgbe_init_n210 - Setup n210 hw info
> > + * @hw: hw information structure
> > + *
> > + * rnpgbe_init_n210 initializes all private
> > + * structure for n210
> > + **/
> > +static void rnpgbe_init_n210(struct mucse_hw *hw)
> > +{
> > + struct mucse_mbx_info *mbx = &hw->mbx;
> > +
> > + mbx->fwpf_ctrl_base = MUCSE_N210_FWPF_CTRL_BASE;
> > + mbx->fwpf_shm_base = MUCSE_N210_FWPF_SHM_BASE;
> > +}
> > +
> > +/**
> > + * rnpgbe_init_hw - Setup hw info according to board_type
> > + * @hw: hw information structure
> > + * @board_type: board type
> > + *
> > + * rnpgbe_init_hw initializes all hw data
> > + *
> > + * Return: 0 on success, negative errno on failure
>
> Maybe say that -EINVAL is the only error returned.
>
Got it.
> > + **/
> > +int rnpgbe_init_hw(struct mucse_hw *hw, int board_type)
> > +{
> > + struct mucse_mbx_info *mbx = &hw->mbx;
> > +
> > + mbx->pf2fw_mbx_ctrl = MUCSE_GBE_PFFW_MBX_CTRL_OFFSET;
> > + mbx->fwpf_mbx_mask = MUCSE_GBE_FWPF_MBX_MASK_OFFSET;
> > +
> > + switch (board_type) {
> > + case board_n500:
> > + rnpgbe_init_n500(hw);
> > + break;
> > + case board_n210:
> > + rnpgbe_init_n210(hw);
> > + break;
> > + default:
> > + return -EINVAL;
> > + }
> > + /* init_params with mbx base */
> > + mucse_init_mbx_params_pf(hw);
> > +
> > + return 0;
> > +}
> > diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h
> > index 3a779806e8be..aad4cb2f4164 100644
> > --- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h
> > +++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h
> > @@ -4,5 +4,12 @@
> > #ifndef _RNPGBE_HW_H
> > #define _RNPGBE_HW_H
> >
> > +#define MUCSE_N500_FWPF_CTRL_BASE 0x28b00
> > +#define MUCSE_N500_FWPF_SHM_BASE 0x2d000
> > +#define MUCSE_GBE_PFFW_MBX_CTRL_OFFSET 0x5500
> > +#define MUCSE_GBE_FWPF_MBX_MASK_OFFSET 0x5700
> > +#define MUCSE_N210_FWPF_CTRL_BASE 0x29400
> > +#define MUCSE_N210_FWPF_SHM_BASE 0x2d900
> > +
> > #define 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 0afe39621661..c6cfb54f7c59 100644
> > --- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c
> > +++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c
> > @@ -68,6 +68,11 @@ static int rnpgbe_add_adapter(struct pci_dev *pdev,
> > }
> >
> > hw->hw_addr = hw_addr;
> > + err = rnpgbe_init_hw(hw, board_type);
> > + if (err) {
> > + dev_err(&pdev->dev, "Init hw err %d\n", err);
> > + goto err_free_net;
> > + }
> >
> > return 0;
> >
> > 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..7501a55c3134
> > --- /dev/null
> > +++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx.c
> > @@ -0,0 +1,420 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/* Copyright(c) 2022 - 2025 Mucse Corporation. */
> > +
> > +#include <linux/errno.h>
> > +#include <linux/bitfield.h>
> > +#include <linux/iopoll.h>
> > +
> > +#include "rnpgbe_mbx.h"
> > +
> > +/**
> > + * mbx_data_rd32 - Reads reg with base mbx->fwpf_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->fwpf_shm_base + reg);
> > +}
> > +
> > +/**
> > + * mbx_data_wr32 - Writes value to reg with base mbx->fwpf_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->fwpf_shm_base + reg);
> > +}
> > +
> > +/**
> > + * mbx_ctrl_rd32 - Reads reg with base mbx->fwpf_ctrl_base
> > + * @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->fwpf_ctrl_base + reg);
> > +}
> > +
> > +/**
> > + * mbx_ctrl_wr32 - Writes value to reg with base mbx->fwpf_ctrl_base
> > + * @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->fwpf_ctrl_base + reg);
> > +}
> > +
> > +/**
> > + * mucse_mbx_get_lock_pf - Write ctrl and read back lock status
> > + * @hw: pointer to the HW structure
> > + *
> > + * Return: register value after write
> > + **/
> > +static u32 mucse_mbx_get_lock_pf(struct mucse_hw *hw)
> > +{
> > + struct mucse_mbx_info *mbx = &hw->mbx;
> > + u32 reg = MUCSE_MBX_PF2FW_CTRL(mbx);
> > +
> > + mbx_ctrl_wr32(mbx, reg, MUCSE_MBX_PFU);
> > +
> > + return mbx_ctrl_rd32(mbx, reg);
> > +}
> > +
> > +/**
> > + * mucse_obtain_mbx_lock_pf - Obtain mailbox lock
> > + * @hw: pointer to the HW structure
> > + *
> > + * Pair with mucse_release_mbx_lock_pf()
> > + * This function maybe used in an irq handler.
> > + *
> > + * Return: 0 on success, negative errno on failure
> > + **/
> > +static int mucse_obtain_mbx_lock_pf(struct mucse_hw *hw)
> > +{
> > + struct mucse_mbx_info *mbx = &hw->mbx;
> > + u32 val;
> > +
> > + return read_poll_timeout_atomic(mucse_mbx_get_lock_pf,
> > + val, val & MUCSE_MBX_PFU,
> > + mbx->delay_us,
> > + mbx->timeout_us,
> > + false, hw);
> > +}
> > +
> > +/**
> > + * mucse_release_mbx_lock_pf - Release mailbox lock
> > + * @hw: pointer to the HW structure
> > + * @req: send a request or not
> > + *
> > + * Pair with mucse_obtain_mbx_lock_pf():
> > + * - Releases the mailbox lock by clearing MUCSE_MBX_PFU bit
> > + * - Simultaneously sends the request by setting MUCSE_MBX_REQ bit
> > + * if req is true
> > + * (Both bits are in the same mailbox control register,
> > + * so operations are combined)
> > + **/
> > +static void mucse_release_mbx_lock_pf(struct mucse_hw *hw, bool req)
> > +{
> > + struct mucse_mbx_info *mbx = &hw->mbx;
> > + u32 reg = MUCSE_MBX_PF2FW_CTRL(mbx);
> > +
> > + if (req)
> > + mbx_ctrl_wr32(mbx, reg, MUCSE_MBX_REQ);
> > + else
> > + mbx_ctrl_wr32(mbx, reg, 0);
>
> Maybe combine this to:
>
> mbx_ctrl_wr32(mbx, reg, req ? MUCSE_MBX_REQ : 0);
>
> or also inlining reg:
>
> mbx_ctrl_wr32(mbx, MUCSE_MBX_PF2FW_CTRL(mbx), req ? MUCSE_MBX_REQ : 0);
>
Got it, I will update to
mbx_ctrl_wr32(mbx, reg, req ? MUCSE_MBX_REQ : 0)
I think it is too long to inlining 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)
> > +{
> > + u32 val = mbx_data_rd32(mbx, MUCSE_MBX_FW2PF_CNT);
> > +
> > + return FIELD_GET(GENMASK_U32(15, 0), val);
> > +}
> > +
> > +/**
> > + * mucse_mbx_inc_pf_ack - Increase ack
> > + * @hw: pointer to the HW structure
> > + *
> > + * mucse_mbx_inc_pf_ack reads pf_ack from hw, then writes
> > + * 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 val;
> > +
> > + val = mbx_data_rd32(mbx, MUCSE_MBX_PF2FW_CNT);
> > + ack = FIELD_GET(GENMASK_U32(31, 16), val);
> > + ack++;
> > + val &= ~GENMASK_U32(31, 16);
> > + val |= FIELD_PREP(GENMASK_U32(31, 16), ack);
> > + mbx_data_wr32(mbx, MUCSE_MBX_PF2FW_CNT, val);
> > + hw->mbx.stats.msgs_rx++;
> > +}
> > +
> > +/**
> > + * mucse_read_mbx_pf - Read a message from the mailbox
> > + * @hw: pointer to the HW structure
> > + * @msg: the message buffer
> > + * @size: length of buffer
> > + *
> > + * mucse_read_mbx_pf copies a message from the mbx 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_in_words = size / 4;
>
> * Make this `const int` or I'm in favour of inlining, because it's used only
> once.
>
> * Maybe `size / sizeof(*msg)` makes it more clearly where the 4 is coming
> from.
>
Maybe like this?
const int size_in_words = size / sizeof(u32);
'4 is the number of bytes in u32' here.
(https://lore.kernel.org/netdev/20250909135554.5013bcb0@kernel.org/)
I think use 'size_in_words' maybe more clearly than inlining...
> > + int ret;
> > + int i;
>
> Because this variable is only used in the for-loop, I would narrow its scope
> and write `for (int i = 0;`
>
Got it, I will update it in next version.
> > +
> > + ret = mucse_obtain_mbx_lock_pf(hw);
> > + if (ret)
> > + return ret;
> > +
> > + for (i = 0; i < size_in_words; i++)
> > + msg[i] = mbx_data_rd32(mbx, MUCSE_MBX_FWPF_SHM + 4 * i);
> > + /* Hw needs write data_reg at last */
> > + mbx_data_wr32(mbx, MUCSE_MBX_FWPF_SHM, 0);
> > + /* flush reqs as we have read this request data */
> > + hw->mbx.fw_req = mucse_mbx_get_fwreq(mbx);
> > + mucse_mbx_inc_pf_ack(hw);
> > + mucse_release_mbx_lock_pf(hw, false);
> > +
> > + return 0;
> > +}
> > +
> > +/**
> > + * 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 fw_req;
> > +
> > + fw_req = mucse_mbx_get_fwreq(mbx);
> > + /* chip's register is reset to 0 when rc send reset
> > + * mbx command. This causes 'fw_req != 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 I get this right, fw_req == 0 means that the request was send, but
> nothing returned so far. Would not be -EAGAIN a better return value in this
> case?
>
> And fw_req == hw->mbx.fw_req means that we've got the old fw_req again which
> means there was an error (-EIO).
>
Maybe not accurate, fw_req == 0 is the condition after fw reset chips,
and fw_req == hw->mbx.fw_req means no new req/response in mbx.
Driver gets hw->mbx.fw_req first, send the request and wait to check the response with
fw_req != hw->mbx.fw_req. (but the req maybe a reset command, then before
fw true response, fw_req from register will be 0 make fw_req !=
hw->mbx.fw_req be true, that is not correct).
This is caused by registers in chip always reset to 0 after fw reset it.
Fw records the register value, then call the reset and write back the value.
And also, fw adds value to the register when powerup to make register
no-zero before driver use it.
> > + if (fw_req != 0 && fw_req != hw->mbx.fw_req) {
> > + hw->mbx.stats.reqs++;
> > + return 0;
> > + }
> > +
> > + return -EIO;
>
> Only a suggestion: Might it be clearer to flip the cases and handle the if
> as error case and continue with the success case?
>
> if (fw_req == 0 || fw_req == hw->mbx.fw_req)
> return -EIO;
>
> hw->mbx.stats.reqs++;
> return 0;
>
That's a good idea. I will update this in next version.
> > +}
> > +
> > +/**
> > + * 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 val;
> > +
> > + return read_poll_timeout(mucse_check_for_msg_pf,
> > + val, !val, mbx->delay_us,
> > + mbx->timeout_us,
> > + false, hw);
> > +}
> > +
> > +/**
> > + * mucse_poll_and_read_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, negative errno on failure
> > + **/
> > +int mucse_poll_and_read_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_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)
> > +{
> > + u32 val = mbx_data_rd32(mbx, MUCSE_MBX_FW2PF_CNT);
> > +
> > + return FIELD_GET(GENMASK_U32(31, 16), val);
> > +}
> > +
> > +/**
> > + * mucse_mbx_inc_pf_req - Increase req
> > + * @hw: pointer to the HW structure
> > + *
> > + * mucse_mbx_inc_pf_req reads pf_req from hw, then writes
> > + * 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 val;
> > +
> > + val = mbx_data_rd32(mbx, MUCSE_MBX_PF2FW_CNT);
> > + req = FIELD_GET(GENMASK_U32(15, 0), val);
>
> Why not assign the values in the declaration like done with mbx?
>
Just like Andrew said, Reverse Christmas tree.
> > + req++;
> > + val &= ~GENMASK_U32(15, 0);
> > + val |= FIELD_PREP(GENMASK_U32(15, 0), req);
> > + mbx_data_wr32(mbx, MUCSE_MBX_PF2FW_CNT, val);
> > + hw->mbx.stats.msgs_tx++;
> > +}
> > +
> > +/**
> > + * mucse_write_mbx_pf - Place a message in the mailbox
> > + * @hw: pointer to the HW structure
> > + * @msg: the message buffer
> > + * @size: length of buffer
> > + *
> > + * Return: 0 if it successfully copied message into the buffer
>
> ... "otherwise the error from obtaining the lock" or something better.
>
Got it.
> > + **/
> > +static int mucse_write_mbx_pf(struct mucse_hw *hw, u32 *msg, u16 size)
> > +{
> > + struct mucse_mbx_info *mbx = &hw->mbx;
> > + int size_in_words = size / 4;
> > + int ret;
> > + int i;
> > +
> > + ret = mucse_obtain_mbx_lock_pf(hw);
> > + if (ret)
> > + return ret;
> > +
> > + for (i = 0; i < size_in_words; i++)
> > + mbx_data_wr32(mbx, MUCSE_MBX_FWPF_SHM + i * 4, msg[i]);
> > +
> > + /* flush acks as we are overwriting the message buffer */
> > + hw->mbx.fw_ack = mucse_mbx_get_fwack(mbx);
> > + mucse_mbx_inc_pf_req(hw);
> > + mucse_release_mbx_lock_pf(hw, true);
> > +
> > + return 0;
> > +}
> > +
> > +/**
> > + * mucse_check_for_ack_pf - Check to see if the fw 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 fw_ack;
> > +
> > + fw_ack = mucse_mbx_get_fwack(mbx);
> > + /* chip's register is reset to 0 when rc send reset
> > + * mbx command. This causes '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 (fw_ack != 0 && 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,
> > + * else negative errno
> > + **/
> > +static int mucse_poll_for_ack(struct mucse_hw *hw)
> > +{
> > + struct mucse_mbx_info *mbx = &hw->mbx;
> > + int val;
> > +
> > + return read_poll_timeout(mucse_check_for_ack_pf,
> > + val, !val, mbx->delay_us,
> > + mbx->timeout_us,
> > + false, hw);
> > +}
> > +
> > +/**
> > + * mucse_write_and_wait_ack_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_cnt period
> > + **/
> > +int mucse_write_and_wait_ack_mbx(struct mucse_hw *hw, u32 *msg, u16 size)
> > +{
> > + int ret;
> > +
> > + ret = mucse_write_mbx_pf(hw, msg, size);
>
> If I see it right, this function is the only user of mucse_write_mbx_pf.
>
> > + if (ret)
> > + return ret;
> > + return mucse_poll_for_ack(hw);
>
> The same here?
>
Yes, it is. So how do I to improve?
> > +}
> > +
> > +/**
> > + * mucse_mbx_reset - Reset mbx info, sync info from regs
> > + * @hw: pointer to the HW structure
> > + *
> > + * mucse_mbx_reset resets all mbx variables to default.
> > + **/
> > +static void mucse_mbx_reset(struct mucse_hw *hw)
> > +{
> > + struct mucse_mbx_info *mbx = &hw->mbx;
> > + u32 val;
> > +
> > + val = mbx_data_rd32(mbx, MUCSE_MBX_FW2PF_CNT);
>
> Because val is assigned only once, you could make it `const u32 val = ...`
>
like this?
struct mucse_mbx_info *mbx = &hw->mbx;
const u32 val = mbx_data_rd32(mbx, MUCSE_MBX_FW2PF_CNT);
But it is not reverse christmas tree
> > + hw->mbx.fw_req = FIELD_GET(GENMASK_U32(15, 0), val);
> > + hw->mbx.fw_ack = FIELD_GET(GENMASK_U32(31, 16), val);
> > + mbx_ctrl_wr32(mbx, MUCSE_MBX_PF2FW_CTRL(mbx), 0);
> > + mbx_ctrl_wr32(mbx, MUCSE_MBX_FWPF_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->delay_us = 100;
> > + mbx->timeout_us = 4 * USEC_PER_SEC;
> > + mbx->stats.msgs_tx = 0;
> > + mbx->stats.msgs_rx = 0;
> > + mbx->stats.reqs = 0;
> > + mbx->stats.acks = 0;
> > + mucse_mbx_reset(hw);
>
> Is mucse_init_mbx_params_pf the only user of mucse_mbx_reset? If so, the
> function should inlined.
>
inline?
You mean make mucse_mbx_reset a inline function? Maybe it is too large
for inline?
or
Remove function mucse_mbx_reset, and copy all code in mucse_init_mbx_params_pf?
But mucse_mbx_reset reads value from reg, and other code in
mucse_init_mbx_params_pf is to init some variables in driver, I think it is not
good to combine all codes in one function?
> > +}
> > 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..eac99f13b6ff
> > --- /dev/null
> > +++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx.h
> > @@ -0,0 +1,20 @@
> > +/* 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_MBX_FW2PF_CNT 0
> > +#define MUCSE_MBX_PF2FW_CNT 4
> > +#define MUCSE_MBX_FWPF_SHM 8
>
> Are these constants used by other c files or should they be private to
> rnpgbe_mbx.c?
>
It is used only in rnpgbe_mbx.c.
But I check other drivers, it seems not common to make this condition
be private?
Just link
GMAC_EXT_CONFIG in dwmac4.h, only used in dwmac4_dma.c,
or
IXGBE_PFMAILBOX_ACK in ixgbe_mbx.h, only used in ixgbe_mbx.c.
> > +#define MUCSE_MBX_PF2FW_CTRL(mbx) ((mbx)->pf2fw_mbx_ctrl)
> > +#define MUCSE_MBX_FWPF_MASK(mbx) ((mbx)->fwpf_mbx_mask)
> > +#define MUCSE_MBX_REQ BIT(0) /* Request a req to mailbox */
> > +#define MUCSE_MBX_PFU BIT(3) /* PF owns the mailbox buffer */
> > +
> > +int mucse_write_and_wait_ack_mbx(struct mucse_hw *hw, u32 *msg, u16 size);
> > +void mucse_init_mbx_params_pf(struct mucse_hw *hw);
> > +int mucse_poll_and_read_mbx(struct mucse_hw *hw, u32 *msg, u16 size);
> > +#endif /* _RNPGBE_MBX_H */
> > --
> > 2.25.1
> >
> >
>
> --
> “Perl—the only language that looks the same
> before and after RSA encryption.” (Keith Bostic)
Thanks for your feedback.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH net-next v12 1/5] net: rnpgbe: Add build support for rnpgbe
2025-09-16 11:29 ` [PATCH net-next v12 1/5] net: rnpgbe: Add build support for rnpgbe Dong Yibo
2025-09-16 19:00 ` Jörg Sommer
@ 2025-09-17 7:01 ` MD Danish Anwar
1 sibling, 0 replies; 21+ messages in thread
From: MD Danish Anwar @ 2025-09-17 7:01 UTC (permalink / raw)
To: Dong Yibo, andrew+netdev, davem, edumazet, kuba, pabeni, horms,
corbet, gur.stavi, maddy, mpe, lee, gongfan1, lorenzo,
geert+renesas, Parthiban.Veerasooran, lukas.bulwahn,
alexanderduyck, richardcochran, kees, gustavoars, rdunlap,
vadim.fedorenko, joerg
Cc: netdev, linux-doc, linux-kernel, linux-hardening, Andrew Lunn
On 16/09/25 4:59 pm, Dong Yibo wrote:
> Add build options and doc for mucse.
> Initialize pci device access for MUCSE devices.
>
> Signed-off-by: Dong Yibo <dong100@mucse.com>
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>
> Reviewed-by: Vadim Fedorenko <vadim.fedorenko@linux.dev>
Reviewed-by: MD Danish Anwar <danishanwar@ti.com>
--
Thanks and Regards,
Danish
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH net-next v12 2/5] net: rnpgbe: Add n500/n210 chip support with BAR2 mapping
2025-09-16 11:29 ` [PATCH net-next v12 2/5] net: rnpgbe: Add n500/n210 chip support with BAR2 mapping Dong Yibo
2025-09-16 19:20 ` Jörg Sommer
@ 2025-09-17 7:02 ` MD Danish Anwar
1 sibling, 0 replies; 21+ messages in thread
From: MD Danish Anwar @ 2025-09-17 7:02 UTC (permalink / raw)
To: Dong Yibo, andrew+netdev, davem, edumazet, kuba, pabeni, horms,
corbet, gur.stavi, maddy, mpe, lee, gongfan1, lorenzo,
geert+renesas, Parthiban.Veerasooran, lukas.bulwahn,
alexanderduyck, richardcochran, kees, gustavoars, rdunlap,
vadim.fedorenko, joerg
Cc: netdev, linux-doc, linux-kernel, linux-hardening
On 16/09/25 4:59 pm, Dong Yibo wrote:
> Add hardware initialization foundation for MUCSE 1Gbe controller,
> including:
> 1. Map PCI BAR2 as hardware register base;
> 2. Bind PCI device to driver private data (struct mucse) and
> initialize hardware context (struct mucse_hw);
> 3. Reserve board-specific init framework via rnpgbe_init_hw.
>
> Signed-off-by: Dong Yibo <dong100@mucse.com>
> Reviewed-by: Vadim Fedorenko <vadim.fedorenko@linux.dev>
Reviewed-by: MD Danish Anwar <danishanwar@ti.com>
--
Thanks and Regards,
Danish
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH net-next v12 5/5] net: rnpgbe: Add register_netdev
2025-09-16 11:29 ` [PATCH net-next v12 5/5] net: rnpgbe: Add register_netdev Dong Yibo
2025-09-16 15:19 ` Vadim Fedorenko
@ 2025-09-17 7:09 ` MD Danish Anwar
2025-09-17 8:33 ` Yibo Dong
1 sibling, 1 reply; 21+ messages in thread
From: MD Danish Anwar @ 2025-09-17 7:09 UTC (permalink / raw)
To: Dong Yibo, andrew+netdev, davem, edumazet, kuba, pabeni, horms,
corbet, gur.stavi, maddy, mpe, lee, gongfan1, lorenzo,
geert+renesas, Parthiban.Veerasooran, lukas.bulwahn,
alexanderduyck, richardcochran, kees, gustavoars, rdunlap,
vadim.fedorenko, joerg
Cc: netdev, linux-doc, linux-kernel, linux-hardening
On 16/09/25 4:59 pm, Dong Yibo wrote:
> Complete the network device (netdev) registration flow for Mucse Gbe
> Ethernet chips, including:
> 1. Hardware state initialization:
> - Send powerup notification to firmware (via echo_fw_status)
> - Sync with firmware
> - Reset hardware
> 2. MAC address handling:
> - Retrieve permanent MAC from firmware (via mucse_mbx_get_macaddr)
> - Fallback to random valid MAC (eth_random_addr) if not valid mac
> from Fw
>
> Signed-off-by: Dong Yibo <dong100@mucse.com>
> ---
> drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h | 18 +++
> .../net/ethernet/mucse/rnpgbe/rnpgbe_chip.c | 80 ++++++++++++++
> drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h | 2 +
> .../net/ethernet/mucse/rnpgbe/rnpgbe_main.c | 103 ++++++++++++++++++
> 4 files changed, 203 insertions(+)
>
> diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h
> index 41b580f2168f..4c4b2f13cb4a 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>
>
> enum rnpgbe_boards {
> board_n500,
> @@ -34,12 +35,26 @@ struct mucse_mbx_info {
> u32 fwpf_ctrl_base;
> };
>
> +enum {
> + mucse_fw_powerup,
> +};
> +
This enum has only one value. You should either use a #define or add
more values to justify having an enum.
> struct mucse_hw {
> void __iomem *hw_addr;
> + struct pci_dev *pdev;
> + const struct mucse_hw_operations *ops;
> struct mucse_mbx_info mbx;
> + int port;
> + u8 perm_addr[ETH_ALEN];
> u8 pfvfnum;
> };
>
> +struct mucse_hw_operations {
> + int (*reset_hw)(struct mucse_hw *hw);
> + int (*get_perm_mac)(struct mucse_hw *hw);
> + int (*mbx_send_notify)(struct mucse_hw *hw, bool enable, int mode);
> +};
> +
> struct mucse {
> struct net_device *netdev;
> struct pci_dev *pdev;
> @@ -54,4 +69,7 @@ int rnpgbe_init_hw(struct mucse_hw *hw, int board_type);
> #define PCI_DEVICE_ID_N500_DUAL_PORT 0x8318
> #define PCI_DEVICE_ID_N210 0x8208
> #define PCI_DEVICE_ID_N210L 0x820a
> +
> +#define mucse_hw_wr32(hw, reg, val) \
> + writel((val), (hw)->hw_addr + (reg))
> #endif /* _RNPGBE_H */
> diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_chip.c b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_chip.c
> index 86f1c75796b0..667e372387a2 100644
> --- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_chip.c
> +++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_chip.c
> @@ -1,11 +1,88 @@
> // SPDX-License-Identifier: GPL-2.0
> /* Copyright(c) 2020 - 2025 Mucse Corporation. */
>
> +#include <linux/pci.h>
> #include <linux/errno.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
> + *
> + * 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 - Do a hardware reset
> + * @hw: hw information structure
> + *
> + * rnpgbe_reset 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(struct mucse_hw *hw)
> +{
> + mucse_hw_wr32(hw, RNPGBE_DMA_AXI_EN, 0);
> + return mucse_mbx_reset_hw(hw);
> +}
> +
> +/**
> + * rnpgbe_mbx_send_notify - 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_mbx_send_notify(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;
> + }
> +
Since you only have one mode currently, this switch statement seems
unnecessary.
> + return err;
> +}
--
Thanks and Regards,
Danish
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH net-next v12 5/5] net: rnpgbe: Add register_netdev
2025-09-17 7:09 ` MD Danish Anwar
@ 2025-09-17 8:33 ` Yibo Dong
0 siblings, 0 replies; 21+ messages in thread
From: Yibo Dong @ 2025-09-17 8:33 UTC (permalink / raw)
To: MD Danish Anwar
Cc: andrew+netdev, davem, edumazet, kuba, pabeni, horms, corbet,
gur.stavi, maddy, mpe, lee, gongfan1, lorenzo, geert+renesas,
Parthiban.Veerasooran, lukas.bulwahn, alexanderduyck,
richardcochran, kees, gustavoars, rdunlap, vadim.fedorenko, joerg,
netdev, linux-doc, linux-kernel, linux-hardening
On Wed, Sep 17, 2025 at 12:39:59PM +0530, MD Danish Anwar wrote:
> On 16/09/25 4:59 pm, Dong Yibo wrote:
> > Complete the network device (netdev) registration flow for Mucse Gbe
> > Ethernet chips, including:
> > 1. Hardware state initialization:
> > - Send powerup notification to firmware (via echo_fw_status)
> > - Sync with firmware
> > - Reset hardware
> > 2. MAC address handling:
> > - Retrieve permanent MAC from firmware (via mucse_mbx_get_macaddr)
> > - Fallback to random valid MAC (eth_random_addr) if not valid mac
> > from Fw
> >
> > Signed-off-by: Dong Yibo <dong100@mucse.com>
> > ---
> > drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h | 18 +++
> > .../net/ethernet/mucse/rnpgbe/rnpgbe_chip.c | 80 ++++++++++++++
> > drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h | 2 +
> > .../net/ethernet/mucse/rnpgbe/rnpgbe_main.c | 103 ++++++++++++++++++
> > 4 files changed, 203 insertions(+)
> >
> > diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h
> > index 41b580f2168f..4c4b2f13cb4a 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>
> >
> > enum rnpgbe_boards {
> > board_n500,
> > @@ -34,12 +35,26 @@ struct mucse_mbx_info {
> > u32 fwpf_ctrl_base;
> > };
> >
> > +enum {
> > + mucse_fw_powerup,
> > +};
> > +
>
> This enum has only one value. You should either use a #define or add
> more values to justify having an enum.
>
Yes, it has only one value now, but more values will be added in the
future.
If I add new value here, but this patch not use it, it is
confict with 'add define along with truely use'.
If I use a #define here, when I add new values in the future, I should
redefine it as enum.
So can I keep enum here with a commit like this ?
/* Enum for firmware notification modes,
more modes (e.g., portup, link_report) will be added in future */
+enum {
+ mucse_fw_powerup,
+};
+
> > struct mucse_hw {
> > void __iomem *hw_addr;
> > + struct pci_dev *pdev;
> > + const struct mucse_hw_operations *ops;
> > struct mucse_mbx_info mbx;
> > + int port;
> > + u8 perm_addr[ETH_ALEN];
> > u8 pfvfnum;
> > };
> >
> > +struct mucse_hw_operations {
> > + int (*reset_hw)(struct mucse_hw *hw);
> > + int (*get_perm_mac)(struct mucse_hw *hw);
> > + int (*mbx_send_notify)(struct mucse_hw *hw, bool enable, int mode);
> > +};
> > +
> > struct mucse {
> > struct net_device *netdev;
> > struct pci_dev *pdev;
> > @@ -54,4 +69,7 @@ int rnpgbe_init_hw(struct mucse_hw *hw, int board_type);
> > #define PCI_DEVICE_ID_N500_DUAL_PORT 0x8318
> > #define PCI_DEVICE_ID_N210 0x8208
> > #define PCI_DEVICE_ID_N210L 0x820a
> > +
> > +#define mucse_hw_wr32(hw, reg, val) \
> > + writel((val), (hw)->hw_addr + (reg))
> > #endif /* _RNPGBE_H */
> > diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_chip.c b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_chip.c
> > index 86f1c75796b0..667e372387a2 100644
> > --- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_chip.c
> > +++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_chip.c
> > @@ -1,11 +1,88 @@
> > // SPDX-License-Identifier: GPL-2.0
> > /* Copyright(c) 2020 - 2025 Mucse Corporation. */
> >
> > +#include <linux/pci.h>
> > #include <linux/errno.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
> > + *
> > + * 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 - Do a hardware reset
> > + * @hw: hw information structure
> > + *
> > + * rnpgbe_reset 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(struct mucse_hw *hw)
> > +{
> > + mucse_hw_wr32(hw, RNPGBE_DMA_AXI_EN, 0);
> > + return mucse_mbx_reset_hw(hw);
> > +}
> > +
> > +/**
> > + * rnpgbe_mbx_send_notify - 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_mbx_send_notify(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;
> > + }
> > +
>
> Since you only have one mode currently, this switch statement seems
> unnecessary.
>
The same reason with enum....
Can I keep switch here with a commit?
/* Keep switch structure to support more modes in the future */
switch (mode) {
case mucse_fw_powerup:
err = mucse_mbx_powerup(hw, enable);
break;
default:
err = -EINVAL;
}
> > + return err;
> > +}
>
>
> --
> Thanks and Regards,
> Danish
>
>
Thanks for feedback.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH net-next v12 4/5] net: rnpgbe: Add basic mbx_fw support
2025-09-16 11:29 ` [PATCH net-next v12 4/5] net: rnpgbe: Add basic mbx_fw support Dong Yibo
@ 2025-09-17 10:45 ` Vadim Fedorenko
2025-09-17 11:05 ` Yibo Dong
0 siblings, 1 reply; 21+ messages in thread
From: Vadim Fedorenko @ 2025-09-17 10:45 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, joerg
Cc: netdev, linux-doc, linux-kernel, linux-hardening
On 16/09/2025 12:29, Dong Yibo wrote:
> Add fundamental firmware (FW) communication operations via PF-FW
> mailbox, including:
> - FW sync (via HW info query with retries)
> - HW reset (post FW command to reset hardware)
> - MAC address retrieval (request FW for port-specific MAC)
> - Power management (powerup/powerdown notification to FW)
>
> Signed-off-by: Dong Yibo <dong100@mucse.com>
Reviewed-by: Vadim Fedorenko <vadim.fedorenko@linux.dev>
small nits below
> +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(MUCSE_MBX_REQ_HDR_LEN);
> + req->reply_lo = 0;
> + req->reply_hi = 0;
> +}
All these build*() functions re-init flags and reply to 0, but all
mbx_fw_cmd_req are zero-inited on the stack. Might be better clean
things assignments, but no strong opinion because the code is explicit
If you will think of refactoring this part, it might be a good idea to
avoid build*() functions at all and do proper initialization of
mbx_fw_cmd_req in callers?
> +
> +/**
> + * 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 = {};
something like:
struct mbx_fw_cmd_req req =
{
.opcode = cpu_to_le16(GET_HW_INFO),
.datalen = cpu_to_le16(MUCSE_MBX_REQ_HDR_LEN),
}
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH net-next v12 4/5] net: rnpgbe: Add basic mbx_fw support
2025-09-17 10:45 ` Vadim Fedorenko
@ 2025-09-17 11:05 ` Yibo Dong
2025-09-17 14:02 ` Vadim Fedorenko
0 siblings, 1 reply; 21+ messages in thread
From: Yibo Dong @ 2025-09-17 11:05 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, joerg,
netdev, linux-doc, linux-kernel, linux-hardening
On Wed, Sep 17, 2025 at 11:45:31AM +0100, Vadim Fedorenko wrote:
> On 16/09/2025 12:29, Dong Yibo wrote:
> > Add fundamental firmware (FW) communication operations via PF-FW
> > mailbox, including:
> > - FW sync (via HW info query with retries)
> > - HW reset (post FW command to reset hardware)
> > - MAC address retrieval (request FW for port-specific MAC)
> > - Power management (powerup/powerdown notification to FW)
> >
> > Signed-off-by: Dong Yibo <dong100@mucse.com>
>
> Reviewed-by: Vadim Fedorenko <vadim.fedorenko@linux.dev>
>
> small nits below
>
>
> > +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(MUCSE_MBX_REQ_HDR_LEN);
> > + req->reply_lo = 0;
> > + req->reply_hi = 0;
> > +}
>
> All these build*() functions re-init flags and reply to 0, but all
> mbx_fw_cmd_req are zero-inited on the stack. Might be better clean
> things assignments, but no strong opinion because the code is explicit
>
> If you will think of refactoring this part, it might be a good idea to
> avoid build*() functions at all and do proper initialization of
> mbx_fw_cmd_req in callers?
>
> > +
> > +/**
> > + * 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 = {};
>
> something like:
>
> struct mbx_fw_cmd_req req =
> {
> .opcode = cpu_to_le16(GET_HW_INFO),
> .datalen = cpu_to_le16(MUCSE_MBX_REQ_HDR_LEN),
> }
>
>
>
That's a good idea! That makes the code more compact.
I think I should update this as your suggestion.
Regarding adding your "Reviewed-by" tag in the next version:
Would it be acceptable to include it when I submit the updated patch (with
the initialization logic adjusted), or should I wait for your further
review of the modified code first?
Thanks for your feedback.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH net-next v12 4/5] net: rnpgbe: Add basic mbx_fw support
2025-09-17 11:05 ` Yibo Dong
@ 2025-09-17 14:02 ` Vadim Fedorenko
2025-09-19 1:17 ` Yibo Dong
0 siblings, 1 reply; 21+ messages in thread
From: Vadim Fedorenko @ 2025-09-17 14:02 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, joerg,
netdev, linux-doc, linux-kernel, linux-hardening
On 17/09/2025 12:05, Yibo Dong wrote:
> On Wed, Sep 17, 2025 at 11:45:31AM +0100, Vadim Fedorenko wrote:
>> On 16/09/2025 12:29, Dong Yibo wrote:
>>> Add fundamental firmware (FW) communication operations via PF-FW
>>> mailbox, including:
>>> - FW sync (via HW info query with retries)
>>> - HW reset (post FW command to reset hardware)
>>> - MAC address retrieval (request FW for port-specific MAC)
>>> - Power management (powerup/powerdown notification to FW)
>>>
>>> Signed-off-by: Dong Yibo <dong100@mucse.com>
>>
>> Reviewed-by: Vadim Fedorenko <vadim.fedorenko@linux.dev>
>>
>> small nits below
>>
>>
>>> +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(MUCSE_MBX_REQ_HDR_LEN);
>>> + req->reply_lo = 0;
>>> + req->reply_hi = 0;
>>> +}
>>
>> All these build*() functions re-init flags and reply to 0, but all
>> mbx_fw_cmd_req are zero-inited on the stack. Might be better clean
>> things assignments, but no strong opinion because the code is explicit
>>
>> If you will think of refactoring this part, it might be a good idea to
>> avoid build*() functions at all and do proper initialization of
>> mbx_fw_cmd_req in callers?
>>
>>> +
>>> +/**
>>> + * 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 = {};
>>
>> something like:
>>
>> struct mbx_fw_cmd_req req =
>> {
>> .opcode = cpu_to_le16(GET_HW_INFO),
>> .datalen = cpu_to_le16(MUCSE_MBX_REQ_HDR_LEN),
>> }
>>
>>
>>
>
> That's a good idea! That makes the code more compact.
> I think I should update this as your suggestion.
>
> Regarding adding your "Reviewed-by" tag in the next version:
> Would it be acceptable to include it when I submit the updated patch (with
> the initialization logic adjusted), or should I wait for your further
> review of the modified code first?
If you will submit another version with this refactoring, I'll better do
another review.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH net-next v12 4/5] net: rnpgbe: Add basic mbx_fw support
2025-09-17 14:02 ` Vadim Fedorenko
@ 2025-09-19 1:17 ` Yibo Dong
0 siblings, 0 replies; 21+ messages in thread
From: Yibo Dong @ 2025-09-19 1:17 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, joerg,
netdev, linux-doc, linux-kernel, linux-hardening
On Wed, Sep 17, 2025 at 03:02:13PM +0100, Vadim Fedorenko wrote:
> On 17/09/2025 12:05, Yibo Dong wrote:
> > On Wed, Sep 17, 2025 at 11:45:31AM +0100, Vadim Fedorenko wrote:
> > > On 16/09/2025 12:29, Dong Yibo wrote:
> > > > Add fundamental firmware (FW) communication operations via PF-FW
> > > > mailbox, including:
> > > > - FW sync (via HW info query with retries)
> > > > - HW reset (post FW command to reset hardware)
> > > > - MAC address retrieval (request FW for port-specific MAC)
> > > > - Power management (powerup/powerdown notification to FW)
> > > >
> > > > Signed-off-by: Dong Yibo <dong100@mucse.com>
> > >
> > > Reviewed-by: Vadim Fedorenko <vadim.fedorenko@linux.dev>
> > >
> > > small nits below
> > >
> > >
> > > > +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(MUCSE_MBX_REQ_HDR_LEN);
> > > > + req->reply_lo = 0;
> > > > + req->reply_hi = 0;
> > > > +}
> > >
> > > All these build*() functions re-init flags and reply to 0, but all
> > > mbx_fw_cmd_req are zero-inited on the stack. Might be better clean
> > > things assignments, but no strong opinion because the code is explicit
> > >
> > > If you will think of refactoring this part, it might be a good idea to
> > > avoid build*() functions at all and do proper initialization of
> > > mbx_fw_cmd_req in callers?
> > >
> > > > +
> > > > +/**
> > > > + * 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 = {};
> > >
> > > something like:
> > >
> > > struct mbx_fw_cmd_req req =
> > > {
> > > .opcode = cpu_to_le16(GET_HW_INFO),
> > > .datalen = cpu_to_le16(MUCSE_MBX_REQ_HDR_LEN),
> > > }
> > >
> > >
> > >
> >
> > That's a good idea! That makes the code more compact.
> > I think I should update this as your suggestion.
> >
> > Regarding adding your "Reviewed-by" tag in the next version:
> > Would it be acceptable to include it when I submit the updated patch (with
> > the initialization logic adjusted), or should I wait for your further
> > review of the modified code first?
>
> If you will submit another version with this refactoring, I'll better do
> another review.
>
I see, I will submit another version later, with this refactoring.
Looking forward to your next review.
Thanks for your feedback.
^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2025-09-19 1:18 UTC | newest]
Thread overview: 21+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-16 11:29 [PATCH net-next v12 0/5] Add driver for 1Gbe network chips from MUCSE Dong Yibo
2025-09-16 11:29 ` [PATCH net-next v12 1/5] net: rnpgbe: Add build support for rnpgbe Dong Yibo
2025-09-16 19:00 ` Jörg Sommer
2025-09-17 1:37 ` Yibo Dong
2025-09-17 7:01 ` MD Danish Anwar
2025-09-16 11:29 ` [PATCH net-next v12 2/5] net: rnpgbe: Add n500/n210 chip support with BAR2 mapping Dong Yibo
2025-09-16 19:20 ` Jörg Sommer
2025-09-17 7:02 ` MD Danish Anwar
2025-09-16 11:29 ` [PATCH net-next v12 3/5] net: rnpgbe: Add basic mbx ops support Dong Yibo
2025-09-16 18:42 ` Jörg Sommer
2025-09-16 19:33 ` Andrew Lunn
2025-09-17 2:55 ` Yibo Dong
2025-09-16 11:29 ` [PATCH net-next v12 4/5] net: rnpgbe: Add basic mbx_fw support Dong Yibo
2025-09-17 10:45 ` Vadim Fedorenko
2025-09-17 11:05 ` Yibo Dong
2025-09-17 14:02 ` Vadim Fedorenko
2025-09-19 1:17 ` Yibo Dong
2025-09-16 11:29 ` [PATCH net-next v12 5/5] net: rnpgbe: Add register_netdev Dong Yibo
2025-09-16 15:19 ` Vadim Fedorenko
2025-09-17 7:09 ` MD Danish Anwar
2025-09-17 8:33 ` Yibo Dong
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).