Re: [PATCH v2 02/15] net: rnpgbe: Add n500/n210 chip support

[Vadim Fedorenko <vadim.fedorenko@linux.dev>]

On 21/07/2025 12:32, Dong Yibo wrote:
> Initialize n500/n210 chip bar resource map and
> dma, eth, mbx ... info for future use.
> 
> Signed-off-by: Dong Yibo <dong100@mucse.com>
> ---
>   drivers/net/ethernet/mucse/rnpgbe/Makefile    |   4 +-
>   drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h    | 138 ++++++++++++++++++
>   .../net/ethernet/mucse/rnpgbe/rnpgbe_chip.c   | 138 ++++++++++++++++++
>   drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h |  27 ++++
>   .../net/ethernet/mucse/rnpgbe/rnpgbe_main.c   |  68 ++++++++-
>   5 files changed, 370 insertions(+), 5 deletions(-)
>   create mode 100644 drivers/net/ethernet/mucse/rnpgbe/rnpgbe_chip.c
>   create mode 100644 drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h
> 
> diff --git a/drivers/net/ethernet/mucse/rnpgbe/Makefile b/drivers/net/ethernet/mucse/rnpgbe/Makefile
> index 0942e27f5913..42c359f459d9 100644
> --- a/drivers/net/ethernet/mucse/rnpgbe/Makefile
> +++ b/drivers/net/ethernet/mucse/rnpgbe/Makefile
> @@ -5,5 +5,5 @@
>   #
>   
>   obj-$(CONFIG_MGBE) += rnpgbe.o
> -
> -rnpgbe-objs := rnpgbe_main.o
> +rnpgbe-objs := rnpgbe_main.o\
> +	       rnpgbe_chip.o
> diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h
> index 224e395d6be3..2ae836fc8951 100644
> --- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h
> +++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h
> @@ -4,21 +4,156 @@
>   #ifndef _RNPGBE_H
>   #define _RNPGBE_H
>   
> +#include <linux/types.h>
> +#include <linux/netdevice.h>
> +
> +extern const struct rnpgbe_info rnpgbe_n500_info;
> +extern const struct rnpgbe_info rnpgbe_n210_info;
> +extern const struct rnpgbe_info rnpgbe_n210L_info;
> +
>   enum rnpgbe_boards {
>   	board_n500,
>   	board_n210,
>   	board_n210L,
>   };
>   
> +enum rnpgbe_hw_type {
> +	rnpgbe_hw_n500 = 0,
> +	rnpgbe_hw_n210,
> +	rnpgbe_hw_n210L,
> +	rnpgbe_hw_unknow
> +};
> +
> +struct mucse_dma_info {
> +	u8 __iomem *dma_base_addr;
> +	u8 __iomem *dma_ring_addr;
> +	void *back;
> +	u32 max_tx_queues;
> +	u32 max_rx_queues;
> +	u32 dma_version;
> +};
> +
> +#define RNPGBE_MAX_MTA 128
> +struct mucse_eth_info {
> +	u8 __iomem *eth_base_addr;
> +	void *back;
> +	u32 mta_shadow[RNPGBE_MAX_MTA];
> +	int mc_filter_type;
> +	u32 mcft_size;
> +	u32 vft_size;
> +	u32 num_rar_entries;
> +};
> +
> +struct mii_regs {
> +	unsigned int addr; /* MII Address */
> +	unsigned int data; /* MII Data */
> +	unsigned int addr_shift; /* MII address shift */
> +	unsigned int reg_shift; /* MII reg shift */
> +	unsigned int addr_mask; /* MII address mask */
> +	unsigned int reg_mask; /* MII reg mask */
> +	unsigned int clk_csr_shift;
> +	unsigned int clk_csr_mask;
> +};
> +
> +struct mucse_mac_info {
> +	u8 __iomem *mac_addr;
> +	void *back;
> +	struct mii_regs mii;
> +	int phy_addr;
> +	int clk_csr;
> +};
> +
> +#define MAX_VF_NUM (8)
> +
> +struct mucse_mbx_info {
> +	u32 timeout;
> +	u32 usec_delay;
> +	u32 v2p_mailbox;
> +	u16 size;
> +	u16 vf_req[MAX_VF_NUM];
> +	u16 vf_ack[MAX_VF_NUM];
> +	u16 fw_req;
> +	u16 fw_ack;
> +	/* lock for only one use mbx */
> +	struct mutex lock;
> +	bool irq_enabled;
> +	int mbx_size;
> +	int mbx_mem_size;
> +#define MBX_FEATURE_NO_ZERO BIT(0)
> +#define MBX_FEATURE_WRITE_DELAY BIT(1)
> +	u32 mbx_feature;
> +	/* fw <--> pf mbx */
> +	u32 fw_pf_shm_base;
> +	u32 pf2fw_mbox_ctrl;
> +	u32 pf2fw_mbox_mask;
> +	u32 fw_pf_mbox_mask;
> +	u32 fw2pf_mbox_vec;
> +	/* pf <--> vf mbx */
> +	u32 pf_vf_shm_base;
> +	u32 pf2vf_mbox_ctrl_base;
> +	u32 pf_vf_mbox_mask_lo;
> +	u32 pf_vf_mbox_mask_hi;
> +	u32 pf2vf_mbox_vec_base;
> +	u32 vf2pf_mbox_vec_base;
> +	u32 fw_vf_share_ram;
> +	int share_size;
> +};
> +
> +struct mucse_hw {
> +	void *back;
> +	u8 pfvfnum;
> +	u8 pfvfnum_system;
> +	u8 __iomem *hw_addr;
> +	u8 __iomem *ring_msix_base;
> +	struct pci_dev *pdev;
> +	u16 device_id;
> +	u16 vendor_id;
> +	u16 subsystem_device_id;
> +	u16 subsystem_vendor_id;
> +	enum rnpgbe_hw_type hw_type;
> +	struct mucse_dma_info dma;
> +	struct mucse_eth_info eth;
> +	struct mucse_mac_info mac;
> +	struct mucse_mbx_info mbx;
> +#define M_NET_FEATURE_SG BIT(0)
> +#define M_NET_FEATURE_TX_CHECKSUM BIT(1)
> +#define M_NET_FEATURE_RX_CHECKSUM BIT(2)
> +#define M_NET_FEATURE_TSO BIT(3)
> +#define M_NET_FEATURE_TX_UDP_TUNNEL BIT(4)
> +#define M_NET_FEATURE_VLAN_FILTER BIT(5)
> +#define M_NET_FEATURE_VLAN_OFFLOAD BIT(6)
> +#define M_NET_FEATURE_RX_NTUPLE_FILTER BIT(7)
> +#define M_NET_FEATURE_TCAM BIT(8)
> +#define M_NET_FEATURE_RX_HASH BIT(9)
> +#define M_NET_FEATURE_RX_FCS BIT(10)
> +#define M_NET_FEATURE_HW_TC BIT(11)
> +#define M_NET_FEATURE_USO BIT(12)
> +#define M_NET_FEATURE_STAG_FILTER BIT(13)
> +#define M_NET_FEATURE_STAG_OFFLOAD BIT(14)
> +#define M_NET_FEATURE_VF_FIXED BIT(15)
> +#define M_VEB_VLAN_MASK_EN BIT(16)
> +#define M_HW_FEATURE_EEE BIT(17)
> +#define M_HW_SOFT_MASK_OTHER_IRQ BIT(18)
> +	u32 feature_flags;
> +	u16 usecstocount;
> +};
> +
>   struct mucse {
>   	struct net_device *netdev;
>   	struct pci_dev *pdev;
> +	struct mucse_hw hw;
>   	/* board number */
>   	u16 bd_number;
>   
>   	char name[60];
>   };
>   
> +struct rnpgbe_info {
> +	int total_queue_pair_cnts;
> +	enum rnpgbe_hw_type hw_type;
> +	void (*get_invariants)(struct mucse_hw *hw);
> +};
> +
>   /* Device IDs */
>   #ifndef PCI_VENDOR_ID_MUCSE
>   #define PCI_VENDOR_ID_MUCSE 0x8848
> @@ -30,4 +165,7 @@ struct mucse {
>   #define PCI_DEVICE_ID_N210 0x8208
>   #define PCI_DEVICE_ID_N210L 0x820a
>   
> +#define m_rd_reg(reg) readl(reg)
> +#define m_wr_reg(reg, val) writel((val), reg)
> +
>   #endif /* _RNPGBE_H */
> 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..38c094965db9
> --- /dev/null
> +++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_chip.c
> @@ -0,0 +1,138 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright(c) 2020 - 2025 Mucse Corporation. */
> +
> +#include <linux/types.h>
> +#include <linux/string.h>
> +
> +#include "rnpgbe.h"
> +#include "rnpgbe_hw.h"
> +
> +/**
> + * rnpgbe_get_invariants_n500 - setup for hw info
> + * @hw: hw information structure
> + *
> + * rnpgbe_get_invariants_n500 initializes all private
> + * structure, such as dma, eth, mac and mbx base on
> + * hw->addr for n500
> + **/
> +static void rnpgbe_get_invariants_n500(struct mucse_hw *hw)
> +{
> +	struct mucse_dma_info *dma = &hw->dma;
> +	struct mucse_eth_info *eth = &hw->eth;
> +	struct mucse_mac_info *mac = &hw->mac;
> +	struct mucse_mbx_info *mbx = &hw->mbx;
> +
> +	/* setup msix base */
> +	hw->ring_msix_base = hw->hw_addr + 0x28700;
> +	/* setup dma info */
> +	dma->dma_base_addr = hw->hw_addr;
> +	dma->dma_ring_addr = hw->hw_addr + RNPGBE_RING_BASE;
> +	dma->max_tx_queues = RNPGBE_MAX_QUEUES;
> +	dma->max_rx_queues = RNPGBE_MAX_QUEUES;
> +	dma->back = hw;
> +	/* setup eth info */
> +	eth->eth_base_addr = hw->hw_addr + RNPGBE_ETH_BASE;
> +	eth->back = hw;
> +	eth->mc_filter_type = 0;
> +	eth->mcft_size = RNPGBE_MC_TBL_SIZE;
> +	eth->vft_size = RNPGBE_VFT_TBL_SIZE;
> +	eth->num_rar_entries = RNPGBE_RAR_ENTRIES;
> +	/* setup mac info */
> +	mac->mac_addr = hw->hw_addr + RNPGBE_MAC_BASE;
> +	mac->back = hw;
> +	/* set mac->mii */
> +	mac->mii.addr = RNPGBE_MII_ADDR;
> +	mac->mii.data = RNPGBE_MII_DATA;
> +	mac->mii.addr_shift = 11;
> +	mac->mii.addr_mask = 0x0000F800;
> +	mac->mii.reg_shift = 6;
> +	mac->mii.reg_mask = 0x000007C0;
> +	mac->mii.clk_csr_shift = 2;
> +	mac->mii.clk_csr_mask = GENMASK(5, 2);
> +	mac->clk_csr = 0x02; /* csr 25M */
> +	/* hw fixed phy_addr */
> +	mac->phy_addr = 0x11;
> +
> +	mbx->mbx_feature |= MBX_FEATURE_NO_ZERO;
> +	/* mbx offset */
> +	mbx->vf2pf_mbox_vec_base = 0x28900;
> +	mbx->fw2pf_mbox_vec = 0x28b00;
> +	mbx->pf_vf_shm_base = 0x29000;
> +	mbx->mbx_mem_size = 64;
> +	mbx->pf2vf_mbox_ctrl_base = 0x2a100;
> +	mbx->pf_vf_mbox_mask_lo = 0x2a200;
> +	mbx->pf_vf_mbox_mask_hi = 0;
> +	mbx->fw_pf_shm_base = 0x2d000;
> +	mbx->pf2fw_mbox_ctrl = 0x2e000;
> +	mbx->fw_pf_mbox_mask = 0x2e200;
> +	mbx->fw_vf_share_ram = 0x2b000;
> +	mbx->share_size = 512;
> +
> +	/* setup net feature here */
> +	hw->feature_flags |= M_NET_FEATURE_SG |
> +			     M_NET_FEATURE_TX_CHECKSUM |
> +			     M_NET_FEATURE_RX_CHECKSUM |
> +			     M_NET_FEATURE_TSO |
> +			     M_NET_FEATURE_VLAN_FILTER |
> +			     M_NET_FEATURE_VLAN_OFFLOAD |
> +			     M_NET_FEATURE_RX_NTUPLE_FILTER |
> +			     M_NET_FEATURE_RX_HASH |
> +			     M_NET_FEATURE_USO |
> +			     M_NET_FEATURE_RX_FCS |
> +			     M_NET_FEATURE_STAG_FILTER |
> +			     M_NET_FEATURE_STAG_OFFLOAD;
> +	/* start the default ahz, update later */
> +	hw->usecstocount = 125;
> +}
> +
> +/**
> + * rnpgbe_get_invariants_n210 - setup for hw info
> + * @hw: hw information structure
> + *
> + * rnpgbe_get_invariants_n210 initializes all private
> + * structure, such as dma, eth, mac and mbx base on
> + * hw->addr for n210
> + **/
> +static void rnpgbe_get_invariants_n210(struct mucse_hw *hw)
> +{
> +	struct mucse_mbx_info *mbx = &hw->mbx;
> +	/* get invariants based from n500 */
> +	rnpgbe_get_invariants_n500(hw);

it's not a good pattern. if you have some configuration that is
shared amoung devices, it's better to create *base() or *common()
helper and call it from each specific initializer. BTW, why do you
name these functions get_invariants*()? They don't get anything, but
rather init/setup configuration values. It's better to rename it
according to the function.

> +
> +	/* update msix base */
> +	hw->ring_msix_base = hw->hw_addr + 0x29000;
> +	/* update mbx offset */
> +	mbx->vf2pf_mbox_vec_base = 0x29200;
> +	mbx->fw2pf_mbox_vec = 0x29400;
> +	mbx->pf_vf_shm_base = 0x29900;
> +	mbx->mbx_mem_size = 64;
> +	mbx->pf2vf_mbox_ctrl_base = 0x2aa00;
> +	mbx->pf_vf_mbox_mask_lo = 0x2ab00;
> +	mbx->pf_vf_mbox_mask_hi = 0;
> +	mbx->fw_pf_shm_base = 0x2d900;
> +	mbx->pf2fw_mbox_ctrl = 0x2e900;
> +	mbx->fw_pf_mbox_mask = 0x2eb00;
> +	mbx->fw_vf_share_ram = 0x2b900;
> +	mbx->share_size = 512;
> +	/* update hw feature */
> +	hw->feature_flags |= M_HW_FEATURE_EEE;
> +	hw->usecstocount = 62;
> +}
> +
> +const struct rnpgbe_info rnpgbe_n500_info = {
> +	.total_queue_pair_cnts = RNPGBE_MAX_QUEUES,
> +	.hw_type = rnpgbe_hw_n500,
> +	.get_invariants = &rnpgbe_get_invariants_n500,
> +};
> +
> +const struct rnpgbe_info rnpgbe_n210_info = {
> +	.total_queue_pair_cnts = RNPGBE_MAX_QUEUES,
> +	.hw_type = rnpgbe_hw_n210,
> +	.get_invariants = &rnpgbe_get_invariants_n210,
> +};
> +
> +const struct rnpgbe_info rnpgbe_n210L_info = {
> +	.total_queue_pair_cnts = RNPGBE_MAX_QUEUES,
> +	.hw_type = rnpgbe_hw_n210L,
> +	.get_invariants = &rnpgbe_get_invariants_n210,
> +};
> diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h
> new file mode 100644
> index 000000000000..2c7372a5e88d
> --- /dev/null
> +++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h
> @@ -0,0 +1,27 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/* Copyright(c) 2020 - 2025 Mucse Corporation. */
> +
> +#ifndef _RNPGBE_HW_H
> +#define _RNPGBE_HW_H
> +/*                     BAR                   */
> +/* ----------------------------------------- */
> +/*      module  | size  |  start   |    end  */
> +/*      DMA     | 32KB  | 0_0000H  | 0_7FFFH */
> +/*      ETH     | 64KB  | 1_0000H  | 1_FFFFH */
> +/*      MAC     | 32KB  | 2_0000H  | 2_7FFFH */
> +/*      MSIX    | 32KB  | 2_8000H  | 2_FFFFH */
> +
> +#define RNPGBE_RING_BASE (0x1000)
> +#define RNPGBE_MAC_BASE (0x20000)
> +#define RNPGBE_ETH_BASE (0x10000)
> +/* chip resourse */
> +#define RNPGBE_MAX_QUEUES (8)
> +/* multicast control table */
> +#define RNPGBE_MC_TBL_SIZE (128)
> +/* vlan filter table */
> +#define RNPGBE_VFT_TBL_SIZE (128)
> +#define RNPGBE_RAR_ENTRIES (32)

no need for extra parentheses

> +
> +#define RNPGBE_MII_ADDR 0x00000010 /* MII Address */
> +#define RNPGBE_MII_DATA 0x00000014 /* MII Data */
> +#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 13b49875006b..08f773199e9b 100644
> --- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c
> +++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c
> @@ -11,6 +11,11 @@
>   #include "rnpgbe.h"
>   
>   char rnpgbe_driver_name[] = "rnpgbe";
> +static const struct rnpgbe_info *rnpgbe_info_tbl[] = {
> +	[board_n500] = &rnpgbe_n500_info,
> +	[board_n210] = &rnpgbe_n210_info,
> +	[board_n210L] = &rnpgbe_n210L_info,
> +};
>   
>   /* rnpgbe_pci_tbl - PCI Device ID Table
>    *
> @@ -33,6 +38,7 @@ static struct pci_device_id rnpgbe_pci_tbl[] = {
>   /**
>    * rnpgbe_add_adapter - add netdev for this pci_dev
>    * @pdev: PCI device information structure
> + * @ii: chip info structure
>    *
>    * rnpgbe_add_adapter initializes a netdev for this pci_dev
>    * structure. Initializes Bar map, private structure, and a
> @@ -40,16 +46,24 @@ static struct pci_device_id rnpgbe_pci_tbl[] = {
>    *
>    * @return: 0 on success, negative on failure
>    **/
> -static int rnpgbe_add_adapter(struct pci_dev *pdev)
> +static int rnpgbe_add_adapter(struct pci_dev *pdev,
> +			      const struct rnpgbe_info *ii)
>   {
>   	struct mucse *mucse = NULL;
> +	struct mucse_hw *hw = NULL;
> +	u8 __iomem *hw_addr = NULL;
>   	struct net_device *netdev;
>   	static int bd_number;
> +	u32 dma_version = 0;
> +	int err = 0;
> +	u32 queues;
>   
> -	netdev = alloc_etherdev_mq(sizeof(struct mucse), 1);
> +	queues = ii->total_queue_pair_cnts;
> +	netdev = alloc_etherdev_mq(sizeof(struct mucse), queues);
>   	if (!netdev)
>   		return -ENOMEM;
>   
> +	SET_NETDEV_DEV(netdev, &pdev->dev);
>   	mucse = netdev_priv(netdev);
>   	mucse->netdev = netdev;
>   	mucse->pdev = pdev;
> @@ -58,7 +72,54 @@ static int rnpgbe_add_adapter(struct pci_dev *pdev)
>   		 rnpgbe_driver_name, mucse->bd_number);
>   	pci_set_drvdata(pdev, mucse);
>   
> +	hw = &mucse->hw;
> +	hw->back = mucse;
> +	hw->hw_type = ii->hw_type;
> +
> +	switch (hw->hw_type) {
> +	case rnpgbe_hw_n500:
> +		/* n500 use bar2 */
> +		hw_addr = devm_ioremap(&pdev->dev,
> +				       pci_resource_start(pdev, 2),
> +				       pci_resource_len(pdev, 2));
> +		if (!hw_addr) {
> +			dev_err(&pdev->dev, "map bar2 failed!\n");
> +			return -EIO;
> +		}
> +
> +		/* get dma version */
> +		dma_version = m_rd_reg(hw_addr);
> +		break;
> +	case rnpgbe_hw_n210:
> +	case rnpgbe_hw_n210L:
> +		/* check bar0 to load firmware */
> +		if (pci_resource_len(pdev, 0) == 0x100000)
> +			return -EIO;
> +		/* n210 use bar2 */
> +		hw_addr = devm_ioremap(&pdev->dev,
> +				       pci_resource_start(pdev, 2),
> +				       pci_resource_len(pdev, 2));
> +		if (!hw_addr) {
> +			dev_err(&pdev->dev, "map bar2 failed!\n");
> +			return -EIO;
> +		}
> +
> +		/* get dma version */
> +		dma_version = m_rd_reg(hw_addr);
> +		break;
> +	default:
> +		err = -EIO;
> +		goto err_free_net;
> +	}
> +	hw->hw_addr = hw_addr;
> +	hw->dma.dma_version = dma_version;
> +	ii->get_invariants(hw);
> +
>   	return 0;
> +
> +err_free_net:
> +	free_netdev(netdev);
> +	return err;
>   }

You have err_free_net label, which is used only in really impossible
case of unknown device, while other cases can return directly and
memleak netdev...


>   
>   /**
> @@ -74,6 +135,7 @@ static int rnpgbe_add_adapter(struct pci_dev *pdev)
>    **/
>   static int rnpgbe_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>   {
> +	const struct rnpgbe_info *ii = rnpgbe_info_tbl[id->driver_data];
>   	int err;
>   
>   	err = pci_enable_device_mem(pdev);
> @@ -97,7 +159,7 @@ static int rnpgbe_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>   
>   	pci_set_master(pdev);
>   	pci_save_state(pdev);
> -	err = rnpgbe_add_adapter(pdev);
> +	err = rnpgbe_add_adapter(pdev, ii);
>   	if (err)
>   		goto err_regions;
>