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

[Matthias Kaehlcke <mka@chromium.org>]

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 <leonard.crestez@nxp.com>
> > Reviewed-by: Matthias Kaehlcke <mka@chromium.org>
> > ---
> >  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?
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.