Messages in this thread | | | Subject | Re: [PATCH] PM / devfreq: passive: Update frequency when start governor | From | Chanwoo Choi <> | Date | Mon, 9 Nov 2020 18:37:54 +0900 |
| |
Hi Marek,
Thanks for bug reporting. I'll check and fix it.
Best Regards, Chanwoo Choi
On 11/9/20 6:22 PM, Marek Szyprowski wrote: > 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 >
| |