Re: [Intel-wired-lan] [PATCH iwl-next] ice: Don't disable PHY before settime

[Jacob Keller <jacob.e.keller@intel.com>]

On 10/11/2023 4:02 AM, Karol Kolacinski wrote:
> When settime is called, the driver tries to disable the PHY to avoid
> PHY clock running and giving incorrect timestamps during time change.
> PHY stop procedure takes more HW writes than just marking timestamps as
> invalid. After settime, the PHYs is recalibrated and timestamping is
> reenabled.
> Change disabling the PHY to marking timestamps as invalid.
> 
> Reviewed-by: Larysa Zaremba <larysa.zaremba@intel.com>
> Signed-off-by: Karol Kolacinski <karol.kolacinski@intel.com>
> ---
>  drivers/net/ethernet/intel/ice/ice_ptp.c    |  7 +--
>  drivers/net/ethernet/intel/ice/ice_ptp_hw.c | 49 +++++++++++++++++++++
>  drivers/net/ethernet/intel/ice/ice_ptp_hw.h |  1 +
>  3 files changed, 54 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.c b/drivers/net/ethernet/intel/ice/ice_ptp.c
> index 1eddcbe89b0c..50ee90fd77d6 100644
> --- a/drivers/net/ethernet/intel/ice/ice_ptp.c
> +++ b/drivers/net/ethernet/intel/ice/ice_ptp.c
> @@ -1796,10 +1796,11 @@ ice_ptp_settime64(struct ptp_clock_info *info, const struct timespec64 *ts)
>  	int err;
>  
>  	/* For Vernier mode, we need to recalibrate after new settime
> -	 * Start with disabling timestamp block
> +	 * Start with marking timestamps as invalid.
>  	 */
> -	if (pf->ptp.port.link_up)
> -		ice_ptp_port_phy_stop(&pf->ptp.port);
> +	err = ice_ptp_clear_phy_offset_ready(hw);
> +	if (err)
> +		dev_warn(ice_pf_to_dev(pf), "Failed to mark timestamps as invalid before settime\n");
>  
>  	if (!ice_ptp_lock(hw)) {
>  		err = -EBUSY;
> diff --git a/drivers/net/ethernet/intel/ice/ice_ptp_hw.c b/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
> index 6d573908de7a..5984df8cb942 100644
> --- a/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
> +++ b/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
> @@ -2323,6 +2323,36 @@ int ice_phy_cfg_rx_offset_e822(struct ice_hw *hw, u8 port)
>  	return 0;
>  }
>  
> +/**
> + * ice_ptp_clear_phy_offset_ready_e822 - Clear PHY TX_/RX_OFFSET_READY registers
> + * @hw: pointer to the HW struct
> + *
> + * Clear PHY TX_/RX_OFFSET_READY registers, effectively marking all transmitted
> + * and received timestamps as invalid.
> + */
> +static int ice_ptp_clear_phy_offset_ready_e822(struct ice_hw *hw)
> +{
> +	u8 port;
> +
> +	for (port = 0; port < hw->phy_ports; port++) {
> +		int err;
> +

I think this is based on some missing work since hw->phy_ports doesn't
exist. Was this meant to go after your other series to fix timestamping?
I think I saw phy_ports introduced in some patches but not sure they've
been queued up.

> +		err = ice_write_phy_reg_e822(hw, port, P_REG_TX_OR, 0);
> +		if (err) {
> +			ice_warn(hw, "Failed to clear PHY TX_OFFSET_READY register\n");
> +			return err;
> +		}

I think you want dev_warn(ice_hw_to_dev(hw) here.

> +
> +		err = ice_write_phy_reg_e822(hw, port, P_REG_RX_OR, 0);
> +		if (err) {
> +			ice_warn(hw, "Failed to clear PHY RX_OFFSET_READY register\n");
> +			return err;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
>  /**
>   * ice_read_phy_and_phc_time_e822 - Simultaneously capture PHC and PHY time
>   * @hw: pointer to the HW struct
> @@ -3505,6 +3535,25 @@ int ice_ptp_adj_clock(struct ice_hw *hw, s32 adj)
>  	return ice_ptp_tmr_cmd(hw, ICE_PTP_ADJ_TIME);
>  }
>  
> +/**
> + * ice_ptp_clear_phy_offset_ready - Clear PHY TX_/RX_OFFSET_READY registers
> + * @hw: pointer to the HW struct
> + *
> + * Clear PHY TX_/RX_OFFSET_READY registers, effectively marking all transmitted
> + * and received timestamps as invalid.
> + */
> +int ice_ptp_clear_phy_offset_ready(struct ice_hw *hw)
> +{
> +	switch (hw->phy_model) {
> +	case ICE_PHY_E810:
> +		return 0;
> +	case ICE_PHY_E822:
> +		return ice_ptp_clear_phy_offset_ready_e822(hw);
> +	default:
> +		return -EOPNOTSUPP;
> +	}
> +}
> +
>  /**
>   * ice_read_phy_tstamp - Read a PHY timestamp from the timestamo block
>   * @hw: pointer to the HW struct
> diff --git a/drivers/net/ethernet/intel/ice/ice_ptp_hw.h b/drivers/net/ethernet/intel/ice/ice_ptp_hw.h
> index 36aeeef99ec0..813ebc254135 100644
> --- a/drivers/net/ethernet/intel/ice/ice_ptp_hw.h
> +++ b/drivers/net/ethernet/intel/ice/ice_ptp_hw.h
> @@ -208,6 +208,7 @@ int ice_ptp_init_time(struct ice_hw *hw, u64 time);
>  int ice_ptp_write_incval(struct ice_hw *hw, u64 incval);
>  int ice_ptp_write_incval_locked(struct ice_hw *hw, u64 incval);
>  int ice_ptp_adj_clock(struct ice_hw *hw, s32 adj);
> +int ice_ptp_clear_phy_offset_ready(struct ice_hw *hw);
>  int ice_read_phy_tstamp(struct ice_hw *hw, u8 block, u8 idx, u64 *tstamp);
>  int ice_clear_phy_tstamp(struct ice_hw *hw, u8 block, u8 idx);
>  void ice_ptp_reset_ts_memory(struct ice_hw *hw);
> 
> base-commit: 2318d58f358e7aef726c038aff87a68bec8f09e0
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan