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?
> +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;
We can then solve RGMII problems when somebody actually needs RGMII.
Andrew
On Tue, Apr 6, 2021 at 11:21 PM Andrew Lunn <[email protected]> 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