diff mbox series

[net] net: sparx5: Fix invalid timestamps

Message ID 20240916051804.27213-1-aakash.menon@protempis.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series [net] net: sparx5: Fix invalid timestamps | 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: 16 this patch: 16
netdev/build_tools success No tools touched, skip
netdev/cc_maintainers success CCed 12 of 12 maintainers
netdev/build_clang success Errors and warnings before: 16 this patch: 16
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 fail Problems with Fixes tag: 2
netdev/build_allmodconfig_warn success Errors and warnings before: 16 this patch: 16
netdev/checkpatch warning WARNING: From:/Signed-off-by: email address mismatch: 'From: Aakash Menon <aakash.r.menon@gmail.com>' != 'Signed-off-by: Aakash Menon <aakash.menon@protempis.com>' WARNING: Please use correct Fixes: style 'Fixes: <12 chars of sha1> ("<title line>")' - ie: 'Fixes: 3f854e8bb9c6 ("net: sparx5: Fix invalid timestamps")'
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
netdev/contest success net-next-2024-09-16--18-00 (tests: 764)

Commit Message

Aakash Menon Sept. 16, 2024, 5:18 a.m. UTC
Bit 270-271 are occasionally unexpectedly set by the hardware. This issue
was observed with 10G SFPs causing huge time errors (> 30ms) in PTP. Only
30 bits are needed for the nanosecond part of the timestamp, clear 2 most
significant bits before extracting timestamp from the internal frame
header.

Fixes: 70dfe25cd866 ("net: sparx5: Update extraction/injection for
timestamping")
Signed-off-by: Aakash Menon <aakash.menon@protempis.com>
---
 drivers/net/ethernet/microchip/sparx5/sparx5_packet.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

Comments

Horatiu Vultur Sept. 16, 2024, 7:17 a.m. UTC | #1
The 09/15/2024 22:18, Aakash Menon wrote:
> [Some people who received this message don't often get email from aakash.r.menon@gmail.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
> 
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> Bit 270-271 are occasionally unexpectedly set by the hardware. This issue
> was observed with 10G SFPs causing huge time errors (> 30ms) in PTP. Only
> 30 bits are needed for the nanosecond part of the timestamp, clear 2 most
> significant bits before extracting timestamp from the internal frame
> header.
> 
> Fixes: 70dfe25cd866 ("net: sparx5: Update extraction/injection for
> timestamping")
> Signed-off-by: Aakash Menon <aakash.menon@protempis.com>

There are some small issues/comments with this patch:
1. The Fixes tag should still be on one line even if it is longer than
75 columns, so the robots can find it.
2. Please use GENMASK(5, 0) instead of 0x3f.

> ---
>  drivers/net/ethernet/microchip/sparx5/sparx5_packet.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_packet.c b/drivers/net/ethernet/microchip/sparx5/sparx5_packet.c
> index f3f5fb420468..a05263488851 100644
> --- a/drivers/net/ethernet/microchip/sparx5/sparx5_packet.c
> +++ b/drivers/net/ethernet/microchip/sparx5/sparx5_packet.c
> @@ -45,8 +45,12 @@ void sparx5_ifh_parse(u32 *ifh, struct frame_info *info)
>         fwd = (fwd >> 5);
>         info->src_port = FIELD_GET(GENMASK(7, 1), fwd);
> 
> +       /*
> +        * Bit 270-271 are occasionally unexpectedly set by the hardware,
> +        * clear bits before extracting timestamp
> +        */
>         info->timestamp =
> -               ((u64)xtr_hdr[2] << 24) |
> +               ((u64)(xtr_hdr[2] & 0x3F) << 24) |
>                 ((u64)xtr_hdr[3] << 16) |
>                 ((u64)xtr_hdr[4] <<  8) |
>                 ((u64)xtr_hdr[5] <<  0);
> --
> 2.46.0
>
Aakash Menon Sept. 17, 2024, 12:25 a.m. UTC | #2
> > Bit 270-271 are occasionally unexpectedly set by the hardware. This issue
> > was observed with 10G SFPs causing huge time errors (> 30ms) in PTP. Only
> > 30 bits are needed for the nanosecond part of the timestamp, clear 2 most
> > significant bits before extracting timestamp from the internal frame
> > header.
> >
> > Fixes: 70dfe25cd866 ("net: sparx5: Update extraction/injection for
> > timestamping")
> > Signed-off-by: Aakash Menon <aakash.menon@protempis.com>
>
> There are some small issues/comments with this patch:
> 1. The Fixes tag should still be on one line even if it is longer than
> 75 columns, so the robots can find it.
> 2. Please use GENMASK(5, 0) instead of 0x3f.

Hi Horatiu,
Thanks for your feedback. I will make the suggested changes and send a
v2 patch in a new email thread.

- Aakash
diff mbox series

Patch

diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_packet.c b/drivers/net/ethernet/microchip/sparx5/sparx5_packet.c
index f3f5fb420468..a05263488851 100644
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_packet.c
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_packet.c
@@ -45,8 +45,12 @@  void sparx5_ifh_parse(u32 *ifh, struct frame_info *info)
 	fwd = (fwd >> 5);
 	info->src_port = FIELD_GET(GENMASK(7, 1), fwd);
 
+	/*
+	 * Bit 270-271 are occasionally unexpectedly set by the hardware,
+	 * clear bits before extracting timestamp
+	 */
 	info->timestamp =
-		((u64)xtr_hdr[2] << 24) |
+		((u64)(xtr_hdr[2] & 0x3F) << 24) |
 		((u64)xtr_hdr[3] << 16) |
 		((u64)xtr_hdr[4] <<  8) |
 		((u64)xtr_hdr[5] <<  0);