[PATCH RFC net-next 0/3] net: dsa: initial support for MaxLinear MxL862xx switches

[PATCH RFC net-next 0/3] net: dsa: initial support for MaxLinear MxL862xx switches

[Daniel Golle]

Hi,

This series adds very basic DSA support for the MaxLinear MxL86252
(5 PHY ports) and MxL86282 (8 PHY ports) switches. The intent is to
validate and get feedback on the overall approach and driver structure,
especially the firmware-mediated host interface.

MxL862xx integrates a firmware running on an embedded processor (Zephyr
RTOS). Host interaction uses a simple API transported over MDIO/MMD.
This series includes only what's needed to pass traffic between user
ports and the CPU port: relayed MDIO to internal PHYs, basic port
enable/disable, and CPU-port special tagging.

Thanks for taking a look.

Daniel Golle (3):
  dt-bindings: net: dsa: add bindings for MaxLinear MxL862xx
  net: dsa: add tag formats for MxL862xx switches
  net: dsa: add basic initial driver for MxL862xx switches

 .../bindings/net/dsa/maxlinear,mxl862xx.yaml  | 160 ++++++++
 MAINTAINERS                                   |   8 +
 drivers/net/dsa/Kconfig                       |   2 +
 drivers/net/dsa/Makefile                      |   1 +
 drivers/net/dsa/mxl862xx/Kconfig              |  12 +
 drivers/net/dsa/mxl862xx/Makefile             |   3 +
 drivers/net/dsa/mxl862xx/mxl862xx-api.h       | 104 +++++
 drivers/net/dsa/mxl862xx/mxl862xx-cmd.h       |  28 ++
 drivers/net/dsa/mxl862xx/mxl862xx-host.c      | 204 ++++++++++
 drivers/net/dsa/mxl862xx/mxl862xx-host.h      |   3 +
 drivers/net/dsa/mxl862xx/mxl862xx.c           | 360 ++++++++++++++++++
 drivers/net/dsa/mxl862xx/mxl862xx.h           |  27 ++
 include/net/dsa.h                             |   2 +
 net/dsa/Kconfig                               |   7 +
 net/dsa/Makefile                              |   1 +
 net/dsa/tag_mxl862xx.c                        | 109 ++++++
 16 files changed, 1031 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/dsa/maxlinear,mxl862xx.yaml
 create mode 100644 drivers/net/dsa/mxl862xx/Kconfig
 create mode 100644 drivers/net/dsa/mxl862xx/Makefile
 create mode 100644 drivers/net/dsa/mxl862xx/mxl862xx-api.h
 create mode 100644 drivers/net/dsa/mxl862xx/mxl862xx-cmd.h
 create mode 100644 drivers/net/dsa/mxl862xx/mxl862xx-host.c
 create mode 100644 drivers/net/dsa/mxl862xx/mxl862xx-host.h
 create mode 100644 drivers/net/dsa/mxl862xx/mxl862xx.c
 create mode 100644 drivers/net/dsa/mxl862xx/mxl862xx.h
 create mode 100644 net/dsa/tag_mxl862xx.c

-- 
2.52.0

[PATCH RFC net-next 1/3] dt-bindings: net: dsa: add bindings for MaxLinear MxL862xx

[Daniel Golle]

Add documentation and an example for MaxLinear MxL86282 and MxL86252
switches.

Signed-off-by: Daniel Golle <daniel@makrotopia.org>
---
 .../bindings/net/dsa/maxlinear,mxl862xx.yaml  | 160 ++++++++++++++++++
 MAINTAINERS                                   |   6 +
 2 files changed, 166 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/dsa/maxlinear,mxl862xx.yaml

diff --git a/Documentation/devicetree/bindings/net/dsa/maxlinear,mxl862xx.yaml b/Documentation/devicetree/bindings/net/dsa/maxlinear,mxl862xx.yaml
new file mode 100644
index 0000000000000..08d9b7f477fa4
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/dsa/maxlinear,mxl862xx.yaml
@@ -0,0 +1,160 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/dsa/maxlinear,mxl862xx.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: MaxLinear MxL862xx Ethernet Switch Family
+
+maintainers:
+  - Daniel Golle <daniel@makrotopia.org>
+
+description:
+  The MaxLinear MxL862xx switch family are multi-port Ethernet switches with
+  integrated 2.5GE PHYs. The MxL86252 has five PHY ports and the MxL86282 has
+  eight PHY ports. They are controlled via MDIO and support DSA tagging.
+
+allOf:
+  - $ref: dsa.yaml#/$defs/ethernet-ports
+
+properties:
+  compatible:
+    enum:
+      - maxlinear,mxl86252
+      - maxlinear,mxl86282
+
+  reg:
+    maxItems: 1
+    description: MDIO address of the switch
+
+  mdio:
+    $ref: /schemas/net/mdio.yaml#
+    unevaluatedProperties: false
+
+required:
+  - compatible
+  - reg
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    mdio {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        switch@0 {
+            compatible = "maxlinear,mxl86282";
+            reg = <0>;
+
+            ethernet-ports {
+                #address-cells = <1>;
+                #size-cells = <0>;
+
+                port@0 {
+                    reg = <0>;
+                    label = "lan1";
+                    phy-handle = <&phy0>;
+                    phy-mode = "internal";
+                };
+
+                port@1 {
+                    reg = <1>;
+                    label = "lan2";
+                    phy-handle = <&phy1>;
+                    phy-mode = "internal";
+                };
+
+                port@2 {
+                    reg = <2>;
+                    label = "lan3";
+                    phy-handle = <&phy2>;
+                    phy-mode = "internal";
+                };
+
+                port@3 {
+                    reg = <3>;
+                    label = "lan4";
+                    phy-handle = <&phy3>;
+                    phy-mode = "internal";
+                };
+
+                port@4 {
+                    reg = <4>;
+                    label = "lan5";
+                    phy-handle = <&phy4>;
+                    phy-mode = "internal";
+                };
+
+                port@5 {
+                    reg = <5>;
+                    label = "lan6";
+                    phy-handle = <&phy5>;
+                    phy-mode = "internal";
+                };
+
+                port@6 {
+                    reg = <6>;
+                    label = "lan7";
+                    phy-handle = <&phy6>;
+                    phy-mode = "internal";
+                };
+
+                port@7 {
+                    reg = <7>;
+                    label = "lan8";
+                    phy-handle = <&phy7>;
+                    phy-mode = "internal";
+                };
+
+                port@8 {
+                    reg = <8>;
+                    label = "cpu";
+                    ethernet = <&gmac0>;
+                    phy-mode = "usxgmii";
+
+                    fixed-link {
+                        speed = <10000>;
+                        full-duplex;
+                    };
+                };
+            };
+
+            mdio {
+                #address-cells = <1>;
+                #size-cells = <0>;
+
+                phy0: ethernet-phy@0 {
+                    reg = <0>;
+                };
+
+                phy1: ethernet-phy@1 {
+                    reg = <1>;
+                };
+
+                phy2: ethernet-phy@2 {
+                    reg = <2>;
+                };
+
+                phy3: ethernet-phy@3 {
+                    reg = <3>;
+                };
+
+                phy4: ethernet-phy@4 {
+                    reg = <4>;
+                };
+
+                phy5: ethernet-phy@5 {
+                    reg = <5>;
+                };
+
+                phy6: ethernet-phy@6 {
+                    reg = <6>;
+                };
+
+                phy7: ethernet-phy@7 {
+                    reg = <7>;
+                };
+            };
+        };
+    };
diff --git a/MAINTAINERS b/MAINTAINERS
index 9c0d30217516a..de0f753080f22 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -15413,6 +15413,12 @@ S:	Supported
 F:	drivers/net/phy/mxl-86110.c
 F:	drivers/net/phy/mxl-gpy.c
 
+MAXLINEAR MXL862XX SWITCH DRIVER
+M:	Daniel Golle <daniel@makrotopia.org>
+L:	netdev@vger.kernel.org
+S:	Maintained
+F:	Documentation/devicetree/bindings/net/dsa/maxlinear,mxl862xx.yaml
+
 MCAN DEVICE DRIVER
 M:	Markus Schneider-Pargmann <msp@baylibre.com>
 L:	linux-can@vger.kernel.org
-- 
2.52.0

[PATCH RFC net-next 2/3] net: dsa: add tag formats for MxL862xx switches

[Daniel Golle]

Add proprietary special tag format for the MaxLinear MXL862xx family of
switches. While using the same Ethertype as MaxLinear's GSW1xx swtiches,
the actual tag format differs significantly, hence we need a dedicated
tag driver for that.

Signed-off-by: Daniel Golle <daniel@makrotopia.org>
---
 MAINTAINERS            |   1 +
 include/net/dsa.h      |   2 +
 net/dsa/Kconfig        |   7 +++
 net/dsa/Makefile       |   1 +
 net/dsa/tag_mxl862xx.c | 109 +++++++++++++++++++++++++++++++++++++++++
 5 files changed, 120 insertions(+)
 create mode 100644 net/dsa/tag_mxl862xx.c

diff --git a/MAINTAINERS b/MAINTAINERS
index de0f753080f22..a5ca0f08938fd 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -15418,6 +15418,7 @@ M:	Daniel Golle <daniel@makrotopia.org>
 L:	netdev@vger.kernel.org
 S:	Maintained
 F:	Documentation/devicetree/bindings/net/dsa/maxlinear,mxl862xx.yaml
+F:	net/dsa/tag_mxl862xx.c
 
 MCAN DEVICE DRIVER
 M:	Markus Schneider-Pargmann <msp@baylibre.com>
diff --git a/include/net/dsa.h b/include/net/dsa.h
index cced1a8667578..799dd10845aef 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -57,6 +57,7 @@ struct tc_action;
 #define DSA_TAG_PROTO_BRCM_LEGACY_FCS_VALUE	29
 #define DSA_TAG_PROTO_YT921X_VALUE		30
 #define DSA_TAG_PROTO_MXL_GSW1XX_VALUE		31
+#define DSA_TAG_PROTO_MXL862_VALUE		32
 
 enum dsa_tag_protocol {
 	DSA_TAG_PROTO_NONE		= DSA_TAG_PROTO_NONE_VALUE,
@@ -91,6 +92,7 @@ enum dsa_tag_protocol {
 	DSA_TAG_PROTO_VSC73XX_8021Q	= DSA_TAG_PROTO_VSC73XX_8021Q_VALUE,
 	DSA_TAG_PROTO_YT921X		= DSA_TAG_PROTO_YT921X_VALUE,
 	DSA_TAG_PROTO_MXL_GSW1XX	= DSA_TAG_PROTO_MXL_GSW1XX_VALUE,
+	DSA_TAG_PROTO_MXL862		= DSA_TAG_PROTO_MXL862_VALUE,
 };
 
 struct dsa_switch;
diff --git a/net/dsa/Kconfig b/net/dsa/Kconfig
index f86b30742122f..c897d62326f5b 100644
--- a/net/dsa/Kconfig
+++ b/net/dsa/Kconfig
@@ -145,6 +145,13 @@ config NET_DSA_TAG_QCA
 	  Say Y or M if you want to enable support for tagging frames for
 	  the Qualcomm Atheros QCA8K switches.
 
+config NET_DSA_TAG_MXL862
+	tristate "Tag driver for MxL862xx switches"
+	help
+	  Say Y or M if you want to enable support for tagging frames for the
+	  Maxlinear MxL86252 and MxL86282 switches using their native 8-byte
+	  tagging protocol.
+
 config NET_DSA_TAG_RTL4_A
 	tristate "Tag driver for Realtek 4 byte protocol A tags"
 	help
diff --git a/net/dsa/Makefile b/net/dsa/Makefile
index 42d173f5a7013..dbe2a742e3322 100644
--- a/net/dsa/Makefile
+++ b/net/dsa/Makefile
@@ -28,6 +28,7 @@ obj-$(CONFIG_NET_DSA_TAG_HELLCREEK) += tag_hellcreek.o
 obj-$(CONFIG_NET_DSA_TAG_KSZ) += tag_ksz.o
 obj-$(CONFIG_NET_DSA_TAG_LAN9303) += tag_lan9303.o
 obj-$(CONFIG_NET_DSA_TAG_MTK) += tag_mtk.o
+obj-$(CONFIG_NET_DSA_TAG_MXL862) += tag_mxl862xx.o
 obj-$(CONFIG_NET_DSA_TAG_MXL_GSW1XX) += tag_mxl-gsw1xx.o
 obj-$(CONFIG_NET_DSA_TAG_NONE) += tag_none.o
 obj-$(CONFIG_NET_DSA_TAG_OCELOT) += tag_ocelot.o
diff --git a/net/dsa/tag_mxl862xx.c b/net/dsa/tag_mxl862xx.c
new file mode 100644
index 0000000000000..335bf45f60e67
--- /dev/null
+++ b/net/dsa/tag_mxl862xx.c
@@ -0,0 +1,109 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * DSA Special Tag for MaxLinear 862xx switch chips
+ *
+ * Copyright (C) 2025 Daniel Golle <daniel@makrotopia.org>
+ * Copyright (C) 2024 MaxLinear Inc.
+ */
+
+#include <linux/bitops.h>
+#include <linux/etherdevice.h>
+#include <linux/skbuff.h>
+#include <net/dsa.h>
+#include "tag.h"
+
+#define MXL862_NAME	"mxl862xx"
+
+/* To define the outgoing port and to discover the incoming port a special
+ * tag is used by the GSW1xx.
+ *
+ *       Dest MAC       Src MAC    special TAG        EtherType
+ * ...| 1 2 3 4 5 6 | 1 2 3 4 5 6 | 1 2 3 4 5 6 7 8 | 1 2 |...
+ *                                |<--------------->|
+ */
+
+#define MXL862_HEADER_LEN 8
+
+/* Byte 7 */
+#define MXL862_IGP_EGP GENMASK(3, 0)
+
+static struct sk_buff *mxl862_tag_xmit(struct sk_buff *skb,
+				       struct net_device *dev)
+{
+	struct dsa_port *dp = dsa_user_to_port(dev);
+	struct dsa_port *cpu_dp = dp->cpu_dp;
+	unsigned int cpu_port = cpu_dp->index + 1;
+	unsigned int usr_port = dp->index + 1;
+	__be16 *mxl862_tag;
+
+	if (!skb)
+		return skb;
+
+	/* provide additional space 'MXL862_HEADER_LEN' bytes */
+	skb_push(skb, MXL862_HEADER_LEN);
+
+	/* shift MAC address to the beginnig of the enlarged buffer,
+	 * releasing the space required for DSA tag (between MAC address and
+	 * Ethertype)
+	 */
+	dsa_alloc_etype_header(skb, MXL862_HEADER_LEN);
+
+	/* special tag ingress */
+	mxl862_tag = dsa_etype_header_pos_tx(skb);
+	mxl862_tag[0] = htons(ETH_P_MXLGSW);
+	mxl862_tag[2] = htons(usr_port + 16 - cpu_port);
+	mxl862_tag[3] = htons(FIELD_PREP(MXL862_IGP_EGP, cpu_port));
+
+	return skb;
+}
+
+static struct sk_buff *mxl862_tag_rcv(struct sk_buff *skb,
+				      struct net_device *dev)
+{
+	int port;
+	__be16 *mxl862_tag;
+
+	if (unlikely(!pskb_may_pull(skb, MXL862_HEADER_LEN))) {
+		dev_warn_ratelimited(&dev->dev, "Cannot pull SKB, packet dropped\n");
+		return NULL;
+	}
+
+	mxl862_tag = dsa_etype_header_pos_rx(skb);
+
+	if (unlikely(mxl862_tag[0] != htons(ETH_P_MXLGSW))) {
+		dev_warn_ratelimited(&dev->dev, "Invalid special tag marker, packet dropped\n");
+		dev_warn_ratelimited(&dev->dev, "Rx Packet Tag: %8ph\n", mxl862_tag);
+		return NULL;
+	}
+
+	/* Get source port information */
+	port = FIELD_GET(MXL862_IGP_EGP, ntohs(mxl862_tag[3]));
+	port = port - 1;
+	skb->dev = dsa_conduit_find_user(dev, 0, port);
+	if (!skb->dev) {
+		dev_warn_ratelimited(&dev->dev, "Invalid source port, packet dropped\n");
+		dev_warn_ratelimited(&dev->dev, "Rx Packet Tag: %8ph\n", mxl862_tag);
+		return NULL;
+	}
+
+	/* remove the MxL862xx special tag between the MAC addresses and the
+	 * current ethertype field.
+	 */
+	skb_pull_rcsum(skb, MXL862_HEADER_LEN);
+	dsa_strip_etype_header(skb, MXL862_HEADER_LEN);
+
+	return skb;
+}
+
+static const struct dsa_device_ops mxl862_netdev_ops = {
+	.name = "mxl862",
+	.proto = DSA_TAG_PROTO_MXL862,
+	.xmit = mxl862_tag_xmit,
+	.rcv = mxl862_tag_rcv,
+	.needed_headroom = MXL862_HEADER_LEN,
+};
+
+MODULE_LICENSE("GPL");
+MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_MXL862, MXL862_NAME);
+
+module_dsa_tag_driver(mxl862_netdev_ops);
-- 
2.52.0

Re: [PATCH RFC net-next 2/3] net: dsa: add tag formats for MxL862xx switches

[Andrew Lunn]

> +	/* special tag ingress */
> +	mxl862_tag = dsa_etype_header_pos_tx(skb);
> +	mxl862_tag[0] = htons(ETH_P_MXLGSW);
> +	mxl862_tag[2] = htons(usr_port + 16 - cpu_port);
> +	mxl862_tag[3] = htons(FIELD_PREP(MXL862_IGP_EGP, cpu_port));

You appear to be leaving mxl862_tag[1] uninitialised. Is that
intentional?

This is a pretty odd tag format. The destination port is relative to
the CPU port? You need to include the CPU port in the tag itself? Is
this publicly documented somewhere?

	Andrew

[PATCH RFC net-next 3/3] net: dsa: add basic initial driver for MxL862xx switches

[Daniel Golle]

Add very basic DSA driver for MaxLinear's MxL862xx switches.

In contrast to previous MaxLinear switches the MxL862xx has a built-in
processor that runs a sophisticated firmware based on Zephyr RTOS.
Interaction between the host and the switch hence is organized using a
software API of that firmware rather than accessing hardware registers
directly.

Add descriptions of the most basic firmware API calls to access the
built-in MDIO bus hosting the 2.5GE PHYs, basic port control as well as
setting up the CPU port.

Implement a very basic DSA driver using that API which is sufficient to
get packets flowing between the user ports and the CPU port.

The firmware offers all features one would expect from a modern switch
hardware, they will be added one by one in follow-up patch series.

Signed-off-by: Daniel Golle <daniel@makrotopia.org>
---
 MAINTAINERS                              |   1 +
 drivers/net/dsa/Kconfig                  |   2 +
 drivers/net/dsa/Makefile                 |   1 +
 drivers/net/dsa/mxl862xx/Kconfig         |  12 +
 drivers/net/dsa/mxl862xx/Makefile        |   3 +
 drivers/net/dsa/mxl862xx/mxl862xx-api.h  | 104 +++++++
 drivers/net/dsa/mxl862xx/mxl862xx-cmd.h  |  28 ++
 drivers/net/dsa/mxl862xx/mxl862xx-host.c | 204 +++++++++++++
 drivers/net/dsa/mxl862xx/mxl862xx-host.h |   3 +
 drivers/net/dsa/mxl862xx/mxl862xx.c      | 360 +++++++++++++++++++++++
 drivers/net/dsa/mxl862xx/mxl862xx.h      |  27 ++
 11 files changed, 745 insertions(+)
 create mode 100644 drivers/net/dsa/mxl862xx/Kconfig
 create mode 100644 drivers/net/dsa/mxl862xx/Makefile
 create mode 100644 drivers/net/dsa/mxl862xx/mxl862xx-api.h
 create mode 100644 drivers/net/dsa/mxl862xx/mxl862xx-cmd.h
 create mode 100644 drivers/net/dsa/mxl862xx/mxl862xx-host.c
 create mode 100644 drivers/net/dsa/mxl862xx/mxl862xx-host.h
 create mode 100644 drivers/net/dsa/mxl862xx/mxl862xx.c
 create mode 100644 drivers/net/dsa/mxl862xx/mxl862xx.h

diff --git a/MAINTAINERS b/MAINTAINERS
index a5ca0f08938fd..2ccca4828952d 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -15418,6 +15418,7 @@ M:	Daniel Golle <daniel@makrotopia.org>
 L:	netdev@vger.kernel.org
 S:	Maintained
 F:	Documentation/devicetree/bindings/net/dsa/maxlinear,mxl862xx.yaml
+F:	drivers/net/dsa/mxl862xx/
 F:	net/dsa/tag_mxl862xx.c
 
 MCAN DEVICE DRIVER
diff --git a/drivers/net/dsa/Kconfig b/drivers/net/dsa/Kconfig
index 7eb301fd987d1..18f6e8b7f4cb2 100644
--- a/drivers/net/dsa/Kconfig
+++ b/drivers/net/dsa/Kconfig
@@ -74,6 +74,8 @@ source "drivers/net/dsa/microchip/Kconfig"
 
 source "drivers/net/dsa/mv88e6xxx/Kconfig"
 
+source "drivers/net/dsa/mxl862xx/Kconfig"
+
 source "drivers/net/dsa/ocelot/Kconfig"
 
 source "drivers/net/dsa/qca/Kconfig"
diff --git a/drivers/net/dsa/Makefile b/drivers/net/dsa/Makefile
index 16de4ba3fa388..f5a463b87ec25 100644
--- a/drivers/net/dsa/Makefile
+++ b/drivers/net/dsa/Makefile
@@ -20,6 +20,7 @@ obj-y				+= hirschmann/
 obj-y				+= lantiq/
 obj-y				+= microchip/
 obj-y				+= mv88e6xxx/
+obj-y				+= mxl862xx/
 obj-y				+= ocelot/
 obj-y				+= qca/
 obj-y				+= realtek/
diff --git a/drivers/net/dsa/mxl862xx/Kconfig b/drivers/net/dsa/mxl862xx/Kconfig
new file mode 100644
index 0000000000000..5c538dfc2763e
--- /dev/null
+++ b/drivers/net/dsa/mxl862xx/Kconfig
@@ -0,0 +1,12 @@
+# SPDX-License-Identifier: GPL-2.0-only
+config NET_DSA_MXL862
+	tristate "MaxLinear MxL862xx"
+	depends on NET_DSA
+	select MAXLINEAR_GPHY
+	select NET_DSA_TAG_MXL862
+	help
+	  This enables support for the MaxLinear MxL862xx switch family.
+	  These switches got two 10GE SerDes interfaces, one typically
+	  used as CPU port.
+	   MxL86282 Eight 2.5 Gigabit PHYs
+	   MxL86252 Five 2.5 Gigabit PHYs
\ No newline at end of file
diff --git a/drivers/net/dsa/mxl862xx/Makefile b/drivers/net/dsa/mxl862xx/Makefile
new file mode 100644
index 0000000000000..d23dd3cd511d4
--- /dev/null
+++ b/drivers/net/dsa/mxl862xx/Makefile
@@ -0,0 +1,3 @@
+# SPDX-License-Identifier: GPL-2.0
+obj-$(CONFIG_NET_DSA_MXL862) += mxl862xx_dsa.o
+mxl862xx_dsa-y := mxl862xx.o mxl862xx-host.o
diff --git a/drivers/net/dsa/mxl862xx/mxl862xx-api.h b/drivers/net/dsa/mxl862xx/mxl862xx-api.h
new file mode 100644
index 0000000000000..8efe7c535fa5a
--- /dev/null
+++ b/drivers/net/dsa/mxl862xx/mxl862xx-api.h
@@ -0,0 +1,104 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/**
+ * struct mdio_relay_data - relayed access to the switch internal MDIO bus
+ * @data: data to be read or written
+ * @phy: PHY index
+ * @mmd: MMD device
+ * @reg: register rndex
+ */
+struct mdio_relay_data {
+	__le16 data;
+	u8 phy;
+	u8 mmd;
+	__le16 reg;
+} __packed;
+
+/* Register access parameter to directly modify internal registers */
+struct mxl862xx_register_mod {
+	__le16 addr;
+	__le16 data;
+	__le16 mask;
+} __packed;
+
+/**
+ * struct mxl862xx_ss_sp_tag
+ * @pid: port ID (1~16)
+ * @mask: bit value 1 to indicate valid field
+ *	0 - rx
+ *	1 - tx
+ *	2 - rx_pen
+ *	3 - tx_pen
+ * @rx: RX special tag mode
+ *	0 - packet does NOT have special tag and special tag is NOT inserted
+ *	1 - packet does NOT have special tag and special tag is inserted
+ *	2 - packet has special tag and special tag is NOT inserted
+ * @tx: TX special tag mode
+ *	0 - packet does NOT have special tag and special tag is NOT removed
+ *	1 - packet has special tag and special tag is replaced
+ *	2 - packet has special tag and special tag is NOT removed
+ *	3 - packet has special tag and special tag is removed
+ * @rx_pen: RX special tag info over preamble
+ *	0 - special tag info inserted from byte 2 to 7 are all 0
+ *	1 - special tag byte 5 is 16, other bytes from 2 to 7 are 0
+ *	2 - special tag byte 5 is from preamble field, others are 0
+ *	3 - special tag byte 2 to 7 are from preabmle field
+ * @tx_pen: TX special tag info over preamble
+ *	0 - disabled
+ *	1 - enabled
+ */
+struct mxl862xx_ss_sp_tag {
+	u8 pid;
+	u8 mask;
+	u8 rx;
+	u8 tx;
+	u8 rx_pen;
+	u8 tx_pen;
+} __packed;
+
+/**
+ * enum mxl862xx_logical_port_mode - Logical port mode
+ * @MXL862XX_LOGICAL_PORT_8BIT_WLAN: WLAN with 8-bit station ID
+ * @MXL862XX_LOGICAL_PORT_9BIT_WLAN: WLAN with 9-bit station ID
+ * @MXL862XX_LOGICAL_PORT_GPON: GPON OMCI context
+ * @MXL862XX_LOGICAL_PORT_EPON: EPON context
+ * @MXL862XX_LOGICAL_PORT_GINT: G.INT context
+ * @MXL862XX_LOGICAL_PORT_OTHER: Others
+ */
+enum mxl862xx_logical_port_mode {
+	MXL862XX_LOGICAL_PORT_8BIT_WLAN = 0,
+	MXL862XX_LOGICAL_PORT_9BIT_WLAN,
+	MXL862XX_LOGICAL_PORT_GPON,
+	MXL862XX_LOGICAL_PORT_EPON,
+	MXL862XX_LOGICAL_PORT_GINT,
+	MXL862XX_LOGICAL_PORT_OTHER = 0xFF,
+};
+
+/**
+ * struct mxl862xx_ctp_port_assignment - CTP Port Assignment/association with logical port
+ * @logical_port_id: Logical Port Id. The valid range is hardware dependent
+ * @first_ctp_port_id: First CTP Port ID mapped to above logical port ID
+ * @number_of_ctp_port: Total number of CTP Ports mapped above logical port ID
+ * @mode: See &enum mxl862xx_logical_port_mode
+ * @bridge_port_id: Bridge ID (FID)
+ */
+struct mxl862xx_ctp_port_assignment {
+	u8 logical_port_id;
+	__le16 first_ctp_port_id;
+	__le16 number_of_ctp_port;
+	enum mxl862xx_logical_port_mode mode;
+	__le16 bridge_port_id;
+} __packed;
+
+/**
+ * struct mxl862xx_sys_fw_image_version - VLAN counter mapping configuration
+ * @iv_major: firmware major version
+ * @iv_minor: firmware minor version
+ * @iv_revision: firmware revision
+ * @iv_build_num: firmware build number
+ */
+struct mxl862xx_sys_fw_image_version {
+	u8 iv_major;
+	u8 iv_minor;
+	__le16 iv_revision;
+	__le32 iv_build_num;
+} __packed;
diff --git a/drivers/net/dsa/mxl862xx/mxl862xx-cmd.h b/drivers/net/dsa/mxl862xx/mxl862xx-cmd.h
new file mode 100644
index 0000000000000..db6a4c3f54f22
--- /dev/null
+++ b/drivers/net/dsa/mxl862xx/mxl862xx-cmd.h
@@ -0,0 +1,28 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+
+#define MXL862XX_MMD_DEV 30
+#define MXL862XX_MMD_REG_CTRL 0
+#define MXL862XX_MMD_REG_LEN_RET 1
+#define MXL862XX_MMD_REG_DATA_FIRST 2
+#define MXL862XX_MMD_REG_DATA_LAST 95
+#define MXL862XX_MMD_REG_DATA_MAX_SIZE \
+	(MXL862XX_MMD_REG_DATA_LAST - MXL862XX_MMD_REG_DATA_FIRST + 1)
+
+#define MXL862XX_COMMON_MAGIC 0x100
+#define MXL862XX_CTP_MAGIC 0x500
+#define MXL862XX_SS_MAGIC 0x1600
+#define GPY_GPY2XX_MAGIC 0x1800
+#define SYS_MISC_MAGIC 0x1900
+
+#define MXL862XX_COMMON_REGISTERMOD (MXL862XX_COMMON_MAGIC + 0x11)
+
+#define MXL862XX_CTP_PORTASSIGNMENTSET (MXL862XX_CTP_MAGIC + 0x3)
+
+#define MXL862XX_SS_SPTAG_SET (MXL862XX_SS_MAGIC + 0x02)
+
+#define INT_GPHY_READ (GPY_GPY2XX_MAGIC + 0x01)
+#define INT_GPHY_WRITE (GPY_GPY2XX_MAGIC + 0x02)
+
+#define SYS_MISC_FW_VERSION (SYS_MISC_MAGIC + 0x02)
+
+#define MMD_API_MAXIMUM_ID 0x7FFF
diff --git a/drivers/net/dsa/mxl862xx/mxl862xx-host.c b/drivers/net/dsa/mxl862xx/mxl862xx-host.c
new file mode 100644
index 0000000000000..7c407329f8184
--- /dev/null
+++ b/drivers/net/dsa/mxl862xx/mxl862xx-host.c
@@ -0,0 +1,204 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Based upon the Maxlinear SDK driver
+ *
+ * Copyright (C) 2025 Daniel Golle <daniel@makrotopia.org>
+ * Copyright (C) 2025 John Crispin <john@phrozen.org>
+ * Copyright (C) 2024 MaxLinear Inc.
+ */
+
+#include <linux/bits.h>
+#include <net/dsa.h>
+#include "mxl862xx.h"
+#include "mxl862xx-host.h"
+
+#define CTRL_BUSY_MASK BIT(15)
+#define CTRL_CMD_MASK (BIT(15) - 1)
+
+#define MAX_BUSY_LOOP 1000 /* roughly 10ms */
+
+#define MXL862XX_MMD_DEV 30
+#define MXL862XX_MMD_REG_CTRL 0
+#define MXL862XX_MMD_REG_LEN_RET 1
+#define MXL862XX_MMD_REG_DATA_FIRST 2
+#define MXL862XX_MMD_REG_DATA_LAST 95
+#define MXL862XX_MMD_REG_DATA_MAX_SIZE \
+		(MXL862XX_MMD_REG_DATA_LAST - MXL862XX_MMD_REG_DATA_FIRST + 1)
+
+#define MMD_API_SET_DATA_0 (0x0 + 0x2)
+#define MMD_API_GET_DATA_0 (0x0 + 0x5)
+#define MMD_API_RST_DATA (0x0 + 0x8)
+
+static int mxl862xx_busy_wait(struct mxl862xx_priv *dev)
+{
+	int ret, i;
+
+	for (i = 0; i < MAX_BUSY_LOOP; i++) {
+		ret = __mdiobus_c45_read(dev->bus, dev->sw_addr,
+					 MXL862XX_MMD_DEV,
+					 MXL862XX_MMD_REG_CTRL);
+		if (ret < 0)
+			return ret;
+
+		if (ret & CTRL_BUSY_MASK)
+			usleep_range(10, 15);
+		else
+			return 0;
+	}
+
+	return -ETIMEDOUT;
+}
+
+static int mxl862xx_set_data(struct mxl862xx_priv *dev, u16 words)
+{
+	int ret;
+	u16 cmd;
+
+	ret = __mdiobus_c45_write(dev->bus, dev->sw_addr, MXL862XX_MMD_DEV,
+				  MXL862XX_MMD_REG_LEN_RET,
+				  MXL862XX_MMD_REG_DATA_MAX_SIZE * sizeof(u16));
+	if (ret < 0)
+		return ret;
+
+	cmd = words / MXL862XX_MMD_REG_DATA_MAX_SIZE - 1;
+	if (!(cmd < 2))
+		return -EINVAL;
+
+	cmd += MMD_API_SET_DATA_0;
+	ret = __mdiobus_c45_write(dev->bus, dev->sw_addr, MXL862XX_MMD_DEV,
+				  MXL862XX_MMD_REG_CTRL, cmd | CTRL_BUSY_MASK);
+	if (ret < 0)
+		return ret;
+
+	return mxl862xx_busy_wait(dev);
+}
+
+static int mxl862xx_get_data(struct mxl862xx_priv *dev, u16 words)
+{
+	int ret;
+	u16 cmd;
+
+	ret = __mdiobus_c45_write(dev->bus, dev->sw_addr, MXL862XX_MMD_DEV,
+				  MXL862XX_MMD_REG_LEN_RET,
+				  MXL862XX_MMD_REG_DATA_MAX_SIZE * sizeof(u16));
+	if (ret < 0)
+		return ret;
+
+	cmd = words / MXL862XX_MMD_REG_DATA_MAX_SIZE;
+	if (!(cmd > 0 && cmd < 3))
+		return -EINVAL;
+
+	cmd += MMD_API_GET_DATA_0;
+	ret = __mdiobus_c45_write(dev->bus, dev->sw_addr, MXL862XX_MMD_DEV,
+				  MXL862XX_MMD_REG_CTRL, cmd | CTRL_BUSY_MASK);
+	if (ret < 0)
+		return ret;
+
+	return mxl862xx_busy_wait(dev);
+}
+
+static int mxl862xx_send_cmd(struct mxl862xx_priv *dev, u16 cmd, u16 size,
+			     s16 *presult)
+{
+	int ret;
+
+	ret = __mdiobus_c45_write(dev->bus, dev->sw_addr, MXL862XX_MMD_DEV,
+				  MXL862XX_MMD_REG_LEN_RET, size);
+	if (ret)
+		return ret;
+
+	ret = __mdiobus_c45_write(dev->bus, dev->sw_addr, MXL862XX_MMD_DEV,
+				  MXL862XX_MMD_REG_CTRL, cmd | CTRL_BUSY_MASK);
+	if (ret)
+		return ret;
+
+	ret = mxl862xx_busy_wait(dev);
+	if (ret)
+		return ret;
+
+	ret = __mdiobus_c45_read(dev->bus, dev->sw_addr, MXL862XX_MMD_DEV,
+				 MXL862XX_MMD_REG_LEN_RET);
+	if (ret < 0)
+		return ret;
+
+	*presult = ret;
+	return 0;
+}
+
+int mxl862xx_api_wrap(struct mxl862xx_priv *priv, u16 cmd, void *_data,
+		      u16 size, bool read)
+{
+	u16 *data = _data;
+	s16 result = 0;
+	u16 max, i;
+	int ret;
+
+	mutex_lock_nested(&priv->bus->mdio_lock, MDIO_MUTEX_NESTED);
+
+	max = (size + 1) / 2;
+
+	ret = mxl862xx_busy_wait(priv);
+	if (ret < 0)
+		goto out;
+
+	for (i = 0; i < max; i++) {
+		u16 off = i % MXL862XX_MMD_REG_DATA_MAX_SIZE;
+
+		if (i && off == 0) {
+			/* Send command to set data when every
+			 * MXL862XX_MMD_REG_DATA_MAX_SIZE of WORDs are written.
+			 */
+			ret = mxl862xx_set_data(priv, i);
+			if (ret < 0)
+				goto out;
+		}
+
+		__mdiobus_c45_write(priv->bus, priv->sw_addr,
+				    MXL862XX_MMD_DEV,
+				    MXL862XX_MMD_REG_DATA_FIRST + off,
+				    le16_to_cpu(data[i]));
+	}
+
+	ret = mxl862xx_send_cmd(priv, cmd, size, &result);
+	if (ret < 0)
+		goto out;
+
+	if (result < 0) {
+		ret = result;
+		goto out;
+	}
+
+	for (i = 0; i < max && read; i++) {
+		u16 off = i % MXL862XX_MMD_REG_DATA_MAX_SIZE;
+
+		if (i && off == 0) {
+			/* Send command to fetch next batch of data
+			 * when every MXL862XX_MMD_REG_DATA_MAX_SIZE of WORDs
+			 * are read.
+			 */
+			ret = mxl862xx_get_data(priv, i);
+			if (ret < 0)
+				goto out;
+		}
+
+		ret = __mdiobus_c45_read(priv->bus, priv->sw_addr,
+					 MXL862XX_MMD_DEV,
+					 MXL862XX_MMD_REG_DATA_FIRST + off);
+		if (ret < 0)
+			goto out;
+
+		if ((i * 2 + 1) == size) {
+			/* Special handling for last BYTE
+			 * if it's not WORD aligned.
+			 */
+			*(uint8_t *)&data[i] = ret & 0xFF;
+		} else {
+			data[i] = cpu_to_le16((u16)ret);
+		}
+	}
+	ret = result;
+
+out:
+	mutex_unlock(&priv->bus->mdio_lock);
+	return ret;
+}
diff --git a/drivers/net/dsa/mxl862xx/mxl862xx-host.h b/drivers/net/dsa/mxl862xx/mxl862xx-host.h
new file mode 100644
index 0000000000000..0a05fe3c7e6bf
--- /dev/null
+++ b/drivers/net/dsa/mxl862xx/mxl862xx-host.h
@@ -0,0 +1,3 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+
+int mxl862xx_api_wrap(struct mxl862xx_priv *priv, u16 cmd, void *data, u16 size, bool read);
diff --git a/drivers/net/dsa/mxl862xx/mxl862xx.c b/drivers/net/dsa/mxl862xx/mxl862xx.c
new file mode 100644
index 0000000000000..0446455c36bb6
--- /dev/null
+++ b/drivers/net/dsa/mxl862xx/mxl862xx.c
@@ -0,0 +1,360 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Driver for MaxLinear MxL862xx switch family
+ *
+ * Copyright (C) 2024 MaxLinear Inc.
+ * Copyright (C) 2025 John Crispin <john@phrozen.org>
+ * Copyright (C) 2025 Daniel Golle <daniel@makrotopia.org>
+ */
+
+#include <linux/module.h>
+#include <linux/delay.h>
+#include <linux/of_device.h>
+#include <linux/of_mdio.h>
+#include <linux/phy.h>
+#include <linux/phylink.h>
+#include <net/dsa.h>
+
+#include "mxl862xx.h"
+#include "mxl862xx-api.h"
+#include "mxl862xx-cmd.h"
+#include "mxl862xx-host.h"
+
+#define MXL862XX_API_WRITE(dev, cmd, data) \
+	mxl862xx_api_wrap(dev, cmd, &(data), sizeof((data)), false)
+#define MXL862XX_API_READ(dev, cmd, data) \
+	mxl862xx_api_wrap(dev, cmd, &(data), sizeof((data)), true)
+
+#define DSA_MXL_PORT(port) ((port) + 1)
+
+#define MXL862XX_SDMA_PCTRLP(p) (0xBC0 + ((p) * 0x6))
+#define MXL862XX_SDMA_PCTRL_EN BIT(0)
+
+#define MXL862XX_FDMA_PCTRLP(p) (0xA80 + ((p) * 0x6))
+#define MXL862XX_FDMA_PCTRL_EN BIT(0)
+
+/* PHY access via firmware relay */
+static int mxl862xx_phy_read_mmd(struct mxl862xx_priv *priv, int port,
+				 int devadd, int reg)
+{
+	struct mdio_relay_data param = {
+		.phy = port,
+		.mmd = devadd,
+		.reg = reg & 0xffff,
+	};
+	int ret;
+
+	ret = MXL862XX_API_READ(priv, INT_GPHY_READ, param);
+	if (ret)
+		return ret;
+
+	return param.data;
+}
+
+static int mxl862xx_phy_write_mmd(struct mxl862xx_priv *priv, int port,
+				  int devadd, int reg, u16 data)
+{
+	struct mdio_relay_data param = {
+		.phy = port,
+		.mmd = devadd,
+		.reg = reg,
+		.data = data,
+	};
+
+	return MXL862XX_API_WRITE(priv, INT_GPHY_WRITE, param);
+}
+
+static int mxl862xx_phy_read(struct dsa_switch *ds, int port, int reg)
+{
+	return mxl862xx_phy_read_mmd(ds->priv, port, 0, reg);
+}
+
+static int mxl862xx_phy_write(struct dsa_switch *ds, int port, int reg, u16 data)
+{
+	return mxl862xx_phy_write_mmd(ds->priv, port, 0, reg, data);
+}
+
+/* Configure CPU tagging */
+static int mxl862xx_configure_tag_proto(struct dsa_switch *ds, int port,
+					bool enable)
+{
+	struct mxl862xx_ss_sp_tag tag = {
+		.pid = DSA_MXL_PORT(port),
+		.mask = BIT(0) | BIT(1),
+		.rx = enable ? 2 : 1,
+		.tx = enable ? 2 : 3,
+	};
+	struct mxl862xx_ctp_port_assignment assign = {
+		.number_of_ctp_port = enable ? (32 - DSA_MXL_PORT(port)) : 1,
+		.logical_port_id = DSA_MXL_PORT(port),
+		.first_ctp_port_id = DSA_MXL_PORT(port),
+		.mode = MXL862XX_LOGICAL_PORT_GPON,
+	};
+	int ret;
+
+	ret = MXL862XX_API_WRITE(ds->priv, MXL862XX_SS_SPTAG_SET, tag);
+	if (ret)
+		return ret;
+
+	return MXL862XX_API_WRITE(ds->priv, MXL862XX_CTP_PORTASSIGNMENTSET, assign);
+}
+
+/* Port enable/disable */
+static int mxl862xx_port_state(struct dsa_switch *ds, int port, bool enable)
+{
+	struct mxl862xx_register_mod sdma = {
+		.addr = MXL862XX_SDMA_PCTRLP(DSA_MXL_PORT(port)),
+		.data = enable ? MXL862XX_SDMA_PCTRL_EN : 0,
+		.mask = MXL862XX_SDMA_PCTRL_EN,
+	};
+	struct mxl862xx_register_mod fdma = {
+		.addr = MXL862XX_FDMA_PCTRLP(DSA_MXL_PORT(port)),
+		.data = enable ? MXL862XX_FDMA_PCTRL_EN : 0,
+		.mask = MXL862XX_FDMA_PCTRL_EN,
+	};
+	int ret;
+
+	if (!dsa_is_user_port(ds, port))
+		return 0;
+
+	ret = MXL862XX_API_WRITE(ds->priv, MXL862XX_COMMON_REGISTERMOD, sdma);
+	if (ret)
+		return ret;
+
+	return MXL862XX_API_WRITE(ds->priv, MXL862XX_COMMON_REGISTERMOD, fdma);
+}
+
+static int mxl862xx_port_enable(struct dsa_switch *ds, int port,
+				struct phy_device *phydev)
+{
+	return mxl862xx_port_state(ds, port, true);
+}
+
+static void mxl862xx_port_disable(struct dsa_switch *ds, int port)
+{
+	mxl862xx_port_state(ds, port, false);
+}
+
+/* MDIO bus for PHYs */
+static int mxl862xx_phy_read_mii_bus(struct mii_bus *bus, int port, int regnum)
+{
+	return mxl862xx_phy_read_mmd(bus->priv, port, 0, regnum);
+}
+
+static int mxl862xx_phy_write_mii_bus(struct mii_bus *bus, int port,
+				      int regnum, u16 val)
+{
+	return mxl862xx_phy_write_mmd(bus->priv, port, 0, regnum, val);
+}
+
+static int mxl862xx_phy_read_c45_mii_bus(struct mii_bus *bus, int port,
+					 int devadd, int regnum)
+{
+	return mxl862xx_phy_read_mmd(bus->priv, port, devadd, regnum);
+}
+
+static int mxl862xx_phy_write_c45_mii_bus(struct mii_bus *bus, int port,
+					  int devadd, int regnum, u16 val)
+{
+	return mxl862xx_phy_write_mmd(bus->priv, port, devadd, regnum, val);
+}
+
+static int mxl862xx_setup_mdio(struct dsa_switch *ds)
+{
+	struct mxl862xx_priv *priv = ds->priv;
+	struct device *dev = ds->dev;
+	struct device_node *mdio_np;
+	struct mii_bus *bus;
+	static int idx;
+	int ret;
+
+	bus = devm_mdiobus_alloc(dev);
+	if (!bus)
+		return -ENOMEM;
+
+	bus->priv = priv;
+	ds->user_mii_bus = bus;
+	bus->name = KBUILD_MODNAME "-mii";
+	snprintf(bus->id, MII_BUS_ID_SIZE, KBUILD_MODNAME "-%d", idx++);
+	bus->read_c45 = mxl862xx_phy_read_c45_mii_bus;
+	bus->write_c45 = mxl862xx_phy_write_c45_mii_bus;
+	bus->read = mxl862xx_phy_read_mii_bus;
+	bus->write = mxl862xx_phy_write_mii_bus;
+	bus->parent = dev;
+	bus->phy_mask = ~ds->phys_mii_mask;
+
+	mdio_np = of_get_child_by_name(dev->of_node, "mdio");
+	if (!mdio_np)
+		return -ENODEV;
+
+	ret = devm_of_mdiobus_register(dev, bus, mdio_np);
+	of_node_put(mdio_np);
+
+	return ret;
+}
+
+/* Reset switch via MMD write */
+static int mxl862xx_mmd_write(struct dsa_switch *ds, int reg, u16 data)
+{
+	struct mxl862xx_priv *priv = ds->priv;
+	int ret;
+
+	mutex_lock(&priv->bus->mdio_lock);
+	ret = __mdiobus_c45_write(priv->bus, priv->sw_addr, MXL862XX_MMD_DEV,
+				  reg, data);
+	mutex_unlock(&priv->bus->mdio_lock);
+
+	return ret;
+}
+
+/* Phylink integration */
+static void mxl862xx_phylink_get_caps(struct dsa_switch *ds, int port,
+				      struct phylink_config *config)
+{
+	struct mxl862xx_priv *priv = ds->priv;
+
+	config->mac_capabilities = MAC_ASYM_PAUSE | MAC_SYM_PAUSE | MAC_10 |
+				   MAC_100 | MAC_1000 | MAC_2500FD;
+
+	if (port < priv->hw_info->phy_ports)
+		__set_bit(PHY_INTERFACE_MODE_INTERNAL,
+			  config->supported_interfaces);
+	else
+		__set_bit(PHY_INTERFACE_MODE_NA,
+			  config->supported_interfaces);
+}
+
+/* Tag protocol */
+static enum dsa_tag_protocol mxl862xx_get_tag_protocol(struct dsa_switch *ds,
+						       int port,
+						       enum dsa_tag_protocol m)
+{
+	return DSA_TAG_PROTO_MXL862;
+}
+
+/* Setup */
+static int mxl862xx_setup(struct dsa_switch *ds)
+{
+	struct mxl862xx_priv *priv = ds->priv;
+	int ret, i;
+
+	for (i = 0; i < ds->num_ports; i++) {
+		if (dsa_is_cpu_port(ds, i)) {
+			priv->cpu_port = i;
+			break;
+		}
+	}
+
+	ret = mxl862xx_setup_mdio(ds);
+	if (ret)
+		return ret;
+
+	/* Software reset */
+	ret = mxl862xx_mmd_write(ds, 1, 0);
+	if (ret)
+		return ret;
+
+	ret = mxl862xx_mmd_write(ds, 0, 0x9907);
+	if (ret)
+		return ret;
+
+	usleep_range(4000000, 6000000);
+
+	return mxl862xx_configure_tag_proto(ds, priv->cpu_port, true);
+}
+
+static const struct dsa_switch_ops mxl862xx_switch_ops = {
+	.get_tag_protocol = mxl862xx_get_tag_protocol,
+	.phylink_get_caps = mxl862xx_phylink_get_caps,
+	.phy_read = mxl862xx_phy_read,
+	.phy_write = mxl862xx_phy_write,
+	.port_disable = mxl862xx_port_disable,
+	.port_enable = mxl862xx_port_enable,
+	.setup = mxl862xx_setup,
+};
+
+/* Probe/remove */
+static int mxl862xx_probe(struct mdio_device *mdiodev)
+{
+	struct device *dev = &mdiodev->dev;
+	struct mxl862xx_priv *priv;
+	struct dsa_switch *ds;
+	struct mxl862xx_sys_fw_image_version fw;
+	int ret;
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->dev = dev;
+	priv->bus = mdiodev->bus;
+	priv->sw_addr = mdiodev->addr;
+	priv->hw_info = of_device_get_match_data(dev);
+	if (!priv->hw_info)
+		return -EINVAL;
+
+	ds = devm_kzalloc(dev, sizeof(*ds), GFP_KERNEL);
+	if (!ds)
+		return -ENOMEM;
+
+	priv->ds = ds;
+	ds->dev = dev;
+	ds->priv = priv;
+	ds->ops = &mxl862xx_switch_ops;
+	ds->num_ports = priv->hw_info->max_ports;
+
+	dev_set_drvdata(dev, ds);
+
+	ret = dsa_register_switch(ds);
+	if (ret)
+		return ret;
+
+	ret = MXL862XX_API_READ(priv, SYS_MISC_FW_VERSION, fw);
+	if (!ret)
+		dev_info(dev, "Firmware version %d.%d.%d.%d\n",
+			 fw.iv_major, fw.iv_minor,
+			 fw.iv_revision, fw.iv_build_num);
+
+	return 0;
+}
+
+static void mxl862xx_remove(struct mdio_device *mdiodev)
+{
+	struct dsa_switch *ds = dev_get_drvdata(&mdiodev->dev);
+
+	dsa_unregister_switch(ds);
+}
+
+static const struct mxl862xx_hw_info mxl86282_data = {
+	.max_ports = MXL862XX_MAX_PORT_NUM,
+	.phy_ports = MXL86282_PHY_PORT_NUM,
+	.ext_ports = MXL86282_EXT_PORT_NUM,
+};
+
+static const struct mxl862xx_hw_info mxl86252_data = {
+	.max_ports = MXL862XX_MAX_PORT_NUM,
+	.phy_ports = MXL86252_PHY_PORT_NUM,
+	.ext_ports = MXL86252_EXT_PORT_NUM,
+};
+
+static const struct of_device_id mxl862xx_of_match[] = {
+	{ .compatible = "maxlinear,mxl86282", .data = &mxl86282_data },
+	{ .compatible = "maxlinear,mxl86252", .data = &mxl86252_data },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, mxl862xx_of_match);
+
+static struct mdio_driver mxl862xx_driver = {
+	.probe  = mxl862xx_probe,
+	.remove = mxl862xx_remove,
+	.mdiodrv.driver = {
+		.name = "mxl862xx",
+		.of_match_table = mxl862xx_of_match,
+	},
+};
+
+mdio_module_driver(mxl862xx_driver);
+
+MODULE_DESCRIPTION("Minimal driver for MaxLinear MxL862xx switch family");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:mxl862xx");
diff --git a/drivers/net/dsa/mxl862xx/mxl862xx.h b/drivers/net/dsa/mxl862xx/mxl862xx.h
new file mode 100644
index 0000000000000..3ecde075212e2
--- /dev/null
+++ b/drivers/net/dsa/mxl862xx/mxl862xx.h
@@ -0,0 +1,27 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+
+#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)
+
+#define MXL86252_PHY_PORT_NUM		5
+#define MXL86282_PHY_PORT_NUM		8
+
+#define MXL86252_EXT_PORT_NUM		2
+#define MXL86282_EXT_PORT_NUM		2
+
+struct mxl862xx_hw_info {
+	u8 max_ports;
+	u8 phy_ports;
+	u8 ext_ports;
+};
+
+struct mxl862xx_priv {
+	struct dsa_switch *ds;
+	struct mii_bus *bus;
+	struct device *dev;
+	int sw_addr;
+	const struct mxl862xx_hw_info *hw_info;
+	u8 cpu_port;
+};
-- 
2.52.0

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

[Andrew Lunn]

> +/**
> + * struct mxl862xx_ss_sp_tag
> + * @pid: port ID (1~16)
> + * @mask: bit value 1 to indicate valid field
> + *	0 - rx
> + *	1 - tx
> + *	2 - rx_pen
> + *	3 - tx_pen
> + * @rx: RX special tag mode
> + *	0 - packet does NOT have special tag and special tag is NOT inserted
> + *	1 - packet does NOT have special tag and special tag is inserted
> + *	2 - packet has special tag and special tag is NOT inserted
> + * @tx: TX special tag mode
> + *	0 - packet does NOT have special tag and special tag is NOT removed
> + *	1 - packet has special tag and special tag is replaced
> + *	2 - packet has special tag and special tag is NOT removed
> + *	3 - packet has special tag and special tag is removed
> + * @rx_pen: RX special tag info over preamble
> + *	0 - special tag info inserted from byte 2 to 7 are all 0
> + *	1 - special tag byte 5 is 16, other bytes from 2 to 7 are 0
> + *	2 - special tag byte 5 is from preamble field, others are 0
> + *	3 - special tag byte 2 to 7 are from preabmle field
> + * @tx_pen: TX special tag info over preamble
> + *	0 - disabled
> + *	1 - enabled
> + */
> +struct mxl862xx_ss_sp_tag {
> +	u8 pid;
> +	u8 mask;
> +	u8 rx;
> +	u8 tx;
> +	u8 rx_pen;
> +	u8 tx_pen;
> +} __packed;
> +
> +/**
> + * enum mxl862xx_logical_port_mode - Logical port mode
> + * @MXL862XX_LOGICAL_PORT_8BIT_WLAN: WLAN with 8-bit station ID
> + * @MXL862XX_LOGICAL_PORT_9BIT_WLAN: WLAN with 9-bit station ID
> + * @MXL862XX_LOGICAL_PORT_GPON: GPON OMCI context
> + * @MXL862XX_LOGICAL_PORT_EPON: EPON context
> + * @MXL862XX_LOGICAL_PORT_GINT: G.INT context
> + * @MXL862XX_LOGICAL_PORT_OTHER: Others
> + */
> +enum mxl862xx_logical_port_mode {
> +	MXL862XX_LOGICAL_PORT_8BIT_WLAN = 0,
> +	MXL862XX_LOGICAL_PORT_9BIT_WLAN,
> +	MXL862XX_LOGICAL_PORT_GPON,
> +	MXL862XX_LOGICAL_PORT_EPON,
> +	MXL862XX_LOGICAL_PORT_GINT,
> +	MXL862XX_LOGICAL_PORT_OTHER = 0xFF,
> +};
> +
> +/**
> + * struct mxl862xx_ctp_port_assignment - CTP Port Assignment/association with logical port
> + * @logical_port_id: Logical Port Id. The valid range is hardware dependent
> + * @first_ctp_port_id: First CTP Port ID mapped to above logical port ID
> + * @number_of_ctp_port: Total number of CTP Ports mapped above logical port ID
> + * @mode: See &enum mxl862xx_logical_port_mode
> + * @bridge_port_id: Bridge ID (FID)
> + */
> +struct mxl862xx_ctp_port_assignment {
> +	u8 logical_port_id;
> +	__le16 first_ctp_port_id;
> +	__le16 number_of_ctp_port;
> +	enum mxl862xx_logical_port_mode mode;
> +	__le16 bridge_port_id;
> +} __packed;

Does the C standard define the size of an enum? Do you assume this is
a byte?


> +
> +/**
> + * struct mxl862xx_sys_fw_image_version - VLAN counter mapping configuration

The text seems wrong, probably cut/paste error?

> +#define MXL862XX_MMD_DEV 30

Please use MDIO_MMD_VEND1

> +#define MXL862XX_MMD_REG_CTRL 0
> +#define MXL862XX_MMD_REG_LEN_RET 1
> +#define MXL862XX_MMD_REG_DATA_FIRST 2
> +#define MXL862XX_MMD_REG_DATA_LAST 95
> +#define MXL862XX_MMD_REG_DATA_MAX_SIZE \
> +		(MXL862XX_MMD_REG_DATA_LAST - MXL862XX_MMD_REG_DATA_FIRST + 1)
> +
> +#define MMD_API_SET_DATA_0 (0x0 + 0x2)
> +#define MMD_API_GET_DATA_0 (0x0 + 0x5)
> +#define MMD_API_RST_DATA (0x0 + 0x8)

What is the significant of these numbers? Can you use #defines to make
it clearer?

> +
> +static int mxl862xx_busy_wait(struct mxl862xx_priv *dev)
> +{
> +	int ret, i;
> +
> +	for (i = 0; i < MAX_BUSY_LOOP; i++) {
> +		ret = __mdiobus_c45_read(dev->bus, dev->sw_addr,
> +					 MXL862XX_MMD_DEV,
> +					 MXL862XX_MMD_REG_CTRL);
> +		if (ret < 0)
> +			return ret;
> +
> +		if (ret & CTRL_BUSY_MASK)
> +			usleep_range(10, 15);
> +		else
> +			return 0;
> +	}
> +
> +	return -ETIMEDOUT;

We already have phy_read_mmd_poll_timeout(). Maybe you should add a
__mdiobus_c45_read_poll_timeout()?

Also, as far as i can see, __mdiobus_c45_read() is always called with
the same first three parameters. Maybe add a mxl862xx_reg_read()?  and
a mxl862xx_reg_write()?

> +static int mxl862xx_send_cmd(struct mxl862xx_priv *dev, u16 cmd, u16 size,
> +			     s16 *presult)
> +{
> +	int ret;
> +
> +	ret = __mdiobus_c45_write(dev->bus, dev->sw_addr, MXL862XX_MMD_DEV,
> +				  MXL862XX_MMD_REG_LEN_RET, size);
> +	if (ret)
> +		return ret;
> +
> +	ret = __mdiobus_c45_write(dev->bus, dev->sw_addr, MXL862XX_MMD_DEV,
> +				  MXL862XX_MMD_REG_CTRL, cmd | CTRL_BUSY_MASK);
> +	if (ret)
> +		return ret;
> +
> +	ret = mxl862xx_busy_wait(dev);
> +	if (ret)
> +		return ret;
> +
> +	ret = __mdiobus_c45_read(dev->bus, dev->sw_addr, MXL862XX_MMD_DEV,
> +				 MXL862XX_MMD_REG_LEN_RET);
> +	if (ret < 0)
> +		return ret;
> +
> +	*presult = ret;
> +	return 0;
> +}
> +
> +int mxl862xx_api_wrap(struct mxl862xx_priv *priv, u16 cmd, void *_data,
> +		      u16 size, bool read)
> +{
> +	u16 *data = _data;
> +	s16 result = 0;
> +	u16 max, i;
> +	int ret;
> +
> +	mutex_lock_nested(&priv->bus->mdio_lock, MDIO_MUTEX_NESTED);
> +
> +	max = (size + 1) / 2;
> +
> +	ret = mxl862xx_busy_wait(priv);
> +	if (ret < 0)
> +		goto out;
> +
> +	for (i = 0; i < max; i++) {
> +		u16 off = i % MXL862XX_MMD_REG_DATA_MAX_SIZE;
> +
> +		if (i && off == 0) {
> +			/* Send command to set data when every
> +			 * MXL862XX_MMD_REG_DATA_MAX_SIZE of WORDs are written.
> +			 */
> +			ret = mxl862xx_set_data(priv, i);
> +			if (ret < 0)
> +				goto out;
> +		}
> +
> +		__mdiobus_c45_write(priv->bus, priv->sw_addr,
> +				    MXL862XX_MMD_DEV,
> +				    MXL862XX_MMD_REG_DATA_FIRST + off,
> +				    le16_to_cpu(data[i]));
> +	}
> +
> +	ret = mxl862xx_send_cmd(priv, cmd, size, &result);
> +	if (ret < 0)
> +		goto out;
> +
> +	if (result < 0) {
> +		ret = result;
> +		goto out;
> +	}

If i'm reading mxl862xx_send_cmd() correct, result is the value of a
register. It seems unlikely this is a Linux error code?

> +
> +	for (i = 0; i < max && read; i++) {

That is an unusual way to use read.

> +		u16 off = i % MXL862XX_MMD_REG_DATA_MAX_SIZE;
> +
> +		if (i && off == 0) {
> +			/* Send command to fetch next batch of data
> +			 * when every MXL862XX_MMD_REG_DATA_MAX_SIZE of WORDs
> +			 * are read.
> +			 */
> +			ret = mxl862xx_get_data(priv, i);
> +			if (ret < 0)
> +				goto out;
> +		}
> +
> +		ret = __mdiobus_c45_read(priv->bus, priv->sw_addr,
> +					 MXL862XX_MMD_DEV,
> +					 MXL862XX_MMD_REG_DATA_FIRST + off);
> +		if (ret < 0)
> +			goto out;
> +
> +		if ((i * 2 + 1) == size) {
> +			/* Special handling for last BYTE
> +			 * if it's not WORD aligned.
> +			 */
> +			*(uint8_t *)&data[i] = ret & 0xFF;
> +		} else {
> +			data[i] = cpu_to_le16((u16)ret);
> +		}
> +	}
> +	ret = result;
> +
> +out:
> +	mutex_unlock(&priv->bus->mdio_lock);
> +	return ret;
> +}
> +#define MXL862XX_API_WRITE(dev, cmd, data) \
> +	mxl862xx_api_wrap(dev, cmd, &(data), sizeof((data)), false)
> +#define MXL862XX_API_READ(dev, cmd, data) \
> +	mxl862xx_api_wrap(dev, cmd, &(data), sizeof((data)), true)

> +/* PHY access via firmware relay */
> +static int mxl862xx_phy_read_mmd(struct mxl862xx_priv *priv, int port,
> +				 int devadd, int reg)
> +{
> +	struct mdio_relay_data param = {
> +		.phy = port,
> +		.mmd = devadd,
> +		.reg = reg & 0xffff,
> +	};
> +	int ret;
> +
> +	ret = MXL862XX_API_READ(priv, INT_GPHY_READ, param);

That looks a bit ugly, using a macro as a function name. I would
suggest tiny functions rather than macros. The compiler should do the
right thing.

> +/* Configure CPU tagging */
> +static int mxl862xx_configure_tag_proto(struct dsa_switch *ds, int port,
> +					bool enable)
> +{
> +	struct mxl862xx_ss_sp_tag tag = {
> +		.pid = DSA_MXL_PORT(port),
> +		.mask = BIT(0) | BIT(1),
> +		.rx = enable ? 2 : 1,
> +		.tx = enable ? 2 : 3,
> +	};

There is a bit comment at the beginning of the patch about these, but
it does not help much here. Please could you add some #defines for
these magic numbers.

> +/* Reset switch via MMD write */
> +static int mxl862xx_mmd_write(struct dsa_switch *ds, int reg, u16 data)

The comment does not fit what the function does.

> +{
> +	struct mxl862xx_priv *priv = ds->priv;
> +	int ret;
> +
> +	mutex_lock(&priv->bus->mdio_lock);
> +	ret = __mdiobus_c45_write(priv->bus, priv->sw_addr, MXL862XX_MMD_DEV,
> +				  reg, data);
> +	mutex_unlock(&priv->bus->mdio_lock);

There is no point using the unlocked version if you wrap it in
lock/unlock...

> +/* Setup */
> +static int mxl862xx_setup(struct dsa_switch *ds)
> +{
> +	struct mxl862xx_priv *priv = ds->priv;
> +	int ret, i;
> +
> +	for (i = 0; i < ds->num_ports; i++) {
> +		if (dsa_is_cpu_port(ds, i)) {
> +			priv->cpu_port = i;
> +			break;
> +		}
> +	}
> +
> +	ret = mxl862xx_setup_mdio(ds);
> +	if (ret)
> +		return ret;
> +
> +	/* Software reset */
> +	ret = mxl862xx_mmd_write(ds, 1, 0);
> +	if (ret)
> +		return ret;
> +
> +	ret = mxl862xx_mmd_write(ds, 0, 0x9907);

More magic numbers.

	Andrew

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

[Russell King (Oracle)]

On Wed, Dec 03, 2025 at 03:07:20AM +0100, Andrew Lunn wrote:
> > +/**
> > + * struct mxl862xx_ss_sp_tag
> > + * @pid: port ID (1~16)
> > + * @mask: bit value 1 to indicate valid field
> > + *	0 - rx
> > + *	1 - tx
> > + *	2 - rx_pen
> > + *	3 - tx_pen
> > + * @rx: RX special tag mode
> > + *	0 - packet does NOT have special tag and special tag is NOT inserted
> > + *	1 - packet does NOT have special tag and special tag is inserted
> > + *	2 - packet has special tag and special tag is NOT inserted
> > + * @tx: TX special tag mode
> > + *	0 - packet does NOT have special tag and special tag is NOT removed
> > + *	1 - packet has special tag and special tag is replaced
> > + *	2 - packet has special tag and special tag is NOT removed
> > + *	3 - packet has special tag and special tag is removed
> > + * @rx_pen: RX special tag info over preamble
> > + *	0 - special tag info inserted from byte 2 to 7 are all 0
> > + *	1 - special tag byte 5 is 16, other bytes from 2 to 7 are 0
> > + *	2 - special tag byte 5 is from preamble field, others are 0
> > + *	3 - special tag byte 2 to 7 are from preabmle field
> > + * @tx_pen: TX special tag info over preamble
> > + *	0 - disabled
> > + *	1 - enabled
> > + */
> > +struct mxl862xx_ss_sp_tag {
> > +	u8 pid;
> > +	u8 mask;
> > +	u8 rx;
> > +	u8 tx;
> > +	u8 rx_pen;
> > +	u8 tx_pen;
> > +} __packed;
> > +
> > +/**
> > + * enum mxl862xx_logical_port_mode - Logical port mode
> > + * @MXL862XX_LOGICAL_PORT_8BIT_WLAN: WLAN with 8-bit station ID
> > + * @MXL862XX_LOGICAL_PORT_9BIT_WLAN: WLAN with 9-bit station ID
> > + * @MXL862XX_LOGICAL_PORT_GPON: GPON OMCI context
> > + * @MXL862XX_LOGICAL_PORT_EPON: EPON context
> > + * @MXL862XX_LOGICAL_PORT_GINT: G.INT context
> > + * @MXL862XX_LOGICAL_PORT_OTHER: Others
> > + */
> > +enum mxl862xx_logical_port_mode {
> > +	MXL862XX_LOGICAL_PORT_8BIT_WLAN = 0,
> > +	MXL862XX_LOGICAL_PORT_9BIT_WLAN,
> > +	MXL862XX_LOGICAL_PORT_GPON,
> > +	MXL862XX_LOGICAL_PORT_EPON,
> > +	MXL862XX_LOGICAL_PORT_GINT,
> > +	MXL862XX_LOGICAL_PORT_OTHER = 0xFF,
> > +};
> > +
> > +/**
> > + * struct mxl862xx_ctp_port_assignment - CTP Port Assignment/association with logical port
> > + * @logical_port_id: Logical Port Id. The valid range is hardware dependent
> > + * @first_ctp_port_id: First CTP Port ID mapped to above logical port ID
> > + * @number_of_ctp_port: Total number of CTP Ports mapped above logical port ID
> > + * @mode: See &enum mxl862xx_logical_port_mode
> > + * @bridge_port_id: Bridge ID (FID)
> > + */
> > +struct mxl862xx_ctp_port_assignment {
> > +	u8 logical_port_id;
> > +	__le16 first_ctp_port_id;
> > +	__le16 number_of_ctp_port;
> > +	enum mxl862xx_logical_port_mode mode;
> > +	__le16 bridge_port_id;
> > +} __packed;
> 
> Does the C standard define the size of an enum? Do you assume this is
> a byte?

It does not. Some architectures are allowed to choose the storage size
of enum depending on the range of values.

> > +static int mxl862xx_send_cmd(struct mxl862xx_priv *dev, u16 cmd, u16 size,
> > +			     s16 *presult)
> > +{
> > +	int ret;
> > +
> > +	ret = __mdiobus_c45_write(dev->bus, dev->sw_addr, MXL862XX_MMD_DEV,
> > +				  MXL862XX_MMD_REG_LEN_RET, size);
> > +	if (ret)
> > +		return ret;
> > +
> > +	ret = __mdiobus_c45_write(dev->bus, dev->sw_addr, MXL862XX_MMD_DEV,
> > +				  MXL862XX_MMD_REG_CTRL, cmd | CTRL_BUSY_MASK);
> > +	if (ret)
> > +		return ret;
> > +
> > +	ret = mxl862xx_busy_wait(dev);
> > +	if (ret)
> > +		return ret;
> > +
> > +	ret = __mdiobus_c45_read(dev->bus, dev->sw_addr, MXL862XX_MMD_DEV,
> > +				 MXL862XX_MMD_REG_LEN_RET);
> > +	if (ret < 0)
> > +		return ret;

Error codes go via this path.

> > +
> > +	*presult = ret;

Register values via this, and if the sign bit is set, *presult is
negative.

> > +	ret = mxl862xx_send_cmd(priv, cmd, size, &result);
> > +	if (ret < 0)
> > +		goto out;
> > +
> > +	if (result < 0) {
> > +		ret = result;
> > +		goto out;
> > +	}
> 
> If i'm reading mxl862xx_send_cmd() correct, result is the value of a
> register. It seems unlikely this is a Linux error code?

result here is the register value, and a negative value is the value
from the register. So I agree - this assigns a register value to
"ret" which gets promoted from s16 to int (sign extension) and thus
gets returned as a Linux error code. So yes, this doesn't seem right.

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

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

[Daniel Golle]

Hi Andrew,

thank you for the detailed review.
See my replies inline below

On Wed, Dec 03, 2025 at 03:07:20AM +0100, Andrew Lunn wrote:
> > [...]
> > +struct mxl862xx_ctp_port_assignment {
> > +	u8 logical_port_id;
> > +	__le16 first_ctp_port_id;
> > +	__le16 number_of_ctp_port;
> > +	enum mxl862xx_logical_port_mode mode;
> > +	__le16 bridge_port_id;
> > +} __packed;
> 
> Does the C standard define the size of an enum? Do you assume this is
> a byte?

Yes, it needs to be a byte, and I guess the best is to then just use
u8 type here and cast to u8 on assignment.

> > + * struct mxl862xx_sys_fw_image_version - VLAN counter mapping configuration
> 
> The text seems wrong, probably cut/paste error?

Yes, I agree ;)

> 
> > +#define MXL862XX_MMD_DEV 30
> 
> Please use MDIO_MMD_VEND1

Ack.

> 
> > +#define MXL862XX_MMD_REG_CTRL 0
> > +#define MXL862XX_MMD_REG_LEN_RET 1
> > +#define MXL862XX_MMD_REG_DATA_FIRST 2
> > +#define MXL862XX_MMD_REG_DATA_LAST 95
> > +#define MXL862XX_MMD_REG_DATA_MAX_SIZE \
> > +		(MXL862XX_MMD_REG_DATA_LAST - MXL862XX_MMD_REG_DATA_FIRST + 1)
> > +
> > +#define MMD_API_SET_DATA_0 (0x0 + 0x2)
> > +#define MMD_API_GET_DATA_0 (0x0 + 0x5)
> > +#define MMD_API_RST_DATA (0x0 + 0x8)
> 
> What is the significant of these numbers? Can you use #defines to make
> it clearer?

I'll simplify this to be

#define MMD_API_SET_DATA		2
#define MMD_API_GET_DATA		5
#define MMD_API_RST_DATA		8

which makes it more clear I hope.

> 
> > +
> > +static int mxl862xx_busy_wait(struct mxl862xx_priv *dev)
> > +{
> > +	int ret, i;
> > +
> > +	for (i = 0; i < MAX_BUSY_LOOP; i++) {
> > +		ret = __mdiobus_c45_read(dev->bus, dev->sw_addr,
> > +					 MXL862XX_MMD_DEV,
> > +					 MXL862XX_MMD_REG_CTRL);
> > +		if (ret < 0)
> > +			return ret;
> > +
> > +		if (ret & CTRL_BUSY_MASK)
> > +			usleep_range(10, 15);
> > +		else
> > +			return 0;
> > +	}
> > +
> > +	return -ETIMEDOUT;
> 
> We already have phy_read_mmd_poll_timeout(). Maybe you should add a
> __mdiobus_c45_read_poll_timeout()?
> 
> Also, as far as i can see, __mdiobus_c45_read() is always called with
> the same first three parameters. Maybe add a mxl862xx_reg_read()?  and
> a mxl862xx_reg_write()?

Imho it would be nice to introduce unlock __mdiodev_c45_* helpers in
include/linux/mdio.h, ie.

static inline int __mdiodev_c45_read(struct mdio_device *mdiodev, int devad,
				     u16 regnum)
{
	return __mdiobus_c45_read(mdiodev->bus, mdiodev->addr, devad, regnum);
}

static inline int __mdiodev_c45_write(struct mdio_device *mdiodev, u32 devad,
				      u16 regnum, u16 val)
{
	return __mdiobus_c45_write(mdiodev->bus, mdiodev->addr, devad, regnum,
				   val);
}

and then have something like this in mxl862xx-host.c:

static int mxl862xx_reg_read(struct mxl862xx_priv *priv, u32 addr)
{
	return __mdiodev_c45_read(priv->mdiodev, MDIO_MMD_VEND1, addr);
}

static int mxl862xx_reg_write(struct mxl862xx_priv *priv, u32 addr, u16 data)
{
	return __mdiodev_c45_write(priv->mdiodev, MDIO_MMD_VEND1, addr, data);
}

static int mxl862xx_ctrl_read(struct mxl862xx_priv *priv)
{
	return mxl862xx_reg_read(priv, MXL862XX_MMD_REG_CTRL);
}

static int mxl862xx_busy_wait(struct mxl862xx_priv *priv)
{
	int val;

	return readx_poll_timeout(mxl862xx_ctrl_read, priv, val,
				  !(val & CTRL_BUSY_MASK), 15, 10000);
}

Do you agree?

> > [...]
> > +	if (result < 0) {
> > +		ret = result;
> > +		goto out;
> > +	}
> 
> If i'm reading mxl862xx_send_cmd() correct, result is the value of a
> register. It seems unlikely this is a Linux error code?

Only someone with insights into the use of error codes by the uC
firmware can really answer that. However, as also Russell pointed out,
the whole use of s16 here with negative values being interpreted as
errors is fishy here, because in the end this is also used to read
registers from external MDIO connected PHYs which may return arbitrary
16-bit values...
Someone in MaxLinear will need to clarify here.

> 
> > +
> > +	for (i = 0; i < max && read; i++) {
> 
> That is an unusual way to use read.

Ack. I suggest

if (!read)
	goto out;

> > +#define MXL862XX_API_WRITE(dev, cmd, data) \
> > +	mxl862xx_api_wrap(dev, cmd, &(data), sizeof((data)), false)
> > +#define MXL862XX_API_READ(dev, cmd, data) \
> > +	mxl862xx_api_wrap(dev, cmd, &(data), sizeof((data)), true)
> 
> > +/* PHY access via firmware relay */
> > +static int mxl862xx_phy_read_mmd(struct mxl862xx_priv *priv, int port,
> > +				 int devadd, int reg)
> > +{
> > +	struct mdio_relay_data param = {
> > +		.phy = port,
> > +		.mmd = devadd,
> > +		.reg = reg & 0xffff,
> > +	};
> > +	int ret;
> > +
> > +	ret = MXL862XX_API_READ(priv, INT_GPHY_READ, param);
> 
> That looks a bit ugly, using a macro as a function name. I would
> suggest tiny functions rather than macros. The compiler should do the
> right thing.

The thing is that the macro way allows to use MXL862XX_API_* on
arbitrary types, such as the packed structs. Using a function would
require the type of the parameter to be defined, which would result
in a lot of code duplication in this case.

> 
> > +/* Configure CPU tagging */
> > +static int mxl862xx_configure_tag_proto(struct dsa_switch *ds, int port,
> > +					bool enable)
> > +{
> > +	struct mxl862xx_ss_sp_tag tag = {
> > +		.pid = DSA_MXL_PORT(port),
> > +		.mask = BIT(0) | BIT(1),
> > +		.rx = enable ? 2 : 1,
> > +		.tx = enable ? 2 : 3,
> > +	};
> 
> There is a bit comment at the beginning of the patch about these, but
> it does not help much here. Please could you add some #defines for
> these magic numbers.

Ack, I'll do that.

> 
> > +/* Reset switch via MMD write */
> > +static int mxl862xx_mmd_write(struct dsa_switch *ds, int reg, u16 data)
> 
> The comment does not fit what the function does.

Ooops...

> 
> > +{
> > +	struct mxl862xx_priv *priv = ds->priv;
> > +	int ret;
> > +
> > +	mutex_lock(&priv->bus->mdio_lock);
> > +	ret = __mdiobus_c45_write(priv->bus, priv->sw_addr, MXL862XX_MMD_DEV,
> > +				  reg, data);
> > +	mutex_unlock(&priv->bus->mdio_lock);
> 
> There is no point using the unlocked version if you wrap it in
> lock/unlock...

I'll throw all this out and instead introduce a high-level reset
function in mxl8622xx-host.c, in that way all MDIO host access is kept
in mxl8622xx-host.c.

> > [...]
> > +	/* Software reset */
> > +	ret = mxl862xx_mmd_write(ds, 1, 0);
> > +	if (ret)
> > +		return ret;
> > +
> > +	ret = mxl862xx_mmd_write(ds, 0, 0x9907);
> 
> More magic numbers.

Maybe someone in MaxLinear can demystify this?

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

[Andrew Lunn]

> Imho it would be nice to introduce unlock __mdiodev_c45_* helpers in
> include/linux/mdio.h, ie.
> 
> static inline int __mdiodev_c45_read(struct mdio_device *mdiodev, int devad,
> 				     u16 regnum)
> {
> 	return __mdiobus_c45_read(mdiodev->bus, mdiodev->addr, devad, regnum);
> }
> 
> static inline int __mdiodev_c45_write(struct mdio_device *mdiodev, u32 devad,
> 				      u16 regnum, u16 val)
> {
> 	return __mdiobus_c45_write(mdiodev->bus, mdiodev->addr, devad, regnum,
> 				   val);
> }

https://elixir.bootlin.com/linux/v6.18/source/drivers/net/phy/mdio_bus.c#L531

> static int mxl862xx_reg_read(struct mxl862xx_priv *priv, u32 addr)
> {
> 	return __mdiodev_c45_read(priv->mdiodev, MDIO_MMD_VEND1, addr);
> }
> 
> static int mxl862xx_reg_write(struct mxl862xx_priv *priv, u32 addr, u16 data)
> {
> 	return __mdiodev_c45_write(priv->mdiodev, MDIO_MMD_VEND1, addr, data);
> }
> 
> static int mxl862xx_ctrl_read(struct mxl862xx_priv *priv)
> {
> 	return mxl862xx_reg_read(priv, MXL862XX_MMD_REG_CTRL);
> }
> 
> static int mxl862xx_busy_wait(struct mxl862xx_priv *priv)
> {
> 	int val;
> 
> 	return readx_poll_timeout(mxl862xx_ctrl_read, priv, val,
> 				  !(val & CTRL_BUSY_MASK), 15, 10000);
> }
> 
> Do you agree?

This part, yes.

> > > +	if (result < 0) {
> > > +		ret = result;
> > > +		goto out;
> > > +	}
> > 
> > If i'm reading mxl862xx_send_cmd() correct, result is the value of a
> > register. It seems unlikely this is a Linux error code?
> 
> Only someone with insights into the use of error codes by the uC
> firmware can really answer that. However, as also Russell pointed out,
> the whole use of s16 here with negative values being interpreted as
> errors is fishy here, because in the end this is also used to read
> registers from external MDIO connected PHYs which may return arbitrary
> 16-bit values...
> Someone in MaxLinear will need to clarify here.

It looks wrong, and since different architectures use different error
code values, it is hard to get right. I would suggest you just return
EPROTO or EIO and add a netdev_err() to print the value of result.

> > > +#define MXL862XX_API_WRITE(dev, cmd, data) \
> > > +	mxl862xx_api_wrap(dev, cmd, &(data), sizeof((data)), false)
> > > +#define MXL862XX_API_READ(dev, cmd, data) \
> > > +	mxl862xx_api_wrap(dev, cmd, &(data), sizeof((data)), true)
> > 
> > > +/* PHY access via firmware relay */
> > > +static int mxl862xx_phy_read_mmd(struct mxl862xx_priv *priv, int port,
> > > +				 int devadd, int reg)
> > > +{
> > > +	struct mdio_relay_data param = {
> > > +		.phy = port,
> > > +		.mmd = devadd,
> > > +		.reg = reg & 0xffff,
> > > +	};
> > > +	int ret;
> > > +
> > > +	ret = MXL862XX_API_READ(priv, INT_GPHY_READ, param);
> > 
> > That looks a bit ugly, using a macro as a function name. I would
> > suggest tiny functions rather than macros. The compiler should do the
> > right thing.
> 
> The thing is that the macro way allows to use MXL862XX_API_* on
> arbitrary types, such as the packed structs. Using a function would
> require the type of the parameter to be defined, which would result
> in a lot of code duplication in this case.

How many different invocations of these macros are there? For MDIO you
need two. How many more are there? 

     Andrew

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

[Daniel Golle]

On Wed, Dec 10, 2025 at 07:56:13PM +0100, Andrew Lunn wrote:
> > Imho it would be nice to introduce unlock __mdiodev_c45_* helpers in
> > include/linux/mdio.h, ie.
> > 
> > static inline int __mdiodev_c45_read(struct mdio_device *mdiodev, int devad,
> > 				     u16 regnum)
> > {
> > 	return __mdiobus_c45_read(mdiodev->bus, mdiodev->addr, devad, regnum);
> > }
> > 
> > static inline int __mdiodev_c45_write(struct mdio_device *mdiodev, u32 devad,
> > 				      u16 regnum, u16 val)
> > {
> > 	return __mdiobus_c45_write(mdiodev->bus, mdiodev->addr, devad, regnum,
> > 				   val);
> > }
> 
> https://elixir.bootlin.com/linux/v6.18/source/drivers/net/phy/mdio_bus.c#L531

That's __mdiobus_c45_*, but having __mdiodev_c45_* would be nice as
well, see above.

> > > > +	if (result < 0) {
> > > > +		ret = result;
> > > > +		goto out;
> > > > +	}
> > > 
> > > If i'm reading mxl862xx_send_cmd() correct, result is the value of a
> > > register. It seems unlikely this is a Linux error code?
> > 
> > Only someone with insights into the use of error codes by the uC
> > firmware can really answer that. However, as also Russell pointed out,
> > the whole use of s16 here with negative values being interpreted as
> > errors is fishy here, because in the end this is also used to read
> > registers from external MDIO connected PHYs which may return arbitrary
> > 16-bit values...
> > Someone in MaxLinear will need to clarify here.
> 
> It looks wrong, and since different architectures use different error
> code values, it is hard to get right. I would suggest you just return
> EPROTO or EIO and add a netdev_err() to print the value of result.

Ack, makes sense.

> > > > +#define MXL862XX_API_WRITE(dev, cmd, data) \
> > > > +	mxl862xx_api_wrap(dev, cmd, &(data), sizeof((data)), false)
> > > > +#define MXL862XX_API_READ(dev, cmd, data) \
> > > > +	mxl862xx_api_wrap(dev, cmd, &(data), sizeof((data)), true)
> > > 
> > > > +/* PHY access via firmware relay */
> > > > +static int mxl862xx_phy_read_mmd(struct mxl862xx_priv *priv, int port,
> > > > +				 int devadd, int reg)
> > > > +{
> > > > +	struct mdio_relay_data param = {
> > > > +		.phy = port,
> > > > +		.mmd = devadd,
> > > > +		.reg = reg & 0xffff,
> > > > +	};
> > > > +	int ret;
> > > > +
> > > > +	ret = MXL862XX_API_READ(priv, INT_GPHY_READ, param);
> > > 
> > > That looks a bit ugly, using a macro as a function name. I would
> > > suggest tiny functions rather than macros. The compiler should do the
> > > right thing.
> > 
> > The thing is that the macro way allows to use MXL862XX_API_* on
> > arbitrary types, such as the packed structs. Using a function would
> > require the type of the parameter to be defined, which would result
> > in a lot of code duplication in this case.
> 
> How many different invocations of these macros are there? For MDIO you
> need two. How many more are there? 

A lot, 80+ in total in the more-or-less complete driver, using 30+
different __packed structs as parameters.

https://github.com/dangowrt/linux/blob/mxl862xx-for-upstream/drivers/net/dsa/mxl862xx/mxl862xx.c

https://github.com/dangowrt/linux/blob/mxl862xx-for-upstream/drivers/net/dsa/mxl862xx/mxl862xx-api.h

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

[Daniel Golle]

Hi Andrew,

On Wed, Dec 10, 2025 at 07:56:13PM +0100, Andrew Lunn wrote:
> > > > +	if (result < 0) {
> > > > +		ret = result;
> > > > +		goto out;
> > > > +	}
> > > 
> > > If i'm reading mxl862xx_send_cmd() correct, result is the value of a
> > > register. It seems unlikely this is a Linux error code?
> > 
> > Only someone with insights into the use of error codes by the uC
> > firmware can really answer that. However, as also Russell pointed out,
> > the whole use of s16 here with negative values being interpreted as
> > errors is fishy here, because in the end this is also used to read
> > registers from external MDIO connected PHYs which may return arbitrary
> > 16-bit values...
> > Someone in MaxLinear will need to clarify here.
> 
> It looks wrong, and since different architectures use different error
> code values, it is hard to get right. I would suggest you just return
> EPROTO or EIO and add a netdev_err() to print the value of result.

MaxLinear folks got back to me. So the error codes returned by the firmware
are basically based on Zephyr's errno.h which seems to be a copy of a BSD
header, see

https://github.com/zephyrproject-rtos/zephyr/blob/main/lib/libc/minimal/include/errno.h

So the best would probably be to modify and include that header with the
driver, together with a function translating the error codes to what ever
is defined in uapi/asm/errno.h for the architecture we are building for.

They also told me that (obviously) not all error codes are currently
used by the firmware, but the best would be to just catch all the possible
error codes and translate Zephyr libc -> Linux kernel.

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

[Andrew Lunn]

On Fri, Dec 12, 2025 at 04:49:47PM +0000, Daniel Golle wrote:
> Hi Andrew,
> 
> On Wed, Dec 10, 2025 at 07:56:13PM +0100, Andrew Lunn wrote:
> > > > > +	if (result < 0) {
> > > > > +		ret = result;
> > > > > +		goto out;
> > > > > +	}
> > > > 
> > > > If i'm reading mxl862xx_send_cmd() correct, result is the value of a
> > > > register. It seems unlikely this is a Linux error code?
> > > 
> > > Only someone with insights into the use of error codes by the uC
> > > firmware can really answer that. However, as also Russell pointed out,
> > > the whole use of s16 here with negative values being interpreted as
> > > errors is fishy here, because in the end this is also used to read
> > > registers from external MDIO connected PHYs which may return arbitrary
> > > 16-bit values...
> > > Someone in MaxLinear will need to clarify here.
> > 
> > It looks wrong, and since different architectures use different error
> > code values, it is hard to get right. I would suggest you just return
> > EPROTO or EIO and add a netdev_err() to print the value of result.
> 
> MaxLinear folks got back to me. So the error codes returned by the firmware
> are basically based on Zephyr's errno.h which seems to be a copy of a BSD
> header, see
> 
> https://github.com/zephyrproject-rtos/zephyr/blob/main/lib/libc/minimal/include/errno.h
> 
> So the best would probably be to modify and include that header with the
> driver, together with a function translating the error codes to what ever
> is defined in uapi/asm/errno.h for the architecture we are building for.
> 
> They also told me that (obviously) not all error codes are currently
> used by the firmware, but the best would be to just catch all the possible
> error codes and translate Zephyr libc -> Linux kernel.

That all sounds way too complex. We don't expect errors do we? So
print the raw value, add a comment about how to manually translate the
number to something potentially meaningful, and return -EIO.

	  Andrew

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

[Russell King (Oracle)]

On Tue, Dec 02, 2025 at 11:38:05PM +0000, Daniel Golle wrote:
> +/* Phylink integration */
> +static void mxl862xx_phylink_get_caps(struct dsa_switch *ds, int port,
> +				      struct phylink_config *config)
> +{
> +	struct mxl862xx_priv *priv = ds->priv;
> +
> +	config->mac_capabilities = MAC_ASYM_PAUSE | MAC_SYM_PAUSE | MAC_10 |
> +				   MAC_100 | MAC_1000 | MAC_2500FD;
> +
> +	if (port < priv->hw_info->phy_ports)
> +		__set_bit(PHY_INTERFACE_MODE_INTERNAL,
> +			  config->supported_interfaces);
> +	else
> +		__set_bit(PHY_INTERFACE_MODE_NA,
> +			  config->supported_interfaces);

What's the purpose of PHY_INTERFACE_MODE_NA here?

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

Re: [PATCH RFC net-next 0/3] net: dsa: initial support for MaxLinear MxL862xx switches

[Vladimir Oltean]

Hi Daniel,

On Tue, Dec 02, 2025 at 11:37:13PM +0000, Daniel Golle wrote:
> Hi,
> 
> This series adds very basic DSA support for the MaxLinear MxL86252
> (5 PHY ports) and MxL86282 (8 PHY ports) switches. The intent is to
> validate and get feedback on the overall approach and driver structure,
> especially the firmware-mediated host interface.
> 
> MxL862xx integrates a firmware running on an embedded processor (Zephyr
> RTOS). Host interaction uses a simple API transported over MDIO/MMD.
> This series includes only what's needed to pass traffic between user
> ports and the CPU port: relayed MDIO to internal PHYs, basic port
> enable/disable, and CPU-port special tagging.
> 
> Thanks for taking a look.

I see no phylink_mac_ops in your patches.

How does this switch architecture deal with SFP cages? I see the I2C
controllers aren't accessible through the MDIO relay protocol
implemented by the microcontroller. So I guess using the sfp-bus code
isn't going to be possible. The firmware manages the SFP cage and you
"just" have to read the USXGMII Status Register (reg 30.19) from the
host? How does that work out in practice?

Re: [PATCH RFC net-next 0/3] net: dsa: initial support for MaxLinear MxL862xx switches

[Daniel Golle]

On Wed, Dec 03, 2025 at 10:26:05PM +0200, Vladimir Oltean wrote:
> Hi Daniel,
> 
> On Tue, Dec 02, 2025 at 11:37:13PM +0000, Daniel Golle wrote:
> > Hi,
> > 
> > This series adds very basic DSA support for the MaxLinear MxL86252
> > (5 PHY ports) and MxL86282 (8 PHY ports) switches. The intent is to
> > validate and get feedback on the overall approach and driver structure,
> > especially the firmware-mediated host interface.
> > 
> > MxL862xx integrates a firmware running on an embedded processor (Zephyr
> > RTOS). Host interaction uses a simple API transported over MDIO/MMD.
> > This series includes only what's needed to pass traffic between user
> > ports and the CPU port: relayed MDIO to internal PHYs, basic port
> > enable/disable, and CPU-port special tagging.
> > 
> > Thanks for taking a look.
> 
> I see no phylink_mac_ops in your patches.


> 
> How does this switch architecture deal with SFP cages? I see the I2C
> controllers aren't accessible through the MDIO relay protocol
> implemented by the microcontroller. So I guess using the sfp-bus code
> isn't going to be possible. The firmware manages the SFP cage and you
> "just" have to read the USXGMII Status Register (reg 30.19) from the
> host? How does that work out in practice?

In practise the I2C bus provided by the switch IC isn't used to connect
an SFP cage when using the chip with DSA. Vendors (Adtran,
BananaPi/Sinovoip) rather use an I2C bus of the SoC for that.
I suppose it is useful when using the chip as standalone switch.

The firmware does provide some kind of limited access to the PCS, ie.
status can be polled, interface mode can be set, autonegotiation can be
enabled or disabled, and so on (but not as nice as we would like it to
be). In that way, most SFP modules and external PHYs can be supported.

See

https://github.com/frank-w/BPI-Router-Linux/commit/c5f7a68e82fe20b9b37a60afd033b2364a8763d8

In general I don't get why all those layers of abstraction are actually
needed when using a full-featured OS on the host -- it'd be much better
to just have direct access to the register space of the switch than
having to deal with that firmware API (the firmware can also provide a
full web UI, SNMP, a CLI interface, ... -- imho more of an obstacle than
a desirable feature when using this thing with DSA).

Re: [PATCH RFC net-next 0/3] net: dsa: initial support for MaxLinear MxL862xx switches

[Russell King (Oracle)]

On Wed, Dec 03, 2025 at 11:23:11PM +0000, Daniel Golle wrote:
> On Wed, Dec 03, 2025 at 10:26:05PM +0200, Vladimir Oltean wrote:
> > Hi Daniel,
> > 
> > On Tue, Dec 02, 2025 at 11:37:13PM +0000, Daniel Golle wrote:
> > > Hi,
> > > 
> > > This series adds very basic DSA support for the MaxLinear MxL86252
> > > (5 PHY ports) and MxL86282 (8 PHY ports) switches. The intent is to
> > > validate and get feedback on the overall approach and driver structure,
> > > especially the firmware-mediated host interface.
> > > 
> > > MxL862xx integrates a firmware running on an embedded processor (Zephyr
> > > RTOS). Host interaction uses a simple API transported over MDIO/MMD.
> > > This series includes only what's needed to pass traffic between user
> > > ports and the CPU port: relayed MDIO to internal PHYs, basic port
> > > enable/disable, and CPU-port special tagging.
> > > 
> > > Thanks for taking a look.
> > 
> > I see no phylink_mac_ops in your patches.
> 

As you didn't respond to Vladimir's statement here, I will also echo
this. Why do you have no phylink_mac_ops ?

New DSA drivers are expected to always have phylink_mac_ops, and not
rely on the legacy fallback in net/dsa/port.c

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

Re: [PATCH RFC net-next 0/3] net: dsa: initial support for MaxLinear MxL862xx switches

[Daniel Golle]

On Thu, Dec 04, 2025 at 01:02:14AM +0000, Russell King (Oracle) wrote:
> On Wed, Dec 03, 2025 at 11:23:11PM +0000, Daniel Golle wrote:
> > On Wed, Dec 03, 2025 at 10:26:05PM +0200, Vladimir Oltean wrote:
> > > Hi Daniel,
> > > 
> > > On Tue, Dec 02, 2025 at 11:37:13PM +0000, Daniel Golle wrote:
> > > > Hi,
> > > > 
> > > > This series adds very basic DSA support for the MaxLinear MxL86252
> > > > (5 PHY ports) and MxL86282 (8 PHY ports) switches. The intent is to
> > > > validate and get feedback on the overall approach and driver structure,
> > > > especially the firmware-mediated host interface.
> > > > 
> > > > MxL862xx integrates a firmware running on an embedded processor (Zephyr
> > > > RTOS). Host interaction uses a simple API transported over MDIO/MMD.
> > > > This series includes only what's needed to pass traffic between user
> > > > ports and the CPU port: relayed MDIO to internal PHYs, basic port
> > > > enable/disable, and CPU-port special tagging.
> > > > 
> > > > Thanks for taking a look.
> > > 
> > > I see no phylink_mac_ops in your patches.
> > 
> 
> As you didn't respond to Vladimir's statement here, I will also echo
> this. Why do you have no phylink_mac_ops ?
> 
> New DSA drivers are expected to always have phylink_mac_ops, and not
> rely on the legacy fallback in net/dsa/port.c

All three phylink_mac_ops functions are no-ops for the internal PHYs,
see also

https://github.com/frank-w/BPI-Router-Linux/blob/6.18-rc/drivers/net/dsa/mxl862xx/mxl862xx.c#L3242

The exception in the reference driver are the SerDes PCS ports, and for
those I'd rather use .pcs_config than setting the interface mode in
.phylink_mac_config.
Hence I was planing to introduce phylink_mac_ops together with support
for the SerDes ports, and it will only have a .mac_select_pcs op.

Re: [PATCH RFC net-next 0/3] net: dsa: initial support for MaxLinear MxL862xx switches

[Russell King (Oracle)]

On Thu, Dec 04, 2025 at 01:08:24PM +0000, Daniel Golle wrote:
> On Thu, Dec 04, 2025 at 01:02:14AM +0000, Russell King (Oracle) wrote:
> > On Wed, Dec 03, 2025 at 11:23:11PM +0000, Daniel Golle wrote:
> > > On Wed, Dec 03, 2025 at 10:26:05PM +0200, Vladimir Oltean wrote:
> > > > Hi Daniel,
> > > > 
> > > > On Tue, Dec 02, 2025 at 11:37:13PM +0000, Daniel Golle wrote:
> > > > > Hi,
> > > > > 
> > > > > This series adds very basic DSA support for the MaxLinear MxL86252
> > > > > (5 PHY ports) and MxL86282 (8 PHY ports) switches. The intent is to
> > > > > validate and get feedback on the overall approach and driver structure,
> > > > > especially the firmware-mediated host interface.
> > > > > 
> > > > > MxL862xx integrates a firmware running on an embedded processor (Zephyr
> > > > > RTOS). Host interaction uses a simple API transported over MDIO/MMD.
> > > > > This series includes only what's needed to pass traffic between user
> > > > > ports and the CPU port: relayed MDIO to internal PHYs, basic port
> > > > > enable/disable, and CPU-port special tagging.
> > > > > 
> > > > > Thanks for taking a look.
> > > > 
> > > > I see no phylink_mac_ops in your patches.
> > > 
> > 
> > As you didn't respond to Vladimir's statement here, I will also echo
> > this. Why do you have no phylink_mac_ops ?
> > 
> > New DSA drivers are expected to always have phylink_mac_ops, and not
> > rely on the legacy fallback in net/dsa/port.c
> 
> All three phylink_mac_ops functions are no-ops for the internal PHYs,
> see also
> 
> https://github.com/frank-w/BPI-Router-Linux/blob/6.18-rc/drivers/net/dsa/mxl862xx/mxl862xx.c#L3242

While you may end up with the same three methods remaining empty,
please do not rely on the legacy fallback, even temporarily.

Thanks.

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

Re: [PATCH RFC net-next 0/3] net: dsa: initial support for MaxLinear MxL862xx switches

[Vladimir Oltean]

On Wed, Dec 03, 2025 at 11:23:11PM +0000, Daniel Golle wrote:
> On Wed, Dec 03, 2025 at 10:26:05PM +0200, Vladimir Oltean wrote:
> > How does this switch architecture deal with SFP cages? I see the I2C
> > controllers aren't accessible through the MDIO relay protocol
> > implemented by the microcontroller. So I guess using the sfp-bus code
> > isn't going to be possible. The firmware manages the SFP cage and you
> > "just" have to read the USXGMII Status Register (reg 30.19) from the
> > host? How does that work out in practice?
> 
> In practise the I2C bus provided by the switch IC isn't used to connect
> an SFP cage when using the chip with DSA. Vendors (Adtran,
> BananaPi/Sinovoip) rather use an I2C bus of the SoC for that.
> I suppose it is useful when using the chip as standalone switch.
> 
> The firmware does provide some kind of limited access to the PCS, ie.
> status can be polled, interface mode can be set, autonegotiation can be
> enabled or disabled, and so on (but not as nice as we would like it to
> be). In that way, most SFP modules and external PHYs can be supported.
> 
> See
> 
> https://github.com/frank-w/BPI-Router-Linux/commit/c5f7a68e82fe20b9b37a60afd033b2364a8763d8
> 
> In general I don't get why all those layers of abstraction are actually
> needed when using a full-featured OS on the host -- it'd be much better
> to just have direct access to the register space of the switch than
> having to deal with that firmware API (the firmware can also provide a
> full web UI, SNMP, a CLI interface, ... -- imho more of an obstacle than
> a desirable feature when using this thing with DSA).

I'm not sure I understand either, but is it possible that since the base
Ethernet switch IP was already MDIO-based, Maxlinear wanted to offer a
single programming interface for the entire SoC as visible from an
external host, so that's why they continued exposing the other stuff
that they did in MMD 30 (temperature sensor, LEDs, etc) using this North
Korean guided tour kind of approach.

I am noting that there also seems no way to control the 'GPIO' pins as
GPIO from the external host. No way to set a direction or a value on
them. They seem to be "GPIO" only for the microcontroller.

It also seems possible that Maxlinear studied what other MDIO-based DSA
drivers offer, and they wanted to keep things in line with that,
(over)simplifying access to resources to keep things tidy when they are
all driven from DSA.

Although for other switches like the NXP SJA1110 (which has a SPI-to-AHB
bridge to gain direct access into sub-devices that are also mapped in the
microcontroller's address space) I try to push for the MFD model to be
used, for better scalability outside of drivers/net/dsa/, I think that
trying to horseshoe MFD on top of the MxL86252, at least in the way exposed
by the MDIO slave API implemented by its microcontroller, would be
overkill and a big a mistake.

But before I give my OK on your driver design choice, could you:
- confirm that the Zephyr-based MDIO slave firmware is the single
  external register access method for these chips in production? No
  register access or other special communication protocol with the
  microcontroller over Ethernet, no secret SPI-to-AHB bridge that can be
  used when there is no firmware running? The firmware image is maintained
  by Maxlinear, and their customers can't customize it, right?
- make a list of all the subsystems you foresee this chip will register
  itself with? Essentially how will it drive the sub-devices that the
  firmware does give it access to.

So what I'm trying to gauge is how complex this driver will be in its
fully developed form, to make sure it won't suffer from early underdesign
issues.