diff mbox series

net: stmmac: dwmac-rk: Fix ethernet on rk3399 based devices

Message ID 20210929135049.3426058-1-punitagrawal@gmail.com (mailing list archive)
State New, archived
Headers show
Series net: stmmac: dwmac-rk: Fix ethernet on rk3399 based devices | expand

Commit Message

Punit Agrawal Sept. 29, 2021, 1:50 p.m. UTC
Commit 2d26f6e39afb ("net: stmmac: dwmac-rk: fix unbalanced pm_runtime_enable warnings")
while getting rid of a runtime PM warning ended up breaking ethernet
on rk3399 based devices. By dropping an extra reference to the device,
the commit ends up enabling suspend / resume of the ethernet device -
which appears to be broken.

While the issue with runtime pm is being investigated, partially
revert commit 2d26f6e39afb to restore the network on rk3399.

Fixes: 2d26f6e39afb ("net: stmmac: dwmac-rk: fix unbalanced pm_runtime_enable warnings")
Suggested-by: Heiko Stuebner <heiko@sntech.de>
Signed-off-by: Punit Agrawal <punitagrawal@gmail.com>
Cc: Michael Riesch <michael.riesch@wolfvision.net>
---
Hi,

There's been a few reports of broken ethernet on rk3399 based
boards. The issue got introduced due to a late commit in the 5.14
cycle.

It would be great if this commit can be taken as a fix for the next rc
as well as applied to the 5.14 stable releases.

Thanks,
Punit

 drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Heiko Stübner Sept. 29, 2021, 9:02 p.m. UTC | #1
Am Mittwoch, 29. September 2021, 15:50:49 CEST schrieb Punit Agrawal:
> Commit 2d26f6e39afb ("net: stmmac: dwmac-rk: fix unbalanced pm_runtime_enable warnings")
> while getting rid of a runtime PM warning ended up breaking ethernet
> on rk3399 based devices. By dropping an extra reference to the device,
> the commit ends up enabling suspend / resume of the ethernet device -
> which appears to be broken.
> 
> While the issue with runtime pm is being investigated, partially
> revert commit 2d26f6e39afb to restore the network on rk3399.
> 
> Fixes: 2d26f6e39afb ("net: stmmac: dwmac-rk: fix unbalanced pm_runtime_enable warnings")
> Suggested-by: Heiko Stuebner <heiko@sntech.de>
> Signed-off-by: Punit Agrawal <punitagrawal@gmail.com>
> Cc: Michael Riesch <michael.riesch@wolfvision.net>

On a rk3399-puma which has the described issue,
Tested-by: Heiko Stuebner <heiko@sntech.de>


> ---
> Hi,
> 
> There's been a few reports of broken ethernet on rk3399 based
> boards. The issue got introduced due to a late commit in the 5.14
> cycle.
> 
> It would be great if this commit can be taken as a fix for the next rc
> as well as applied to the 5.14 stable releases.
> 
> Thanks,
> Punit
> 
>  drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> index ed817011a94a..6924a6aacbd5 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> @@ -21,6 +21,7 @@
>  #include <linux/delay.h>
>  #include <linux/mfd/syscon.h>
>  #include <linux/regmap.h>
> +#include <linux/pm_runtime.h>
>  
>  #include "stmmac_platform.h"
>  
> @@ -1528,6 +1529,8 @@ static int rk_gmac_powerup(struct rk_priv_data *bsp_priv)
>  		return ret;
>  	}
>  
> +	pm_runtime_get_sync(dev);
> +
>  	if (bsp_priv->integrated_phy)
>  		rk_gmac_integrated_phy_powerup(bsp_priv);
>  
> @@ -1539,6 +1542,8 @@ static void rk_gmac_powerdown(struct rk_priv_data *gmac)
>  	if (gmac->integrated_phy)
>  		rk_gmac_integrated_phy_powerdown(gmac);
>  
> +	pm_runtime_put_sync(&gmac->pdev->dev);
> +
>  	phy_power_on(gmac, false);
>  	gmac_clk_enable(gmac, false);
>  }
>
Jakub Kicinski Oct. 1, 2021, 11:02 p.m. UTC | #2
On Wed, 29 Sep 2021 23:02:35 +0200 Heiko Stübner wrote:
> Am Mittwoch, 29. September 2021, 15:50:49 CEST schrieb Punit Agrawal:
> > Commit 2d26f6e39afb ("net: stmmac: dwmac-rk: fix unbalanced pm_runtime_enable warnings")
> > while getting rid of a runtime PM warning ended up breaking ethernet
> > on rk3399 based devices. By dropping an extra reference to the device,
> > the commit ends up enabling suspend / resume of the ethernet device -
> > which appears to be broken.
> > 
> > While the issue with runtime pm is being investigated, partially
> > revert commit 2d26f6e39afb to restore the network on rk3399.
> > 
> > Fixes: 2d26f6e39afb ("net: stmmac: dwmac-rk: fix unbalanced pm_runtime_enable warnings")
> > Suggested-by: Heiko Stuebner <heiko@sntech.de>
> > Signed-off-by: Punit Agrawal <punitagrawal@gmail.com>
> > Cc: Michael Riesch <michael.riesch@wolfvision.net>  
> 
> On a rk3399-puma which has the described issue,
> Tested-by: Heiko Stuebner <heiko@sntech.de>

Applied, thanks!
Andreas Rammhold Oct. 2, 2021, 9:33 p.m. UTC | #3
On 16:02 01.10.21, Jakub Kicinski wrote:
> On Wed, 29 Sep 2021 23:02:35 +0200 Heiko Stübner wrote:
> > Am Mittwoch, 29. September 2021, 15:50:49 CEST schrieb Punit Agrawal:
> > > Commit 2d26f6e39afb ("net: stmmac: dwmac-rk: fix unbalanced pm_runtime_enable warnings")
> > > while getting rid of a runtime PM warning ended up breaking ethernet
> > > on rk3399 based devices. By dropping an extra reference to the device,
> > > the commit ends up enabling suspend / resume of the ethernet device -
> > > which appears to be broken.
> > > 
> > > While the issue with runtime pm is being investigated, partially
> > > revert commit 2d26f6e39afb to restore the network on rk3399.
> > > 
> > > Fixes: 2d26f6e39afb ("net: stmmac: dwmac-rk: fix unbalanced pm_runtime_enable warnings")
> > > Suggested-by: Heiko Stuebner <heiko@sntech.de>
> > > Signed-off-by: Punit Agrawal <punitagrawal@gmail.com>
> > > Cc: Michael Riesch <michael.riesch@wolfvision.net>  
> > 
> > On a rk3399-puma which has the described issue,
> > Tested-by: Heiko Stuebner <heiko@sntech.de>
> 
> Applied, thanks!

This also fixed the issue on a RockPi4.

Will any of you submit this to the stable kernels (as this broke within
3.13 for me) or shall I do that?
Jakub Kicinski Oct. 3, 2021, 12:20 a.m. UTC | #4
On Sat, 2 Oct 2021 23:33:03 +0200 Andreas Rammhold wrote:
> On 16:02 01.10.21, Jakub Kicinski wrote:
> > On Wed, 29 Sep 2021 23:02:35 +0200 Heiko Stübner wrote:  
> > > On a rk3399-puma which has the described issue,
> > > Tested-by: Heiko Stuebner <heiko@sntech.de>  
> > 
> > Applied, thanks!  
> 
> This also fixed the issue on a RockPi4.
> 
> Will any of you submit this to the stable kernels (as this broke within
> 3.13 for me) or shall I do that?

I won't. The patch should be in Linus's tree in around 1 week - at which
point anyone can request the backport.

That said, as you probably know, 4.4 is the oldest active stable branch,
the ship has sailed for anything 3.x.
Andreas Rammhold Oct. 3, 2021, 12:41 a.m. UTC | #5
On 17:20 02.10.21, Jakub Kicinski wrote:
> On Sat, 2 Oct 2021 23:33:03 +0200 Andreas Rammhold wrote:
> > On 16:02 01.10.21, Jakub Kicinski wrote:
> > > On Wed, 29 Sep 2021 23:02:35 +0200 Heiko Stübner wrote:  
> > > > On a rk3399-puma which has the described issue,
> > > > Tested-by: Heiko Stuebner <heiko@sntech.de>  
> > > 
> > > Applied, thanks!  
> > 
> > This also fixed the issue on a RockPi4.
> > 
> > Will any of you submit this to the stable kernels (as this broke within
> > 3.13 for me) or shall I do that?
> 
> I won't. The patch should be in Linus's tree in around 1 week - at which
> point anyone can request the backport.
> 
> That said, as you probably know, 4.4 is the oldest active stable branch,
> the ship has sailed for anything 3.x.

I am sorry. I meant 5.13.
Heiko Stübner Oct. 3, 2021, 9:15 a.m. UTC | #6
Am Sonntag, 3. Oktober 2021, 02:41:03 CEST schrieb Andreas Rammhold:
> On 17:20 02.10.21, Jakub Kicinski wrote:
> > On Sat, 2 Oct 2021 23:33:03 +0200 Andreas Rammhold wrote:
> > > On 16:02 01.10.21, Jakub Kicinski wrote:
> > > > On Wed, 29 Sep 2021 23:02:35 +0200 Heiko Stübner wrote:  
> > > > > On a rk3399-puma which has the described issue,
> > > > > Tested-by: Heiko Stuebner <heiko@sntech.de>  
> > > > 
> > > > Applied, thanks!  
> > > 
> > > This also fixed the issue on a RockPi4.
> > > 
> > > Will any of you submit this to the stable kernels (as this broke within
> > > 3.13 for me) or shall I do that?
> > 
> > I won't. The patch should be in Linus's tree in around 1 week - at which
> > point anyone can request the backport.
> > 
> > That said, as you probably know, 4.4 is the oldest active stable branch,
> > the ship has sailed for anything 3.x.
> 
> I am sorry. I meant 5.13.
> 

As the commit has "fixes" tag, it should be picked up automatically
for stable kernels that include the original commit.


Heiko
Punit Agrawal Oct. 4, 2021, 12:06 p.m. UTC | #7
Andreas Rammhold <andreas@rammhold.de> writes:

> On 17:20 02.10.21, Jakub Kicinski wrote:
>> On Sat, 2 Oct 2021 23:33:03 +0200 Andreas Rammhold wrote:
>> > On 16:02 01.10.21, Jakub Kicinski wrote:
>> > > On Wed, 29 Sep 2021 23:02:35 +0200 Heiko Stübner wrote:  
>> > > > On a rk3399-puma which has the described issue,
>> > > > Tested-by: Heiko Stuebner <heiko@sntech.de>  
>> > > 
>> > > Applied, thanks!  
>> > 
>> > This also fixed the issue on a RockPi4.
>> > 
>> > Will any of you submit this to the stable kernels (as this broke within
>> > 3.13 for me) or shall I do that?
>> 
>> I won't. The patch should be in Linus's tree in around 1 week - at which
>> point anyone can request the backport.
>> 
>> That said, as you probably know, 4.4 is the oldest active stable branch,
>> the ship has sailed for anything 3.x.
>
> I am sorry. I meant 5.13.

AFAICT, 2d26f6e39afb ("net: stmmac: dwmac-rk: fix unbalanced
pm_runtime_enable warnings") is not in 5.13 stable.

Either you're not using the stable kernel or there's another issue
breaking things on the RockPi4.
Andreas Rammhold Oct. 5, 2021, 9:08 a.m. UTC | #8
On 21:06 04.10.21, Punit Agrawal wrote:
> Andreas Rammhold <andreas@rammhold.de> writes:
> 
> > On 17:20 02.10.21, Jakub Kicinski wrote:
> >> On Sat, 2 Oct 2021 23:33:03 +0200 Andreas Rammhold wrote:
> >> > On 16:02 01.10.21, Jakub Kicinski wrote:
> >> > > On Wed, 29 Sep 2021 23:02:35 +0200 Heiko Stübner wrote:  
> >> > > > On a rk3399-puma which has the described issue,
> >> > > > Tested-by: Heiko Stuebner <heiko@sntech.de>  
> >> > > 
> >> > > Applied, thanks!  
> >> > 
> >> > This also fixed the issue on a RockPi4.
> >> > 
> >> > Will any of you submit this to the stable kernels (as this broke within
> >> > 3.13 for me) or shall I do that?
> >> 
> >> I won't. The patch should be in Linus's tree in around 1 week - at which
> >> point anyone can request the backport.
> >> 
> >> That said, as you probably know, 4.4 is the oldest active stable branch,
> >> the ship has sailed for anything 3.x.
> >
> > I am sorry. I meant 5.13.
> 
> AFAICT, 2d26f6e39afb ("net: stmmac: dwmac-rk: fix unbalanced
> pm_runtime_enable warnings") is not in 5.13 stable.
> 
> Either you're not using the stable kernel or there's another issue
> breaking things on the RockPi4.

I was using the 5.13 branch, somewhere after 5.13.12 the network started
to fail on bootup. Due to the nature of the system I don't have
persistent logs on it. When I next looked at the system (and updated to
5.14.x) the issue still occured until I applied this patch on to 5.14.8.
Might have been the same use-facing issue but different bugs?

I can try to narrow the issue down further. It'll probably take a few
evenings to test this out.
diff mbox series

Patch

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
index ed817011a94a..6924a6aacbd5 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
@@ -21,6 +21,7 @@ 
 #include <linux/delay.h>
 #include <linux/mfd/syscon.h>
 #include <linux/regmap.h>
+#include <linux/pm_runtime.h>
 
 #include "stmmac_platform.h"
 
@@ -1528,6 +1529,8 @@  static int rk_gmac_powerup(struct rk_priv_data *bsp_priv)
 		return ret;
 	}
 
+	pm_runtime_get_sync(dev);
+
 	if (bsp_priv->integrated_phy)
 		rk_gmac_integrated_phy_powerup(bsp_priv);
 
@@ -1539,6 +1542,8 @@  static void rk_gmac_powerdown(struct rk_priv_data *gmac)
 	if (gmac->integrated_phy)
 		rk_gmac_integrated_phy_powerdown(gmac);
 
+	pm_runtime_put_sync(&gmac->pdev->dev);
+
 	phy_power_on(gmac, false);
 	gmac_clk_enable(gmac, false);
 }