[dpdk-dev] [PATCH v2 01/20] net/ice: add base code

[Varghese, Vipin]

Snipped

> +Intel® ICE driver
> +==================
> +
> +This directory contains source code of FreeBSD ice driver of version
> +2018.10.30 released by the team which develops
> +basic drivers for any ice NIC. The directory of base/ contains the
> +original source package.
> +This driver is valid for the product(s) listed below
> +
> +* Intel® Ethernet Network Adapters E810
> +
> +Updating the driver
> +===================
> +
> +NOTE: The source code in this directory should not be modified apart from
> +the following file(s):
> +
> +    ice_osdep.h

I this README persistent in upcoming releases of 'driver/net/ice'?

Snipped

> +/* Manage MAC address, write command - direct (0x0108) */
> +struct ice_aqc_manage_mac_write {
> +	u8 port_num;
> +	u8 flags;
> +#define ICE_AQC_MAN_MAC_WR_MC_MAG_EN		BIT(0)
> +#define ICE_AQC_MAN_MAC_WR_WOL_LAA_PFR_KEEP	BIT(1)
> +#define ICE_AQC_MAN_MAC_WR_S		6
> +#define ICE_AQC_MAN_MAC_WR_M		(3 <<
> ICE_AQC_MAN_MAC_WR_S)

Is this value '3' or 'BIT(3)'?

> +#define ICE_AQC_MAN_MAC_UPDATE_LAA	0
> +#define ICE_AQC_MAN_MAC_UPDATE_LAA_WOL	(BIT(0) <<
> ICE_AQC_MAN_MAC_WR_S)

Can the code be rearranged for? 

#define ICE_AQC_MAN_MAC_WR_S		6
#define ICE_AQC_MAN_MAC_UPDATE_LAA	0
#define ICE_AQC_MAN_MAC_WR_M		(3 << ICE_AQC_MAN_MAC_WR_S)
#define ICE_AQC_MAN_MAC_UPDATE_LAA_WOL	(BIT(0) << ICE_AQC_MAN_MAC_WR_S)

Snipped

> +/* Each entry in the response buffer is of the following type: */
> +struct ice_aqc_get_sw_cfg_resp_elem {
> +	/* VSI/Port Number */
> +	__le16 vsi_port_num;
> +#define ICE_AQC_GET_SW_CONF_RESP_VSI_PORT_NUM_S	0
> +#define ICE_AQC_GET_SW_CONF_RESP_VSI_PORT_NUM_M	\
> +			(0x3FF <<
> ICE_AQC_GET_SW_CONF_RESP_VSI_PORT_NUM_S)
> +#define ICE_AQC_GET_SW_CONF_RESP_TYPE_S	14
> +#define ICE_AQC_GET_SW_CONF_RESP_TYPE_M	(0x3 <<
> ICE_AQC_GET_SW_CONF_RESP_TYPE_S)
> +#define ICE_AQC_GET_SW_CONF_RESP_PHYS_PORT	0
> +#define ICE_AQC_GET_SW_CONF_RESP_VIRT_PORT	1
> +#define ICE_AQC_GET_SW_CONF_RESP_VSI		2
> +

Can the code be rearranged for? 

#define ICE_AQC_GET_SW_CONF_RESP_VSI_PORT_NUM_S	0
#define ICE_AQC_GET_SW_CONF_RESP_TYPE_S	14
#define ICE_AQC_GET_SW_CONF_RESP_VSI_PORT_NUM_M	\
			(0x3FF << ICE_AQC_GET_SW_CONF_RESP_VSI_PORT_NUM_S)
#define ICE_AQC_GET_SW_CONF_RESP_TYPE_M	(0x3 << ICE_AQC_GET_SW_CONF_RESP_TYPE_S)

snipped

 +
> +struct ice_aqc_get_phy_caps_data {
> +	__le64 phy_type_low; /* Use values from ICE_PHY_TYPE_LOW_* */
> +	__le64 reserved;
> +	u8 caps;
> +#define ICE_AQC_PHY_EN_TX_LINK_PAUSE			BIT(0)
> +#define ICE_AQC_PHY_EN_RX_LINK_PAUSE			BIT(1)
> +#define ICE_AQC_PHY_LOW_POWER_MODE			BIT(2)
> +#define ICE_AQC_PHY_EN_LINK				BIT(3)
> +#define ICE_AQC_PHY_AN_MODE				BIT(4)
> +#define ICE_AQC_PHY_EN_MOD_QUAL				BIT(5)
> +#define ICE_AQC_PHY_EN_LESM				BIT(6)
> +#define ICE_AQC_PHY_EN_AUTO_FEC				BIT(7)
> +#define ICE_AQC_PHY_CAPS_MASK
> 	MAKEMASK(0xff, 0)
> +	u8 low_power_ctrl;
> +#define ICE_AQC_PHY_EN_D3COLD_LOW_POWER_AUTONEG		BIT(0)
> +	__le16 eee_cap;
> +#define ICE_AQC_PHY_EEE_EN_100BASE_TX			BIT(0)
> +#define ICE_AQC_PHY_EEE_EN_1000BASE_T			BIT(1)
> +#define ICE_AQC_PHY_EEE_EN_10GBASE_T			BIT(2)
> +#define ICE_AQC_PHY_EEE_EN_1000BASE_KX			BIT(3)
> +#define ICE_AQC_PHY_EEE_EN_10GBASE_KR			BIT(4)
> +#define ICE_AQC_PHY_EEE_EN_25GBASE_KR			BIT(5)
> +#define ICE_AQC_PHY_EEE_EN_40GBASE_KR4			BIT(6)
> +	__le16 eeer_value;
> +	u8 phy_id_oui[4]; /* PHY/Module ID connected on the port */
> +	u8 phy_fw_ver[8];
> +	u8 link_fec_options;
> +#define ICE_AQC_PHY_FEC_10G_KR_40G_KR4_EN		BIT(0)
> +#define ICE_AQC_PHY_FEC_10G_KR_40G_KR4_REQ		BIT(1)
> +#define ICE_AQC_PHY_FEC_25G_RS_528_REQ			BIT(2)
> +#define ICE_AQC_PHY_FEC_25G_KR_REQ			BIT(3)
> +#define ICE_AQC_PHY_FEC_25G_RS_544_REQ			BIT(4)
> +#define ICE_AQC_PHY_FEC_25G_RS_CLAUSE91_EN		BIT(6)
> +#define ICE_AQC_PHY_FEC_25G_KR_CLAUSE74_EN		BIT(7)
> +#define ICE_AQC_PHY_FEC_MASK
> 	MAKEMASK(0xdf, 0)
> +	u8 extended_compliance_code;
> +#define ICE_MODULE_TYPE_TOTAL_BYTE			3
> +	u8 module_type[ICE_MODULE_TYPE_TOTAL_BYTE];
> +#define ICE_AQC_MOD_TYPE_BYTE0_SFP_PLUS			0xA0
> +#define ICE_AQC_MOD_TYPE_BYTE0_QSFP_PLUS		0x80
> +#define ICE_AQC_MOD_TYPE_BYTE1_SFP_PLUS_CU_PASSIVE	BIT(0)
> +#define ICE_AQC_MOD_TYPE_BYTE1_SFP_PLUS_CU_ACTIVE	BIT(1)
> +#define ICE_AQC_MOD_TYPE_BYTE1_10G_BASE_SR		BIT(4)
> +#define ICE_AQC_MOD_TYPE_BYTE1_10G_BASE_LR		BIT(5)
> +#define ICE_AQC_MOD_TYPE_BYTE1_10G_BASE_LRM		BIT(6)
> +#define ICE_AQC_MOD_TYPE_BYTE1_10G_BASE_ER		BIT(7)
> +#define ICE_AQC_MOD_TYPE_BYTE2_SFP_PLUS			0xA0
> +#define ICE_AQC_MOD_TYPE_BYTE2_QSFP_PLUS		0x86
> +	u8 qualified_module_count;
> +#define ICE_AQC_QUAL_MOD_COUNT_MAX			16
> +	struct {
> +		u8 v_oui[3];
> +		u8 rsvd3;
> +		u8 v_part[16];
> +		__le32 v_rev;
> +		__le64 rsvd8;
> +	} qual_modules[ICE_AQC_QUAL_MOD_COUNT_MAX];
> +};
> +

Does the NIC support physical loopback? I am not able to find here.
 
> +#define ICE_AQ_PHY_ENA_LOW_POWER	BIT(2)

Does Low Power PMD is exposed to DPDK? If yes, can you mention the performance numbers or variance in Release documents?

Snipped

> +
> +/* Memory types */
> +enum ice_memset_type {
> +	ICE_NONDMA_MEM = 0,
> +	ICE_DMA_MEM
> +};
> +
> +/* Memcpy types */
> +enum ice_memcpy_type {
> +	ICE_NONDMA_TO_NONDMA = 0,
> +	ICE_NONDMA_TO_DMA,
> +	ICE_DMA_TO_DMA,
> +	ICE_DMA_TO_NONDMA
> +};
> +

Is this exposed to user (rte_eth_dev) API? If yes, can you please let know the performance impact in RX|TX in release notes too.

Snipped

Suggestion: patch 01/20 is bit too long