Re: [PATCH net v3] net: stmmac: enable the MAC on link up for all supported speeds

[Maxime Chevallier]

Hi,

On 7/13/26 09:49, vadik likholetov wrote:
> stmmac_mac_link_down() clears the MAC's transmit and receive enable bits.
> stmmac_mac_link_up() is expected to set them again through
> stmmac_mac_set(..., true), but it first switches on the negotiated speed
> and returns early for a speed the switch does not list. The MAC is then
> left gated off.
> 
> The speed selection is split into three switches, keyed on the interface.
> The generic branch -- taken for everything that is neither USXGMII nor
> XLGMII, so including PHY_INTERFACE_MODE_10GBASER -- lists only SPEED_2500,
> SPEED_1000, SPEED_100 and SPEED_10.
> 
> MGBE on Tegra234 runs 10GBASE-R into an Aquantia AQR113C. That PHY does
> rate matching, so phylink_link_up() replaces the media speed with the
> MAC-side interface speed before calling into the MAC:
> 
> 	case RATE_MATCH_PAUSE:
> 		speed = phylink_interface_max_speed(link_state.interface);
> 		duplex = DUPLEX_FULL;
> 
> The driver is therefore called as
> 
> 	stmmac_mac_link_up(interface=10GBASER, speed=10000, duplex=1)
> 
> which falls through to "default: return;". The interface stops passing
> traffic after the first link flap.
> 
> The failure is easy to misread. The link still comes up, because the PHY
> is polled over MDIO and needs no MAC, so the interface reports carrier 1
> at the media speed. The DMA is untouched, so its start bits stay set and
> descriptors are still consumed. Only the MAC itself is gated off: the
> receiver counts nothing (mmc_rx_framecount_gb stops advancing, RE is 0)
> and nothing reaches the wire (TE is 0). The interface survives boot only
> because stmmac_hw_setup(), called from ndo_open, enables the MAC
> unconditionally -- so the problem appears only once the cable has been
> unplugged and plugged back in, and "ip link set dev <ethX> down && ip
> link set dev <ethX> up" appears to fix it.
> 
> The interface is not what the speed bits depend on: with the single
> exception of 2.5G, which is selected through the XGMII block on USXGMII
> and through the regular speed bits otherwise, each speed maps to one
> field of struct mac_link. The per-interface switches are speed
> validation, and phylink already validates the speed against
> priv->hw->link.caps. So collapse the three switches into one keyed on the
> speed alone, keeping the interface test only for the 2.5G case. This
> covers 10G on 10GBASE-R, and equally 5G, and 1G/100/10 on USXGMII, all of
> which hit "default: return;" today.
> 
> A core that does not support a speed leaves the corresponding mac_link
> field at 0, and phylink will not offer it that speed in the first place.
> For dwxgmac2 at 10G, link.xgmii.speed10000 is XGMAC_CONFIG_SS_10000,
> which is 0 and is the correct speed selection for a 10GBASE-R MAC: ctrl
> then equals old_ctrl, the register write is skipped, and execution
> reaches stmmac_mac_set(..., true).
> 
> Log an error in the default case, since a speed with no entry here leaves
> the MAC disabled and the symptom does not point at the cause.
> 
> Fixes: d8ca113724e7 ("net: stmmac: tegra: Add MGBE support")
> Suggested-by: Maxime Chevallier <maxime.chevallier@xxxxxxxxxxx>
> Signed-off-by: vadik likholetov <vadikas@xxxxxxxxx>
> ---

Reviewed-by: Maxime Chevallier <maxime.chevallier@xxxxxxxxxxx>

Thanks,

Maxime