diff mbox series

[net-next] net: dsa: mv88e6xxx: Enable PTP receive for mv88e6390

Message ID 20230111080417.147231-1-kurt@linutronix.de (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series [net-next] net: dsa: mv88e6xxx: Enable PTP receive for mv88e6390 | expand

Checks

Context Check Description
netdev/tree_selection success Clearly marked for net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/subject_prefix success Link
netdev/cover_letter success Single patches do not need cover letters
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/cc_maintainers success CCed 8 of 8 maintainers
netdev/build_clang success Errors and warnings before: 0 this patch: 0
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 93 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Kurt Kanzenbach Jan. 11, 2023, 8:04 a.m. UTC 
The switch receives management traffic such as STP and LLDP. However, PTP
messages are not received, only transmitted.

Ideally, the switch would trap all PTP messages to the management CPU. This
particular switch has a PTP block which identifies PTP messages and traps them
to a dedicated port. There is a register to program this destination. This is
not used at the moment.

Therefore, program it to the same port as the MGMT traffic is trapped to. This
allows to receive PTP messages as soon as timestamping is enabled.

In addition, the datasheet mentions that this register is not valid e.g. for
6190 variants. So, add a new cpu port method for 6390 which programs the MGTM
and PTP destination.

Tested simply like this on Marvell 88E6390, revision 1:

|/ # ptp4l -2 -i lan4 --tx_timestamp_timeout=40 -m
|[...]
|ptp4l[147.450]: master offset         56 s2 freq   +1262 path delay       413
|ptp4l[148.450]: master offset         22 s2 freq   +1244 path delay       434
|ptp4l[149.450]: master offset          5 s2 freq   +1234 path delay       446
|ptp4l[150.451]: master offset          3 s2 freq   +1233 path delay       451
|ptp4l[151.451]: master offset          1 s2 freq   +1232 path delay       451
|ptp4l[152.451]: master offset         -3 s2 freq   +1229 path delay       451
|ptp4l[153.451]: master offset          9 s2 freq   +1240 path delay       451

Signed-off-by: Kurt Kanzenbach <kurt@linutronix.de>
---

Note, This might be related:

https://lore.kernel.org/netdev/CAFSKS=PJBpvtRJxrR4sG1hyxpnUnQpiHg4SrUNzAhkWnyt9ivg@mail.gmail.com/

 drivers/net/dsa/mv88e6xxx/chip.c    | 12 ++++++------
 drivers/net/dsa/mv88e6xxx/global1.c | 19 ++++++++++++++++++-
 drivers/net/dsa/mv88e6xxx/global1.h |  2 ++
 3 files changed, 26 insertions(+), 7 deletions(-)

Comments

Andrew Lunn Jan. 11, 2023, 1:38 p.m. UTC | #1 
On Wed, Jan 11, 2023 at 09:04:17AM +0100, Kurt Kanzenbach wrote:
> The switch receives management traffic such as STP and LLDP. However, PTP
> messages are not received, only transmitted.
> 
> Ideally, the switch would trap all PTP messages to the management CPU. This
> particular switch has a PTP block which identifies PTP messages and traps them
> to a dedicated port. There is a register to program this destination. This is
> not used at the moment.
> 
> Therefore, program it to the same port as the MGMT traffic is trapped to. This
> allows to receive PTP messages as soon as timestamping is enabled.
> 
> In addition, the datasheet mentions that this register is not valid e.g. for
> 6190 variants. So, add a new cpu port method for 6390 which programs the MGTM
> and PTP destination.

The mv88e6190x_ops and ops mv88e6190_ops structure does not have a
ptp_ops member. So these two devices do not support PTP.

I think it would be cleaner to implement setting the PTP MGMT port as
part of the ptp_ops. Maybe add a new op, which is called from
mv88e6xxx_ptp_setup() if set?

	Andrew
Kurt Kanzenbach Jan. 11, 2023, 2:20 p.m. UTC | #2 
On Wed Jan 11 2023, Andrew Lunn wrote:
> On Wed, Jan 11, 2023 at 09:04:17AM +0100, Kurt Kanzenbach wrote:
>> The switch receives management traffic such as STP and LLDP. However, PTP
>> messages are not received, only transmitted.
>> 
>> Ideally, the switch would trap all PTP messages to the management CPU. This
>> particular switch has a PTP block which identifies PTP messages and traps them
>> to a dedicated port. There is a register to program this destination. This is
>> not used at the moment.
>> 
>> Therefore, program it to the same port as the MGMT traffic is trapped to. This
>> allows to receive PTP messages as soon as timestamping is enabled.
>> 
>> In addition, the datasheet mentions that this register is not valid e.g. for
>> 6190 variants. So, add a new cpu port method for 6390 which programs the MGTM
>> and PTP destination.
>
> The mv88e6190x_ops and ops mv88e6190_ops structure does not have a
> ptp_ops member. So these two devices do not support PTP.
>
> I think it would be cleaner to implement setting the PTP MGMT port as
> part of the ptp_ops. Maybe add a new op, which is called from
> mv88e6xxx_ptp_setup() if set?

OK, can do. The 6390 currently uses mv88e6352_ptp_ops. This one is used
by a lot of other devices, too. For instance, the 6341 doesn't have this
PTP MGMT port, but uses the same ops structure. Thus, I'd have to
introduce a new ptp ops structure for both 6390 devices.

Thanks,
Kurt
diff mbox series

Patch

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 1168ea75f5f5..5762a24dc061 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -4348,7 +4348,7 @@  static const struct mv88e6xxx_ops mv88e6141_ops = {
 	.stats_get_sset_count = mv88e6320_stats_get_sset_count,
 	.stats_get_strings = mv88e6320_stats_get_strings,
 	.stats_get_stats = mv88e6390_stats_get_stats,
-	.set_cpu_port = mv88e6390_g1_set_cpu_port,
+	.set_cpu_port = mv88e6190_g1_set_cpu_port,
 	.set_egress_port = mv88e6390_g1_set_egress_port,
 	.watchdog_ops = &mv88e6390_watchdog_ops,
 	.mgmt_rsvd2cpu =  mv88e6390_g1_mgmt_rsvd2cpu,
@@ -4753,7 +4753,7 @@  static const struct mv88e6xxx_ops mv88e6190_ops = {
 	.stats_get_sset_count = mv88e6320_stats_get_sset_count,
 	.stats_get_strings = mv88e6320_stats_get_strings,
 	.stats_get_stats = mv88e6390_stats_get_stats,
-	.set_cpu_port = mv88e6390_g1_set_cpu_port,
+	.set_cpu_port = mv88e6190_g1_set_cpu_port,
 	.set_egress_port = mv88e6390_g1_set_egress_port,
 	.watchdog_ops = &mv88e6390_watchdog_ops,
 	.mgmt_rsvd2cpu = mv88e6390_g1_mgmt_rsvd2cpu,
@@ -4818,7 +4818,7 @@  static const struct mv88e6xxx_ops mv88e6190x_ops = {
 	.stats_get_sset_count = mv88e6320_stats_get_sset_count,
 	.stats_get_strings = mv88e6320_stats_get_strings,
 	.stats_get_stats = mv88e6390_stats_get_stats,
-	.set_cpu_port = mv88e6390_g1_set_cpu_port,
+	.set_cpu_port = mv88e6190_g1_set_cpu_port,
 	.set_egress_port = mv88e6390_g1_set_egress_port,
 	.watchdog_ops = &mv88e6390_watchdog_ops,
 	.mgmt_rsvd2cpu = mv88e6390_g1_mgmt_rsvd2cpu,
@@ -4881,7 +4881,7 @@  static const struct mv88e6xxx_ops mv88e6191_ops = {
 	.stats_get_sset_count = mv88e6320_stats_get_sset_count,
 	.stats_get_strings = mv88e6320_stats_get_strings,
 	.stats_get_stats = mv88e6390_stats_get_stats,
-	.set_cpu_port = mv88e6390_g1_set_cpu_port,
+	.set_cpu_port = mv88e6190_g1_set_cpu_port,
 	.set_egress_port = mv88e6390_g1_set_egress_port,
 	.watchdog_ops = &mv88e6390_watchdog_ops,
 	.mgmt_rsvd2cpu = mv88e6390_g1_mgmt_rsvd2cpu,
@@ -5053,7 +5053,7 @@  static const struct mv88e6xxx_ops mv88e6290_ops = {
 	.stats_get_sset_count = mv88e6320_stats_get_sset_count,
 	.stats_get_strings = mv88e6320_stats_get_strings,
 	.stats_get_stats = mv88e6390_stats_get_stats,
-	.set_cpu_port = mv88e6390_g1_set_cpu_port,
+	.set_cpu_port = mv88e6190_g1_set_cpu_port,
 	.set_egress_port = mv88e6390_g1_set_egress_port,
 	.watchdog_ops = &mv88e6390_watchdog_ops,
 	.mgmt_rsvd2cpu = mv88e6390_g1_mgmt_rsvd2cpu,
@@ -5214,7 +5214,7 @@  static const struct mv88e6xxx_ops mv88e6341_ops = {
 	.stats_get_sset_count = mv88e6320_stats_get_sset_count,
 	.stats_get_strings = mv88e6320_stats_get_strings,
 	.stats_get_stats = mv88e6390_stats_get_stats,
-	.set_cpu_port = mv88e6390_g1_set_cpu_port,
+	.set_cpu_port = mv88e6190_g1_set_cpu_port,
 	.set_egress_port = mv88e6390_g1_set_egress_port,
 	.watchdog_ops = &mv88e6390_watchdog_ops,
 	.mgmt_rsvd2cpu =  mv88e6390_g1_mgmt_rsvd2cpu,
diff --git a/drivers/net/dsa/mv88e6xxx/global1.c b/drivers/net/dsa/mv88e6xxx/global1.c
index 5848112036b0..ad2d8d07fef5 100644
--- a/drivers/net/dsa/mv88e6xxx/global1.c
+++ b/drivers/net/dsa/mv88e6xxx/global1.c
@@ -391,7 +391,7 @@  int mv88e6390_g1_set_egress_port(struct mv88e6xxx_chip *chip,
 	return mv88e6390_g1_monitor_write(chip, ptr, port);
 }
 
-int mv88e6390_g1_set_cpu_port(struct mv88e6xxx_chip *chip, int port)
+int mv88e6190_g1_set_cpu_port(struct mv88e6xxx_chip *chip, int port)
 {
 	u16 ptr = MV88E6390_G1_MONITOR_MGMT_CTL_PTR_CPU_DEST;
 
@@ -403,6 +403,23 @@  int mv88e6390_g1_set_cpu_port(struct mv88e6xxx_chip *chip, int port)
 	return mv88e6390_g1_monitor_write(chip, ptr, port);
 }
 
+int mv88e6390_g1_set_cpu_port(struct mv88e6xxx_chip *chip, int port)
+{
+	u16 ptr = MV88E6390_G1_MONITOR_MGMT_CTL_PTR_PTP_CPU_DEST;
+	int ret;
+
+	ret = mv88e6190_g1_set_cpu_port(chip, port);
+	if (ret)
+		return ret;
+
+	/* Use the default high priority for PTP frames sent to
+	 * the CPU.
+	 */
+	port |= MV88E6390_G1_MONITOR_MGMT_CTL_PTR_CPU_DEST_MGMTPRI;
+
+	return mv88e6390_g1_monitor_write(chip, ptr, port);
+}
+
 int mv88e6390_g1_mgmt_rsvd2cpu(struct mv88e6xxx_chip *chip)
 {
 	u16 ptr;
diff --git a/drivers/net/dsa/mv88e6xxx/global1.h b/drivers/net/dsa/mv88e6xxx/global1.h
index 65958b2a0d3a..f0b303f38764 100644
--- a/drivers/net/dsa/mv88e6xxx/global1.h
+++ b/drivers/net/dsa/mv88e6xxx/global1.h
@@ -214,6 +214,7 @@ 
 #define MV88E6390_G1_MONITOR_MGMT_CTL_PTR_INGRESS_DEST		0x2000
 #define MV88E6390_G1_MONITOR_MGMT_CTL_PTR_EGRESS_DEST		0x2100
 #define MV88E6390_G1_MONITOR_MGMT_CTL_PTR_CPU_DEST		0x3000
+#define MV88E6390_G1_MONITOR_MGMT_CTL_PTR_PTP_CPU_DEST		0x3200
 #define MV88E6390_G1_MONITOR_MGMT_CTL_PTR_CPU_DEST_MGMTPRI	0x00e0
 #define MV88E6390_G1_MONITOR_MGMT_CTL_DATA_MASK			0x00ff
 
@@ -302,6 +303,7 @@  int mv88e6390_g1_set_egress_port(struct mv88e6xxx_chip *chip,
 				 enum mv88e6xxx_egress_direction direction,
 				 int port);
 int mv88e6095_g1_set_cpu_port(struct mv88e6xxx_chip *chip, int port);
+int mv88e6190_g1_set_cpu_port(struct mv88e6xxx_chip *chip, int port);
 int mv88e6390_g1_set_cpu_port(struct mv88e6xxx_chip *chip, int port);
 int mv88e6390_g1_mgmt_rsvd2cpu(struct mv88e6xxx_chip *chip);