Re: [PATCH v1] cpufreq: tegra20: Use resource-managed API

[Dmitry Osipenko <digetx@gmail.com>]

18.01.2021 09:39, Viresh Kumar пишет:
> On 18-01-21, 02:18, Dmitry Osipenko wrote:
>> 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.
>>
>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
>> ---
>>  drivers/cpufreq/tegra20-cpufreq.c | 45 +++++++++++++++----------------
>>  1 file changed, 22 insertions(+), 23 deletions(-)
>>
>> diff --git a/drivers/cpufreq/tegra20-cpufreq.c b/drivers/cpufreq/tegra20-cpufreq.c
>> index 8c893043953e..e8db3d75be25 100644
>> --- a/drivers/cpufreq/tegra20-cpufreq.c
>> +++ b/drivers/cpufreq/tegra20-cpufreq.c
>> @@ -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",
>>  	},
> 
> Applied. Thanks.
> 
> 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:

diff --git a/drivers/opp/core.c b/drivers/opp/core.c
index ca8c6acc29f4..e2eb0f9cf46c 100644
--- a/drivers/opp/core.c
+++ b/drivers/opp/core.c
@@ -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;