Andrew On 11/19/20 7:49 PM, Andrew Lunn wrote:
+static int dp83td510_config_init(struct phy_device *phydev) +{ + struct dp83td510_private *dp83td510 = phydev->priv; + int ret = 0; + + if (phy_interface_is_rgmii(phydev)) { + if (dp83td510->rgmii_delay) { + ret = phy_set_bits_mmd(phydev, DP83TD510_DEVADDR, + DP83TD510_MAC_CFG_1, + dp83td510->rgmii_delay);Just to be safe, you should always write rgmii_delay, even if it is zero. We have had too many bugs with RGMII delays which cause bad backwards compatibility problems, so i would prefer to do a write which might be unneeded, that find a bug here in a few years time.
OK.
+ if (ret) + return ret; + } + } + + if (phydev->interface == PHY_INTERFACE_MODE_RMII) { + ret = phy_modify(phydev, DP83TD510_GEN_CFG, + DP83TD510_FIFO_DEPTH_MASK, + dp83td510->tx_fifo_depth);So there is no need to set the FIFO depth for the other three RGMII modes? Or should this also be phy_interface_is_rgmii(phydev)?
According to the data sheet the FIFO depth is for RMII. "Fifo depth for RMII Tx fifo" But I will ask the HW team for clarification.
+#if IS_ENABLED(CONFIG_OF_MDIO) +static int dp83td510_of_init(struct phy_device *phydev) +{ + struct dp83td510_private *dp83td510 = phydev->priv; + struct device *dev = &phydev->mdio.dev; + struct device_node *of_node = dev->of_node;You need to move this assignment to later in order to keep with reverse christmas tree.
Well this is only used once so I will just remove the of_node declaration
+#else +static int dp83869_of_init(struct phy_device *phydev) +{ + dp83td510->hi_diff_output = DP83TD510_2_4V_P2P + dp83td510->tx_fifo_depth = DP83TD510_FIFO_DEPTH_5_B_NIBYou don't have DT, so there is no fine control, but you still need to do the basic 2ns delay as indicated by the phydev->interface value. So i think you still need to set dp83td510->rgmii_delay depending on which RGMII mode is requested.
The RGMII delay is fixed in the PHY. The user can either turn it on or off. The default is 'off' which is 0.
I can explicitly set the rgmii_delay to 0 in non-OF cases. Dan
