-----Original Message----- From: Florian Fainelli f.fainelli@gmail.com Sent: Thursday, 10 November, 2022 11:54 AM To: Jamaluddin, Aminuddin aminuddin.jamaluddin@intel.com; Andrew Lunn andrew@lunn.ch; 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 Cc: 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; Looi, Hong Aun hong.aun.looi@intel.com Subject: Re: [PATCH net-next v2] net: phy: marvell: add sleep time after enabling the loopback bit
On 11/7/2022 11:40 PM, Aminuddin Jamaluddin wrote:
Sleep time is added to ensure the phy to be ready after loopback bit was set. This to prevent the phy loopback test from failing.
V1:
https://patchwork.kernel.org/project/netdevbpf/patch/20220825082238.11
056-1-aminuddin.jamaluddin@intel.com/
Fixes: 020a45aff119 ("net: phy: marvell: add Marvell specific PHY loopback") Cc: stable@vger.kernel.org # 5.15.x Signed-off-by: Muhammad Husaini Zulkifli muhammad.husaini.zulkifli@intel.com Signed-off-by: Aminuddin Jamaluddin aminuddin.jamaluddin@intel.com
drivers/net/phy/marvell.c | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c index a3e810705ce2..860610ba4d00 100644 --- a/drivers/net/phy/marvell.c +++ b/drivers/net/phy/marvell.c @@ -2015,14 +2015,16 @@ 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);
err = phy_modify(phydev, MII_BMCR, BMCR_LOOPBACK,BMCR_LOOPBACK);
return phy_modify(phydev, MII_BMCR, BMCR_LOOPBACK,BMCR_LOOPBACK);
if (!err) {/* It takes some time for PHY device to switch* into/out-of loopback mode.*/msleep(1000);Is not there a better indication than waiting a full second to ensure the PHY exited loopback?
Previously we implemented the link status check that waiting phy to be ready before the loopback bit being set in V1. But we removed it due to simpler behaviour as that can be achieve with this. And previous discussion with Marvell stated the delay timing was needed after loopback bit is set.
--
Amin