netdev - Re: [PATCH] net: phy: marvell10g: report if the PHY fails to boot firmware

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] [day] [month] [year] [list]

Date:   Thu, 30 May 2019 16:12:20 +0200
From:   Andrew Lunn <andrew@...n.ch>
To:     Russell King - ARM Linux admin <linux@...linux.org.uk>
Cc:     Florian Fainelli <f.fainelli@...il.com>,
        Heiner Kallweit <hkallweit1@...il.com>,
        Maxime Chevallier <maxime.chevallier@...tlin.com>,
        "David S. Miller" <davem@...emloft.net>, netdev@...r.kernel.org
Subject: Re: [PATCH] net: phy: marvell10g: report if the PHY fails to boot
 firmware

> Here's the fuller patch for what I'm suggesting:
> 
>  drivers/net/phy/marvell10g.c | 25 ++++++++++++++++++++-----
>  1 file changed, 20 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/net/phy/marvell10g.c b/drivers/net/phy/marvell10g.c
> index 754cde873dde..86333d98b384 100644
> --- a/drivers/net/phy/marvell10g.c
> +++ b/drivers/net/phy/marvell10g.c
> @@ -60,6 +60,8 @@ enum {
>  };
>  
>  struct mv3310_priv {
> +	bool firmware_failed;
> +
>  	struct device *hwmon_dev;
>  	char *hwmon_name;
>  };
> @@ -214,6 +216,10 @@ static int mv3310_probe(struct phy_device *phydev)
>  	    (phydev->c45_ids.devices_in_package & mmd_mask) != mmd_mask)
>  		return -ENODEV;
>  
> +	priv = devm_kzalloc(&phydev->mdio.dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
>  	ret = phy_read_mmd(phydev, MDIO_MMD_PMAPMD, MV_PMA_BOOT);
>  	if (ret < 0)
>  		return ret;
> @@ -221,13 +227,9 @@ static int mv3310_probe(struct phy_device *phydev)
>  	if (ret & MV_PMA_BOOT_FATAL) {
>  		dev_warn(&phydev->mdio.dev,
>  			 "PHY failed to boot firmware, status=%04x\n", ret);
> -		return -ENODEV;
> +		priv->firmware_failed = true;
>  	}
>  
> -	priv = devm_kzalloc(&phydev->mdio.dev, sizeof(*priv), GFP_KERNEL);
> -	if (!priv)
> -		return -ENOMEM;
> -
>  	dev_set_drvdata(&phydev->mdio.dev, priv);
>  
>  	ret = mv3310_hwmon_probe(phydev);
> @@ -247,6 +249,19 @@ static int mv3310_resume(struct phy_device *phydev)
>  	return mv3310_hwmon_config(phydev, true);
>  }
>  
> +static void mv3310_link_change_notify(struct phy_device *phydev)
> +{
> +	struct mv3310_priv *priv = dev_get_drvdata(&phydev->mdio.dev);
> +	enum phy_state state = phydev->state;
> +
> +	if (priv->firmware_failed &&
> +	    (state == PHY_UP || state == PHY_RESUMING)) {
> +		dev_warn(&phydev->mdio.dev,
> +			 "PHY firmware failure: link forced down");
> +		phydev->state = state = PHY_HALTED;
> +	}
> +}

Hi Russell

This looks good.

     Andrew