[PATCH 1/2] net: ti: am65-cpsw-nuss: Workaround for buggy PHY/Board

[Siddharth Vadapalli]

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 <rogerq at 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.