| From 01d3e9097fed7aa543ed5a0fa1005efccb4ea3a5 Mon Sep 17 00:00:00 2001 |
| From: Sasha Levin <sashal@kernel.org> |
| Date: Tue, 1 Apr 2025 16:35:29 -0700 |
| Subject: igc: fix PTM cycle trigger logic |
| |
| From: Christopher S M Hall <christopher.s.hall@intel.com> |
| |
| [ Upstream commit 8e404ad95d2c10c261e2ef6992c7c12dde03df0e ] |
| |
| Writing to clear the PTM status 'valid' bit while the PTM cycle is |
| triggered results in unreliable PTM operation. To fix this, clear the |
| PTM 'trigger' and status after each PTM transaction. |
| |
| The issue can be reproduced with the following: |
| |
| $ sudo phc2sys -R 1000 -O 0 -i tsn0 -m |
| |
| Note: 1000 Hz (-R 1000) is unrealistically large, but provides a way to |
| quickly reproduce the issue. |
| |
| PHC2SYS exits with: |
| |
| "ioctl PTP_OFFSET_PRECISE: Connection timed out" when the PTM transaction |
| fails |
| |
| This patch also fixes a hang in igc_probe() when loading the igc |
| driver in the kdump kernel on systems supporting PTM. |
| |
| The igc driver running in the base kernel enables PTM trigger in |
| igc_probe(). Therefore the driver is always in PTM trigger mode, |
| except in brief periods when manually triggering a PTM cycle. |
| |
| When a crash occurs, the NIC is reset while PTM trigger is enabled. |
| Due to a hardware problem, the NIC is subsequently in a bad busmaster |
| state and doesn't handle register reads/writes. When running |
| igc_probe() in the kdump kernel, the first register access to a NIC |
| register hangs driver probing and ultimately breaks kdump. |
| |
| With this patch, igc has PTM trigger disabled most of the time, |
| and the trigger is only enabled for very brief (10 - 100 us) periods |
| when manually triggering a PTM cycle. Chances that a crash occurs |
| during a PTM trigger are not 0, but extremely reduced. |
| |
| Fixes: a90ec8483732 ("igc: Add support for PTP getcrosststamp()") |
| Reviewed-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com> |
| Tested-by: Mor Bar-Gabay <morx.bar.gabay@intel.com> |
| Tested-by: Avigail Dahan <avigailx.dahan@intel.com> |
| Signed-off-by: Christopher S M Hall <christopher.s.hall@intel.com> |
| Reviewed-by: Corinna Vinschen <vinschen@redhat.com> |
| Signed-off-by: Jacob Keller <jacob.e.keller@intel.com> |
| Tested-by: Corinna Vinschen <vinschen@redhat.com> |
| Acked-by: Vinicius Costa Gomes <vinicius.gomes@intel.com> |
| Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com> |
| Signed-off-by: Sasha Levin <sashal@kernel.org> |
| --- |
| drivers/net/ethernet/intel/igc/igc_defines.h | 1 + |
| drivers/net/ethernet/intel/igc/igc_ptp.c | 70 ++++++++++++-------- |
| 2 files changed, 42 insertions(+), 29 deletions(-) |
| |
| diff --git a/drivers/net/ethernet/intel/igc/igc_defines.h b/drivers/net/ethernet/intel/igc/igc_defines.h |
| index 8187a658dcbd5..17a7e3c875ccb 100644 |
| --- a/drivers/net/ethernet/intel/igc/igc_defines.h |
| +++ b/drivers/net/ethernet/intel/igc/igc_defines.h |
| @@ -574,6 +574,7 @@ |
| #define IGC_PTM_STAT_T4M1_OVFL BIT(3) /* T4 minus T1 overflow */ |
| #define IGC_PTM_STAT_ADJUST_1ST BIT(4) /* 1588 timer adjusted during 1st PTM cycle */ |
| #define IGC_PTM_STAT_ADJUST_CYC BIT(5) /* 1588 timer adjusted during non-1st PTM cycle */ |
| +#define IGC_PTM_STAT_ALL GENMASK(5, 0) /* Used to clear all status */ |
| |
| /* PCIe PTM Cycle Control */ |
| #define IGC_PTM_CYCLE_CTRL_CYC_TIME(msec) ((msec) & 0x3ff) /* PTM Cycle Time (msec) */ |
| diff --git a/drivers/net/ethernet/intel/igc/igc_ptp.c b/drivers/net/ethernet/intel/igc/igc_ptp.c |
| index 14cd7f995280d..b4720545191ff 100644 |
| --- a/drivers/net/ethernet/intel/igc/igc_ptp.c |
| +++ b/drivers/net/ethernet/intel/igc/igc_ptp.c |
| @@ -844,13 +844,40 @@ static void igc_ptm_log_error(struct igc_adapter *adapter, u32 ptm_stat) |
| } |
| } |
| |
| +static void igc_ptm_trigger(struct igc_hw *hw) |
| +{ |
| + u32 ctrl; |
| + |
| + /* To "manually" start the PTM cycle we need to set the |
| + * trigger (TRIG) bit |
| + */ |
| + ctrl = rd32(IGC_PTM_CTRL); |
| + ctrl |= IGC_PTM_CTRL_TRIG; |
| + wr32(IGC_PTM_CTRL, ctrl); |
| + /* Perform flush after write to CTRL register otherwise |
| + * transaction may not start |
| + */ |
| + wrfl(); |
| +} |
| + |
| +static void igc_ptm_reset(struct igc_hw *hw) |
| +{ |
| + u32 ctrl; |
| + |
| + ctrl = rd32(IGC_PTM_CTRL); |
| + ctrl &= ~IGC_PTM_CTRL_TRIG; |
| + wr32(IGC_PTM_CTRL, ctrl); |
| + /* Write to clear all status */ |
| + wr32(IGC_PTM_STAT, IGC_PTM_STAT_ALL); |
| +} |
| + |
| static int igc_phc_get_syncdevicetime(ktime_t *device, |
| struct system_counterval_t *system, |
| void *ctx) |
| { |
| - u32 stat, t2_curr_h, t2_curr_l, ctrl; |
| struct igc_adapter *adapter = ctx; |
| struct igc_hw *hw = &adapter->hw; |
| + u32 stat, t2_curr_h, t2_curr_l; |
| int err, count = 100; |
| ktime_t t1, t2_curr; |
| |
| @@ -864,25 +891,13 @@ static int igc_phc_get_syncdevicetime(ktime_t *device, |
| * are transitory. Repeating the process returns valid |
| * data eventually. |
| */ |
| - |
| - /* To "manually" start the PTM cycle we need to clear and |
| - * then set again the TRIG bit. |
| - */ |
| - ctrl = rd32(IGC_PTM_CTRL); |
| - ctrl &= ~IGC_PTM_CTRL_TRIG; |
| - wr32(IGC_PTM_CTRL, ctrl); |
| - ctrl |= IGC_PTM_CTRL_TRIG; |
| - wr32(IGC_PTM_CTRL, ctrl); |
| - |
| - /* The cycle only starts "for real" when software notifies |
| - * that it has read the registers, this is done by setting |
| - * VALID bit. |
| - */ |
| - wr32(IGC_PTM_STAT, IGC_PTM_STAT_VALID); |
| + igc_ptm_trigger(hw); |
| |
| err = readx_poll_timeout(rd32, IGC_PTM_STAT, stat, |
| stat, IGC_PTM_STAT_SLEEP, |
| IGC_PTM_STAT_TIMEOUT); |
| + igc_ptm_reset(hw); |
| + |
| if (err < 0) { |
| netdev_err(adapter->netdev, "Timeout reading IGC_PTM_STAT register\n"); |
| return err; |
| @@ -891,15 +906,7 @@ static int igc_phc_get_syncdevicetime(ktime_t *device, |
| if ((stat & IGC_PTM_STAT_VALID) == IGC_PTM_STAT_VALID) |
| break; |
| |
| - if (stat & ~IGC_PTM_STAT_VALID) { |
| - /* An error occurred, log it. */ |
| - igc_ptm_log_error(adapter, stat); |
| - /* The STAT register is write-1-to-clear (W1C), |
| - * so write the previous error status to clear it. |
| - */ |
| - wr32(IGC_PTM_STAT, stat); |
| - continue; |
| - } |
| + igc_ptm_log_error(adapter, stat); |
| } while (--count); |
| |
| if (!count) { |
| @@ -1081,7 +1088,7 @@ void igc_ptp_stop(struct igc_adapter *adapter) |
| void igc_ptp_reset(struct igc_adapter *adapter) |
| { |
| struct igc_hw *hw = &adapter->hw; |
| - u32 cycle_ctrl, ctrl; |
| + u32 cycle_ctrl, ctrl, stat; |
| unsigned long flags; |
| u32 timadj; |
| |
| @@ -1116,14 +1123,19 @@ void igc_ptp_reset(struct igc_adapter *adapter) |
| ctrl = IGC_PTM_CTRL_EN | |
| IGC_PTM_CTRL_START_NOW | |
| IGC_PTM_CTRL_SHRT_CYC(IGC_PTM_SHORT_CYC_DEFAULT) | |
| - IGC_PTM_CTRL_PTM_TO(IGC_PTM_TIMEOUT_DEFAULT) | |
| - IGC_PTM_CTRL_TRIG; |
| + IGC_PTM_CTRL_PTM_TO(IGC_PTM_TIMEOUT_DEFAULT); |
| |
| wr32(IGC_PTM_CTRL, ctrl); |
| |
| /* Force the first cycle to run. */ |
| - wr32(IGC_PTM_STAT, IGC_PTM_STAT_VALID); |
| + igc_ptm_trigger(hw); |
| + |
| + if (readx_poll_timeout_atomic(rd32, IGC_PTM_STAT, stat, |
| + stat, IGC_PTM_STAT_SLEEP, |
| + IGC_PTM_STAT_TIMEOUT)) |
| + netdev_err(adapter->netdev, "Timeout reading IGC_PTM_STAT register\n"); |
| |
| + igc_ptm_reset(hw); |
| break; |
| default: |
| /* No work to do. */ |
| -- |
| 2.39.5 |
| |