diff mbox series

[iwl-net,v1,4/5] igc: Reduce retry count to a more reasonable number

Message ID 20240807003032.10300-5-christopher.s.hall@intel.com (mailing list archive)
State Awaiting Upstream
Delegated to: Netdev Maintainers
Headers show
Series igc: PTM timeout fix | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for net
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag present in non-next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 29 this patch: 29
netdev/build_tools success No tools touched, skip
netdev/cc_maintainers fail 1 blamed authors not CCed: anthony.l.nguyen@intel.com; 6 maintainers not CCed: edumazet@google.com richardcochran@gmail.com kuba@kernel.org pabeni@redhat.com przemyslaw.kitszel@intel.com anthony.l.nguyen@intel.com
netdev/build_clang success Errors and warnings before: 29 this patch: 29
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success Fixes tag looks correct
netdev/build_allmodconfig_warn success Errors and warnings before: 29 this patch: 29
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 9 lines checked
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 6 this patch: 6
netdev/source_inline success Was 0 now: 0

Commit Message

Hall, Christopher S Aug. 7, 2024, 12:30 a.m. UTC
From: Christopher S M Hall <christopher.s.hall@intel.com>

Setting the retry count to 8x is more than sufficient. 100x is unreasonable
 and would indicate broken hardware/firmware.

Fixes: a90ec8483732 ("igc: Add support for PTP getcrosststamp()")
Signed-off-by: Christopher S M Hall <christopher.s.hall@intel.com>
---
 drivers/net/ethernet/intel/igc/igc_ptp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Paul Menzel Aug. 7, 2024, 5:28 a.m. UTC | #1
Dear Christopher,


Thank you for your patch.

In the summary, I’d add specific values:

igc: Reduce retry count to from 100 to reasonable 8


Am 07.08.24 um 02:30 schrieb christopher.s.hall@intel.com:
> From: Christopher S M Hall <christopher.s.hall@intel.com>
> 
> Setting the retry count to 8x is more than sufficient. 100x is unreasonable
>  and would indicate broken hardware/firmware.

I’d remove the leading space.

Is using a 100 causing so much more delay and debugging an issue is harder?

> Fixes: a90ec8483732 ("igc: Add support for PTP getcrosststamp()")
> Signed-off-by: Christopher S M Hall <christopher.s.hall@intel.com>
> ---
>   drivers/net/ethernet/intel/igc/igc_ptp.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/intel/igc/igc_ptp.c b/drivers/net/ethernet/intel/igc/igc_ptp.c
> index fb885fcaa97c..f770e39650ef 100644
> --- a/drivers/net/ethernet/intel/igc/igc_ptp.c
> +++ b/drivers/net/ethernet/intel/igc/igc_ptp.c
> @@ -1008,8 +1008,8 @@ static int igc_phc_get_syncdevicetime(ktime_t *device,
>   	u32 stat, t2_curr_h, t2_curr_l;
>   	struct igc_adapter *adapter = ctx;
>   	struct igc_hw *hw = &adapter->hw;
> -	int err, count = 100;
>   	ktime_t t1, t2_curr;
> +	int err, count = 8;

Is there data available that no more than 8 retries were needed?

>   	/* Doing this in a loop because in the event of a
>   	 * badly timed (ha!) system clock adjustment, we may


Kind regards,

Paul
diff mbox series

Patch

diff --git a/drivers/net/ethernet/intel/igc/igc_ptp.c b/drivers/net/ethernet/intel/igc/igc_ptp.c
index fb885fcaa97c..f770e39650ef 100644
--- a/drivers/net/ethernet/intel/igc/igc_ptp.c
+++ b/drivers/net/ethernet/intel/igc/igc_ptp.c
@@ -1008,8 +1008,8 @@  static int igc_phc_get_syncdevicetime(ktime_t *device,
 	u32 stat, t2_curr_h, t2_curr_l;
 	struct igc_adapter *adapter = ctx;
 	struct igc_hw *hw = &adapter->hw;
-	int err, count = 100;
 	ktime_t t1, t2_curr;
+	int err, count = 8;
 
 	/* Doing this in a loop because in the event of a
 	 * badly timed (ha!) system clock adjustment, we may