Re: [PATCH] devfreq: do not ignore errors during store min, max frequency

[Lukasz Luba <lukasz.luba@arm.com>]

On 28/04/17 00:41, MyungJoo Ham wrote:
>>>
>>> ...
>>>
>>>> +	ret = devfreq_get_freq_level(df, value);
>>>> +	if (ret < 0) {
>>>> +		dev_warn(dev, "Storing min freq failed with (%d) error\n", ret);
>>>> +		goto unlock;
>>>> +	}
>>>> +
>>>
>>> This is not good.
>>>
>>> For a device that supports [ 100, 400, 800, 1000 ] MHz,
>>> saying "min = 200 Mhz" or "max = 600 MHz" shouldn't be prohibited.
>>>
>>> Those functions are to express lower bound and upper bound, not to
>>> designate the exact operating frequencies.
>>
>> Would it be possible to convince you to store a
>> 'posible/valid frequency' (from OPP) in that value?
>> Governors (like simpleondemand) and drivers use it with the flags.
>> It would be useful for thermal subsystem. It could have the max/min for
>> the devfreq device.
>>
>> I could prepare a patch which sets the freq from OPP respecting
>> rounding up/down based on 'devfreq_recommended_opp()'.
>>
>> Regards,
>> Lukasz
>
> Before talking about implementation detail,
> what's the benefit of what you are suggesting?
>
> What's the scenario that you cannot do with the current system
> while you can do with what you suggest?
>
> Why do you need rounded (OPP-enabled) max/min values?
> Why unrounded (lower/upper limits) max/min values cannot do what you want?
>
>
> Cheers,
> MyungJoo
>
>
> ps. I'll be away from the network for about 10 days after few hours.
>
i.e. when you look at the function:
devfreq_cooling_get_max_state() in drivers/thermal/devfreq_cooling
you will see that it is not aware of any changes
(there are some other issues as well).
I am going to modify devfreq_cooling a bit.

I thought it would be good to reuse the df->max_freq
as much as possible.

Regards,
Lukasz Luba