* [PATCH] PM / devfreq: passive: Update frequency when start governor [not found] <CGME20201103065250epcas1p4421332566646a4464d7871429aabafd9@epcas1p4.samsung.com> @ 2020-11-03 7:06 ` Chanwoo Choi 2020-11-09 9:22 ` Marek Szyprowski 0 siblings, 1 reply; 3+ messages in thread From: Chanwoo Choi @ 2020-11-03 7:06 UTC (permalink / raw) To: linux-pm, linux-kernel; +Cc: cw00.choi, chanwoo, myungjoo.ham, kyungmin.park 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> --- 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); -- 2.17.1 ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] PM / devfreq: passive: Update frequency when start governor 2020-11-03 7:06 ` [PATCH] PM / devfreq: passive: Update frequency when start governor Chanwoo Choi @ 2020-11-09 9:22 ` Marek Szyprowski 2020-11-09 9:37 ` Chanwoo Choi 0 siblings, 1 reply; 3+ messages in thread From: Marek Szyprowski @ 2020-11-09 9:22 UTC (permalink / raw) To: Chanwoo Choi, linux-pm, linux-kernel Cc: chanwoo, myungjoo.ham, kyungmin.park, Bartlomiej Zolnierkiewicz 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 ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] PM / devfreq: passive: Update frequency when start governor 2020-11-09 9:22 ` Marek Szyprowski @ 2020-11-09 9:37 ` Chanwoo Choi 0 siblings, 0 replies; 3+ messages in thread From: Chanwoo Choi @ 2020-11-09 9:37 UTC (permalink / raw) To: Marek Szyprowski, linux-pm, linux-kernel Cc: chanwoo, myungjoo.ham, kyungmin.park, Bartlomiej Zolnierkiewicz 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 > ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-11-09 9:24 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <CGME20201103065250epcas1p4421332566646a4464d7871429aabafd9@epcas1p4.samsung.com>
2020-11-03 7:06 ` [PATCH] PM / devfreq: passive: Update frequency when start governor Chanwoo Choi
2020-11-09 9:22 ` Marek Szyprowski
2020-11-09 9:37 ` Chanwoo Choi
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).