Re: [PATCH net-next v12 5/5] net: rnpgbe: Add register_netdev

MD Danish Anwar
2025-09-17T07:12:13Z

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