On Wed, Dec 04, 2024 at 01:03:44PM -0500, Anton Nadezhdin wrote:
> From: Jacob Keller <[email protected]>
>
> The ice_read_phy_tstamp_ll_e810 function repeatedly reads the PF_SB_ATQBAL
> register until the TS_LL_READ_TS bit is cleared. This is a perfect
> candidate for using rd32_poll_timeout. However, the default implementation
> uses a sleep-based wait.
>
> Add a new rd32_poll_timeout_atomic macro which is based on the non-sleeping
> read_poll_timeout_atomic implementation. Use this to replace the loop
> reading in the ice_read_phy_tstamp_ll_e810 function.
>
> This will also be used in the future when low latency PHY timer updates are
> supported.
>
> Co-developed-by: Karol Kolacinski <[email protected]>
> Signed-off-by: Karol Kolacinski <[email protected]>
> Signed-off-by: Jacob Keller <[email protected]>
> Reviewed-by: Milena Olech <[email protected]>
> Signed-off-by: Anton Nadezhdin <[email protected]>
> ---
> drivers/net/ethernet/intel/ice/ice_osdep.h | 3 +++
> drivers/net/ethernet/intel/ice/ice_ptp_hw.c | 30 +++++++++------------
> drivers/net/ethernet/intel/ice/ice_ptp_hw.h | 2 +-
> 3 files changed, 17 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/ice/ice_osdep.h
> b/drivers/net/ethernet/intel/ice/ice_osdep.h
> index b9f383494b3f..9bb343de80a9 100644
> --- a/drivers/net/ethernet/intel/ice/ice_osdep.h
> +++ b/drivers/net/ethernet/intel/ice/ice_osdep.h
> @@ -26,6 +26,9 @@
>
> #define rd32_poll_timeout(a, addr, val, cond, delay_us, timeout_us) \
> read_poll_timeout(rd32, val, cond, delay_us, timeout_us, false, a, addr)
> +#define rd32_poll_timeout_atomic(a, addr, val, cond, delay_us, timeout_us) \
> + read_poll_timeout_atomic(rd32, val, cond, delay_us, timeout_us, false, \
> + a, addr)
>
> #define ice_flush(a) rd32((a), GLGEN_STAT)
> #define ICE_M(m, s) ((m ## U) << (s))
> diff --git a/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
> b/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
> index e55aeab0975c..b9cf8ce9644a 100644
> --- a/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
> +++ b/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
> @@ -4868,32 +4868,28 @@ static int
> ice_read_phy_tstamp_ll_e810(struct ice_hw *hw, u8 idx, u8 *hi, u32 *lo)
> {
> u32 val;
> - u8 i;
> + u8 err;
>
> /* Write TS index to read to the PF register so the FW can read it */
> val = FIELD_PREP(TS_LL_READ_TS_IDX, idx) | TS_LL_READ_TS;
> wr32(hw, PF_SB_ATQBAL, val);
>
> /* Read the register repeatedly until the FW provides us the TS */
> - for (i = TS_LL_READ_RETRIES; i > 0; i--) {
> - val = rd32(hw, PF_SB_ATQBAL);
> -
> - /* When the bit is cleared, the TS is ready in the register */
> - if (!(FIELD_GET(TS_LL_READ_TS, val))) {
> - /* High 8 bit value of the TS is on the bits 16:23 */
> - *hi = FIELD_GET(TS_LL_READ_TS_HIGH, val);
> + err = rd32_poll_timeout_atomic(hw, PF_SB_ATQBAL, val,
> + !FIELD_GET(TS_LL_READ_TS, val),
> + 10, TS_LL_READ_TIMEOUT);
Hi Jakob and Karol,
A minor nit from my side: rd32_poll_timeout_atomic may return a negative
error value but err is unsigned. This doesn't seem ideal.
Flagged by Smatch
> + if (err) {
> + ice_debug(hw, ICE_DBG_PTP, "Failed to read PTP timestamp using
> low latency read\n");
> + return err;
> + }
>
> - /* Read the low 32 bit value and set the TS valid bit */
> - *lo = rd32(hw, PF_SB_ATQBAH) | TS_VALID;
> - return 0;
> - }
> + /* High 8 bit value of the TS is on the bits 16:23 */
> + *hi = FIELD_GET(TS_LL_READ_TS_HIGH, val);
>
> - udelay(10);
> - }
> + /* Read the low 32 bit value and set the TS valid bit */
> + *lo = rd32(hw, PF_SB_ATQBAH) | TS_VALID;
>
> - /* FW failed to provide the TS in time */
> - ice_debug(hw, ICE_DBG_PTP, "Failed to read PTP timestamp using low
> latency read\n");
> - return -EINVAL;
> + return 0;
> }
>
> /**
...