On Tue, 12 Nov 2024 11:12:32 +0100 Kory Maincent wrote:

quoted

Storing the info about the "user" (netdev, phydev) in the "provider"
(PHC) feels too much like a layering violation. Why do you need this?  

The things is that, the way to manage the phc depends on the "user".
ndo_hwtstamp_set for netdev and phy_hwtstamp_set for phydev.
https://elixir.bootlin.com/linux/v6.11.6/source/net/core/dev_ioctl.c#L323

Before PHC was managed by the driver "user" so there was no need for this
information as the core only gives the task to the single "user". This didn't
really works when there is more than one user possible on the net topology.

I don't understand. I'm complaining storing netdev state in 
struct ptp_clock. It's perfectly fine to add the extra info to netdev
and PHY topology maintained by the core.

quoted

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.  

Indeed PHC per socket would be neat but it would need a lot more work and I am
even not sure how it should be done. Maybe with a new cmsg structure containing
the information of the PHC provider?
In any case the new ETHTOOL UAPI is ready to support multiple PHC at the same
time when it will be supported.
This patch series is something in the middle, being able to enable all the PHC
on a net topology but only one at a time.

I understand, I don't want to push you towards implementing all that.
But if we keep that in mind as the north star we should try to align
this default / temporary solution. If the socket API takes a PHC ID
as an input, the configuration we take in should also be maintained
as "int default_phc", not pointers to things.

IOW I'm struggling to connect the dots how the code you're adding now
will be built _upon_ rather than _on the side_ of when socket PHC
selection is in place.