Subject: Re: [PATCH v9 6/8] PM / devfreq: Introduce get_freq_range helper

From: Chanwoo Choi <[email-masked]>
Date: Thu, 31 Oct 2019 11:44:25 +0900

On 19. 10. 12. 오전 3:29, Matthias Kaehlcke wrote:
> On Thu, Oct 03, 2019 at 11:19:38AM -0700, Matthias Kaehlcke wrote:
>> On Wed, Oct 02, 2019 at 10:25:09PM +0300, Leonard Crestez wrote:
>>> Moving handling of min/max freq to a single function and call it from
>>> update_devfreq and for printing min/max freq values in sysfs.
>>>
>>> This changes the behavior of out-of-range min_freq/max_freq: clamping
>>> is now done at evaluation time. This means that if an out-of-range
>>> constraint is imposed by sysfs and it later becomes valid then it will
>>> be enforced.
>>>
>>> Signed-off-by: Leonard Crestez <[email-masked]>
>>> Reviewed-by: Matthias Kaehlcke <[email-masked]>
>>> ---
>>>  drivers/devfreq/devfreq.c | 110 +++++++++++++++++++++-----------------
>>>  1 file changed, 62 insertions(+), 48 deletions(-)
>>>
>>> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
>>> index 87eff789ce24..2d63692903ff 100644
>>> --- a/drivers/devfreq/devfreq.c
>>> +++ b/drivers/devfreq/devfreq.c
>>>
>>> ...
>>>
>>>  static ssize_t min_freq_show(struct device *dev, struct device_attribute *attr,
>>>  			     char *buf)
>>>  {
>>>  	struct devfreq *df = to_devfreq(dev);
>>> +	unsigned long min_freq, max_freq;
>>>  
>>> -	return sprintf(buf, "%lu\n", max(df->scaling_min_freq, df->min_freq));
>>> +	mutex_lock(&df->lock);
>>> +	get_freq_range(df, &min_freq, &max_freq);
>>
>> With this min/max_freq shown aren't necessarily those set through sysfs,
>> but the aggregated PM QoS values (plus OPP constraints).
>>
>> I did some testing with a WIP patch that converts devfreq_cooling.c to
>> PM QoS. When reading sysfs min/max values to double check the limits
>> set earlier I found it utterly confusing to see the sysfs min/max values
>> fluctuating due to thermal throttling, and not being able to see the
>> configured values.
>>
>> Looks like cpufreq does the same, but I'm not convinced this is a good
>> idea. I think we want to display the values set by userspace, as done
>> before managing the limits through PM QoS. Viresh, was this change of
>> userspace visible behavior done intentionally for cpufreq?
> 
> ping
> 
> Viresh / devfreq maintainers:
> 
> Do we really want to expose through sysfs the potentially constantly
> changing aggregate min/max values, instead of those set by userspace?

Until now, the devfreq core show the available min/max frequency
through sysfs interface. I think the available min/max frequency is proper
for sysfs.

> At least for cpufreq that's a divergence from previous behavior, and
> from a userspace perspective it seems odd that you can't reliably read
> back the limit set by userspace.
> 
> 

-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

_______________________________________________
linux-arm-kernel mailing list
[email-masked]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel