Hello Roger,
On 22-08-2023 17:43, Roger Quadros wrote:
> Beagleplay has a buggy Ethernet PHY implementation for the Gigabit
> PHY in the sense that it is non responsive over MDIO immediately
> after power-up/reset.
>
> We need to either try multiple times or wait sufficiently long enough
> (couple of 10s of ms?) before the PHY begins to respond correctly.
>
> One theory is that the PHY is configured to operate on MDIO address 0
> which it treats as a special broadcast address.
>
> Datasheet states:
> "PHYAD (config pins) sets the PHY address for the device.
> The RTL8211F(I)/RTL8211FD(I) supports PHY addresses from 0x01 to 0x07.
> Note 1: An MDIO command with PHY address=0 is a broadcast from the MAC;
> each PHY device should respond."
>
> This issue is not seen with the other PHY (different make) on the board
> which is configured for address 0x1.
>
> As a woraround we try to probe the PHY multiple times instead of giving
> up on the first attempt.
>
> Signed-off-by: Roger Quadros <rog...@kernel.org>
> ---
> drivers/net/ti/am65-cpsw-nuss.c | 19 ++++++++++++++-----
> 1 file changed, 14 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/ti/am65-cpsw-nuss.c b/drivers/net/ti/am65-cpsw-nuss.c
> index 51a8167d14..4f8a2f9b93 100644
> --- a/drivers/net/ti/am65-cpsw-nuss.c
> +++ b/drivers/net/ti/am65-cpsw-nuss.c
> @@ -27,6 +27,7 @@
> #include <syscon.h>
> #include <linux/bitops.h>
> #include <linux/soc/ti/ti-udma.h>
> +#include <linux/delay.h>
>
> #include "cpsw_mdio.h"
>
> @@ -678,14 +679,22 @@ static int am65_cpsw_phy_init(struct udevice *dev)
> struct am65_cpsw_priv *priv = dev_get_priv(dev);
> struct am65_cpsw_common *cpsw_common = priv->cpsw_common;
> struct eth_pdata *pdata = dev_get_plat(dev);
> - struct phy_device *phydev;
> u32 supported = PHY_GBIT_FEATURES;
> + struct phy_device *phydev;
> + int tries;
> int ret;
>
> - phydev = phy_connect(cpsw_common->bus,
> - priv->phy_addr,
> - priv->dev,
> - pdata->phy_interface);
> + /* Some boards (e.g. beagleplay) don't work on first go */
> + for (tries = 1; tries <= 5; tries++) {
> + phydev = phy_connect(cpsw_common->bus,
> + priv->phy_addr,
> + priv->dev,
> + pdata->phy_interface);
> + if (phydev)
> + break;
> +
> + mdelay(10);
> + }
After rethinking about the above implementation and the second patch of
this series, the second patch could be dropped altogether if the
following implementation is acceptable:
phydev = phy_connect(cpsw_common->bus,
priv->phy_addr,
priv->dev,
pdata->phy_interface);
if (!phydev) {
/* Some boards (e.g. beagleplay) don't work on first go */
mdelay(50);
phydev = phy_connect(cpsw_common->bus,
priv->phy_addr,
priv->dev,
pdata->phy_interface);
}
if (!phydev) {
dev_err(dev, "phy_connect() failed\n");
...
With this, there would be at most one "PHY not found" print, which
should be fine. The mdelay value of 50 could be replaced with a
sufficiently large value which guarantees success for Beagleplay.
--
Regards,
Siddharth.
