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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 9 Nov 2022 19:54:10 -0800
From:   Florian Fainelli <f.fainelli@...il.com>
To:     Aminuddin Jamaluddin <aminuddin.jamaluddin@...el.com>,
        Andrew Lunn <andrew@...n.ch>,
        Heiner Kallweit <hkallweit1@...il.com>,
        Russell King <linux@...linux.org.uk>,
        "David S . Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>,
        Mohammad Athari Bin Ismail <mohammad.athari.ismail@...el.com>
Cc:     netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
        stable@...r.kernel.org, tee.min.tan@...el.com,
        muhammad.husaini.zulkifli@...el.com, hong.aun.looi@...el.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.11056-1-aminuddin.jamaluddin@intel.com/
> ---
> 
> Fixes: 020a45aff119 ("net: phy: marvell: add Marvell specific PHY loopback")
> Cc: <stable@...r.kernel.org> # 5.15.x
> Signed-off-by: Muhammad Husaini Zulkifli <muhammad.husaini.zulkifli@...el.com>
> Signed-off-by: Aminuddin Jamaluddin <aminuddin.jamaluddin@...el.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