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

[MD Danish Anwar <danishanwar@ti.com>]

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