Re: [PATCH net-next 2] net: ngbe: sw init and hw init

[Andrew Lunn <andrew@lunn.ch>]

> +static void ngbe_get_mac_addr(struct ngbe_hw *hw, u8 *mac_addr)
> +{
> +	u32 rar_high;
> +	u32 rar_low;
> +	u16 i;
> +
> +	wr32(hw, NGBE_PSR_MAC_SWC_IDX, 0);
> +	rar_high = rd32(hw, NGBE_PSR_MAC_SWC_AD_H);
> +	rar_low = rd32(hw, NGBE_PSR_MAC_SWC_AD_L);
> +
> +	for (i = 0; i < 2; i++)
> +		mac_addr[i] = (u8)(rar_high >> (1 - i) * 8);
> +
> +	for (i = 0; i < 4; i++)
> +		mac_addr[i + 2] = (u8)(rar_low >> (3 - i) * 8);
> +}

This looks identical to txgbe_get_mac_addr():

https://patchwork.kernel.org/project/netdevbpf/patch/20220830070454.146211-4-jiawenwu@trustnetic.com/

How similar is txgbe and ngdb? Should there be some shared code?

> +
> +/**
> + *  ngbe_validate_mac_addr - Validate MAC address
> + *  @mac_addr: pointer to MAC address.
> + *
> + *  Tests a MAC address to ensure it is a valid Individual Address
> + **/
> +static int ngbe_validate_mac_addr(u8 *mac_addr)
> +{
> +	/* Make sure it is not a multicast address */
> +	if (is_multicast_ether_addr(mac_addr))
> +		return -EINVAL;
> +	/* Not a broadcast address */
> +	else if (is_broadcast_ether_addr(mac_addr))
> +		return -EINVAL;
> +	/* Reject the zero address */
> +	else if (is_zero_ether_addr(mac_addr))
> +		return -EINVAL;
> +	return 0;
> +}
> +
> +/**
> + *  ngbe_set_rar - Set Rx address register
> + *  @hw: pointer to hardware structure
> + *  @index: Receive address register to write
> + *  @addr: Address to put into receive address register
> + *  @pools: "pool" index
> + *  @enable_addr: set flag that address is active
> + *
> + *  Puts an ethernet address into a receive address register.
> + **/
> +static int ngbe_set_rar(struct ngbe_hw *hw, u32 index, u8 *addr, u64 pools,
> +			u32 enable_addr)
> +{

This function looks identical to txgbe_set_rar().

> +/**
> + *  ngbe_init_rx_addrs - Initializes receive address filters.
> + *  @hw: pointer to hardware structure
> + *
> + *  Places the MAC address in receive address register 0 and clears the rest
> + *  of the receive address registers. Clears the multicast table. Assumes
> + *  the receiver is in reset when the routine is called.
> + **/
> +static void ngbe_init_rx_addrs(struct ngbe_hw *hw)
> +{
> +	u32 i;
> +	u32 rar_entries = hw->mac.num_rar_entries;
> +	u32 psrctl;

Apart from txgbe_init_rx_addrs() uses is_valid_ether_addr(), this is
also identical.

> +static int ngbe_fmgr_cmd_op(struct ngbe_hw *hw, u32 cmd, u32 cmd_addr)
> +{
> +	u32 cmd_val = 0, val = 0;
> +
> +	cmd_val = (cmd << NGBE_SPI_CLK_CMD_OFFSET) |
> +		  (NGBE_SPI_CLK_DIV << NGBE_SPI_CLK_DIV_OFFSET) | cmd_addr;
> +	wr32(hw, NGBE_SPI_H_CMD_REG_ADDR, cmd_val);
> +
> +	return read_poll_timeout(rd32, val, (val & 0x1), 10, NGBE_SPI_TIME_OUT_VALUE,
> +				 false, hw, NGBE_SPI_H_STA_REG_ADDR);
> +}
> +
> +int ngbe_flash_read_dword(struct ngbe_hw *hw, u32 addr, u32 *data)
> +{
> +	int ret = 0;
> +
> +	ret = ngbe_fmgr_cmd_op(hw, NGBE_SPI_CMD_READ_DWORD, addr);
> +	if (ret < 0)
> +		return ret;
> +	*data = rd32(hw, NGBE_SPI_H_DAT_REG_ADDR);
> +
> +	return 0;
> +}

Identical to txgbe_flash_read_dword

You need to work with Jiawen Wu <jiawe...@trustnetic.com> and pull the
common code out into a library that the two drivers share. Is
jiawenwu@net-swift.com the same person ?

       Andrew