[PATCH 04/31] cpufreq: imx: Migrate to dev_pm_opp_set_config()

[PATCH 04/31] cpufreq: imx: Migrate to dev_pm_opp_set_config()

[Viresh Kumar]

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/cpufreq/imx-cpufreq-dt.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/cpufreq/imx-cpufreq-dt.c b/drivers/cpufreq/imx-cpufreq-dt.c
index 3fe9125156b4..57917b0670f2 100644
--- a/drivers/cpufreq/imx-cpufreq-dt.c
+++ b/drivers/cpufreq/imx-cpufreq-dt.c
@@ -86,6 +86,10 @@ static int imx_cpufreq_dt_probe(struct platform_device *pdev)
 	u32 cell_value, supported_hw[2];
 	int speed_grade, mkt_segment;
 	int ret;
+	struct dev_pm_opp_config config = {
+		.supported_hw = supported_hw,
+		.supported_hw_count = ARRAY_SIZE(supported_hw),
+	};
 
 	cpu_dev = get_cpu_device(0);
 
@@ -153,17 +157,17 @@ static int imx_cpufreq_dt_probe(struct platform_device *pdev)
 	dev_info(&pdev->dev, "cpu speed grade %d mkt segment %d supported-hw %#x %#x\n",
 			speed_grade, mkt_segment, supported_hw[0], supported_hw[1]);
 
-	cpufreq_opp_table = dev_pm_opp_set_supported_hw(cpu_dev, supported_hw, 2);
+	cpufreq_opp_table = dev_pm_opp_set_config(cpu_dev, &config);
 	if (IS_ERR(cpufreq_opp_table)) {
 		ret = PTR_ERR(cpufreq_opp_table);
-		dev_err(&pdev->dev, "Failed to set supported opp: %d\n", ret);
+		dev_err(&pdev->dev, "Failed to set Opp config: %d\n", ret);
 		return ret;
 	}
 
 	cpufreq_dt_pdev = platform_device_register_data(
 			&pdev->dev, "cpufreq-dt", -1, NULL, 0);
 	if (IS_ERR(cpufreq_dt_pdev)) {
-		dev_pm_opp_put_supported_hw(cpufreq_opp_table);
+		dev_pm_opp_clear_config(cpufreq_opp_table);
 		ret = PTR_ERR(cpufreq_dt_pdev);
 		dev_err(&pdev->dev, "Failed to register cpufreq-dt: %d\n", ret);
 		return ret;
@@ -176,7 +180,7 @@ static int imx_cpufreq_dt_remove(struct platform_device *pdev)
 {
 	platform_device_unregister(cpufreq_dt_pdev);
 	if (!of_machine_is_compatible("fsl,imx7ulp"))
-		dev_pm_opp_put_supported_hw(cpufreq_opp_table);
+		dev_pm_opp_clear_config(cpufreq_opp_table);
 	else
 		clk_bulk_put(ARRAY_SIZE(imx7ulp_clks), imx7ulp_clks);
 
-- 
2.31.1.272.g89b43f80a514


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 06/31] cpufreq: sti: Migrate to dev_pm_opp_set_config()

[Viresh Kumar]

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/cpufreq/sti-cpufreq.c | 22 ++++++++--------------
 1 file changed, 8 insertions(+), 14 deletions(-)

diff --git a/drivers/cpufreq/sti-cpufreq.c b/drivers/cpufreq/sti-cpufreq.c
index fdb0a722d881..f4121a9d27e5 100644
--- a/drivers/cpufreq/sti-cpufreq.c
+++ b/drivers/cpufreq/sti-cpufreq.c
@@ -159,6 +159,11 @@ static int sti_cpufreq_set_opp_info(void)
 	int ret;
 	char name[MAX_PCODE_NAME_LEN];
 	struct opp_table *opp_table;
+	struct dev_pm_opp_config config = {
+		.supported_hw = version,
+		.supported_hw_count = ARRAY_SIZE(version),
+		.prop_name = name,
+	};
 
 	reg_fields = sti_cpufreq_match();
 	if (!reg_fields) {
@@ -210,21 +215,14 @@ static int sti_cpufreq_set_opp_info(void)
 
 	snprintf(name, MAX_PCODE_NAME_LEN, "pcode%d", pcode);
 
-	opp_table = dev_pm_opp_set_prop_name(dev, name);
-	if (IS_ERR(opp_table)) {
-		dev_err(dev, "Failed to set prop name\n");
-		return PTR_ERR(opp_table);
-	}
-
 	version[0] = BIT(major);
 	version[1] = BIT(minor);
 	version[2] = BIT(substrate);
 
-	opp_table = dev_pm_opp_set_supported_hw(dev, version, VERSION_ELEMENTS);
+	opp_table = dev_pm_opp_set_config(dev, &config);
 	if (IS_ERR(opp_table)) {
-		dev_err(dev, "Failed to set supported hardware\n");
-		ret = PTR_ERR(opp_table);
-		goto err_put_prop_name;
+		dev_err(dev, "Failed to set OPP config\n");
+		return PTR_ERR(opp_table);
 	}
 
 	dev_dbg(dev, "pcode: %d major: %d minor: %d substrate: %d\n",
@@ -233,10 +231,6 @@ static int sti_cpufreq_set_opp_info(void)
 		version[0], version[1], version[2]);
 
 	return 0;
-
-err_put_prop_name:
-	dev_pm_opp_put_prop_name(opp_table);
-	return ret;
 }
 
 static int sti_cpufreq_fetch_syscon_registers(void)
-- 
2.31.1.272.g89b43f80a514


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 07/31] cpufreq: sun50i: Migrate to dev_pm_opp_set_config()

[Viresh Kumar]

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/cpufreq/sun50i-cpufreq-nvmem.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/cpufreq/sun50i-cpufreq-nvmem.c b/drivers/cpufreq/sun50i-cpufreq-nvmem.c
index 2deed8d8773f..c1bee39758e2 100644
--- a/drivers/cpufreq/sun50i-cpufreq-nvmem.c
+++ b/drivers/cpufreq/sun50i-cpufreq-nvmem.c
@@ -104,6 +104,9 @@ static int sun50i_cpufreq_nvmem_probe(struct platform_device *pdev)
 	snprintf(name, MAX_NAME_LEN, "speed%d", speed);
 
 	for_each_possible_cpu(cpu) {
+		struct dev_pm_opp_config config = {
+			.prop_name = name,
+		};
 		struct device *cpu_dev = get_cpu_device(cpu);
 
 		if (!cpu_dev) {
@@ -111,10 +114,10 @@ static int sun50i_cpufreq_nvmem_probe(struct platform_device *pdev)
 			goto free_opp;
 		}
 
-		opp_tables[cpu] = dev_pm_opp_set_prop_name(cpu_dev, name);
+		opp_tables[cpu] = dev_pm_opp_set_config(cpu_dev, &config);
 		if (IS_ERR(opp_tables[cpu])) {
 			ret = PTR_ERR(opp_tables[cpu]);
-			pr_err("Failed to set prop name\n");
+			pr_err("Failed to set OPP config\n");
 			goto free_opp;
 		}
 	}
@@ -133,7 +136,7 @@ static int sun50i_cpufreq_nvmem_probe(struct platform_device *pdev)
 	for_each_possible_cpu(cpu) {
 		if (IS_ERR_OR_NULL(opp_tables[cpu]))
 			break;
-		dev_pm_opp_put_prop_name(opp_tables[cpu]);
+		dev_pm_opp_clear_config(opp_tables[cpu]);
 	}
 	kfree(opp_tables);
 
@@ -148,7 +151,7 @@ static int sun50i_cpufreq_nvmem_remove(struct platform_device *pdev)
 	platform_device_unregister(cpufreq_dt_pdev);
 
 	for_each_possible_cpu(cpu)
-		dev_pm_opp_put_prop_name(opp_tables[cpu]);
+		dev_pm_opp_clear_config(opp_tables[cpu]);
 
 	kfree(opp_tables);
 
-- 
2.31.1.272.g89b43f80a514


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Viresh Kumar]

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;
-- 
2.31.1.272.g89b43f80a514


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 11/31] devfreq: sun8i: Migrate to dev_pm_opp_set_config()

[Viresh Kumar]

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/sun8i-a33-mbus.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/devfreq/sun8i-a33-mbus.c b/drivers/devfreq/sun8i-a33-mbus.c
index 13d32213139f..125b479c9d6d 100644
--- a/drivers/devfreq/sun8i-a33-mbus.c
+++ b/drivers/devfreq/sun8i-a33-mbus.c
@@ -337,6 +337,9 @@ static int sun8i_a33_mbus_probe(struct platform_device *pdev)
 	unsigned int max_state;
 	const char *err;
 	int i, ret;
+	struct dev_pm_opp_config config = {
+		.clk_name = "dram",
+	};
 
 	variant = device_get_match_data(dev);
 	if (!variant)
@@ -404,9 +407,9 @@ static int sun8i_a33_mbus_probe(struct platform_device *pdev)
 	priv->profile.freq_table	= priv->freq_table;
 	priv->profile.max_state		= max_state;
 
-	ret = devm_pm_opp_set_clkname(dev, "dram");
+	ret = devm_pm_opp_set_config(dev, &config);
 	if (ret) {
-		err = "failed to add OPP table\n";
+		err = "failed to set OPP config\n";
 		goto err_unlock_mbus;
 	}
 
-- 
2.31.1.272.g89b43f80a514


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Chanwoo Choi]

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)



-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Chanwoo Choi]

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Viresh Kumar]

On 31-05-22, 13:12, Chanwoo Choi wrote:
> 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);

Does this fixes it for you ?

It isn't allowed to call this routine with opp_table as NULL, I should
rather have a WARN() for the same instead.

Can you check why exynos is passing NULL here as I don't see an
obvious reason currently.

-- 
viresh

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Viresh Kumar]

On 31-05-22, 09:45, Viresh Kumar wrote:
> On 31-05-22, 13:12, Chanwoo Choi wrote:
> > 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);
> 
> Does this fixes it for you ?
> 
> It isn't allowed to call this routine with opp_table as NULL, I should
> rather have a WARN() for the same instead.
> 
> Can you check why exynos is passing NULL here as I don't see an
> obvious reason currently.

Looking at the exynos devfreq driver again, it feels like the design
of the driver itself is causing all these issues.

Ideally, whatever resources are acquired by probe() must be freed only
and only by remove()/shutdown(). But you are trying to do it from
exynos_bus_exit() as well. Calling dev_pm_opp_of_remove_table() as
well from this function is wrong as you may very well end up
corrupting the OPP refcount and OPP may never get freed or something
else may come up.

For now I am adding following to the patch, please see if it fixes it
or not (I have pushed the changes to my branch as well).

diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c
index 780e525eb92a..8fca24565e7d 100644
--- a/drivers/devfreq/exynos-bus.c
+++ b/drivers/devfreq/exynos-bus.c
@@ -161,8 +161,11 @@ static void exynos_bus_exit(struct device *dev)

        dev_pm_opp_of_remove_table(dev);
        clk_disable_unprepare(bus->clk);
-       dev_pm_opp_clear_config(bus->opp_table);
-       bus->opp_table = NULL;
+
+       if (bus->opp_table) {
+               dev_pm_opp_clear_config(bus->opp_table);
+               bus->opp_table = NULL;
+       }
 }

 static void exynos_bus_passive_exit(struct device *dev)
@@ -463,8 +466,10 @@ 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_clear_config(bus->opp_table);
-       bus->opp_table = NULL;
+       if (bus->opp_table) {
+               dev_pm_opp_clear_config(bus->opp_table);
+               bus->opp_table = NULL;
+       }

        return ret;
 }

-- 
viresh

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Chanwoo Choi]

On 5/31/22 1:15 PM, Viresh Kumar wrote:
> On 31-05-22, 13:12, Chanwoo Choi wrote:
>> 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);
> 
> Does this fixes it for you ?
> 
> It isn't allowed to call this routine with opp_table as NULL, I should
> rather have a WARN() for the same instead.
> 
> Can you check why exynos is passing NULL here as I don't see an
> obvious reason currently.
> 

exynos-bus.c contains the two type of devfreq device as following:
1. devfreq device controls the regulator
2. devfreq device doesn't control the regulator

Above two devfreq devices share the same error path
because two devices are similar.

As you said, if you use WARN(),
I can change it on exynos-bus.c as following:

if (bus->opp_table)
    dev_pm_opp_clear_config(bus->opp_table)


-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Chanwoo Choi]

On 5/31/22 1:38 PM, Viresh Kumar wrote:
> On 31-05-22, 09:45, Viresh Kumar wrote:
>> On 31-05-22, 13:12, Chanwoo Choi wrote:
>>> 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);
>>
>> Does this fixes it for you ?
>>
>> It isn't allowed to call this routine with opp_table as NULL, I should
>> rather have a WARN() for the same instead.
>>
>> Can you check why exynos is passing NULL here as I don't see an
>> obvious reason currently.
> 
> Looking at the exynos devfreq driver again, it feels like the design
> of the driver itself is causing all these issues.
> 
> Ideally, whatever resources are acquired by probe() must be freed only
> and only by remove()/shutdown(). But you are trying to do it from
> exynos_bus_exit() as well. Calling dev_pm_opp_of_remove_table() as
> well from this function is wrong as you may very well end up
> corrupting the OPP refcount and OPP may never get freed or something
> else may come up.
> 
> For now I am adding following to the patch, please see if it fixes it
> or not (I have pushed the changes to my branch as well).
> 
> diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c
> index 780e525eb92a..8fca24565e7d 100644
> --- a/drivers/devfreq/exynos-bus.c
> +++ b/drivers/devfreq/exynos-bus.c
> @@ -161,8 +161,11 @@ static void exynos_bus_exit(struct device *dev)
> 
>         dev_pm_opp_of_remove_table(dev);
>         clk_disable_unprepare(bus->clk);
> -       dev_pm_opp_clear_config(bus->opp_table);
> -       bus->opp_table = NULL;
> +
> +       if (bus->opp_table) {
> +               dev_pm_opp_clear_config(bus->opp_table);
> +               bus->opp_table = NULL;
> +       }
>  }
> 
>  static void exynos_bus_passive_exit(struct device *dev)
> @@ -463,8 +466,10 @@ 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_clear_config(bus->opp_table);
> -       bus->opp_table = NULL;
> +       if (bus->opp_table) {
> +               dev_pm_opp_clear_config(bus->opp_table);
> +               bus->opp_table = NULL;
> +       }
> 
>         return ret;
>  }
> 

This change is enough to remove the null pointer error. Thanks.

-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Viresh Kumar]

On 31-05-22, 14:05, Chanwoo Choi wrote:
> > diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c
> > index 780e525eb92a..8fca24565e7d 100644
> > --- a/drivers/devfreq/exynos-bus.c
> > +++ b/drivers/devfreq/exynos-bus.c
> > @@ -161,8 +161,11 @@ static void exynos_bus_exit(struct device *dev)
> > 
> >         dev_pm_opp_of_remove_table(dev);
> >         clk_disable_unprepare(bus->clk);
> > -       dev_pm_opp_clear_config(bus->opp_table);
> > -       bus->opp_table = NULL;
> > +
> > +       if (bus->opp_table) {
> > +               dev_pm_opp_clear_config(bus->opp_table);
> > +               bus->opp_table = NULL;
> > +       }
> >  }
> > 
> >  static void exynos_bus_passive_exit(struct device *dev)
> > @@ -463,8 +466,10 @@ 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_clear_config(bus->opp_table);
> > -       bus->opp_table = NULL;
> > +       if (bus->opp_table) {
> > +               dev_pm_opp_clear_config(bus->opp_table);
> > +               bus->opp_table = NULL;
> > +       }
> > 
> >         return ret;
> >  }
> > 
> 
> This change is enough to remove the null pointer error. Thanks.

Pushed this and WARN_ON() in OPP core. Thanks.

-- 
viresh

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel