[PATCH] can: rcar_can: Fix suspend/resume

Subsystems: can network drivers, the rest

STALE1804d REVIEWED: 1 (0M)

1 review trailer.

5 messages, 4 authors, 2021-09-24 · open the first message on its own page

[PATCH] can: rcar_can: Fix suspend/resume

From: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Date: 2021-09-21 05:25:25

If the driver was not opened, rcar_can_suspend() should not call
clk_disable() because the clock was not enabled.

Fixes: fd1159318e55 ("can: add Renesas R-Car CAN driver")
Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Tested-by: Ayumi Nakamichi <redacted>
---
 drivers/net/can/rcar/rcar_can.c | 21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)
diff --git a/drivers/net/can/rcar/rcar_can.c b/drivers/net/can/rcar/rcar_can.c
index 00e4533c8bdd..6b4eefb03044 100644
--- a/drivers/net/can/rcar/rcar_can.c
+++ b/drivers/net/can/rcar/rcar_can.c
@@ -846,10 +846,12 @@ static int __maybe_unused rcar_can_suspend(struct device *dev)
 	struct rcar_can_priv *priv = netdev_priv(ndev);
 	u16 ctlr;
 
-	if (netif_running(ndev)) {
-		netif_stop_queue(ndev);
-		netif_device_detach(ndev);
-	}
+	if (!netif_running(ndev))
+		return 0;
+
+	netif_stop_queue(ndev);
+	netif_device_detach(ndev);
+
 	ctlr = readw(&priv->regs->ctlr);
 	ctlr |= RCAR_CAN_CTLR_CANM_HALT;
 	writew(ctlr, &priv->regs->ctlr);
@@ -858,6 +860,7 @@ static int __maybe_unused rcar_can_suspend(struct device *dev)
 	priv->can.state = CAN_STATE_SLEEPING;
 
 	clk_disable(priv->clk);
+
 	return 0;
 }
 
@@ -868,6 +871,9 @@ static int __maybe_unused rcar_can_resume(struct device *dev)
 	u16 ctlr;
 	int err;
 
+	if (!netif_running(ndev))
+		return 0;
+
 	err = clk_enable(priv->clk);
 	if (err) {
 		netdev_err(ndev, "clk_enable() failed, error %d\n", err);
@@ -881,10 +887,9 @@ static int __maybe_unused rcar_can_resume(struct device *dev)
 	writew(ctlr, &priv->regs->ctlr);
 	priv->can.state = CAN_STATE_ERROR_ACTIVE;
 
-	if (netif_running(ndev)) {
-		netif_device_attach(ndev);
-		netif_start_queue(ndev);
-	}
+	netif_device_attach(ndev);
+	netif_start_queue(ndev);
+
 	return 0;
 }
 
-- 
2.25.1

Re: [PATCH] can: rcar_can: Fix suspend/resume

From: Ulrich Hecht <hidden>
Date: 2021-09-21 10:38:20

quoted hunk
On 09/21/2021 7:19 AM Yoshihiro Shimoda [off-list ref] wrote:

 
If the driver was not opened, rcar_can_suspend() should not call
clk_disable() because the clock was not enabled.

Fixes: fd1159318e55 ("can: add Renesas R-Car CAN driver")
Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Tested-by: Ayumi Nakamichi <redacted>
---
 drivers/net/can/rcar/rcar_can.c | 21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)
diff --git a/drivers/net/can/rcar/rcar_can.c b/drivers/net/can/rcar/rcar_can.c
index 00e4533c8bdd..6b4eefb03044 100644
--- a/drivers/net/can/rcar/rcar_can.c
+++ b/drivers/net/can/rcar/rcar_can.c
@@ -846,10 +846,12 @@ static int __maybe_unused rcar_can_suspend(struct device *dev)
 	struct rcar_can_priv *priv = netdev_priv(ndev);
 	u16 ctlr;
 
-	if (netif_running(ndev)) {
-		netif_stop_queue(ndev);
-		netif_device_detach(ndev);
-	}
+	if (!netif_running(ndev))
+		return 0;
+
+	netif_stop_queue(ndev);
+	netif_device_detach(ndev);
+
 	ctlr = readw(&priv->regs->ctlr);
 	ctlr |= RCAR_CAN_CTLR_CANM_HALT;
 	writew(ctlr, &priv->regs->ctlr);
@@ -858,6 +860,7 @@ static int __maybe_unused rcar_can_suspend(struct device *dev)
 	priv->can.state = CAN_STATE_SLEEPING;
 
 	clk_disable(priv->clk);
+
 	return 0;
 }
 
@@ -868,6 +871,9 @@ static int __maybe_unused rcar_can_resume(struct device *dev)
 	u16 ctlr;
 	int err;
 
+	if (!netif_running(ndev))
+		return 0;
+
 	err = clk_enable(priv->clk);
 	if (err) {
 		netdev_err(ndev, "clk_enable() failed, error %d\n", err);
@@ -881,10 +887,9 @@ static int __maybe_unused rcar_can_resume(struct device *dev)
 	writew(ctlr, &priv->regs->ctlr);
 	priv->can.state = CAN_STATE_ERROR_ACTIVE;
 
-	if (netif_running(ndev)) {
-		netif_device_attach(ndev);
-		netif_start_queue(ndev);
-	}
+	netif_device_attach(ndev);
+	netif_start_queue(ndev);
+
 	return 0;
 }
 
-- 
2.25.1
Reviewed-by: Ulrich Hecht <redacted>

CU
Uli

Re: [PATCH] can: rcar_can: Fix suspend/resume

From: Simon Horman <horms@kernel.org>
Date: 2021-09-22 15:23:41

On Tue, Sep 21, 2021 at 12:38:13PM +0200, Ulrich Hecht wrote:
quoted
On 09/21/2021 7:19 AM Yoshihiro Shimoda [off-list ref] wrote:

 
If the driver was not opened, rcar_can_suspend() should not call
clk_disable() because the clock was not enabled.

Fixes: fd1159318e55 ("can: add Renesas R-Car CAN driver")
Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Tested-by: Ayumi Nakamichi <redacted>
---
 drivers/net/can/rcar/rcar_can.c | 21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)
diff --git a/drivers/net/can/rcar/rcar_can.c b/drivers/net/can/rcar/rcar_can.c
index 00e4533c8bdd..6b4eefb03044 100644
--- a/drivers/net/can/rcar/rcar_can.c
+++ b/drivers/net/can/rcar/rcar_can.c
...
quoted
@@ -858,6 +860,7 @@ static int __maybe_unused rcar_can_suspend(struct device *dev)
 	priv->can.state = CAN_STATE_SLEEPING;
 
 	clk_disable(priv->clk);
+
 	return 0;
 }
 
nit: this hunk seems unrelated to the rest of the patch

...

RE: [PATCH] can: rcar_can: Fix suspend/resume

From: Biju Das <biju.das.jz@bp.renesas.com>
Date: 2021-09-23 13:31:33

Hi Shimoda-San,

Thanks for the patch. I have tested Wake on LAN(Magic packet) on Hihope RZ/G2M board
and this patches fixes the warning messages reported by kernel during suspend.

Tested-by: Biju Das <biju.das.jz@bp.renesas.com>


Before applying patch
------------------------
root@hihope-rzg2m:~# ethtool -s eth0 wol g
root@hihope-rzg2m:~# echo mem > /sys/power/state
[   92.220742] PM: suspend entry (s2idle)
[   92.228123] Filesystems sync: 0.003 seconds
[   92.248521] Freezing user space processes ... (elapsed 0.007 seconds) done.
[   92.263935] OOM killer disabled.
[   92.267265] Freezing remaining freezable tasks ... (elapsed 0.004 seconds) done.
[   92.279787] printk: Suspending console(s) (use no_console_suspend to debug)
[   92.310223] wl1271_sdio mmc2:0001:2: no wilink module was probed
[   92.541847] ------------[ cut here ]------------
[   92.541885] can-if1 already disabled
[   92.541932] WARNING: CPU: 0 PID: 480 at drivers/clk/clk.c:952 clk_core_disable+0x250/0x288
[   92.541962] CPU: 0 PID: 480 Comm: sh Not tainted 5.15.0-rc1-arm64-renesas-00402-gf0c16ee77cf1 #297
[   92.541973] Hardware name: HopeRun HiHope RZ/G2M with sub board (DT)
[   92.541981] pstate: 800000c5 (Nzcv daIF -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[   92.541990] pc : clk_core_disable+0x250/0x288
[   92.542000] lr : clk_core_disable+0x250/0x288
[   92.542008] sp : ffff80001263b980
[   92.542014] x29: ffff80001263b980 x28: ffff8000114ea508 x27: 0000000000000000
[   92.542037] x26: ffff8000113b0b48 x25: ffff80001115f000 x24: ffff8000114ea518
[   92.542057] x23: ffff0005c0b5f010 x22: ffff8000113b0b48 x21: 0000000000000000
[   92.542077] x20: ffff0005c0d09800 x19: ffff0005c0d09800 x18: 000000000000b948
[   92.542096] x17: 0000000000000000 x16: 000000000000b94e x15: ffff8000115c5e68
[   92.542116] x14: fffffffffffc63d7 x13: 0000000000000003 x12: ffff8000113d3e58
[   92.542135] x11: 0000000000000001 x10: 00000000000000f0 x9 : ffff8000111619e4
[   92.542154] x8 : ffff0005c67a4380 x7 : 0000000000000002 x6 : 0000000000000000
[   92.542173] x5 : ffff00063f727bf0 x4 : 0000000000000000 x3 : 0000000000000027
[   92.542192] x2 : 0000000000000023 x1 : 2b864f21865f9400 x0 : 0000000000000000
[   92.542212] Call trace:
[   92.542218]  clk_core_disable+0x250/0x288
[   92.542227]  clk_core_disable_lock+0x20/0x38
[   92.542236]  clk_disable+0x1c/0x28
[   92.542244]  rcar_can_suspend+0x74/0xb0
[   92.542257]  dpm_run_callback+0x5c/0x2b8
[   92.542268]  __device_suspend+0x110/0x5d8
[   92.542276]  dpm_suspend+0x124/0x438
[   92.542284]  dpm_suspend_start+0x78/0x90
[   92.542292]  suspend_devices_and_enter+0x10c/0xbb0
[   92.542304]  pm_suspend+0x274/0x338
[   92.542312]  state_store+0x88/0x110
[   92.542321]  kobj_attr_store+0x14/0x28
[   92.542335]  sysfs_kf_write+0x48/0x70
[   92.542345]  kernfs_fop_write_iter+0x118/0x1a8
[   92.542352]  new_sync_write+0xe4/0x180
[   92.542363]  vfs_write+0x29c/0x468
[   92.542371]  ksys_write+0x68/0xf0
[   92.542379]  __arm64_sys_write+0x18/0x20
[   92.542387]  invoke_syscall+0x40/0xf8
[   92.542398]  el0_svc_common.constprop.0+0xf0/0x110
[   92.542407]  do_el0_svc+0x20/0x78
[   92.542414]  el0_svc+0x50/0xe0
[   92.542423]  el0t_64_sync_handler+0xa8/0xb0
[   92.542431]  el0t_64_sync+0x158/0x15c
[   92.542440] irq event stamp: 46832
[   92.542445] hardirqs last  enabled at (46831): [<ffff800010c960dc>] _raw_spin_unlock_irqrestore+0x8c/0x90
[   92.542459] hardirqs last disabled at (46832): [<ffff8000105b4964>] clk_enable_lock+0xcc/0x118
[   92.542469] softirqs last  enabled at (42392): [<ffff800010010464>] _stext+0x464/0x5d8
[   92.542477] softirqs last disabled at (42387): [<ffff80001008f480>] irq_exit+0x198/0x1b8
[   92.542490] ---[ end trace e2078d4610595648 ]---
[   92.542527] ------------[ cut here ]------------
[   92.542532] can-if0 already disabled
[   92.542556] WARNING: CPU: 0 PID: 480 at drivers/clk/clk.c:952 clk_core_disable+0x250/0x288
[   92.542569] CPU: 0 PID: 480 Comm: sh Tainted: G        W         5.15.0-rc1-arm64-renesas-00402-gf0c16ee77cf1 #297
[   92.542578] Hardware name: HopeRun HiHope RZ/G2M with sub board (DT)
[   92.542583] pstate: 800000c5 (Nzcv daIF -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[   92.542590] pc : clk_core_disable+0x250/0x288
[   92.542598] lr : clk_core_disable+0x250/0x288
[   92.542606] sp : ffff80001263b980
[   92.542611] x29: ffff80001263b980 x28: ffff8000114ea508 x27: 0000000000000000
[   92.542631] x26: ffff8000113b0b48 x25: ffff80001115f000 x24: ffff8000114ea518
[   92.542650] x23: ffff0005c0b5d810 x22: ffff8000113b0b48 x21: 0000000000000000
[   92.542669] x20: ffff0005c0d09900 x19: ffff0005c0d09900 x18: 000000000000b948
[   92.542688] x17: 0000000000000000 x16: 000000000000b94e x15: ffff8000115c5e68
[   92.542707] x14: fffffffffffc6ec7 x13: 0000000000000003 x12: ffff8000113d3e58
[   92.542726] x11: 0000000000000001 x10: 00000000000000f0 x9 : ffff8000111619e4
[   92.542745] x8 : ffff0005c67a4380 x7 : 0000000000000002 x6 : 0000000000000000
[   92.542763] x5 : ffff00063f727bf0 x4 : 0000000000000000 x3 : 0000000000000027
[   92.542782] x2 : 0000000000000023 x1 : 2b864f21865f9400 x0 : 0000000000000000
[   92.542800] Call trace:
[   92.542806]  clk_core_disable+0x250/0x288
[   92.542814]  clk_core_disable_lock+0x20/0x38
[   92.542822]  clk_disable+0x1c/0x28
[   92.542828]  rcar_can_suspend+0x74/0xb0
[   92.542837]  dpm_run_callback+0x5c/0x2b8
[   92.542845]  __device_suspend+0x110/0x5d8
[   92.542852]  dpm_suspend+0x124/0x438
[   92.542860]  dpm_suspend_start+0x78/0x90
[   92.542868]  suspend_devices_and_enter+0x10c/0xbb0
[   92.542876]  pm_suspend+0x274/0x338
[   92.542884]  state_store+0x88/0x110
[   92.542892]  kobj_attr_store+0x14/0x28
[   92.542901]  sysfs_kf_write+0x48/0x70
[   92.542908]  kernfs_fop_write_iter+0x118/0x1a8
[   92.542915]  new_sync_write+0xe4/0x180
[   92.542923]  vfs_write+0x29c/0x468
[   92.542930]  ksys_write+0x68/0xf0
[   92.542938]  __arm64_sys_write+0x18/0x20
[   92.542945]  invoke_syscall+0x40/0xf8
[   92.542954]  el0_svc_common.constprop.0+0xf0/0x110
[   92.542962]  do_el0_svc+0x20/0x78
[   92.542969]  el0_svc+0x50/0xe0
[   92.542977]  el0t_64_sync_handler+0xa8/0xb0
[   92.542984]  el0t_64_sync+0x158/0x15c
[   92.542991] irq event stamp: 46852
[   92.542995] hardirqs last  enabled at (46851): [<ffff800010c960dc>] _raw_spin_unlock_irqrestore+0x8c/0x90
[   92.543004] hardirqs last disabled at (46852): [<ffff8000105b4964>] clk_enable_lock+0xcc/0x118
[   92.543013] softirqs last  enabled at (42392): [<ffff800010010464>] _stext+0x464/0x5d8
[   92.543020] softirqs last disabled at (42387): [<ffff80001008f480>] irq_exit+0x198/0x1b8
[   92.543029] ---[ end trace e2078d4610595649 ]---
[  148.223753] ravb e6800000.ethernet eth0: Link is Down
[  148.242725] RTL8211E Gigabit Ethernet e6800000.ethernet-ffffffff:00: attached PHY driver (mii_bus:phy_addr=e6800000.ethernet-ffffffff:00, irq=199)
[  148.809790] OOM killer enabled.
[  148.813013] Restarting tasks ... done.
[  148.824071] PM: suspend exit
[  152.628415] ravb e6800000.ethernet eth0: Link is Up - 1Gbps/Full - flow control off
root@hihope-rzg2m:~#

After applying the patch:-
-------------------------

root@hihope-rzg2m:~# ethtool -s eth0 wol g
root@hihope-rzg2m:~# echo mem > /sys/power/state

[   38.607753] PM: suspend entry (s2idle)
[   38.616233] Filesystems sync: 0.004 seconds
[   38.638404] Freezing user space processes ... (elapsed 0.007 seconds) done.
[   38.654476] OOM killer disabled.
[   38.657879] Freezing remaining freezable tasks ... (elapsed 0.004 seconds) done.
[   38.670526] printk: Suspending console(s) (use no_console_suspend to debug)
[   38.698823] wl1271_sdio mmc2:0001:2: no wilink module was probed
[   45.828031] ravb e6800000.ethernet eth0: Link is Down
[   45.846544] RTL8211E Gigabit Ethernet e6800000.ethernet-ffffffff:00: attached PHY driver (mii_bus:phy_addr=e6800000.ethernet-ffffffff:00, irq=199)
[   45.888434] OOM killer enabled.
[   45.891614] Restarting tasks ... done.
[   45.901692] PM: suspend exit
root@hihope-rzg2m:~#
root@hihope-rzg2m:~# [   49.468617] ravb e6800000.ethernet eth0: Link is Up - 1Gbps/Full - flow control off

regards,
Biju


 
quoted hunk
-----Original Message-----
From: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Sent: 21 September 2021 06:20
To: wg@grandegger.com; mkl@pengutronix.de
Cc: davem@davemloft.net; kuba@kernel.org; linux-can@vger.kernel.org;
netdev@vger.kernel.org; linux-renesas-soc@vger.kernel.org; Yoshihiro
Shimoda [off-list ref]; Ayumi Nakamichi
[off-list ref]
Subject: [PATCH] can: rcar_can: Fix suspend/resume

If the driver was not opened, rcar_can_suspend() should not call
clk_disable() because the clock was not enabled.

Fixes: fd1159318e55 ("can: add Renesas R-Car CAN driver")
Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Tested-by: Ayumi Nakamichi <redacted>
---
 drivers/net/can/rcar/rcar_can.c | 21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)
diff --git a/drivers/net/can/rcar/rcar_can.c
b/drivers/net/can/rcar/rcar_can.c index 00e4533c8bdd..6b4eefb03044 100644
--- a/drivers/net/can/rcar/rcar_can.c
+++ b/drivers/net/can/rcar/rcar_can.c
@@ -846,10 +846,12 @@ static int __maybe_unused rcar_can_suspend(struct
device *dev)
 	struct rcar_can_priv *priv = netdev_priv(ndev);
 	u16 ctlr;

-	if (netif_running(ndev)) {
-		netif_stop_queue(ndev);
-		netif_device_detach(ndev);
-	}
+	if (!netif_running(ndev))
+		return 0;
+
+	netif_stop_queue(ndev);
+	netif_device_detach(ndev);
+
 	ctlr = readw(&priv->regs->ctlr);
 	ctlr |= RCAR_CAN_CTLR_CANM_HALT;
 	writew(ctlr, &priv->regs->ctlr);
@@ -858,6 +860,7 @@ static int __maybe_unused rcar_can_suspend(struct
device *dev)
 	priv->can.state = CAN_STATE_SLEEPING;

 	clk_disable(priv->clk);
+
 	return 0;
 }
@@ -868,6 +871,9 @@ static int __maybe_unused rcar_can_resume(struct
device *dev)
 	u16 ctlr;
 	int err;

+	if (!netif_running(ndev))
+		return 0;
+
 	err = clk_enable(priv->clk);
 	if (err) {
 		netdev_err(ndev, "clk_enable() failed, error %d\n", err); @@ -
881,10 +887,9 @@ static int __maybe_unused rcar_can_resume(struct device
*dev)
 	writew(ctlr, &priv->regs->ctlr);
 	priv->can.state = CAN_STATE_ERROR_ACTIVE;

-	if (netif_running(ndev)) {
-		netif_device_attach(ndev);
-		netif_start_queue(ndev);
-	}
+	netif_device_attach(ndev);
+	netif_start_queue(ndev);
+
 	return 0;
 }

--
2.25.1

RE: [PATCH] can: rcar_can: Fix suspend/resume

From: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Date: 2021-09-24 07:43:47

Hi Simon-san,
From: Simon Horman, Sent: Thursday, September 23, 2021 12:24 AM

On Tue, Sep 21, 2021 at 12:38:13PM +0200, Ulrich Hecht wrote:
quoted
quoted
On 09/21/2021 7:19 AM Yoshihiro Shimoda [off-list ref] wrote:


If the driver was not opened, rcar_can_suspend() should not call
clk_disable() because the clock was not enabled.

Fixes: fd1159318e55 ("can: add Renesas R-Car CAN driver")
Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Tested-by: Ayumi Nakamichi <redacted>
---
 drivers/net/can/rcar/rcar_can.c | 21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)
diff --git a/drivers/net/can/rcar/rcar_can.c b/drivers/net/can/rcar/rcar_can.c
index 00e4533c8bdd..6b4eefb03044 100644
--- a/drivers/net/can/rcar/rcar_can.c
+++ b/drivers/net/can/rcar/rcar_can.c
...
quoted
quoted
@@ -858,6 +860,7 @@ static int __maybe_unused rcar_can_suspend(struct device *dev)
 	priv->can.state = CAN_STATE_SLEEPING;

 	clk_disable(priv->clk);
+
 	return 0;
 }
nit: this hunk seems unrelated to the rest of the patch
Thank you for your comment! I'll remove this.

Best regards,
Yoshihiro Shimda
...
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help