Re: [PATCH v4 03/24] PM / devfreq: tegra30: Handle possible round-rate error

[Dmitry Osipenko]

16.07.2019 14:50, Chanwoo Choi ÐÐÑÐÑ:
> On 19. 7. 8. ìì 7:32, Dmitry Osipenko wrote:
>> The EMC clock rate rounding technically could fail, hence let's handle
>> the error cases properly.
>>
>> Signed-off-by: Dmitry Osipenko <digetx@xxxxxxxxx>
>> ---
>>  drivers/devfreq/tegra30-devfreq.c | 17 +++++++++++++++--
>>  1 file changed, 15 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/devfreq/tegra30-devfreq.c b/drivers/devfreq/tegra30-devfreq.c
>> index 5e2b133babdd..5e606ae3f238 100644
>> --- a/drivers/devfreq/tegra30-devfreq.c
>> +++ b/drivers/devfreq/tegra30-devfreq.c
>> @@ -592,8 +592,8 @@ static int tegra_devfreq_probe(struct platform_device *pdev)
>>  	struct tegra_devfreq_device *dev;
>>  	struct tegra_devfreq *tegra;
>>  	struct devfreq *devfreq;
>> -	unsigned long rate;
>>  	unsigned int i;
>> +	long rate;
>>  	int err;
>>  
>>  	tegra = devm_kzalloc(&pdev->dev, sizeof(*tegra), GFP_KERNEL);
>> @@ -650,8 +650,14 @@ static int tegra_devfreq_probe(struct platform_device *pdev)
>>  
>>  	reset_control_deassert(tegra->reset);
>>  
>> -	tegra->max_freq = clk_round_rate(tegra->emc_clock, ULONG_MAX) / KHZ;
>> +	rate = clk_round_rate(tegra->emc_clock, ULONG_MAX);
>> +	if (rate < 0) {
>> +		dev_err(&pdev->dev, "Failed to round clock rate: %ld\n", rate);
>> +		return rate;
>> +	}
>> +
>>  	tegra->cur_freq = clk_get_rate(tegra->emc_clock) / KHZ;
>> +	tegra->max_freq = rate / KHZ;
>>  
>>  	for (i = 0; i < ARRAY_SIZE(actmon_device_configs); i++) {
>>  		dev = tegra->devices + i;
>> @@ -662,6 +668,13 @@ static int tegra_devfreq_probe(struct platform_device *pdev)
>>  	for (rate = 0; rate <= tegra->max_freq * KHZ; rate++) {
>>  		rate = clk_round_rate(tegra->emc_clock, rate);
>>  
> 
> Please remove unneeded blank line.

I can remove it, but it was added specifically to ease reading of the code.

>> +		if (rate < 0) {
>> +			dev_err(&pdev->dev,
>> +				"Failed to round clock rate: %ld\n", rate);
>> +			err = rate;
>> +			goto remove_opps;
>> +		}
> 
> Also, this patch doesn't contain code which restore the previous
> tegra->cur_freq/max_freq when error happen.

The error here results in abortion of the driver's probing, hence
nothing need to be restored in that case because nothing was changed at
this point yet.