lkml.org 
[lkml]   [2020]   [Nov]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH] PM / devfreq: passive: Update frequency when start governor
From
Date
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
>

\
 
 \ /
  Last update: 2020-11-09 10:24    [W:0.600 / U:1.128 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site