Messages in this thread Patch in this message | | | Subject | Re: [PATCH] PM / devfreq: passive: Update frequency when start governor | From | Marek Szyprowski <> | Date | Mon, 9 Nov 2020 10:22:31 +0100 |
| |
Hi Chanwoo,
On 03.11.2020 08:06, Chanwoo Choi wrote: > If the parent device changes the their frequency before registering > the passive device, the passive device cannot receive the notification > from parent device and then the passive device cannot be able to > set the proper frequency according to the frequency of parent device. > > So, when start the passive governor, update the frequency > according to the frequency of parent device. > > Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
This patch landed in linux next-20201109 as commit 9b38a4f6b664 ("PM / devfreq: passive: Update frequency when start governor"). Sadly it causes the following warning on various Exynos boards with default exynos_defconfig build:
exynos-bus: new bus device registered: soc:bus_dmc ( 50000 KHz ~ 400000 KHz) exynos-bus: new bus device registered: soc:bus_leftbus ( 50000 KHz ~ 200000 KHz) ------------[ cut here ]------------ WARNING: CPU: 1 PID: 22 at drivers/devfreq/devfreq.c:411 devfreq_update_target+0xdc/0xec Modules linked in: CPU: 1 PID: 22 Comm: kworker/1:1 Not tainted 5.10.0-rc2-next-20201109 #1898 Hardware name: Samsung Exynos (Flattened Device Tree) Workqueue: events deferred_probe_work_func [<c0111648>] (unwind_backtrace) from [<c010d000>] (show_stack+0x10/0x14) [<c010d000>] (show_stack) from [<c0b224a0>] (dump_stack+0xb4/0xd4) [<c0b224a0>] (dump_stack) from [<c0126844>] (__warn+0xd8/0x11c) [<c0126844>] (__warn) from [<c0126938>] (warn_slowpath_fmt+0xb0/0xb8) [<c0126938>] (warn_slowpath_fmt) from [<c08ae39c>] (devfreq_update_target+0xdc/0xec) [<c08ae39c>] (devfreq_update_target) from [<c08afa0c>] (devfreq_passive_event_handler+0x80/0xc4) [<c08afa0c>] (devfreq_passive_event_handler) from [<c08ade70>] (devfreq_add_device+0x4f8/0x5a0) [<c08ade70>] (devfreq_add_device) from [<c08adf60>] (devm_devfreq_add_device+0x48/0x84) [<c08adf60>] (devm_devfreq_add_device) from [<c08b01c8>] (exynos_bus_probe+0x2ac/0x5c8) [<c08b01c8>] (exynos_bus_probe) from [<c0692a70>] (platform_drv_probe+0x6c/0xa4) [<c0692a70>] (platform_drv_probe) from [<c0690020>] (really_probe+0x200/0x4fc) [<c0690020>] (really_probe) from [<c06904e4>] (driver_probe_device+0x78/0x1fc) [<c06904e4>] (driver_probe_device) from [<c068de7c>] (bus_for_each_drv+0x74/0xb8) [<c068de7c>] (bus_for_each_drv) from [<c068fd80>] (__device_attach+0xe4/0x17c) [<c068fd80>] (__device_attach) from [<c068ee40>] (bus_probe_device+0x88/0x90) [<c068ee40>] (bus_probe_device) from [<c068f358>] (deferred_probe_work_func+0x3c/0xd0) [<c068f358>] (deferred_probe_work_func) from [<c0148a70>] (process_one_work+0x234/0x7e4) [<c0148a70>] (process_one_work) from [<c0149064>] (worker_thread+0x44/0x51c) [<c0149064>] (worker_thread) from [<c014fdc8>] (kthread+0x158/0x1a0) [<c014fdc8>] (kthread) from [<c010011c>] (ret_from_fork+0x14/0x38) Exception stack(0xc1e27fb0 to 0xc1e27ff8) 7fa0: 00000000 00000000 00000000 00000000 7fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 7fe0: 00000000 00000000 00000000 00000000 00000013 00000000 irq event stamp: 12147 hardirqs last enabled at (12171): [<c01a0358>] console_unlock+0x430/0x6b0 dwc2 12480000.hsotg: new device is high-speed hardirqs last disabled at (12176): [<c01a0350>] console_unlock+0x428/0x6b0 softirqs last enabled at (12168): [<c01017a8>] __do_softirq+0x528/0x674 softirqs last disabled at (12155): [<c012fdb0>] irq_exit+0x1dc/0x1e8 ---[ end trace 89c1007ec29b9250 ]---
Reverting it on top of the linux-next fixes the warning, but I assume that the proper fix would require locking changes during the devfreq registration.
> --- > drivers/devfreq/governor_passive.c | 15 +++++++++++++++ > 1 file changed, 15 insertions(+) > > diff --git a/drivers/devfreq/governor_passive.c b/drivers/devfreq/governor_passive.c > index 63332e4a65ae..375aa636027c 100644 > --- a/drivers/devfreq/governor_passive.c > +++ b/drivers/devfreq/governor_passive.c > @@ -141,6 +141,21 @@ static int devfreq_passive_event_handler(struct devfreq *devfreq, > if (!p_data->this) > p_data->this = devfreq; > > + /* > + * If the parent device changes the their frequency before > + * registering the passive device, the passive device cannot > + * receive the notification from parent device and then the > + * passive device cannot be able to set the proper frequency > + * according to the frequency of parent device. > + * > + * When start the passive governor, update the frequency > + * according to the frequency of parent device. > + */ > + ret = devfreq_update_target(devfreq, parent->previous_freq); > + if (ret < 0) > + dev_warn(&devfreq->dev, > + "failed to update devfreq using passive governor\n"); > + > nb->notifier_call = devfreq_passive_notifier_call; > ret = devfreq_register_notifier(parent, nb, > DEVFREQ_TRANSITION_NOTIFIER);
Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland
| |