Re: [PATCH 10/31] devfreq: exynos: Migrate to dev_pm_opp_set_config()
From: Chanwoo Choi
Date: Tue May 31 2022 - 00:12:55 EST
Hi,
On 5/30/22 6:45 PM, Chanwoo Choi wrote:
> Hi,
>
> On 5/26/22 8:42 PM, Viresh Kumar wrote:
>> The OPP core now provides a unified API for setting all configuration
>> types, i.e. dev_pm_opp_set_config().
>>
>> Lets start using it.
>>
>> Signed-off-by: Viresh Kumar <viresh.kumar@xxxxxxxxxx>
>> ---
>> drivers/devfreq/exynos-bus.c | 14 +++++++++-----
>> 1 file changed, 9 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c
>> index e689101abc93..780e525eb92a 100644
>> --- a/drivers/devfreq/exynos-bus.c
>> +++ b/drivers/devfreq/exynos-bus.c
>> @@ -161,7 +161,7 @@ static void exynos_bus_exit(struct device *dev)
>>
>> dev_pm_opp_of_remove_table(dev);
>> clk_disable_unprepare(bus->clk);
>> - dev_pm_opp_put_regulators(bus->opp_table);
>> + dev_pm_opp_clear_config(bus->opp_table);
>> bus->opp_table = NULL;
>> }
>>
>> @@ -182,11 +182,15 @@ static int exynos_bus_parent_parse_of(struct device_node *np,
>> struct opp_table *opp_table;
>> const char *vdd = "vdd";
>> int i, ret, count, size;
>> + struct dev_pm_opp_config config = {
>> + .regulator_names = &vdd,
>> + .regulator_count = 1,
>> + };
>>
>> - opp_table = dev_pm_opp_set_regulators(dev, &vdd, 1);
>> + opp_table = dev_pm_opp_set_config(dev, &config);
>> if (IS_ERR(opp_table)) {
>> ret = PTR_ERR(opp_table);
>> - dev_err(dev, "failed to set regulators %d\n", ret);
>> + dev_err(dev, "failed to set OPP config %d\n", ret);
>> return ret;
>> }
>>
>> @@ -236,7 +240,7 @@ static int exynos_bus_parent_parse_of(struct device_node *np,
>> return 0;
>>
>> err_regulator:
>> - dev_pm_opp_put_regulators(bus->opp_table);
>> + dev_pm_opp_clear_config(bus->opp_table);
>> bus->opp_table = NULL;
>>
>> return ret;
>> @@ -459,7 +463,7 @@ static int exynos_bus_probe(struct platform_device *pdev)
>> dev_pm_opp_of_remove_table(dev);
>> clk_disable_unprepare(bus->clk);
>> err_reg:
>> - dev_pm_opp_put_regulators(bus->opp_table);
>> + dev_pm_opp_clear_config(bus->opp_table);
>> bus->opp_table = NULL;
>>
>> return ret;
>>
>
> When I tested them with patch1/2 and patch10/11/12,
> I got the following message.
>
> [ 1.083669] Unable to handle kernel NULL pointer dereference at virtual address 000000b4
> [ 1.083683] [000000b4] *pgd=00000000
> [ 1.083719] Internal error: Oops: 5 [#1] PREEMPT SMP ARM
> [ 1.083735] Modules linked in:
> [ 1.083764] CPU: 6 PID: 1 Comm: swapper/0 Not tainted 5.18.0-11272-g7093c1f2a99c #2
> [ 1.083780] Hardware name: Samsung Exynos (Flattened Device Tree)
> [ 1.083797] PC is at dev_pm_opp_clear_config+0x10/0x90
> [ 1.083823] LR is at exynos_bus_probe+0x384/0x634
> [ 1.083843] pc : [<c0933740>] lr : [<c09a33e0>] psr: 60000013
> [ 1.083859] sp : f0835d38 ip : c1988000 fp : 00000001
> [ 1.083874] r10: c207bc10 r9 : c1b1a580 r8 : c160b708
> [ 1.083890] r7 : c207bc00 r6 : c1b1a580 r5 : fffffdfb r4 : 00000000
> [ 1.083907] r3 : c1988000 r2 : 00000000 r1 : 00000000 r0 : 00000000
> [ 1.083924] Flags: nZCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none
> [ 1.083940] Control: 10c5387d Table: 4000406a DAC: 00000051
> [ 1.083955] Register r0 information: NULL pointer
> [ 1.083989] Register r1 information: NULL pointer
> [ 1.084021] Register r2 information: NULL pointer
> [ 1.084053] Register r3 information: slab task_struct start c1988000 pointer offset 0
> [ 1.084126] Register r4 information: NULL pointer
> [ 1.084157] Register r5 information: non-paged memory
> [ 1.084189] Register r6 information: slab kmalloc-64 start c1b1a580 pointer offset 0 size 64
> [ 1.084270] Register r7 information: slab kmalloc-1k start c207bc00 pointer offset 0 size 1024
> [ 1.084351] Register r8 information: non-slab/vmalloc memory
> [ 1.084383] Register r9 information: slab kmalloc-64 start c1b1a580 pointer offset 0 size 64
> [ 1.084463] Register r10 information: slab kmalloc-1k start c207bc00 pointer offset 16 size 1024
> [ 1.084542] Register r11 information: non-paged memory
> [ 1.084573] Register r12 information: slab task_struct start c1988000 pointer offset 0
> [ 1.084635] Process swapper/0 (pid: 1, stack limit = 0x(ptrval))
> [ 1.084651] Stack: (0xf0835d38 to 0xf0836000)
> [ 1.084669] 5d20: c26b3940 c09a33e0
> [ 1.084687] 5d40: 00000000 f0835d9c a0000013 cfd97040 df8a6280 c0e031ec c17188f8 c0994758
> [ 1.084703] 5d60: 069db9c0 c0e0338c 60000013 c0e03380 c1784000 c0e0338c c17188f8 c0994758
> [ 1.084717] 5d80: f0835dc8 c160b708 ffffffff df8a6280 c12de76c c0994bb8 c181fdd4 df8a601c
> [ 1.084737] 5da0: 00000000 c160b708 c1718f84 c1765ea0 00000000 000001bb c1550860 c0995684
> [ 1.084755] 5dc0: ffffffff 00000000 00000000 00000000 00000000 00000000 00000000 00000000
> [ 1.084772] 5de0: 00000000 0f7634d1 00000000 c207bc10 00000000 c1718f84 c1765ea0 00000000
> [ 1.084787] 5e00: 000001bb c1550860 c1784000 c061fb8c c207bc10 00000000 c1718f84 c1765ea0
> [ 1.084803] 5e20: 00000000 c061d078 c207bc10 c062b764 c207bc10 c207bc10 c1718f84 c1718f84
> [ 1.084818] 5e40: c1765ea0 c061d3e0 c1988000 000001bb c1550860 c17cb200 c17cb204 c1718f84
> [ 1.084834] 5e60: c207bc10 00000000 000001bb c1550860 c1784000 c061d588 00000000 c207bc10
> [ 1.084849] 5e80: c1718f84 c160b708 c1988000 c061dabc 00000000 c1718f84 c061d9d0 c061ac7c
> [ 1.084865] 5ea0: c1784000 c2040074 c2054bc0 0f7634d1 c1718f84 c1718f84 cfd92f00 c16edb98
> [ 1.084882] 5ec0: 00000000 c061c050 c1367300 c1758040 c1533c54 c1718f84 c1758040 c1533c54
> [ 1.084895] 5ee0: 00000000 c061e568 c160b708 c1758040 c1533c54 c0102078 c0191390 c1500504
> [ 1.084911] 5f00: c1762f78 00000000 00000006 c132f7c4 c136690c 00000000 00000000 c160b708
> [ 1.084927] 5f20: c12a2e5c c1297aec 39360000 c202722b c2027233 0f7634d1 c168112c c1406728
> [ 1.084943] 5f40: 00000007 0f7634d1 c1406728 c15a7da0 00000007 c1550840 c2027200 c15014d0
> [ 1.084959] 5f60: 00000006 00000006 00000000 c1500504 00000000 c1500504 00000000 c160b700
> [ 1.084976] 5f80: c0df8c68 00000000 00000000 00000000 00000000 00000000 00000000 c0df8c88
> [ 1.084991] 5fa0: 00000000 c0df8c68 00000000 c0100148 00000000 00000000 00000000 00000000
> [ 1.085006] 5fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
> [ 1.085022] 5fe0: 00000000 00000000 00000000 00000000 00000013 00000000 00000000 00000000
> [ 1.085042] dev_pm_opp_clear_config from exynos_bus_probe+0x384/0x634
> [ 1.085063] exynos_bus_probe from platform_probe+0x64/0xc0
> [ 1.085085] platform_probe from really_probe+0x104/0x3c4
> [ 1.085109] really_probe from __driver_probe_device+0xa8/0x214
> [ 1.085130] __driver_probe_device from driver_probe_device+0x3c/0xdc
> [ 1.085149] driver_probe_device from __driver_attach+0xec/0x190
> [ 1.085170] __driver_attach from bus_for_each_dev+0x7c/0xbc
> [ 1.085191] bus_for_each_dev from bus_add_driver+0x17c/0x218
> [ 1.085211] bus_add_driver from driver_register+0x7c/0x110
> [ 1.085234] driver_register from do_one_initcall+0x50/0x268
> [ 1.085257] do_one_initcall from kernel_init_freeable+0x234/0x280
> [ 1.085279] kernel_init_freeable from kernel_init+0x20/0x140
> [ 1.085305] kernel_init from ret_from_fork+0x14/0x2c
> [ 1.085322] Exception stack(0xf0835fb0 to 0xf0835ff8)
I try to find the cause of this warning.
I think that dev_pm_opp_clear_config needs to check
whether 'opp_table' is NULL or not as following:
diff --git a/drivers/opp/core.c b/drivers/opp/core.c
index fba6e2b20b8f..cbf8f10b9ff0 100644
--- a/drivers/opp/core.c
+++ b/drivers/opp/core.c
@@ -2598,6 +2598,9 @@ EXPORT_SYMBOL_GPL(dev_pm_opp_set_config);
*/
void dev_pm_opp_clear_config(struct opp_table *opp_table)
{
+ if (unlikely(!opp_table))
+ return;
+
if (opp_table->genpd_virt_devs)
dev_pm_opp_detach_genpd(opp_table);
--
Best Regards,
Chanwoo Choi
Samsung Electronics