diff mbox series

[net,1/2] net: marvell10g: report firmware version

Message ID E1jOQK7-0001WH-KM@rmk-PC.armlinux.org.uk
State Superseded
Delegated to: David Miller
Headers show
Series Fix 88x3310 leaving power save mode | expand

Commit Message

Russell King (Oracle) April 14, 2020, 6:30 p.m. UTC 
Report the firmware version when probing the PHY to allow issues
attributable to firmware to be diagnosed.

Tested-by: Matteo Croce <mcroce@redhat.com>
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 drivers/net/phy/marvell10g.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

Comments

Andrew Lunn April 14, 2020, 6:47 p.m. UTC | #1 
On Tue, Apr 14, 2020 at 07:30:15PM +0100, Russell King wrote:
> Report the firmware version when probing the PHY to allow issues
> attributable to firmware to be diagnosed.
> 
> Tested-by: Matteo Croce <mcroce@redhat.com>
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> ---
>  drivers/net/phy/marvell10g.c | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
> 
> diff --git a/drivers/net/phy/marvell10g.c b/drivers/net/phy/marvell10g.c
> index 7621badae64d..ee60417cdc55 100644
> --- a/drivers/net/phy/marvell10g.c
> +++ b/drivers/net/phy/marvell10g.c
> @@ -33,6 +33,8 @@
>  #define MV_PHY_ALASKA_NBT_QUIRK_REV	(MARVELL_PHY_ID_88X3310 | 0xa)
>  
>  enum {
> +	MV_PMA_FW_VER0		= 0xc011,
> +	MV_PMA_FW_VER1		= 0xc012,
>  	MV_PMA_BOOT		= 0xc050,
>  	MV_PMA_BOOT_FATAL	= BIT(0),
>  
> @@ -83,6 +85,8 @@ enum {
>  };
>  
>  struct mv3310_priv {
> +	u32 firmware_ver;
> +
>  	struct device *hwmon_dev;
>  	char *hwmon_name;
>  };
> @@ -355,6 +359,23 @@ static int mv3310_probe(struct phy_device *phydev)
>  
>  	dev_set_drvdata(&phydev->mdio.dev, priv);
>  
> +	ret = phy_read_mmd(phydev, MDIO_MMD_PMAPMD, MV_PMA_FW_VER0);
> +	if (ret < 0)
> +		return ret;
> +
> +	priv->firmware_ver = ret << 16;
> +
> +	ret = phy_read_mmd(phydev, MDIO_MMD_PMAPMD, MV_PMA_FW_VER1);
> +	if (ret < 0)
> +		return ret;
> +
> +	priv->firmware_ver |= ret;
> +
> +	dev_info(&phydev->mdio.dev,
> +		 "Firmware version %u.%u.%u.%u\n",
> +		 priv->firmware_ver >> 24, (priv->firmware_ver >> 16) & 255,
> +		 (priv->firmware_ver >> 8) & 255, priv->firmware_ver & 255);
> +

Hi Russell

Did you consider using phydev_info()? Or is it too early, and you
don't get a sensible name?

      Andrew
Russell King (Oracle) April 14, 2020, 7:45 p.m. UTC | #2 
On Tue, Apr 14, 2020 at 08:47:39PM +0200, Andrew Lunn wrote:
> On Tue, Apr 14, 2020 at 07:30:15PM +0100, Russell King wrote:
> > Report the firmware version when probing the PHY to allow issues
> > attributable to firmware to be diagnosed.
> > 
> > Tested-by: Matteo Croce <mcroce@redhat.com>
> > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> > ---
> >  drivers/net/phy/marvell10g.c | 21 +++++++++++++++++++++
> >  1 file changed, 21 insertions(+)
> > 
> > diff --git a/drivers/net/phy/marvell10g.c b/drivers/net/phy/marvell10g.c
> > index 7621badae64d..ee60417cdc55 100644
> > --- a/drivers/net/phy/marvell10g.c
> > +++ b/drivers/net/phy/marvell10g.c
> > @@ -33,6 +33,8 @@
> >  #define MV_PHY_ALASKA_NBT_QUIRK_REV	(MARVELL_PHY_ID_88X3310 | 0xa)
> >  
> >  enum {
> > +	MV_PMA_FW_VER0		= 0xc011,
> > +	MV_PMA_FW_VER1		= 0xc012,
> >  	MV_PMA_BOOT		= 0xc050,
> >  	MV_PMA_BOOT_FATAL	= BIT(0),
> >  
> > @@ -83,6 +85,8 @@ enum {
> >  };
> >  
> >  struct mv3310_priv {
> > +	u32 firmware_ver;
> > +
> >  	struct device *hwmon_dev;
> >  	char *hwmon_name;
> >  };
> > @@ -355,6 +359,23 @@ static int mv3310_probe(struct phy_device *phydev)
> >  
> >  	dev_set_drvdata(&phydev->mdio.dev, priv);
> >  
> > +	ret = phy_read_mmd(phydev, MDIO_MMD_PMAPMD, MV_PMA_FW_VER0);
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	priv->firmware_ver = ret << 16;
> > +
> > +	ret = phy_read_mmd(phydev, MDIO_MMD_PMAPMD, MV_PMA_FW_VER1);
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	priv->firmware_ver |= ret;
> > +
> > +	dev_info(&phydev->mdio.dev,
> > +		 "Firmware version %u.%u.%u.%u\n",
> > +		 priv->firmware_ver >> 24, (priv->firmware_ver >> 16) & 255,
> > +		 (priv->firmware_ver >> 8) & 255, priv->firmware_ver & 255);
> > +
> 
> Hi Russell
> 
> Did you consider using phydev_info()? Or is it too early, and you
> don't get a sensible name?

No, I keep forgetting those exist!  I'll resend shortly.
diff mbox series

Patch

diff --git a/drivers/net/phy/marvell10g.c b/drivers/net/phy/marvell10g.c
index 7621badae64d..ee60417cdc55 100644
--- a/drivers/net/phy/marvell10g.c
+++ b/drivers/net/phy/marvell10g.c
@@ -33,6 +33,8 @@ 
 #define MV_PHY_ALASKA_NBT_QUIRK_REV	(MARVELL_PHY_ID_88X3310 | 0xa)
 
 enum {
+	MV_PMA_FW_VER0		= 0xc011,
+	MV_PMA_FW_VER1		= 0xc012,
 	MV_PMA_BOOT		= 0xc050,
 	MV_PMA_BOOT_FATAL	= BIT(0),
 
@@ -83,6 +85,8 @@  enum {
 };
 
 struct mv3310_priv {
+	u32 firmware_ver;
+
 	struct device *hwmon_dev;
 	char *hwmon_name;
 };
@@ -355,6 +359,23 @@  static int mv3310_probe(struct phy_device *phydev)
 
 	dev_set_drvdata(&phydev->mdio.dev, priv);
 
+	ret = phy_read_mmd(phydev, MDIO_MMD_PMAPMD, MV_PMA_FW_VER0);
+	if (ret < 0)
+		return ret;
+
+	priv->firmware_ver = ret << 16;
+
+	ret = phy_read_mmd(phydev, MDIO_MMD_PMAPMD, MV_PMA_FW_VER1);
+	if (ret < 0)
+		return ret;
+
+	priv->firmware_ver |= ret;
+
+	dev_info(&phydev->mdio.dev,
+		 "Firmware version %u.%u.%u.%u\n",
+		 priv->firmware_ver >> 24, (priv->firmware_ver >> 16) & 255,
+		 (priv->firmware_ver >> 8) & 255, priv->firmware_ver & 255);
+
 	/* Powering down the port when not in use saves about 600mW */
 	ret = mv3310_power_down(phydev);
 	if (ret)