Re: [PATCH net-next v4 2/2] net: dsa: realtek: rtl8365mb: add HSGMII support for RTL8367S
From: Johan Alvarado <hidden>
Date: 2026-07-09 07:34:06
Also in:
lkml
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