| Message ID | 20210921051959.50309-1-yoshihiro.shimoda.uh@renesas.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
| Series | can: rcar_can: Fix suspend/resume | expand |
> On 09/21/2021 7:19 AM Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> 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 <ayumi.nakamichi.kf@renesas.com> > --- > 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 <uli+renesas@fpond.eu> CU Uli
On Tue, Sep 21, 2021 at 12:38:13PM +0200, Ulrich Hecht wrote: > > > On 09/21/2021 7:19 AM Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> 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 <ayumi.nakamichi.kf@renesas.com> > > --- > > 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 ... > > @@ -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 ...
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 > -----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 <yoshihiro.shimoda.uh@renesas.com>; Ayumi Nakamichi > <ayumi.nakamichi.kf@renesas.com> > 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 <ayumi.nakamichi.kf@renesas.com> > --- > 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
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: > > > > > On 09/21/2021 7:19 AM Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> 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 <ayumi.nakamichi.kf@renesas.com> > > > --- > > > 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 > > ... > > > > @@ -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 > ...
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; }
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 <ayumi.nakamichi.kf@renesas.com> --- drivers/net/can/rcar/rcar_can.c | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-)