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

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

On 09/09/2025 13:09, 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>

[...]

> +struct mucse_hw;

why do you need this forward declaration ...> +
> +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);
> +};
> +
> +enum {
> +	mucse_fw_powerup,
> +};
> +
>   struct mucse_hw {
>   	void __iomem *hw_addr;
> +	struct pci_dev *pdev;
> +	const struct mucse_hw_operations *ops;
> +	struct mucse_dma_info dma;
>   	struct mucse_mbx_info mbx;
> +	int port;
> +	u8 perm_addr[ETH_ALEN];
>   	u8 pfvfnum;
>   };

... if you can simply move mucse_hw_operations down here?

>   
> @@ -54,4 +76,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 rnpgbe_dma_wr32(dma, reg, val) \
> +	writel((val), (dma)->dma_base_addr + (reg))

[...]

> @@ -48,8 +127,14 @@ static void rnpgbe_init_n210(struct mucse_hw *hw)
>    **/
>   int rnpgbe_init_hw(struct mucse_hw *hw, int board_type)
>   {
> +	struct mucse_dma_info *dma = &hw->dma;
>   	struct mucse_mbx_info *mbx = &hw->mbx;
>   
> +	hw->ops = &rnpgbe_hw_ops;
> +	hw->port = 0;
> +
> +	dma->dma_base_addr = hw->hw_addr;

not quite sure why do you need additional structure just to store the
value that already exists in mucse_hw?

> +
>   	mbx->pf2fw_mbx_ctrl = MUCSE_GBE_PFFW_MBX_CTRL_OFFSET;
>   	mbx->fwpf_mbx_mask = MUCSE_GBE_FWPF_MBX_MASK_OFFSET;
>