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