Re: [PATCH net-next v6 2/2] net: dsa: realtek: rtl8365mb: add HSGMII 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>
> 
> 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.
> 
>  - Raise the port 6 ingress and egress rate limiters to their maximum
>    at setup time, as the vendor switch init does unconditionally for
>    the whole chip family. The chip resets them to 0x1FFFF (~1.048 Gbps
>    in units of 8 Kbps), which caps the aggregate HSGMII throughput at
>    roughly 1 Gbps. The vendor documentation describes the reset
>    default as disabling the limiter, but the cap is real: on an
>    RTL8367S-based Mercusys MR85X running an OpenWrt backport of this
>    series, several clients on 1 Gbps user ports were limited to about
>    1.02 Gbps combined across the HSGMII CPU port until these limiters
>    were raised, after which throughput reached about 2 Gbps [1]. The
>    related HSGMII scheduler line rate (LINE_RATE_HSG_H) is already set
>    to its maximum by the common init jam table.
> 
> Tested on a Mercusys MR80X v2.20, where the RTL8367S is connected to
> the SoC over HSGMII.
> 
> Link: https://github.com/openwrt/openwrt/pull/19445#issuecomment-4505613294 [1]
> Suggested-by: Luiz Angelo Daros de Luca <luizluca@gmail.com>
> Suggested-by: Mieczyslaw Nalewaj <namiltd@yahoo.com>
> Signed-off-by: Johan Alvarado <contact@c127.dev>

Thanks for the patch, this matches what I've seen too on similar
hardware.

As discussed earlier in the thread: the rate limiter helper, as well
as pcs_config()/RTL8365MB_SDS_BYPASS_LINE_RATE_MASK, currently assume
the SerDes is always muxed to external interface 1 / port 6
(RTL8365MB_SDS_EXT_INTERFACE_ID / _PORT). That's true for every chip
currently in rtl8365mb_chip_infos[] (RTL8367S and RTL8367SB both have
their SGMII/HSGMII-capable extint at { 6, 1, ... }), so nothing is
broken today.

I still think it's worth guarding against this ahead of time though.
It isn't an architectural constant of the family, just something that
happens to hold for the two chips currently in the table, and there's
no guarantee a future chip won't mux its SerDes to a different port
or extint id. If that happens, pcs_config() would silently program
the wrong DIGITAL_INTERFACE_SELECT/FORCE register, and the rate
limiter helper would write to the wrong port's registers (or need to
guess an address it can't verify) - the kind of thing that's easy to
miss in review since it only breaks on hardware nobody in this thread
has to test with.

Since rtl8365mb_sds_probe_option() already walks
chip_info->extints[] looking for the SGMII/HSGMII-capable one, I'd
like to have it cache the discovered port and id (mb->sds_port /
mb->sds_id) instead of relying on the fixed
RTL8365MB_SDS_EXT_INTERFACE_ID/_PORT. pcs_config() and the bypass
line-rate mask would then use the cached values, and
rtl8365mb_sds_raise_rate_limits() would check mb->sds_port == 6 and
warn+skip instead of assuming, since only the port 6 register
addresses are known/verified so far.

Draft below, as a preparatory patch to go before this one. No
functional change for RTL8367S/RTL8367SB - just removes the
assumption for whatever comes next. Let me know what you think.

---
  #define RTL8365MB_BYPASS_LINE_RATE_REG		0x03F7
  #define RTL8365MB_BYPASS_LINE_RATE_MASK(_port)	BIT((_port) - 5)
  
 +/* Port 6 ingress and egress rate limiter registers. Each limit is a 19-bit
 + * value in units of 8 Kbps, split across a 16-bit LSB register (CTRL0) and a
 + * 3-bit MSB field (CTRL1). The chip resets them to 0x1FFFF; see
-+ * rtl8365mb_sds_raise_rate_limits().
++ * rtl8365mb_sds_raise_rate_limits(). Only port 6 addresses are known - every
++ * chip currently supported muxes the SerDes to port 6 (see mb->sds_port),
++ * but this is not guaranteed for chips that may be added later.
 + */
 +#define RTL8365MB_INGRESSBW_PORT6_RATE_CTRL0_REG	0x00CF
 +#define RTL8365MB_INGRESSBW_PORT6_RATE_CTRL1_REG	0x00D0
@@ -30,7 +32,29 @@
  /* SerDes indirect access registers */
  #define RTL8365MB_SDS_INDACS_CMD_REG		0x6600
  #define   RTL8365MB_SDS_INDACS_CMD_BUSY_MASK	0x0100
-@@ -637,6 +649,18 @@ static const struct rtl8365mb_jam_tbl_entry rtl8365mb_sds_jam_sgmii[] = {
+@@ -270,12 +284,11 @@
+ #define   RTL8365MB_SDS_MISC_SGMII_SPD_MASK		0x0180
+ #define   RTL8365MB_SDS_MISC_MAC8_SEL_SGMII_MASK	0x0040
+ 
+-/* The embedded SerDes can only be muxed to external interface 1 (MAC8),
+- * which is port 6.
++/* The embedded SerDes is muxed to whichever external interface the chip's
++ * extint table marks as SGMII/HSGMII-capable. For every chip currently
++ * supported this is external interface 1 (MAC8) on port 6, but this is
++ * discovered per-chip in rtl8365mb_sds_probe_option() and cached in
++ * mb->sds_port / mb->sds_id rather than assumed, since it need not hold for
++ * chips added in the future.
+  */
+-#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
+@@ -637,6 +650,18 @@ static const struct rtl8365mb_jam_tbl_entry rtl8365mb_sds_jam_sgmii[] = {
  	{ 0x0424, 0xD810 }, { 0x002E, 0x83F2 },
  };
  
@@ -49,25 +73,61 @@
  enum rtl8365mb_phy_interface_mode {
  	RTL8365MB_PHY_INTERFACE_MODE_INVAL = 0,
  	RTL8365MB_PHY_INTERFACE_MODE_INTERNAL = BIT(0),
-@@ -1242,20 +1266,70 @@ static int rtl8365mb_sds_probe_option(struct realtek_priv *priv)
+@@ -871,6 +896,11 @@ struct rtl8365mb {
+ 	struct rtl8365mb_port ports[RTL8365MB_MAX_NUM_PORTS];
+ 	struct phylink_pcs pcs;
+ 	bool sds_supported;
++	/* Port and extint id the SerDes is muxed to on this chip, discovered
++	 * in rtl8365mb_sds_probe_option() from the chip's extint table.
++	 * Valid only if sds_supported is true.
++	 */
++	int sds_port;
++	int sds_id;
+ };
+ 
+ #define pcs_to_rtl8365mb(_pcs) container_of((_pcs), struct rtl8365mb, pcs)
+@@ -1229,6 +1259,9 @@ static int rtl8365mb_sds_probe_option(struct realtek_priv *priv)
+ 	if (i == RTL8365MB_MAX_NUM_EXTINTS)
+ 		return 0;
+ 
++	mb->sds_port = extint->port;
++	mb->sds_id = extint->id;
++
+ 	ret = regmap_write(priv->map, RTL8365MB_SDS_OPTION_ARM_REG,
+ 			   RTL8365MB_SDS_OPTION_ARM_KEY);
+ 	if (ret)
+@@ -1242,20 +1275,74 @@ static int rtl8365mb_sds_probe_option(struct realtek_priv *priv)
  	return 0;
  }
  
-+/* The vendor driver raises the port 6 ingress and egress rate limiters to
-+ * their maximum in its switch init, unconditionally for the whole chip
-+ * family. The chip reset in rtl8365mb_setup() puts them back to their reset
-+ * default of 0x1FFFF, a ~1.048 Gbps limit which caps the aggregate
-+ * throughput of an HSGMII CPU port at roughly 1 Gbps. The vendor
++/* The vendor driver raises the ingress and egress rate limiters of the
++ * SerDes-connected port to their maximum in its switch init, unconditionally
++ * for the whole chip family. The chip reset in rtl8365mb_setup() puts them
++ * back to their reset default of 0x1FFFF, a ~1.048 Gbps limit which caps the
++ * aggregate throughput of an HSGMII CPU port at roughly 1 Gbps. The vendor
 + * documentation describes the reset default as disabling the limiter, but
 + * the cap has been observed on hardware. Raise them likewise, to 0x7FFFF
 + * (~4.19 Gbps, above the HSGMII line rate). The related HSGMII scheduler
 + * line rate register (LINE_RATE_HSG_H, 0x03FA) is already set to its
 + * maximum by the common init jam table.
++ *
++ * Only the port 6 register addresses are known. Every chip currently
++ * supported muxes the SerDes to port 6 (see mb->sds_port, discovered in
++ * rtl8365mb_sds_probe_option()); if a future chip ends up muxing it
++ * elsewhere, skip rather than guess at an address.
 + */
 +static int rtl8365mb_sds_raise_rate_limits(struct realtek_priv *priv)
 +{
++	struct rtl8365mb *mb = priv->chip_data;
 +	int ret;
 +
++	if (mb->sds_port != 6) {
++		dev_warn(priv->dev,
++			 "SerDes on port %d has no known rate limiter registers, skipping\n",
++			 mb->sds_port);
++		return 0;
++	}
++
 +	ret = regmap_write(priv->map, RTL8365MB_INGRESSBW_PORT6_RATE_CTRL0_REG,
 +			   0xFFFF);
 +	if (ret)
@@ -96,17 +156,18 @@
  				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;
++	int id;
 +	u32 mode;
  	u16 val;
  	int ret;
  	int i;
  
  	priv = mb->priv;
- 
++	id = mb->sds_id;
++
 +	if (interface == PHY_INTERFACE_MODE_2500BASEX) {
 +		sds_jam = rtl8365mb_sds_jam_hsgmii;
 +		sds_jam_size = ARRAY_SIZE(rtl8365mb_sds_jam_hsgmii);
@@ -116,8 +177,15 @@
  
  	/* 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
-@@ -1283,24 +1357,24 @@ static int rtl8365mb_pcs_config(struct phylink_pcs *pcs, unsigned int neg_mode,
+@@ -1272,17 +1359,20 @@ static int rtl8365mb_pcs_config(struct phylink_pcs *pcs, unsigned int neg_mode,
+ 	/* 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);
++				 RTL8365MB_BYPASS_LINE_RATE_MASK(mb->sds_port),
++				 0);
+ 	if (ret)
  		return ret;
  
  	/* Tune the SerDes with vendor-prescribed parameters */