ACK: [PATCH 1/1] Revert "igc: Disable PTM sequences when interface goes down"

[Thadeu Lima de Souza Cascardo]

On Wed, Sep 13, 2023 at 12:47:36PM -0400, Philip Cox wrote:
> BugLink: https://bugs.launchpad.net/bugs/2035361
> 
> This reverts commit c4dc67fc3e38fb2fd877674b21d972392ae0852a.
> commit c4dc67fc3e38fb2fd877674b21d972392ae0852a
> Author: Aravindhan Gunasekaran <aravindhan.gunasekaran at intel.com>
> Date:   Fri Apr 21 13:20:10 2023 -0400
> 
>     igc: Disable PTM sequences when interface goes down
> 
>     BugLink: https://bugs.launchpad.net/bugs/2019222
> 

Given the original Bug was the enablement of Time Sensitive Networking and this
was part of a large patchset, and that we clarified that the bug this was
supposed to fix has not been reproduced:

Acked-by: Thadeu Lima de Souza Cascardo <cascardo at canonical.com>

Also, it's good to point out that this was not an upstream commit, so we are
not diverging when we revert it. Actually the opposite.

Thanks.
Cascardo.

>     Kernel hangs or reboots reported in some boards with a
>     combination of interface up/down or reset. It turns out
>     that this occurs due to Foxville bus master disabling
>     when PTM sequences remain enabled.
> 
>     We do not need to always enable PTM in the reset
>     sequence as igc_ptp_reset is also called during
>     interface down. This caused PTM sequences be enabled
>     but Foxville tries to disable bus mastering before
>     going through controller reset.
> 
>     This patch disables PCIe PTM when interface goes down.
> 
>     Signed-off-by: Aravindhan Gunasekaran <aravindhan.gunasekaran at intel.com>
>     (back-ported from https://github.com/intel/linux-intel-quilt/tree/mainline-tracking-v5.19-linux-221019T120731Z/patches/0001-igc-Disable-PTM-sequences-when-interface-goes-down.tsn
>     [context changes])
>     Signed-off-by: Philip Cox <philip.cox at canonical.com>
>     Acked-by: Jian Hui Lee <jianhui.lee at canonical.com>
>     Acked-by: Tim Gardner <tim.gardner at canonical.com>
> 
> Signed-off-by: Philip Cox <philip.cox at canonical.com>
> ---
>  drivers/net/ethernet/intel/igc/igc_ptp.c | 50 ++++++++++--------------
>  1 file changed, 21 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/igc/igc_ptp.c b/drivers/net/ethernet/intel/igc/igc_ptp.c
> index 993d339205ed..b1b93e9a4399 100644
> --- a/drivers/net/ethernet/intel/igc/igc_ptp.c
> +++ b/drivers/net/ethernet/intel/igc/igc_ptp.c
> @@ -1109,30 +1109,6 @@ static void igc_ptp_time_restore(struct igc_adapter *adapter)
>  	igc_ptp_write_i225(adapter, &ts);
>  }
>  
> -static void igc_ptm_start(struct igc_adapter *adapter)
> -{
> -	struct igc_hw *hw = &adapter->hw;
> -	u32 cycle_ctrl, ctrl;
> -
> -	wr32(IGC_PCIE_DIG_DELAY, IGC_PCIE_DIG_DELAY_DEFAULT);
> -	wr32(IGC_PCIE_PHY_DELAY, IGC_PCIE_PHY_DELAY_DEFAULT);
> -
> -	cycle_ctrl = IGC_PTM_CYCLE_CTRL_CYC_TIME(IGC_PTM_CYC_TIME_DEFAULT);
> -
> -	wr32(IGC_PTM_CYCLE_CTRL, cycle_ctrl);
> -
> -	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;
> -
> -	wr32(IGC_PTM_CTRL, ctrl);
> -
> -	/* Force the first cycle to run. */
> -	wr32(IGC_PTM_STAT, IGC_PTM_STAT_VALID);
> -}
> -
>  static void igc_ptm_stop(struct igc_adapter *adapter)
>  {
>  	struct igc_hw *hw = &adapter->hw;
> @@ -1166,8 +1142,10 @@ void igc_ptp_suspend(struct igc_adapter *adapter)
>  
>  	spin_unlock(&adapter->ptp_tx_lock);
>  
> -	if (pci_device_is_present(adapter->pdev))
> +	if (pci_device_is_present(adapter->pdev)) {
>  		igc_ptp_time_save(adapter);
> +		igc_ptm_stop(adapter);
> +	}
>  }
>  
>  /**
> @@ -1196,6 +1174,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;
>  	unsigned long flags;
>  	u32 timadj;
>  
> @@ -1220,10 +1199,23 @@ void igc_ptp_reset(struct igc_adapter *adapter)
>  		if (!igc_is_crosststamp_supported(adapter))
>  			break;
>  
> -		if (!test_bit(__IGC_DOWN, &adapter->state))
> -			igc_ptm_start(adapter);
> -		else
> -			igc_ptm_stop(adapter);
> +		wr32(IGC_PCIE_DIG_DELAY, IGC_PCIE_DIG_DELAY_DEFAULT);
> +		wr32(IGC_PCIE_PHY_DELAY, IGC_PCIE_PHY_DELAY_DEFAULT);
> +
> +		cycle_ctrl = IGC_PTM_CYCLE_CTRL_CYC_TIME(IGC_PTM_CYC_TIME_DEFAULT);
> +
> +		wr32(IGC_PTM_CYCLE_CTRL, cycle_ctrl);
> +
> +		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;
> +
> +		wr32(IGC_PTM_CTRL, ctrl);
> +
> +		/* Force the first cycle to run. */
> +		wr32(IGC_PTM_STAT, IGC_PTM_STAT_VALID);
>  
>  		break;
>  	default:
> -- 
> 2.34.1
> 
> 
> -- 
> kernel-team mailing list
> kernel-team at lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team