[PATCH] PM / devfreq: Remove redundant frequency adjustment from governors

[PATCH] PM / devfreq: Remove redundant frequency adjustment from governors

[Matthias Kaehlcke]

The performance, powersave, simpleondemand and userspace governors
determine a target frequency and then adjust it according to the
df->min/max_freq limits that might have been set by user space. This
adjustment is redundant, it is done in update_devfreq() for any
governor, right after governor->get_target_freq().

Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
---
 drivers/devfreq/governor_performance.c    | 10 ++--------
 drivers/devfreq/governor_powersave.c      |  5 -----
 drivers/devfreq/governor_simpleondemand.c |  7 +------
 drivers/devfreq/governor_userspace.c      | 16 ++++------------
 4 files changed, 7 insertions(+), 31 deletions(-)

diff --git a/drivers/devfreq/governor_performance.c b/drivers/devfreq/governor_performance.c
index 4d23ecfbd948..31ee30622c00 100644
--- a/drivers/devfreq/governor_performance.c
+++ b/drivers/devfreq/governor_performance.c
@@ -16,14 +16,8 @@
 static int devfreq_performance_func(struct devfreq *df,
 				    unsigned long *freq)
 {
-	/*
-	 * target callback should be able to get floor value as
-	 * said in devfreq.h
-	 */
-	if (!df->max_freq)
-		*freq = UINT_MAX;
-	else
-		*freq = df->max_freq;
+	*freq = UINT_MAX;
+
 	return 0;
 }
 
diff --git a/drivers/devfreq/governor_powersave.c b/drivers/devfreq/governor_powersave.c
index 0c42f23249ef..50a891f7c92d 100644
--- a/drivers/devfreq/governor_powersave.c
+++ b/drivers/devfreq/governor_powersave.c
@@ -16,11 +16,6 @@
 static int devfreq_powersave_func(struct devfreq *df,
 				  unsigned long *freq)
 {
-	/*
-	 * target callback should be able to get ceiling value as
-	 * said in devfreq.h
-	 */
-	*freq = df->min_freq;
 	return 0;
 }
 
diff --git a/drivers/devfreq/governor_simpleondemand.c b/drivers/devfreq/governor_simpleondemand.c
index 28e0f2de7100..7ed733c528f4 100644
--- a/drivers/devfreq/governor_simpleondemand.c
+++ b/drivers/devfreq/governor_simpleondemand.c
@@ -27,7 +27,7 @@ static int devfreq_simple_ondemand_func(struct devfreq *df,
 	unsigned int dfso_upthreshold = DFSO_UPTHRESHOLD;
 	unsigned int dfso_downdifferential = DFSO_DOWNDIFFERENCTIAL;
 	struct devfreq_simple_ondemand_data *data = df->data;
-	unsigned long max = (df->max_freq) ? df->max_freq : UINT_MAX;
+	unsigned long max = UINT_MAX;
 
 	err = devfreq_update_stats(df);
 	if (err)
@@ -85,11 +85,6 @@ static int devfreq_simple_ondemand_func(struct devfreq *df,
 	b = div_u64(b, (dfso_upthreshold - dfso_downdifferential / 2));
 	*freq = (unsigned long) b;
 
-	if (df->min_freq && *freq < df->min_freq)
-		*freq = df->min_freq;
-	if (df->max_freq && *freq > df->max_freq)
-		*freq = df->max_freq;
-
 	return 0;
 }
 
diff --git a/drivers/devfreq/governor_userspace.c b/drivers/devfreq/governor_userspace.c
index 080607c3f34d..378d84c011df 100644
--- a/drivers/devfreq/governor_userspace.c
+++ b/drivers/devfreq/governor_userspace.c
@@ -26,19 +26,11 @@ static int devfreq_userspace_func(struct devfreq *df, unsigned long *freq)
 {
 	struct userspace_data *data = df->data;
 
-	if (data->valid) {
-		unsigned long adjusted_freq = data->user_frequency;
-
-		if (df->max_freq && adjusted_freq > df->max_freq)
-			adjusted_freq = df->max_freq;
-
-		if (df->min_freq && adjusted_freq < df->min_freq)
-			adjusted_freq = df->min_freq;
-
-		*freq = adjusted_freq;
-	} else {
+	if (data->valid)
+		*freq = data->user_frequency;
+	else
 		*freq = df->previous_freq; /* No user freq specified yet */
-	}
+
 	return 0;
 }
 
-- 
2.17.0.441.gb46fe60e1d-goog

Re: [PATCH] PM / devfreq: Remove redundant frequency adjustment from governors

[Chanwoo Choi]

Hi,

On 2018년 05월 17일 06:10, Matthias Kaehlcke wrote:
> The performance, powersave, simpleondemand and userspace governors
> determine a target frequency and then adjust it according to the
> df->min/max_freq limits that might have been set by user space. This
> adjustment is redundant, it is done in update_devfreq() for any
> governor, right after governor->get_target_freq().
> 
> Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> ---
>  drivers/devfreq/governor_performance.c    | 10 ++--------
>  drivers/devfreq/governor_powersave.c      |  5 -----
>  drivers/devfreq/governor_simpleondemand.c |  7 +------
>  drivers/devfreq/governor_userspace.c      | 16 ++++------------
>  4 files changed, 7 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/devfreq/governor_performance.c b/drivers/devfreq/governor_performance.c
> index 4d23ecfbd948..31ee30622c00 100644
> --- a/drivers/devfreq/governor_performance.c
> +++ b/drivers/devfreq/governor_performance.c
> @@ -16,14 +16,8 @@
>  static int devfreq_performance_func(struct devfreq *df,
>  				    unsigned long *freq)
>  {
> -	/*
> -	 * target callback should be able to get floor value as
> -	 * said in devfreq.h
> -	 */
> -	if (!df->max_freq)
> -		*freq = UINT_MAX;
> -	else
> -		*freq = df->max_freq;
> +	*freq = UINT_MAX;
> +

It is difficult to understand why use UINT_MAX instead of df->max_freq.

Instead, after merged the commit ab8f58ad72c4 ("PM / devfreq: Set min/max_freq
when adding the devfreq device"), df->max/min_freq have the specific frequency
value always. So, we can change it as following without UINT_MAX. 

	*freq = df->max_freq;


>  	return 0;
>  }
>  
> diff --git a/drivers/devfreq/governor_powersave.c b/drivers/devfreq/governor_powersave.c
> index 0c42f23249ef..50a891f7c92d 100644
> --- a/drivers/devfreq/governor_powersave.c
> +++ b/drivers/devfreq/governor_powersave.c
> @@ -16,11 +16,6 @@
>  static int devfreq_powersave_func(struct devfreq *df,
>  				  unsigned long *freq)
>  {
> -	/*
> -	 * target callback should be able to get ceiling value as
> -	 * said in devfreq.h
> -	 */
> -	*freq = df->min_freq;
>  	return 0;

Each function have to keep their own function role.
Please keep '*freq = df->min_freq'.

>  }
>  
> diff --git a/drivers/devfreq/governor_simpleondemand.c b/drivers/devfreq/governor_simpleondemand.c
> index 28e0f2de7100..7ed733c528f4 100644
> --- a/drivers/devfreq/governor_simpleondemand.c
> +++ b/drivers/devfreq/governor_simpleondemand.c
> @@ -27,7 +27,7 @@ static int devfreq_simple_ondemand_func(struct devfreq *df,
>  	unsigned int dfso_upthreshold = DFSO_UPTHRESHOLD;
>  	unsigned int dfso_downdifferential = DFSO_DOWNDIFFERENCTIAL;
>  	struct devfreq_simple_ondemand_data *data = df->data;
> -	unsigned long max = (df->max_freq) ? df->max_freq : UINT_MAX;
> +	unsigned long max = UINT_MAX;

Use df->max_freq instead of UINT_MAX as following.

	unsigned long max = df->max_freq;

>  
>  	err = devfreq_update_stats(df);
>  	if (err)
> @@ -85,11 +85,6 @@ static int devfreq_simple_ondemand_func(struct devfreq *df,
>  	b = div_u64(b, (dfso_upthreshold - dfso_downdifferential / 2));
>  	*freq = (unsigned long) b;
>  
> -	if (df->min_freq && *freq < df->min_freq)
> -		*freq = df->min_freq;
> -	if (df->max_freq && *freq > df->max_freq)
> -		*freq = df->max_freq;
> -
>  	return 0;
>  }
>  
> diff --git a/drivers/devfreq/governor_userspace.c b/drivers/devfreq/governor_userspace.c
> index 080607c3f34d..378d84c011df 100644
> --- a/drivers/devfreq/governor_userspace.c
> +++ b/drivers/devfreq/governor_userspace.c
> @@ -26,19 +26,11 @@ static int devfreq_userspace_func(struct devfreq *df, unsigned long *freq)
>  {
>  	struct userspace_data *data = df->data;
>  
> -	if (data->valid) {
> -		unsigned long adjusted_freq = data->user_frequency;
> -
> -		if (df->max_freq && adjusted_freq > df->max_freq)
> -			adjusted_freq = df->max_freq;
> -
> -		if (df->min_freq && adjusted_freq < df->min_freq)
> -			adjusted_freq = df->min_freq;
> -
> -		*freq = adjusted_freq;
> -	} else {
> +	if (data->valid)
> +		*freq = data->user_frequency;
> +	else
>  		*freq = df->previous_freq; /* No user freq specified yet */
> -	}
> +
>  	return 0;
>  }
>  
> 


-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

Re: [PATCH] PM / devfreq: Remove redundant frequency adjustment from governors

[Matthias Kaehlcke]

Hi,

On Thu, May 17, 2018 at 10:44:08AM +0900, Chanwoo Choi wrote:
> Hi,
> 
> On 2018년 05월 17일 06:10, Matthias Kaehlcke wrote:
> > The performance, powersave, simpleondemand and userspace governors
> > determine a target frequency and then adjust it according to the
> > df->min/max_freq limits that might have been set by user space. This
> > adjustment is redundant, it is done in update_devfreq() for any
> > governor, right after governor->get_target_freq().
> > 
> > Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> > ---
> >  drivers/devfreq/governor_performance.c    | 10 ++--------
> >  drivers/devfreq/governor_powersave.c      |  5 -----
> >  drivers/devfreq/governor_simpleondemand.c |  7 +------
> >  drivers/devfreq/governor_userspace.c      | 16 ++++------------
> >  4 files changed, 7 insertions(+), 31 deletions(-)
> > 
> > diff --git a/drivers/devfreq/governor_performance.c b/drivers/devfreq/governor_performance.c
> > index 4d23ecfbd948..31ee30622c00 100644
> > --- a/drivers/devfreq/governor_performance.c
> > +++ b/drivers/devfreq/governor_performance.c
> > @@ -16,14 +16,8 @@
> >  static int devfreq_performance_func(struct devfreq *df,
> >  				    unsigned long *freq)
> >  {
> > -	/*
> > -	 * target callback should be able to get floor value as
> > -	 * said in devfreq.h
> > -	 */
> > -	if (!df->max_freq)
> > -		*freq = UINT_MAX;
> > -	else
> > -		*freq = df->max_freq;
> > +	*freq = UINT_MAX;
> > +
> 
> It is difficult to understand why use UINT_MAX instead of df->max_freq.
> 
> Instead, after merged the commit ab8f58ad72c4 ("PM / devfreq: Set min/max_freq
> when adding the devfreq device"), df->max/min_freq have the specific frequency
> value always. So, we can change it as following without UINT_MAX. 
> 
> 	*freq = df->max_freq;

There are two reasons why I don't like to return df->max_freq:

1. update_devfreq() already handles the user limits (which is what
min/max_freq actually are), no need to spread parts of this
additionally over all governors.

2. I plan to introduce the concept of a devfreq policy [1], which
would introduce another pair of frequencies, df->policy.min/max, and
min/max_freq would become df->policy.user.min/max. The governors would
then return df->policy.user.min/max, which isn't really incorrect
since update_devfreq() takes care of adjusting the value with
df->policy.min/max if needed, but it also isn't very clear. And we
almost certainly shouldn't additionally handle df->policy.min/max in
the governors.

I agree though that just returning UINT_MAX isn't very clear either,
even though that's what some governors are doing currently when
df->min/max_freq is not set (which can still occur, since user space
is free to set the value to 0).

I think there are two better options than returning df->min/max_freq:

a) create constants DEVFREQ_MIN/MAX_FREQ and return them, this clearly
states the intent.

b) return df->scaling_min/max_freq, which is the min/max frequency
that is actually available on the device side, depending on the
enabled OPPs.

A slightly related question: Is it actually intended to keep
supporting a value of 0 for df->min/max_freq to keep backwards
compatibility, or should the related code be removed?

Thanks

Matthias

[1] https://patchwork.kernel.org/patch/10401999/ (first draft, without
df->policy.min/max)

Re: [PATCH] PM / devfreq: Remove redundant frequency adjustment from governors

[Matthias Kaehlcke]

On Wed, May 16, 2018 at 02:10:51PM -0700, Matthias Kaehlcke wrote:
> The performance, powersave, simpleondemand and userspace governors
> determine a target frequency and then adjust it according to the
> df->min/max_freq limits that might have been set by user space. This
> adjustment is redundant, it is done in update_devfreq() for any
> governor, right after governor->get_target_freq().
> 
> Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> ---
>  drivers/devfreq/governor_performance.c    | 10 ++--------
>  drivers/devfreq/governor_powersave.c      |  5 -----
>  drivers/devfreq/governor_simpleondemand.c |  7 +------
>  drivers/devfreq/governor_userspace.c      | 16 ++++------------
>  4 files changed, 7 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/devfreq/governor_performance.c b/drivers/devfreq/governor_performance.c
> index 4d23ecfbd948..31ee30622c00 100644
> --- a/drivers/devfreq/governor_performance.c
> +++ b/drivers/devfreq/governor_performance.c
> @@ -16,14 +16,8 @@
>  static int devfreq_performance_func(struct devfreq *df,
>  				    unsigned long *freq)
>  {
> -	/*
> -	 * target callback should be able to get floor value as
> -	 * said in devfreq.h
> -	 */
> -	if (!df->max_freq)
> -		*freq = UINT_MAX;
> -	else
> -		*freq = df->max_freq;
> +	*freq = UINT_MAX;
> +
>  	return 0;
>  }

For the record, the frequency adjustment in update_devfreq() is
currently broken for df->max_freq == 0:

https://patchwork.kernel.org/patch/10407827/

Re: [PATCH] PM / devfreq: Remove redundant frequency adjustment from governors

[Chanwoo Choi]

Hi,

On 2018년 05월 18일 00:47, Matthias Kaehlcke wrote:
> Hi,
> 
> On Thu, May 17, 2018 at 10:44:08AM +0900, Chanwoo Choi wrote:
>> Hi,
>>
>> On 2018년 05월 17일 06:10, Matthias Kaehlcke wrote:
>>> The performance, powersave, simpleondemand and userspace governors
>>> determine a target frequency and then adjust it according to the
>>> df->min/max_freq limits that might have been set by user space. This
>>> adjustment is redundant, it is done in update_devfreq() for any
>>> governor, right after governor->get_target_freq().
>>>
>>> Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
>>> ---
>>>  drivers/devfreq/governor_performance.c    | 10 ++--------
>>>  drivers/devfreq/governor_powersave.c      |  5 -----
>>>  drivers/devfreq/governor_simpleondemand.c |  7 +------
>>>  drivers/devfreq/governor_userspace.c      | 16 ++++------------
>>>  4 files changed, 7 insertions(+), 31 deletions(-)
>>>
>>> diff --git a/drivers/devfreq/governor_performance.c b/drivers/devfreq/governor_performance.c
>>> index 4d23ecfbd948..31ee30622c00 100644
>>> --- a/drivers/devfreq/governor_performance.c
>>> +++ b/drivers/devfreq/governor_performance.c
>>> @@ -16,14 +16,8 @@
>>>  static int devfreq_performance_func(struct devfreq *df,
>>>  				    unsigned long *freq)
>>>  {
>>> -	/*
>>> -	 * target callback should be able to get floor value as
>>> -	 * said in devfreq.h
>>> -	 */
>>> -	if (!df->max_freq)
>>> -		*freq = UINT_MAX;
>>> -	else
>>> -		*freq = df->max_freq;
>>> +	*freq = UINT_MAX;
>>> +
>>
>> It is difficult to understand why use UINT_MAX instead of df->max_freq.
>>
>> Instead, after merged the commit ab8f58ad72c4 ("PM / devfreq: Set min/max_freq
>> when adding the devfreq device"), df->max/min_freq have the specific frequency
>> value always. So, we can change it as following without UINT_MAX. 
>>
>> 	*freq = df->max_freq;
> 
> There are two reasons why I don't like to return df->max_freq:
> 
> 1. update_devfreq() already handles the user limits (which is what
> min/max_freq actually are), no need to spread parts of this
> additionally over all governors.

As I already commented, each function have to keep their own role.
Actually, this function doesn't know the future work in update_devfreq().
Only, devfreq_performance_func have to set the maximum frequency to "*freq".
It is role of performance governor.

> 
> 2. I plan to introduce the concept of a devfreq policy [1], which
> would introduce another pair of frequencies, df->policy.min/max, and
> min/max_freq would become df->policy.user.min/max. The governors would
> then return df->policy.user.min/max, which isn't really incorrect
> since update_devfreq() takes care of adjusting the value with
> df->policy.min/max if needed, but it also isn't very clear. And we
> almost certainly shouldn't additionally handle df->policy.min/max in
> the governors.

I have not seen any patch. Also, it is not proper to discuss on this patch
because this patch doesn't include devfreq policy(?).

> 
> I agree though that just returning UINT_MAX isn't very clear either,
> even though that's what some governors are doing currently when
> df->min/max_freq is not set (which can still occur, since user space
> is free to set the value to 0).
> 
> I think there are two better options than returning df->min/max_freq:
> 
> a) create constants DEVFREQ_MIN/MAX_FREQ and return them, this clearly
> states the intent.
> 
> b) return df->scaling_min/max_freq, which is the min/max frequency
> that is actually available on the device side, depending on the
> enabled OPPs.
> 
> A slightly related question: Is it actually intended to keep
> supporting a value of 0 for df->min/max_freq to keep backwards
> compatibility, or should the related code be removed?
> 
> Thanks
> 
> Matthias
> 
> [1] https://patchwork.kernel.org/patch/10401999/ (first draft, without
> df->policy.min/max)
> 
> 
> 

And when you reply, please remain previous my comments of another point.

-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

Re: [PATCH] PM / devfreq: Remove redundant frequency adjustment from governors

[Chanwoo Choi]

On 2018년 05월 18일 07:41, Matthias Kaehlcke wrote:
> On Wed, May 16, 2018 at 02:10:51PM -0700, Matthias Kaehlcke wrote:
>> The performance, powersave, simpleondemand and userspace governors
>> determine a target frequency and then adjust it according to the
>> df->min/max_freq limits that might have been set by user space. This
>> adjustment is redundant, it is done in update_devfreq() for any
>> governor, right after governor->get_target_freq().
>>
>> Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
>> ---
>>  drivers/devfreq/governor_performance.c    | 10 ++--------
>>  drivers/devfreq/governor_powersave.c      |  5 -----
>>  drivers/devfreq/governor_simpleondemand.c |  7 +------
>>  drivers/devfreq/governor_userspace.c      | 16 ++++------------
>>  4 files changed, 7 insertions(+), 31 deletions(-)
>>
>> diff --git a/drivers/devfreq/governor_performance.c b/drivers/devfreq/governor_performance.c
>> index 4d23ecfbd948..31ee30622c00 100644
>> --- a/drivers/devfreq/governor_performance.c
>> +++ b/drivers/devfreq/governor_performance.c
>> @@ -16,14 +16,8 @@
>>  static int devfreq_performance_func(struct devfreq *df,
>>  				    unsigned long *freq)
>>  {
>> -	/*
>> -	 * target callback should be able to get floor value as
>> -	 * said in devfreq.h
>> -	 */
>> -	if (!df->max_freq)
>> -		*freq = UINT_MAX;
>> -	else
>> -		*freq = df->max_freq;
>> +	*freq = UINT_MAX;
>> +
>>  	return 0;
>>  }
> 
> For the record, the frequency adjustment in update_devfreq() is
> currently broken for df->max_freq == 0:
> 
> https://patchwork.kernel.org/patch/10407827/

Why don't you send patch set? It is very difficult to track
the history and a correlation between patches. Usually, if patches
have the dependency between patches, send the patch set with cover-letter.

-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

Re: [PATCH] PM / devfreq: Remove redundant frequency adjustment from governors

[Matthias Kaehlcke]

On Fri, May 18, 2018 at 08:15:55AM +0900, Chanwoo Choi wrote:
> Hi,
> 
> On 2018년 05월 18일 00:47, Matthias Kaehlcke wrote:
> > Hi,
> > 
> > On Thu, May 17, 2018 at 10:44:08AM +0900, Chanwoo Choi wrote:
> >> Hi,
> >>
> >> On 2018년 05월 17일 06:10, Matthias Kaehlcke wrote:
> >>> The performance, powersave, simpleondemand and userspace governors
> >>> determine a target frequency and then adjust it according to the
> >>> df->min/max_freq limits that might have been set by user space. This
> >>> adjustment is redundant, it is done in update_devfreq() for any
> >>> governor, right after governor->get_target_freq().
> >>>
> >>> Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> >>> ---
> >>>  drivers/devfreq/governor_performance.c    | 10 ++--------
> >>>  drivers/devfreq/governor_powersave.c      |  5 -----
> >>>  drivers/devfreq/governor_simpleondemand.c |  7 +------
> >>>  drivers/devfreq/governor_userspace.c      | 16 ++++------------
> >>>  4 files changed, 7 insertions(+), 31 deletions(-)
> >>>
> >>> diff --git a/drivers/devfreq/governor_performance.c b/drivers/devfreq/governor_performance.c
> >>> index 4d23ecfbd948..31ee30622c00 100644
> >>> --- a/drivers/devfreq/governor_performance.c
> >>> +++ b/drivers/devfreq/governor_performance.c
> >>> @@ -16,14 +16,8 @@
> >>>  static int devfreq_performance_func(struct devfreq *df,
> >>>  				    unsigned long *freq)
> >>>  {
> >>> -	/*
> >>> -	 * target callback should be able to get floor value as
> >>> -	 * said in devfreq.h
> >>> -	 */
> >>> -	if (!df->max_freq)
> >>> -		*freq = UINT_MAX;
> >>> -	else
> >>> -		*freq = df->max_freq;
> >>> +	*freq = UINT_MAX;
> >>> +
> >>
> >> It is difficult to understand why use UINT_MAX instead of df->max_freq.
> >>
> >> Instead, after merged the commit ab8f58ad72c4 ("PM / devfreq: Set min/max_freq
> >> when adding the devfreq device"), df->max/min_freq have the specific frequency
> >> value always. So, we can change it as following without UINT_MAX. 
> >>
> >> 	*freq = df->max_freq;
> > 
> > There are two reasons why I don't like to return df->max_freq:
> > 
> > 1. update_devfreq() already handles the user limits (which is what
> > min/max_freq actually are), no need to spread parts of this
> > additionally over all governors.
> 
> As I already commented, each function have to keep their own role.

Actually I agree on that :)

> Actually, this function doesn't know the future work in update_devfreq().
> Only, devfreq_performance_func have to set the maximum frequency to "*freq".
> It is role of performance governor.

My first point doesn't refer to the future work, but is about not
spreading the handling of the user defined limits without need into
the governors. Below I offer two alternatives (and a question) to
which you didn't reply.

> > 
> > 2. I plan to introduce the concept of a devfreq policy [1], which
> > would introduce another pair of frequencies, df->policy.min/max, and
> > min/max_freq would become df->policy.user.min/max. The governors would
> > then return df->policy.user.min/max, which isn't really incorrect
> > since update_devfreq() takes care of adjusting the value with
> > df->policy.min/max if needed, but it also isn't very clear. And we
> > almost certainly shouldn't additionally handle df->policy.min/max in
> > the governors.
> 
> I have not seen any patch. Also, it is not proper to discuss on this patch
> because this patch doesn't include devfreq policy(?).
> 
> > 
> > I agree though that just returning UINT_MAX isn't very clear either,
> > even though that's what some governors are doing currently when
> > df->min/max_freq is not set (which can still occur, since user space
> > is free to set the value to 0).
> > 
> > I think there are two better options than returning df->min/max_freq:
> > 
> > a) create constants DEVFREQ_MIN/MAX_FREQ and return them, this clearly
> > states the intent.
> > 
> > b) return df->scaling_min/max_freq, which is the min/max frequency
> > that is actually available on the device side, depending on the
> > enabled OPPs.
> > 
> > A slightly related question: Is it actually intended to keep
> > supporting a value of 0 for df->min/max_freq to keep backwards
> > compatibility, or should the related code be removed?
> > 
> > Thanks
> > 
> > Matthias
> > 
> > [1] https://patchwork.kernel.org/patch/10401999/ (first draft, without
> > df->policy.min/max)
> > 
> > 
> > 
> 
> And when you reply, please remain previous my comments of another point.

Sorry, it wasn't my intention to 'ignore' your other comments. Since
they made similar points as the one to which I responded I didn't
consider it necessary to quote them below my reply, without me adding
anything that I hadn't already said above.

Best regards

Matthias