Re: [PATCH 1/2] cpufreq: Return error if ->get() failed in cpufreq_update_policy()

[Viresh Kumar]

On 17 February 2014 05:58, Rafael J. Wysocki <rjw@xxxxxxxxxxxxx> wrote:
> On Friday, February 14, 2014 04:30:40 PM Viresh Kumar wrote:

> Good to know that you chat with each other, but it really is not a useful piece
> of information until you say what *exactly* you were talking about.

All that is mentioned in commit logs of both the patches :) .. That's all we
discussed.

>>  drivers/cpufreq/cpufreq.c | 7 +++++++
>>  1 file changed, 7 insertions(+)
>>
>> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
>> index 08ca8c9..383362b 100644
>> --- a/drivers/cpufreq/cpufreq.c
>> +++ b/drivers/cpufreq/cpufreq.c
>> @@ -2151,6 +2151,13 @@ int cpufreq_update_policy(unsigned int cpu)
>>        */
>>       if (cpufreq_driver->get) {
>>               new_policy.cur = cpufreq_driver->get(cpu);
>> +
>> +             if (!new_policy.cur) {
>> +                     pr_err("%s: ->get() returned 0 KHz\n", __func__);
>> +                     ret = -EINVAL;
>
> That isn't -EINVAL.  It may be -EIO or -ENODEV, but not -EINVAL.  Please.

Hmm.. Correct. I will use EIO then..

>> +                     goto no_policy;
>
> And is it unsafe to continue here?  Or can we continue regardless of getting 0?

We were supposed to set this frequency as the current frequency in policy->cur,
what else can we do now in this function when we aren't able to read current
freq? And so I thought that's all we can do here.

>> +             }
>> +
>>               if (!policy->cur) {
>>                       pr_debug("Driver did not initialize current freq");
>>                       policy->cur = new_policy.cur;
>>
>
> --
> I speak only for myself.
> Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/