Re: [PATCH net-next v9 4/4] net: dsa: add basic initial driver for MxL862xx switches

On Wed, Jan 28, 2026 at 04:45:38PM +0000, Daniel Golle wrote:

quoted

quoted

+static int mxl862xx_configure_tag_proto(struct dsa_switch *ds, int port, bool enable)
+{
+	struct mxl862xx_ctp_port_assignment assign = {
+		.number_of_ctp_port = cpu_to_le16(enable ? (32 - DSA_MXL_PORT(port)) : 1),
+		.logical_port_id = DSA_MXL_PORT(port),
+		.first_ctp_port_id = cpu_to_le16(DSA_MXL_PORT(port)),
+		.mode = cpu_to_le32(MXL862XX_LOGICAL_PORT_ETHERNET),

I have a really hard time understanding the physical port to CTP port
mapping here. Specifically, I see you allocate 1 CTP port (sub-interface)
per user port, and the rest up to 32 CTP ports as sub-interfaces of the
DSA CPU port.

But I can only see this working if the index of the DSA CPU port is the
last. Otherwise, won't the CTP port IDs of user ports subsequent to the
DSA CPU port overlap with sub-interfaces of the latter?

Example:
mxl862xx_configure_tag_proto(ds, 3, true) // CPU port
-> .number_of_ctp_port = cpu_to_le16(32 - DSA_MXL_PORT(3)), // 28
-> .logical_port_id = DSA_MXL_PORT(3), // 4
-> .first_ctp_port_id = cpu_to_le16(DSA_MXL_PORT(3)), // 4
mxl862xx_configure_tag_proto(ds, 4, false) // user port
-> .number_of_ctp_port = cpu_to_le16(1), // 1
-> .logical_port_id = DSA_MXL_PORT(4), // 5
-> .first_ctp_port_id = cpu_to_le16(DSA_MXL_PORT(4)), // 5

Doesn't the CTP port ID of user port 4 (5) collide with sub-interface 2
of CPU port 3?

Absolutely true. It works coincidental because only SerDes port 0
(port_id 9) is allowed to be CPU port. When using SerDes port 1
(port_id 13) as a user port it already starts to be tricky and more care
needs to be taken when setting up CTP port assignment...

Now that I understand more of what it actually does I'll try to come up
with something smarter.

I think the next step is to figure out how to best allocate the
limited sub-interfaces for the CPU port depending on what is actually
needed from them, because...

quoted

If the hardware supports multiple CPU ports, then please describe all
CPU ports as such in the device tree, and make an effort to handle that
description gracefully even if you cannot make use of the second CPU port.

The number of available CTP (32) limits the possible setup for CPU ports.
The reference driver only allows port_id 9 (SerDes 0) to be used as CPU
port. SerDes 1 may also be used as CPU port in theory.

quoted

See the rules by which dsa_tree_setup_cpu_ports() creates the initial
user to CPU port mapping. This avoids a future breakage you'll cause
with old kernels when you update the device tree to describe the second
CPU port for what it is.

Understood. On old kernel, lets assume with this basic driver just added
but future DT with more than one CPU port described one should still end
up with the first CPU port used and all ports assigned to that CPU port.
Right?

...with multiple CPU ports, calls such as dsa_switch_for_each_cpu_port()
will enumerate them all. The second CPU port will exist and will be set
up like all others (including mxl862xx_configure_tag_proto()), it's just
that no user port will be mapped to it (dp->cpu_dp will all point to the
first CPU port).

quoted

quoted

[...]
+#define MXL862XX_MAX_PHY_PORT_NUM	8
+#define MXL862XX_MAX_EXT_PORT_NUM	7
+#define MXL862XX_MAX_PORT_NUM		(MXL862XX_MAX_PHY_PORT_NUM + \
+					 MXL862XX_MAX_EXT_PORT_NUM)

8 + 7 == 15. This is written into ds->num_ports.

Good you are asking this. It's another oddity which came from the vendor
driver. It's wrong in many ways, see below.

Ok, so in the next revision you'll set it to 17 according to the table
below, right?

quoted

Could you please explain how this agrees with the Kconfig help text:
	  These switches have two 10GE SerDes interfaces, one typically
	  used as CPU port.
	   MxL86282 has eight 2.5 Gigabit PHYs
	   MxL86252 has five 2.5 Gigabit PHYs

What are the extra port indices used for, and is it ok that MxL86282 and
MxL86252 are registered with the same ds->num_ports value? This should
be visible in the "devlink port" command - even unused ports have
devlink instances.

This is probably because the SerDes ports also support 10G QXGMII PHYs
with 4x 2.5G TP ports connected via a single pair of 10G SerDes lanes.
In this case 4 port indexes are used for the 4 ports.

My understanding of the internal port IDs by now:
	 MxL86282				MxL86252
port 0:  microcontroller			microcontroller
port 1:  PHY port 0				PHY port 0
port 2:  PHY port 1				PHY port 1
port 3:  PHY port 2				PHY port 2
port 4:  PHY port 3				PHY port 3
port 5:  PHY port 4				PHY port 4
port 6:  PHY port 5				n/c
port 7:  PHY port 6				n/c
port 8:  PHY port 7				n/c
port 9:  SerDes PCS 0				SerDes PCS 0
port 10: SerDes PCS 0 (QXGMII)			SerDes PCS 0 (QXGMII)
port 11: SerDes PCS 0 (QXGMII)			SerDes PCS 0 (QXGMII)
port 12: SerDes PCS 0 (QXGMII)			SerDes PCS 0 (QXGMII)

For multi-port SerDes interfaces like 10G-QXGMII, technically each port
has its own PCS (port 10 -> PCS 1, port 11 -> PCS 2, etc). But I think I
understood the general idea. Maybe it would have been better to say
"SerDes lane 0 PCS 0" etc.

port 13: SerDes PCS 1				SerDes PCS 1
port 14: SerDes PCS 1 (QXGMII)			SerDes PCS 1 (QXGMII)
port 15: SerDes PCS 1 (QXGMII)			SerDes PCS 1 (QXGMII)
port 16: SerDes PCS 1 (QXGMII)			SerDes PCS 1 (QXGMII)

To be clear, what would be a proper name for the first column in this table?
Physical ports, right?

MaxLinear seems to tell board designers to always use port 9 as CPU port
and port 13 for an SFP cage or for an additional single-port PHY.

Ok, I assume that's what your board has as well. If I quickly run those
numbers into mxl862_tag_xmit(), then I guess that if you wanted to ping
each port (with cpu_port = 9), you'd set:

         MXL862_SUBIF_ID			MXL862_IGP_EGP
port 0:  7					9
port 1:  8					9
port 2:  9					9
port 3:  10					9
port 4:  11					9
port 5:  12					9
port 6:  13					9
port 7:  14					9
port 8:  15					9
port 9:  16					9
port 10: 17					9
port 11: 18					9
port 12: 19					9
port 13: 20					9
port 14: 21					9
port 15: 22					9
port 16: 23					9

I wonder how those MXL862_SUBIF_ID values map to the .number_of_ctp_port
configured by mxl862xx_configure_tag_proto(), which is 32 - DSA_MXL_PORT(9) == 22.
So to target port 16, you'd need a MXL862_SUBIF_ID which is out of range,
and results in an unrepresentable CTP index of 32. And subif_id values
0..6 are unused. Something doesn't add up...

Where does the 16 come from in the "usr_port + 16 - cpu_port" formula?

But this seem to be limitations of their current downstream driver
implementation and the hardware would be capable of using either
or both SerDes port as CPU port, and also support connecting a QXGMII PHY
to end up with 12 TP ports in total.

Interesting. If your host SoC supported 10G-QXGMII, you could
reconfigure both sides of that 10G SerDes interface on your existing
board and test quad CPU ports just for fun and giggles.

Port 0 indeed turned out to be the microcontroller, I can confirm that
because it screams when poking it with IEEE1588v2 packets, which you
get to see in the kernel logs:
net eth1: Invalid source port, packet dropped, tag: 88 c3 0f 04 00 00 00 10
                                                                         ^^
                                          IGP/EGP is 0 here, Record-ID is 1

The datasheet says that Record-ID "[...] is used for logging information
for PTP and OAM packets".

And calling mxl862xx_port_disable() on it makes it shut up...

In SJA1110 I could also ping the microcontroller by putting an IP
address on that user port. I am shutting it down during probe time, in
sja1110_disable_microcontroller(), and not relying on the firmware at all.


Maybe this is a stupid question, but I still don't quite grasp the purpose
of the logical port + sub-interface scheme from its use.

From what I could gather:
- xmit from Linux targets the logical port ID of the CPU port, and a
  sub-interface somehow in an implicit 1:1 association with user ports,
  which seems under-dimensioned in the maximal scenario
- the P-Mapper documentation says it can map one of {VLAN PCP, IP DSCP,
  LAG ID} to a sub-interface, but this feature is left to the firmware
  default state (MXL862XX_BRIDGE_PORT_CONFIG_MASK_EGRESS_CTP_MAPPING is
  never set), so I cannot make any assumptions about its influence

I don't quite understand how these two uses interact.