Message-ID: <20240726210427.525c7abc@kmaincent-XPS-13-7390>
Date: Fri, 26 Jul 2024 21:04:27 +0200
From: Kory Maincent <kory.maincent@...tlin.com>
To: Jacob Keller <jacob.e.keller@...el.com>
Cc: Florian Fainelli <florian.fainelli@...adcom.com>, Broadcom internal
 kernel review list <bcm-kernel-feedback-list@...adcom.com>, Andrew Lunn
 <andrew@...n.ch>, Heiner Kallweit <hkallweit1@...il.com>, Russell King
 <linux@...linux.org.uk>, "David S. Miller" <davem@...emloft.net>, "Eric
 Dumazet" <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>, "Paolo
 Abeni" <pabeni@...hat.com>, Richard Cochran <richardcochran@...il.com>,
 "Radu Pirea" <radu-nicolae.pirea@....nxp.com>, Jay Vosburgh
 <j.vosburgh@...il.com>, Andy Gospodarek <andy@...yhouse.net>, Nicolas Ferre
 <nicolas.ferre@...rochip.com>, Claudiu Beznea <claudiu.beznea@...on.dev>,
 Willem de Bruijn <willemdebruijn.kernel@...il.com>, Jonathan Corbet
 <corbet@....net>, Horatiu Vultur <horatiu.vultur@...rochip.com>,
 <UNGLinuxDriver@...rochip.com>, Simon Horman <horms@...nel.org>, "Vladimir
 Oltean" <vladimir.oltean@....com>, <donald.hunter@...il.com>,
 <danieller@...dia.com>, <ecree.xilinx@...il.com>, Thomas Petazzoni
 <thomas.petazzoni@...tlin.com>, <linux-kernel@...r.kernel.org>,
 <netdev@...r.kernel.org>, <linux-doc@...r.kernel.org>, Maxime Chevallier
 <maxime.chevallier@...tlin.com>, Rahul Rameshbabu <rrameshbabu@...dia.com>,
 Willem de Bruijn <willemb@...gle.com>, Shannon Nelson
 <shannon.nelson@....com>, Alexandra Winter <wintera@...ux.ibm.com>
Subject: Re: [PATCH net-next v17 12/14] net: ethtool: tsinfo: Add support
 for reading tsinfo for a specific hwtstamp provider

Hello Jacob,

Thanks a lot for your full review! 

On Wed, 17 Jul 2024 10:35:20 -0700
Jacob Keller <jacob.e.keller@...el.com> wrote:

> On 7/9/2024 6:53 AM, Kory Maincent wrote:
>  [...]  
> 
> Reviewed-by: Jacob Keller <jacob.e.keller@...el.com>
> 
> One thing which applies more broadly to the whole series, but I see the
> focus right now is on selecting between NETDEV and PHYLIB.
> 
> For ice (E800 series) hardware, the timestamps are captured by the PHY,
> but its not managed by phylib, its managed by firmware. In our case we
> would obviously report NETDEV in this case. The hardware only has one
> timestamp point and the fact that it happens at the PHY layer is not
> relevant since you can't select or change it.
> 
> There are some future plans in the work for hardware based on the ixgbe
> driver which could timestamp at either the MAC or PHY (with varying
> trade-offs in precision vs what can be timestamped), and (perhaps
> unfortunately), the PHY would likely not manageable by phylib.
> 
> There is also the possibility of something like DMA or completion
> timestamps which are distinct from MAC timestamps. But again can have
> varying trade offs.

As we already discussed in older version of this patch series the
hwtstamp qualifier will be used to select between IEEE 1588 timestamp or DMA
timestamp. See patch 8 :
+/*
+ * Possible type of htstamp provider. Mainly "precise" the default one
+ * is for IEEE 1588 quality and "approx" is for NICs DMA point.
+ */

We could add other enumeration values in the future if needed, to manage new
cases.

Just figured out there is a NIT in the doc. h*w*tstamp.

> I'm hopeful this work can be extended somehow to enable selection
> between the different mechanisms, even when the kernel device being
> represented is the same netdev.

Another nice features would be the support for simultaneous hardware timestamp
but I sadly won't be able to work on this.

Regards,
-- 
Köry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

Powered by blists - more mailing lists