Received-SPF: pass (google.com: domain of linux-kernel+bounces-149056-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Date: Wed, 17 Apr 2024 20:33:38 +0200
From: Andrew Lunn <andrew@lunn.ch>
To: Oleksij Rempel <o.rempel@pengutronix.de>
Cc: Alexandre Torgue <alexandre.torgue@foss.st.com>,
	Jose Abreu <joabreu@synopsys.com>,
	"David S. Miller" <davem@davemloft.net>,
	Heiner Kallweit <hkallweit1@gmail.com>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	Maxime Coquelin <mcoquelin.stm32@gmail.com>,
	Woojung Huh <woojung.huh@microchip.com>,
	Arun Ramadoss <arun.ramadoss@microchip.com>,
	Richard Cochran <richardcochran@gmail.com>,
	Russell King <linux@armlinux.org.uk>, 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
Message-ID: <898f435b-99dd-4636-9a52-558780c1bb06@lunn.ch>
References: <20240417164316.1755299-1-o.rempel@pengutronix.de>
 <20240417164316.1755299-2-o.rempel@pengutronix.de>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20240417164316.1755299-2-o.rempel@pengutronix.de>

On Wed, Apr 17, 2024 at 06:43:13PM +0200, Oleksij Rempel wrote:
> Add a new phy_get_timesync_data_path_delays() function, to the PHY
> device API. This function enables querying the ingress and egress
> TimeSync delays for PHY devices, as specified in IEEE 802.3-2022
> sections 30.13.1.3 to 30.13.1.6. The function adds the capability to
> obtain the average delays in nanoseconds, which can be used to
> compensate for time variations added by the PHY to PTP packets.
> 
> Since most PHYs do not provide register-based delay information, PHY
> drivers should supply this data, typically dependent on the interface
> type (MII, RGMII, etc.) and link speed. The MAC driver, or consumer of
> this API, is expected to invoke this function upon link establishment to
> accurately compensate for any PHY-induced time discrepancies.

So your intention is that this function is called from within the
adjust_link callback? Hence there is no locking being performed
because the lock is already held?

> +/**
> + * 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.

Please document the context this function should be used in. If users
use it outside of the adjust_link callback, the locking will be
wrong....

> + *
> + * Returns 0 on success, negative value on failure.

I think kdocs now requires a : after Returns ?

> +	/**
> +	 * @get_timesync_data_path_delays: Get the PHY time sync delay values
> +	 * @dev: PHY device
> +	 * @tsd: PHY time sync delay values
> +	 *
> +	 * Returns 0 on success, or an error code.

Same here.

     Andrew