From: Chenguang Zhao <chenguang.zhao@linux.dev>
To: Andrew Lunn <andrew@lunn.ch>
Cc: hkallweit1@gmail.com, linux@armlinux.org.uk, davem@davemloft.net,
edumazet@google.com, kuba@kernel.org, pabeni@redhat.com,
netdev@vger.kernel.org, Chenguang Zhao <zhaochenguang@kylinos.cn>,
syzbot+694b49f41098a5df4fd7@syzkaller.appspotmail.com
Subject: Re: [PATCH net] net: phy: allow phy_detach() before netdev registration
Date: Wed, 23 Sep 2026 15:16:56 +0800 [thread overview]
Message-ID: <20260923071656.GA323450@pc> (raw)
In-Reply-To: <f1f795ed-0511-4a0a-a3e6-70c49d6735e2@lunn.ch>
On Mon, Sep 21, 2026 at 02:15:58PM +0200, Andrew Lunn wrote:
> > > 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
Hi Andrew,
I've just sent out v3, addressing your comments on the provider lifetime
asymmetry.
https://lore.kernel.org/all/20260923071132.908826-1-chenguang.zhao@linux.dev/
Happy to iterate further if anything still needs adjustment.
Thanks,
Chenguang
next prev parent reply other threads:[~2026-09-23 7:16 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-16 2:15 [PATCH net] net: phy: allow phy_detach() before netdev registration Chenguang Zhao
2026-09-16 12:12 ` Andrew Lunn
2026-09-17 7:06 ` Chenguang Zhao
2026-09-17 12:04 ` Andrew Lunn
2026-09-21 2:03 ` Chenguang Zhao
2026-09-21 12:15 ` Andrew Lunn
2026-09-23 7:16 ` Chenguang Zhao [this message]
2026-09-20 2:32 ` netdev-bot+sashiko
2026-09-21 2:09 ` Chenguang Zhao
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260923071656.GA323450@pc \
--to=chenguang.zhao@linux.dev \
--cc=andrew@lunn.ch \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=hkallweit1@gmail.com \
--cc=kuba@kernel.org \
--cc=linux@armlinux.org.uk \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=syzbot+694b49f41098a5df4fd7@syzkaller.appspotmail.com \
--cc=zhaochenguang@kylinos.cn \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox