[PATCH net-next v4 1/2] net: dsa: realtek: rtl8365mb: add SGMII support for RTL8367S

[PATCH net-next v4 1/2] net: dsa: realtek: rtl8365mb: add SGMII support for RTL8367S

[Johan Alvarado]

The RTL8367S can mux its embedded SerDes to external interface 1,
which is typically used to connect the switch to a CPU port. The chip
info table already declares SGMII as a supported interface mode for
this chip, but the driver only implements RGMII so far.

Implement SGMII support as a phylink PCS, with the configuration
sequence derived from the GPL-licensed Realtek rtl8367c vendor driver
as distributed in the Mercusys MR80X GPL code drop:

 - Add accessors for the SerDes indirect access registers (SDS_INDACS),
   through which the SerDes internal registers are reached.

 - Register a phylink_pcs for the SerDes, selected from mac_select_pcs
   for the SGMII interface, so the SerDes handling lives in the PCS
   operations rather than in the MAC operations.

 - Probe the SerDes tuning variant from the chip option register once
   at setup. The vendor driver keeps two sets of SerDes tuning
   parameters and selects between them based on this option; only the
   variant for a non-zero option (which all RTL8367S parts seen so far
   report) has been validated on hardware, so the SerDes interface
   modes are only advertised in that case. An unsupported variant thus
   fails at phylink validation time instead of at link configuration
   time.

 - Keep the embedded DW8051 microcontroller in reset and disabled. The
   vendor driver loads firmware into it to manage the SerDes link, but
   analysis of that firmware shows it only duplicates the link
   management phylink already performs: it polls the port status and
   writes the external interface force registers behind the driver's
   back.

 - Clear the line rate bypass bit for the external interface, tune the
   SerDes with the vendor-prescribed parameters, mux the SerDes to MAC8
   in SGMII mode and only then take the SerDes out of reset, as the
   vendor driver does.

 - After deasserting the SerDes reset, reset the SerDes data path via
   the SerDes BMCR register to flush the FIFOs and resync the PLL.
   This mirrors what the vendor firmware does right after deasserting
   the SerDes reset, and ensures a clean link state from cold boot.

 - Force the SGMII link parameters (link, speed, duplex) in the SDS_MISC
   register from pcs_link_up(). SGMII in-band autonegotiation is not
   implemented, so only fixed-link and conventional PHY setups are
   supported, just like RGMII. This is reported to phylink through
   pcs_inband_caps() returning LINK_INBAND_DISABLE, so phylink never
   selects an in-band-enabled negotiation mode for this PCS.

 - Program the SerDes pause controls in SDS_MISC from the resolved
   pause modes when forcing the MAC external interface in mac_link_up,
   as the vendor driver does, rather than leaving whatever state the
   boot firmware left there. This is done in the MAC layer because
   pcs_link_up() carries no pause information.

 - Implement pcs_get_state() by reading the link status from the
   SerDes, with the forced speed and duplex read back from SDS_MISC.
   Although the supported fixed-link and conventional PHY setups do not
   use it, the PCS owns the SerDes link state, and phylink consults
   pcs_get_state() to track the physical link when operating in in-band
   mode with autonegotiation disabled. The SerDes has no link interrupt
   wired up, so the PCS sets its poll flag.

Tested on a Mercusys MR80X v2.20, where the RTL8367S is connected to
the SoC over SGMII.

Suggested-by: Luiz Angelo Daros de Luca <luizluca@gmail.com>
Suggested-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
Suggested-by: Mieczyslaw Nalewaj <namiltd@yahoo.com>
Signed-off-by: Johan Alvarado <contact@c127.dev>
---
 drivers/net/dsa/realtek/rtl8365mb_main.c | 518 ++++++++++++++++++++++-
 1 file changed, 514 insertions(+), 4 deletions(-)

diff --git a/drivers/net/dsa/realtek/rtl8365mb_main.c b/drivers/net/dsa/realtek/rtl8365mb_main.c
index 5ac091bf93c9..2d202120cfd9 100644
--- a/drivers/net/dsa/realtek/rtl8365mb_main.c
+++ b/drivers/net/dsa/realtek/rtl8365mb_main.c
@@ -40,7 +40,8 @@
  * driver has only been tested with a fixed-link, but in principle it should not
  * matter.
  *
- * NOTE: Currently, only the RGMII interface is implemented in this driver.
+ * NOTE: Currently, only the RGMII and SGMII interfaces are implemented in this
+ * driver.
  *
  * The interrupt line is asserted on link UP/DOWN events. The driver creates a
  * custom irqchip to handle this interrupt and demultiplex the events by reading
@@ -94,11 +95,13 @@
 #include <linux/bitops.h>
 #include <linux/interrupt.h>
 #include <linux/irqdomain.h>
+#include <linux/mii.h>
 #include <linux/mutex.h>
 #include <linux/of_irq.h>
 #include <linux/regmap.h>
 #include <linux/if_bridge.h>
 #include <linux/if_vlan.h>
+#include <linux/phylink.h>
 
 #include "realtek.h"
 #include "realtek-smi.h"
@@ -129,6 +132,7 @@
 
 /* Chip reset register */
 #define RTL8365MB_CHIP_RESET_REG	0x1322
+#define RTL8365MB_CHIP_RESET_DW8051_MASK	0x0010
 #define RTL8365MB_CHIP_RESET_SW_MASK	0x0002
 #define RTL8365MB_CHIP_RESET_HW_MASK	0x0001
 
@@ -238,6 +242,76 @@
 #define   RTL8365MB_EXT_RGMXF_RXDELAY_MASK	0x0007
 #define   RTL8365MB_EXT_RGMXF_TXDELAY_MASK	0x0008
 
+/* External interface line rate bypass register - one bit per external
+ * interface, indexed by the external port number with port 5 (the first
+ * external port) as the base. Other RTL8367 families index this register
+ * differently (e.g. the RTL8367R uses (id + 1) % 2), so this mapping only
+ * holds for the RTL8367C-style parts this driver supports.
+ */
+#define RTL8365MB_BYPASS_LINE_RATE_REG		0x03F7
+#define RTL8365MB_BYPASS_LINE_RATE_MASK(_port)	BIT((_port) - 5)
+
+/* SerDes indirect access registers */
+#define RTL8365MB_SDS_INDACS_CMD_REG		0x6600
+#define   RTL8365MB_SDS_INDACS_CMD_BUSY_MASK	0x0100
+#define   RTL8365MB_SDS_INDACS_CMD_RUN_MASK	0x0080
+#define   RTL8365MB_SDS_INDACS_CMD_WR_MASK	0x0040
+#define RTL8365MB_SDS_INDACS_ADR_REG		0x6601
+#define RTL8365MB_SDS_INDACS_DATA_REG		0x6602
+
+/* SerDes miscellaneous configuration register */
+#define RTL8365MB_SDS_MISC_REG				0x1D11
+#define   RTL8365MB_SDS_MISC_SGMII_RXFC_MASK		0x4000
+#define   RTL8365MB_SDS_MISC_SGMII_TXFC_MASK		0x2000
+#define   RTL8365MB_SDS_MISC_MAC8_SEL_HSGMII_MASK	0x0800
+#define   RTL8365MB_SDS_MISC_SGMII_FDUP_MASK		0x0400
+#define   RTL8365MB_SDS_MISC_SGMII_LINK_MASK		0x0200
+#define   RTL8365MB_SDS_MISC_SGMII_SPD_MASK		0x0180
+#define   RTL8365MB_SDS_MISC_MAC8_SEL_SGMII_MASK	0x0040
+
+/* SerDes internal registers, accessed via the SDS_INDACS registers. The BMCR
+ * data path reset holds BMCR_ANENABLE | BMCR_ISOLATE while toggling the
+ * vendor-specific low bits from phase 1 to phase 2, which triggers a data path
+ * reset and PLL resync.
+ */
+#define RTL8365MB_SDS_REG_BMCR			0x0000
+#define   RTL8365MB_SDS_BMCR_DPRST_PHASE1	(BMCR_ANENABLE | BMCR_ISOLATE | 0x1)
+#define   RTL8365MB_SDS_BMCR_DPRST_PHASE2	(BMCR_ANENABLE | BMCR_ISOLATE | 0x3)
+#define RTL8365MB_SDS_REG_NWAY			0x0002
+#define   RTL8365MB_SDS_NWAY_EN_MASK		0x0200
+#define   RTL8365MB_SDS_NWAY_RESTART_MASK	0x0100
+#define RTL8365MB_SDS_REG_RESET			0x0003
+#define   RTL8365MB_SDS_RESET_DEASSERT		0x7106
+#define RTL8365MB_SDS_REG_LINK_STATUS		0x003d
+#define   RTL8365MB_SDS_LINK_STATUS_LINK_MASK	0x0010
+
+/* The embedded SerDes can only be muxed to external interface 1 (MAC8),
+ * which is port 6.
+ */
+#define RTL8365MB_SDS_EXT_INTERFACE_ID		1
+#define RTL8365MB_SDS_EXT_INTERFACE_PORT	6
+
+/* Line rate bypass bit for the SerDes external interface */
+#define RTL8365MB_SDS_BYPASS_LINE_RATE_MASK \
+	RTL8365MB_BYPASS_LINE_RATE_MASK(RTL8365MB_SDS_EXT_INTERFACE_PORT)
+
+/* SerDes tuning parameter variant selector. The vendor driver picks between
+ * two sets of SerDes tuning parameters based on this chip option. Reading it
+ * requires first arming the read by writing a magic key to the arm register,
+ * then disarming it afterwards.
+ */
+#define RTL8365MB_SDS_OPTION_ARM_REG		0x13C0
+#define   RTL8365MB_SDS_OPTION_ARM_KEY		0x0249
+#define RTL8365MB_SDS_OPTION_REG		0x13C1
+
+/* Embedded DW8051 microcontroller control registers. The microcontroller
+ * can run firmware to manage the SerDes link, but this driver keeps it in
+ * reset and disabled: phylink already performs the link management that
+ * the firmware would otherwise do.
+ */
+#define RTL8365MB_MISC_CFG0_REG			0x130C
+#define   RTL8365MB_MISC_CFG0_DW8051_EN_MASK	0x0020
+
 /* External interface port speed values - used in DIGITAL_INTERFACE_FORCE */
 #define RTL8365MB_PORT_SPEED_10M	0
 #define RTL8365MB_PORT_SPEED_100M	1
@@ -551,6 +625,18 @@ static const struct rtl8365mb_jam_tbl_entry rtl8365mb_init_jam_common[] = {
 	{ 0x1D32, 0x0002 },
 };
 
+/* SGMII SerDes tuning parameters, lifted from the vendor driver sources. The
+ * vendor driver keeps two variants of this table and selects between them
+ * based on the chip option register; these are the values for a non-zero
+ * option, which is what RTL8367S parts seen so far report. See
+ * rtl8365mb_sds_probe_option().
+ */
+static const struct rtl8365mb_jam_tbl_entry rtl8365mb_sds_jam_sgmii[] = {
+	{ 0x0480, 0x04D7 }, { 0x0481, 0xF994 }, { 0x0482, 0x2420 },
+	{ 0x0483, 0x6960 }, { 0x0484, 0x9728 }, { 0x0423, 0x9D85 },
+	{ 0x0424, 0xD810 }, { 0x002E, 0x83F2 },
+};
+
 enum rtl8365mb_phy_interface_mode {
 	RTL8365MB_PHY_INTERFACE_MODE_INVAL = 0,
 	RTL8365MB_PHY_INTERFACE_MODE_INTERNAL = BIT(0),
@@ -730,6 +816,9 @@ struct rtl8365mb_port {
  * @cpu: CPU tagging and CPU port configuration for this chip
  * @mib_lock: prevent concurrent reads of MIB counters
  * @ports: per-port data
+ * @pcs: PCS for the SerDes external interface
+ * @sds_supported: SerDes tuning parameters match the chip option, so the
+ *                 SerDes interface modes can be advertised
  *
  * Private data for this driver.
  */
@@ -740,8 +829,12 @@ struct rtl8365mb {
 	struct rtl8365mb_cpu cpu;
 	struct mutex mib_lock;
 	struct rtl8365mb_port ports[RTL8365MB_MAX_NUM_PORTS];
+	struct phylink_pcs pcs;
+	bool sds_supported;
 };
 
+#define pcs_to_rtl8365mb(_pcs) container_of((_pcs), struct rtl8365mb, pcs)
+
 static int rtl8365mb_phy_poll_busy(struct realtek_priv *priv)
 {
 	u32 val;
@@ -1042,6 +1135,342 @@ static int rtl8365mb_ext_config_rgmii(struct realtek_priv *priv, int port,
 	return 0;
 }
 
+static int rtl8365mb_sds_write(struct realtek_priv *priv, u16 addr, u16 data)
+{
+	int ret;
+
+	ret = regmap_write(priv->map, RTL8365MB_SDS_INDACS_DATA_REG, data);
+	if (ret)
+		return ret;
+
+	ret = regmap_write(priv->map, RTL8365MB_SDS_INDACS_ADR_REG, addr);
+	if (ret)
+		return ret;
+
+	/* The SerDes indirect access engine completes the command within the
+	 * register write transaction, so there is no need to wait or poll for
+	 * completion before the next access, matching the vendor driver.
+	 */
+	return regmap_write(priv->map, RTL8365MB_SDS_INDACS_CMD_REG,
+			    RTL8365MB_SDS_INDACS_CMD_RUN_MASK |
+			    RTL8365MB_SDS_INDACS_CMD_WR_MASK);
+}
+
+static int rtl8365mb_sds_read(struct realtek_priv *priv, u16 addr, u16 *data)
+{
+	u32 val;
+	int ret;
+
+	ret = regmap_write(priv->map, RTL8365MB_SDS_INDACS_ADR_REG, addr);
+	if (ret)
+		return ret;
+
+	ret = regmap_write(priv->map, RTL8365MB_SDS_INDACS_CMD_REG,
+			   RTL8365MB_SDS_INDACS_CMD_RUN_MASK);
+	if (ret)
+		return ret;
+
+	/* Wait for the indirect read to complete: the engine clears the BUSY
+	 * bit once the data register holds the result.
+	 */
+	ret = regmap_read_poll_timeout(priv->map, RTL8365MB_SDS_INDACS_CMD_REG,
+				       val,
+				       !(val & RTL8365MB_SDS_INDACS_CMD_BUSY_MASK),
+				       10, 1000);
+	if (ret)
+		return ret;
+
+	ret = regmap_read(priv->map, RTL8365MB_SDS_INDACS_DATA_REG, &val);
+	if (ret)
+		return ret;
+
+	*data = val;
+
+	return 0;
+}
+
+/* The vendor driver selects between two sets of SerDes tuning parameters based
+ * on the chip option register. Only the variant for a non-zero option has been
+ * tested on real hardware - the RTL8367S parts seen so far all report 1. The
+ * variant for option 0 uses different tuning values that cannot be verified,
+ * so probe the option once at setup and only advertise the SerDes interface
+ * modes when the tuning parameters are known to match, so that an unsupported
+ * variant fails at phylink validation time rather than when configuring the
+ * link.
+ */
+static int rtl8365mb_sds_probe_option(struct realtek_priv *priv)
+{
+	struct rtl8365mb *mb = priv->chip_data;
+	const struct rtl8365mb_extint *extint;
+	u32 option;
+	int ret;
+	int i;
+
+	/* Nothing to probe if no external interface is wired to the SerDes */
+	for (i = 0; i < RTL8365MB_MAX_NUM_EXTINTS; i++) {
+		extint = &mb->chip_info->extints[i];
+
+		if (extint->supported_interfaces &
+		    (RTL8365MB_PHY_INTERFACE_MODE_SGMII |
+		     RTL8365MB_PHY_INTERFACE_MODE_HSGMII))
+			break;
+	}
+	if (i == RTL8365MB_MAX_NUM_EXTINTS)
+		return 0;
+
+	ret = regmap_write(priv->map, RTL8365MB_SDS_OPTION_ARM_REG,
+			   RTL8365MB_SDS_OPTION_ARM_KEY);
+	if (ret)
+		return ret;
+
+	ret = regmap_read(priv->map, RTL8365MB_SDS_OPTION_REG, &option);
+	if (ret)
+		return ret;
+
+	ret = regmap_write(priv->map, RTL8365MB_SDS_OPTION_ARM_REG, 0);
+	if (ret)
+		return ret;
+
+	if (option == 0) {
+		dev_warn(priv->dev,
+			 "unsupported SerDes tuning variant (chip option 0), disabling SerDes interface modes\n");
+		return 0;
+	}
+
+	mb->sds_supported = true;
+
+	return 0;
+}
+
+static int rtl8365mb_pcs_config(struct phylink_pcs *pcs, unsigned int neg_mode,
+				phy_interface_t interface,
+				const unsigned long *advertising,
+				bool permit_pause_to_mac)
+{
+	const int id = RTL8365MB_SDS_EXT_INTERFACE_ID;
+	struct rtl8365mb *mb = pcs_to_rtl8365mb(pcs);
+	struct realtek_priv *priv;
+	u16 val;
+	int ret;
+	int i;
+
+	priv = mb->priv;
+
+	/* This driver does not implement SGMII in-band autonegotiation yet, so
+	 * the link parameters are forced from rtl8365mb_pcs_link_up() instead.
+	 * rtl8365mb_pcs_inband_caps() reports this to phylink, which should
+	 * therefore never select an in-band-enabled negotiation mode.
+	 */
+	if (neg_mode == PHYLINK_PCS_NEG_INBAND_ENABLED)
+		return -EOPNOTSUPP;
+
+	/* Hold the embedded DW8051 microcontroller in reset and keep it
+	 * disabled. The vendor driver loads firmware into it to manage the
+	 * SerDes link, but the firmware only duplicates work that phylink
+	 * already does: it polls the port status and forces the external
+	 * interface configuration in the very registers this driver manages.
+	 * Letting it run would race with phylink.
+	 */
+	ret = regmap_update_bits(priv->map, RTL8365MB_CHIP_RESET_REG,
+				 RTL8365MB_CHIP_RESET_DW8051_MASK,
+				 RTL8365MB_CHIP_RESET_DW8051_MASK);
+	if (ret)
+		return ret;
+
+	ret = regmap_update_bits(priv->map, RTL8365MB_MISC_CFG0_REG,
+				 RTL8365MB_MISC_CFG0_DW8051_EN_MASK, 0);
+	if (ret)
+		return ret;
+
+	/* The vendor driver clears the line rate bypass for all interface
+	 * modes except TMII.
+	 */
+	ret = regmap_update_bits(priv->map, RTL8365MB_BYPASS_LINE_RATE_REG,
+				 RTL8365MB_SDS_BYPASS_LINE_RATE_MASK, 0);
+	if (ret)
+		return ret;
+
+	/* Tune the SerDes with vendor-prescribed parameters */
+	for (i = 0; i < ARRAY_SIZE(rtl8365mb_sds_jam_sgmii); i++) {
+		ret = rtl8365mb_sds_write(priv,
+					  rtl8365mb_sds_jam_sgmii[i].reg,
+					  rtl8365mb_sds_jam_sgmii[i].val);
+		if (ret)
+			return ret;
+	}
+
+	/* Mux the SerDes to MAC8 in SGMII mode */
+	ret = regmap_update_bits(priv->map, RTL8365MB_SDS_MISC_REG,
+				 RTL8365MB_SDS_MISC_MAC8_SEL_SGMII_MASK |
+					 RTL8365MB_SDS_MISC_MAC8_SEL_HSGMII_MASK,
+				 RTL8365MB_SDS_MISC_MAC8_SEL_SGMII_MASK);
+	if (ret)
+		return ret;
+
+	val = RTL8365MB_EXT_PORT_MODE_SGMII
+	      << RTL8365MB_DIGITAL_INTERFACE_SELECT_MODE_OFFSET(id);
+	ret = regmap_update_bits(priv->map,
+				 RTL8365MB_DIGITAL_INTERFACE_SELECT_REG(id),
+				 RTL8365MB_DIGITAL_INTERFACE_SELECT_MODE_MASK(id),
+				 val);
+	if (ret)
+		return ret;
+
+	/* Take the SerDes out of reset. The vendor driver does this only
+	 * after the SerDes mux and the interface mode are configured.
+	 */
+	ret = rtl8365mb_sds_write(priv, RTL8365MB_SDS_REG_RESET,
+				  RTL8365MB_SDS_RESET_DEASSERT);
+	if (ret)
+		return ret;
+
+	/* Reset the SerDes data path and resync its PLL, mirroring what the
+	 * vendor firmware does right after deasserting the SerDes reset.
+	 * This flushes the FIFOs and ensures a clean state for the link,
+	 * preventing silent drops and CRC errors.
+	 */
+	ret = rtl8365mb_sds_write(priv, RTL8365MB_SDS_REG_BMCR,
+				  RTL8365MB_SDS_BMCR_DPRST_PHASE1);
+	if (ret)
+		return ret;
+
+	ret = rtl8365mb_sds_write(priv, RTL8365MB_SDS_REG_BMCR,
+				  RTL8365MB_SDS_BMCR_DPRST_PHASE2);
+	if (ret)
+		return ret;
+
+	/* Keep SGMII in-band autonegotiation disabled: the link parameters are
+	 * forced from rtl8365mb_pcs_link_up() instead.
+	 */
+	ret = rtl8365mb_sds_read(priv, RTL8365MB_SDS_REG_NWAY, &val);
+	if (ret)
+		return ret;
+
+	val &= ~RTL8365MB_SDS_NWAY_EN_MASK;
+	val |= RTL8365MB_SDS_NWAY_RESTART_MASK;
+
+	return rtl8365mb_sds_write(priv, RTL8365MB_SDS_REG_NWAY, val);
+}
+
+static bool rtl8365mb_interface_is_serdes(phy_interface_t interface)
+{
+	return interface == PHY_INTERFACE_MODE_SGMII;
+}
+
+static unsigned int rtl8365mb_pcs_inband_caps(struct phylink_pcs *pcs,
+					      phy_interface_t interface)
+{
+	/* In-band autonegotiation is not implemented; the link is always
+	 * forced. Report that to phylink so that it never selects an
+	 * in-band-enabled negotiation mode for this PCS.
+	 */
+	return LINK_INBAND_DISABLE;
+}
+
+static void rtl8365mb_pcs_get_state(struct phylink_pcs *pcs,
+				    unsigned int neg_mode,
+				    struct phylink_link_state *state)
+{
+	struct rtl8365mb *mb = pcs_to_rtl8365mb(pcs);
+	struct realtek_priv *priv = mb->priv;
+	u16 status;
+	u32 val;
+	int ret;
+
+	/* In-band autonegotiation is not implemented, so the link parameters are
+	 * forced from rtl8365mb_pcs_link_up(). The real link state must still be
+	 * read from the SerDes itself: the embedded DW8051 microcontroller that
+	 * the vendor firmware uses to poll the SerDes is kept disabled (see
+	 * rtl8365mb_pcs_config()), so the link status register can be read
+	 * directly through the SDS_INDACS window without racing the auto-poll.
+	 */
+	ret = rtl8365mb_sds_read(priv, RTL8365MB_SDS_REG_LINK_STATUS, &status);
+	if (ret) {
+		state->link = false;
+		return;
+	}
+
+	state->link = !!(status & RTL8365MB_SDS_LINK_STATUS_LINK_MASK);
+	state->an_complete = state->link;
+	if (!state->link)
+		return;
+
+	/* The speed and duplex are forced; read them back from the values
+	 * programmed into the SerDes MISC register.
+	 */
+	ret = regmap_read(priv->map, RTL8365MB_SDS_MISC_REG, &val);
+	if (ret) {
+		state->link = false;
+		return;
+	}
+
+	state->duplex = (val & RTL8365MB_SDS_MISC_SGMII_FDUP_MASK) ?
+				DUPLEX_FULL : DUPLEX_HALF;
+
+	switch (FIELD_GET(RTL8365MB_SDS_MISC_SGMII_SPD_MASK, val)) {
+	case RTL8365MB_PORT_SPEED_1000M:
+		state->speed = SPEED_1000;
+		break;
+	case RTL8365MB_PORT_SPEED_100M:
+		state->speed = SPEED_100;
+		break;
+	case RTL8365MB_PORT_SPEED_10M:
+		state->speed = SPEED_10;
+		break;
+	}
+}
+
+static void rtl8365mb_pcs_link_up(struct phylink_pcs *pcs,
+				  unsigned int neg_mode,
+				  phy_interface_t interface, int speed,
+				  int duplex)
+{
+	struct rtl8365mb *mb = pcs_to_rtl8365mb(pcs);
+	struct realtek_priv *priv = mb->priv;
+	u32 mask = RTL8365MB_SDS_MISC_SGMII_FDUP_MASK |
+		   RTL8365MB_SDS_MISC_SGMII_LINK_MASK |
+		   RTL8365MB_SDS_MISC_SGMII_SPD_MASK;
+	u32 val = RTL8365MB_SDS_MISC_SGMII_LINK_MASK;
+	u32 r_speed;
+	int ret;
+
+	if (speed == SPEED_1000) {
+		r_speed = RTL8365MB_PORT_SPEED_1000M;
+	} else if (speed == SPEED_100) {
+		r_speed = RTL8365MB_PORT_SPEED_100M;
+	} else if (speed == SPEED_10) {
+		r_speed = RTL8365MB_PORT_SPEED_10M;
+	} else {
+		dev_err(priv->dev, "unsupported SerDes speed %s\n",
+			phy_speed_to_str(speed));
+		return;
+	}
+
+	val |= FIELD_PREP(RTL8365MB_SDS_MISC_SGMII_SPD_MASK, r_speed);
+
+	if (duplex == DUPLEX_FULL)
+		val |= RTL8365MB_SDS_MISC_SGMII_FDUP_MASK;
+
+	/* pcs_link_up() carries no pause information, so the SerDes flow
+	 * control bits are programmed together with the MAC external interface
+	 * force from rtl8365mb_phylink_mac_link_up(), where the resolved pause
+	 * modes are known.
+	 */
+	ret = regmap_update_bits(priv->map, RTL8365MB_SDS_MISC_REG, mask, val);
+	if (ret) {
+		dev_err(priv->dev, "failed to force SerDes link: %pe\n",
+			ERR_PTR(ret));
+		return;
+	}
+}
+
+static const struct phylink_pcs_ops rtl8365mb_pcs_ops = {
+	.pcs_inband_caps = rtl8365mb_pcs_inband_caps,
+	.pcs_config = rtl8365mb_pcs_config,
+	.pcs_get_state = rtl8365mb_pcs_get_state,
+	.pcs_link_up = rtl8365mb_pcs_link_up,
+};
+
 static int rtl8365mb_ext_config_forcemode(struct realtek_priv *priv, int port,
 					  bool link, int speed, int duplex,
 					  bool tx_pause, bool rx_pause)
@@ -1118,6 +1547,8 @@ static void rtl8365mb_phylink_get_caps(struct dsa_switch *ds, int port,
 {
 	const struct rtl8365mb_extint *extint =
 		rtl8365mb_get_port_extint(ds->priv, port);
+	struct realtek_priv *priv = ds->priv;
+	struct rtl8365mb *mb = priv->chip_data;
 
 	config->mac_capabilities = MAC_SYM_PAUSE | MAC_ASYM_PAUSE |
 				   MAC_10 | MAC_100 | MAC_1000FD;
@@ -1141,6 +1572,25 @@ static void rtl8365mb_phylink_get_caps(struct dsa_switch *ds, int port,
 
 	if (extint->supported_interfaces & RTL8365MB_PHY_INTERFACE_MODE_RGMII)
 		phy_interface_set_rgmii(config->supported_interfaces);
+
+	if (extint->supported_interfaces & RTL8365MB_PHY_INTERFACE_MODE_SGMII &&
+	    mb->sds_supported)
+		__set_bit(PHY_INTERFACE_MODE_SGMII,
+			  config->supported_interfaces);
+}
+
+static struct phylink_pcs *
+rtl8365mb_phylink_mac_select_pcs(struct phylink_config *config,
+				 phy_interface_t interface)
+{
+	struct dsa_port *dp = dsa_phylink_to_port(config);
+	struct realtek_priv *priv = dp->ds->priv;
+	struct rtl8365mb *mb = priv->chip_data;
+
+	if (rtl8365mb_interface_is_serdes(interface))
+		return &mb->pcs;
+
+	return NULL;
 }
 
 static void rtl8365mb_phylink_mac_config(struct phylink_config *config,
@@ -1168,6 +1618,12 @@ static void rtl8365mb_phylink_mac_config(struct phylink_config *config,
 		return;
 	}
 
+	/* SGMII is handled by the SerDes PCS, configured through the
+	 * phylink_pcs ops, so there is nothing to do here for it.
+	 */
+	if (rtl8365mb_interface_is_serdes(state->interface))
+		return;
+
 	/* TODO: Implement MII and RMII modes, which the RTL8365MB-VC also
 	 * supports
 	 */
@@ -1188,7 +1644,13 @@ static void rtl8365mb_phylink_mac_link_down(struct phylink_config *config,
 	p = &mb->ports[port];
 	cancel_delayed_work_sync(&p->mib_work);
 
-	if (phy_interface_mode_is_rgmii(interface)) {
+	/* phylink has no pcs_link_down callback, so on the SerDes path only the
+	 * MAC external interface force is reset here. Clearing the MAC force is
+	 * enough to bring the link down; the SerDes keeps presenting its last
+	 * forced state until the next pcs_link_up() reprograms it.
+	 */
+	if (phy_interface_mode_is_rgmii(interface) ||
+	    rtl8365mb_interface_is_serdes(interface)) {
 		ret = rtl8365mb_ext_config_forcemode(priv, port, false, 0, 0,
 						     false, false);
 		if (ret)
@@ -1218,14 +1680,46 @@ static void rtl8365mb_phylink_mac_link_up(struct phylink_config *config,
 	p = &mb->ports[port];
 	schedule_delayed_work(&p->mib_work, 0);
 
-	if (phy_interface_mode_is_rgmii(interface)) {
+	/* The SerDes forced link state is programmed by the PCS in
+	 * rtl8365mb_pcs_link_up(); here only the MAC external interface force
+	 * is configured, for both RGMII and SerDes.
+	 */
+	if (phy_interface_mode_is_rgmii(interface) ||
+	    rtl8365mb_interface_is_serdes(interface)) {
 		ret = rtl8365mb_ext_config_forcemode(priv, port, true, speed,
 						     duplex, tx_pause,
 						     rx_pause);
-		if (ret)
+		if (ret) {
 			dev_err(priv->dev,
 				"failed to force mode on port %d: %pe\n", port,
 				ERR_PTR(ret));
+			return;
+		}
+
+		/* The SerDes has its own pause controls; program them from
+		 * the resolved pause modes, as the vendor driver does when
+		 * forcing the link on a SerDes external interface. This is
+		 * done here rather than in rtl8365mb_pcs_link_up() because
+		 * pcs_link_up() carries no pause information.
+		 */
+		if (rtl8365mb_interface_is_serdes(interface)) {
+			u32 val = 0;
+
+			if (tx_pause)
+				val |= RTL8365MB_SDS_MISC_SGMII_TXFC_MASK;
+			if (rx_pause)
+				val |= RTL8365MB_SDS_MISC_SGMII_RXFC_MASK;
+
+			ret = regmap_update_bits(priv->map,
+						 RTL8365MB_SDS_MISC_REG,
+						 RTL8365MB_SDS_MISC_SGMII_TXFC_MASK |
+							 RTL8365MB_SDS_MISC_SGMII_RXFC_MASK,
+						 val);
+			if (ret)
+				dev_err(priv->dev,
+					"failed to force SerDes pause modes on port %d: %pe\n",
+					port, ERR_PTR(ret));
+		}
 
 		return;
 	}
@@ -2419,6 +2913,14 @@ static int rtl8365mb_setup(struct dsa_switch *ds)
 	mb = priv->chip_data;
 	cpu = &mb->cpu;
 
+	mb->pcs.ops = &rtl8365mb_pcs_ops;
+
+	/* The SerDes has no link interrupt wired up, so phylink must poll the
+	 * PCS for link changes when it tracks the link through pcs_get_state()
+	 * (in-band mode with autonegotiation disabled).
+	 */
+	mb->pcs.poll = true;
+
 	ret = rtl8365mb_reset_chip(priv);
 	if (ret) {
 		dev_err(priv->dev, "failed to reset chip: %pe\n",
@@ -2426,6 +2928,13 @@ static int rtl8365mb_setup(struct dsa_switch *ds)
 		goto out_error;
 	}
 
+	ret = rtl8365mb_sds_probe_option(priv);
+	if (ret) {
+		dev_err(priv->dev, "failed to probe SerDes chip option: %pe\n",
+			ERR_PTR(ret));
+		goto out_error;
+	}
+
 	/* Configure switch to vendor-defined initial state */
 	ret = rtl8365mb_switch_init(priv);
 	if (ret) {
@@ -2658,6 +3167,7 @@ static int rtl8365mb_detect(struct realtek_priv *priv)
 }
 
 static const struct phylink_mac_ops rtl8365mb_phylink_mac_ops = {
+	.mac_select_pcs = rtl8365mb_phylink_mac_select_pcs,
 	.mac_config = rtl8365mb_phylink_mac_config,
 	.mac_link_down = rtl8365mb_phylink_mac_link_down,
 	.mac_link_up = rtl8365mb_phylink_mac_link_up,

base-commit: d6e81529749190123aa0040626c7e5dbc20fdc9a
-- 
2.55.0

Re: [PATCH net-next v4 1/2] net: dsa: realtek: rtl8365mb: add SGMII support for RTL8367S

[Maxime Chevallier]

Hi Johan,

On 7/2/26 22:47, Johan Alvarado wrote:
> The RTL8367S can mux its embedded SerDes to external interface 1,
> which is typically used to connect the switch to a CPU port. The chip
> info table already declares SGMII as a supported interface mode for
> this chip, but the driver only implements RGMII so far.
> 
> Implement SGMII support as a phylink PCS, with the configuration
> sequence derived from the GPL-licensed Realtek rtl8367c vendor driver
> as distributed in the Mercusys MR80X GPL code drop:
> 
>  - Add accessors for the SerDes indirect access registers (SDS_INDACS),
>    through which the SerDes internal registers are reached.
> 
>  - Register a phylink_pcs for the SerDes, selected from mac_select_pcs
>    for the SGMII interface, so the SerDes handling lives in the PCS
>    operations rather than in the MAC operations.
> 
>  - Probe the SerDes tuning variant from the chip option register once
>    at setup. The vendor driver keeps two sets of SerDes tuning
>    parameters and selects between them based on this option; only the
>    variant for a non-zero option (which all RTL8367S parts seen so far
>    report) has been validated on hardware, so the SerDes interface
>    modes are only advertised in that case. An unsupported variant thus
>    fails at phylink validation time instead of at link configuration
>    time.
> 
>  - Keep the embedded DW8051 microcontroller in reset and disabled. The
>    vendor driver loads firmware into it to manage the SerDes link, but
>    analysis of that firmware shows it only duplicates the link
>    management phylink already performs: it polls the port status and
>    writes the external interface force registers behind the driver's
>    back.
> 
>  - Clear the line rate bypass bit for the external interface, tune the
>    SerDes with the vendor-prescribed parameters, mux the SerDes to MAC8
>    in SGMII mode and only then take the SerDes out of reset, as the
>    vendor driver does.
> 
>  - After deasserting the SerDes reset, reset the SerDes data path via
>    the SerDes BMCR register to flush the FIFOs and resync the PLL.
>    This mirrors what the vendor firmware does right after deasserting
>    the SerDes reset, and ensures a clean link state from cold boot.
> 
>  - Force the SGMII link parameters (link, speed, duplex) in the SDS_MISC
>    register from pcs_link_up(). SGMII in-band autonegotiation is not
>    implemented, so only fixed-link and conventional PHY setups are
>    supported, just like RGMII. This is reported to phylink through
>    pcs_inband_caps() returning LINK_INBAND_DISABLE, so phylink never
>    selects an in-band-enabled negotiation mode for this PCS.
> 
>  - Program the SerDes pause controls in SDS_MISC from the resolved
>    pause modes when forcing the MAC external interface in mac_link_up,
>    as the vendor driver does, rather than leaving whatever state the
>    boot firmware left there. This is done in the MAC layer because
>    pcs_link_up() carries no pause information.
> 
>  - Implement pcs_get_state() by reading the link status from the
>    SerDes, with the forced speed and duplex read back from SDS_MISC.
>    Although the supported fixed-link and conventional PHY setups do not
>    use it, the PCS owns the SerDes link state, and phylink consults
>    pcs_get_state() to track the physical link when operating in in-band
>    mode with autonegotiation disabled. The SerDes has no link interrupt
>    wired up, so the PCS sets its poll flag.
> 
> Tested on a Mercusys MR80X v2.20, where the RTL8367S is connected to
> the SoC over SGMII.

Ah nice conversion to phylink PCS :) I have a few comments below

> 
> Suggested-by: Luiz Angelo Daros de Luca <luizluca@gmail.com>
> Suggested-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
> Suggested-by: Mieczyslaw Nalewaj <namiltd@yahoo.com>
> Signed-off-by: Johan Alvarado <contact@c127.dev>
> ---

[...]

> +static int rtl8365mb_pcs_config(struct phylink_pcs *pcs, unsigned int neg_mode,
> +				phy_interface_t interface,
> +				const unsigned long *advertising,
> +				bool permit_pause_to_mac)
> +{
> +	const int id = RTL8365MB_SDS_EXT_INTERFACE_ID;
> +	struct rtl8365mb *mb = pcs_to_rtl8365mb(pcs);
> +	struct realtek_priv *priv;
> +	u16 val;
> +	int ret;
> +	int i;
> +
> +	priv = mb->priv;
> +
> +	/* This driver does not implement SGMII in-band autonegotiation yet, so
> +	 * the link parameters are forced from rtl8365mb_pcs_link_up() instead.
> +	 * rtl8365mb_pcs_inband_caps() reports this to phylink, which should
> +	 * therefore never select an in-band-enabled negotiation mode.
> +	 */
> +	if (neg_mode == PHYLINK_PCS_NEG_INBAND_ENABLED)
> +		return -EOPNOTSUPP;

As you implement the .pcs_inband_caps() method, phylink will pass you a valid
mode, no need for that check :)

[...]
> @@ -1218,14 +1680,46 @@ static void rtl8365mb_phylink_mac_link_up(struct phylink_config *config,
>  	p = &mb->ports[port];
>  	schedule_delayed_work(&p->mib_work, 0);
>  
> -	if (phy_interface_mode_is_rgmii(interface)) {
> +	/* The SerDes forced link state is programmed by the PCS in
> +	 * rtl8365mb_pcs_link_up(); here only the MAC external interface force
> +	 * is configured, for both RGMII and SerDes.
> +	 */
> +	if (phy_interface_mode_is_rgmii(interface) ||
> +	    rtl8365mb_interface_is_serdes(interface)) {
>  		ret = rtl8365mb_ext_config_forcemode(priv, port, true, speed,
>  						     duplex, tx_pause,
>  						     rx_pause);
> -		if (ret)
> +		if (ret) {
>  			dev_err(priv->dev,
>  				"failed to force mode on port %d: %pe\n", port,
>  				ERR_PTR(ret));
> +			return;
> +		}
> +
> +		/* The SerDes has its own pause controls; program them from
> +		 * the resolved pause modes, as the vendor driver does when
> +		 * forcing the link on a SerDes external interface. This is
> +		 * done here rather than in rtl8365mb_pcs_link_up() because
> +		 * pcs_link_up() carries no pause information.
> +		 */
> +		if (rtl8365mb_interface_is_serdes(interface)) {
> +			u32 val = 0;
> +
> +			if (tx_pause)
> +				val |= RTL8365MB_SDS_MISC_SGMII_TXFC_MASK;
> +			if (rx_pause)
> +				val |= RTL8365MB_SDS_MISC_SGMII_RXFC_MASK;

Do you know what this does in HW ? Is this so that the PCS lets the Pause frames through
in either directions ?

I suspect this is something that would be only used for inband advertising
of pause settings (in such case, you don't even need that), but ofc I'm not sure :)

You already configure the MAC pause settings, can you test that these bits actually do
anything by exercising a bit flow control and checking if these registers are used ?

Otherwise, this is looking pretty good :)

Maxime

Re: [PATCH net-next v4 1/2] net: dsa: realtek: rtl8365mb: add SGMII support for RTL8367S

[Johan Alvarado]

Hi Maxime,

On 7/3/26 09:29, Maxime Chevallier wrote:
> Ah nice conversion to phylink PCS :) I have a few comments below

Thanks for the review!

[...]

>> +	/* This driver does not implement SGMII in-band autonegotiation yet, so
>> +	 * the link parameters are forced from rtl8365mb_pcs_link_up() instead.
>> +	 * rtl8365mb_pcs_inband_caps() reports this to phylink, which should
>> +	 * therefore never select an in-band-enabled negotiation mode.
>> +	 */
>> +	if (neg_mode == PHYLINK_PCS_NEG_INBAND_ENABLED)
>> +		return -EOPNOTSUPP;
>
> As you implement the .pcs_inband_caps() method, phylink will pass you a valid
> mode, no need for that check :)

Indeed. I went through phylink_pcs_neg_mode() again and with
pcs_inband_caps() returning LINK_INBAND_DISABLE every path resolves to
an outband (or inband-disabled, for the 802.3z no-PHY case) negotiation
mode, so this is dead code. I will drop it in v5.

[...]

>> +		/* The SerDes has its own pause controls; program them from
>> +		 * the resolved pause modes, as the vendor driver does when
>> +		 * forcing the link on a SerDes external interface. This is
>> +		 * done here rather than in rtl8365mb_pcs_link_up() because
>> +		 * pcs_link_up() carries no pause information.
>> +		 */
>> +		if (rtl8365mb_interface_is_serdes(interface)) {
>> +			u32 val = 0;
>> +
>> +			if (tx_pause)
>> +				val |= RTL8365MB_SDS_MISC_SGMII_TXFC_MASK;
>> +			if (rx_pause)
>> +				val |= RTL8365MB_SDS_MISC_SGMII_RXFC_MASK;
>
> Do you know what this does in HW ? Is this so that the PCS lets the Pause frames through
> in either directions ?
>
> I suspect this is something that would be only used for inband advertising
> of pause settings (in such case, you don't even need that), but ofc I'm not sure :)
>
> You already configure the MAC pause settings, can you test that these bits actually do
> anything by exercising a bit flow control and checking if these registers are used ?

I tested it on the hardware, and it turns out these bits are the
operative pause controls for the SerDes port: pause frames are only
ever emitted when SDS_MISC_SGMII_TXFC is set, and the MAC force-mode
pause bits alone have no observable effect on this port. They are not
tied to in-band advertisement either - they act with the link fully
forced.

For context, there is no public documentation for these bits, but in
the vendor GPL code every write to them lives in
rtl8367c_setAsicPortForceLinkExt(), the forced-link path, where they
are programmed from the resolved tx/rx pause settings right alongside
the MAC force-mode pause bits - never from the nway/in-band paths.

Test setup: RTL8367S (Mercusys MR80X), CPU port 6 on HSGMII,
2500base-x fixed link to an IPQ5018. I enabled regmap debugfs writes
so I could flip the bits live, and forced congestion by L2-forwarding
~350 Mbit/s of SoC-generated UDP in through the CPU port and out of a
100M user port (~3.5x oversubscription, ~135 MB per run), watching the
CPU port's dot3OutPauseFrames MIB counter:

  everything cleared (baseline):                 0 pause frames
  MAC force pause only (DIGITAL_INTERFACE_FORCE
      TXPAUSE|RXPAUSE, reg 0x1311):              0 pause frames
  MAC force pause + SDS TXFC|RXFC:            2396 pause frames
  SDS TXFC|RXFC only (MAC pause cleared):     2362 pause frames
  SDS TXFC only:                              2934 pause frames
  SDS RXFC only:                                 0 pause frames

RXFC did not influence pause emission, so by symmetry it presumably
gates honouring received pause frames; I could not exercise that
direction because the SoC MAC on this board never transmits pause.

So the write has to stay - without it flow control simply does not
work on SGMII/HSGMII. I will expand the comment in v5 to say that
these are the functional pause enables for the SerDes, rather than
describing it as mirroring the vendor forced-link sequence.

Best regards,
Johan

Re: [PATCH net-next v4 1/2] net: dsa: realtek: rtl8365mb: add SGMII support for RTL8367S

[Maxime Chevallier]

Hi,

On 7/8/26 03:09, Johan Alvarado wrote:

> 
> [...]
> 
>>> +		/* The SerDes has its own pause controls; program them from
>>> +		 * the resolved pause modes, as the vendor driver does when
>>> +		 * forcing the link on a SerDes external interface. This is
>>> +		 * done here rather than in rtl8365mb_pcs_link_up() because
>>> +		 * pcs_link_up() carries no pause information.
>>> +		 */
>>> +		if (rtl8365mb_interface_is_serdes(interface)) {
>>> +			u32 val = 0;
>>> +
>>> +			if (tx_pause)
>>> +				val |= RTL8365MB_SDS_MISC_SGMII_TXFC_MASK;
>>> +			if (rx_pause)
>>> +				val |= RTL8365MB_SDS_MISC_SGMII_RXFC_MASK;
>>
>> Do you know what this does in HW ? Is this so that the PCS lets the Pause frames through
>> in either directions ?
>>
>> I suspect this is something that would be only used for inband advertising
>> of pause settings (in such case, you don't even need that), but ofc I'm not sure :)
>>
>> You already configure the MAC pause settings, can you test that these bits actually do
>> anything by exercising a bit flow control and checking if these registers are used ?
> 
> I tested it on the hardware, and it turns out these bits are the
> operative pause controls for the SerDes port: pause frames are only
> ever emitted when SDS_MISC_SGMII_TXFC is set, and the MAC force-mode
> pause bits alone have no observable effect on this port. They are not
> tied to in-band advertisement either - they act with the link fully
> forced.
> 
> For context, there is no public documentation for these bits, but in
> the vendor GPL code every write to them lives in
> rtl8367c_setAsicPortForceLinkExt(), the forced-link path, where they
> are programmed from the resolved tx/rx pause settings right alongside
> the MAC force-mode pause bits - never from the nway/in-band paths.
> 
> Test setup: RTL8367S (Mercusys MR80X), CPU port 6 on HSGMII,
> 2500base-x fixed link to an IPQ5018. I enabled regmap debugfs writes
> so I could flip the bits live, and forced congestion by L2-forwarding
> ~350 Mbit/s of SoC-generated UDP in through the CPU port and out of a
> 100M user port (~3.5x oversubscription, ~135 MB per run), watching the
> CPU port's dot3OutPauseFrames MIB counter:
> 
>   everything cleared (baseline):                 0 pause frames
>   MAC force pause only (DIGITAL_INTERFACE_FORCE
>       TXPAUSE|RXPAUSE, reg 0x1311):              0 pause frames
>   MAC force pause + SDS TXFC|RXFC:            2396 pause frames
>   SDS TXFC|RXFC only (MAC pause cleared):     2362 pause frames
>   SDS TXFC only:                              2934 pause frames
>   SDS RXFC only:                                 0 pause frames
> 
> RXFC did not influence pause emission, so by symmetry it presumably
> gates honouring received pause frames; I could not exercise that
> direction because the SoC MAC on this board never transmits pause.
> 
> So the write has to stay - without it flow control simply does not
> work on SGMII/HSGMII. I will expand the comment in v5 to say that
> these are the functional pause enables for the SerDes, rather than
> describing it as mirroring the vendor forced-link sequence.
> 

Thanks a lot for the investigation ! Ok so you do need to configure that
in the PCS. Maybe at some point we'll have to pass tx/rx negotiated pause
settings to the PCS, just like we to for the MAC, but it's strange as the
PCS isn't supposed to be the block handling pause :/

I'm ok leaving this as-is right now, you've added a nice comment on why
PCS regs are accessed in the MAC's link_up(). If we find other devices
that behave the same w.r.t. pause, we may have to pass these to the pcs_link_up,
but it's rather unusual I think.

Thanks :)

Maxime 

[PATCH net-next v4 2/2] net: dsa: realtek: rtl8365mb: add HSGMII support for RTL8367S

[Johan Alvarado]

In addition to SGMII, the RTL8367S SerDes also supports HSGMII, which
carries 2.5 Gbps with the same signaling as SGMII at 2.5x clock rate.
The chip info table already declares HSGMII as a supported interface
mode for external interface 1.

Extend the SerDes PCS to handle HSGMII, which phylink represents as
2500base-x:

 - Select the HSGMII SerDes tuning parameters and external interface
   mode, and mux the SerDes to MAC8 in HSGMII mode, from pcs_config()
   according to the interface. The parameters are again lifted from the
   GPL-licensed Realtek rtl8367c vendor driver, and again only cover
   the tuning variant for a non-zero chip option, so the mode is gated
   on the option probed at setup.

 - Advertise 2500base-x and MAC_2500FD on ports whose external
   interface supports HSGMII.

 - Accept SPEED_2500 in the forced link configuration. The MAC speed
   field has no 2.5 Gbps value: the rate is determined by the HSGMII
   SerDes configuration, and the vendor driver programs the 1 Gbps
   value here, so do the same.

Tested on a Mercusys MR80X v2.20, where the RTL8367S is connected to
the SoC over HSGMII.

Suggested-by: Luiz Angelo Daros de Luca <luizluca@gmail.com>
Signed-off-by: Johan Alvarado <contact@c127.dev>
---
 drivers/net/dsa/realtek/rtl8365mb_main.c | 75 +++++++++++++++++++-----
 1 file changed, 59 insertions(+), 16 deletions(-)

diff --git a/drivers/net/dsa/realtek/rtl8365mb_main.c b/drivers/net/dsa/realtek/rtl8365mb_main.c
index 2d202120cfd9..38e5804f9ff6 100644
--- a/drivers/net/dsa/realtek/rtl8365mb_main.c
+++ b/drivers/net/dsa/realtek/rtl8365mb_main.c
@@ -40,8 +40,8 @@
  * driver has only been tested with a fixed-link, but in principle it should not
  * matter.
  *
- * NOTE: Currently, only the RGMII and SGMII interfaces are implemented in this
- * driver.
+ * NOTE: Currently, only the RGMII, SGMII and HSGMII interfaces are implemented
+ * in this driver.
  *
  * The interrupt line is asserted on link UP/DOWN events. The driver creates a
  * custom irqchip to handle this interrupt and demultiplex the events by reading
@@ -637,6 +637,18 @@ static const struct rtl8365mb_jam_tbl_entry rtl8365mb_sds_jam_sgmii[] = {
 	{ 0x0424, 0xD810 }, { 0x002E, 0x83F2 },
 };
 
+/* HSGMII SerDes tuning parameters, lifted from the vendor driver sources. As
+ * with the SGMII table, the vendor driver keeps several variants and selects
+ * one based on the chip option register; these are the values for a non-zero
+ * option, which is what RTL8367S parts seen so far report. See
+ * rtl8365mb_sds_probe_option().
+ */
+static const struct rtl8365mb_jam_tbl_entry rtl8365mb_sds_jam_hsgmii[] = {
+	{ 0x0500, 0x82F0 }, { 0x0501, 0xF195 }, { 0x0502, 0x31A2 },
+	{ 0x0503, 0x7960 }, { 0x0504, 0x9728 }, { 0x0423, 0x9D85 },
+	{ 0x0424, 0xD810 }, { 0x0001, 0x0F80 }, { 0x002E, 0x83F2 },
+};
+
 enum rtl8365mb_phy_interface_mode {
 	RTL8365MB_PHY_INTERFACE_MODE_INVAL = 0,
 	RTL8365MB_PHY_INTERFACE_MODE_INTERNAL = BIT(0),
@@ -1247,9 +1259,12 @@ static int rtl8365mb_pcs_config(struct phylink_pcs *pcs, unsigned int neg_mode,
 				const unsigned long *advertising,
 				bool permit_pause_to_mac)
 {
+	const struct rtl8365mb_jam_tbl_entry *sds_jam;
 	const int id = RTL8365MB_SDS_EXT_INTERFACE_ID;
 	struct rtl8365mb *mb = pcs_to_rtl8365mb(pcs);
 	struct realtek_priv *priv;
+	size_t sds_jam_size;
+	u32 mode;
 	u16 val;
 	int ret;
 	int i;
@@ -1264,6 +1279,16 @@ static int rtl8365mb_pcs_config(struct phylink_pcs *pcs, unsigned int neg_mode,
 	if (neg_mode == PHYLINK_PCS_NEG_INBAND_ENABLED)
 		return -EOPNOTSUPP;
 
+	if (interface == PHY_INTERFACE_MODE_2500BASEX) {
+		sds_jam = rtl8365mb_sds_jam_hsgmii;
+		sds_jam_size = ARRAY_SIZE(rtl8365mb_sds_jam_hsgmii);
+		mode = RTL8365MB_EXT_PORT_MODE_HSGMII;
+	} else {
+		sds_jam = rtl8365mb_sds_jam_sgmii;
+		sds_jam_size = ARRAY_SIZE(rtl8365mb_sds_jam_sgmii);
+		mode = RTL8365MB_EXT_PORT_MODE_SGMII;
+	}
+
 	/* Hold the embedded DW8051 microcontroller in reset and keep it
 	 * disabled. The vendor driver loads firmware into it to manage the
 	 * SerDes link, but the firmware only duplicates work that phylink
@@ -1291,24 +1316,24 @@ static int rtl8365mb_pcs_config(struct phylink_pcs *pcs, unsigned int neg_mode,
 		return ret;
 
 	/* Tune the SerDes with vendor-prescribed parameters */
-	for (i = 0; i < ARRAY_SIZE(rtl8365mb_sds_jam_sgmii); i++) {
-		ret = rtl8365mb_sds_write(priv,
-					  rtl8365mb_sds_jam_sgmii[i].reg,
-					  rtl8365mb_sds_jam_sgmii[i].val);
+	for (i = 0; i < sds_jam_size; i++) {
+		ret = rtl8365mb_sds_write(priv, sds_jam[i].reg,
+					  sds_jam[i].val);
 		if (ret)
 			return ret;
 	}
 
-	/* Mux the SerDes to MAC8 in SGMII mode */
+	/* Mux the SerDes to MAC8 in the requested mode */
 	ret = regmap_update_bits(priv->map, RTL8365MB_SDS_MISC_REG,
 				 RTL8365MB_SDS_MISC_MAC8_SEL_SGMII_MASK |
 					 RTL8365MB_SDS_MISC_MAC8_SEL_HSGMII_MASK,
-				 RTL8365MB_SDS_MISC_MAC8_SEL_SGMII_MASK);
+				 mode == RTL8365MB_EXT_PORT_MODE_SGMII ?
+					 RTL8365MB_SDS_MISC_MAC8_SEL_SGMII_MASK :
+					 RTL8365MB_SDS_MISC_MAC8_SEL_HSGMII_MASK);
 	if (ret)
 		return ret;
 
-	val = RTL8365MB_EXT_PORT_MODE_SGMII
-	      << RTL8365MB_DIGITAL_INTERFACE_SELECT_MODE_OFFSET(id);
+	val = mode << RTL8365MB_DIGITAL_INTERFACE_SELECT_MODE_OFFSET(id);
 	ret = regmap_update_bits(priv->map,
 				 RTL8365MB_DIGITAL_INTERFACE_SELECT_REG(id),
 				 RTL8365MB_DIGITAL_INTERFACE_SELECT_MODE_MASK(id),
@@ -1354,7 +1379,8 @@ static int rtl8365mb_pcs_config(struct phylink_pcs *pcs, unsigned int neg_mode,
 
 static bool rtl8365mb_interface_is_serdes(phy_interface_t interface)
 {
-	return interface == PHY_INTERFACE_MODE_SGMII;
+	return interface == PHY_INTERFACE_MODE_SGMII ||
+	       interface == PHY_INTERFACE_MODE_2500BASEX;
 }
 
 static unsigned int rtl8365mb_pcs_inband_caps(struct phylink_pcs *pcs,
@@ -1409,7 +1435,9 @@ static void rtl8365mb_pcs_get_state(struct phylink_pcs *pcs,
 
 	switch (FIELD_GET(RTL8365MB_SDS_MISC_SGMII_SPD_MASK, val)) {
 	case RTL8365MB_PORT_SPEED_1000M:
-		state->speed = SPEED_1000;
+		state->speed =
+			state->interface == PHY_INTERFACE_MODE_2500BASEX ?
+				SPEED_2500 : SPEED_1000;
 		break;
 	case RTL8365MB_PORT_SPEED_100M:
 		state->speed = SPEED_100;
@@ -1434,7 +1462,11 @@ static void rtl8365mb_pcs_link_up(struct phylink_pcs *pcs,
 	u32 r_speed;
 	int ret;
 
-	if (speed == SPEED_1000) {
+	/* The speed field has no value for 2.5 Gbps: the rate is determined by
+	 * the HSGMII SerDes configuration, and the vendor driver programs the
+	 * 1 Gbps value here.
+	 */
+	if (speed == SPEED_2500 || speed == SPEED_1000) {
 		r_speed = RTL8365MB_PORT_SPEED_1000M;
 	} else if (speed == SPEED_100) {
 		r_speed = RTL8365MB_PORT_SPEED_100M;
@@ -1494,7 +1526,11 @@ static int rtl8365mb_ext_config_forcemode(struct realtek_priv *priv, int port,
 		r_rx_pause = rx_pause ? 1 : 0;
 		r_tx_pause = tx_pause ? 1 : 0;
 
-		if (speed == SPEED_1000) {
+		/* The speed field has no value for 2.5 Gbps: the rate is
+		 * determined by the HSGMII SerDes configuration, and the
+		 * vendor driver programs the 1 Gbps value here.
+		 */
+		if (speed == SPEED_2500 || speed == SPEED_1000) {
 			r_speed = RTL8365MB_PORT_SPEED_1000M;
 		} else if (speed == SPEED_100) {
 			r_speed = RTL8365MB_PORT_SPEED_100M;
@@ -1577,6 +1613,13 @@ static void rtl8365mb_phylink_get_caps(struct dsa_switch *ds, int port,
 	    mb->sds_supported)
 		__set_bit(PHY_INTERFACE_MODE_SGMII,
 			  config->supported_interfaces);
+
+	if (extint->supported_interfaces & RTL8365MB_PHY_INTERFACE_MODE_HSGMII &&
+	    mb->sds_supported) {
+		__set_bit(PHY_INTERFACE_MODE_2500BASEX,
+			  config->supported_interfaces);
+		config->mac_capabilities |= MAC_2500FD;
+	}
 }
 
 static struct phylink_pcs *
@@ -1618,8 +1661,8 @@ static void rtl8365mb_phylink_mac_config(struct phylink_config *config,
 		return;
 	}
 
-	/* SGMII is handled by the SerDes PCS, configured through the
-	 * phylink_pcs ops, so there is nothing to do here for it.
+	/* SGMII and 2500base-x are handled by the SerDes PCS, configured
+	 * through the phylink_pcs ops, so nothing to do here for them.
 	 */
 	if (rtl8365mb_interface_is_serdes(state->interface))
 		return;
-- 
2.55.0

Re: [PATCH net-next v4 2/2] net: dsa: realtek: rtl8365mb: add HSGMII support for RTL8367S

[Mieczyslaw Nalewaj]

On 7/2/2026 10:47 PM, Johan Alvarado wrote:
> In addition to SGMII, the RTL8367S SerDes also supports HSGMII, which
> carries 2.5 Gbps with the same signaling as SGMII at 2.5x clock rate.
> The chip info table already declares HSGMII as a supported interface
> mode for external interface 1.
> 
> Extend the SerDes PCS to handle HSGMII, which phylink represents as
> 2500base-x:
> 
>  - Select the HSGMII SerDes tuning parameters and external interface
>    mode, and mux the SerDes to MAC8 in HSGMII mode, from pcs_config()
>    according to the interface. The parameters are again lifted from the
>    GPL-licensed Realtek rtl8367c vendor driver, and again only cover
>    the tuning variant for a non-zero chip option, so the mode is gated
>    on the option probed at setup.

[...]

> @@ -1264,6 +1279,16 @@ static int rtl8365mb_pcs_config(struct phylink_pcs *pcs, unsigned int neg_mode,
>  	if (neg_mode == PHYLINK_PCS_NEG_INBAND_ENABLED)
>  		return -EOPNOTSUPP;
>  
> +	if (interface == PHY_INTERFACE_MODE_2500BASEX) {
> +		sds_jam = rtl8365mb_sds_jam_hsgmii;
> +		sds_jam_size = ARRAY_SIZE(rtl8365mb_sds_jam_hsgmii);
> +		mode = RTL8365MB_EXT_PORT_MODE_HSGMII;
> +	} else {
> +		sds_jam = rtl8365mb_sds_jam_sgmii;
> +		sds_jam_size = ARRAY_SIZE(rtl8365mb_sds_jam_sgmii);
> +		mode = RTL8365MB_EXT_PORT_MODE_SGMII;
> +	}
> +

Johan, looks like you forgot to include the scheduler bandwidth bits for the CPU port. Without this, HSGMII will still be capped at the old SGMII rate limits. Something like:

#define RTL8365MB_REG_INGRESSBW_PORT6_RATE_CTRL1	0x00d0
#define  RTL8365MB_INGRESSBW_PORT6_RATE_CTRL1_INGRESSBW_RATE16_MASK GENMASK(2, 0)

#define RTL8365MB_REG_PORT6_EGRESSBW_CTRL1		0x0399
#define  RTL8365MB_PORT6_EGRESSBW_CTRL1_MASK		GENMASK(2, 0)

#define RTL8365MB_REG_LINE_RATE_HSG_H			0x03fa
#define  RTL8365MB_LINE_RATE_HSG_H_MASK			GENMASK(2, 0)

[...]

		/* Allow full 2.5G on HSGMII CPU port: set scheduler
		 * bandwidth limits to max (0x7). Fixed-link init-only;
		 * no runtime SGMII reconfiguration is expected here.
		 */
		ret = regmap_write(priv->map,
				   RTL8365MB_REG_INGRESSBW_PORT6_RATE_CTRL1,
				   FIELD_PREP(RTL8365MB_INGRESSBW_PORT6_RATE_CTRL1_INGRESSBW_RATE16_MASK,
					      7));
		if (ret)
			return ret;

		ret = regmap_write(priv->map,
				   RTL8365MB_REG_PORT6_EGRESSBW_CTRL1,
				   FIELD_PREP(RTL8365MB_PORT6_EGRESSBW_CTRL1_MASK,
					      7));
		if (ret)
			return ret;

		ret = regmap_write(priv->map,
				   RTL8365MB_REG_LINE_RATE_HSG_H,
				   FIELD_PREP(RTL8365MB_LINE_RATE_HSG_H_MASK,
					      7));
		if (ret)
			return ret;

One more thing while we're on this: I checked the equivalent registers for RGMII on the RTL8367S, and they come out to 1, 1, 7 respectively (INGRESSBW_PORT6_RATE_CTRL1, PORT6_EGRESSBW_CTRL1, LINE_RATE_HSG_H). For correctness these should be set to those values in the RGMII path as well, rather than left at whatever reset/default state they're currently in.

Re: [PATCH net-next v4 2/2] net: dsa: realtek: rtl8365mb: add HSGMII support for RTL8367S

[Johan Alvarado]

Hi Mieczyslaw,

On 7/3/2026 5:12 PM, Mieczyslaw Nalewaj wrote:
> Johan, looks like you forgot to include the scheduler bandwidth bits
> for the CPU port. Without this, HSGMII will still be capped at the
> old SGMII rate limits. Something like:

Good catch. I had dropped these on the assumption that the vendor
driver never touches them, but your mail made me look again: the
"Max rate" block in dal_rtl8367c_switch_init() programs exactly these
registers for EXT_PORT0 - the port 6 ingress and egress rate limiters
are raised to 0x7FFFF via rtl8367c_setAsicPortIngressBandwidth() and
rtl8367c_setAsicPortEgressRate(), followed by a raw write of 0x0007
to 0x03fa (LINE_RATE_HSG_H). I had missed it because those writes go
through the indexed register macros and a raw address rather than the
PORT6_* register names.

On my board this was masked by the bootloader: U-Boot runs the vendor
switch init (rtk_switch_init() from the ipq5018 gmac driver) and
leaves the limits raised, but the kernel driver then issues a full
chip reset in rtl8365mb_setup(), which puts these registers back to
their reset defaults. With the ingress/egress limiters back at their
defaults that would indeed cap HSGMII at roughly 1 Gbps, and I could
not have seen it in testing because my downstream ports are 1G only
(as noted in the cover letter, 2.5G line rate is unverified on my
hardware).

One caveat I want to be transparent about: for the same reason I
cannot verify the cap itself, and the vendor documentation leaves
some room for doubt. The rtl8367c_setAsicPortIngressBandwidth()
comment describes the bandwidth value 0x1FFFF as "disable", and
that is exactly the reset default - I read back both limiters as
0x1FFFF (LSBs at 0xFFFF, MSBs at 1) on my RTL8367S after the
driver's chip reset - so the limiter may in fact be disabled out
of reset (though that note looks like stale text from earlier
17-bit-granularity chips, given this family's maximum is 0x7FFFF).

Raising the limiters is safe under either reading: if the default
really is a ~1 Gbps limit, the write removes it, and if it means
"disabled", the new value (0x7FFFF * 8 Kbps, about 4.19 Gbps, well
above the 2.5G line rate) never engages.

While I cannot push more than 1 Gbps, I did verify that these
registers really are the live rate limiters for the SerDes CPU
port, by going the other way: on a build with the setup-time
writes described below already in place, I lowered the limiters at
runtime (via regmap debugfs) to 12500 * 8 Kbps = 100 Mbps
and ran iperf3 across the CPU port. Throughput clamps from ~460 to
~95 Mbps with the ingress limiter lowered (CPU-originated traffic)
and from ~600 to ~96 Mbps with the egress limiter lowered
(CPU-bound traffic); restoring the maximum returns both directions
to baseline. The ~95/100 ratio is the expected TCP goodput for a
100 Mbps wire-rate limit, so both limiters demonstrably meter this
port at the documented 8 Kbps granularity. The only thing this
cannot settle is the meaning of the 0x1FFFF default itself
(~1.048 Gbps cap vs. disabled), since any value at or above the
traffic I can generate behaves identically. If you have an
RTL8367S setup that can push more than 1 Gbps across the HSGMII
CPU port, a before/after measurement would be a very welcome
confirmation of the cap.

So I'll add the writes in v5, following the vendor init: raise the
port 6 ingress/egress limiters to their maximum once at setup time,
rather than per interface mode. Two differences from your snippet:
the vendor init writes the full 19-bit rate value, i.e. the LSB
halves too (0x00cf and 0x0398 set to 0xFFFF), so I'll program both
halves rather than only the *_CTRL1 registers, to avoid depending on
the reset defaults of the LSB registers. And LINE_RATE_HSG_H needs no
new write at all: the driver's common init jam table has always
written 0x03fa = 0x0007 in rtl8365mb_switch_init(), so that one is
already at its maximum on every probe (which also matches the 7 you
read back for it). I have this running on my board already: with the
setup-time writes in place both limiters read back 0x7FFFF after
probe, and the HSGMII link comes up and forwards as before.

As an aside, a per-interface-mode variant of these writes does exist
in the vendor code, but only for other chip families: in
rtl8367c_setAsicPortExtMode(), the chip id 0x0652/0x6368 branch
programs its rate registers (0x0130/0x039f/0x03fa) on every mode
change, with 7s for EXT_HSGMII and lower values otherwise, and the
0x0801/0x6511 branch writes 0x00d0/0x0399/0x03fa = 7 only for
EXT_HSGMII (which is where the pattern in your snippet comes from).
For the 0x6367 family the ext-mode path never touches them, and the
vendor only programs them once, unconditionally, in switch_init -
which is another reason I prefer the one-time setup write over doing
it from the PCS path.

For completeness, the same vendor block also touches three related
bits: rtl8367c_setAsicPortIngressBandwidth() additionally clears the
port 6 ingress metering pre-IFG bit and sets the "flow control when
ingress rate exceeded" bit (PORT6_MISC_CFG, 0x00ce bits 10/11), and
switch_init follows the egress rate write with
rtl8367c_setAsicPortEgressRateIfg(ENABLED), which sets the global
"egress rate accounting includes IFG" bit (SCHEDULE_WFQ_CTRL 0x0300
bit 0). With the limiters raised to maximum the meter never engages,
so neither the exceed action nor the IFG accounting has any
observable effect. I plan to leave those three bits at their reset
defaults and program only the rate values and LINE_RATE_HSG_H, to
keep the driver's footprint on the QoS block minimal - but I'm happy
to mirror the vendor init exactly if you'd rather not deviate from
it.

> One more thing while we're on this: I checked the equivalent
> registers for RGMII on the RTL8367S, and they come out to 1, 1, 7
> respectively (INGRESSBW_PORT6_RATE_CTRL1, PORT6_EGRESSBW_CTRL1,
> LINE_RATE_HSG_H). For correctness these should be set to those
> values in the RGMII path as well, rather than left at whatever
> reset/default state they're currently in.

The 1/1 you read back for the rate limiter MSBs match the reset
defaults I see on my RTL8367S as well, and the 7 for LINE_RATE_HSG_H
is the value the common init jam table writes on every probe, as
above. Since the driver hard-resets the chip and then replays the
init jam on every probe, those values are already guaranteed in
the RGMII case, so writing them again from the RGMII path would be
a no-op. These fields are also the MSBs of the per-port QoS rate
limiters rather than link-mode configuration, so I would prefer to
touch them as little as possible from the link paths. My plan for
v5 is therefore the single write at setup time described above,
which covers both SGMII and HSGMII and has no functional effect on
RGMII either way: both the default value (~1.048 Gbps, if it acts
as a limit at all) and the raised maximum sit above the 1G RGMII
line rate, so the meter never engages there. Let me know if you
see a problem with that.

Best regards,
Johan

Re: [PATCH net-next v4 2/2] net: dsa: realtek: rtl8365mb: add HSGMII support for RTL8367S

[Mieczyslaw Nalewaj]

Hi Johan

On 7/9/2026 9:34 AM, Johan Alvarado wrote:
> Hi Mieczyslaw,
> 
> On 7/3/2026 5:12 PM, Mieczyslaw Nalewaj wrote:
>> Johan, looks like you forgot to include the scheduler bandwidth bits
>> for the CPU port. Without this, HSGMII will still be capped at the
>> old SGMII rate limits. Something like:
> 
> Good catch. I had dropped these on the assumption that the vendor
> driver never touches them, but your mail made me look again: the
> "Max rate" block in dal_rtl8367c_switch_init() programs exactly these
> registers for EXT_PORT0 - the port 6 ingress and egress rate limiters
> are raised to 0x7FFFF via rtl8367c_setAsicPortIngressBandwidth() and
> rtl8367c_setAsicPortEgressRate(), followed by a raw write of 0x0007
> to 0x03fa (LINE_RATE_HSG_H). I had missed it because those writes go
> through the indexed register macros and a raw address rather than the
> PORT6_* register names.
> 
> On my board this was masked by the bootloader: U-Boot runs the vendor
> switch init (rtk_switch_init() from the ipq5018 gmac driver) and
> leaves the limits raised, but the kernel driver then issues a full
> chip reset in rtl8365mb_setup(), which puts these registers back to
> their reset defaults. With the ingress/egress limiters back at their
> defaults that would indeed cap HSGMII at roughly 1 Gbps, and I could
> not have seen it in testing because my downstream ports are 1G only
> (as noted in the cover letter, 2.5G line rate is unverified on my
> hardware).
> 
> One caveat I want to be transparent about: for the same reason I
> cannot verify the cap itself, and the vendor documentation leaves
> some room for doubt. The rtl8367c_setAsicPortIngressBandwidth()
> comment describes the bandwidth value 0x1FFFF as "disable", and
> that is exactly the reset default - I read back both limiters as
> 0x1FFFF (LSBs at 0xFFFF, MSBs at 1) on my RTL8367S after the
> driver's chip reset - so the limiter may in fact be disabled out
> of reset (though that note looks like stale text from earlier
> 17-bit-granularity chips, given this family's maximum is 0x7FFFF).
> 
> Raising the limiters is safe under either reading: if the default
> really is a ~1 Gbps limit, the write removes it, and if it means
> "disabled", the new value (0x7FFFF * 8 Kbps, about 4.19 Gbps, well
> above the 2.5G line rate) never engages.
> 
> While I cannot push more than 1 Gbps, I did verify that these
> registers really are the live rate limiters for the SerDes CPU
> port, by going the other way: on a build with the setup-time
> writes described below already in place, I lowered the limiters at
> runtime (via regmap debugfs) to 12500 * 8 Kbps = 100 Mbps
> and ran iperf3 across the CPU port. Throughput clamps from ~460 to
> ~95 Mbps with the ingress limiter lowered (CPU-originated traffic)
> and from ~600 to ~96 Mbps with the egress limiter lowered
> (CPU-bound traffic); restoring the maximum returns both directions
> to baseline. The ~95/100 ratio is the expected TCP goodput for a
> 100 Mbps wire-rate limit, so both limiters demonstrably meter this
> port at the documented 8 Kbps granularity. The only thing this
> cannot settle is the meaning of the 0x1FFFF default itself
> (~1.048 Gbps cap vs. disabled), since any value at or above the
> traffic I can generate behaves identically. If you have an
> RTL8367S setup that can push more than 1 Gbps across the HSGMII
> CPU port, a before/after measurement would be a very welcome
> confirmation of the cap.
> 
> So I'll add the writes in v5, following the vendor init: raise the
> port 6 ingress/egress limiters to their maximum once at setup time,
> rather than per interface mode. Two differences from your snippet:
> the vendor init writes the full 19-bit rate value, i.e. the LSB
> halves too (0x00cf and 0x0398 set to 0xFFFF), so I'll program both
> halves rather than only the *_CTRL1 registers, to avoid depending on
> the reset defaults of the LSB registers. And LINE_RATE_HSG_H needs no
> new write at all: the driver's common init jam table has always
> written 0x03fa = 0x0007 in rtl8365mb_switch_init(), so that one is
> already at its maximum on every probe (which also matches the 7 you
> read back for it). I have this running on my board already: with the
> setup-time writes in place both limiters read back 0x7FFFF after
> probe, and the HSGMII link comes up and forwards as before.
> 
> As an aside, a per-interface-mode variant of these writes does exist
> in the vendor code, but only for other chip families: in
> rtl8367c_setAsicPortExtMode(), the chip id 0x0652/0x6368 branch
> programs its rate registers (0x0130/0x039f/0x03fa) on every mode
> change, with 7s for EXT_HSGMII and lower values otherwise, and the
> 0x0801/0x6511 branch writes 0x00d0/0x0399/0x03fa = 7 only for
> EXT_HSGMII (which is where the pattern in your snippet comes from).
> For the 0x6367 family the ext-mode path never touches them, and the
> vendor only programs them once, unconditionally, in switch_init -
> which is another reason I prefer the one-time setup write over doing
> it from the PCS path.
> 
> For completeness, the same vendor block also touches three related
> bits: rtl8367c_setAsicPortIngressBandwidth() additionally clears the
> port 6 ingress metering pre-IFG bit and sets the "flow control when
> ingress rate exceeded" bit (PORT6_MISC_CFG, 0x00ce bits 10/11), and
> switch_init follows the egress rate write with
> rtl8367c_setAsicPortEgressRateIfg(ENABLED), which sets the global
> "egress rate accounting includes IFG" bit (SCHEDULE_WFQ_CTRL 0x0300
> bit 0). With the limiters raised to maximum the meter never engages,
> so neither the exceed action nor the IFG accounting has any
> observable effect. I plan to leave those three bits at their reset
> defaults and program only the rate values and LINE_RATE_HSG_H, to
> keep the driver's footprint on the QoS block minimal - but I'm happy
> to mirror the vendor init exactly if you'd rather not deviate from
> it.
> 
>> One more thing while we're on this: I checked the equivalent
>> registers for RGMII on the RTL8367S, and they come out to 1, 1, 7
>> respectively (INGRESSBW_PORT6_RATE_CTRL1, PORT6_EGRESSBW_CTRL1,
>> LINE_RATE_HSG_H). For correctness these should be set to those
>> values in the RGMII path as well, rather than left at whatever
>> reset/default state they're currently in.
> 
> The 1/1 you read back for the rate limiter MSBs match the reset
> defaults I see on my RTL8367S as well, and the 7 for LINE_RATE_HSG_H
> is the value the common init jam table writes on every probe, as
> above. Since the driver hard-resets the chip and then replays the
> init jam on every probe, those values are already guaranteed in
> the RGMII case, so writing them again from the RGMII path would be
> a no-op. These fields are also the MSBs of the per-port QoS rate
> limiters rather than link-mode configuration, so I would prefer to
> touch them as little as possible from the link paths. My plan for
> v5 is therefore the single write at setup time described above,
> which covers both SGMII and HSGMII and has no functional effect on
> RGMII either way: both the default value (~1.048 Gbps, if it acts
> as a limit at all) and the raised maximum sit above the 1G RGMII
> line rate, so the meter never engages there. Let me know if you
> see a problem with that.
> 
> Best regards,
> Johan

Performance degradation without setting these registers can only be
observed in HSGMII mode with simultaneous load on multiple LAN ports.

This was noticed during work on adding support for the Mercusys MR85X
router with DSA to OpenWrt: https://github.com/openwrt/openwrt/pull/19445#issuecomment-4505613294

Regards

Re: [PATCH net-next v4 2/2] net: dsa: realtek: rtl8365mb: add HSGMII support for RTL8367S

[Johan Alvarado]

On 7/9/2026 5:02 PM, Mieczyslaw Nalewaj wrote:
> Performance degradation without setting these registers can only be
> observed in HSGMII mode with simultaneous load on multiple LAN ports.
>
> This was noticed during work on adding support for the Mercusys MR85X
> router with DSA to OpenWrt: https://github.com/openwrt/openwrt/pull/19445#issuecomment-4505613294

Thanks, that is exactly the measurement I was hoping for. The ~1020
Mbit/s combined ceiling reported there with multiple clients sits
right where the 0x1FFFF reset default (~1.048 Gbps of wire rate)
would put it, and the jump to ~2 Gbit/s once the limiters are raised
settles the open question: the reset default is a real cap, and the
"disable" wording in the vendor comment does not match the silicon.
It also confirms why neither of us could see it with a single client:
one 1G port never reaches the limit.

I have updated v5 accordingly: the code comment and commit message
now state the cap as observed instead of hedging between the two
readings, and patch 2 carries a Link to that GitHub comment as the
reference for the measurement. I will send the series shortly.

Best regards,
Johan