Thread (5 messages) 5 messages, 2 authors, 9d ago

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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help