-----Original Message----- From: Andrew Lunn andrew@lunn.ch Sent: Thursday, 25 August, 2022 9:27 PM To: Jamaluddin, Aminuddin aminuddin.jamaluddin@intel.com Cc: Heiner Kallweit hkallweit1@gmail.com; Russell King linux@armlinux.org.uk; David S . Miller davem@davemloft.net; Eric Dumazet edumazet@google.com; Jakub Kicinski kuba@kernel.org; Paolo Abeni pabeni@redhat.com; Ismail, Mohammad Athari mohammad.athari.ismail@intel.com; netdev@vger.kernel.org; linux- kernel@vger.kernel.org; stable@vger.kernel.org; Tan, Tee Min tee.min.tan@intel.com; Zulkifli, Muhammad Husaini muhammad.husaini.zulkifli@intel.com Subject: Re: [PATCH net 1/1] net: phy: marvell: add link status check before enabling phy loopback
@@ -2015,14 +2016,23 @@ static int m88e1510_loopback(struct
phy_device *phydev, bool enable)
if (err < 0) return err;
/* FIXME: Based on trial and error test, it seem 1G need tohave
* delay between soft reset and loopback enablement.*/if (phydev->speed == SPEED_1000)msleep(1000);
if (phydev->speed == SPEED_1000) {err = phy_read_poll_timeout(phydev, MII_BMSR,val, val & BMSR_LSTATUS,
PHY_LOOP_BACK_SLEEP,PHY_LOOP_BACK_TIMEOUT, true);
Is this link with itself?
Its required cabled plug in, back to back connection.
Have you tested this with the cable unplugged?
Yes we have and its expected to have the timeout. But the self-test required the link to be up first before it can be run.
if (err)return err;I'm just trying to ensure we don't end up here with -ETIMEDOUT.
+#define PHY_LOOP_BACK_SLEEP 1000000 +#define PHY_LOOP_BACK_TIMEOUT 8000000
The kernel seems to be pretty consistent in having loopback as one word.
Noted will update in v2.
Andrew