On Tue, Apr 6, 2021 at 11:21 PM Andrew Lunn <andrew@xxxxxxx> wrote: > > On Tue, Apr 06, 2021 at 10:18:16PM +0800, DENG Qingfang wrote: > > Add support for MediaTek PHYs found in MT7530 and MT7531 switches. > > Do you know if this PHY is available standalone? Not that I know of. > > > +static int mt7531_phy_config_init(struct phy_device *phydev) > > +{ > > + mtk_phy_config_init(phydev); > > + > > + /* PHY link down power saving enable */ > > + phy_set_bits(phydev, 0x17, BIT(4)); > > + phy_clear_bits_mmd(phydev, MDIO_MMD_VEND1, 0xc6, 0x300); > > + > > + /* Set TX Pair delay selection */ > > + phy_write_mmd(phydev, MDIO_MMD_VEND1, 0x13, 0x404); > > + phy_write_mmd(phydev, MDIO_MMD_VEND1, 0x14, 0x404); > > This gets me worried about RGMII delays. We have had bad backwards > compatibility problems with PHY drivers which get RGMII delays wrong. > > Since this is an internal PHY, i suggest you add a test to the > beginning of mt7531_phy_config_init(): > > if (phydev->interface != PHY_INTERFACE_MODE_INTERNAL) > return -EINVAL; Okay. Will add it to v2. > > We can then solve RGMII problems when somebody actually needs RGMII. > > Andrew