RE: [PATCH v2] net/ice: fix incorrect Rx timestamp

["Wu, Wenjun1" <wenjun1.wu@intel.com>]

> -----Original Message-----
> From: Su, Simei <simei.su@intel.com>
> Sent: Wednesday, March 8, 2023 12:37 PM
> To: Zhang, Qi Z <qi.z.zhang@intel.com>; Yang, Qiming
> <qiming.yang@intel.com>
> Cc: dev@dpdk.org; Wu, Wenjun1 <wenjun1.wu@intel.com>; Su, Simei
> <simei.su@intel.com>; stable@dpdk.org
> Subject: [PATCH v2] net/ice: fix incorrect Rx timestamp
> 
> For E822, the time value in Rx Flex Descriptors is 0 due to the missing PHY
> clock timer setup. Also, the source clock index in use is based on device
> capabilities instead of always being zero.
> 
> Fixes: 953e74e6b73a ("net/ice: enable Rx timestamp on flex descriptor")
> Fixes: 646dcbe6c701 ("net/ice: support IEEE 1588 PTP")
> Fixes: fb800fde66f4 ("net/ice/base: work around missing PTP capabilities")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Simei Su <simei.su@intel.com>
> ---
> v2:
> * Refine commit title and commit log.
> * Remove duplicate code.
> * Rework share code for "SIMICS_SUPPORT".
> 
>  drivers/net/ice/base/ice_common.c |  4 +---
>  drivers/net/ice/ice_ethdev.c      | 36 ++++++++++++++++++++++--------------
>  drivers/net/ice/ice_rxtx.h        | 11 ++++++-----
>  3 files changed, 29 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/net/ice/base/ice_common.c
> b/drivers/net/ice/base/ice_common.c
> index 5391bd6..1a02aad 100644
> --- a/drivers/net/ice/base/ice_common.c
> +++ b/drivers/net/ice/base/ice_common.c
> @@ -2554,9 +2554,7 @@ ice_parse_1588_func_caps(struct ice_hw *hw,
> struct ice_hw_func_caps *func_p,
>  			 struct ice_aqc_list_caps_elem *cap)  {
>  	struct ice_ts_func_info *info = &func_p->ts_func_info;
> -	u32 number = ICE_TS_FUNC_ENA_M | ICE_TS_SRC_TMR_OWND_M |
> -		     ICE_TS_TMR_ENA_M | ICE_TS_TMR_IDX_OWND_M |
> -		     ICE_TS_TMR_IDX_ASSOC_M;
> +	u32 number = LE32_TO_CPU(cap->number);
>  	u8 clk_freq;
> 
>  	ice_debug(hw, ICE_DBG_INIT, "1588 func caps: raw value %x\n",
> number); diff --git a/drivers/net/ice/ice_ethdev.c
> b/drivers/net/ice/ice_ethdev.c index 0d011bb..9a88cf9 100644
> --- a/drivers/net/ice/ice_ethdev.c
> +++ b/drivers/net/ice/ice_ethdev.c
> @@ -2413,6 +2413,17 @@ ice_dev_init(struct rte_eth_dev *dev)
>  	/* Initialize TM configuration */
>  	ice_tm_conf_init(dev);
> 
> +	if (ice_is_e810(hw))
> +		hw->phy_cfg = ICE_PHY_E810;
> +	else
> +		hw->phy_cfg = ICE_PHY_E822;
> +
> +	if (hw->phy_cfg == ICE_PHY_E822) {
> +		ret = ice_start_phy_timer_e822(hw, hw->pf_id, true);
> +		if (ret)
> +			PMD_INIT_LOG(ERR, "Failed to start phy timer\n");
> +	}
> +
>  	if (!ad->is_safe_mode) {
>  		ret = ice_flow_init(ad);
>  		if (ret) {
> @@ -5814,11 +5825,6 @@ ice_timesync_enable(struct rte_eth_dev *dev)
>  		return -1;
>  	}
> 
> -	if (ice_is_e810(hw))
> -		hw->phy_cfg = ICE_PHY_E810;
> -	else
> -		hw->phy_cfg = ICE_PHY_E822;
> -
>  	if (hw->func_caps.ts_func_info.src_tmr_owned) {
>  		ret = ice_ptp_init_phc(hw);
>  		if (ret) {
> @@ -5939,16 +5945,17 @@ ice_timesync_read_time(struct rte_eth_dev
> *dev, struct timespec *ts)
>  	struct ice_hw *hw = ICE_DEV_PRIVATE_TO_HW(dev->data-
> >dev_private);
>  	struct ice_adapter *ad =
>  			ICE_DEV_PRIVATE_TO_ADAPTER(dev->data-
> >dev_private);
> +	uint8_t tmr_idx = hw->func_caps.ts_func_info.tmr_index_assoc;
>  	uint32_t hi, lo, lo2;
>  	uint64_t time, ns;
> 
> -	lo = ICE_READ_REG(hw, GLTSYN_TIME_L(0));
> -	hi = ICE_READ_REG(hw, GLTSYN_TIME_H(0));
> -	lo2 = ICE_READ_REG(hw, GLTSYN_TIME_L(0));
> +	lo = ICE_READ_REG(hw, GLTSYN_TIME_L(tmr_idx));
> +	hi = ICE_READ_REG(hw, GLTSYN_TIME_H(tmr_idx));
> +	lo2 = ICE_READ_REG(hw, GLTSYN_TIME_L(tmr_idx));
> 
>  	if (lo2 < lo) {
> -		lo = ICE_READ_REG(hw, GLTSYN_TIME_L(0));
> -		hi = ICE_READ_REG(hw, GLTSYN_TIME_H(0));
> +		lo = ICE_READ_REG(hw, GLTSYN_TIME_L(tmr_idx));
> +		hi = ICE_READ_REG(hw, GLTSYN_TIME_H(tmr_idx));
>  	}
> 
>  	time = ((uint64_t)hi << 32) | lo;
> @@ -5964,6 +5971,7 @@ ice_timesync_disable(struct rte_eth_dev *dev)
>  	struct ice_hw *hw = ICE_DEV_PRIVATE_TO_HW(dev->data-
> >dev_private);
>  	struct ice_adapter *ad =
>  			ICE_DEV_PRIVATE_TO_ADAPTER(dev->data-
> >dev_private);
> +	uint8_t tmr_idx = hw->func_caps.ts_func_info.tmr_index_assoc;
>  	uint64_t val;
>  	uint8_t lport;
> 
> @@ -5971,12 +5979,12 @@ ice_timesync_disable(struct rte_eth_dev *dev)
> 
>  	ice_clear_phy_tstamp(hw, lport, 0);
> 
> -	val = ICE_READ_REG(hw, GLTSYN_ENA(0));
> +	val = ICE_READ_REG(hw, GLTSYN_ENA(tmr_idx));
>  	val &= ~GLTSYN_ENA_TSYN_ENA_M;
> -	ICE_WRITE_REG(hw, GLTSYN_ENA(0), val);
> +	ICE_WRITE_REG(hw, GLTSYN_ENA(tmr_idx), val);
> 
> -	ICE_WRITE_REG(hw, GLTSYN_INCVAL_L(0), 0);
> -	ICE_WRITE_REG(hw, GLTSYN_INCVAL_H(0), 0);
> +	ICE_WRITE_REG(hw, GLTSYN_INCVAL_L(tmr_idx), 0);
> +	ICE_WRITE_REG(hw, GLTSYN_INCVAL_H(tmr_idx), 0);
> 
>  	ad->ptp_ena = 0;
> 
> diff --git a/drivers/net/ice/ice_rxtx.h b/drivers/net/ice/ice_rxtx.h index
> 4947d5c..94f6bcf 100644
> --- a/drivers/net/ice/ice_rxtx.h
> +++ b/drivers/net/ice/ice_rxtx.h
> @@ -349,26 +349,27 @@ static inline
>  uint64_t ice_tstamp_convert_32b_64b(struct ice_hw *hw, struct ice_adapter
> *ad,
>  				    uint32_t flag, uint32_t in_timestamp)  {
> +	uint8_t tmr_idx = hw->func_caps.ts_func_info.tmr_index_assoc;
>  	const uint64_t mask = 0xFFFFFFFF;
>  	uint32_t hi, lo, lo2, delta;
>  	uint64_t ns;
> 
>  	if (flag) {
> -		lo = ICE_READ_REG(hw, GLTSYN_TIME_L(0));
> -		hi = ICE_READ_REG(hw, GLTSYN_TIME_H(0));
> +		lo = ICE_READ_REG(hw, GLTSYN_TIME_L(tmr_idx));
> +		hi = ICE_READ_REG(hw, GLTSYN_TIME_H(tmr_idx));
> 
>  		/*
>  		 * On typical system, the delta between lo and lo2 is ~1000ns,
>  		 * so 10000 seems a large-enough but not overly-big guard
> band.
>  		 */
>  		if (lo > (UINT32_MAX -
> ICE_TIMESYNC_REG_WRAP_GUARD_BAND))
> -			lo2 = ICE_READ_REG(hw, GLTSYN_TIME_L(0));
> +			lo2 = ICE_READ_REG(hw, GLTSYN_TIME_L(tmr_idx));
>  		else
>  			lo2 = lo;
> 
>  		if (lo2 < lo) {
> -			lo = ICE_READ_REG(hw, GLTSYN_TIME_L(0));
> -			hi = ICE_READ_REG(hw, GLTSYN_TIME_H(0));
> +			lo = ICE_READ_REG(hw, GLTSYN_TIME_L(tmr_idx));
> +			hi = ICE_READ_REG(hw, GLTSYN_TIME_H(tmr_idx));
>  		}
> 
>  		ad->time_hw = ((uint64_t)hi << 32) | lo;
> --
> 2.9.5

Acked-by: Wenjun Wu <wenjun1.wu@intel.com>

Regards,
Wenjun