| Message ID | 20230822124044.301654-5-karol.kolacinski@intel.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | ice: fix timestamping in reset process | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Guessing tree name failed - patch did not apply |
On 8/22/2023 5:40 AM, Karol Kolacinski wrote: > From: Jacob Keller <jacob.e.keller@intel.com> > > The tx->verify_cached flag is used to inform the Tx timestamp tracking > code whether it needs to verify the cached Tx timestamp value against > a previous captured value. This is necessary on E810 hardware which does > not have a Tx timestamp ready bitmap. > > In addition, we currently rely on the fact that the > ice_get_phy_tx_tstamp_ready() function returns all 1s for E810 hardware. > Instead of introducing a brand new flag, rename and verify_cached to > has_ready_bitmap, inverting the relevant checks. > > The ice_ptp_tx_cfg_intr() function sends a control queue message to > configure the PHY timestamp interrupt block. This is a very similar name > to a function which is used to configure the MAC Other Interrupt Cause > Enable register. > > Rename this function to ice_ptp_cfg_phy_interrupt in order to make it > more obvious to the reader what action it performs, and distinguish it > from other similarly named functions. > > The ice_ptp_configure_tx_tstamp function writes to PFINT_OICR_ENA to > configure it with the PFINT_OICR_TX_TSYN_M bit. The name of this > function is confusing because there are multiple other functions with > almost identical names. > > Rename it to ice_ptp_cfg_tx_interrupt in order to make it more obvious > to the reader what action it performs. > > Signed-off-by: Jacob Keller <jacob.e.keller@intel.com> > Signed-off-by: Karol Kolacinski <karol.kolacinski@intel.com> > --- This left verify_cached in the structure, but nothing sets it. It should be removed. Thanks, Jake > drivers/net/ethernet/intel/ice/ice_ptp.c | 41 +++++++++++++----------- > drivers/net/ethernet/intel/ice/ice_ptp.h | 6 +++- > 2 files changed, 27 insertions(+), 20 deletions(-) > > diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.c b/drivers/net/ethernet/intel/ice/ice_ptp.c > index bd94b42e19dd..393156b9b426 100644 > --- a/drivers/net/ethernet/intel/ice/ice_ptp.c > +++ b/drivers/net/ethernet/intel/ice/ice_ptp.c > @@ -281,11 +281,11 @@ static const char *ice_ptp_state_str(enum ice_ptp_state state) > } > > /** > - * ice_ptp_configure_tx_tstamp - Enable or disable Tx timestamp interrupt > - * @pf: The PF pointer to search in > + * ice_ptp_cfg_tx_interrupt - Configure Tx timestamp interrupt for the device > + * @pf: Board private structure > * @on: bool value for whether timestamp interrupt is enabled or disabled > */ > -static void ice_ptp_configure_tx_tstamp(struct ice_pf *pf, bool on) > +static void ice_ptp_cfg_tx_interrupt(struct ice_pf *pf, bool on) > { > u32 val; > > @@ -320,7 +320,7 @@ static void ice_set_tx_tstamp(struct ice_pf *pf, bool on) > } > > if (pf->ptp.tx_interrupt_mode == ICE_PTP_TX_INTERRUPT_SELF) > - ice_ptp_configure_tx_tstamp(pf, on); > + ice_ptp_cfg_tx_interrupt(pf, on); > > pf->ptp.tstamp_config.tx_type = on ? HWTSTAMP_TX_ON : HWTSTAMP_TX_OFF; > } > @@ -591,9 +591,11 @@ static void ice_ptp_process_tx_tstamp(struct ice_ptp_tx *tx) > hw = &pf->hw; > > /* Read the Tx ready status first */ > - err = ice_get_phy_tx_tstamp_ready(hw, tx->block, &tstamp_ready); > - if (err) > - return; > + if (tx->has_ready_bitmap) { > + err = ice_get_phy_tx_tstamp_ready(hw, tx->block, &tstamp_ready); > + if (err) > + return; > + } > > /* Drop packets if the link went down */ > link_up = ptp_port->link_up; > @@ -621,7 +623,8 @@ static void ice_ptp_process_tx_tstamp(struct ice_ptp_tx *tx) > * If we do not, the hardware logic for generating a new > * interrupt can get stuck on some devices. > */ > - if (!(tstamp_ready & BIT_ULL(phy_idx))) { > + if (tx->has_ready_bitmap && > + !(tstamp_ready & BIT_ULL(phy_idx))) { > if (drop_ts) > goto skip_ts_read; > > @@ -641,7 +644,7 @@ static void ice_ptp_process_tx_tstamp(struct ice_ptp_tx *tx) > * from the last cached timestamp. If it is not, skip this for > * now assuming it hasn't yet been captured by hardware. > */ > - if (!drop_ts && tx->verify_cached && > + if (!drop_ts && !tx->has_ready_bitmap && > raw_tstamp == tx->tstamps[idx].cached_tstamp) > continue; > > @@ -651,7 +654,7 @@ static void ice_ptp_process_tx_tstamp(struct ice_ptp_tx *tx) > > skip_ts_read: > spin_lock(&tx->lock); > - if (tx->verify_cached && raw_tstamp) > + if (!tx->has_ready_bitmap && raw_tstamp) > tx->tstamps[idx].cached_tstamp = raw_tstamp; > clear_bit(idx, tx->in_use); > skb = tx->tstamps[idx].skb; > @@ -895,7 +898,7 @@ ice_ptp_init_tx_e822(struct ice_pf *pf, struct ice_ptp_tx *tx, u8 port) > tx->block = port / ICE_PORTS_PER_QUAD; > tx->offset = (port % ICE_PORTS_PER_QUAD) * INDEX_PER_PORT_E822; > tx->len = INDEX_PER_PORT_E822; > - tx->verify_cached = 0; > + tx->has_ready_bitmap = 1; > > return ice_ptp_alloc_tx_tracker(tx); > } > @@ -918,7 +921,7 @@ ice_ptp_init_tx_e810(struct ice_pf *pf, struct ice_ptp_tx *tx) > * verify new timestamps against cached copy of the last read > * timestamp. > */ > - tx->verify_cached = 1; > + tx->has_ready_bitmap = 0; > > return ice_ptp_alloc_tx_tracker(tx); > } > @@ -1338,14 +1341,14 @@ void ice_ptp_link_change(struct ice_pf *pf, u8 port, bool linkup) > } > > /** > - * ice_ptp_tx_ena_intr - Enable or disable the Tx timestamp interrupt > + * ice_ptp_cfg_phy_interrupt - Configure PHY interrupt settings > * @pf: PF private structure > * @ena: bool value to enable or disable interrupt > * @threshold: Minimum number of packets at which intr is triggered > * > * Utility function to enable or disable Tx timestamp interrupt and threshold > */ > -static int ice_ptp_tx_ena_intr(struct ice_pf *pf, bool ena, u32 threshold) > +static int ice_ptp_cfg_phy_interrupt(struct ice_pf *pf, bool ena, u32 threshold) > { > struct ice_hw *hw = &pf->hw; > int err = 0; > @@ -2507,8 +2510,8 @@ void ice_ptp_reset(struct ice_pf *pf, enum ice_reset_req reset_type) > struct ice_ptp *ptp = &pf->ptp; > struct ice_hw *hw = &pf->hw; > struct timespec64 ts; > - int err, itr = 1; > u64 time_diff; > + int err; > > if (ptp->state != ICE_PTP_RESETTING) { > if (ptp->state == ICE_PTP_READY) { > @@ -2561,7 +2564,7 @@ void ice_ptp_reset(struct ice_pf *pf, enum ice_reset_req reset_type) > > if (!ice_is_e810(hw)) { > /* Enable quad interrupts */ > - err = ice_ptp_tx_ena_intr(pf, true, itr); > + err = ice_ptp_cfg_phy_interrupt(pf, true, 1); > if (err) > goto err; > } > @@ -2847,13 +2850,13 @@ static int ice_ptp_init_owner(struct ice_pf *pf) > /* The clock owner for this device type handles the timestamp > * interrupt for all ports. > */ > - ice_ptp_configure_tx_tstamp(pf, true); > + ice_ptp_cfg_tx_interrupt(pf, true); > > /* React on all quads interrupts for E82x */ > wr32(hw, PFINT_TSYN_MSK + (0x4 * hw->pf_id), (u32)0x1f); > > /* Enable quad interrupts */ > - err = ice_ptp_tx_ena_intr(pf, true, itr); > + err = ice_ptp_cfg_phy_interrupt(pf, true, itr); > if (err) > goto err_exit; > } > @@ -2925,7 +2928,7 @@ static int ice_ptp_init_port(struct ice_pf *pf, struct ice_ptp_port *ptp_port) > * neither on own quad nor on others > */ > if (!ice_ptp_pf_handles_tx_interrupt(pf)) { > - ice_ptp_configure_tx_tstamp(pf, false); > + ice_ptp_cfg_tx_interrupt(pf, false); > wr32(hw, PFINT_TSYN_MSK + (0x4 * hw->pf_id), (u32)0x0); > } > kthread_init_delayed_work(&ptp_port->ov_work, > diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.h b/drivers/net/ethernet/intel/ice/ice_ptp.h > index 48c0d56c0568..30ad714a2a21 100644 > --- a/drivers/net/ethernet/intel/ice/ice_ptp.h > +++ b/drivers/net/ethernet/intel/ice/ice_ptp.h > @@ -100,7 +100,7 @@ struct ice_perout_channel { > * the last timestamp we read for a given index. If the current timestamp > * value is the same as the cached value, we assume a new timestamp hasn't > * been captured. This avoids reporting stale timestamps to the stack. This is > - * only done if the verify_cached flag is set in ice_ptp_tx structure. > + * only done if the has_ready_bitmap flag is not set in ice_ptp_tx structure. > */ > struct ice_tx_tstamp { > struct sk_buff *skb; > @@ -131,6 +131,9 @@ enum ice_tx_tstamp_work { > * @calibrating: if true, the PHY is calibrating the Tx offset. During this > * window, timestamps are temporarily disabled. > * @verify_cached: if true, verify new timestamp differs from last read value > + * @has_ready_bitmap: if true, the hardware has a valid Tx timestamp ready > + * bitmap register. If false, fall back to verifying new > + * timestamp values against previously cached copy. > */ > struct ice_ptp_tx { > spinlock_t lock; /* lock protecting in_use bitmap */ > @@ -143,6 +146,7 @@ struct ice_ptp_tx { > u8 init : 1; > u8 calibrating : 1; > u8 verify_cached : 1; > + u8 has_ready_bitmap : 1; > }; > > /* Quad and port information for initializing timestamp blocks */
On 8/22/2023 5:40 AM, Karol Kolacinski wrote: > From: Jacob Keller <jacob.e.keller@intel.com> > > The tx->verify_cached flag is used to inform the Tx timestamp tracking > code whether it needs to verify the cached Tx timestamp value against > a previous captured value. This is necessary on E810 hardware which does > not have a Tx timestamp ready bitmap. > > In addition, we currently rely on the fact that the > ice_get_phy_tx_tstamp_ready() function returns all 1s for E810 hardware. > Instead of introducing a brand new flag, rename and verify_cached to > has_ready_bitmap, inverting the relevant checks. > > The ice_ptp_tx_cfg_intr() function sends a control queue message to > configure the PHY timestamp interrupt block. This is a very similar name > to a function which is used to configure the MAC Other Interrupt Cause > Enable register. > > Rename this function to ice_ptp_cfg_phy_interrupt in order to make it > more obvious to the reader what action it performs, and distinguish it > from other similarly named functions. > > The ice_ptp_configure_tx_tstamp function writes to PFINT_OICR_ENA to > configure it with the PFINT_OICR_TX_TSYN_M bit. The name of this > function is confusing because there are multiple other functions with > almost identical names. > > Rename it to ice_ptp_cfg_tx_interrupt in order to make it more obvious > to the reader what action it performs. > > Signed-off-by: Jacob Keller <jacob.e.keller@intel.com> > Signed-off-by: Karol Kolacinski <karol.kolacinski@intel.com> > --- > drivers/net/ethernet/intel/ice/ice_ptp.c | 41 +++++++++++++----------- > drivers/net/ethernet/intel/ice/ice_ptp.h | 6 +++- > 2 files changed, 27 insertions(+), 20 deletions(-) > > diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.c b/drivers/net/ethernet/intel/ice/ice_ptp.c > index bd94b42e19dd..393156b9b426 100644 > --- a/drivers/net/ethernet/intel/ice/ice_ptp.c > +++ b/drivers/net/ethernet/intel/ice/ice_ptp.c > @@ -281,11 +281,11 @@ static const char *ice_ptp_state_str(enum ice_ptp_state state) > } > > /** > - * ice_ptp_configure_tx_tstamp - Enable or disable Tx timestamp interrupt > - * @pf: The PF pointer to search in > + * ice_ptp_cfg_tx_interrupt - Configure Tx timestamp interrupt for the device > + * @pf: Board private structure > * @on: bool value for whether timestamp interrupt is enabled or disabled > */ > -static void ice_ptp_configure_tx_tstamp(struct ice_pf *pf, bool on) > +static void ice_ptp_cfg_tx_interrupt(struct ice_pf *pf, bool on) > { > u32 val; > > @@ -320,7 +320,7 @@ static void ice_set_tx_tstamp(struct ice_pf *pf, bool on) > } > > if (pf->ptp.tx_interrupt_mode == ICE_PTP_TX_INTERRUPT_SELF) > - ice_ptp_configure_tx_tstamp(pf, on); > + ice_ptp_cfg_tx_interrupt(pf, on); > > pf->ptp.tstamp_config.tx_type = on ? HWTSTAMP_TX_ON : HWTSTAMP_TX_OFF; > } > @@ -591,9 +591,11 @@ static void ice_ptp_process_tx_tstamp(struct ice_ptp_tx *tx) > hw = &pf->hw; > > /* Read the Tx ready status first */ > - err = ice_get_phy_tx_tstamp_ready(hw, tx->block, &tstamp_ready); > - if (err) > - return; > + if (tx->has_ready_bitmap) { > + err = ice_get_phy_tx_tstamp_ready(hw, tx->block, &tstamp_ready); > + if (err) > + return; > + } > > /* Drop packets if the link went down */ > link_up = ptp_port->link_up; > @@ -621,7 +623,8 @@ static void ice_ptp_process_tx_tstamp(struct ice_ptp_tx *tx) > * If we do not, the hardware logic for generating a new > * interrupt can get stuck on some devices. > */ > - if (!(tstamp_ready & BIT_ULL(phy_idx))) { > + if (tx->has_ready_bitmap && > + !(tstamp_ready & BIT_ULL(phy_idx))) { > if (drop_ts) > goto skip_ts_read; > > @@ -641,7 +644,7 @@ static void ice_ptp_process_tx_tstamp(struct ice_ptp_tx *tx) > * from the last cached timestamp. If it is not, skip this for > * now assuming it hasn't yet been captured by hardware. > */ > - if (!drop_ts && tx->verify_cached && > + if (!drop_ts && !tx->has_ready_bitmap && > raw_tstamp == tx->tstamps[idx].cached_tstamp) > continue; > > @@ -651,7 +654,7 @@ static void ice_ptp_process_tx_tstamp(struct ice_ptp_tx *tx) > > skip_ts_read: > spin_lock(&tx->lock); > - if (tx->verify_cached && raw_tstamp) > + if (!tx->has_ready_bitmap && raw_tstamp) > tx->tstamps[idx].cached_tstamp = raw_tstamp; > clear_bit(idx, tx->in_use); > skb = tx->tstamps[idx].skb; > @@ -895,7 +898,7 @@ ice_ptp_init_tx_e822(struct ice_pf *pf, struct ice_ptp_tx *tx, u8 port) > tx->block = port / ICE_PORTS_PER_QUAD; > tx->offset = (port % ICE_PORTS_PER_QUAD) * INDEX_PER_PORT_E822; > tx->len = INDEX_PER_PORT_E822; > - tx->verify_cached = 0; > + tx->has_ready_bitmap = 1; > > return ice_ptp_alloc_tx_tracker(tx); > } > @@ -918,7 +921,7 @@ ice_ptp_init_tx_e810(struct ice_pf *pf, struct ice_ptp_tx *tx) > * verify new timestamps against cached copy of the last read > * timestamp. > */ > - tx->verify_cached = 1; > + tx->has_ready_bitmap = 0; > > return ice_ptp_alloc_tx_tracker(tx); > } > @@ -1338,14 +1341,14 @@ void ice_ptp_link_change(struct ice_pf *pf, u8 port, bool linkup) > } > > /** > - * ice_ptp_tx_ena_intr - Enable or disable the Tx timestamp interrupt > + * ice_ptp_cfg_phy_interrupt - Configure PHY interrupt settings This should be in a separate patch, it looks like it got accidentally squashed into this when rebasing. > * @pf: PF private structure > * @ena: bool value to enable or disable interrupt > * @threshold: Minimum number of packets at which intr is triggered > * > * Utility function to enable or disable Tx timestamp interrupt and threshold > */ > -static int ice_ptp_tx_ena_intr(struct ice_pf *pf, bool ena, u32 threshold) > +static int ice_ptp_cfg_phy_interrupt(struct ice_pf *pf, bool ena, u32 threshold) > { > struct ice_hw *hw = &pf->hw; > int err = 0; > @@ -2507,8 +2510,8 @@ void ice_ptp_reset(struct ice_pf *pf, enum ice_reset_req reset_type) > struct ice_ptp *ptp = &pf->ptp; > struct ice_hw *hw = &pf->hw; > struct timespec64 ts; > - int err, itr = 1; > u64 time_diff; > + int err; > > if (ptp->state != ICE_PTP_RESETTING) { > if (ptp->state == ICE_PTP_READY) { > @@ -2561,7 +2564,7 @@ void ice_ptp_reset(struct ice_pf *pf, enum ice_reset_req reset_type) > > if (!ice_is_e810(hw)) { > /* Enable quad interrupts */ > - err = ice_ptp_tx_ena_intr(pf, true, itr); > + err = ice_ptp_cfg_phy_interrupt(pf, true, 1); > if (err) > goto err; > } > @@ -2847,13 +2850,13 @@ static int ice_ptp_init_owner(struct ice_pf *pf) > /* The clock owner for this device type handles the timestamp > * interrupt for all ports. > */ > - ice_ptp_configure_tx_tstamp(pf, true); > + ice_ptp_cfg_tx_interrupt(pf, true); This looks like a change that got accidentally squashed into this patch. > > /* React on all quads interrupts for E82x */ > wr32(hw, PFINT_TSYN_MSK + (0x4 * hw->pf_id), (u32)0x1f); > > /* Enable quad interrupts */ > - err = ice_ptp_tx_ena_intr(pf, true, itr); > + err = ice_ptp_cfg_phy_interrupt(pf, true, itr); > if (err) > goto err_exit; > } > @@ -2925,7 +2928,7 @@ static int ice_ptp_init_port(struct ice_pf *pf, struct ice_ptp_port *ptp_port) > * neither on own quad nor on others > */ > if (!ice_ptp_pf_handles_tx_interrupt(pf)) { > - ice_ptp_configure_tx_tstamp(pf, false); > + ice_ptp_cfg_tx_interrupt(pf, false);> wr32(hw, PFINT_TSYN_MSK + (0x4 * hw->pf_id), (u32)0x0); > } > kthread_init_delayed_work(&ptp_port->ov_work, > diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.h b/drivers/net/ethernet/intel/ice/ice_ptp.h > index 48c0d56c0568..30ad714a2a21 100644 > --- a/drivers/net/ethernet/intel/ice/ice_ptp.h > +++ b/drivers/net/ethernet/intel/ice/ice_ptp.h > @@ -100,7 +100,7 @@ struct ice_perout_channel { > * the last timestamp we read for a given index. If the current timestamp > * value is the same as the cached value, we assume a new timestamp hasn't > * been captured. This avoids reporting stale timestamps to the stack. This is > - * only done if the verify_cached flag is set in ice_ptp_tx structure. > + * only done if the has_ready_bitmap flag is not set in ice_ptp_tx structure. > */ > struct ice_tx_tstamp { > struct sk_buff *skb; > @@ -131,6 +131,9 @@ enum ice_tx_tstamp_work { > * @calibrating: if true, the PHY is calibrating the Tx offset. During this > * window, timestamps are temporarily disabled. > * @verify_cached: if true, verify new timestamp differs from last read value We're using has_ready_bitmap instead, but we still kept verify_cached in the structure. Oops. > + * @has_ready_bitmap: if true, the hardware has a valid Tx timestamp ready > + * bitmap register. If false, fall back to verifying new > + * timestamp values against previously cached copy. > */ > struct ice_ptp_tx { > spinlock_t lock; /* lock protecting in_use bitmap */ > @@ -143,6 +146,7 @@ struct ice_ptp_tx { > u8 init : 1; > u8 calibrating : 1; > u8 verify_cached : 1; > + u8 has_ready_bitmap : 1; > }; > > /* Quad and port information for initializing timestamp blocks */
On 8/22/2023 5:40 AM, Karol Kolacinski wrote: > From: Jacob Keller <jacob.e.keller@intel.com> > > The tx->verify_cached flag is used to inform the Tx timestamp tracking > code whether it needs to verify the cached Tx timestamp value against > a previous captured value. This is necessary on E810 hardware which does > not have a Tx timestamp ready bitmap. > > In addition, we currently rely on the fact that the > ice_get_phy_tx_tstamp_ready() function returns all 1s for E810 hardware. > Instead of introducing a brand new flag, rename and verify_cached to > has_ready_bitmap, inverting the relevant checks. > > The ice_ptp_tx_cfg_intr() function sends a control queue message to > configure the PHY timestamp interrupt block. This is a very similar name > to a function which is used to configure the MAC Other Interrupt Cause > Enable register. > > Rename this function to ice_ptp_cfg_phy_interrupt in order to make it > more obvious to the reader what action it performs, and distinguish it > from other similarly named functions. > > The ice_ptp_configure_tx_tstamp function writes to PFINT_OICR_ENA to > configure it with the PFINT_OICR_TX_TSYN_M bit. The name of this > function is confusing because there are multiple other functions with > almost identical names. > > Rename it to ice_ptp_cfg_tx_interrupt in order to make it more obvious > to the reader what action it performs. > Oh. Technically this commit does describe all the changes it makes.. but I think it would be better to separate these for review. They were separate originally I think... Thanks, Jake > Signed-off-by: Jacob Keller <jacob.e.keller@intel.com> > Signed-off-by: Karol Kolacinski <karol.kolacinski@intel.com> > --- > drivers/net/ethernet/intel/ice/ice_ptp.c | 41 +++++++++++++----------- > drivers/net/ethernet/intel/ice/ice_ptp.h | 6 +++- > 2 files changed, 27 insertions(+), 20 deletions(-) > > diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.c b/drivers/net/ethernet/intel/ice/ice_ptp.c > index bd94b42e19dd..393156b9b426 100644 > --- a/drivers/net/ethernet/intel/ice/ice_ptp.c > +++ b/drivers/net/ethernet/intel/ice/ice_ptp.c > @@ -281,11 +281,11 @@ static const char *ice_ptp_state_str(enum ice_ptp_state state) > } > > /** > - * ice_ptp_configure_tx_tstamp - Enable or disable Tx timestamp interrupt > - * @pf: The PF pointer to search in > + * ice_ptp_cfg_tx_interrupt - Configure Tx timestamp interrupt for the device > + * @pf: Board private structure > * @on: bool value for whether timestamp interrupt is enabled or disabled > */ > -static void ice_ptp_configure_tx_tstamp(struct ice_pf *pf, bool on) > +static void ice_ptp_cfg_tx_interrupt(struct ice_pf *pf, bool on) > { > u32 val; > > @@ -320,7 +320,7 @@ static void ice_set_tx_tstamp(struct ice_pf *pf, bool on) > } > > if (pf->ptp.tx_interrupt_mode == ICE_PTP_TX_INTERRUPT_SELF) > - ice_ptp_configure_tx_tstamp(pf, on); > + ice_ptp_cfg_tx_interrupt(pf, on); > > pf->ptp.tstamp_config.tx_type = on ? HWTSTAMP_TX_ON : HWTSTAMP_TX_OFF; > } > @@ -591,9 +591,11 @@ static void ice_ptp_process_tx_tstamp(struct ice_ptp_tx *tx) > hw = &pf->hw; > > /* Read the Tx ready status first */ > - err = ice_get_phy_tx_tstamp_ready(hw, tx->block, &tstamp_ready); > - if (err) > - return; > + if (tx->has_ready_bitmap) { > + err = ice_get_phy_tx_tstamp_ready(hw, tx->block, &tstamp_ready); > + if (err) > + return; > + } > > /* Drop packets if the link went down */ > link_up = ptp_port->link_up; > @@ -621,7 +623,8 @@ static void ice_ptp_process_tx_tstamp(struct ice_ptp_tx *tx) > * If we do not, the hardware logic for generating a new > * interrupt can get stuck on some devices. > */ > - if (!(tstamp_ready & BIT_ULL(phy_idx))) { > + if (tx->has_ready_bitmap && > + !(tstamp_ready & BIT_ULL(phy_idx))) { > if (drop_ts) > goto skip_ts_read; > > @@ -641,7 +644,7 @@ static void ice_ptp_process_tx_tstamp(struct ice_ptp_tx *tx) > * from the last cached timestamp. If it is not, skip this for > * now assuming it hasn't yet been captured by hardware. > */ > - if (!drop_ts && tx->verify_cached && > + if (!drop_ts && !tx->has_ready_bitmap && > raw_tstamp == tx->tstamps[idx].cached_tstamp) > continue; > > @@ -651,7 +654,7 @@ static void ice_ptp_process_tx_tstamp(struct ice_ptp_tx *tx) > > skip_ts_read: > spin_lock(&tx->lock); > - if (tx->verify_cached && raw_tstamp) > + if (!tx->has_ready_bitmap && raw_tstamp) > tx->tstamps[idx].cached_tstamp = raw_tstamp; > clear_bit(idx, tx->in_use); > skb = tx->tstamps[idx].skb; > @@ -895,7 +898,7 @@ ice_ptp_init_tx_e822(struct ice_pf *pf, struct ice_ptp_tx *tx, u8 port) > tx->block = port / ICE_PORTS_PER_QUAD; > tx->offset = (port % ICE_PORTS_PER_QUAD) * INDEX_PER_PORT_E822; > tx->len = INDEX_PER_PORT_E822; > - tx->verify_cached = 0; > + tx->has_ready_bitmap = 1; > > return ice_ptp_alloc_tx_tracker(tx); > } > @@ -918,7 +921,7 @@ ice_ptp_init_tx_e810(struct ice_pf *pf, struct ice_ptp_tx *tx) > * verify new timestamps against cached copy of the last read > * timestamp. > */ > - tx->verify_cached = 1; > + tx->has_ready_bitmap = 0; > > return ice_ptp_alloc_tx_tracker(tx); > } > @@ -1338,14 +1341,14 @@ void ice_ptp_link_change(struct ice_pf *pf, u8 port, bool linkup) > } > > /** > - * ice_ptp_tx_ena_intr - Enable or disable the Tx timestamp interrupt > + * ice_ptp_cfg_phy_interrupt - Configure PHY interrupt settings > * @pf: PF private structure > * @ena: bool value to enable or disable interrupt > * @threshold: Minimum number of packets at which intr is triggered > * > * Utility function to enable or disable Tx timestamp interrupt and threshold > */ > -static int ice_ptp_tx_ena_intr(struct ice_pf *pf, bool ena, u32 threshold) > +static int ice_ptp_cfg_phy_interrupt(struct ice_pf *pf, bool ena, u32 threshold) > { > struct ice_hw *hw = &pf->hw; > int err = 0; > @@ -2507,8 +2510,8 @@ void ice_ptp_reset(struct ice_pf *pf, enum ice_reset_req reset_type) > struct ice_ptp *ptp = &pf->ptp; > struct ice_hw *hw = &pf->hw; > struct timespec64 ts; > - int err, itr = 1; > u64 time_diff; > + int err; > > if (ptp->state != ICE_PTP_RESETTING) { > if (ptp->state == ICE_PTP_READY) { > @@ -2561,7 +2564,7 @@ void ice_ptp_reset(struct ice_pf *pf, enum ice_reset_req reset_type) > > if (!ice_is_e810(hw)) { > /* Enable quad interrupts */ > - err = ice_ptp_tx_ena_intr(pf, true, itr); > + err = ice_ptp_cfg_phy_interrupt(pf, true, 1); > if (err) > goto err; > } > @@ -2847,13 +2850,13 @@ static int ice_ptp_init_owner(struct ice_pf *pf) > /* The clock owner for this device type handles the timestamp > * interrupt for all ports. > */ > - ice_ptp_configure_tx_tstamp(pf, true); > + ice_ptp_cfg_tx_interrupt(pf, true); > > /* React on all quads interrupts for E82x */ > wr32(hw, PFINT_TSYN_MSK + (0x4 * hw->pf_id), (u32)0x1f); > > /* Enable quad interrupts */ > - err = ice_ptp_tx_ena_intr(pf, true, itr); > + err = ice_ptp_cfg_phy_interrupt(pf, true, itr); > if (err) > goto err_exit; > } > @@ -2925,7 +2928,7 @@ static int ice_ptp_init_port(struct ice_pf *pf, struct ice_ptp_port *ptp_port) > * neither on own quad nor on others > */ > if (!ice_ptp_pf_handles_tx_interrupt(pf)) { > - ice_ptp_configure_tx_tstamp(pf, false); > + ice_ptp_cfg_tx_interrupt(pf, false); > wr32(hw, PFINT_TSYN_MSK + (0x4 * hw->pf_id), (u32)0x0); > } > kthread_init_delayed_work(&ptp_port->ov_work, > diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.h b/drivers/net/ethernet/intel/ice/ice_ptp.h > index 48c0d56c0568..30ad714a2a21 100644 > --- a/drivers/net/ethernet/intel/ice/ice_ptp.h > +++ b/drivers/net/ethernet/intel/ice/ice_ptp.h > @@ -100,7 +100,7 @@ struct ice_perout_channel { > * the last timestamp we read for a given index. If the current timestamp > * value is the same as the cached value, we assume a new timestamp hasn't > * been captured. This avoids reporting stale timestamps to the stack. This is > - * only done if the verify_cached flag is set in ice_ptp_tx structure. > + * only done if the has_ready_bitmap flag is not set in ice_ptp_tx structure. > */ > struct ice_tx_tstamp { > struct sk_buff *skb; > @@ -131,6 +131,9 @@ enum ice_tx_tstamp_work { > * @calibrating: if true, the PHY is calibrating the Tx offset. During this > * window, timestamps are temporarily disabled. > * @verify_cached: if true, verify new timestamp differs from last read value > + * @has_ready_bitmap: if true, the hardware has a valid Tx timestamp ready > + * bitmap register. If false, fall back to verifying new > + * timestamp values against previously cached copy. > */ > struct ice_ptp_tx { > spinlock_t lock; /* lock protecting in_use bitmap */ > @@ -143,6 +146,7 @@ struct ice_ptp_tx { > u8 init : 1; > u8 calibrating : 1; > u8 verify_cached : 1; > + u8 has_ready_bitmap : 1; > }; > > /* Quad and port information for initializing timestamp blocks */
diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.c b/drivers/net/ethernet/intel/ice/ice_ptp.c index bd94b42e19dd..393156b9b426 100644 --- a/drivers/net/ethernet/intel/ice/ice_ptp.c +++ b/drivers/net/ethernet/intel/ice/ice_ptp.c @@ -281,11 +281,11 @@ static const char *ice_ptp_state_str(enum ice_ptp_state state) } /** - * ice_ptp_configure_tx_tstamp - Enable or disable Tx timestamp interrupt - * @pf: The PF pointer to search in + * ice_ptp_cfg_tx_interrupt - Configure Tx timestamp interrupt for the device + * @pf: Board private structure * @on: bool value for whether timestamp interrupt is enabled or disabled */ -static void ice_ptp_configure_tx_tstamp(struct ice_pf *pf, bool on) +static void ice_ptp_cfg_tx_interrupt(struct ice_pf *pf, bool on) { u32 val; @@ -320,7 +320,7 @@ static void ice_set_tx_tstamp(struct ice_pf *pf, bool on) } if (pf->ptp.tx_interrupt_mode == ICE_PTP_TX_INTERRUPT_SELF) - ice_ptp_configure_tx_tstamp(pf, on); + ice_ptp_cfg_tx_interrupt(pf, on); pf->ptp.tstamp_config.tx_type = on ? HWTSTAMP_TX_ON : HWTSTAMP_TX_OFF; } @@ -591,9 +591,11 @@ static void ice_ptp_process_tx_tstamp(struct ice_ptp_tx *tx) hw = &pf->hw; /* Read the Tx ready status first */ - err = ice_get_phy_tx_tstamp_ready(hw, tx->block, &tstamp_ready); - if (err) - return; + if (tx->has_ready_bitmap) { + err = ice_get_phy_tx_tstamp_ready(hw, tx->block, &tstamp_ready); + if (err) + return; + } /* Drop packets if the link went down */ link_up = ptp_port->link_up; @@ -621,7 +623,8 @@ static void ice_ptp_process_tx_tstamp(struct ice_ptp_tx *tx) * If we do not, the hardware logic for generating a new * interrupt can get stuck on some devices. */ - if (!(tstamp_ready & BIT_ULL(phy_idx))) { + if (tx->has_ready_bitmap && + !(tstamp_ready & BIT_ULL(phy_idx))) { if (drop_ts) goto skip_ts_read; @@ -641,7 +644,7 @@ static void ice_ptp_process_tx_tstamp(struct ice_ptp_tx *tx) * from the last cached timestamp. If it is not, skip this for * now assuming it hasn't yet been captured by hardware. */ - if (!drop_ts && tx->verify_cached && + if (!drop_ts && !tx->has_ready_bitmap && raw_tstamp == tx->tstamps[idx].cached_tstamp) continue; @@ -651,7 +654,7 @@ static void ice_ptp_process_tx_tstamp(struct ice_ptp_tx *tx) skip_ts_read: spin_lock(&tx->lock); - if (tx->verify_cached && raw_tstamp) + if (!tx->has_ready_bitmap && raw_tstamp) tx->tstamps[idx].cached_tstamp = raw_tstamp; clear_bit(idx, tx->in_use); skb = tx->tstamps[idx].skb; @@ -895,7 +898,7 @@ ice_ptp_init_tx_e822(struct ice_pf *pf, struct ice_ptp_tx *tx, u8 port) tx->block = port / ICE_PORTS_PER_QUAD; tx->offset = (port % ICE_PORTS_PER_QUAD) * INDEX_PER_PORT_E822; tx->len = INDEX_PER_PORT_E822; - tx->verify_cached = 0; + tx->has_ready_bitmap = 1; return ice_ptp_alloc_tx_tracker(tx); } @@ -918,7 +921,7 @@ ice_ptp_init_tx_e810(struct ice_pf *pf, struct ice_ptp_tx *tx) * verify new timestamps against cached copy of the last read * timestamp. */ - tx->verify_cached = 1; + tx->has_ready_bitmap = 0; return ice_ptp_alloc_tx_tracker(tx); } @@ -1338,14 +1341,14 @@ void ice_ptp_link_change(struct ice_pf *pf, u8 port, bool linkup) } /** - * ice_ptp_tx_ena_intr - Enable or disable the Tx timestamp interrupt + * ice_ptp_cfg_phy_interrupt - Configure PHY interrupt settings * @pf: PF private structure * @ena: bool value to enable or disable interrupt * @threshold: Minimum number of packets at which intr is triggered * * Utility function to enable or disable Tx timestamp interrupt and threshold */ -static int ice_ptp_tx_ena_intr(struct ice_pf *pf, bool ena, u32 threshold) +static int ice_ptp_cfg_phy_interrupt(struct ice_pf *pf, bool ena, u32 threshold) { struct ice_hw *hw = &pf->hw; int err = 0; @@ -2507,8 +2510,8 @@ void ice_ptp_reset(struct ice_pf *pf, enum ice_reset_req reset_type) struct ice_ptp *ptp = &pf->ptp; struct ice_hw *hw = &pf->hw; struct timespec64 ts; - int err, itr = 1; u64 time_diff; + int err; if (ptp->state != ICE_PTP_RESETTING) { if (ptp->state == ICE_PTP_READY) { @@ -2561,7 +2564,7 @@ void ice_ptp_reset(struct ice_pf *pf, enum ice_reset_req reset_type) if (!ice_is_e810(hw)) { /* Enable quad interrupts */ - err = ice_ptp_tx_ena_intr(pf, true, itr); + err = ice_ptp_cfg_phy_interrupt(pf, true, 1); if (err) goto err; } @@ -2847,13 +2850,13 @@ static int ice_ptp_init_owner(struct ice_pf *pf) /* The clock owner for this device type handles the timestamp * interrupt for all ports. */ - ice_ptp_configure_tx_tstamp(pf, true); + ice_ptp_cfg_tx_interrupt(pf, true); /* React on all quads interrupts for E82x */ wr32(hw, PFINT_TSYN_MSK + (0x4 * hw->pf_id), (u32)0x1f); /* Enable quad interrupts */ - err = ice_ptp_tx_ena_intr(pf, true, itr); + err = ice_ptp_cfg_phy_interrupt(pf, true, itr); if (err) goto err_exit; } @@ -2925,7 +2928,7 @@ static int ice_ptp_init_port(struct ice_pf *pf, struct ice_ptp_port *ptp_port) * neither on own quad nor on others */ if (!ice_ptp_pf_handles_tx_interrupt(pf)) { - ice_ptp_configure_tx_tstamp(pf, false); + ice_ptp_cfg_tx_interrupt(pf, false); wr32(hw, PFINT_TSYN_MSK + (0x4 * hw->pf_id), (u32)0x0); } kthread_init_delayed_work(&ptp_port->ov_work, diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.h b/drivers/net/ethernet/intel/ice/ice_ptp.h index 48c0d56c0568..30ad714a2a21 100644 --- a/drivers/net/ethernet/intel/ice/ice_ptp.h +++ b/drivers/net/ethernet/intel/ice/ice_ptp.h @@ -100,7 +100,7 @@ struct ice_perout_channel { * the last timestamp we read for a given index. If the current timestamp * value is the same as the cached value, we assume a new timestamp hasn't * been captured. This avoids reporting stale timestamps to the stack. This is - * only done if the verify_cached flag is set in ice_ptp_tx structure. + * only done if the has_ready_bitmap flag is not set in ice_ptp_tx structure. */ struct ice_tx_tstamp { struct sk_buff *skb; @@ -131,6 +131,9 @@ enum ice_tx_tstamp_work { * @calibrating: if true, the PHY is calibrating the Tx offset. During this * window, timestamps are temporarily disabled. * @verify_cached: if true, verify new timestamp differs from last read value + * @has_ready_bitmap: if true, the hardware has a valid Tx timestamp ready + * bitmap register. If false, fall back to verifying new + * timestamp values against previously cached copy. */ struct ice_ptp_tx { spinlock_t lock; /* lock protecting in_use bitmap */ @@ -143,6 +146,7 @@ struct ice_ptp_tx { u8 init : 1; u8 calibrating : 1; u8 verify_cached : 1; + u8 has_ready_bitmap : 1; }; /* Quad and port information for initializing timestamp blocks */