Received-SPF: pass (google.com: domain of linux-kernel+bounces-149080-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
From: <Woojung.Huh@microchip.com>
To: <o.rempel@pengutronix.de>, <alexandre.torgue@foss.st.com>,
	<joabreu@synopsys.com>, <davem@davemloft.net>, <andrew@lunn.ch>,
	<hkallweit1@gmail.com>, <edumazet@google.com>, <kuba@kernel.org>,
	<pabeni@redhat.com>, <mcoquelin.stm32@gmail.com>,
	<Arun.Ramadoss@microchip.com>, <richardcochran@gmail.com>,
	<linux@armlinux.org.uk>
CC: <kernel@pengutronix.de>, <linux-kernel@vger.kernel.org>,
	<netdev@vger.kernel.org>, <UNGLinuxDriver@microchip.com>,
	<linux-stm32@st-md-mailman.stormreply.com>
Subject: RE: [PATCH net-next v1 1/4] net: phy: Add TimeSync delay query
 support to PHYlib API
Thread-Topic: [PATCH net-next v1 1/4] net: phy: Add TimeSync delay query
 support to PHYlib API
Thread-Index: AQHakOZ/fzt9Dn9Rq0KDo2lZUBVTB7FszMaA
Date: Wed, 17 Apr 2024 18:48:37 +0000
Message-ID: <BL0PR11MB2913839BD93ACB61B06C30F4E70F2@BL0PR11MB2913.namprd11.prod.outlook.com>
References: <20240417164316.1755299-1-o.rempel@pengutronix.de>
 <20240417164316.1755299-2-o.rempel@pengutronix.de>
In-Reply-To: <20240417164316.1755299-2-o.rempel@pengutronix.de>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?eUfbsqbBP6hVsBAgs6rXnjTuNQVK4nlXyqXi/QaV/CuIELQHY7+AlbfM/RxL?=
 =?us-ascii?Q?K55l8FP9sBnNgm/uXPWgLtVewBcDJC1Fhs4Dmr1NgawvKcQRvr1W9r9/5p+/?=
 =?us-ascii?Q?pf17O61OqNQDU5JHof85fihQ9GlSmGnrocHj3EMiLZ5b9xJRxnWcMNBOZv5H?=
 =?us-ascii?Q?2fznWpGl2lVJ6W0VmsUzAAriUcBn59+ZIYPEnVdo2kRTTPrr2LGQPLTwyodK?=
 =?us-ascii?Q?fQaKaS9vk6h5W1Zfzdugr4JtCHrzLwXp1J9A3wIGKizoRMG7hOn+JdWgdOCv?=
 =?us-ascii?Q?iT8lN7TRolRBeNxt3gqVpoSkWYfFBY8qioNtNWEeG4lNmR92IQ8b9sw5Sdif?=
 =?us-ascii?Q?BC+5Rix82+90aOIBPakCxt3iEEfgW7veH1sKM3qgbqqsY53761juQ4usaoAO?=
 =?us-ascii?Q?qhdufqz4QPO/EQY0OSmayTq5ijJUNiE6+rVhRV5YIq2XlzH54IMXgQ63CXZ2?=
 =?us-ascii?Q?ZMDMJrh9rQYcqndIL4SxoKSFY7r3+/BVdyFe0Ax2+5TI5IjlW6utY4j3yBCe?=
 =?us-ascii?Q?KlTi9jmFQNvgMOMY5/mx8mjxKPPuZigvYSJuUsbTPQlwO6U5kxrAr2WzuJYz?=
 =?us-ascii?Q?pmIIEjZbBUTDWaAcBYERSaA86R5iwSTi0Z77lYk1Tm4mibU+OuCqXijRYOhr?=
 =?us-ascii?Q?KS6Z1D6QNImvOUrSm+2/WX6zdFNZK5fVGBlGc2mVEVKpRNH2vG/2+xmlEDPS?=
 =?us-ascii?Q?wmfuyyJ8z2eaZQdyv7USu5jxfPUPPECWUHbGiskxhxq/5KdfU2h5+LxuFyDQ?=
 =?us-ascii?Q?vSBDQHEDZODob2wCG7ACO1huNnvXOaDlU3H5rVZzSox/nt1zneL3lctSRUAB?=
 =?us-ascii?Q?CADEtSR0EHoC8/QdBcUk/Z+ITQ+HelU2SzYuin/P2bq3oCgRLUN+nLbkKyeJ?=
 =?us-ascii?Q?lzrpodgWS5vfe98kPy012FBqKHLbRY4mxk1jREFLckohZhcgcKUB59e71jhA?=
 =?us-ascii?Q?1/qncpOkLXsC0TBnYd2oLISUBuIeFKigDIHBYHFvgCdRbZQ0xohg41/3fXUz?=
 =?us-ascii?Q?9Y3kS7AAbyu5w45idAlQBSky8KJC6AakJzcWxJmNfUgkJlBQWLKeWT/xomAt?=
 =?us-ascii?Q?TmlBURNbY0krHFubXhQyQC0GZocX6PnIWEFjOE/zmSyLtHfDa/xYqp7JzbS2?=
 =?us-ascii?Q?uXALUHEygc7nYdLtj7N8bolK4fXRBSs7WXV9YcYcpAH2M9Wn1Nj+c7ZwXieQ?=
 =?us-ascii?Q?1GpAjJduFLVBWI4Lpa11zGHWcM3yNsh4SqMtBh7KsFfzo/hlrkbQ2apyE46W?=
 =?us-ascii?Q?4pKkdZxmSwKaioy6UI18zjyDc2nX7kxnx8Ra714LI80POl3D1zyp926YEeCL?=
 =?us-ascii?Q?LO4koflUWqDT8MWz2EhyJdaUk95UKT8Zt0TRFsqwGSCO6ONZDVMPXahz/0wd?=
 =?us-ascii?Q?LMYFAVxWwZCWnggYMU6rYswyC6e6bTG6wlEqKwv2vA7avITwCJe4tj0UNneH?=
 =?us-ascii?Q?VMl9EBU/2ja6WQflQP8pmThAnqyuMp/gIukKNVcVukni9g3GDBW3vecUfUIW?=
 =?us-ascii?Q?DvcbirxwbhRh1T3lZ1ri2icnAHONuqhDmBjHtMFY0z1lvPmHV5Ho7eqy3800?=
 =?us-ascii?Q?RQYo4RuIFnOBf8Nq9mUfuiAPAgkgaXaClMp08QOF?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk
X-Mailing-List: linux-kernel@vger.kernel.org
List-Id: <linux-kernel.vger.kernel.org>
List-Subscribe: <mailto:linux-kernel+subscribe@vger.kernel.org>
List-Unsubscribe: <mailto:linux-kernel+unsubscribe@vger.kernel.org>
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: BL0PR11MB2913.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 000d264e-8171-401b-7ece-08dc5f0efdf3
X-MS-Exchange-CrossTenant-originalarrivaltime: 17 Apr 2024 18:48:37.4017
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 3f4057f3-b418-4d4e-ba84-d55b4e897d88
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: hNO19DNRGT+ON526tr4DnE+QL7scYzsbhtGRGBiqZUwlNN0sNHMT7XLEMMCrSs4RO3CiE33YA+9e0GR8NR41PlT5GuN86M55bu657HPaekg=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MW3PR11MB4668

Hi

> +/**
> + * phy_get_timesync_data_path_delays - get the TimeSync data path
> ingress/egress
> + *                                     delays
> + * @phydev: phy_device struct
> + * @tx_delay_ns: pointer to the transmit delay in nanoseconds
> + * @rx_delay_ns: pointer to the receive delay in nanoseconds
> + *
> + * This function is used to get the TimeSync data path ingress/egress
> delays
> + * as described in IEEE 802.3-2022 sections:
> + * 30.13.1.3 aTimeSyncDelayTXmax, 30.13.1.4 aTimeSyncDelayTXmin,
> + * 30.13.1.5 aTimeSyncDelayRXmax and 30.13.1.6 aTimeSyncDelayRXmin.
> + *
> + * The delays are returned in nanoseconds and can be used to compensate
> time
> + * added by the PHY to the PTP packets.
> + *
> + * Returns 0 on success, negative value on failure.
> + */
> +int phy_get_timesync_data_path_delays(struct phy_device *phydev,
> +                                     u64 *tx_delay_ns, u64 *rx_delay_ns)
> +{
> +       struct phy_timesync_delay tsd =3D { 0 };
> +       int err;
> +
> +       if (!phydev->drv->get_timesync_data_path_delays)
> +               return -EOPNOTSUPP;
> +
> +       if (!tx_delay_ns || !rx_delay_ns)
> +               return -EINVAL;
> +
> +       err =3D phydev->drv->get_timesync_data_path_delays(phydev, &tsd);
> +       if (err)
> +               return err;
> +
> +       if ((!tsd.tx_max_delay_ns && !tsd.tx_min_delay_ns) ||
> +           (!tsd.rx_max_delay_ns && !tsd.rx_min_delay_ns)) {
> +               phydev_err(phydev, "Invalid TimeSync data path delays\n")=
;
> +               return -EINVAL;
> +       }

Because all 4 variables are u64, it can be zero technically.
I think the condition of "max >=3D min" could be better option for validati=
on
because actual *_delay_ns is average of min and max value.

> +
> +       if (tsd.tx_max_delay_ns && tsd.tx_min_delay_ns)
> +               *tx_delay_ns =3D (tsd.tx_max_delay_ns + tsd.tx_min_delay_=
ns) /
> 2;
> +       else if (tsd.tx_max_delay_ns)
> +               *tx_delay_ns =3D tsd.tx_max_delay_ns;
> +       else
> +               *tx_delay_ns =3D tsd.tx_min_delay_ns;
> +
> +       if (tsd.rx_max_delay_ns && tsd.rx_min_delay_ns)
> +               *rx_delay_ns =3D (tsd.rx_max_delay_ns + tsd.rx_min_delay_=
ns) /
> 2;
> +       else if (tsd.rx_max_delay_ns)
> +               *rx_delay_ns =3D tsd.rx_max_delay_ns;
> +       else
> +               *rx_delay_ns =3D tsd.rx_min_delay_ns;
> +
> +       return 0;
> +}
> +EXPORT_SYMBOL(phy_get_timesync_data_path_delays);
> +

Thanks.
Woojung