Hi Tony, On 1/19/22 02:31, Tony Dinh wrote:
<snip>
+#if defined(CONFIG_RESET_PHY_R) +/* Configure and initialize PHY */ +void reset_phy(void) +{ + u16 reg; + int phyaddr; + char *name = "ethernet-controller@72000"; + char *eth0_path = "/ocp@f1000000/ethernet-controller@72000"; + + if (miiphy_set_current_dev(name)) + return; + + phyaddr = fdt_get_phy_addr(eth0_path); + if (phyaddr < 0) + return; + + /* + * Enable RGMII delay on Tx and Rx for CPU port + * Ref: sec 4.7.2 of chip datasheet + */ + miiphy_write(name, phyaddr, MV88E1116_PGADR_REG, 2); + miiphy_read(name, phyaddr, MV88E1116_MAC_CTRL_REG, ®); + reg |= (MV88E1116_RGMII_RXTM_CTRL | MV88E1116_RGMII_TXTM_CTRL); + miiphy_write(name, phyaddr, MV88E1116_MAC_CTRL_REG, reg); + miiphy_write(name, phyaddr, MV88E1116_PGADR_REG, 0);Please take a look at drivers/net/phy/marvell.c / m88e1310_config(). Here you will find (amongst others): /* Set RGMII delay */ phy_write(phydev, MDIO_DEVAD_NONE, MIIM_88E1310_PHY_PAGE, 0x0002); reg = phy_read(phydev, MDIO_DEVAD_NONE, MIIM_88E1310_PHY_RGMII_CTRL); reg |= 0x0030; phy_write(phydev, MDIO_DEVAD_NONE, MIIM_88E1310_PHY_RGMII_CTRL, reg); Can't you use the Marvell PHY driver instead of this ad-hoc implementation? I didn't check the compatibilty of your PHY and this driver though.Thanks for the advice. Marek had a similar comment regarding this code. https://lists.denx.de/pipermail/u-boot/2021-December/470019.html Marek said, "There the m88e1118_config() method already does one thing of what you are doing here: enabling rgmii delays. It also sets LED config, but does not reset the PHY. You can add call to phy_reset() there..." Currently, all other Kirkwood boards also use this old ad-hoc implementation. And the PHY addr is different in some of these boards. If we add phy_reset() in m88e1118_config(), we would need to make sure all existing boards that use the driver will work... OTOH, perhaps we can keep the phy_reset() call in each Kirkwood board...
I should be checked, if this phy_reset() is really necessary. Perhaps by checking in the Linux Kernel code as well and the documentation of the PHY.
Sounds like an approach that will potentially touch a common area, so I think it is best that I will do the modernization and more testing in a separate patch series, after we're done with this Pogo V4 board. Does that sound reasonable?
Yes, I'm fine with this approach. Let's handle this in some follow-up patches ( from you ;) ). Thanks, Stefan
