Re: [PATCH net] net: phy: allow phy_detach() before netdev registration

[Andrew Lunn <andrew@lunn.ch>]

> > Lets zoom out and look at the big picture....
> > 
> > Why is phy_detach() not symmetric with phy_attach()?
> > 
> > Why is phy_detach() touching dev->hwprov, when phy_attach() does not?
> > 
> > Where is dev->hwprov set and why is the mirror function not dealing
> > with this?

> The dev->hwprov is set by ETHTOOL_MSG_TSCONFIG_SET in
> ethnl_set_tsconfig(), which is the only writer besides phy_detach().

So this is the real problem. It does not fit the usual pattern of
setup and tairdown.

> phy_detach() clears hwprov only when the selected provider is this PHY,
> so skb_clone_tx_timestamp() / skb_defer_rx_timestamp() do not keep a
> dangling phydev pointer after the PHY is gone. That is not the reverse
> of phy_attach(); it is lifetime cleanup for a pointer that ethtool
> stashed in the netdev.

Can we make it fit the usual pattern?

https://elixir.bootlin.com/linux/v7.2.5/source/net/ethtool/tsconfig.c#L413

This sets the stamper to zero_config, via dev_set_hwtstamp_phylib().
Could that be done in phy_attach()?  phy_detach() would then have the
opposite, call dev_clear_hwtstamp_phylib().

ethnl_set_tsconfig() then just replaces the setting?

I would put dev_clear_hwtstamp_phylib() next to
dev_set_hwtstamp_phylib() so it is obvious the locking is the same in
both.

	Andrew