linux-kernel - Re: [PATCH net v2] phy: mscc: Fix PTP for vsc8574 and VSC8572

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250922121524.3baplkjgw2xnwizr@skbuf>
Date: Mon, 22 Sep 2025 15:15:24 +0300
From: Vladimir Oltean <vladimir.oltean@....com>
To: Jakub Kicinski <kuba@...nel.org>,
	Horatiu Vultur <horatiu.vultur@...rochip.com>
Cc: andrew@...n.ch, hkallweit1@...il.com, linux@...linux.org.uk,
	davem@...emloft.net, edumazet@...gle.com, pabeni@...hat.com,
	richardcochran@...il.com, vadim.fedorenko@...ux.dev,
	rmk+kernel@...linux.org.uk, christophe.jaillet@...adoo.fr,
	rosenp@...il.com, steen.hegelund@...rochip.com,
	netdev@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH net v2] phy: mscc: Fix PTP for vsc8574 and VSC8572

On Thu, Sep 18, 2025 at 04:09:42PM -0700, Jakub Kicinski wrote:
> On Wed, 17 Sep 2025 13:33:16 +0200 Horatiu Vultur wrote:
> > When trying to enable PTP on vsc8574 and vsc8572 it is not working even
> > if the function vsc8584_ptp_init it says that it has support for PHY
> > timestamping. It is not working because there is no PTP device.
> > So, to fix this make sure to create a PTP device also for this PHYs as
> > they have the same PTP IP as the other vsc PHYs.
> 
> May be useful to proof read your commit message, or run it thru 
> a grammar checker. Copy & paste into a Google Doc would be enough..

I agree, and I did not understand the problem from the commit message.

I would suggest something like below (maybe not identical).

The PTP initialization is two-step: first we have vsc8584_ptp_probe_once() /
vsc8584_ptp_probe() at probe() time, then we have vsc8584_ptp_init() at
config_init() time.

For VSC8574 and VSC8572, the PTP initialization is incomplete. We are
making the second step without having previously made the first one.
This means, for example, that ptp_clock_register() is never called.

Nothing crashes as a result of this, but it is unexpected that some PHY
generations have PTP functionality exposed by the driver and some do
not, even though they share the same PTP clock IP.

> Regarding the patch the new function looks like a spitting image 
> of vsc8584_probe(), minus the revision check.
> -- 
> pw-bot: cr

Also, even without this patch, vsc8574_probe() and vsc8584_probe() are
structurally very similar and could use some consolidation.

Would it make sense to create a static int vsc8531_probe_common(struct
phy_device *phydev, bool ptp) and call it from multiple wrappers? The
VSC8584_REVB check can go in the vsc8584_probe() wrapper. The "size_t
priv_size" argument of devm_phy_package_join() can be set based on the
"bool ptp" argument, because struct vsc85xx_shared_private is used only
in PTP code.

You can make a preparatory change in 'net' patch sets, even without
a Fixes: tag, if you clearly explain what it's for.