Hi Vipin,
> -----Original Message-----
> From: Varghese, Vipin
> Sent: Tuesday, December 4, 2018 12:19 PM
> To: Lu, Wenzhuo <wenzhuo...@intel.com>; dev@dpdk.org
> Cc: Lu, Wenzhuo <wenzhuo...@intel.com>
> Subject: RE: [dpdk-dev] [PATCH v2 01/20] net/ice: add base code
>
> 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'?
Yes.
>
> 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)'?
It's 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?
We don’t want to change the base code for the sake of maintenance.
>
> #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?
The same as above.
>
> #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.
Not sure about it. But no plan for this at this stage.
>
> > +#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?
No plan for it at this release.
>
> 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.
No plan for it at this release.
>
> Snipped
>
> Suggestion: patch 01/20 is bit too long
Discussing in another thread.
