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

[Maxime Chevallier <maxime.chevallier@bootlin.com>]

Hi,

On 7/8/26 03:09, Johan Alvarado wrote:

> 
> [...]
> 
>>> +		/* The SerDes has its own pause controls; program them from
>>> +		 * the resolved pause modes, as the vendor driver does when
>>> +		 * forcing the link on a SerDes external interface. This is
>>> +		 * done here rather than in rtl8365mb_pcs_link_up() because
>>> +		 * pcs_link_up() carries no pause information.
>>> +		 */
>>> +		if (rtl8365mb_interface_is_serdes(interface)) {
>>> +			u32 val = 0;
>>> +
>>> +			if (tx_pause)
>>> +				val |= RTL8365MB_SDS_MISC_SGMII_TXFC_MASK;
>>> +			if (rx_pause)
>>> +				val |= RTL8365MB_SDS_MISC_SGMII_RXFC_MASK;
>>
>> Do you know what this does in HW ? Is this so that the PCS lets the Pause frames through
>> in either directions ?
>>
>> I suspect this is something that would be only used for inband advertising
>> of pause settings (in such case, you don't even need that), but ofc I'm not sure :)
>>
>> You already configure the MAC pause settings, can you test that these bits actually do
>> anything by exercising a bit flow control and checking if these registers are used ?
> 
> I tested it on the hardware, and it turns out these bits are the
> operative pause controls for the SerDes port: pause frames are only
> ever emitted when SDS_MISC_SGMII_TXFC is set, and the MAC force-mode
> pause bits alone have no observable effect on this port. They are not
> tied to in-band advertisement either - they act with the link fully
> forced.
> 
> For context, there is no public documentation for these bits, but in
> the vendor GPL code every write to them lives in
> rtl8367c_setAsicPortForceLinkExt(), the forced-link path, where they
> are programmed from the resolved tx/rx pause settings right alongside
> the MAC force-mode pause bits - never from the nway/in-band paths.
> 
> Test setup: RTL8367S (Mercusys MR80X), CPU port 6 on HSGMII,
> 2500base-x fixed link to an IPQ5018. I enabled regmap debugfs writes
> so I could flip the bits live, and forced congestion by L2-forwarding
> ~350 Mbit/s of SoC-generated UDP in through the CPU port and out of a
> 100M user port (~3.5x oversubscription, ~135 MB per run), watching the
> CPU port's dot3OutPauseFrames MIB counter:
> 
>   everything cleared (baseline):                 0 pause frames
>   MAC force pause only (DIGITAL_INTERFACE_FORCE
>       TXPAUSE|RXPAUSE, reg 0x1311):              0 pause frames
>   MAC force pause + SDS TXFC|RXFC:            2396 pause frames
>   SDS TXFC|RXFC only (MAC pause cleared):     2362 pause frames
>   SDS TXFC only:                              2934 pause frames
>   SDS RXFC only:                                 0 pause frames
> 
> RXFC did not influence pause emission, so by symmetry it presumably
> gates honouring received pause frames; I could not exercise that
> direction because the SoC MAC on this board never transmits pause.
> 
> So the write has to stay - without it flow control simply does not
> work on SGMII/HSGMII. I will expand the comment in v5 to say that
> these are the functional pause enables for the SerDes, rather than
> describing it as mirroring the vendor forced-link sequence.
> 

Thanks a lot for the investigation ! Ok so you do need to configure that
in the PCS. Maybe at some point we'll have to pass tx/rx negotiated pause
settings to the PCS, just like we to for the MAC, but it's strange as the
PCS isn't supposed to be the block handling pause :/

I'm ok leaving this as-is right now, you've added a nice comment on why
PCS regs are accessed in the MAC's link_up(). If we find other devices
that behave the same w.r.t. pause, we may have to pass these to the pcs_link_up,
but it's rather unusual I think.

Thanks :)

Maxime