Re: [PATCH 10/31] devfreq: exynos: Migrate to dev_pm_opp_set_config()

[Chanwoo Choi <cw00.choi@samsung.com>]

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@linaro.org>
>> ---
>>  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