Re: [PATCH net-next v19 03/10] ptp: Add phc source and helpers to register specific PTP clock or get information
From: Jakub Kicinski <kuba@kernel.org>
Date: 2024-11-11 23:06:12
Also in:
linux-doc, lkml
On Wed, 30 Oct 2024 14:54:45 +0100 Kory Maincent wrote:
quoted hunk ↗ jump to hunk
@@ -41,6 +43,11 @@ struct ptp_clock { struct ptp_clock_info *info; dev_t devid; int index; /* index into clocks.map */ + enum hwtstamp_source phc_source; + union { /* Pointer of the phc_source device */ + struct net_device *netdev; + struct phy_device *phydev; + };
Storing the info about the "user" (netdev, phydev) in the "provider" (PHC) feels too much like a layering violation. Why do you need this? In general I can't shake the feeling that we're trying to configure the "default" PHC for a narrow use case, while the goal should be to let the user pick the PHC per socket.
+/** + * netdev_ptp_clock_register() - Register a PTP hardware clock driver for + * a net device + * + * @info: Structure describing the new clock. + * @dev: Pointer of the net device.
+/** + * ptp_clock_from_netdev() - Does the PTP clock comes from netdev + * + * @ptp: The clock obtained from net/phy_ptp_clock_register(). + * + * Return: True if the PTP clock comes from netdev, false otherwise.
+/** + * ptp_clock_netdev() - Obtain the net_device reference of PTP clock
nit: pick one way to spell netdev ?
+ ret = ptp_clock_get(dev, ptp); + if (ret) + return ERR_PTR(ret);
why do you take references on the ptp device?