| Message ID | 20241204180709.307607-2-anton.nadezhdin@intel.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | ice: implement low latency PHY timer updates | expand |
On Wed, Dec 04, 2024 at 01:03:44PM -0500, Anton Nadezhdin wrote: > From: Jacob Keller <jacob.e.keller@intel.com> > > 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 <karol.kolacinski@intel.com> > Signed-off-by: Karol Kolacinski <karol.kolacinski@intel.com> > Signed-off-by: Jacob Keller <jacob.e.keller@intel.com> > Reviewed-by: Milena Olech <milena.olech@intel.com> > Signed-off-by: Anton Nadezhdin <anton.nadezhdin@intel.com> > --- > 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; > } > > /** ...
> -----Original Message----- > From: Simon Horman <horms@kernel.org> > Sent: Friday, December 6, 2024 5:30 AM > To: Nadezhdin, Anton <anton.nadezhdin@intel.com> > Cc: intel-wired-lan@lists.osuosl.org; netdev@vger.kernel.org; Nguyen, Anthony L > <anthony.l.nguyen@intel.com>; Kitszel, Przemyslaw > <przemyslaw.kitszel@intel.com>; richardcochran@gmail.com; Keller, Jacob E > <jacob.e.keller@intel.com>; Kolacinski, Karol <karol.kolacinski@intel.com>; Olech, > Milena <milena.olech@intel.com> > Subject: Re: [PATCH iwl-next 1/5] ice: use rd32_poll_timeout_atomic in > ice_read_phy_tstamp_ll_e810 > > On Wed, Dec 04, 2024 at 01:03:44PM -0500, Anton Nadezhdin wrote: > > From: Jacob Keller <jacob.e.keller@intel.com> > > > > 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 <karol.kolacinski@intel.com> > > Signed-off-by: Karol Kolacinski <karol.kolacinski@intel.com> > > Signed-off-by: Jacob Keller <jacob.e.keller@intel.com> > > Reviewed-by: Milena Olech <milena.olech@intel.com> > > Signed-off-by: Anton Nadezhdin <anton.nadezhdin@intel.com> > > --- > > 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 > Yep, err should just be an integer here. Thanks, Jake > > + 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; > > } > > > > /** > > ...
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); + 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; } /** diff --git a/drivers/net/ethernet/intel/ice/ice_ptp_hw.h b/drivers/net/ethernet/intel/ice/ice_ptp_hw.h index 5c11d8a69fd3..4c059e2f4d96 100644 --- a/drivers/net/ethernet/intel/ice/ice_ptp_hw.h +++ b/drivers/net/ethernet/intel/ice/ice_ptp_hw.h @@ -694,7 +694,7 @@ static inline bool ice_is_dual(struct ice_hw *hw) #define BYTES_PER_IDX_ADDR_L 4 /* Tx timestamp low latency read definitions */ -#define TS_LL_READ_RETRIES 200 +#define TS_LL_READ_TIMEOUT 2000 #define TS_LL_READ_TS_HIGH GENMASK(23, 16) #define TS_LL_READ_TS_IDX GENMASK(29, 24) #define TS_LL_READ_TS_INTR BIT(30)