Roger,
On 22/08/23 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.
Based on the datasheet at: https://datasheet.lcsc.com/lcsc/1912111437_Realtek-Semicon-RTL8211F-CG_C1879... in the section 7.17. PHY Reset (Hardware Reset), it is stated that software has to wait for at least 50 ms before accessing the PHY registers. Is this why the for-loop in the code below tries for at most 5 times with a delay of 10 ms before the next try? If yes, then isn't it better to move the delay into the realtek PHY driver at drivers/net/phy/realtek.c Shouldn't it be the PHY driver which ensures that any reads/writes to the PHY registers are valid? It can ensure this by having a one time 50ms delay for the very first access to the PHY registers.
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@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);}
if (!phydev) { dev_err(dev, "phy_connect() failed\n");