Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   "Keller, Jacob E" <jacob.e.keller@intel.com>
To:     Yuezhen Luan <eggcar.luan@gmail.com>,
        "Brandeburg, Jesse" <jesse.brandeburg@intel.com>,
        "Nguyen, Anthony L" <anthony.l.nguyen@intel.com>,
        "davem@davemloft.net" <davem@davemloft.net>,
        "edumazet@google.com" <edumazet@google.com>,
        "kuba@kernel.org" <kuba@kernel.org>,
        "pabeni@redhat.com" <pabeni@redhat.com>
CC:     "intel-wired-lan@lists.osuosl.org" <intel-wired-lan@lists.osuosl.org>,
        "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: RE: [PATCH v2] igb: Fix extts capture value format for
 82580/i354/i350
Thread-Topic: [PATCH v2] igb: Fix extts capture value format for
 82580/i354/i350
Thread-Index: AQHZlFbnwvMiSHn1HE+RDF8B6BOL0q92LT0Q
Date:   Thu, 1 Jun 2023 17:05:13 +0000
Message-ID: <CO1PR11MB5089465F5D37EBA62BB1A123D6499@CO1PR11MB5089.namprd11.prod.outlook.com>
References: <20230601070058.2117-1-eggcar.luan@gmail.com>
In-Reply-To: <20230601070058.2117-1-eggcar.luan@gmail.com>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?RYJqrNgXW5UDfYLOKZdNyOzl4HDBgS1yMJONXbT0oIveMgMwkP+bjV7c0MbU?=
 =?us-ascii?Q?IbItjRPhw5RcAtJTGWDD7AB0owdBHFrf2uIwyVl+hTz7BpEIRkQ736P96yPw?=
 =?us-ascii?Q?shggCXas9gfgluNp01St83fFqJKGfwIealkp7vKU6r1RveyYbJvvplI5qEWs?=
 =?us-ascii?Q?Tst7lUcsVIh6hWLyrWfPuxhZyyPl+UEAb4w/ma+L+M2oEBYIfS6IYlJG6H4D?=
 =?us-ascii?Q?BTXeYN9mlT6j1l4BgeZrGhXXyCEHxrGt8h+TLyf/7WX5JctAt4WSKuYJAK0J?=
 =?us-ascii?Q?8a3OI2NEQp4/Zlby7wCsQNXnK3uzlbI6nZp5fOmrLPzJFycGPgVWgTBI3RDZ?=
 =?us-ascii?Q?cP7eBtQEqNPnjaC4glPaSfR7nv7EYGCmcCxGu0PChtgup7e9SbAfsDnbtaax?=
 =?us-ascii?Q?c8kA/XkYHOALWblsWxtRE1OvfiNbxn7smLF/ZkEIijNQZJK0ZoZ/xERRwE0q?=
 =?us-ascii?Q?XeDYlMgl20X2Wh3IyDv6KxKVuea1pZ7SUpzs0YmDsAkcVlYmgHUYeQAkEWM4?=
 =?us-ascii?Q?b4RoO6R5V/nvnb7NoiK4AUwiqWarROOjXEOWgWKf6m+uHfx6qKcRdEMy3ru1?=
 =?us-ascii?Q?257ArICCiw8IjqyQPT+MNSx24jk68E3YkfdbAQ1kBMy8AvXz+QA1ZWMcMoz9?=
 =?us-ascii?Q?aoKQqiKOnQWUbYzcXHe162rhslqfY7BgY6sMdVBsYiyUYjGh7/iZmt9AKdsm?=
 =?us-ascii?Q?Fcm8n1smbmOm4uy/sjMmgUXCx+X8enKL1RZ2TgLfvlMgFHwtXlaMIYfl/G1L?=
 =?us-ascii?Q?Kw3Zdb2z4Q/DOBJOnUJJ9g+QK1an1Qr37YGKwUB29NH05YuWFF0T+Z7T0sBb?=
 =?us-ascii?Q?npUQeA/05Zr2PbwVs2YC7kFWzAKu1KSynekmUHZZgvmOia08Od1XIBGmNCEO?=
 =?us-ascii?Q?mNg1cftxGJipWU+uxf6W/N4UW+pR1d9P1HwUo0KlUVmBWoLEQsHuEFW3q4+R?=
 =?us-ascii?Q?Q/3u4Nnx7MXdwXkNGqlZpTliJ2RX62bfyBJ1NzitVDvprRR3IzYSmZw8oEbp?=
 =?us-ascii?Q?Wv16fdjeplN5KIz1FySfK7Be5NPtIvXMbeMPiN16JDVtBVSbjuhBBHkrbVs/?=
 =?us-ascii?Q?S/W/iaXAdzGJBhkWCpLDYldYPtbTHwOM5kpT4pauDeFDpNAyd0JKvDc1BnR1?=
 =?us-ascii?Q?zvRDku7BbZyyYvmGLdUre5NSZ0urZOY9/0Jz5HqvHzYutBBvJK47S0FvIm5K?=
 =?us-ascii?Q?/LsjbHemIVHpaPpJc0ecn1DM6XWkiAhcCQKsoyjhHpzdVrCyBlmOzlfnhZEC?=
 =?us-ascii?Q?nHcIMdTkpzYDSZALfw/0N3aKzEtII3AhPvNe51YUWDkcCI5TVcWVtpgVorsK?=
 =?us-ascii?Q?49potziLD4dK0PNTjlFn2A2YdD7Wh4JSzQrA2tKuxLkG4CgDkceQj+iVjXA2?=
 =?us-ascii?Q?P1KDpBkr9VHckNsqEw6kZGGraSmKnfFZM9wj06nMPKfSbDt5SAI38vGQ1wjD?=
 =?us-ascii?Q?r5mog5/DapX/29lIEz+zPgF78gLVwB3MYPCoOBgLtEJ2jnufvMz3fdh4ReCT?=
 =?us-ascii?Q?aoXFDCNtOmf3AhmSmoORGTH1KlIqWHrbTZLJciB1BblL0wib5jO50JIX4d6Y?=
 =?us-ascii?Q?5B3cip609qPLISP6gsIy6Zrczy1Byy8MIBn22sUr?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: CO1PR11MB5089.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 9b6ede03-137a-4bdf-664a-08db62c25d6b
X-MS-Exchange-CrossTenant-originalarrivaltime: 01 Jun 2023 17:05:13.3091
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: P/RWUkXvwF2+2TCwAwF2ADlTrlmuGONKi+RJ+8iLpP8eXd3+VbCyikWp0oSU+YJ8ukJQ2+MEDS25ptnIQyFoHufPX9bFu97g/UX5Dw1O10k=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH7PR11MB7661
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-4.6 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_MED,
        SPF_HELO_NONE,SPF_NONE,T_SCC_BODY_TEXT_LINE autolearn=ham
        autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


> -----Original Message-----
> From: Yuezhen Luan <eggcar.luan@gmail.com>
> Sent: Thursday, June 1, 2023 12:01 AM
> To: Brandeburg, Jesse <jesse.brandeburg@intel.com>; Nguyen, Anthony L
> <anthony.l.nguyen@intel.com>; davem@davemloft.net;
> edumazet@google.com; kuba@kernel.org; pabeni@redhat.com
> Cc: intel-wired-lan@lists.osuosl.org; netdev@vger.kernel.org; linux-
> kernel@vger.kernel.org; Keller, Jacob E <jacob.e.keller@intel.com>; Yuezh=
en
> Luan <eggcar.luan@gmail.com>
> Subject: [PATCH v2] igb: Fix extts capture value format for 82580/i354/i3=
50
>=20
> 82580/i354/i350 features circle-counter-like timestamp registers
> that are different with newer i210. The EXTTS capture value in
> AUXTSMPx should be converted from raw circle counter value to
> timestamp value in resolution of 1 nanosec by the driver.
>=20
> This issue can be reproduced on i350 nics, connecting an 1PPS
> signal to a SDP pin, and run 'ts2phc' command to read external
> 1PPS timestamp value. On i210 this works fine, but on i350 the
> extts is not correctly converted.
>=20
> The i350/i354/82580's SYSTIM and other timestamp registers are
> 40bit counters, presenting time range of 2^40 ns, that means these
> registers overflows every about 1099s. This causes all these regs
> can't be used directly in contrast to the newer i210/i211s.
>=20
> The igb driver needs to convert these raw register values to
> valid time stamp format by using kernel timecounter apis for i350s
> families. Here the igb_extts() just forgot to do the convert.
>=20
> Signed-off-by: Yuezhen Luan <eggcar.luan@gmail.com>

Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>=09

Thanks for fixing this!

@Nguyen, Anthony L
I think this is a worthy net fix.

> ---
>  drivers/net/ethernet/intel/igb/igb_main.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
>=20
> diff --git a/drivers/net/ethernet/intel/igb/igb_main.c
> b/drivers/net/ethernet/intel/igb/igb_main.c
> index 58872a4c2..bb3db387d 100644
> --- a/drivers/net/ethernet/intel/igb/igb_main.c
> +++ b/drivers/net/ethernet/intel/igb/igb_main.c
> @@ -6947,6 +6947,7 @@ static void igb_extts(struct igb_adapter *adapter, =
int
> tsintr_tt)
>  	struct e1000_hw *hw =3D &adapter->hw;
>  	struct ptp_clock_event event;
>  	struct timespec64 ts;
> +	unsigned long flags;
>=20
>  	if (pin < 0 || pin >=3D IGB_N_SDP)
>  		return;
> @@ -6954,9 +6955,12 @@ static void igb_extts(struct igb_adapter *adapter,=
 int
> tsintr_tt)
>  	if (hw->mac.type =3D=3D e1000_82580 ||
>  	    hw->mac.type =3D=3D e1000_i354 ||
>  	    hw->mac.type =3D=3D e1000_i350) {
> -		s64 ns =3D rd32(auxstmpl);
> +		u64 ns =3D rd32(auxstmpl);
>=20
> -		ns +=3D ((s64)(rd32(auxstmph) & 0xFF)) << 32;
> +		ns +=3D ((u64)(rd32(auxstmph) & 0xFF)) << 32;
> +		spin_lock_irqsave(&adapter->tmreg_lock, flags);
> +		ns =3D timecounter_cyc2time(&adapter->tc, ns);
> +		spin_unlock_irqrestore(&adapter->tmreg_lock, flags);
>  		ts =3D ns_to_timespec64(ns);
>  	} else {
>  		ts.tv_nsec =3D rd32(auxstmpl);
> --
> 2.34.1