Re: [PATCH net-next v2] net: phy: marvell: add sleep time after enabling the loopback bit

Florian Fainelli
2022-11-10T03:54:19Z

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.11056-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 to have
> - * 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?
--
Florian