diff mbox series

[iwl-net] ice: fix incorrect PHY settings for 100 GB/s

Message ID 20241126102311.344972-1-przemyslaw.korba@intel.com (mailing list archive)
State Awaiting Upstream
Delegated to: Netdev Maintainers
Headers show
Series [iwl-net] ice: fix incorrect PHY settings for 100 GB/s | expand

Checks

Context Check Description
netdev/series_format success Single patches do not need cover letters
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: 3 this patch: 3
netdev/build_tools success No tools touched, skip
netdev/cc_maintainers fail 1 blamed authors not CCed: jacob.e.keller@intel.com; 6 maintainers not CCed: kuba@kernel.org jacob.e.keller@intel.com pabeni@redhat.com edumazet@google.com andrew+netdev@lunn.ch richardcochran@gmail.com
netdev/build_clang success Errors and warnings before: 3 this patch: 3
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: 5 this patch: 5
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 11 lines checked
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Przemyslaw Korba Nov. 26, 2024, 10:23 a.m. UTC
ptp4l application reports too high offset when ran on E823 device
with a 100GB/s link. Those values cannot go under 100ns, like in a
PTP working case when using 100 GB/s cable.
This is due to incorrect frequency settings on the PHY clocks for
100 GB/s speed. Changes are introduced to align with the internal
hardware documentation, and correctly initialize frequency in PHY
clocks with the frequency values that are in our HW spec.
To reproduce the issue run ptp4l as a Time Receiver on E823 device,
and observe the offset, which will never approach values seen
in the PTP working case.

Fixes: 3a7496234d17 ("ice: implement basic E822 PTP support")
Reviewed-by: Milena Olech <milena.olech@intel.com>
Signed-off-by: Przemyslaw Korba <przemyslaw.korba@intel.com>
---
 drivers/net/ethernet/intel/ice/ice_ptp_consts.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)


base-commit: 6ef5f61a4aa7d4df94a855a44f996bff08b0be83

Comments

Paul Menzel Nov. 26, 2024, 10:53 a.m. UTC | #1
Dear Przemyslaw,



Thank you for your patch.

Am 26.11.24 um 11:23 schrieb Przemyslaw Korba:
> ptp4l application reports too high offset when ran on E823 device
> with a 100GB/s link. Those values cannot go under 100ns, like in a
> PTP working case when using 100 GB/s cable.
> This is due to incorrect frequency settings on the PHY clocks for
> 100 GB/s speed. Changes are introduced to align with the internal
> hardware documentation, and correctly initialize frequency in PHY

It’d be great if you added the documentation name.

> clocks with the frequency values that are in our HW spec.
> To reproduce the issue run ptp4l as a Time Receiver on E823 device,
> and observe the offset, which will never approach values seen
> in the PTP working case.

(I’d add a blank line between paragraphs.)

Also, I always like to see pastes from the commands you ran to reproduce 
this. It’s always good for comparison.

> Fixes: 3a7496234d17 ("ice: implement basic E822 PTP support")

Any idea, where the wrong values came from? Will your test be added to 
the test procedure?

> Reviewed-by: Milena Olech <milena.olech@intel.com>
> Signed-off-by: Przemyslaw Korba <przemyslaw.korba@intel.com>
> ---
>   drivers/net/ethernet/intel/ice/ice_ptp_consts.h | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_ptp_consts.h b/drivers/net/ethernet/intel/ice/ice_ptp_consts.h
> index 6620642077bb..bdb1020147d1 100644
> --- a/drivers/net/ethernet/intel/ice/ice_ptp_consts.h
> +++ b/drivers/net/ethernet/intel/ice/ice_ptp_consts.h
> @@ -761,9 +761,9 @@ const struct ice_vernier_info_e82x e822_vernier[NUM_ICE_PTP_LNK_SPD] = {
>   		/* rx_desk_rsgb_par */
>   		644531250, /* 644.53125 MHz Reed Solomon gearbox */
>   		/* tx_desk_rsgb_pcs */
> -		644531250, /* 644.53125 MHz Reed Solomon gearbox */
> +		390625000, /* 390.625 MHz Reed Solomon gearbox */
>   		/* rx_desk_rsgb_pcs */
> -		644531250, /* 644.53125 MHz Reed Solomon gearbox */
> +		390625000, /* 390.625 MHz Reed Solomon gearbox */
>   		/* tx_fixed_delay */
>   		1620,
>   		/* pmd_adj_divisor */

Kind regards,

Paul
Andrew Lunn Nov. 26, 2024, 5:42 p.m. UTC | #2
On Tue, Nov 26, 2024 at 11:23:11AM +0100, Przemyslaw Korba wrote:
> ptp4l application reports too high offset when ran on E823 device
> with a 100GB/s link. Those values cannot go under 100ns, like in a
> PTP working case when using 100 GB/s cable.
> This is due to incorrect frequency settings on the PHY clocks for
> 100 GB/s speed. Changes are introduced to align with the internal
> hardware documentation, and correctly initialize frequency in PHY
> clocks with the frequency values that are in our HW spec.
> To reproduce the issue run ptp4l as a Time Receiver on E823 device,
> and observe the offset, which will never approach values seen
> in the PTP working case.

You forgot to Cc: the PTP maintainer.

If i spent the time to measure the latency and configured ptp4l
correctly to take into account the latency, would i not see this
issue? And will this change then cause a regression because it changes
the latency invalidating my measurements?

    Andrew
diff mbox series

Patch

diff --git a/drivers/net/ethernet/intel/ice/ice_ptp_consts.h b/drivers/net/ethernet/intel/ice/ice_ptp_consts.h
index 6620642077bb..bdb1020147d1 100644
--- a/drivers/net/ethernet/intel/ice/ice_ptp_consts.h
+++ b/drivers/net/ethernet/intel/ice/ice_ptp_consts.h
@@ -761,9 +761,9 @@  const struct ice_vernier_info_e82x e822_vernier[NUM_ICE_PTP_LNK_SPD] = {
 		/* rx_desk_rsgb_par */
 		644531250, /* 644.53125 MHz Reed Solomon gearbox */
 		/* tx_desk_rsgb_pcs */
-		644531250, /* 644.53125 MHz Reed Solomon gearbox */
+		390625000, /* 390.625 MHz Reed Solomon gearbox */
 		/* rx_desk_rsgb_pcs */
-		644531250, /* 644.53125 MHz Reed Solomon gearbox */
+		390625000, /* 390.625 MHz Reed Solomon gearbox */
 		/* tx_fixed_delay */
 		1620,
 		/* pmd_adj_divisor */