The PTP initialization is two-step. First part are the function
vsc8584_ptp_probe_once() and vsc8584_ptp_probe() at probe time which
initialize the locks, queues, creates the PTP device. The second part is
the function vsc8584_ptp_init() at config_init() time which initialize
PTP in the HW.

For VSC8574 and VSC8572, the PTP initialization is incomplete. It is
missing the first part but it makes the second part. Meaning that the
ptp_clock_register() is never called.

There is no crash without the first part when enabling PTP but this is
unexpected because some PHys have PTP functionality exposed by the
driver and some don't even though they share the same PTP clock PTP.

Fixes: 774626fa440e ("net: phy: mscc: Add PTP support for 2 more VSC PHYs")
Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
---
 drivers/net/phy/mscc/mscc_main.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/phy/mscc/mscc_main.c b/drivers/net/phy/mscc/mscc_main.c
index d05f6ed052ad0..90b62b8fd4af6 100644
--- a/drivers/net/phy/mscc/mscc_main.c
+++ b/drivers/net/phy/mscc/mscc_main.c
@@ -2613,7 +2613,7 @@ static struct phy_driver vsc85xx_driver[] = {
 	.suspend	= &genphy_suspend,
 	.resume		= &genphy_resume,
 	.remove		= &vsc85xx_remove,
-	.probe		= &vsc8574_probe,
+	.probe		= &vsc8584_probe,
 	.set_wol	= &vsc85xx_wol_set,
 	.get_wol	= &vsc85xx_wol_get,
 	.get_tunable	= &vsc85xx_get_tunable,
@@ -2636,12 +2636,12 @@ static struct phy_driver vsc85xx_driver[] = {
 	.config_aneg    = &vsc85xx_config_aneg,
 	.aneg_done	= &genphy_aneg_done,
 	.read_status	= &vsc85xx_read_status,
-	.handle_interrupt = vsc85xx_handle_interrupt,
+	.handle_interrupt = vsc8584_handle_interrupt,
 	.config_intr    = &vsc85xx_config_intr,
 	.suspend	= &genphy_suspend,
 	.resume		= &genphy_resume,
 	.remove		= &vsc85xx_remove,
-	.probe		= &vsc8574_probe,
+	.probe		= &vsc8584_probe,
 	.set_wol	= &vsc85xx_wol_set,
 	.get_wol	= &vsc85xx_wol_get,
 	.get_tunable	= &vsc85xx_get_tunable,
-- 
2.34.1

On Mon, Sep 29, 2025 at 11:13:02AM +0200, Horatiu Vultur wrote:
> The PTP initialization is two-step. First part are the function
> vsc8584_ptp_probe_once() and vsc8584_ptp_probe() at probe time which
> initialize the locks, queues, creates the PTP device. The second part is
> the function vsc8584_ptp_init() at config_init() time which initialize
> PTP in the HW.

So, to summarise, you register the PTP clock at probe time? At that
point, you get a /dev/ptp* device. Is that device functional without
the config_init() initialisation?

I would hope it is, because one of the fundamental concepts in kernel
programming is... don't publish devices to userspace that are not
completely ready for operation.

Conversely, don't tear down a published device without first
unpublishing it from userspace.

Thanks.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

The 09/29/2025 14:33, Russell King (Oracle) wrote:

Hi Russell,

> 
> On Mon, Sep 29, 2025 at 11:13:02AM +0200, Horatiu Vultur wrote:
> > The PTP initialization is two-step. First part are the function
> > vsc8584_ptp_probe_once() and vsc8584_ptp_probe() at probe time which
> > initialize the locks, queues, creates the PTP device. The second part is
> > the function vsc8584_ptp_init() at config_init() time which initialize
> > PTP in the HW.
> 
> So, to summarise, you register the PTP clock at probe time? At that
> point, you get a /dev/ptp* device. Is that device functional without
> the config_init() initialisation?

That is correct. I register the PTP clock at probe time.
It is not fully functional, for example calling "phc_ctl /dev/ptp0 get", I
always get "clock time is 0.000000000 or Thu Jan  1 00:00:00 1970"

# phc_ctl /dev/ptp0 get
phc_ctl[414.412]: clock time is 0.000000000 or Thu Jan  1 00:00:00 1970

# phc_ctl /dev/ptp0 get
phc_ctl[418.453]: clock time is 0.000000000 or Thu Jan  1 00:00:00 1970

Then after setting up, I can see the time increasing:

# phc_ctl /dev/ptp0 get
phc_ctl[511.841]: clock time is 2.232610928 or Thu Jan  1 00:00:02 1970

# phc_ctl /dev/ptp0 get
phc_ctl[515.113]: clock time is 5.504853240 or Thu Jan  1 00:00:05 1970

> 
> I would hope it is, because one of the fundamental concepts in kernel
> programming is... don't publish devices to userspace that are not
> completely ready for operation.
> 
> Conversely, don't tear down a published device without first
> unpublishing it from userspace.

Thanks for reminding me about this. Unfortunately, the same behaviour is for
all the devices that this driver is covering. So, then maybe I would
need to create a patch in the future to change this to have the correct
behaviour.

> 
> Thanks.
> 
> --
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

-- 
/Horatiu