Refactor _opp_table_find_key() to improve readability by moving the
reference count increment and key update inside the match condition block.
Also make the 'assert' check mandatory instead of treating it as optional.
Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
---
drivers/opp/core.c | 14 ++++++--------
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/drivers/opp/core.c b/drivers/opp/core.c
index a36c3daac39cd0bdd2a1f7e9bad5b92f0c756153..bf49709b8c39271431772924daf0c003b45eec7f 100644
--- a/drivers/opp/core.c
+++ b/drivers/opp/core.c
@@ -544,24 +544,22 @@ static struct dev_pm_opp *_opp_table_find_key(struct opp_table *opp_table,
struct dev_pm_opp *temp_opp, *opp = ERR_PTR(-ERANGE);
/* Assert that the requirement is met */
- if (assert && !assert(opp_table, index))
+ if (!assert(opp_table, index))
return ERR_PTR(-EINVAL);
guard(mutex)(&opp_table->lock);
list_for_each_entry(temp_opp, &opp_table->opp_list, node) {
if (temp_opp->available == available) {
- if (compare(&opp, temp_opp, read(temp_opp, index), *key))
+ if (compare(&opp, temp_opp, read(temp_opp, index), *key)) {
+ /* Increment the reference count of OPP */
+ *key = read(opp, index);
+ dev_pm_opp_get(opp);
break;
+ }
}
}
- /* Increment the reference count of OPP */
- if (!IS_ERR(opp)) {
- *key = read(opp, index);
- dev_pm_opp_get(opp);
- }
-
return opp;
}
--
2.34.1On 20.08.2025 10:28, Krishna Chaitanya Chundru wrote:
> Refactor _opp_table_find_key() to improve readability by moving the
> reference count increment and key update inside the match condition block.
>
> Also make the 'assert' check mandatory instead of treating it as optional.
>
> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
This patch landed in today's linux-next (20250825) as commit
b5323835f050 ("OPP: Reorganize _opp_table_find_key()"). In my tests I
found that it causes regressions on my test boards. Reverting this
change on top of linux-next fixes booting of all the affected boards.
Here are kernel logs with lockdep enabled:
1. Exynos4412-based Odroid-U3 board (ARM 32bit):
============================================
WARNING: possible recursive locking detected
6.17.0-rc3-next-20250825 #10901 Not tainted
--------------------------------------------
kworker/u16:0/12 is trying to acquire lock:
cf896040 (&devfreq->lock){+.+.}-{3:3}, at: devfreq_notifier_call+0x30/0x124
but task is already holding lock:
cf896040 (&devfreq->lock){+.+.}-{3:3}, at: devfreq_monitor+0x1c/0x1a4
other info that might help us debug this:
Possible unsafe locking scenario:
CPU0
----
lock(&devfreq->lock);
lock(&devfreq->lock);
*** DEADLOCK ***
May be due to missing lock nesting notation
4 locks held by kworker/u16:0/12:
#0: c289d0b4 ((wq_completion)devfreq_wq){+.+.}-{0:0}, at:
process_one_work+0x1b0/0x70c
#1: f0899f18
((work_completion)(&(&devfreq->work)->work)#2){+.+.}-{0:0}, at:
process_one_work+0x1dc/0x70c
#2: cf896040 (&devfreq->lock){+.+.}-{3:3}, at: devfreq_monitor+0x1c/0x1a4
#3: c2e78c4c (&(&opp_table->head)->rwsem){++++}-{3:3}, at:
blocking_notifier_call_chain+0x28/0x60
stack backtrace:
CPU: 2 UID: 0 PID: 12 Comm: kworker/u16:0 Not tainted
6.17.0-rc3-next-20250825 #10901 PREEMPT
Hardware name: Samsung Exynos (Flattened Device Tree)
Workqueue: devfreq_wq devfreq_monitor
Call trace:
unwind_backtrace from show_stack+0x10/0x14
show_stack from dump_stack_lvl+0x68/0x88
dump_stack_lvl from print_deadlock_bug+0x370/0x380
print_deadlock_bug from __lock_acquire+0x1428/0x29ec
__lock_acquire from lock_acquire+0x134/0x388
lock_acquire from __mutex_lock+0xac/0x10c0
__mutex_lock from mutex_lock_nested+0x1c/0x24
mutex_lock_nested from devfreq_notifier_call+0x30/0x124
devfreq_notifier_call from notifier_call_chain+0x84/0x1d4
notifier_call_chain from blocking_notifier_call_chain+0x44/0x60
blocking_notifier_call_chain from _opp_kref_release+0x3c/0x5c
_opp_kref_release from exynos_bus_target+0x24/0x70
exynos_bus_target from devfreq_set_target+0x8c/0x2e8
devfreq_set_target from devfreq_update_target+0x9c/0xf8
devfreq_update_target from devfreq_monitor+0x28/0x1a4
devfreq_monitor from process_one_work+0x24c/0x70c
process_one_work from worker_thread+0x1b8/0x3bc
worker_thread from kthread+0x13c/0x264
kthread from ret_from_fork+0x14/0x28
Exception stack(0xf0899fb0 to 0xf0899ff8)
...
2. Exynos5422-based Odroid-XU3 board (ARM 32bit):
8<--- cut here ---
Unable to handle kernel NULL pointer dereference at virtual address
00000000 when read
[00000000] *pgd=00000000
Internal error: Oops: 5 [#1] SMP ARM
Modules linked in:
CPU: 7 UID: 0 PID: 68 Comm: kworker/u34:1 Not tainted
6.17.0-rc3-next-20250825 #10901 PREEMPT
Hardware name: Samsung Exynos (Flattened Device Tree)
Workqueue: devfreq_wq devfreq_monitor
PC is at _opp_compare_key+0x30/0xb4
LR is at 0xfffffffc
pc : [<c09831c4>] lr : [<fffffffc>] psr: 20000013
sp : f0a89de0 ip : cfb0e94c fp : c1574880
r10: c14095a4 r9 : f0a89e44 r8 : c2a9c010
r7 : cfb0ea80 r6 : 00000001 r5 : cfb0e900 r4 : 00000001
r3 : 00000000 r2 : cfb0e900 r1 : cfb0ea80 r0 : cfaf5800
Flags: nzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none
Control: 10c5387d Table: 4000406a DAC: 00000051
Register r0 information: slab kmalloc-1k start cfaf5800 pointer offset 0
size 1024
Register r1 information: slab kmalloc-128 start cfb0ea80 pointer offset
0 size 128
Register r2 information: slab kmalloc-128 start cfb0e900 pointer offset
0 size 128
Register r3 information: NULL pointer
Register r4 information: non-paged memory
Register r5 information: slab kmalloc-128 start cfb0e900 pointer offset
0 size 128
Register r6 information: non-paged memory
Register r7 information: slab kmalloc-128 start cfb0ea80 pointer offset
0 size 128
Register r8 information: slab kmalloc-1k start c2a9c000 pointer offset
16 size 1024
Register r9 information: 2-page vmalloc region starting at 0xf0a88000
allocated at kernel_clone+0x58/0x3c4
Register r10 information: non-slab/vmalloc memory
Register r11 information: non-slab/vmalloc memory
Register r12 information: slab kmalloc-128 start cfb0e900 pointer offset
76 size 128
Process kworker/u34:1 (pid: 68, stack limit = 0x050eb3d7)
Stack: (0xf0a89de0 to 0xf0a8a000)
..
Call trace:
_opp_compare_key from _set_opp+0x78/0x50c
_set_opp from dev_pm_opp_set_rate+0x15c/0x21c
dev_pm_opp_set_rate from panfrost_devfreq_target+0x2c/0x3c
panfrost_devfreq_target from devfreq_set_target+0x8c/0x2e8
devfreq_set_target from devfreq_update_target+0x9c/0xf8
devfreq_update_target from devfreq_monitor+0x28/0x1a4
devfreq_monitor from process_one_work+0x24c/0x70c
process_one_work from worker_thread+0x1b8/0x3bc
worker_thread from kthread+0x13c/0x264
kthread from ret_from_fork+0x14/0x28
Exception stack(0xf0a89fb0 to 0xf0a89ff8)
...
---[ end trace 0000000000000000 ]---
3. Qualcomm Technologies, Inc. Robotics RB5(ARM 64bit):
ufshcd-qcom 1d84000.ufshc: freq-table-hz property not specified
ufshcd-qcom 1d84000.ufshc: ufshcd_populate_vreg: Unable to find
vdd-hba-supply regulator, assuming enabled
ufshcd-qcom 1d84000.ufshc: Failed to find OPP for MIN frequency
ufshcd-qcom 1d84000.ufshc: ufshcd_pltfrm_init: OPP parse failed -34
ufshcd-qcom 1d84000.ufshc: error -ERANGE: ufshcd_pltfrm_init() failed
ufshcd-qcom 1d84000.ufshc: probe with driver ufshcd-qcom failed with
error -34
> ---
> drivers/opp/core.c | 14 ++++++--------
> 1 file changed, 6 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/opp/core.c b/drivers/opp/core.c
> index a36c3daac39cd0bdd2a1f7e9bad5b92f0c756153..bf49709b8c39271431772924daf0c003b45eec7f 100644
> --- a/drivers/opp/core.c
> +++ b/drivers/opp/core.c
> @@ -544,24 +544,22 @@ static struct dev_pm_opp *_opp_table_find_key(struct opp_table *opp_table,
> struct dev_pm_opp *temp_opp, *opp = ERR_PTR(-ERANGE);
>
> /* Assert that the requirement is met */
> - if (assert && !assert(opp_table, index))
> + if (!assert(opp_table, index))
> return ERR_PTR(-EINVAL);
>
> guard(mutex)(&opp_table->lock);
>
> list_for_each_entry(temp_opp, &opp_table->opp_list, node) {
> if (temp_opp->available == available) {
> - if (compare(&opp, temp_opp, read(temp_opp, index), *key))
> + if (compare(&opp, temp_opp, read(temp_opp, index), *key)) {
> + /* Increment the reference count of OPP */
> + *key = read(opp, index);
> + dev_pm_opp_get(opp);
> break;
> + }
> }
> }
>
> - /* Increment the reference count of OPP */
> - if (!IS_ERR(opp)) {
> - *key = read(opp, index);
> - dev_pm_opp_get(opp);
> - }
> -
> return opp;
> }
>
>
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
On 25/08/2025 15:59, Marek Szyprowski wrote: > Register r10 information: non-slab/vmalloc memory > Register r11 information: non-slab/vmalloc memory > Register r12 information: slab kmalloc-128 start cfb0e900 pointer offset > 76 size 128 > Process kworker/u34:1 (pid: 68, stack limit = 0x050eb3d7) > Stack: (0xf0a89de0 to 0xf0a8a000) > .. > Call trace: > _opp_compare_key from _set_opp+0x78/0x50c > _set_opp from dev_pm_opp_set_rate+0x15c/0x21c > dev_pm_opp_set_rate from panfrost_devfreq_target+0x2c/0x3c > panfrost_devfreq_target from devfreq_set_target+0x8c/0x2e8 > devfreq_set_target from devfreq_update_target+0x9c/0xf8 > devfreq_update_target from devfreq_monitor+0x28/0x1a4 > devfreq_monitor from process_one_work+0x24c/0x70c > process_one_work from worker_thread+0x1b8/0x3bc > worker_thread from kthread+0x13c/0x264 > kthread from ret_from_fork+0x14/0x28 > Exception stack(0xf0a89fb0 to 0xf0a89ff8) I also saw this on today's next: https://krzk.eu/#/builders/21/builds/6690/steps/13/logs/serial0 Best regards, Krzysztof
Marek,
Thanks for the detailed logs. I would need a little more help from
you.
Can you give this a try over your failing branch (I have already
dropped the patch from my tree for now):
diff --git a/drivers/opp/core.c b/drivers/opp/core.c
index 81fb7dd7f323..5b24255733b5 100644
--- a/drivers/opp/core.c
+++ b/drivers/opp/core.c
@@ -554,10 +554,10 @@ static struct dev_pm_opp *_opp_table_find_key(struct opp_table *opp_table,
list_for_each_entry(temp_opp, &opp_table->opp_list, node) {
if (temp_opp->available == available) {
if (compare(&opp, temp_opp, read(temp_opp, index), *key)) {
- *key = read(opp, index);
-
- /* Increment the reference count of OPP */
- dev_pm_opp_get(opp);
+ if (!IS_ERR(opp)) {
+ *key = read(opp, index);
+ dev_pm_opp_get(opp);
+ }
break;
}
}
On 25-08-25, 15:59, Marek Szyprowski wrote:
> 1. Exynos4412-based Odroid-U3 board (ARM 32bit):
>
> ============================================
> WARNING: possible recursive locking detected
> 6.17.0-rc3-next-20250825 #10901 Not tainted
> --------------------------------------------
> kworker/u16:0/12 is trying to acquire lock:
> cf896040 (&devfreq->lock){+.+.}-{3:3}, at: devfreq_notifier_call+0x30/0x124
>
> but task is already holding lock:
> cf896040 (&devfreq->lock){+.+.}-{3:3}, at: devfreq_monitor+0x1c/0x1a4
>
> other info that might help us debug this:
> Possible unsafe locking scenario:
>
> CPU0
> ----
> lock(&devfreq->lock);
> lock(&devfreq->lock);
>
> *** DEADLOCK ***
>
> May be due to missing lock nesting notation
>
> 4 locks held by kworker/u16:0/12:
> #0: c289d0b4 ((wq_completion)devfreq_wq){+.+.}-{0:0}, at:
> process_one_work+0x1b0/0x70c
> #1: f0899f18
> ((work_completion)(&(&devfreq->work)->work)#2){+.+.}-{0:0}, at:
> process_one_work+0x1dc/0x70c
> #2: cf896040 (&devfreq->lock){+.+.}-{3:3}, at: devfreq_monitor+0x1c/0x1a4
> #3: c2e78c4c (&(&opp_table->head)->rwsem){++++}-{3:3}, at:
> blocking_notifier_call_chain+0x28/0x60
>
> stack backtrace:
> CPU: 2 UID: 0 PID: 12 Comm: kworker/u16:0 Not tainted
> 6.17.0-rc3-next-20250825 #10901 PREEMPT
> Hardware name: Samsung Exynos (Flattened Device Tree)
> Workqueue: devfreq_wq devfreq_monitor
> Call trace:
> unwind_backtrace from show_stack+0x10/0x14
> show_stack from dump_stack_lvl+0x68/0x88
> dump_stack_lvl from print_deadlock_bug+0x370/0x380
> print_deadlock_bug from __lock_acquire+0x1428/0x29ec
> __lock_acquire from lock_acquire+0x134/0x388
> lock_acquire from __mutex_lock+0xac/0x10c0
> __mutex_lock from mutex_lock_nested+0x1c/0x24
> mutex_lock_nested from devfreq_notifier_call+0x30/0x124
> devfreq_notifier_call from notifier_call_chain+0x84/0x1d4
> notifier_call_chain from blocking_notifier_call_chain+0x44/0x60
> blocking_notifier_call_chain from _opp_kref_release+0x3c/0x5c
> _opp_kref_release from exynos_bus_target+0x24/0x70
> exynos_bus_target from devfreq_set_target+0x8c/0x2e8
> devfreq_set_target from devfreq_update_target+0x9c/0xf8
> devfreq_update_target from devfreq_monitor+0x28/0x1a4
> devfreq_monitor from process_one_work+0x24c/0x70c
> process_one_work from worker_thread+0x1b8/0x3bc
> worker_thread from kthread+0x13c/0x264
> kthread from ret_from_fork+0x14/0x28
> Exception stack(0xf0899fb0 to 0xf0899ff8)
I guess there is some issue with devfreq here which showed up because
we tried to do a dev_pm_opp_get() for an invalid opp (which very well
could have been valid here anyway). This was always done with the OPP
table's lock anyway, nothing changed after the commit AFAICT.
> 2. Exynos5422-based Odroid-XU3 board (ARM 32bit):
>
> 8<--- cut here ---
> Unable to handle kernel NULL pointer dereference at virtual address
> 00000000 when read
> [00000000] *pgd=00000000
> Internal error: Oops: 5 [#1] SMP ARM
> Modules linked in:
> CPU: 7 UID: 0 PID: 68 Comm: kworker/u34:1 Not tainted
> 6.17.0-rc3-next-20250825 #10901 PREEMPT
> Hardware name: Samsung Exynos (Flattened Device Tree)
> Workqueue: devfreq_wq devfreq_monitor
> PC is at _opp_compare_key+0x30/0xb4
> LR is at 0xfffffffc
> pc : [<c09831c4>] lr : [<fffffffc>] psr: 20000013
> sp : f0a89de0 ip : cfb0e94c fp : c1574880
> r10: c14095a4 r9 : f0a89e44 r8 : c2a9c010
> r7 : cfb0ea80 r6 : 00000001 r5 : cfb0e900 r4 : 00000001
> r3 : 00000000 r2 : cfb0e900 r1 : cfb0ea80 r0 : cfaf5800
> Flags: nzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none
> Control: 10c5387d Table: 4000406a DAC: 00000051
> Register r0 information: slab kmalloc-1k start cfaf5800 pointer offset 0
> size 1024
> Register r1 information: slab kmalloc-128 start cfb0ea80 pointer offset
> 0 size 128
> Register r2 information: slab kmalloc-128 start cfb0e900 pointer offset
> 0 size 128
> Register r3 information: NULL pointer
> Register r4 information: non-paged memory
> Register r5 information: slab kmalloc-128 start cfb0e900 pointer offset
> 0 size 128
> Register r6 information: non-paged memory
> Register r7 information: slab kmalloc-128 start cfb0ea80 pointer offset
> 0 size 128
> Register r8 information: slab kmalloc-1k start c2a9c000 pointer offset
> 16 size 1024
> Register r9 information: 2-page vmalloc region starting at 0xf0a88000
> allocated at kernel_clone+0x58/0x3c4
> Register r10 information: non-slab/vmalloc memory
> Register r11 information: non-slab/vmalloc memory
> Register r12 information: slab kmalloc-128 start cfb0e900 pointer offset
> 76 size 128
> Process kworker/u34:1 (pid: 68, stack limit = 0x050eb3d7)
> Stack: (0xf0a89de0 to 0xf0a8a000)
> ..
> Call trace:
> _opp_compare_key from _set_opp+0x78/0x50c
> _set_opp from dev_pm_opp_set_rate+0x15c/0x21c
> dev_pm_opp_set_rate from panfrost_devfreq_target+0x2c/0x3c
> panfrost_devfreq_target from devfreq_set_target+0x8c/0x2e8
> devfreq_set_target from devfreq_update_target+0x9c/0xf8
> devfreq_update_target from devfreq_monitor+0x28/0x1a4
> devfreq_monitor from process_one_work+0x24c/0x70c
> process_one_work from worker_thread+0x1b8/0x3bc
> worker_thread from kthread+0x13c/0x264
> kthread from ret_from_fork+0x14/0x28
> Exception stack(0xf0a89fb0 to 0xf0a89ff8)
I don't fully understand why this happened yet.
> ---[ end trace 0000000000000000 ]---
>
>
> 3. Qualcomm Technologies, Inc. Robotics RB5(ARM 64bit):
>
> ufshcd-qcom 1d84000.ufshc: freq-table-hz property not specified
> ufshcd-qcom 1d84000.ufshc: ufshcd_populate_vreg: Unable to find
> vdd-hba-supply regulator, assuming enabled
> ufshcd-qcom 1d84000.ufshc: Failed to find OPP for MIN frequency
> ufshcd-qcom 1d84000.ufshc: ufshcd_pltfrm_init: OPP parse failed -34
> ufshcd-qcom 1d84000.ufshc: error -ERANGE: ufshcd_pltfrm_init() failed
> ufshcd-qcom 1d84000.ufshc: probe with driver ufshcd-qcom failed with
> error -34
This too.
--
viresh
On 26.08.2025 08:06, Viresh Kumar wrote:
> Marek,
>
> Thanks for the detailed logs. I would need a little more help from
> you.
>
> Can you give this a try over your failing branch (I have already
> dropped the patch from my tree for now):
>
> diff --git a/drivers/opp/core.c b/drivers/opp/core.c
> index 81fb7dd7f323..5b24255733b5 100644
> --- a/drivers/opp/core.c
> +++ b/drivers/opp/core.c
> @@ -554,10 +554,10 @@ static struct dev_pm_opp *_opp_table_find_key(struct opp_table *opp_table,
> list_for_each_entry(temp_opp, &opp_table->opp_list, node) {
> if (temp_opp->available == available) {
> if (compare(&opp, temp_opp, read(temp_opp, index), *key)) {
> - *key = read(opp, index);
> -
> - /* Increment the reference count of OPP */
> - dev_pm_opp_get(opp);
> + if (!IS_ERR(opp)) {
> + *key = read(opp, index);
> + dev_pm_opp_get(opp);
> + }
> break;
> }
> }
This doesn't help. I've stared a bit at that code and did some tests
and it looks that the issue is caused by _opp_table_find_key() returning
the last opp from opp_list without updating the *key and calling
opp_get() for it (happens when compare() returns false).
> ...
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
On 26-08-25, 09:26, Marek Szyprowski wrote: > This doesn't help. I've stared a bit at that code and did some tests > and it looks that the issue is caused by _opp_table_find_key() returning > the last opp from opp_list without updating the *key and calling > opp_get() for it (happens when compare() returns false). Ahh, right. So `compare()` may never return `true` and in that case we returned the last OPP of the table earlier. Thanks. -- viresh
On 8/25/2025 7:29 PM, Marek Szyprowski wrote:
> On 20.08.2025 10:28, Krishna Chaitanya Chundru wrote:
>> Refactor _opp_table_find_key() to improve readability by moving the
>> reference count increment and key update inside the match condition block.
>>
>> Also make the 'assert' check mandatory instead of treating it as optional.
>>
>> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
>
> This patch landed in today's linux-next (20250825) as commit
> b5323835f050 ("OPP: Reorganize _opp_table_find_key()"). In my tests I
> found that it causes regressions on my test boards. Reverting this
> change on top of linux-next fixes booting of all the affected boards.
>
> Here are kernel logs with lockdep enabled:
>
> 1. Exynos4412-based Odroid-U3 board (ARM 32bit):
>
> ============================================
> WARNING: possible recursive locking detected
> 6.17.0-rc3-next-20250825 #10901 Not tainted
> --------------------------------------------
> kworker/u16:0/12 is trying to acquire lock:
> cf896040 (&devfreq->lock){+.+.}-{3:3}, at: devfreq_notifier_call+0x30/0x124
>
> but task is already holding lock:
> cf896040 (&devfreq->lock){+.+.}-{3:3}, at: devfreq_monitor+0x1c/0x1a4
>
> other info that might help us debug this:
> Possible unsafe locking scenario:
>
> CPU0
> ----
> lock(&devfreq->lock);
> lock(&devfreq->lock);
>
> *** DEADLOCK ***
>
> May be due to missing lock nesting notation
>
> 4 locks held by kworker/u16:0/12:
> #0: c289d0b4 ((wq_completion)devfreq_wq){+.+.}-{0:0}, at:
> process_one_work+0x1b0/0x70c
> #1: f0899f18
> ((work_completion)(&(&devfreq->work)->work)#2){+.+.}-{0:0}, at:
> process_one_work+0x1dc/0x70c
> #2: cf896040 (&devfreq->lock){+.+.}-{3:3}, at: devfreq_monitor+0x1c/0x1a4
> #3: c2e78c4c (&(&opp_table->head)->rwsem){++++}-{3:3}, at:
> blocking_notifier_call_chain+0x28/0x60
>
> stack backtrace:
> CPU: 2 UID: 0 PID: 12 Comm: kworker/u16:0 Not tainted
> 6.17.0-rc3-next-20250825 #10901 PREEMPT
> Hardware name: Samsung Exynos (Flattened Device Tree)
> Workqueue: devfreq_wq devfreq_monitor
> Call trace:
> unwind_backtrace from show_stack+0x10/0x14
> show_stack from dump_stack_lvl+0x68/0x88
> dump_stack_lvl from print_deadlock_bug+0x370/0x380
> print_deadlock_bug from __lock_acquire+0x1428/0x29ec
> __lock_acquire from lock_acquire+0x134/0x388
> lock_acquire from __mutex_lock+0xac/0x10c0
> __mutex_lock from mutex_lock_nested+0x1c/0x24
> mutex_lock_nested from devfreq_notifier_call+0x30/0x124
> devfreq_notifier_call from notifier_call_chain+0x84/0x1d4
> notifier_call_chain from blocking_notifier_call_chain+0x44/0x60
> blocking_notifier_call_chain from _opp_kref_release+0x3c/0x5c
> _opp_kref_release from exynos_bus_target+0x24/0x70
> exynos_bus_target from devfreq_set_target+0x8c/0x2e8
> devfreq_set_target from devfreq_update_target+0x9c/0xf8
> devfreq_update_target from devfreq_monitor+0x28/0x1a4
> devfreq_monitor from process_one_work+0x24c/0x70c
> process_one_work from worker_thread+0x1b8/0x3bc
> worker_thread from kthread+0x13c/0x264
> kthread from ret_from_fork+0x14/0x28
> Exception stack(0xf0899fb0 to 0xf0899ff8)
>
> ...
>
>
> 2. Exynos5422-based Odroid-XU3 board (ARM 32bit):
>
> 8<--- cut here ---
> Unable to handle kernel NULL pointer dereference at virtual address
> 00000000 when read
> [00000000] *pgd=00000000
> Internal error: Oops: 5 [#1] SMP ARM
> Modules linked in:
> CPU: 7 UID: 0 PID: 68 Comm: kworker/u34:1 Not tainted
> 6.17.0-rc3-next-20250825 #10901 PREEMPT
> Hardware name: Samsung Exynos (Flattened Device Tree)
> Workqueue: devfreq_wq devfreq_monitor
> PC is at _opp_compare_key+0x30/0xb4
> LR is at 0xfffffffc
> pc : [<c09831c4>] lr : [<fffffffc>] psr: 20000013
> sp : f0a89de0 ip : cfb0e94c fp : c1574880
> r10: c14095a4 r9 : f0a89e44 r8 : c2a9c010
> r7 : cfb0ea80 r6 : 00000001 r5 : cfb0e900 r4 : 00000001
> r3 : 00000000 r2 : cfb0e900 r1 : cfb0ea80 r0 : cfaf5800
> Flags: nzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none
> Control: 10c5387d Table: 4000406a DAC: 00000051
> Register r0 information: slab kmalloc-1k start cfaf5800 pointer offset 0
> size 1024
> Register r1 information: slab kmalloc-128 start cfb0ea80 pointer offset
> 0 size 128
> Register r2 information: slab kmalloc-128 start cfb0e900 pointer offset
> 0 size 128
> Register r3 information: NULL pointer
> Register r4 information: non-paged memory
> Register r5 information: slab kmalloc-128 start cfb0e900 pointer offset
> 0 size 128
> Register r6 information: non-paged memory
> Register r7 information: slab kmalloc-128 start cfb0ea80 pointer offset
> 0 size 128
> Register r8 information: slab kmalloc-1k start c2a9c000 pointer offset
> 16 size 1024
> Register r9 information: 2-page vmalloc region starting at 0xf0a88000
> allocated at kernel_clone+0x58/0x3c4
> Register r10 information: non-slab/vmalloc memory
> Register r11 information: non-slab/vmalloc memory
> Register r12 information: slab kmalloc-128 start cfb0e900 pointer offset
> 76 size 128
> Process kworker/u34:1 (pid: 68, stack limit = 0x050eb3d7)
> Stack: (0xf0a89de0 to 0xf0a8a000)
> ..
> Call trace:
> _opp_compare_key from _set_opp+0x78/0x50c
> _set_opp from dev_pm_opp_set_rate+0x15c/0x21c
> dev_pm_opp_set_rate from panfrost_devfreq_target+0x2c/0x3c
> panfrost_devfreq_target from devfreq_set_target+0x8c/0x2e8
> devfreq_set_target from devfreq_update_target+0x9c/0xf8
> devfreq_update_target from devfreq_monitor+0x28/0x1a4
> devfreq_monitor from process_one_work+0x24c/0x70c
> process_one_work from worker_thread+0x1b8/0x3bc
> worker_thread from kthread+0x13c/0x264
> kthread from ret_from_fork+0x14/0x28
> Exception stack(0xf0a89fb0 to 0xf0a89ff8)
> ...
> ---[ end trace 0000000000000000 ]---
>
>
> 3. Qualcomm Technologies, Inc. Robotics RB5(ARM 64bit):
>
> ufshcd-qcom 1d84000.ufshc: freq-table-hz property not specified
> ufshcd-qcom 1d84000.ufshc: ufshcd_populate_vreg: Unable to find
> vdd-hba-supply regulator, assuming enabled
> ufshcd-qcom 1d84000.ufshc: Failed to find OPP for MIN frequency
> ufshcd-qcom 1d84000.ufshc: ufshcd_pltfrm_init: OPP parse failed -34
> ufshcd-qcom 1d84000.ufshc: error -ERANGE: ufshcd_pltfrm_init() failed
> ufshcd-qcom 1d84000.ufshc: probe with driver ufshcd-qcom failed with
> error -34
>
>
>
>> ---
>> drivers/opp/core.c | 14 ++++++--------
>> 1 file changed, 6 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/opp/core.c b/drivers/opp/core.c
>> index a36c3daac39cd0bdd2a1f7e9bad5b92f0c756153..bf49709b8c39271431772924daf0c003b45eec7f 100644
>> --- a/drivers/opp/core.c
>> +++ b/drivers/opp/core.c
>> @@ -544,24 +544,22 @@ static struct dev_pm_opp *_opp_table_find_key(struct opp_table *opp_table,
>> struct dev_pm_opp *temp_opp, *opp = ERR_PTR(-ERANGE);
>>
>> /* Assert that the requirement is met */
>> - if (assert && !assert(opp_table, index))
>> + if (!assert(opp_table, index))
>> return ERR_PTR(-EINVAL);
>>
>> guard(mutex)(&opp_table->lock);
>>
>> list_for_each_entry(temp_opp, &opp_table->opp_list, node) {
>> if (temp_opp->available == available) {
>> - if (compare(&opp, temp_opp, read(temp_opp, index), *key))
>> + if (compare(&opp, temp_opp, read(temp_opp, index), *key)) {
>> + /* Increment the reference count of OPP */
>> + *key = read(opp, index);
>> + dev_pm_opp_get(opp);
>> break;
>> + }
>> }
>> }
>>
>> - /* Increment the reference count of OPP */
>> - if (!IS_ERR(opp)) {
>> - *key = read(opp, index);
>> - dev_pm_opp_get(opp);
>> - }
>> -
>> return opp;
>> }
>>
>>
> Best regards
Thanks Marek for reporting.
Viresh,
looks like for compare_floor we need to iterate to the OPP table till
the OPP key is greater than the target key and return previous OPP.
In that case the updation of the key and dev_pm_opp_get() should be
outside as before. We need to remove this part of the patch.
Thanks & Regards,
Krishna Chaitanya.
On 25-08-25, 21:26, Krishna Chaitanya Chundru wrote: > looks like for compare_floor we need to iterate to the OPP table till > the OPP key is greater than the target key and return previous OPP. It depends on what kind of comparison we need to do. And this this is how it works for the _floor variants. But we don't always return the previous OPP. `compare` directly updates the `opp` and so it can be currently iterated one too. > In that case the updation of the key and dev_pm_opp_get() should be > outside as before. We need to remove this part of the patch. I think all we need to do here is check if an error is there or not, maybe I am misreading it. -- viresh
On 20-08-25, 13:58, Krishna Chaitanya Chundru wrote:
> Refactor _opp_table_find_key() to improve readability by moving the
> reference count increment and key update inside the match condition block.
>
> Also make the 'assert' check mandatory instead of treating it as optional.
>
> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
> ---
> drivers/opp/core.c | 14 ++++++--------
> 1 file changed, 6 insertions(+), 8 deletions(-)
Applied with:
@@ -554,8 +554,9 @@ static struct dev_pm_opp *_opp_table_find_key(struct opp_table *opp_table,
list_for_each_entry(temp_opp, &opp_table->opp_list, node) {
if (temp_opp->available == available) {
if (compare(&opp, temp_opp, read(temp_opp, index), *key)) {
- /* Increment the reference count of OPP */
*key = read(opp, index);
+
+ /* Increment the reference count of OPP */
dev_pm_opp_get(opp);
break;
}
--
viresh
© 2016 - 2025 Red Hat, Inc.