Re: [PATCH v2 5/5] PM / devfreq: passive: Update frequency when start governor

From: Chanwoo Choi
Date: Mon May 09 2022 - 06:31:08 EST


Hi,

On 22. 5. 9. 13:25, Chen-Yu Tsai wrote:
Hi,

On Sun, May 08, 2022 at 12:01:45AM +0900, 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@xxxxxxxxxxx>
Link: https://lore.kernel.org/r/20220507150145.531864-6-cw00.choi@xxxxxxxxxxx
---
drivers/devfreq/governor_passive.c | 17 +++++++++++++++++
1 file changed, 17 insertions(+)

diff --git a/drivers/devfreq/governor_passive.c b/drivers/devfreq/governor_passive.c
index b34dbe750c0a..74d26c193fdb 100644
--- a/drivers/devfreq/governor_passive.c
+++ b/drivers/devfreq/governor_passive.c
@@ -412,6 +412,23 @@ 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.
+ */
+ mutex_lock(&devfreq->lock);
+ ret = devfreq_update_target(devfreq, parent->previous_freq);

This crashes when parent is NULL, in the case where parent is cpufreq.
This is the case with the MTK ccifreq driver, which produces the panic
and backtrace below [1].

I made a fix for a previous version of this patch:

https://github.com/wens/linux/commit/f85c1834dd07388abb57a00200c80f7440823a03

BTW, could you CC me on future revisions? I'm not subscribed to the
linux-pm mailing list.

OK. I'll.



Regards
ChenYu

[1]

Unable to handle kernel read from unreadable memory at virtual address 0000000000000420
Mem abort info:
ESR = 0x0000000096000005
EC = 0x25: DABT (current EL), IL = 32 bits
SET = 0, FnV = 0
EA = 0, S1PTW = 0
FSC = 0x05: level 1 translation fault
Data abort info:
ISV = 0, ISS = 0x00000005
CM = 0, WnR = 0
[0000000000000420] user address but active_mm is swapper
Internal error: Oops: 96000005 [#1] PREEMPT SMP
Modules linked in:
CPU: 7 PID: 1 Comm: swapper/0 Not tainted 5.18.0-rc5-next-20220505-09393-g38dc825c1d73 #155 b348fdb8d61a403eef7a9c5857bc02a261fcb213
Hardware name: Google juniper sku16 board (DT)
pstate: 60400005 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
pc : devfreq_passive_event_handler (drivers/devfreq/governor_passive.c:426)
lr : devfreq_passive_event_handler (drivers/devfreq/governor_passive.c:426)
sp : ffffffc00808ba80
x29: ffffffc00808ba80 x28: 0000000000000000 x27: ffffffe99bb90458
x26: 0000000000000010 x25: ffffff80c1843848 x24: ffffff80c1843810
x23: ffffffe99babf3f5 x22: ffffffe99c278190 x21: ffffff80c0924d80
x20: ffffff80c1843800 x19: 0000000000000000 x18: 0000000000000000
x17: 0000000065516d0e x16: 00000000fc90660b x15: 0000000000000018
x14: 0000000000000000 x13: ffffffffff000000 x12: 0000000000000038
x11: 0101010101010101 x10: 8000000000000000 x9 : ffffffe99acb8458
x8 : 0065766973000000 x7 : 0000000000000080 x6 : 0000000000000000
x5 : 8000000000000000 x4 : 0000000000000000 x3 : ffffff80c1843810
x2 : ffffff80c0228000 x1 : 0000000000000000 x0 : 0000000000000000
Call trace:
devfreq_passive_event_handler (drivers/devfreq/governor_passive.c:426)
devfreq_add_device (drivers/devfreq/devfreq.c:932)
devm_devfreq_add_device (drivers/devfreq/devfreq.c:1028)
mtk_ccifreq_probe (drivers/devfreq/mtk-cci-devfreq.c:366)
platform_probe (drivers/base/platform.c:1398)
really_probe (drivers/base/dd.c:542 drivers/base/dd.c:621 drivers/base/dd.c:566)
__driver_probe_device (drivers/base/dd.c:752)
driver_probe_device (drivers/base/dd.c:782)
__driver_attach (drivers/base/dd.c:1143 drivers/base/dd.c:1094)
bus_for_each_dev (drivers/base/bus.c:301)
driver_attach (drivers/base/dd.c:1160)
bus_add_driver (drivers/base/bus.c:619)
driver_register (drivers/base/driver.c:240)
__platform_driver_register (drivers/base/platform.c:866)
mtk_ccifreq_platdrv_init (drivers/devfreq/mtk-cci-devfreq.c:468)
do_one_initcall (init/main.c:1301)
kernel_init_freeable (init/main.c:1375 init/main.c:1392 init/main.c:1411 init/main.c:1618)
kernel_init (init/main.c:1511)
ret_from_fork (arch/arm64/kernel/entry.S:868)
Code: f9000eb4 91004298 aa1803e0 940979d4 (f9421261)
All code
========
0: f9000eb4 str x20, [x21, #24]
4: 91004298 add x24, x20, #0x10
8: aa1803e0 mov x0, x24
c: 940979d4 bl 0x25e75c
10:* f9421261 ldr x1, [x19, #1056] <-- trapping instruction

Code starting with the faulting instruction
===========================================
0: f9421261 ldr x1, [x19, #1056]
---[ end trace 0000000000000000 ]---
Kernel panic - not syncing: Oops: Fatal exception
SMP: stopping secondary CPUs
Kernel Offset: 0x2992c00000 from 0xffffffc008000000
PHYS_OFFSET: 0x40000000
CPU features: 0x000,00324811,00001086
Memory Limit: none
PANIC in EL3.

+ if (ret < 0)
+ dev_warn(&devfreq->dev,
+ "failed to update devfreq using passive governor\n");
+ mutex_unlock(&devfreq->lock);
+
if (p_data->parent_type == DEVFREQ_PARENT_DEV)
ret = devfreq_passive_register_notifier(devfreq);
else if (p_data->parent_type == CPUFREQ_PARENT_DEV)

--
2.25.1


Thanks for the testing. I'll drop the last patch
and then send next version.

--
Best Regards,
Samsung Electronics
Chanwoo Choi