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:   Sai Krishna Gajula <saikrishnag@marvell.com>
To:     Jakub Kicinski <kuba@kernel.org>
CC:     "davem@davemloft.net" <davem@davemloft.net>,
        "edumazet@google.com" <edumazet@google.com>,
        "pabeni@redhat.com" <pabeni@redhat.com>,
        "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        Sunil Kovvuri Goutham <sgoutham@marvell.com>,
        Geethasowjanya Akula <gakula@marvell.com>,
        Subbaraya Sundeep Bhatta <sbhatta@marvell.com>,
        Hariprasad Kelam <hkelam@marvell.com>,
        "richardcochran@gmail.com" <richardcochran@gmail.com>,
        Naveen Mamindlapalli <naveenm@marvell.com>
Subject: Re: [net-next PATCH v2] octeontx2-pf: Use PTP HW timestamp counter
 atomic update feature
Thread-Topic: [net-next PATCH v2] octeontx2-pf: Use PTP HW timestamp counter
 atomic update feature
Thread-Index: AQHZzBd5EeKIotOBfk6aipYP34LBZg==
Date:   Fri, 11 Aug 2023 05:48:38 +0000
Message-ID: <BY3PR18MB470745294C1B03C0DA5364E6A010A@BY3PR18MB4707.namprd18.prod.outlook.com>
References: <20230807140535.3070350-1-saikrishnag@marvell.com>
 <20230809155022.132a69a7@kernel.org>
In-Reply-To: <20230809155022.132a69a7@kernel.org>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?9oL6sJfjPZDWDEQBShdwlEz3La7gWH8Uy4Ar/HDXdhFo7iKMhYeDtFf/2ERK?=
 =?us-ascii?Q?qsUe03DImB75UJt/7ylroBqmIVPL9+E+eXSST8zerqAhTC4Iv3J/Lc9AOr2A?=
 =?us-ascii?Q?VCZMw7psKdXXSNd+7XuBiJ9+yiN0uL3KzdwromXydGml0/bcJ3NHzSzwxSar?=
 =?us-ascii?Q?6nr8npZgfJ7lFUZ98dYdULKFbFVxqrxNAhCbbQJgURmjPoysL1agR5Kav6XJ?=
 =?us-ascii?Q?F2rHSI9ueMrC9oH/VLOVN8NLY48MjaUxQnfDCSgzN5H+sbtfYvA8sD1n/H1P?=
 =?us-ascii?Q?4fffn1e9EOagW+Zd+YoDFqvViV1FMXgoMCdhNHqhU3zM6hIh/DdZd/uTYgS/?=
 =?us-ascii?Q?sZXlg4z8L0d6KTn2rPngdUOaAo9eQ+D+OGgERhzW39jgtEUhCa4VPR8WMmwa?=
 =?us-ascii?Q?jcBEgwtM0PoQYfjsw2/MOOomVMbuqwKLX8Oo14NzfYhsCn3Y2+ufhobsFuvc?=
 =?us-ascii?Q?lxzE4XTSqlXUt+R2Gr/sl0WRhF3nF2EahYA2vUbmqze3g7072SZRBNazFrFB?=
 =?us-ascii?Q?4sb2TsvNGCNnpq4MmhlMNBZ9Q6jB5vpASBc6QjfvNF8pax8pHrPDpK50F9Bh?=
 =?us-ascii?Q?WIcS4HQnlcejXo1gGOLlnSRl29fWd/UpCt4Bln98q3Kowgt5r4UZXdyLr1px?=
 =?us-ascii?Q?fn04G8FxmvjgtgjFNxZZd3U6Lp5IWI4wfCS6Qw+QpIyFcwDQNkYucI4pOFeV?=
 =?us-ascii?Q?WV/VNL2VzUXDTs4QAddBp7HXXnAHrNKtAul5ty0QwHOLTL6ccsvRjv1ihkgk?=
 =?us-ascii?Q?7SOD+DJeHaTyrJpZ8DVrjUd6Mq3xmr0gJ0+0f9Xgcjy4/Lqr7VBR14TfitBX?=
 =?us-ascii?Q?Oxd0eIceoL7HBkdqjP4wh6+ejNaxvz4G/0svlgVNtZbR9EIfuMn1YUrMuQK2?=
 =?us-ascii?Q?kSMh52ikgRYNPlq3NBASCGBSy62IQig5Zxa5DyqVRqBLP7gdAQ3IG+mUZnnS?=
 =?us-ascii?Q?NbE5Yb2X1rPb+k63n6Ey6Oll8/3wV7fsFqH+PtxMj51NOFROLmPdvysR/CjT?=
 =?us-ascii?Q?xD9XEft5orjLqsAC5edXr4NbDEyENwVWWKrxsi1PSPhXf+/oOIi1bnqm5uCd?=
 =?us-ascii?Q?MYXHnWo3Jv1i7ut1b4qgU5EA8KjRwjdA0aNMoI6G7eVbOsXYKFVeRQLlMN3b?=
 =?us-ascii?Q?bk+MEPk62xOaG+K+xtbT6HfexH706GD5xGOpaMRUfikU/xunViGENkpQL68E?=
 =?us-ascii?Q?qYRLX0pBzDdedOLiZ9Uivgx18Qi+1KtyvrJsDM+WjFmDFRf1MMA7IGu0PdHq?=
 =?us-ascii?Q?vmc+zjgbQ9eOqnhOlNYx59CA97kMatTd6AJ8BSXCg3dg2GDaIk2Xp9Z7xIZT?=
 =?us-ascii?Q?pmoOzOoIbzXXsCL8VnPB+ZbNdOabNHOt3W4NQv5Amke3QqlivE+doXqGGrbg?=
 =?us-ascii?Q?J1L8/bfCSlnpJbW2A6QTzYniUJrsRbaZQW886Yrc6xBplWW8/TghMq0EXlg9?=
 =?us-ascii?Q?gkQct0v7CYRexxFflX24/u2VOoSg7GAh1oYmQCIGpUN2zKGkD2ImwuxI7Ys2?=
 =?us-ascii?Q?+vSK8izMA+p3qdldqGt2iOvQJNLJ2t6hvrqr7tdO5taLH0Q1obRoZJix3XxE?=
 =?us-ascii?Q?lfbLCjr7k5FPHCEyE2BBgtc4JitjAQYYGEHV4E3J?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-OriginatorOrg: marvell.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: BY3PR18MB4707.namprd18.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 2897a2fc-ca7f-4d4c-477e-08db9a2e9c64
X-MS-Exchange-CrossTenant-originalarrivaltime: 11 Aug 2023 05:48:38.5844
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 70e1fb47-1155-421d-87fc-2e58f638b6e0
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: tvi3hZAIOJIZZB+xztoCVDsWHKiII/Ch9FyGpuPncAnfNAXpATG0DHpqUnRYlK4vNFziahXbW6tO0DbLG+mGqg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SN4PR18MB4888
X-Proofpoint-ORIG-GUID: Tah04fgDMEhcfvuZ8twxsk7nHPek-P4I
X-Proofpoint-GUID: Tah04fgDMEhcfvuZ8twxsk7nHPek-P4I
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.267,Aquarius:18.0.957,Hydra:6.0.591,FMLib:17.11.176.26
 definitions=2023-08-10_20,2023-08-10_01,2023-05-22_02
X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,RCVD_IN_DNSWL_BLOCKED,SPF_HELO_NONE,SPF_PASS 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: Jakub Kicinski <kuba@kernel.org>
> Sent: Thursday, August 10, 2023 4:20 AM
> To: Sai Krishna Gajula <saikrishnag@marvell.com>
> Cc: davem@davemloft.net; edumazet@google.com; pabeni@redhat.com;
> netdev@vger.kernel.org; linux-kernel@vger.kernel.org; Sunil Kovvuri
> Goutham <sgoutham@marvell.com>; Geethasowjanya Akula
> <gakula@marvell.com>; Subbaraya Sundeep Bhatta
> <sbhatta@marvell.com>; Hariprasad Kelam <hkelam@marvell.com>;
> richardcochran@gmail.com; Naveen Mamindlapalli
> <naveenm@marvell.com>
> Subject: Re: [net-next PATCH v2] octeontx2-pf: Use PTP HW timestamp
> counter atomic update feature
>=20
> On Mon, 7 Aug 2023 19:35:35 +0530 Sai Krishna wrote:
> > Some of the newer silicon versions in CN10K series supports a feature
> > where in the current PTP timestamp in HW can be updated atomically
> > without losing any cpu cycles unlike read/modify/write register.
> > This patch uses this feature so that PTP accuracy can be improved
> > while adjusting the master offset in HW. There is no need for SW
> > timecounter when using this feature. So removed references to SW
> > timecounter wherever appropriate.
>=20
> > -#include "ptp.h"
> >  #include "mbox.h"
> >  #include "rvu.h"
> > +#include "ptp.h"
>=20
> If you reorder the includes - maybe put them in alphabetical order?

There are some structure definitions in rvu.h which are required in ptp.h. =
So, re-ordering in alphabetical order will give compilation issue.=20

> >  static bool cn10k_ptp_errata(struct ptp *ptp)  {
> > -	if (ptp->pdev->subsystem_device =3D=3D
> PCI_SUBSYS_DEVID_CN10K_A_PTP ||
> > -	    ptp->pdev->subsystem_device =3D=3D
> PCI_SUBSYS_DEVID_CNF10K_A_PTP)
> > +	if ((is_ptp_dev_cn10ka(ptp) &&
> > +	     ((ptp->pdev->revision & 0x0F) =3D=3D 0x0 || (ptp->pdev->revision=
 &
> 0x0F) =3D=3D 0x1)) ||
> > +	    (is_ptp_dev_cnf10ka(ptp) &&
> > +	     ((ptp->pdev->revision & 0x0F) =3D=3D 0x0 || (ptp->pdev->revision=
 &
> > +0x0F) =3D=3D 0x1)))
>=20
> Please refactor the revision check to avoid these long lines repeating th=
e
> same logic

Sai:  Ack, will submit refactor changes in V3 patch.

>=20
> >  		return true;
> > +
> >  	return false;
> >  }
> >
> > -static bool is_ptp_tsfmt_sec_nsec(struct ptp *ptp)
> > +static bool is_tstmp_atomic_update_supported(struct rvu *rvu)
> >  {
> > -	if (ptp->pdev->subsystem_device =3D=3D
> PCI_SUBSYS_DEVID_CN10K_A_PTP ||
> > -	    ptp->pdev->subsystem_device =3D=3D
> PCI_SUBSYS_DEVID_CNF10K_A_PTP)
> > -		return true;
> > -	return false;
> > +	struct ptp *ptp =3D rvu->ptp;
> > +	struct pci_dev *pdev;
> > +
> > +	if (is_rvu_otx2(rvu))
> > +		return false;
> > +
> > +	pdev =3D ptp->pdev;
> > +
> > +	/* On older silicon variants of CN10K, atomic update feature
> > +	 * is not available.
> > +	 */
> > +	if ((pdev->subsystem_device =3D=3D PCI_SUBSYS_DEVID_CN10K_A_PTP
> &&
> > +	     (pdev->revision & 0x0F) =3D=3D 0x0) ||
> > +	     (pdev->subsystem_device =3D=3D PCI_SUBSYS_DEVID_CN10K_A_PTP
> &&
> > +	     (pdev->revision & 0x0F) =3D=3D 0x1) ||
> > +	     (pdev->subsystem_device =3D=3D
> PCI_SUBSYS_DEVID_CNF10K_A_PTP &&
> > +	     (pdev->revision & 0x0F) =3D=3D 0x0) ||
> > +	     (pdev->subsystem_device =3D=3D
> PCI_SUBSYS_DEVID_CNF10K_A_PTP &&
> > +	     (pdev->revision & 0x0F) =3D=3D 0x1))
>=20
> why are you not using cn10k_ptp_errata() here?

Ack, will submit the changes in V3 patch.

>=20
> > +		return false;
> > +
> > +	return true;
> >  }
>=20
> > -static int otx2_ptp_adjtime(struct ptp_clock_info *ptp_info, s64
> > delta)
> > +static int otx2_ptp_tc_adjtime(struct ptp_clock_info *ptp_info, s64
> > +delta)
> >  {
> >  	struct otx2_ptp *ptp =3D container_of(ptp_info, struct otx2_ptp,
> >  					    ptp_info);
> >  	struct otx2_nic *pfvf =3D ptp->nic;
> >
> > +	if (!ptp->nic)
> > +		return -ENODEV;
>=20
> Is this check related to the rest of the patch?

I will remove this check and submit V3 patch. Will submit this check as a s=
eparate patch, if required.

Thanks,
Sai
>=20
> >  	mutex_lock(&pfvf->mbox.lock);
> >  	timecounter_adjtime(&ptp->time_counter, delta);
> >  	mutex_unlock(&pfvf->mbox.lock);
> --
> pw-bot: cr