On Wed, Aug 21, 2019 at 04:43:34PM +0200, René van Dorst wrote: > +static void mtk_mac_link_down(struct phylink_config *config, unsigned int > mode, > + phy_interface_t interface) > +{ > + struct mtk_mac *mac = container_of(config, struct mtk_mac, > + phylink_config); > > - return 0; > + mtk_w32(mac->hw, MAC_MCR_FORCE_LINK_DOWN, MTK_MAC_MCR(mac->id)); > }
You set the MAC_MCR_FORCE_MODE bit here... > +static void mtk_mac_link_up(struct phylink_config *config, unsigned int mode, > + phy_interface_t interface, > + struct phy_device *phy) > { > + struct mtk_mac *mac = container_of(config, struct mtk_mac, > + phylink_config); > + u32 mcr = mtk_r32(mac->hw, MTK_MAC_MCR(mac->id)); > > + mcr |= MAC_MCR_TX_EN | MAC_MCR_RX_EN; > + mtk_w32(mac->hw, mcr, MTK_MAC_MCR(mac->id)); > +} Looking at this, a link_down() followed by a link_up() would result in this register containing MAC_MCR_FORCE_MODE | MAC_MCR_TX_EN | MAC_MCR_RX_EN ? Is that actually correct? (MAC_MCR_FORCE_LINK isn't set, so it looks to me like it still forces the link down.) Note that link up/down forcing should not be done for in-band AN. > +static void mtk_validate(struct phylink_config *config, > + unsigned long *supported, > + struct phylink_link_state *state) > +{ > + struct mtk_mac *mac = container_of(config, struct mtk_mac, > + phylink_config); > + __ETHTOOL_DECLARE_LINK_MODE_MASK(mask) = { 0, }; > > + if (state->interface != PHY_INTERFACE_MODE_NA && > + state->interface != PHY_INTERFACE_MODE_MII && > + state->interface != PHY_INTERFACE_MODE_GMII && > + !(MTK_HAS_CAPS(mac->hw->soc->caps, MTK_RGMII) && > + phy_interface_mode_is_rgmii(state->interface)) && > + !(MTK_HAS_CAPS(mac->hw->soc->caps, MTK_TRGMII) && > + !mac->id && state->interface == PHY_INTERFACE_MODE_TRGMII)) { > + linkmode_zero(supported); > + return; > } > > + phylink_set_port_modes(mask); > + phylink_set(mask, Autoneg); > > + if (state->interface == PHY_INTERFACE_MODE_TRGMII) { > + phylink_set(mask, 1000baseT_Full); > + } else { > + phylink_set(mask, 10baseT_Half); > + phylink_set(mask, 10baseT_Full); > + phylink_set(mask, 100baseT_Half); > + phylink_set(mask, 100baseT_Full); > + > + if (state->interface != PHY_INTERFACE_MODE_MII) { > + phylink_set(mask, 1000baseT_Half); > + phylink_set(mask, 1000baseT_Full); > + phylink_set(mask, 1000baseX_Full); > + } > + } > > + phylink_set(mask, Pause); > + phylink_set(mask, Asym_Pause); > > + linkmode_and(supported, supported, mask); > + linkmode_and(state->advertising, state->advertising, mask); > } This looks fine. Thanks. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up According to speedtest.net: 11.9Mbps down 500kbps up