[PATCH net v2 0/2] Fix 88x3310 leaving power save mode

[PATCH net v2 0/2] Fix 88x3310 leaving power save mode

[Russell King - ARM Linux admin]

Hi,

This series fixes a problem with the 88x3310 PHY on Macchiatobin
coming out of powersave mode noticed by Matteo Croce.  It seems
that certain PHY firmwares do not properly exit powersave mode,
resulting in a fibre link not coming up.

The solution appears to be to soft-reset the PHY after clearing
the powersave bit.

We add support for reporting the PHY firmware version to the kernel
log, and use it to trigger this new behaviour if we have v0.3.x.x
or more recent firmware on the PHY.  This, however, is a guess as
the firmware revision documentation does not mention this issue,
and we know that v0.2.1.0 works without this fix but v0.3.3.0 and
later does not.

 drivers/net/phy/marvell10g.c | 36 +++++++++++++++++++++++++++++++++---
 1 file changed, 33 insertions(+), 3 deletions(-)

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 10.2Mbps down 587kbps up

[PATCH net v2 1/2] net: marvell10g: report firmware version

[Russell King]

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 | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/drivers/net/phy/marvell10g.c b/drivers/net/phy/marvell10g.c
index 7621badae64d..748532d9e1ae 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,22 @@ 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;
+
+	phydev_info(phydev, "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)
-- 
2.20.1

[PATCH net v2 2/2] net: marvell10g: soft-reset the PHY when coming out of low power

[Russell King]

Soft-reset the PHY when coming out of low power mode, which seems to
be necessary with firmware versions 0.3.3.0 and 0.3.10.0.

This depends on ("net: marvell10g: report firmware version")

Fixes: c9cc1c815d36 ("net: phy: marvell10g: place in powersave mode at probe")
Reported-by: Matteo Croce <mcroce@redhat.com>
Tested-by: Matteo Croce <mcroce@redhat.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 drivers/net/phy/marvell10g.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/drivers/net/phy/marvell10g.c b/drivers/net/phy/marvell10g.c
index 748532d9e1ae..95e3f4644aeb 100644
--- a/drivers/net/phy/marvell10g.c
+++ b/drivers/net/phy/marvell10g.c
@@ -75,7 +75,8 @@ enum {
 
 	/* Vendor2 MMD registers */
 	MV_V2_PORT_CTRL		= 0xf001,
-	MV_V2_PORT_CTRL_PWRDOWN = 0x0800,
+	MV_V2_PORT_CTRL_SWRST	= BIT(15),
+	MV_V2_PORT_CTRL_PWRDOWN = BIT(11),
 	MV_V2_TEMP_CTRL		= 0xf08a,
 	MV_V2_TEMP_CTRL_MASK	= 0xc000,
 	MV_V2_TEMP_CTRL_SAMPLE	= 0x0000,
@@ -239,8 +240,17 @@ static int mv3310_power_down(struct phy_device *phydev)
 
 static int mv3310_power_up(struct phy_device *phydev)
 {
-	return phy_clear_bits_mmd(phydev, MDIO_MMD_VEND2, MV_V2_PORT_CTRL,
-				  MV_V2_PORT_CTRL_PWRDOWN);
+	struct mv3310_priv *priv = dev_get_drvdata(&phydev->mdio.dev);
+	int ret;
+
+	ret = phy_clear_bits_mmd(phydev, MDIO_MMD_VEND2, MV_V2_PORT_CTRL,
+				 MV_V2_PORT_CTRL_PWRDOWN);
+
+	if (priv->firmware_ver < 0x00030000)
+		return ret;
+
+	return phy_set_bits_mmd(phydev, MDIO_MMD_VEND2, MV_V2_PORT_CTRL,
+				MV_V2_PORT_CTRL_SWRST);
 }
 
 static int mv3310_reset(struct phy_device *phydev, u32 unit)
-- 
2.20.1

Re: [PATCH net v2 1/2] net: marvell10g: report firmware version

[Andrew Lunn]

On Tue, Apr 14, 2020 at 08:49:03PM +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>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [PATCH net v2 1/2] net: marvell10g: report firmware version

[Florian Fainelli]

On 4/14/2020 12:49 PM, 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>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

Re: [PATCH net v2 2/2] net: marvell10g: soft-reset the PHY when coming out of low power

[Florian Fainelli]

On 4/14/2020 12:49 PM, Russell King wrote:
> Soft-reset the PHY when coming out of low power mode, which seems to
> be necessary with firmware versions 0.3.3.0 and 0.3.10.0.
> 
> This depends on ("net: marvell10g: report firmware version")
> 
> Fixes: c9cc1c815d36 ("net: phy: marvell10g: place in powersave mode at probe")
> Reported-by: Matteo Croce <mcroce@redhat.com>
> Tested-by: Matteo Croce <mcroce@redhat.com>
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

Re: [PATCH net v2 0/2] Fix 88x3310 leaving power save mode

[David Miller]

From: Russell King - ARM Linux admin <linux@armlinux.org.uk>
Date: Tue, 14 Apr 2020 20:47:53 +0100

> This series fixes a problem with the 88x3310 PHY on Macchiatobin
> coming out of powersave mode noticed by Matteo Croce.  It seems
> that certain PHY firmwares do not properly exit powersave mode,
> resulting in a fibre link not coming up.
> 
> The solution appears to be to soft-reset the PHY after clearing
> the powersave bit.
> 
> We add support for reporting the PHY firmware version to the kernel
> log, and use it to trigger this new behaviour if we have v0.3.x.x
> or more recent firmware on the PHY.  This, however, is a guess as
> the firmware revision documentation does not mention this issue,
> and we know that v0.2.1.0 works without this fix but v0.3.3.0 and
> later does not.

Series applied, thanks.

Re: [PATCH net v2 0/2] Fix 88x3310 leaving power save mode

[Matteo Croce]

On Wed, Apr 15, 2020 at 1:48 AM David Miller <davem@davemloft.net> wrote:
>
> From: Russell King - ARM Linux admin <linux@armlinux.org.uk>
> Date: Tue, 14 Apr 2020 20:47:53 +0100
>
> > This series fixes a problem with the 88x3310 PHY on Macchiatobin
> > coming out of powersave mode noticed by Matteo Croce.  It seems
> > that certain PHY firmwares do not properly exit powersave mode,
> > resulting in a fibre link not coming up.
> >
> > The solution appears to be to soft-reset the PHY after clearing
> > the powersave bit.
> >
> > We add support for reporting the PHY firmware version to the kernel
> > log, and use it to trigger this new behaviour if we have v0.3.x.x
> > or more recent firmware on the PHY.  This, however, is a guess as
> > the firmware revision documentation does not mention this issue,
> > and we know that v0.2.1.0 works without this fix but v0.3.3.0 and
> > later does not.
>
> Series applied, thanks.
>

Hi,

should we queue this to -stable?
The 10 gbit ports don't work without this fix.

Regards,
-- 
Matteo Croce
per aspera ad upstream

Re: [PATCH net v2 0/2] Fix 88x3310 leaving power save mode

[Russell King - ARM Linux admin]

On Fri, May 08, 2020 at 11:32:39PM +0200, Matteo Croce wrote:
> On Wed, Apr 15, 2020 at 1:48 AM David Miller <davem@davemloft.net> wrote:
> >
> > From: Russell King - ARM Linux admin <linux@armlinux.org.uk>
> > Date: Tue, 14 Apr 2020 20:47:53 +0100
> >
> > > This series fixes a problem with the 88x3310 PHY on Macchiatobin
> > > coming out of powersave mode noticed by Matteo Croce.  It seems
> > > that certain PHY firmwares do not properly exit powersave mode,
> > > resulting in a fibre link not coming up.
> > >
> > > The solution appears to be to soft-reset the PHY after clearing
> > > the powersave bit.
> > >
> > > We add support for reporting the PHY firmware version to the kernel
> > > log, and use it to trigger this new behaviour if we have v0.3.x.x
> > > or more recent firmware on the PHY.  This, however, is a guess as
> > > the firmware revision documentation does not mention this issue,
> > > and we know that v0.2.1.0 works without this fix but v0.3.3.0 and
> > > later does not.
> >
> > Series applied, thanks.
> >
> 
> Hi,
> 
> should we queue this to -stable?
> The 10 gbit ports don't work without this fix.

It has a "Fixes:" tag, so it should be backported automatically.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 10.2Mbps down 587kbps up

Re: [PATCH net v2 0/2] Fix 88x3310 leaving power save mode

[Greg Kroah-Hartman]

On Fri, May 08, 2020 at 10:38:16PM +0100, Russell King - ARM Linux admin wrote:
> On Fri, May 08, 2020 at 11:32:39PM +0200, Matteo Croce wrote:
> > On Wed, Apr 15, 2020 at 1:48 AM David Miller <davem@davemloft.net> wrote:
> > >
> > > From: Russell King - ARM Linux admin <linux@armlinux.org.uk>
> > > Date: Tue, 14 Apr 2020 20:47:53 +0100
> > >
> > > > This series fixes a problem with the 88x3310 PHY on Macchiatobin
> > > > coming out of powersave mode noticed by Matteo Croce.  It seems
> > > > that certain PHY firmwares do not properly exit powersave mode,
> > > > resulting in a fibre link not coming up.
> > > >
> > > > The solution appears to be to soft-reset the PHY after clearing
> > > > the powersave bit.
> > > >
> > > > We add support for reporting the PHY firmware version to the kernel
> > > > log, and use it to trigger this new behaviour if we have v0.3.x.x
> > > > or more recent firmware on the PHY.  This, however, is a guess as
> > > > the firmware revision documentation does not mention this issue,
> > > > and we know that v0.2.1.0 works without this fix but v0.3.3.0 and
> > > > later does not.
> > >
> > > Series applied, thanks.
> > >
> > 
> > Hi,
> > 
> > should we queue this to -stable?
> > The 10 gbit ports don't work without this fix.
> 
> It has a "Fixes:" tag, so it should be backported automatically.

That is a wild guess that it might happen sometime in the future.
Please use the cc: stable@ tag as is documented for the past 15+ years
instead of relying on us to randomly notice a Fixes: tag.

thanks,

greg k-h

Re: [PATCH net v2 0/2] Fix 88x3310 leaving power save mode

[Russell King - ARM Linux admin]

On Sat, May 09, 2020 at 08:36:31AM +0200, Greg Kroah-Hartman wrote:
> On Fri, May 08, 2020 at 10:38:16PM +0100, Russell King - ARM Linux admin wrote:
> > On Fri, May 08, 2020 at 11:32:39PM +0200, Matteo Croce wrote:
> > > On Wed, Apr 15, 2020 at 1:48 AM David Miller <davem@davemloft.net> wrote:
> > > >
> > > > From: Russell King - ARM Linux admin <linux@armlinux.org.uk>
> > > > Date: Tue, 14 Apr 2020 20:47:53 +0100
> > > >
> > > > > This series fixes a problem with the 88x3310 PHY on Macchiatobin
> > > > > coming out of powersave mode noticed by Matteo Croce.  It seems
> > > > > that certain PHY firmwares do not properly exit powersave mode,
> > > > > resulting in a fibre link not coming up.
> > > > >
> > > > > The solution appears to be to soft-reset the PHY after clearing
> > > > > the powersave bit.
> > > > >
> > > > > We add support for reporting the PHY firmware version to the kernel
> > > > > log, and use it to trigger this new behaviour if we have v0.3.x.x
> > > > > or more recent firmware on the PHY.  This, however, is a guess as
> > > > > the firmware revision documentation does not mention this issue,
> > > > > and we know that v0.2.1.0 works without this fix but v0.3.3.0 and
> > > > > later does not.
> > > >
> > > > Series applied, thanks.
> > > >
> > > 
> > > Hi,
> > > 
> > > should we queue this to -stable?
> > > The 10 gbit ports don't work without this fix.
> > 
> > It has a "Fixes:" tag, so it should be backported automatically.
> 
> That is a wild guess that it might happen sometime in the future.
> Please use the cc: stable@ tag as is documented for the past 15+ years
> instead of relying on us to randomly notice a Fixes: tag.

Not for netdev material.  Netdev has its own rules:

https://www.kernel.org/doc/Documentation/networking/netdev-FAQ.txt

Q: I see a network patch and I think it should be backported to stable.
   various stable releases?

A: Normally Greg Kroah-Hartman collects stable commits himself, but
   for networking, Dave collects up patches he deems critical for the
   networking subsystem, and then hands them off to Greg.
...

Q: I see a network patch and I think it should be backported to stable.
   Should I request it via "stable@vger.kernel.org" like the references in
   the kernel's Documentation/process/stable-kernel-rules.rst file say?

A: No, not for networking.  Check the stable queues as per above 1st to see
   if it is already queued.  If not, then send a mail to netdev, listing
   the upstream commit ID and why you think it should be a stable candidate.
...

Q: I have created a network patch and I think it should be backported to
   stable.  Should I add a "Cc: stable@vger.kernel.org" like the references
   in the kernel's Documentation/ directory say?

A: No.  See above answer.  In short, if you think it really belongs in
   stable, then ensure you write a decent commit log that describes who
   gets impacted by the bugfix and how it manifests itself, and when the
   bug was introduced.  If you do that properly, then the commit will
   get handled appropriately and most likely get put in the patchworks
   stable queue if it really warrants it.
...

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 10.2Mbps down 587kbps up

Re: [PATCH net v2 0/2] Fix 88x3310 leaving power save mode

[Greg Kroah-Hartman]

On Sat, May 09, 2020 at 08:32:56AM +0100, Russell King - ARM Linux admin wrote:
> On Sat, May 09, 2020 at 08:36:31AM +0200, Greg Kroah-Hartman wrote:
> > On Fri, May 08, 2020 at 10:38:16PM +0100, Russell King - ARM Linux admin wrote:
> > > On Fri, May 08, 2020 at 11:32:39PM +0200, Matteo Croce wrote:
> > > > On Wed, Apr 15, 2020 at 1:48 AM David Miller <davem@davemloft.net> wrote:
> > > > >
> > > > > From: Russell King - ARM Linux admin <linux@armlinux.org.uk>
> > > > > Date: Tue, 14 Apr 2020 20:47:53 +0100
> > > > >
> > > > > > This series fixes a problem with the 88x3310 PHY on Macchiatobin
> > > > > > coming out of powersave mode noticed by Matteo Croce.  It seems
> > > > > > that certain PHY firmwares do not properly exit powersave mode,
> > > > > > resulting in a fibre link not coming up.
> > > > > >
> > > > > > The solution appears to be to soft-reset the PHY after clearing
> > > > > > the powersave bit.
> > > > > >
> > > > > > We add support for reporting the PHY firmware version to the kernel
> > > > > > log, and use it to trigger this new behaviour if we have v0.3.x.x
> > > > > > or more recent firmware on the PHY.  This, however, is a guess as
> > > > > > the firmware revision documentation does not mention this issue,
> > > > > > and we know that v0.2.1.0 works without this fix but v0.3.3.0 and
> > > > > > later does not.
> > > > >
> > > > > Series applied, thanks.
> > > > >
> > > > 
> > > > Hi,
> > > > 
> > > > should we queue this to -stable?
> > > > The 10 gbit ports don't work without this fix.
> > > 
> > > It has a "Fixes:" tag, so it should be backported automatically.
> > 
> > That is a wild guess that it might happen sometime in the future.
> > Please use the cc: stable@ tag as is documented for the past 15+ years
> > instead of relying on us to randomly notice a Fixes: tag.
> 
> Not for netdev material.  Netdev has its own rules:
> 
> https://www.kernel.org/doc/Documentation/networking/netdev-FAQ.txt

<snip>

Sorry, missed that this was netdev, my fault.

greg k-h