| Message ID | 20220830070148.2021947-1-wei.fang@nxp.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 7d650df99d528f674cc744719a00a20be1f912f8 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net] net: fec: add pm_qos support on imx6q platform | expand |
On Tue, 30 Aug 2022 15:01:48 +0800 wei.fang@nxp.com wrote: > There is a very low probability that tx timeout will occur during > suspend and resume stress test on imx6q platform. So we add pm_qos > support to prevent system from entering low level idles which may > affect the transmission of tx. Any more info on the issue? Is there an errata for it? What's the expected power consumption change?
> -----Original Message----- > From: Jakub Kicinski <kuba@kernel.org> > Sent: 2022年9月1日 10:36 > To: Wei Fang <wei.fang@nxp.com> > Cc: davem@davemloft.net; edumazet@google.com; pabeni@redhat.com; > netdev@vger.kernel.org; linux-kernel@vger.kernel.org > Subject: Re: [PATCH net] net: fec: add pm_qos support on imx6q platform > > On Tue, 30 Aug 2022 15:01:48 +0800 wei.fang@nxp.com wrote: > > There is a very low probability that tx timeout will occur during > > suspend and resume stress test on imx6q platform. So we add pm_qos > > support to prevent system from entering low level idles which may > > affect the transmission of tx. > > Any more info on the issue? Is there an errata for it? > What's the expected power consumption change? On imx6q platform, cpuidle has some levels which may disable system/bus clocks, this may cause tx packets can not be sent in time and the ndo_tx_timeout() will be called. Seeing the following console logs for details. [ 5170.028381] ------------[ cut here ]------------ [ 5170.033795] WARNING: CPU: 0 PID: 0 at net/sched/sch_generic.c:454 dev_watchdog+0x394/0x3c8 [ 5170.042153] NETDEV WATCHDOG: eth0 (fec): transmit queue 0 timed out [ 5170.048494] Modules linked in: snvs_ui(O) mxc_v4l2_capture ipu_bg_overlay_sdc ipu_still ipu_prp_enc ipu_csi_enc ipu_fg_overlay_sdc imx_vdoa ov5640_camera_mipi_int ov5640_camera_int v4l2_int_device galcore(O) [last unloaded: snvs_ui] [ 5170.069267] CPU: 0 PID: 0 Comm: swapper/0 Tainted: G O 5.4.0-rc7-5.4-zeus-next+g56a9ca3b7f4e #1 [ 5170.079313] Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree) [ 5170.085920] [<c0112e30>] (unwind_backtrace) from [<c010cdc0>] (show_stack+0x10/0x14) [ 5170.093724] [<c010cdc0>] (show_stack) from [<c0d2a73c>] (dump_stack+0xe0/0x114) [ 5170.101089] [<c0d2a73c>] (dump_stack) from [<c0137380>] (__warn+0xc0/0x10c) [ 5170.108104] [<c0137380>] (__warn) from [<c0137780>] (warn_slowpath_fmt+0x90/0xc0) [ 5170.115644] [<c0137780>] (warn_slowpath_fmt) from [<c0ab1730>] (dev_watchdog+0x394/0x3c8) [ 5170.123877] [<c0ab1730>] (dev_watchdog) from [<c01c47fc>] (call_timer_fn+0xc0/0x320) [ 5170.131672] [<c01c47fc>] (call_timer_fn) from [<c01c5114>] (run_timer_softirq+0x210/0x688) [ 5170.139991] [<c01c5114>] (run_timer_softirq) from [<c0102314>] (__do_softirq+0xf4/0x50c) [ 5170.148139] [<c0102314>] (__do_softirq) from [<c013f094>] (irq_exit+0x128/0x180) [ 5170.155595] [<c013f094>] (irq_exit) from [<c01a1420>] (__handle_domain_irq+0x6c/0xdc) [ 5170.163492] [<c01a1420>] (__handle_domain_irq) from [<c054bcd0>] (gic_handle_irq+0x48/0x9c) [ 5170.171900] [<c054bcd0>] (gic_handle_irq) from [<c0101a70>] (__irq_svc+0x70/0x98) [ 5170.178527] regulator regulator.5: Failed to increase supply voltage: -110 [ 5170.179418] Exception stack(0xc1401ed8 to 0xc1401f20) [ 5170.179519] 1ec0: 00000001 00000006 [ 5170.179546] 1ee0: 00000000 c140c600 00000000 af6bbd9b c140f934 db6ebdb8 c1504668 000004b3 [ 5170.179572] 1f00: 000004b3 bdb0c416 00000000 c1401f28 c0194520 c0938fcc 20010013 ffffffff [ 5170.216432] [<c0101a70>] (__irq_svc) from [<c0938fcc>] (cpuidle_enter_state+0x16c/0x520) [ 5170.224584] [<c0938fcc>] (cpuidle_enter_state) from [<c09393bc>] (cpuidle_enter+0x28/0x38) [ 5170.232913] [<c09393bc>] (cpuidle_enter) from [<c016f6a4>] (do_idle+0x1dc/0x2b0) [ 5170.240367] [<c016f6a4>] (do_idle) from [<c016fb2c>] (cpu_startup_entry+0x18/0x1c) [ 5170.248002] [<c016fb2c>] (cpu_startup_entry) from [<c1300e20>] (start_kernel+0x404/0x4cc) [ 5170.256598] irq event stamp: 51412072 [ 5170.260390] hardirqs last enabled at (51412084): [<c0101a74>] __irq_svc+0x74/0x98 [ 5170.268076] hardirqs last disabled at (51412095): [<c0101a60>] __irq_svc+0x60/0x98 [ 5170.275767] softirqs last enabled at (51411992): [<c013ef50>] irq_enter+0x68/0x84 [ 5170.283455] softirqs last disabled at (51411993): [<c013f094>] irq_exit+0x128/0x180 [ 5170.291217] ---[ end trace 781c3e037025657e ]--- [ 5170.295951] fec 2188000.ethernet eth0: TX ring dump [ 5170.300930] Nr SC addr len SKB [ 5170.305398] 0 0x0400 0x00000000 66 322bbb12 [ 5170.310296] 1 0x0400 0x00000000 124 322bbb12 [ 5170.315139] 2 0x0400 0x00000000 112 322bbb12 [ 5170.319957] 3 H 0x1c00 0x00000000 112 322bbb12 [ 5170.324775] 4 0x1c00 0x28933800 130 2734ee11 [ 5170.329593] 5 0x1c00 0x28934000 130 f36d5b3e [ 5170.334410] 6 0x0400 0x28934800 66 322bbb12 [ 5170.339227] 7 0x0400 0x29030000 124 322bbb12 [ 5170.344097] 8 0x0400 0x29a48000 112 322bbb12 [ 5170.348997] 9 0x1c00 0x294e8000 112 dbee6d93 [ 5170.353895] 10 0x0400 0x28936800 66 322bbb12 [ 5170.358791] 11 0x0400 0x294e8070 112 322bbb12 [ 5170.363687] 12 0x0400 0x29d40000 112 322bbb12 [ 5170.368585] 13 0x0400 0x29250000 112 322bbb12 [ 5170.373483] 14 0x0400 0x29b38000 112 322bbb12 [ 5170.378350] 15 0x0400 0x29bd0000 112 322bbb12 [ 5170.383216] 16 0x0400 0x29110000 112 322bbb12 [ 5170.388083] 17 0x0400 0x29590000 112 322bbb12 So we add pm_qos to prevent cpuidle from entering low level idles and make sure system/bus clocks are enabled. In terms of power consumption changes, we did not measure the change, but there will be some increase.
On Tue, 2022-08-30 at 15:01 +0800, wei.fang@nxp.com wrote: > From: Wei Fang <wei.fang@nxp.com> > > There is a very low probability that tx timeout will occur during > suspend and resume stress test on imx6q platform. So we add pm_qos > support to prevent system from entering low level idles which may > affect the transmission of tx. > > Signed-off-by: Wei Fang <wei.fang@nxp.com> Since this IMHO causes a significal behavior change I suggest to target the net-next tree, does that fit you? Additionally, it would be great if you could provide in the changelog the references to the relevant platform documentation and (even rough) power consumption delta estimates. Thanks! Paolo
On Thu, 01 Sep 2022 09:17:37 +0200 Paolo Abeni wrote: > On Tue, 2022-08-30 at 15:01 +0800, wei.fang@nxp.com wrote: > > From: Wei Fang <wei.fang@nxp.com> > > > > There is a very low probability that tx timeout will occur during > > suspend and resume stress test on imx6q platform. So we add pm_qos > > support to prevent system from entering low level idles which may > > affect the transmission of tx. > > > > Signed-off-by: Wei Fang <wei.fang@nxp.com> > > Since this IMHO causes a significal behavior change I suggest to target > the net-next tree, does that fit you? > > Additionally, it would be great if you could provide in the changelog > the references to the relevant platform documentation and (even rough) > power consumption delta estimates. It's a tricky one, we don't want older kernels to potentially hang either. IIRC Florian did some WoL extensions for BRCM, maybe he has the right experience. Florian, what would you recommend? net or net-next?
> -----Original Message----- > From: Paolo Abeni <pabeni@redhat.com> > Sent: 2022年9月1日 15:18 > To: Wei Fang <wei.fang@nxp.com>; davem@davemloft.net; > edumazet@google.com; kuba@kernel.org > Cc: netdev@vger.kernel.org; linux-kernel@vger.kernel.org > Subject: Re: [PATCH net] net: fec: add pm_qos support on imx6q platform > > On Tue, 2022-08-30 at 15:01 +0800, wei.fang@nxp.com wrote: > > From: Wei Fang <wei.fang@nxp.com> > > > > There is a very low probability that tx timeout will occur during > > suspend and resume stress test on imx6q platform. So we add pm_qos > > support to prevent system from entering low level idles which may > > affect the transmission of tx. > > > > Signed-off-by: Wei Fang <wei.fang@nxp.com> > > Since this IMHO causes a significal behavior change I suggest to target the > net-next tree, does that fit you? > In my opinion, the patch is to fix a bug rather than add a new feature, so I think it should be net tree. But it's fine that if the majority think the net-next is more suitable. > Additionally, it would be great if you could provide in the changelog the > references to the relevant platform documentation and (even rough) power > consumption delta estimates. >
Hello: This patch was applied to netdev/net.git (master) by Jakub Kicinski <kuba@kernel.org>: On Tue, 30 Aug 2022 15:01:48 +0800 you wrote: > From: Wei Fang <wei.fang@nxp.com> > > There is a very low probability that tx timeout will occur during > suspend and resume stress test on imx6q platform. So we add pm_qos > support to prevent system from entering low level idles which may > affect the transmission of tx. > > [...] Here is the summary with links: - [net] net: fec: add pm_qos support on imx6q platform https://git.kernel.org/netdev/net/c/7d650df99d52 You are awesome, thank you!
diff --git a/drivers/net/ethernet/freescale/fec.h b/drivers/net/ethernet/freescale/fec.h index ed7301b69169..a5fed00cb971 100644 --- a/drivers/net/ethernet/freescale/fec.h +++ b/drivers/net/ethernet/freescale/fec.h @@ -16,6 +16,7 @@ #include <linux/clocksource.h> #include <linux/net_tstamp.h> +#include <linux/pm_qos.h> #include <linux/ptp_clock_kernel.h> #include <linux/timecounter.h> @@ -498,6 +499,9 @@ struct bufdesc_ex { /* i.MX8MQ SoC integration mix wakeup interrupt signal into "int2" interrupt line. */ #define FEC_QUIRK_WAKEUP_FROM_INT2 (1 << 22) +/* i.MX6Q adds pm_qos support */ +#define FEC_QUIRK_HAS_PMQOS BIT(23) + struct bufdesc_prop { int qid; /* Address of Rx and Tx buffers */ @@ -608,6 +612,7 @@ struct fec_enet_private { struct delayed_work time_keep; struct regulator *reg_phy; struct fec_stop_mode_gpr stop_gpr; + struct pm_qos_request pm_qos_req; unsigned int tx_align; unsigned int rx_align; diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index f5f34cdba131..bcc441d9a499 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -111,7 +111,8 @@ static const struct fec_devinfo fec_imx6q_info = { .quirks = FEC_QUIRK_ENET_MAC | FEC_QUIRK_HAS_GBIT | FEC_QUIRK_HAS_BUFDESC_EX | FEC_QUIRK_HAS_CSUM | FEC_QUIRK_HAS_VLAN | FEC_QUIRK_ERR006358 | - FEC_QUIRK_HAS_RACC | FEC_QUIRK_CLEAR_SETUP_MII, + FEC_QUIRK_HAS_RACC | FEC_QUIRK_CLEAR_SETUP_MII | + FEC_QUIRK_HAS_PMQOS, }; static const struct fec_devinfo fec_mvf600_info = { @@ -3218,6 +3219,9 @@ fec_enet_open(struct net_device *ndev) if (fep->quirks & FEC_QUIRK_ERR006687) imx6q_cpuidle_fec_irqs_used(); + if (fep->quirks & FEC_QUIRK_HAS_PMQOS) + cpu_latency_qos_add_request(&fep->pm_qos_req, 0); + napi_enable(&fep->napi); phy_start(ndev->phydev); netif_tx_start_all_queues(ndev); @@ -3259,6 +3263,9 @@ fec_enet_close(struct net_device *ndev) fec_enet_update_ethtool_stats(ndev); fec_enet_clk_enable(ndev, false); + if (fep->quirks & FEC_QUIRK_HAS_PMQOS) + cpu_latency_qos_remove_request(&fep->pm_qos_req); + pinctrl_pm_select_sleep_state(&fep->pdev->dev); pm_runtime_mark_last_busy(&fep->pdev->dev); pm_runtime_put_autosuspend(&fep->pdev->dev);