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

[Mieczyslaw Nalewaj <namiltd@yahoo.com>]

Hi Johan

On 7/12/2026 6:31 AM, Johan Alvarado via B4 Relay wrote:
> From: Johan Alvarado <contact@c127.dev>
> 
> 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 enables 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. Flow control testing shows these bits,
>    not the MAC force pause bits, gate pause on the SerDes external
>    interface. 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 | 515 ++++++++++++++++++++++++++++++-
>  1 file changed, 511 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/dsa/realtek/rtl8365mb_main.c b/drivers/net/dsa/realtek/rtl8365mb_main.c
> index 5ac091bf93c9..ea03c42d0f1a 100644
> --- a/drivers/net/dsa/realtek/rtl8365mb_main.c
> +++ b/drivers/net/dsa/realtek/rtl8365mb_main.c

[...]

> +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;
> +
> +	/* 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);
> +}

For verification on real hardware:

The SGMII/HSGMII link on RTL8367S (and compatible RTL8365MB-VC) can be
intermittently unstable after cold boot, manifesting as egress stalls,
CRC errors or complete packet loss on the SerDes-attached CPU port.
This has been observed as a non-deterministic failure depending on the
PLL lock state at power-on.

Reverse-engineering of the vendor DW8051 firmware blob (Sgmii_Init[])
shows that the vendor driver performs two critical steps after the
SerDes data-path reset (BMCR DPRST_PHASE2) which were missing from the
Linux driver:

1. A ~98 ms delay to let the SerDes PLL fully lock before any further
   register access. The vendor firmware uses a timer interrupt to count
   this delay; without it the analog front-end may still be settling.

2. Writing a "Local Jam Table" calibration vector to internal ASIC
   registers 0x060C-0x060F (values 0x83, 0xAA, 0x7E, 0x80). These
   registers configure the SerDes analog equalizer and DC-offset and
   are not exposed through the normal SDS_INDACS window. Omitting them
   leaves the analog front-end in an uncalibrated state.

Add both steps to rtl8365mb_pcs_config(), immediately after the BMCR
phase-2 data-path reset and before the NWAY configuration. This mirrors
the exact vendor firmware bring-up sequence and eliminates the cold-boot
race.

The 98 ms delay matches the vendor firmware timeout constant
(SGMII_TIMEOUT_98MS). It is a one-time cost during interface bring-up
and ensures reliable link establishment regardless of the PLL state at
reset.

--- a/drivers/net/dsa/realtek/rtl8365mb_main.c
+++ b/drivers/net/dsa/realtek/rtl8365mb_main.c
@@ -95,6 +95,7 @@
 #include <linux/if_bridge.h>
 #include <linux/if_vlan.h>
 #include <linux/phylink.h>
+#include <linux/delay.h>
 
 #include "realtek.h"
 #include "realtek-smi.h"
@@ -1235,6 +1236,21 @@ static int rtl8365mb_pcs_config(struct phylink_pcs *pcs, unsigned int neg_mode,
 	if (ret)
 		return ret;
 
+	/* The vendor firmware waits ~98 ms here before writing the
+	 * calibration vector.
+	 */
+	usleep_range(98000, 98500);
+
+	/* Vendor firmware writes a "Local Jam Table" to internal ASIC
+	 * registers 0x060C-0x060F after the data path reset to calibrate
+	 * the SerDes analog front-end. Without this, the link can be
+	 * unstable after cold boot.
+	 */
+	regmap_write(priv->map, 0x060C, 0x0083);
+	regmap_write(priv->map, 0x060D, 0x00AA);
+	regmap_write(priv->map, 0x060E, 0x007E);
+	regmap_write(priv->map, 0x060F, 0x0080);
+
 	/* Keep SGMII in-band autonegotiation disabled: the link parameters are
 	 * forced from rtl8365mb_pcs_link_up() instead.
 	 */