[PATCH net-next v2 0/2] Add Marvell PHY PTP support

Kory Maincent posted 2 patches 10 months, 1 week ago
MAINTAINERS                                        |   2 +-
drivers/net/phy/Kconfig                            |  23 +-
drivers/net/phy/Makefile                           |   5 +-
drivers/net/phy/marvell/Kconfig                    |  35 +
drivers/net/phy/marvell/Makefile                   |   7 +
drivers/net/phy/{ => marvell}/marvell-88q2xxx.c    |   0
drivers/net/phy/{ => marvell}/marvell-88x2222.c    |   0
drivers/net/phy/{ => marvell}/marvell10g.c         |   0
.../net/phy/{marvell.c => marvell/marvell_main.c}  |  18 +-
drivers/net/phy/marvell/marvell_ptp.c              | 725 +++++++++++++++++++++
drivers/net/phy/marvell/marvell_ptp.h              |  26 +
drivers/net/phy/marvell/marvell_tai.c              | 279 ++++++++
drivers/net/phy/marvell/marvell_tai.h              |  36 +
13 files changed, 1128 insertions(+), 28 deletions(-)
[PATCH net-next v2 0/2] Add Marvell PHY PTP support
Posted by Kory Maincent 10 months, 1 week ago
Add PTP basic support for Marvell 88E151x PHYs.

Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>
---
Kory Maincent (1):
      net: phy: Move Marvell PHY drivers to its own subdirectory

Russell King (1):
      net: phy: Add Marvell PHY PTP support

 MAINTAINERS                                        |   2 +-
 drivers/net/phy/Kconfig                            |  23 +-
 drivers/net/phy/Makefile                           |   5 +-
 drivers/net/phy/marvell/Kconfig                    |  35 +
 drivers/net/phy/marvell/Makefile                   |   7 +
 drivers/net/phy/{ => marvell}/marvell-88q2xxx.c    |   0
 drivers/net/phy/{ => marvell}/marvell-88x2222.c    |   0
 drivers/net/phy/{ => marvell}/marvell10g.c         |   0
 .../net/phy/{marvell.c => marvell/marvell_main.c}  |  18 +-
 drivers/net/phy/marvell/marvell_ptp.c              | 725 +++++++++++++++++++++
 drivers/net/phy/marvell/marvell_ptp.h              |  26 +
 drivers/net/phy/marvell/marvell_tai.c              | 279 ++++++++
 drivers/net/phy/marvell/marvell_tai.h              |  36 +
 13 files changed, 1128 insertions(+), 28 deletions(-)
---
base-commit: e9363f5834ade5fa092751ed42080edfdf4ff93e
change-id: 20250124-feature_marvell_ptp-f8730c6a94c2

Best regards,
-- 
Köry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

Re: [PATCH net-next v2 0/2] Add Marvell PHY PTP support
Posted by Russell King (Oracle) 10 months, 1 week ago
On Mon, Apr 07, 2025 at 04:02:59PM +0200, Kory Maincent wrote:
> Add PTP basic support for Marvell 88E151x PHYs.
> 
> Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>

Is the PTP selection stuff actually sorted now? Last time I tested it
after it having been merged into the kernel for a while, it didn't work,
and I reported that fact. You haven't told me that you now expect it to
work.

I don't want this merged until such time that we can be sure that MVPP2
platforms can continue using the MVPP2 PTP support, which to me means
that the PTP selection between a MAC and PHY needs to work.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH net-next v2 0/2] Add Marvell PHY PTP support
Posted by Kory Maincent 10 months, 1 week ago
On Mon, 7 Apr 2025 17:02:28 +0100
"Russell King (Oracle)" <linux@armlinux.org.uk> wrote:

> On Mon, Apr 07, 2025 at 04:02:59PM +0200, Kory Maincent wrote:
> > Add PTP basic support for Marvell 88E151x PHYs.
> > 
> > Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>  
> 
> Is the PTP selection stuff actually sorted now? Last time I tested it
> after it having been merged into the kernel for a while, it didn't work,
> and I reported that fact. You haven't told me that you now expect it to
> work.

The last part of the series, the PTP selection support wasn't merged when you
tested it, although the default PTP choice that causes your regression was
merged.
Now it is fully merged, even the ethtool support.
https://lore.kernel.org/netdev/mjn6eeo6lestvo6z3utb7aemufmfhn5alecyoaz46dt4pwjn6v@4aaaz6qpqd4b/

The only issue is the rtln warning from the phy_detach function. About it, I
have already sent you the work I have done throwing ASSERT_RTNL in phy_detach.
Maybe I should resend it as RFC.

> I don't want this merged until such time that we can be sure that MVPP2
> platforms can continue using the MVPP2 PTP support, which to me means
> that the PTP selection between a MAC and PHY needs to work.

It should works, the default PTP will be the MAC PTP and you will be able to
select the current PTP between MAC and PHY with the following command:
# ethtool --set-hwtimestamp-cfg eth0 index 0 qualifier precise
Time stamping configuration for eth0:
Hardware timestamp provider index: 0
Hardware timestamp provider qualifier: Precise (IEEE 1588 quality)
Hardware Transmit Timestamp Mode:
	off
Hardware Receive Filter Mode:
	none
Hardware Flags: none
# ethtool --set-hwtimestamp-cfg eth0 index 1 qualifier precise
Time stamping configuration for eth0:
Hardware timestamp provider index: 1
Hardware timestamp provider qualifier: Precise (IEEE 1588 quality)
Hardware Transmit Timestamp Mode:
	off
Hardware Receive Filter Mode:
	none
Hardware Flags: none

You can list the PTPs with the dump command:
# ethtool --show-time-stamping "*"

You will need to stop phc2sys and ptp4l during these change as linuxptp may
face some issues during the PTP change.

Regards,
-- 
Köry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com
Re: [PATCH net-next v2 0/2] Add Marvell PHY PTP support
Posted by Russell King (Oracle) 10 months, 1 week ago
On Mon, Apr 07, 2025 at 06:20:28PM +0200, Kory Maincent wrote:
> On Mon, 7 Apr 2025 17:02:28 +0100
> "Russell King (Oracle)" <linux@armlinux.org.uk> wrote:
> 
> > On Mon, Apr 07, 2025 at 04:02:59PM +0200, Kory Maincent wrote:
> > > Add PTP basic support for Marvell 88E151x PHYs.
> > > 
> > > Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>  
> > 
> > Is the PTP selection stuff actually sorted now? Last time I tested it
> > after it having been merged into the kernel for a while, it didn't work,
> > and I reported that fact. You haven't told me that you now expect it to
> > work.
> 
> The last part of the series, the PTP selection support wasn't merged when you
> tested it, although the default PTP choice that causes your regression was
> merged.
> Now it is fully merged, even the ethtool support.
> https://lore.kernel.org/netdev/mjn6eeo6lestvo6z3utb7aemufmfhn5alecyoaz46dt4pwjn6v@4aaaz6qpqd4b/
> 
> The only issue is the rtln warning from the phy_detach function. About it, I
> have already sent you the work I have done throwing ASSERT_RTNL in phy_detach.
> Maybe I should resend it as RFC.
> 
> > I don't want this merged until such time that we can be sure that MVPP2
> > platforms can continue using the MVPP2 PTP support, which to me means
> > that the PTP selection between a MAC and PHY needs to work.
> 
> It should works, the default PTP will be the MAC PTP and you will be able to
> select the current PTP between MAC and PHY with the following command:
> # ethtool --set-hwtimestamp-cfg eth0 index 0 qualifier precise
> Time stamping configuration for eth0:
> Hardware timestamp provider index: 0
> Hardware timestamp provider qualifier: Precise (IEEE 1588 quality)
> Hardware Transmit Timestamp Mode:
> 	off
> Hardware Receive Filter Mode:
> 	none
> Hardware Flags: none
> # ethtool --set-hwtimestamp-cfg eth0 index 1 qualifier precise
> Time stamping configuration for eth0:
> Hardware timestamp provider index: 1
> Hardware timestamp provider qualifier: Precise (IEEE 1588 quality)
> Hardware Transmit Timestamp Mode:
> 	off
> Hardware Receive Filter Mode:
> 	none
> Hardware Flags: none
> 
> You can list the PTPs with the dump command:
> # ethtool --show-time-stamping "*"
> 
> You will need to stop phc2sys and ptp4l during these change as linuxptp may
> face some issues during the PTP change.

I'm preferring to my emails in connection with:

https://lore.kernel.org/r/ZzTMhGDoi3WcY6MR@shell.armlinux.org.uk

when I tested your work last time, it seemed that what was merged hadn't
even been tested. In the last email, you said you'd look into it, but I
didn't hear anything further. Have the problems I reported been
addressed?

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH net-next v2 0/2] Add Marvell PHY PTP support
Posted by Kory Maincent 10 months, 1 week ago
On Mon, 7 Apr 2025 17:32:43 +0100
"Russell King (Oracle)" <linux@armlinux.org.uk> wrote:

> On Mon, Apr 07, 2025 at 06:20:28PM +0200, Kory Maincent wrote:
> > On Mon, 7 Apr 2025 17:02:28 +0100
> > "Russell King (Oracle)" <linux@armlinux.org.uk> wrote:
> >   
> > > On Mon, Apr 07, 2025 at 04:02:59PM +0200, Kory Maincent wrote:  
>  [...]  
> > > 
> > > Is the PTP selection stuff actually sorted now? Last time I tested it
> > > after it having been merged into the kernel for a while, it didn't work,
> > > and I reported that fact. You haven't told me that you now expect it to
> > > work.  
> > 
> > The last part of the series, the PTP selection support wasn't merged when
> > you tested it, although the default PTP choice that causes your regression
> > was merged.
> > Now it is fully merged, even the ethtool support.
> > https://lore.kernel.org/netdev/mjn6eeo6lestvo6z3utb7aemufmfhn5alecyoaz46dt4pwjn6v@4aaaz6qpqd4b/
> > 
> > The only issue is the rtln warning from the phy_detach function. About it, I
> > have already sent you the work I have done throwing ASSERT_RTNL in
> > phy_detach. Maybe I should resend it as RFC.
> >   
> > > I don't want this merged until such time that we can be sure that MVPP2
> > > platforms can continue using the MVPP2 PTP support, which to me means
> > > that the PTP selection between a MAC and PHY needs to work.  
> > 
> > It should works, the default PTP will be the MAC PTP and you will be able to
> > select the current PTP between MAC and PHY with the following command:
> > # ethtool --set-hwtimestamp-cfg eth0 index 0 qualifier precise
> > Time stamping configuration for eth0:
> > Hardware timestamp provider index: 0
> > Hardware timestamp provider qualifier: Precise (IEEE 1588 quality)
> > Hardware Transmit Timestamp Mode:
> > 	off
> > Hardware Receive Filter Mode:
> > 	none
> > Hardware Flags: none
> > # ethtool --set-hwtimestamp-cfg eth0 index 1 qualifier precise
> > Time stamping configuration for eth0:
> > Hardware timestamp provider index: 1
> > Hardware timestamp provider qualifier: Precise (IEEE 1588 quality)
> > Hardware Transmit Timestamp Mode:
> > 	off
> > Hardware Receive Filter Mode:
> > 	none
> > Hardware Flags: none
> > 
> > You can list the PTPs with the dump command:
> > # ethtool --show-time-stamping "*"
> > 
> > You will need to stop phc2sys and ptp4l during these change as linuxptp may
> > face some issues during the PTP change.  
> 
> I'm preferring to my emails in connection with:
> 
> https://lore.kernel.org/r/ZzTMhGDoi3WcY6MR@shell.armlinux.org.uk
> 
> when I tested your work last time, it seemed that what was merged hadn't
> even been tested. In the last email, you said you'd look into it, but I
> didn't hear anything further. Have the problems I reported been
> addressed?

It wasn't merged it was 19th version and it worked and was tested, but not
with the best development design. I have replied to you that I will do some
change in v20 to address this.
https://lore.kernel.org/all/20241113171443.697ac278@kmaincent-XPS-13-7390/

It gets finally merged in v21.

Regards,
-- 
Köry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com
Re: [PATCH net-next v2 0/2] Add Marvell PHY PTP support
Posted by Russell King (Oracle) 10 months ago
On Mon, Apr 07, 2025 at 06:39:14PM +0200, Kory Maincent wrote:
> On Mon, 7 Apr 2025 17:32:43 +0100
> "Russell King (Oracle)" <linux@armlinux.org.uk> wrote:
> 
> > On Mon, Apr 07, 2025 at 06:20:28PM +0200, Kory Maincent wrote:
> > > On Mon, 7 Apr 2025 17:02:28 +0100
> > > "Russell King (Oracle)" <linux@armlinux.org.uk> wrote:
> > >   
> > > > On Mon, Apr 07, 2025 at 04:02:59PM +0200, Kory Maincent wrote:  
> >  [...]  
> > > > 
> > > > Is the PTP selection stuff actually sorted now? Last time I tested it
> > > > after it having been merged into the kernel for a while, it didn't work,
> > > > and I reported that fact. You haven't told me that you now expect it to
> > > > work.  
> > > 
> > > The last part of the series, the PTP selection support wasn't merged when
> > > you tested it, although the default PTP choice that causes your regression
> > > was merged.
> > > Now it is fully merged, even the ethtool support.
> > > https://lore.kernel.org/netdev/mjn6eeo6lestvo6z3utb7aemufmfhn5alecyoaz46dt4pwjn6v@4aaaz6qpqd4b/
> > > 
> > > The only issue is the rtln warning from the phy_detach function. About it, I
> > > have already sent you the work I have done throwing ASSERT_RTNL in
> > > phy_detach. Maybe I should resend it as RFC.
> > >   
> > > > I don't want this merged until such time that we can be sure that MVPP2
> > > > platforms can continue using the MVPP2 PTP support, which to me means
> > > > that the PTP selection between a MAC and PHY needs to work.  
> > > 
> > > It should works, the default PTP will be the MAC PTP and you will be able to
> > > select the current PTP between MAC and PHY with the following command:
> > > # ethtool --set-hwtimestamp-cfg eth0 index 0 qualifier precise
> > > Time stamping configuration for eth0:
> > > Hardware timestamp provider index: 0
> > > Hardware timestamp provider qualifier: Precise (IEEE 1588 quality)
> > > Hardware Transmit Timestamp Mode:
> > > 	off
> > > Hardware Receive Filter Mode:
> > > 	none
> > > Hardware Flags: none
> > > # ethtool --set-hwtimestamp-cfg eth0 index 1 qualifier precise
> > > Time stamping configuration for eth0:
> > > Hardware timestamp provider index: 1
> > > Hardware timestamp provider qualifier: Precise (IEEE 1588 quality)
> > > Hardware Transmit Timestamp Mode:
> > > 	off
> > > Hardware Receive Filter Mode:
> > > 	none
> > > Hardware Flags: none
> > > 
> > > You can list the PTPs with the dump command:
> > > # ethtool --show-time-stamping "*"
> > > 
> > > You will need to stop phc2sys and ptp4l during these change as linuxptp may
> > > face some issues during the PTP change.  
> > 
> > I'm preferring to my emails in connection with:
> > 
> > https://lore.kernel.org/r/ZzTMhGDoi3WcY6MR@shell.armlinux.org.uk
> > 
> > when I tested your work last time, it seemed that what was merged hadn't
> > even been tested. In the last email, you said you'd look into it, but I
> > didn't hear anything further. Have the problems I reported been
> > addressed?
> 
> It wasn't merged it was 19th version and it worked and was tested, but not
> with the best development design. I have replied to you that I will do some
> change in v20 to address this.
> https://lore.kernel.org/all/20241113171443.697ac278@kmaincent-XPS-13-7390/
> 
> It gets finally merged in v21.

Okay, so I'm pleased to report that this now works on the Macchiatobin:

# ./ynl/cli.py --spec netlink/specs/ethtool.yaml --no-schema --dump tsinfo-get --json '{"header":{"dev-name":"eth2"}}' 
[{'header': {'dev-index': 5, 'dev-name': 'eth2'},
  'hwtstamp-provider': {'index': 2, 'qualifier': 0},
  'phc-index': 2,
  'rx-filters': {'bits': {'bit': [{'index': 0, 'name': 'none'},
                                  {'index': 1, 'name': 'all'}]},
                 'nomask': True,
                 'size': 16},
  'timestamping': {'bits': {'bit': [{'index': 0, 'name': 'hardware-transmit'},
                                    {'index': 1, 'name': 'software-transmit'},
                                    {'index': 2, 'name': 'hardware-receive'},
                                    {'index': 3, 'name': 'software-receive'},
                                    {'index': 4,
                                     'name': 'software-system-clock'},
                                    {'index': 6,
                                     'name': 'hardware-raw-clock'}]},
                   'nomask': True,
                   'size': 18},
  'tx-types': {'bits': {'bit': [{'index': 0, 'name': 'off'},
                                {'index': 1, 'name': 'on'},
                                {'index': 2, 'name': 'onestep-sync'},
                                {'index': 3, 'name': 'onestep-p2p'}]},
               'nomask': True,
               'size': 4}},
 {'header': {'dev-index': 5, 'dev-name': 'eth2'},
  'hwtstamp-provider': {'index': 0, 'qualifier': 0},
  'phc-index': 0,
  'rx-filters': {'bits': {'bit': [{'index': 0, 'name': 'none'},
                                  {'index': 2, 'name': 'some'}]},
                 'nomask': True,
                 'size': 16},
  'timestamping': {'bits': {'bit': [{'index': 0, 'name': 'hardware-transmit'},
                                    {'index': 2, 'name': 'hardware-receive'},
                                    {'index': 3, 'name': 'software-receive'},
                                    {'index': 4,
                                     'name': 'software-system-clock'},
                                    {'index': 6,
                                     'name': 'hardware-raw-clock'}]},
                   'nomask': True,
                   'size': 18},
  'tx-types': {'bits': {'bit': [{'index': 0, 'name': 'off'},
                                {'index': 1, 'name': 'on'}]},
               'nomask': True,
               'size': 4}}]

where phc 2 is the mvpp2 clock, and phc 0 is the PHY.

# ethtool -T eth2
Time stamping parameters for eth2:
Capabilities:
        hardware-transmit
        software-transmit
        hardware-receive
        software-receive
        software-system-clock
        hardware-raw-clock
PTP Hardware Clock: 2
Hardware Transmit Timestamp Modes:
        off
        on
        onestep-sync
        onestep-p2p
Hardware Receive Filter Modes:
        none
        all

So I guess that means that by default it's using PHC 2, and thus using
the MVPP2 PTP implementation - which is good, it means that when we add
Marvell PHY support, this won't switch to the PHY implementation.

Now, testing ethtool:

$ ./ethtool --get-hwtimestamp-cfg eth2
netlink error: Operation not supported

Using ynl:

# ./ynl/cli.py --spec netlink/specs/ethtool.yaml --no-schema --dump tsconfig-get --json '{"header":{"dev-name":"eth2"}}'
[]

So, It's better, something still isn't correct as there's no
configuration. Maybe mvpp2 needs updating first? If that's the case,
then we're not yet in a position to merge PHY PTP support.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH net-next v2 0/2] Add Marvell PHY PTP support
Posted by Kory Maincent 10 months ago
On Tue, 8 Apr 2025 21:38:19 +0100
"Russell King (Oracle)" <linux@armlinux.org.uk> wrote:

> On Mon, Apr 07, 2025 at 06:39:14PM +0200, Kory Maincent wrote:
> > On Mon, 7 Apr 2025 17:32:43 +0100
> > "Russell King (Oracle)" <linux@armlinux.org.uk> wrote:
> > > I'm preferring to my emails in connection with:
> > > 
> > > https://lore.kernel.org/r/ZzTMhGDoi3WcY6MR@shell.armlinux.org.uk
> > > 
> > > when I tested your work last time, it seemed that what was merged hadn't
> > > even been tested. In the last email, you said you'd look into it, but I
> > > didn't hear anything further. Have the problems I reported been
> > > addressed?  
> > 
> > It wasn't merged it was 19th version and it worked and was tested, but not
> > with the best development design. I have replied to you that I will do some
> > change in v20 to address this.
> > https://lore.kernel.org/all/20241113171443.697ac278@kmaincent-XPS-13-7390/
> > 
> > It gets finally merged in v21.  
> 
> Okay, so I'm pleased to report that this now works on the Macchiatobin:
> 
> where phc 2 is the mvpp2 clock, and phc 0 is the PHY.

Great, thank you for the testing!

> 
> # ethtool -T eth2
> Time stamping parameters for eth2:
> Capabilities:
>         hardware-transmit
>         software-transmit
>         hardware-receive
>         software-receive
>         software-system-clock
>         hardware-raw-clock
> PTP Hardware Clock: 2
> Hardware Transmit Timestamp Modes:
>         off
>         on
>         onestep-sync
>         onestep-p2p
> Hardware Receive Filter Modes:
>         none
>         all
> 
> So I guess that means that by default it's using PHC 2, and thus using
> the MVPP2 PTP implementation - which is good, it means that when we add
> Marvell PHY support, this won't switch to the PHY implementation.

Yes.

> 
> Now, testing ethtool:
> 
> $ ./ethtool --get-hwtimestamp-cfg eth2
> netlink error: Operation not supported
> 
> Using ynl:
> 
> # ./ynl/cli.py --spec netlink/specs/ethtool.yaml --no-schema --dump
> tsconfig-get --json '{"header":{"dev-name":"eth2"}}' []
> 
> So, It's better, something still isn't correct as there's no
> configuration. Maybe mvpp2 needs updating first? If that's the case,
> then we're not yet in a position to merge PHY PTP support.

Indeed mvpp2 has not been update to support the ndo_hwtstamp_get/set NDOs.
Vlad had made some work to update all net drivers to these NDOs but he never
send it mainline:
https://github.com/vladimiroltean/linux/commits/ndo-hwtstamp-v9

I have already try to ping him on this but without success.
Vlad any idea on when you could send your series upstream?

Regards,
-- 
Köry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com
Re: [PATCH net-next v2 0/2] Add Marvell PHY PTP support
Posted by Russell King (Oracle) 10 months ago
On Wed, Apr 09, 2025 at 10:31:30AM +0200, Kory Maincent wrote:
> On Tue, 8 Apr 2025 21:38:19 +0100
> "Russell King (Oracle)" <linux@armlinux.org.uk> wrote:
> 
> > On Mon, Apr 07, 2025 at 06:39:14PM +0200, Kory Maincent wrote:
> > > On Mon, 7 Apr 2025 17:32:43 +0100
> > > "Russell King (Oracle)" <linux@armlinux.org.uk> wrote:
> > > > I'm preferring to my emails in connection with:
> > > > 
> > > > https://lore.kernel.org/r/ZzTMhGDoi3WcY6MR@shell.armlinux.org.uk
> > > > 
> > > > when I tested your work last time, it seemed that what was merged hadn't
> > > > even been tested. In the last email, you said you'd look into it, but I
> > > > didn't hear anything further. Have the problems I reported been
> > > > addressed?  
> > > 
> > > It wasn't merged it was 19th version and it worked and was tested, but not
> > > with the best development design. I have replied to you that I will do some
> > > change in v20 to address this.
> > > https://lore.kernel.org/all/20241113171443.697ac278@kmaincent-XPS-13-7390/
> > > 
> > > It gets finally merged in v21.  
> > 
> > Okay, so I'm pleased to report that this now works on the Macchiatobin:
> > 
> > where phc 2 is the mvpp2 clock, and phc 0 is the PHY.
> 
> Great, thank you for the testing!
> 
> > 
> > # ethtool -T eth2
> > Time stamping parameters for eth2:
> > Capabilities:
> >         hardware-transmit
> >         software-transmit
> >         hardware-receive
> >         software-receive
> >         software-system-clock
> >         hardware-raw-clock
> > PTP Hardware Clock: 2
> > Hardware Transmit Timestamp Modes:
> >         off
> >         on
> >         onestep-sync
> >         onestep-p2p
> > Hardware Receive Filter Modes:
> >         none
> >         all
> > 
> > So I guess that means that by default it's using PHC 2, and thus using
> > the MVPP2 PTP implementation - which is good, it means that when we add
> > Marvell PHY support, this won't switch to the PHY implementation.
> 
> Yes.
> 
> > 
> > Now, testing ethtool:
> > 
> > $ ./ethtool --get-hwtimestamp-cfg eth2
> > netlink error: Operation not supported
> > 
> > Using ynl:
> > 
> > # ./ynl/cli.py --spec netlink/specs/ethtool.yaml --no-schema --dump
> > tsconfig-get --json '{"header":{"dev-name":"eth2"}}' []
> > 
> > So, It's better, something still isn't correct as there's no
> > configuration. Maybe mvpp2 needs updating first? If that's the case,
> > then we're not yet in a position to merge PHY PTP support.
> 
> Indeed mvpp2 has not been update to support the ndo_hwtstamp_get/set NDOs.
> Vlad had made some work to update all net drivers to these NDOs but he never
> send it mainline:
> https://github.com/vladimiroltean/linux/commits/ndo-hwtstamp-v9
> 
> I have already try to ping him on this but without success.
> Vlad any idea on when you could send your series upstream?

Right, and that means that the kernel is not yet ready to support
Marvell PHY PTP, because all the pre-requisits to avoid breaking
mvpp2 have not yet been merged.

So that's a NAK on this series from me.

I'd have thought this would be obvious given my well known stance
on why I haven't merged Marvell PHY PTP support before.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH net-next v2 0/2] Add Marvell PHY PTP support
Posted by Kory Maincent 10 months ago
On Wed, 9 Apr 2025 09:35:59 +0100
"Russell King (Oracle)" <linux@armlinux.org.uk> wrote:

> On Wed, Apr 09, 2025 at 10:31:30AM +0200, Kory Maincent wrote:
> > On Tue, 8 Apr 2025 21:38:19 +0100
> > "Russell King (Oracle)" <linux@armlinux.org.uk> wrote:
> >   
> > > On Mon, Apr 07, 2025 at 06:39:14PM +0200, Kory Maincent wrote:  
>  [...]  
>  [...]  
>  [...]  
> > > 
> > > Okay, so I'm pleased to report that this now works on the Macchiatobin:
> > > 
> > > where phc 2 is the mvpp2 clock, and phc 0 is the PHY.  
> > 
> > Great, thank you for the testing!
> >   
> > > 
> > > # ethtool -T eth2
> > > Time stamping parameters for eth2:
> > > Capabilities:
> > >         hardware-transmit
> > >         software-transmit
> > >         hardware-receive
> > >         software-receive
> > >         software-system-clock
> > >         hardware-raw-clock
> > > PTP Hardware Clock: 2
> > > Hardware Transmit Timestamp Modes:
> > >         off
> > >         on
> > >         onestep-sync
> > >         onestep-p2p
> > > Hardware Receive Filter Modes:
> > >         none
> > >         all
> > > 
> > > So I guess that means that by default it's using PHC 2, and thus using
> > > the MVPP2 PTP implementation - which is good, it means that when we add
> > > Marvell PHY support, this won't switch to the PHY implementation.  
> > 
> > Yes.
> >   
> > > 
> > > Now, testing ethtool:
> > > 
> > > $ ./ethtool --get-hwtimestamp-cfg eth2
> > > netlink error: Operation not supported
> > > 
> > > Using ynl:
> > > 
> > > # ./ynl/cli.py --spec netlink/specs/ethtool.yaml --no-schema --dump
> > > tsconfig-get --json '{"header":{"dev-name":"eth2"}}' []
> > > 
> > > So, It's better, something still isn't correct as there's no
> > > configuration. Maybe mvpp2 needs updating first? If that's the case,
> > > then we're not yet in a position to merge PHY PTP support.  
> > 
> > Indeed mvpp2 has not been update to support the ndo_hwtstamp_get/set NDOs.
> > Vlad had made some work to update all net drivers to these NDOs but he never
> > send it mainline:
> > https://github.com/vladimiroltean/linux/commits/ndo-hwtstamp-v9
> > 
> > I have already try to ping him on this but without success.
> > Vlad any idea on when you could send your series upstream?  
> 
> Right, and that means that the kernel is not yet ready to support
> Marvell PHY PTP, because all the pre-requisits to avoid breaking
> mvpp2 have not yet been merged.

Still I don't understand how this break mvpp2.
As you just tested this won't switch to the PHY PTP implementation. The old
usage of using SIOCG/SHWTSTAMP will still work, you simply won't be able to use
the new netlink feature of switching between the two PTP as long as
ndo_hwtstamp_get/set NDOs is not supported in mvpp2.

Regards,
-- 
Köry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com
Re: [PATCH net-next v2 0/2] Add Marvell PHY PTP support
Posted by Russell King (Oracle) 10 months ago
On Wed, Apr 09, 2025 at 10:46:37AM +0200, Kory Maincent wrote:
> On Wed, 9 Apr 2025 09:35:59 +0100
> "Russell King (Oracle)" <linux@armlinux.org.uk> wrote:
> 
> > On Wed, Apr 09, 2025 at 10:31:30AM +0200, Kory Maincent wrote:
> > > On Tue, 8 Apr 2025 21:38:19 +0100
> > > "Russell King (Oracle)" <linux@armlinux.org.uk> wrote:
> > >   
> > > > On Mon, Apr 07, 2025 at 06:39:14PM +0200, Kory Maincent wrote:  
> >  [...]  
> >  [...]  
> >  [...]  
> > > > 
> > > > Okay, so I'm pleased to report that this now works on the Macchiatobin:
> > > > 
> > > > where phc 2 is the mvpp2 clock, and phc 0 is the PHY.  
> > > 
> > > Great, thank you for the testing!
> > >   
> > > > 
> > > > # ethtool -T eth2
> > > > Time stamping parameters for eth2:
> > > > Capabilities:
> > > >         hardware-transmit
> > > >         software-transmit
> > > >         hardware-receive
> > > >         software-receive
> > > >         software-system-clock
> > > >         hardware-raw-clock
> > > > PTP Hardware Clock: 2
> > > > Hardware Transmit Timestamp Modes:
> > > >         off
> > > >         on
> > > >         onestep-sync
> > > >         onestep-p2p
> > > > Hardware Receive Filter Modes:
> > > >         none
> > > >         all
> > > > 
> > > > So I guess that means that by default it's using PHC 2, and thus using
> > > > the MVPP2 PTP implementation - which is good, it means that when we add
> > > > Marvell PHY support, this won't switch to the PHY implementation.  
> > > 
> > > Yes.
> > >   
> > > > 
> > > > Now, testing ethtool:
> > > > 
> > > > $ ./ethtool --get-hwtimestamp-cfg eth2
> > > > netlink error: Operation not supported
> > > > 
> > > > Using ynl:
> > > > 
> > > > # ./ynl/cli.py --spec netlink/specs/ethtool.yaml --no-schema --dump
> > > > tsconfig-get --json '{"header":{"dev-name":"eth2"}}' []
> > > > 
> > > > So, It's better, something still isn't correct as there's no
> > > > configuration. Maybe mvpp2 needs updating first? If that's the case,
> > > > then we're not yet in a position to merge PHY PTP support.  
> > > 
> > > Indeed mvpp2 has not been update to support the ndo_hwtstamp_get/set NDOs.
> > > Vlad had made some work to update all net drivers to these NDOs but he never
> > > send it mainline:
> > > https://github.com/vladimiroltean/linux/commits/ndo-hwtstamp-v9
> > > 
> > > I have already try to ping him on this but without success.
> > > Vlad any idea on when you could send your series upstream?  
> > 
> > Right, and that means that the kernel is not yet ready to support
> > Marvell PHY PTP, because all the pre-requisits to avoid breaking
> > mvpp2 have not yet been merged.
> 
> Still I don't understand how this break mvpp2.
> As you just tested this won't switch to the PHY PTP implementation.

How do I know that from the output? Nothing in the output appears to
tells me which PTP implementation will be used.

Maybe you have some understanding that makes this obvious that I don't
have.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH net-next v2 0/2] Add Marvell PHY PTP support
Posted by Kory Maincent 10 months ago
On Wed, 9 Apr 2025 10:29:52 +0100
"Russell King (Oracle)" <linux@armlinux.org.uk> wrote:

> On Wed, Apr 09, 2025 at 10:46:37AM +0200, Kory Maincent wrote:
> > On Wed, 9 Apr 2025 09:35:59 +0100
> > "Russell King (Oracle)" <linux@armlinux.org.uk> wrote:

> > > Right, and that means that the kernel is not yet ready to support
> > > Marvell PHY PTP, because all the pre-requisits to avoid breaking
> > > mvpp2 have not yet been merged.  
> > 
> > Still I don't understand how this break mvpp2.
> > As you just tested this won't switch to the PHY PTP implementation.  
> 
> How do I know that from the output? Nothing in the output appears to
> tells me which PTP implementation will be used.
> 
> Maybe you have some understanding that makes this obvious that I don't
> have.

You are right there is no report of the PTP source device info in ethtool.
With all the design change of the PTP series this has not made through my brain
that we lost this information along the way.

You can still know the source like that but that's not the best.
# ls -l /sys/class/ptp

It will be easy to add the source name support in netlink but which names are
better report to the user?
- dev_name of the netdev->dev and phydev->mdio.dev?
  Maybe not the best naming for the phy PTP source
  (ff0d0000.ethernet-ffffffff:01)
- "PHY" + the PHY ID and "MAC" string?
- Another idea? 

Regards,
-- 
Köry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com
Re: [PATCH net-next v2 0/2] Add Marvell PHY PTP support
Posted by Maxime Chevallier 10 months ago
On Wed, 9 Apr 2025 14:23:09 +0200
Kory Maincent <kory.maincent@bootlin.com> wrote:

> On Wed, 9 Apr 2025 10:29:52 +0100
> "Russell King (Oracle)" <linux@armlinux.org.uk> wrote:
> 
> > On Wed, Apr 09, 2025 at 10:46:37AM +0200, Kory Maincent wrote:  
> > > On Wed, 9 Apr 2025 09:35:59 +0100
> > > "Russell King (Oracle)" <linux@armlinux.org.uk> wrote:  
> 
> > > > Right, and that means that the kernel is not yet ready to support
> > > > Marvell PHY PTP, because all the pre-requisits to avoid breaking
> > > > mvpp2 have not yet been merged.    
> > > 
> > > Still I don't understand how this break mvpp2.
> > > As you just tested this won't switch to the PHY PTP implementation.    
> > 
> > How do I know that from the output? Nothing in the output appears to
> > tells me which PTP implementation will be used.
> > 
> > Maybe you have some understanding that makes this obvious that I don't
> > have.  
> 
> You are right there is no report of the PTP source device info in ethtool.
> With all the design change of the PTP series this has not made through my brain
> that we lost this information along the way.
> 
> You can still know the source like that but that's not the best.
> # ls -l /sys/class/ptp
> 
> It will be easy to add the source name support in netlink but which names are
> better report to the user?
> - dev_name of the netdev->dev and phydev->mdio.dev?
>   Maybe not the best naming for the phy PTP source
>   (ff0d0000.ethernet-ffffffff:01)
> - "PHY" + the PHY ID and "MAC" string?

How about an enum instead of a string indicating the device type, and if
PHY, the phy_index ? (phy ID has another meaning :) )

Maxime
Re: [PATCH net-next v2 0/2] Add Marvell PHY PTP support
Posted by Kory Maincent 10 months ago
On Wed, 9 Apr 2025 14:46:54 +0200
Maxime Chevallier <maxime.chevallier@bootlin.com> wrote:

> On Wed, 9 Apr 2025 14:23:09 +0200
> Kory Maincent <kory.maincent@bootlin.com> wrote:
> 
> > On Wed, 9 Apr 2025 10:29:52 +0100
> > "Russell King (Oracle)" <linux@armlinux.org.uk> wrote:
> >   
> > > On Wed, Apr 09, 2025 at 10:46:37AM +0200, Kory Maincent wrote:    
>  [...]  
> >   
>  [...]  
>  [...]  
> > > 
> > > How do I know that from the output? Nothing in the output appears to
> > > tells me which PTP implementation will be used.
> > > 
> > > Maybe you have some understanding that makes this obvious that I don't
> > > have.    
> > 
> > You are right there is no report of the PTP source device info in ethtool.
> > With all the design change of the PTP series this has not made through my
> > brain that we lost this information along the way.
> > 
> > You can still know the source like that but that's not the best.
> > # ls -l /sys/class/ptp
> > 
> > It will be easy to add the source name support in netlink but which names
> > are better report to the user?
> > - dev_name of the netdev->dev and phydev->mdio.dev?
> >   Maybe not the best naming for the phy PTP source
> >   (ff0d0000.ethernet-ffffffff:01)
> > - "PHY" + the PHY ID and "MAC" string?  
> 
> How about an enum instead of a string indicating the device type, and if
> PHY, the phy_index ? (phy ID has another meaning :) )

This will raise the same question I faced during the ptp series mainline
process. In Linux, the PTP is managed through netdev or phylib API.
In case of a NIC all is managed through netdev. So if a NIC has a PTP at the PHY
layer how should we report that? As MAC PTP because it goes thought netdev, as
PHY PTP but without phyindex?
That's why maybe using netlink string could assure we won't have UAPI breakage
in the future due to weird cases.
What do you think?
 
Regards,
-- 
Köry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com
Re: [PATCH net-next v2 0/2] Add Marvell PHY PTP support
Posted by Maxime Chevallier 10 months ago
On Wed, 9 Apr 2025 16:49:20 +0200
Kory Maincent <kory.maincent@bootlin.com> wrote:

> On Wed, 9 Apr 2025 14:46:54 +0200
> Maxime Chevallier <maxime.chevallier@bootlin.com> wrote:
> 
> > On Wed, 9 Apr 2025 14:23:09 +0200
> > Kory Maincent <kory.maincent@bootlin.com> wrote:
> >   
> > > On Wed, 9 Apr 2025 10:29:52 +0100
> > > "Russell King (Oracle)" <linux@armlinux.org.uk> wrote:
> > >     
> > > > On Wed, Apr 09, 2025 at 10:46:37AM +0200, Kory Maincent wrote:      
> >  [...]    
> > >     
> >  [...]  
> >  [...]    
> > > > 
> > > > How do I know that from the output? Nothing in the output appears to
> > > > tells me which PTP implementation will be used.
> > > > 
> > > > Maybe you have some understanding that makes this obvious that I don't
> > > > have.      
> > > 
> > > You are right there is no report of the PTP source device info in ethtool.
> > > With all the design change of the PTP series this has not made through my
> > > brain that we lost this information along the way.
> > > 
> > > You can still know the source like that but that's not the best.
> > > # ls -l /sys/class/ptp
> > > 
> > > It will be easy to add the source name support in netlink but which names
> > > are better report to the user?
> > > - dev_name of the netdev->dev and phydev->mdio.dev?
> > >   Maybe not the best naming for the phy PTP source
> > >   (ff0d0000.ethernet-ffffffff:01)
> > > - "PHY" + the PHY ID and "MAC" string?    
> > 
> > How about an enum instead of a string indicating the device type, and if
> > PHY, the phy_index ? (phy ID has another meaning :) )  
> 
> This will raise the same question I faced during the ptp series mainline
> process. In Linux, the PTP is managed through netdev or phylib API.
> In case of a NIC all is managed through netdev. So if a NIC has a PTP at the PHY
> layer how should we report that? As MAC PTP because it goes thought netdev, as
> PHY PTP but without phyindex?

Are you referring to the case where the PHY is transparently handled by
the MAC driver (i.e. controlled through a firmware of some sort) ?

In such case, how do you even know that timestamping is done in a PHY,
as the kernel doesn't know the PHY even exists ? The
HWTSTAMP_SOURCE_XXX enum either says it's from PHYLIB or NETDEV. As
PHYs handled by firmwares don't go through phylib, I'd say reporting
"PHY with no index" won't be accurate.

In such case I'd probably expect the NIC driver to register several
hwtstamp_provider with different qualifiers

> That's why maybe using netlink string could assure we won't have UAPI breakage
> in the future due to weird cases.
> What do you think?

Well I'd say this is the same for enums, nothing prevents you from
adding more values to your enum ?

Maxime
Re: [PATCH net-next v2 0/2] Add Marvell PHY PTP support
Posted by Kory Maincent 10 months ago
On Wed, 9 Apr 2025 17:10:55 +0200
Maxime Chevallier <maxime.chevallier@bootlin.com> wrote:

> On Wed, 9 Apr 2025 16:49:20 +0200
> Kory Maincent <kory.maincent@bootlin.com> wrote:
> 
> > On Wed, 9 Apr 2025 14:46:54 +0200
> > Maxime Chevallier <maxime.chevallier@bootlin.com> wrote:
> >   
> > > On Wed, 9 Apr 2025 14:23:09 +0200
> > > Kory Maincent <kory.maincent@bootlin.com> wrote:

> > > How about an enum instead of a string indicating the device type, and if
> > > PHY, the phy_index ? (phy ID has another meaning :) )    
> > 
> > This will raise the same question I faced during the ptp series mainline
> > process. In Linux, the PTP is managed through netdev or phylib API.
> > In case of a NIC all is managed through netdev. So if a NIC has a PTP at
> > the PHY layer how should we report that? As MAC PTP because it goes thought
> > netdev, as PHY PTP but without phyindex?  
> 
> Are you referring to the case where the PHY is transparently handled by
> the MAC driver (i.e. controlled through a firmware of some sort) ?

Yes I was.
 
> In such case, how do you even know that timestamping is done in a PHY,
> as the kernel doesn't know the PHY even exists ? The
> HWTSTAMP_SOURCE_XXX enum either says it's from PHYLIB or NETDEV. As
> PHYs handled by firmwares don't go through phylib, I'd say reporting
> "PHY with no index" won't be accurate.
> 
> In such case I'd probably expect the NIC driver to register several
> hwtstamp_provider with different qualifiers
> 
> > That's why maybe using netlink string could assure we won't have UAPI
> > breakage in the future due to weird cases.
> > What do you think?  
> 
> Well I'd say this is the same for enums, nothing prevents you from
> adding more values to your enum ?

Thanks! I am ok with that.

Regards,
-- 
Köry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com
Re: [PATCH net-next v2 0/2] Add Marvell PHY PTP support
Posted by Vladimir Oltean 10 months ago
On Wed, Apr 09, 2025 at 09:35:59AM +0100, Russell King (Oracle) wrote:
> On Wed, Apr 09, 2025 at 10:31:30AM +0200, Kory Maincent wrote:
> > On Tue, 8 Apr 2025 21:38:19 +0100
> > "Russell King (Oracle)" <linux@armlinux.org.uk> wrote:
> > 
> > > On Mon, Apr 07, 2025 at 06:39:14PM +0200, Kory Maincent wrote:
> > > > On Mon, 7 Apr 2025 17:32:43 +0100
> > > > "Russell King (Oracle)" <linux@armlinux.org.uk> wrote:
> > > > > I'm preferring to my emails in connection with:
> > > > > 
> > > > > https://lore.kernel.org/r/ZzTMhGDoi3WcY6MR@shell.armlinux.org.uk
> > > > > 
> > > > > when I tested your work last time, it seemed that what was merged hadn't
> > > > > even been tested. In the last email, you said you'd look into it, but I
> > > > > didn't hear anything further. Have the problems I reported been
> > > > > addressed?  
> > > > 
> > > > It wasn't merged it was 19th version and it worked and was tested, but not
> > > > with the best development design. I have replied to you that I will do some
> > > > change in v20 to address this.
> > > > https://lore.kernel.org/all/20241113171443.697ac278@kmaincent-XPS-13-7390/
> > > > 
> > > > It gets finally merged in v21.  
> > > 
> > > Okay, so I'm pleased to report that this now works on the Macchiatobin:
> > > 
> > > where phc 2 is the mvpp2 clock, and phc 0 is the PHY.
> > 
> > Great, thank you for the testing!
> > 
> > > 
> > > # ethtool -T eth2
> > > Time stamping parameters for eth2:
> > > Capabilities:
> > >         hardware-transmit
> > >         software-transmit
> > >         hardware-receive
> > >         software-receive
> > >         software-system-clock
> > >         hardware-raw-clock
> > > PTP Hardware Clock: 2
> > > Hardware Transmit Timestamp Modes:
> > >         off
> > >         on
> > >         onestep-sync
> > >         onestep-p2p
> > > Hardware Receive Filter Modes:
> > >         none
> > >         all
> > > 
> > > So I guess that means that by default it's using PHC 2, and thus using
> > > the MVPP2 PTP implementation - which is good, it means that when we add
> > > Marvell PHY support, this won't switch to the PHY implementation.
> > 
> > Yes.
> > 
> > > 
> > > Now, testing ethtool:
> > > 
> > > $ ./ethtool --get-hwtimestamp-cfg eth2
> > > netlink error: Operation not supported
> > > 
> > > Using ynl:
> > > 
> > > # ./ynl/cli.py --spec netlink/specs/ethtool.yaml --no-schema --dump
> > > tsconfig-get --json '{"header":{"dev-name":"eth2"}}' []
> > > 
> > > So, It's better, something still isn't correct as there's no
> > > configuration. Maybe mvpp2 needs updating first? If that's the case,
> > > then we're not yet in a position to merge PHY PTP support.
> > 
> > Indeed mvpp2 has not been update to support the ndo_hwtstamp_get/set NDOs.
> > Vlad had made some work to update all net drivers to these NDOs but he never
> > send it mainline:
> > https://github.com/vladimiroltean/linux/commits/ndo-hwtstamp-v9
> > 
> > I have already try to ping him on this but without success.
> > Vlad any idea on when you could send your series upstream?
> 
> Right, and that means that the kernel is not yet ready to support
> Marvell PHY PTP, because all the pre-requisits to avoid breaking
> mvpp2 have not yet been merged.
> 
> So that's a NAK on this series from me.
> 
> I'd have thought this would be obvious given my well known stance
> on why I haven't merged Marvell PHY PTP support before.
> 
> -- 
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

I will try to update and submit that patch set over the course of this
weekend.
Re: [PATCH net-next v2 0/2] Add Marvell PHY PTP support
Posted by Russell King (Oracle) 10 months ago
On Wed, Apr 09, 2025 at 11:38:35AM +0300, Vladimir Oltean wrote:
> On Wed, Apr 09, 2025 at 09:35:59AM +0100, Russell King (Oracle) wrote:
> > On Wed, Apr 09, 2025 at 10:31:30AM +0200, Kory Maincent wrote:
> > > On Tue, 8 Apr 2025 21:38:19 +0100
> > > "Russell King (Oracle)" <linux@armlinux.org.uk> wrote:
> > > 
> > > > On Mon, Apr 07, 2025 at 06:39:14PM +0200, Kory Maincent wrote:
> > > > > On Mon, 7 Apr 2025 17:32:43 +0100
> > > > > "Russell King (Oracle)" <linux@armlinux.org.uk> wrote:
> > > > > > I'm preferring to my emails in connection with:
> > > > > > 
> > > > > > https://lore.kernel.org/r/ZzTMhGDoi3WcY6MR@shell.armlinux.org.uk
> > > > > > 
> > > > > > when I tested your work last time, it seemed that what was merged hadn't
> > > > > > even been tested. In the last email, you said you'd look into it, but I
> > > > > > didn't hear anything further. Have the problems I reported been
> > > > > > addressed?  
> > > > > 
> > > > > It wasn't merged it was 19th version and it worked and was tested, but not
> > > > > with the best development design. I have replied to you that I will do some
> > > > > change in v20 to address this.
> > > > > https://lore.kernel.org/all/20241113171443.697ac278@kmaincent-XPS-13-7390/
> > > > > 
> > > > > It gets finally merged in v21.  
> > > > 
> > > > Okay, so I'm pleased to report that this now works on the Macchiatobin:
> > > > 
> > > > where phc 2 is the mvpp2 clock, and phc 0 is the PHY.
> > > 
> > > Great, thank you for the testing!
> > > 
> > > > 
> > > > # ethtool -T eth2
> > > > Time stamping parameters for eth2:
> > > > Capabilities:
> > > >         hardware-transmit
> > > >         software-transmit
> > > >         hardware-receive
> > > >         software-receive
> > > >         software-system-clock
> > > >         hardware-raw-clock
> > > > PTP Hardware Clock: 2
> > > > Hardware Transmit Timestamp Modes:
> > > >         off
> > > >         on
> > > >         onestep-sync
> > > >         onestep-p2p
> > > > Hardware Receive Filter Modes:
> > > >         none
> > > >         all
> > > > 
> > > > So I guess that means that by default it's using PHC 2, and thus using
> > > > the MVPP2 PTP implementation - which is good, it means that when we add
> > > > Marvell PHY support, this won't switch to the PHY implementation.
> > > 
> > > Yes.
> > > 
> > > > 
> > > > Now, testing ethtool:
> > > > 
> > > > $ ./ethtool --get-hwtimestamp-cfg eth2
> > > > netlink error: Operation not supported
> > > > 
> > > > Using ynl:
> > > > 
> > > > # ./ynl/cli.py --spec netlink/specs/ethtool.yaml --no-schema --dump
> > > > tsconfig-get --json '{"header":{"dev-name":"eth2"}}' []
> > > > 
> > > > So, It's better, something still isn't correct as there's no
> > > > configuration. Maybe mvpp2 needs updating first? If that's the case,
> > > > then we're not yet in a position to merge PHY PTP support.
> > > 
> > > Indeed mvpp2 has not been update to support the ndo_hwtstamp_get/set NDOs.
> > > Vlad had made some work to update all net drivers to these NDOs but he never
> > > send it mainline:
> > > https://github.com/vladimiroltean/linux/commits/ndo-hwtstamp-v9
> > > 
> > > I have already try to ping him on this but without success.
> > > Vlad any idea on when you could send your series upstream?
> > 
> > Right, and that means that the kernel is not yet ready to support
> > Marvell PHY PTP, because all the pre-requisits to avoid breaking
> > mvpp2 have not yet been merged.
> > 
> > So that's a NAK on this series from me.
> > 
> > I'd have thought this would be obvious given my well known stance
> > on why I haven't merged Marvell PHY PTP support before.
> > 
> > -- 
> > RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> > FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
> 
> I will try to update and submit that patch set over the course of this
> weekend.

Thanks. Friday is really the last day for me for an uncertain period
thereafter where I won't be able to do any further testing.

I don't run PTP on my network as it's IMHO not as good as NTP with the
hardware I have. It's also been five years since I last had something
setup, which was when I was working on Marvell PHY support, All the
knowledge I had back then for PTP support has been "swapped out" into
/dev/null. I don't even remember which machines I was using, and thus
have no idea if they're even still connected to the network.

As this has already been blocked on this for five years, I don't think
it's unreasonable that it takes longer, so please don't feel that you
need to get them done by Friday.

Just be aware that I won't be able to test again for an uncertain
period thereafter.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH net-next v2 0/2] Add Marvell PHY PTP support
Posted by Kory Maincent 10 months ago
On Wed, 9 Apr 2025 11:38:35 +0300
Vladimir Oltean <vladimir.oltean@nxp.com> wrote:

> On Wed, Apr 09, 2025 at 09:35:59AM +0100, Russell King (Oracle) wrote:
> > On Wed, Apr 09, 2025 at 10:31:30AM +0200, Kory Maincent wrote:  
> > > On Tue, 8 Apr 2025 21:38:19 +0100
> > > "Russell King (Oracle)" <linux@armlinux.org.uk> wrote:
> > >   
>  [...]  
>  [...]  
>  [...]  
>  [...]  
>  [...]  
> > > 
> > > Great, thank you for the testing!
> > >   
>  [...]  
> > > 
> > > Yes.
> > >   
>  [...]  
> > > 
> > > Indeed mvpp2 has not been update to support the ndo_hwtstamp_get/set NDOs.
> > > Vlad had made some work to update all net drivers to these NDOs but he
> > > never send it mainline:
> > > https://github.com/vladimiroltean/linux/commits/ndo-hwtstamp-v9
> > > 
> > > I have already try to ping him on this but without success.
> > > Vlad any idea on when you could send your series upstream?  
> > 
> > Right, and that means that the kernel is not yet ready to support
> > Marvell PHY PTP, because all the pre-requisits to avoid breaking
> > mvpp2 have not yet been merged.
> > 
> > So that's a NAK on this series from me.
> > 
> > I'd have thought this would be obvious given my well known stance
> > on why I haven't merged Marvell PHY PTP support before.
> > 
> > -- 
> > RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> > FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!  
> 
> I will try to update and submit that patch set over the course of this
> weekend.

That's great, thanks for the update status!

Regards,
-- 
Köry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com
Re: [PATCH net-next v2 0/2] Add Marvell PHY PTP support
Posted by Andrew Lunn 10 months, 1 week ago
On Mon, Apr 07, 2025 at 04:02:59PM +0200, Kory Maincent wrote:
> Add PTP basic support for Marvell 88E151x PHYs.

Russell has repeatedly said this will cause regressions in some setups
where there are now two PTP implementations, and the wrong one will be
chosen by default. I would expect some comments in the commit message
explaining how this has been addressed, so it is clear a regression
will not happen.

     Andrew
Re: [PATCH net-next v2 0/2] Add Marvell PHY PTP support
Posted by Kory Maincent 10 months, 1 week ago
On Mon, 7 Apr 2025 16:08:04 +0200
Andrew Lunn <andrew@lunn.ch> wrote:

> On Mon, Apr 07, 2025 at 04:02:59PM +0200, Kory Maincent wrote:
> > Add PTP basic support for Marvell 88E151x PHYs.  
> 
> Russell has repeatedly said this will cause regressions in some setups
> where there are now two PTP implementations, and the wrong one will be
> chosen by default. I would expect some comments in the commit message
> explaining how this has been addressed, so it is clear a regression
> will not happen.

This was fixed by the following patch series which have parts that get merged
along the way to version 21. It adds support to select the hardware PTP
provider and change the default to MAC PTP for newly introduced PHY PTP support
(default_timestamp flag in phy_device struct).
https://lore.kernel.org/netdev/20241212-feature_ptp_netnext-v21-0-2c282a941518@bootlin.com/

I will add the description in v3. I will wait at least one week to let people
review the patch.

Regards,
-- 
Köry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com