[PATCH net] net: dsa: mv88e6xxx: continue without PTP if the TAI period is invalid

[PATCH net] net: dsa: mv88e6xxx: continue without PTP if the TAI period is invalid

[Nicolo Giuliani via B4 Relay]

From: Nicolo Giuliani <nicolo.giuliani6@studio.unibo.it>

Since commit 7e3c18097a70 ("net: dsa: mv88e6xxx: read cycle counter
period from hardware"), mv88e6xxx_ptp_setup() reads the TAI clock period
register and fails with -ENODEV if the value is not one of the supported
periods. mv88e6xxx_setup() propagates the error, so the switch does not
probe at all, although it is fully usable without PTP. Before that commit
the register was not read and the probe did not depend on its value.

The 88E6193X on the Sophos XGS 107w reads 0 in that register, so its
probe fails with:

  mv88e6xxx ...: unexpected cycle counter period of 0 ps

Treat -ENODEV from mv88e6xxx_ptp_setup() as the absence of a usable PTP
clock: warn, skip the hardware timestamping setup and carry on without
registering a PHC. -ENODEV can only come from mv88e6xxx_cc_coeff_get(),
which runs before ptp_setup changes any state, so there is nothing to
undo, and all other errors still fail the probe.

Without a PHC chip->ptp_clock stays NULL, and the timestamping entry
points dereference it (ptp_clock_index() in get_ts_info,
ptp_schedule_worker() in the rx and tx paths). Make get_ts_info,
port_hwtstamp_set/get and mv88e6xxx_should_tstamp() treat a missing
clock like a chip without ptp_support. Switches that register a PHC are
unaffected.

Fixes: 7e3c18097a70 ("net: dsa: mv88e6xxx: read cycle counter period from hardware")
Assisted-by: LLM
Signed-off-by: Nicolo Giuliani <nicolo.giuliani6@studio.unibo.it>
---
 drivers/net/dsa/mv88e6xxx/chip.c     | 19 ++++++++++++++-----
 drivers/net/dsa/mv88e6xxx/hwtstamp.c |  8 ++++----
 2 files changed, 18 insertions(+), 9 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 7f68a0c55..2d6de43f2 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -4106,12 +4106,21 @@ static int mv88e6xxx_setup(struct dsa_switch *ds)
 	/* Setup PTP Hardware Clock and timestamping */
 	if (chip->info->ptp_support) {
 		err = mv88e6xxx_ptp_setup(chip);
-		if (err)
-			goto unlock;
-
-		err = mv88e6xxx_hwtstamp_setup(chip);
-		if (err)
+		if (err == -ENODEV) {
+			/* The TAI clock period is not one that the driver
+			 * supports: run the switch without PTP rather than
+			 * failing the whole probe.
+			 */
+			dev_warn(chip->dev,
+				 "PTP clock unavailable, hardware timestamping disabled\n");
+			err = 0;
+		} else if (err) {
 			goto unlock;
+		} else {
+			err = mv88e6xxx_hwtstamp_setup(chip);
+			if (err)
+				goto unlock;
+		}
 	}
 
 	err = mv88e6xxx_stats_setup(chip);
diff --git a/drivers/net/dsa/mv88e6xxx/hwtstamp.c b/drivers/net/dsa/mv88e6xxx/hwtstamp.c
index 6e6472a3b..847f9dd44 100644
--- a/drivers/net/dsa/mv88e6xxx/hwtstamp.c
+++ b/drivers/net/dsa/mv88e6xxx/hwtstamp.c
@@ -72,7 +72,7 @@ int mv88e6xxx_get_ts_info(struct dsa_switch *ds, int port,
 	chip = ds->priv;
 	ptp_ops = chip->info->ops->ptp_ops;
 
-	if (!chip->info->ptp_support)
+	if (!chip->info->ptp_support || !chip->ptp_clock)
 		return -EOPNOTSUPP;
 
 	info->so_timestamping =
@@ -176,7 +176,7 @@ int mv88e6xxx_port_hwtstamp_set(struct dsa_switch *ds, int port,
 	struct mv88e6xxx_port_hwtstamp *ps = &chip->port_hwtstamp[port];
 	int err;
 
-	if (!chip->info->ptp_support)
+	if (!chip->info->ptp_support || !chip->ptp_clock)
 		return -EOPNOTSUPP;
 
 	err = mv88e6xxx_set_hwtstamp_config(chip, port, config);
@@ -195,7 +195,7 @@ int mv88e6xxx_port_hwtstamp_get(struct dsa_switch *ds, int port,
 	struct mv88e6xxx_chip *chip = ds->priv;
 	struct mv88e6xxx_port_hwtstamp *ps = &chip->port_hwtstamp[port];
 
-	if (!chip->info->ptp_support)
+	if (!chip->info->ptp_support || !chip->ptp_clock)
 		return -EOPNOTSUPP;
 
 	*config = ps->tstamp_config;
@@ -213,7 +213,7 @@ static struct ptp_header *mv88e6xxx_should_tstamp(struct mv88e6xxx_chip *chip,
 	struct mv88e6xxx_port_hwtstamp *ps = &chip->port_hwtstamp[port];
 	struct ptp_header *hdr;
 
-	if (!chip->info->ptp_support)
+	if (!chip->info->ptp_support || !chip->ptp_clock)
 		return NULL;
 
 	hdr = ptp_parse_header(skb, type);

---
base-commit: 6c096bb08de97cdca051fecddad22cac6a1fd275
change-id: 20260919-send-net-97aa5653ac48

Best regards,
--  
Nicolo Giuliani <nicolo.giuliani6@studio.unibo.it>

Re: [PATCH net] net: dsa: mv88e6xxx: continue without PTP if the TAI period is invalid

[Andrew Lunn]

On Sat, Sep 19, 2026 at 09:52:07PM +0200, Nicolo Giuliani via B4 Relay wrote:
> From: Nicolo Giuliani <nicolo.giuliani6@studio.unibo.it>
> 
> Since commit 7e3c18097a70 ("net: dsa: mv88e6xxx: read cycle counter
> period from hardware"), mv88e6xxx_ptp_setup() reads the TAI clock period
> register and fails with -ENODEV if the value is not one of the supported
> periods. mv88e6xxx_setup() propagates the error, so the switch does not
> probe at all, although it is fully usable without PTP. Before that commit
> the register was not read and the probe did not depend on its value.

The point of the failure is to indicate an assumption in the driver is
not valid, and we need to examine the assumption.

> 
> The 88E6193X on the Sophos XGS 107w reads 0 in that register, so its
> probe fails with:
> 
>   mv88e6xxx ...: unexpected cycle counter period of 0 ps

So the assumption is, all devices have valid values here. So the
correct fix is to refine the assumption. What does the datasheet for
the 88E6193X say about this register? Has its meaning changed? Marvell
like moving registers around, is it somewhere else?

    Andrew

---
pw-bot: cr

R: [PATCH net] net: dsa: mv88e6xxx: continue without PTP if the TAI period is invalid

[Nicolò Giuliani - nicolo.giuliani6@studio.unibo.it]

Hi Andrew,

> The point of the failure is to indicate an assumption in the driver is
> not valid, and we need to examine the assumption.

You are right, and my patch only works around it.

> What does the datasheet for the 88E6193X say about this register? Has
> its meaning changed? Marvell like moving registers around, is it
> somewhere else?

I do not have the datasheet for this chip, so I cannot answer that from documentation. What I can say from the board: the read of MV88E6XXX_TAI_CLOCK_PERIOD in mv88e6xxx_cc_coeff_get() returns 0 at probe on this 88E6193X, both when the boot loader has reset the switch and when the driver has. The vendor firmware drives the switch from user space, so I do not know whether it programs this register.

Before I send anything again I will read the register at other points (right after a hardware reset, before and after the PTP block is enabled) and check how the other 6390 family entries in the driver get their TAI period, then report what I find in this thread. If the period has to be programmed on this family, that is a different patch, and I will drop this one.

Thanks,
Nicolo Giuliani

________________________________________
Da: Andrew Lunn <andrew@lunn.ch>
Inviato: domenica 20 settembre 2026 16:48
A: Nicolò Giuliani - nicolo.giuliani6@studio.unibo.it
Cc: Vladimir Oltean; David S. Miller; Eric Dumazet; Jakub Kicinski; Paolo Abeni; Richard Cochran; netdev@vger.kernel.org; linux-kernel@vger.kernel.org
Oggetto: Re: [PATCH net] net: dsa: mv88e6xxx: continue without PTP if the TAI period is invalid

On Sat, Sep 19, 2026 at 09:52:07PM +0200, Nicolo Giuliani via B4 Relay wrote:
> From: Nicolo Giuliani <nicolo.giuliani6@studio.unibo.it>
>
> Since commit 7e3c18097a70 ("net: dsa: mv88e6xxx: read cycle counter
> period from hardware"), mv88e6xxx_ptp_setup() reads the TAI clock period
> register and fails with -ENODEV if the value is not one of the supported
> periods. mv88e6xxx_setup() propagates the error, so the switch does not
> probe at all, although it is fully usable without PTP. Before that commit
> the register was not read and the probe did not depend on its value.

The point of the failure is to indicate an assumption in the driver is
not valid, and we need to examine the assumption.

>
> The 88E6193X on the Sophos XGS 107w reads 0 in that register, so its
> probe fails with:
>
>   mv88e6xxx ...: unexpected cycle counter period of 0 ps

So the assumption is, all devices have valid values here. So the
correct fix is to refine the assumption. What does the datasheet for
the 88E6193X say about this register? Has its meaning changed? Marvell
like moving registers around, is it somewhere else?

    Andrew

---
pw-bot: cr

Re: R: [PATCH net] net: dsa: mv88e6xxx: continue without PTP if the TAI period is invalid

[Andrew Lunn]

On Sun, Sep 20, 2026 at 05:34:01PM +0000, Nicolò Giuliani - nicolo.giuliani6@studio.unibo.it wrote:
> 
> Hi Andrew,
> 
> > The point of the failure is to indicate an assumption in the driver is
> > not valid, and we need to examine the assumption.
> 
> You are right, and my patch only works around it.
> 
> > What does the datasheet for the 88E6193X say about this register? Has
> > its meaning changed? Marvell like moving registers around, is it
> > somewhere else?
> 
> I do not have the datasheet for this chip, so I cannot answer that from documentation.

I don't have the 88E6193X either. But i do have some other datasheets.

If the device is using the internal 250MHz clock, the register is
expected to contain 0x0FA0, 4000picoseconds. If i remember correctly,
the patch which changed things was because different devices have
different internal clocks, hence the need to read it.

There is however the option to use an external clock. This is
controlled via TAI register 0x1e. If bit 14 is 0, the internal clock
is used. If 1, the external clock is used. With the external clock,
you need to write to register 0x01 what the external clock period is,
in picoseconds.

So you probably want to check what register 0x1e contains.

	Andrew

R: R: [PATCH net] net: dsa: mv88e6xxx: continue without PTP if the TAI period is invalid

[Nicolò Giuliani - nicolo.giuliani6@studio.unibo.it]

Hi Andrew,

Thanks, that is what I needed. I checked it on the board (chip ID 0x1930, so the driver entry is the 88E6193X, family 6393, which uses mv88e6390_avb_ops and mv88e6352_ptp_ops):

- TAI register 0x1e reads 0x0000, so bit 14 is 0: the internal clock is selected. Register 0x01 should then hold 0x0FA0, but it reads 0x0000.
- The whole TAI global block reads zero through the driver's accessor: all 32 registers of the TAI global, of the PTP global and of one port PTP block. Writing 8000 to register 0x01 and 0x8000 to register 0x00 reads back 0x0000.
- To see whether the block is somewhere else, I scanned the whole indirect space of the Global 2 AVB command/data registers (0x16/0x17): every port field 0-31, block 0-7 and address 0-31 with the 6390 command encoding, and every port field 0-15 with the 6352 encoding. Every read returns 0x0000. The only exceptions were a few reads of 0xc801, at a different place on every run, which looks like an access by the driver interleaving with my raw scan, not a register.
- The commands complete (the busy bit clears and there is no MDIO error), but writing a pattern to Global 2 register 0x17 or 0x16 reads back 0, while other Global 2 registers hold non-zero values.

So the indirect AVB interface used by the 6390 ops does not answer on this chip: this is not only a TAI period register that is not programmed. Either the PTP unit is not reachable this way on the 6393X family, or it is not present or gated on the 88E6193X. The 6393X entries in the driver reuse the 6390 ops, and I cannot tell from the driver whether PTP was ever verified on them.

I do not have the datasheet, but you said you have others: does the 88E6393X family datasheet put the PTP/TAI access somewhere other than Global 2 0x16/0x17, or say that the 88E6193X and 88E6191X have no PTP unit? If the access is different, I will write a patch that fixes the ops for the family; if the 88E6193X has no PTP, I will write a patch that clears ptp_support for it. I will not send the patch that only skips PTP.

Thanks,
Nicolo Giuliani

________________________________________
Da: Andrew Lunn <andrew@lunn.ch>
Inviato: domenica 20 settembre 2026 20:58
A: Nicolò Giuliani - nicolo.giuliani6@studio.unibo.it
Cc: Vladimir Oltean; David S. Miller; Eric Dumazet; Jakub Kicinski; Paolo Abeni; Richard Cochran; netdev@vger.kernel.org; linux-kernel@vger.kernel.org
Oggetto: Re: R: [PATCH net] net: dsa: mv88e6xxx: continue without PTP if the TAI period is invalid

On Sun, Sep 20, 2026 at 05:34:01PM +0000, Nicolò Giuliani - nicolo.giuliani6@studio.unibo.it wrote:
>
> Hi Andrew,
>
> > The point of the failure is to indicate an assumption in the driver is
> > not valid, and we need to examine the assumption.
>
> You are right, and my patch only works around it.
>
> > What does the datasheet for the 88E6193X say about this register? Has
> > its meaning changed? Marvell like moving registers around, is it
> > somewhere else?
>
> I do not have the datasheet for this chip, so I cannot answer that from documentation.

I don't have the 88E6193X either. But i do have some other datasheets.

If the device is using the internal 250MHz clock, the register is
expected to contain 0x0FA0, 4000picoseconds. If i remember correctly,
the patch which changed things was because different devices have
different internal clocks, hence the need to read it.

There is however the option to use an external clock. This is
controlled via TAI register 0x1e. If bit 14 is 0, the internal clock
is used. If 1, the external clock is used. With the external clock,
you need to write to register 0x01 what the external clock period is,
in picoseconds.

So you probably want to check what register 0x1e contains.

        Andrew

Re: R: R: [PATCH net] net: dsa: mv88e6xxx: continue without PTP if the TAI period is invalid

[Andrew Lunn]

On Sun, Sep 20, 2026 at 07:08:57PM +0000, Nicolò Giuliani - nicolo.giuliani6@studio.unibo.it wrote:
> Hi Andrew,
> 

> Thanks, that is what I needed. I checked it on the board (chip ID
> 0x1930, so the driver entry is the 88E6193X, family 6393, which uses
> mv88e6390_avb_ops and mv88e6352_ptp_ops):

Marvell names all there devices 88e6xyz. The x indicates the model
within the family y. The lower the value of x, the less features the
device has.

So i could well be the 88E6193X does not have avb, tsn and ptp.

Marvell has a vendor driver for these devices called UMSD. It has an
open source licence, but it is well hidden.

include/driver/msdSysConfig.h:	MSD_88E6193X = 0x193,   /* 88E6193X  - BGA package - No AVB, No Routing, No Cut-through*/

So i would say PTP is not available on this device. Please could you
submit a patch removing the mv88e6390_avb_ops and mv88e6352_ptp_ops.

There is also:

        MSD_88E6191X = 0x192,  /* Amethyst 88E6191X  - BGA package - No AVB, No Routing, No Cut-through, No TCAM, No 802.1BR*/

Maybe you can review that entry as well.

       Andrew

R: R: R: [PATCH net] net: dsa: mv88e6xxx: continue without PTP if the TAI period is invalid

[Nicolò Giuliani - nicolo.giuliani6@studio.unibo.it]

Hi Andrew,

Thanks, that explains it. I have sent the patch as v2 in a new thread:

https://patch.msgid.link/20260921-send-net-v2-1-031ad720f140@studio.unibo.it

It adds mv88e6191x_ops, a copy of mv88e6393x_ops without avb_ops and ptp_ops, uses it for the 88E6191X and the 88E6193X and drops ptp_support for both, so the 88E6393X is unchanged. I included the 88E6191X because UMSD describes it the same way; I only have an 88E6193X, so that entry is untested.

I tested it on the 88E6193X of the Sophos XGS 107w (Linux 6.18.52): with the driver unbound and the module replaced by one built with the patch, and without the workaround from v1, the switch is detected as 0x1930, the eight PHYs and the 10gbase-r link come up, there is no PTP error and no PHC is registered.

Thanks,
Nicolo Giuliani

________________________________________
Da: Andrew Lunn <andrew@lunn.ch>
Inviato: domenica 20 settembre 2026 23:21
A: Nicolò Giuliani - nicolo.giuliani6@studio.unibo.it
Cc: Vladimir Oltean; David S. Miller; Eric Dumazet; Jakub Kicinski; Paolo Abeni; Richard Cochran; netdev@vger.kernel.org; linux-kernel@vger.kernel.org
Oggetto: Re: R: R: [PATCH net] net: dsa: mv88e6xxx: continue without PTP if the TAI period is invalid

On Sun, Sep 20, 2026 at 07:08:57PM +0000, Nicolò Giuliani - nicolo.giuliani6@studio.unibo.it wrote:
> Hi Andrew,
>

> Thanks, that is what I needed. I checked it on the board (chip ID
> 0x1930, so the driver entry is the 88E6193X, family 6393, which uses
> mv88e6390_avb_ops and mv88e6352_ptp_ops):

Marvell names all there devices 88e6xyz. The x indicates the model
within the family y. The lower the value of x, the less features the
device has.

So i could well be the 88E6193X does not have avb, tsn and ptp.

Marvell has a vendor driver for these devices called UMSD. It has an
open source licence, but it is well hidden.

include/driver/msdSysConfig.h:  MSD_88E6193X = 0x193,   /* 88E6193X  - BGA package - No AVB, No Routing, No Cut-through*/

So i would say PTP is not available on this device. Please could you
submit a patch removing the mv88e6390_avb_ops and mv88e6352_ptp_ops.

There is also:

        MSD_88E6191X = 0x192,  /* Amethyst 88E6191X  - BGA package - No AVB, No Routing, No Cut-through, No TCAM, No 802.1BR*/

Maybe you can review that entry as well.

       Andrew