Skip to contentSkip to conversation

cpufreq: tegra20: Use resource-managed API

PATCH Applied Dmitry Osipenko 2 participants 4 messages Jan 17, 2021 – Jan 20, 2021

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 cpufreq: tegra20: Use resource-managed API

Switch cpufreq-tegra20 driver to use resource-managed API. This removes the need to get opp_table pointer using dev_pm_opp_get_opp_table() in order to release OPP table that was requested by dev_pm_opp_set_supported_hw(), making the code a bit more straightforward.

---
 drivers/cpufreq/tegra20-cpufreq.c | 45 +++++++++++++++----------------
 1 file changed, 22 insertions(+), 23 deletions(-)
drivers/cpufreq/tegra20-cpufreq.c +22 -23
index 8c893043953e..e8db3d75be25 100644
@@ -32,6 +32,16 @@ static bool cpu0_node_has_opp_v2_prop(void)
 	return ret;
 }
 
+static void tegra20_cpufreq_put_supported_hw(void *opp_table)
+{
+	dev_pm_opp_put_supported_hw(opp_table);
+}
+
+static void tegra20_cpufreq_dt_unregister(void *cpufreq_dt)
+{
+	platform_device_unregister(cpufreq_dt);
+}
+
 static int tegra20_cpufreq_probe(struct platform_device *pdev)
 {
 	struct platform_device *cpufreq_dt;
@@ -68,42 +78,31 @@ static int tegra20_cpufreq_probe(struct platform_device *pdev)
 		return err;
 	}
 
+	err = devm_add_action_or_reset(&pdev->dev,
+				       tegra20_cpufreq_put_supported_hw,
+				       opp_table);
+	if (err)
+		return err;
+
 	cpufreq_dt = platform_device_register_simple("cpufreq-dt", -1, NULL, 0);
 	err = PTR_ERR_OR_ZERO(cpufreq_dt);
 	if (err) {
 		dev_err(&pdev->dev,
 			"failed to create cpufreq-dt device: %d\n", err);
-		goto err_put_supported_hw;
+		return err;
 	}
 
-	platform_set_drvdata(pdev, cpufreq_dt);
-
-	return 0;
-
-err_put_supported_hw:
-	dev_pm_opp_put_supported_hw(opp_table);
-
-	return err;
-}
-
-static int tegra20_cpufreq_remove(struct platform_device *pdev)
-{
-	struct platform_device *cpufreq_dt;
-	struct opp_table *opp_table;
-
-	cpufreq_dt = platform_get_drvdata(pdev);
-	platform_device_unregister(cpufreq_dt);
-
-	opp_table = dev_pm_opp_get_opp_table(get_cpu_device(0));
-	dev_pm_opp_put_supported_hw(opp_table);
-	dev_pm_opp_put_opp_table(opp_table);
+	err = devm_add_action_or_reset(&pdev->dev,
+				       tegra20_cpufreq_dt_unregister,
+				       cpufreq_dt);
+	if (err)
+		return err;
 
 	return 0;
 }
 
 static struct platform_driver tegra20_cpufreq_driver = {
 	.probe		= tegra20_cpufreq_probe,
-	.remove		= tegra20_cpufreq_remove,
 	.driver		= {
 		.name	= "tegra20-cpufreq",
 	},
Signature · 1 line
2.29.2
maintainer Applied
#1 lore raw
Quoted 77 lines · Dmitry Osipenko

Switch cpufreq-tegra20 driver to use resource-managed API. This removes the need to get opp_table pointer using dev_pm_opp_get_opp_table() in order to release OPP table that was requested by dev_pm_opp_set_supported_hw(), making the code a bit more straightforward.

---
 drivers/cpufreq/tegra20-cpufreq.c | 45 +++++++++++++++----------------
 1 file changed, 22 insertions(+), 23 deletions(-)
drivers/cpufreq/tegra20-cpufreq.c +22 -22
index 8c893043953e..e8db3d75be25 100644
@@ -32,6 +32,16 @@ static bool cpu0_node_has_opp_v2_prop(void)
 	return ret;
 }
 
+static void tegra20_cpufreq_put_supported_hw(void *opp_table)
+{
+	dev_pm_opp_put_supported_hw(opp_table);
+}
+
+static void tegra20_cpufreq_dt_unregister(void *cpufreq_dt)
+{
+	platform_device_unregister(cpufreq_dt);
+}
+
 static int tegra20_cpufreq_probe(struct platform_device *pdev)
 {
 	struct platform_device *cpufreq_dt;
@@ -68,42 +78,31 @@ static int tegra20_cpufreq_probe(struct platform_device *pdev)
 		return err;
 	}
 
+	err = devm_add_action_or_reset(&pdev->dev,
+				       tegra20_cpufreq_put_supported_hw,
+				       opp_table);
+	if (err)
+		return err;
+
 	cpufreq_dt = platform_device_register_simple("cpufreq-dt", -1, NULL, 0);
 	err = PTR_ERR_OR_ZERO(cpufreq_dt);
 	if (err) {
 		dev_err(&pdev->dev,
 			"failed to create cpufreq-dt device: %d\n", err);
-		goto err_put_supported_hw;
+		return err;
 	}
 
-	platform_set_drvdata(pdev, cpufreq_dt);
-
-	return 0;
-
-err_put_supported_hw:
-	dev_pm_opp_put_supported_hw(opp_table);
-
-	return err;
-}
-
-static int tegra20_cpufreq_remove(struct platform_device *pdev)
-{
-	struct platform_device *cpufreq_dt;
-	struct opp_table *opp_table;
-
-	cpufreq_dt = platform_get_drvdata(pdev);
-	platform_device_unregister(cpufreq_dt);
-
-	opp_table = dev_pm_opp_get_opp_table(get_cpu_device(0));
-	dev_pm_opp_put_supported_hw(opp_table);
-	dev_pm_opp_put_opp_table(opp_table);
+	err = devm_add_action_or_reset(&pdev->dev,
+				       tegra20_cpufreq_dt_unregister,
+				       cpufreq_dt);
+	if (err)
+		return err;

drivers/cpufreq/tegra20-cpufreq.c static int tegra20_cpufreq_probe(struct platform_device *pdev) · line 108

Quoted 10 lines
drivers/cpufreq/tegra20-cpufreq.c +0 -1
 
 	return 0;
 }
 
 static struct platform_driver tegra20_cpufreq_driver = {
 	.probe		= tegra20_cpufreq_probe,
-	.remove		= tegra20_cpufreq_remove,
 	.driver		= {
 		.name	= "tegra20-cpufreq",
 	},

Applied. Thanks.

Though please remember to update this to use the devm_ variant when it comes out.

Signature · 1 line
viresh
#2 lore raw

18.01.2021 09:39, Viresh Kumar пишет:

Quoted 91 lines · Dmitry Osipenko
Quoted 77 lines

Switch cpufreq-tegra20 driver to use resource-managed API. This removes the need to get opp_table pointer using dev_pm_opp_get_opp_table() in order to release OPP table that was requested by dev_pm_opp_set_supported_hw(), making the code a bit more straightforward.

---
 drivers/cpufreq/tegra20-cpufreq.c | 45 +++++++++++++++----------------
 1 file changed, 22 insertions(+), 23 deletions(-)
drivers/cpufreq/tegra20-cpufreq.c +22 -22
index 8c893043953e..e8db3d75be25 100644
@@ -32,6 +32,16 @@ static bool cpu0_node_has_opp_v2_prop(void)
 	return ret;
 }
 
+static void tegra20_cpufreq_put_supported_hw(void *opp_table)
+{
+	dev_pm_opp_put_supported_hw(opp_table);
+}
+
+static void tegra20_cpufreq_dt_unregister(void *cpufreq_dt)
+{
+	platform_device_unregister(cpufreq_dt);
+}
+
 static int tegra20_cpufreq_probe(struct platform_device *pdev)
 {
 	struct platform_device *cpufreq_dt;
@@ -68,42 +78,31 @@ static int tegra20_cpufreq_probe(struct platform_device *pdev)
 		return err;
 	}
 
+	err = devm_add_action_or_reset(&pdev->dev,
+				       tegra20_cpufreq_put_supported_hw,
+				       opp_table);
+	if (err)
+		return err;
+
 	cpufreq_dt = platform_device_register_simple("cpufreq-dt", -1, NULL, 0);
 	err = PTR_ERR_OR_ZERO(cpufreq_dt);
 	if (err) {
 		dev_err(&pdev->dev,
 			"failed to create cpufreq-dt device: %d\n", err);
-		goto err_put_supported_hw;
+		return err;
 	}
 
-	platform_set_drvdata(pdev, cpufreq_dt);
-
-	return 0;
-
-err_put_supported_hw:
-	dev_pm_opp_put_supported_hw(opp_table);
-
-	return err;
-}
-
-static int tegra20_cpufreq_remove(struct platform_device *pdev)
-{
-	struct platform_device *cpufreq_dt;
-	struct opp_table *opp_table;
-
-	cpufreq_dt = platform_get_drvdata(pdev);
-	platform_device_unregister(cpufreq_dt);
-
-	opp_table = dev_pm_opp_get_opp_table(get_cpu_device(0));
-	dev_pm_opp_put_supported_hw(opp_table);
-	dev_pm_opp_put_opp_table(opp_table);
+	err = devm_add_action_or_reset(&pdev->dev,
+				       tegra20_cpufreq_dt_unregister,
+				       cpufreq_dt);
+	if (err)
+		return err;

drivers/cpufreq/tegra20-cpufreq.c static int tegra20_cpufreq_probe(struct platform_device *pdev) · line 108

Quoted 10 lines
drivers/cpufreq/tegra20-cpufreq.c +0 -1
 
 	return 0;
 }
 
 static struct platform_driver tegra20_cpufreq_driver = {
 	.probe		= tegra20_cpufreq_probe,
-	.remove		= tegra20_cpufreq_remove,
 	.driver		= {
 		.name	= "tegra20-cpufreq",
 	},

Applied. Thanks.

Quoted 2 lines

Though please remember to update this to use the devm_ variant when it comes out.

The regular devm_opp_* helpers won't be usable for CPUFreq drivers because OPP is applied to the CPU device and not the device of the CPUFreq driver.

But maybe we could support such cases by the helpers?

I CC'd Yangtao Li.

For example we could do this:

drivers/opp/core.c +4 -2
index ca8c6acc29f4..e2eb0f9cf46c 100644
@@ -1771,7 +1771,8 @@ static void devm_pm_opp_supported_hw_release(void *data)
 
 /**
  * devm_pm_opp_set_supported_hw() - Set supported platforms
- * @dev: Device for which supported-hw has to be set.
+ * @dev: Device which manages allocated resources.
+ * @opp_dev: Device for which supported-hw has to be set.
  * @versions: Array of hierarchy of versions to match.
  * @count: Number of elements in the array.
  *
@@ -1783,13 +1784,14 @@ static void devm_pm_opp_supported_hw_release(void *data)
  * The opp_table structure will be freed after the device is destroyed.
  */
 struct opp_table *devm_pm_opp_set_supported_hw(struct device *dev,
+					       struct device *opp_dev,
 					       const u32 *versions,
 					       unsigned int count)
 {
 	struct opp_table *opp_table;
 	int err;
 
-	opp_table = dev_pm_opp_set_supported_hw(dev, versions, count);
+	opp_table = dev_pm_opp_set_supported_hw(opp_dev, versions, count);
 	if (IS_ERR(opp_table))
 		return opp_table;
maintainer
#3 lore raw
Quoted 1 line · Dmitry Osipenko

The regular devm_opp_* helpers won't be usable for CPUFreq drivers because OPP is applied to the CPU device and not the device of the CPUFreq driver.

Ahh, I missed that.

Quoted 25 lines

But maybe we could support such cases by the helpers?

I CC'd Yangtao Li.

For example we could do this:

drivers/opp/core.c +3 -1
index ca8c6acc29f4..e2eb0f9cf46c 100644
@@ -1771,7 +1771,8 @@ static void devm_pm_opp_supported_hw_release(void *data)
 
 /**
  * devm_pm_opp_set_supported_hw() - Set supported platforms
- * @dev: Device for which supported-hw has to be set.
+ * @dev: Device which manages allocated resources.
+ * @opp_dev: Device for which supported-hw has to be set.
  * @versions: Array of hierarchy of versions to match.
  * @count: Number of elements in the array.
  *
@@ -1783,13 +1784,14 @@ static void devm_pm_opp_supported_hw_release(void *data)
  * The opp_table structure will be freed after the device is destroyed.
  */
 struct opp_table *devm_pm_opp_set_supported_hw(struct device *dev,
+					       struct device *opp_dev,

drivers/opp/core.c static void devm_pm_opp_supported_hw_release(void *data) · line 1796

Quoted 10 lines
drivers/opp/core.c +1 -1
 					       const u32 *versions,
 					       unsigned int count)
 {
 	struct opp_table *opp_table;
 	int err;
 
-	opp_table = dev_pm_opp_set_supported_hw(dev, versions, count);
+	opp_table = dev_pm_opp_set_supported_hw(opp_dev, versions, count);
 	if (IS_ERR(opp_table))
 		return opp_table;

No please :)

Signature · 1 line
viresh