All of lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH v2] PM / devfreq: Use freq_table for available_frequencies
@ 2014-04-28  1:41 ` MyungJoo Ham
  0 siblings, 0 replies; 10+ messages in thread
From: MyungJoo Ham @ 2014-04-28  1:41 UTC (permalink / raw)
  To: Saravana Kannan, 박경민
  Cc: linux-pm, linux-kernel, linux-arm-msm, linux-arm-kernel

> Some devices use freq_table instead of OPP. For those devices, the
> available_frequencies file shows up empty. Fix that by using freq_table to
> generate the available_frequencies data when it's available.
> 
> OPP find frequency APIs also skips frequencies that have been temporarily
> disabled (say, due to thermal, etc). Since available_frequencies is
> supposed to show the entire list of available frequencies without taking
> temporary limits into consideration, preference is given to freq_table when
> available.
> 
> Signed-off-by: Saravana Kannan <skannan@codeaurora.org>
> ---
>  drivers/devfreq/devfreq.c | 29 ++++++++++++++++++-----------
>  1 file changed, 18 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
> index 2042ec3..527cbe2 100644
> --- a/drivers/devfreq/devfreq.c
> +++ b/drivers/devfreq/devfreq.c
> @@ -912,20 +912,27 @@ static ssize_t available_frequencies_show(struct device *d,
>  	struct devfreq *df = to_devfreq(d);
>  	struct device *dev = df->dev.parent;
>  	struct dev_pm_opp *opp;
> +	unsigned int i = 0;
>  	ssize_t count = 0;
>  	unsigned long freq = 0;
>  
> -	rcu_read_lock();
> -	do {
> -		opp = dev_pm_opp_find_freq_ceil(dev, &freq);
> -		if (IS_ERR(opp))
> -			break;
> -
> -		count += scnprintf(&buf[count], (PAGE_SIZE - count - 2),
> -				   "%lu ", freq);
> -		freq++;
> -	} while (1);
> -	rcu_read_unlock();
> +	if (df->profile->freq_table) {
> +		for (i = 0; i < df->profile->max_state; i++)
> +			count += scnprintf(&buf[count], (PAGE_SIZE - count - 2),
> +					   "%u ", df->profile->freq_table[i]);

You are hereby changing the semmantics of the original
available_frequencies node.

When a frequency/voltage pair has been disabled (opp_disable), probably
by opp_disable(), the frequency is no more "available".
However, when the driver author supplied freq_table as well as OPP
in order to see the statistics, the node will behave differently.

Please do not affect the current users as long as it does not give
additional benefit or fix a bug.


Cheers,
MyungJoo.


> +	} else {
> +		rcu_read_lock();
> +		do {
> +			opp = dev_pm_opp_find_freq_ceil(dev, &freq);
> +			if (IS_ERR(opp))
> +				break;
> +
> +			count += scnprintf(&buf[count], (PAGE_SIZE - count - 2),
> +					   "%lu ", freq);
> +			freq++;
> +		} while (1);
> +		rcu_read_unlock();
> +	}
>  
>  	/* Truncate the trailing space */
>  	if (count)
> -- 
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> hosted by The Linux Foundation
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] PM / devfreq: Use freq_table for available_frequencies
@ 2014-04-28  1:41 ` MyungJoo Ham
  0 siblings, 0 replies; 10+ messages in thread
From: MyungJoo Ham @ 2014-04-28  1:41 UTC (permalink / raw)
  To: Saravana Kannan, 박경민
  Cc: linux-pm, linux-kernel, linux-arm-msm, linux-arm-kernel

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=euc-kr, Size: 2699 bytes --]

> Some devices use freq_table instead of OPP. For those devices, the
> available_frequencies file shows up empty. Fix that by using freq_table to
> generate the available_frequencies data when it's available.
> 
> OPP find frequency APIs also skips frequencies that have been temporarily
> disabled (say, due to thermal, etc). Since available_frequencies is
> supposed to show the entire list of available frequencies without taking
> temporary limits into consideration, preference is given to freq_table when
> available.
> 
> Signed-off-by: Saravana Kannan <skannan@codeaurora.org>
> ---
>  drivers/devfreq/devfreq.c | 29 ++++++++++++++++++-----------
>  1 file changed, 18 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
> index 2042ec3..527cbe2 100644
> --- a/drivers/devfreq/devfreq.c
> +++ b/drivers/devfreq/devfreq.c
> @@ -912,20 +912,27 @@ static ssize_t available_frequencies_show(struct device *d,
>  	struct devfreq *df = to_devfreq(d);
>  	struct device *dev = df->dev.parent;
>  	struct dev_pm_opp *opp;
> +	unsigned int i = 0;
>  	ssize_t count = 0;
>  	unsigned long freq = 0;
>  
> -	rcu_read_lock();
> -	do {
> -		opp = dev_pm_opp_find_freq_ceil(dev, &freq);
> -		if (IS_ERR(opp))
> -			break;
> -
> -		count += scnprintf(&buf[count], (PAGE_SIZE - count - 2),
> -				   "%lu ", freq);
> -		freq++;
> -	} while (1);
> -	rcu_read_unlock();
> +	if (df->profile->freq_table) {
> +		for (i = 0; i < df->profile->max_state; i++)
> +			count += scnprintf(&buf[count], (PAGE_SIZE - count - 2),
> +					   "%u ", df->profile->freq_table[i]);

You are hereby changing the semmantics of the original
available_frequencies node.

When a frequency/voltage pair has been disabled (opp_disable), probably
by opp_disable(), the frequency is no more "available".
However, when the driver author supplied freq_table as well as OPP
in order to see the statistics, the node will behave differently.

Please do not affect the current users as long as it does not give
additional benefit or fix a bug.


Cheers,
MyungJoo.


> +	} else {
> +		rcu_read_lock();
> +		do {
> +			opp = dev_pm_opp_find_freq_ceil(dev, &freq);
> +			if (IS_ERR(opp))
> +				break;
> +
> +			count += scnprintf(&buf[count], (PAGE_SIZE - count - 2),
> +					   "%lu ", freq);
> +			freq++;
> +		} while (1);
> +		rcu_read_unlock();
> +	}
>  
>  	/* Truncate the trailing space */
>  	if (count)
> -- 
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> hosted by The Linux Foundation
> 
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] PM / devfreq: Use freq_table for available_frequencies
  2014-04-28  1:41 ` MyungJoo Ham
@ 2014-04-29 20:00   ` Saravana Kannan
  -1 siblings, 0 replies; 10+ messages in thread
From: Saravana Kannan @ 2014-04-29 20:00 UTC (permalink / raw)
  To: myungjoo.ham
  Cc: 박경민,
	linux-pm, linux-kernel, linux-arm-msm, linux-arm-kernel

On 04/27/2014 06:41 PM, MyungJoo Ham wrote:
>> Some devices use freq_table instead of OPP. For those devices, the
>> available_frequencies file shows up empty. Fix that by using freq_table to
>> generate the available_frequencies data when it's available.
>>
>> OPP find frequency APIs also skips frequencies that have been temporarily
>> disabled (say, due to thermal, etc). Since available_frequencies is
>> supposed to show the entire list of available frequencies without taking
>> temporary limits into consideration, preference is given to freq_table when
>> available.
>>
>> Signed-off-by: Saravana Kannan <skannan@codeaurora.org>
>> ---
>>   drivers/devfreq/devfreq.c | 29 ++++++++++++++++++-----------
>>   1 file changed, 18 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
>> index 2042ec3..527cbe2 100644
>> --- a/drivers/devfreq/devfreq.c
>> +++ b/drivers/devfreq/devfreq.c
>> @@ -912,20 +912,27 @@ static ssize_t available_frequencies_show(struct device *d,
>>   	struct devfreq *df = to_devfreq(d);
>>   	struct device *dev = df->dev.parent;
>>   	struct dev_pm_opp *opp;
>> +	unsigned int i = 0;
>>   	ssize_t count = 0;
>>   	unsigned long freq = 0;
>>   
>> -	rcu_read_lock();
>> -	do {
>> -		opp = dev_pm_opp_find_freq_ceil(dev, &freq);
>> -		if (IS_ERR(opp))
>> -			break;
>> -
>> -		count += scnprintf(&buf[count], (PAGE_SIZE - count - 2),
>> -				   "%lu ", freq);
>> -		freq++;
>> -	} while (1);
>> -	rcu_read_unlock();
>> +	if (df->profile->freq_table) {
>> +		for (i = 0; i < df->profile->max_state; i++)
>> +			count += scnprintf(&buf[count], (PAGE_SIZE - count - 2),
>> +					   "%u ", df->profile->freq_table[i]);
> 
> You are hereby changing the semmantics of the original
> available_frequencies node.
> 
> When a frequency/voltage pair has been disabled (opp_disable), probably
> by opp_disable(), the frequency is no more "available".
> However, when the driver author supplied freq_table as well as OPP
> in order to see the statistics, the node will behave differently.
> 
> Please do not affect the current users as long as it does not give
> additional benefit or fix a bug.

I was actually trying to stick with the semantics as it was documented.
The documentation for this file says it'll show frequencies that are not
allowed by the current min/max settings either. To me, an OPP disable
seems similar to some frequencies "disabled" by min/max settings.

Giving preference to OPP is not a hard change to do, but it seems to go
againsts the documented semantics.

Thoughts?

-Saravana


-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v2] PM / devfreq: Use freq_table for available_frequencies
@ 2014-04-29 20:00   ` Saravana Kannan
  0 siblings, 0 replies; 10+ messages in thread
From: Saravana Kannan @ 2014-04-29 20:00 UTC (permalink / raw)
  To: linux-arm-kernel

On 04/27/2014 06:41 PM, MyungJoo Ham wrote:
>> Some devices use freq_table instead of OPP. For those devices, the
>> available_frequencies file shows up empty. Fix that by using freq_table to
>> generate the available_frequencies data when it's available.
>>
>> OPP find frequency APIs also skips frequencies that have been temporarily
>> disabled (say, due to thermal, etc). Since available_frequencies is
>> supposed to show the entire list of available frequencies without taking
>> temporary limits into consideration, preference is given to freq_table when
>> available.
>>
>> Signed-off-by: Saravana Kannan <skannan@codeaurora.org>
>> ---
>>   drivers/devfreq/devfreq.c | 29 ++++++++++++++++++-----------
>>   1 file changed, 18 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
>> index 2042ec3..527cbe2 100644
>> --- a/drivers/devfreq/devfreq.c
>> +++ b/drivers/devfreq/devfreq.c
>> @@ -912,20 +912,27 @@ static ssize_t available_frequencies_show(struct device *d,
>>   	struct devfreq *df = to_devfreq(d);
>>   	struct device *dev = df->dev.parent;
>>   	struct dev_pm_opp *opp;
>> +	unsigned int i = 0;
>>   	ssize_t count = 0;
>>   	unsigned long freq = 0;
>>   
>> -	rcu_read_lock();
>> -	do {
>> -		opp = dev_pm_opp_find_freq_ceil(dev, &freq);
>> -		if (IS_ERR(opp))
>> -			break;
>> -
>> -		count += scnprintf(&buf[count], (PAGE_SIZE - count - 2),
>> -				   "%lu ", freq);
>> -		freq++;
>> -	} while (1);
>> -	rcu_read_unlock();
>> +	if (df->profile->freq_table) {
>> +		for (i = 0; i < df->profile->max_state; i++)
>> +			count += scnprintf(&buf[count], (PAGE_SIZE - count - 2),
>> +					   "%u ", df->profile->freq_table[i]);
> 
> You are hereby changing the semmantics of the original
> available_frequencies node.
> 
> When a frequency/voltage pair has been disabled (opp_disable), probably
> by opp_disable(), the frequency is no more "available".
> However, when the driver author supplied freq_table as well as OPP
> in order to see the statistics, the node will behave differently.
> 
> Please do not affect the current users as long as it does not give
> additional benefit or fix a bug.

I was actually trying to stick with the semantics as it was documented.
The documentation for this file says it'll show frequencies that are not
allowed by the current min/max settings either. To me, an OPP disable
seems similar to some frequencies "disabled" by min/max settings.

Giving preference to OPP is not a hard change to do, but it seems to go
againsts the documented semantics.

Thoughts?

-Saravana


-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] PM / devfreq: Use freq_table for available_frequencies
  2014-04-29 20:00   ` Saravana Kannan
@ 2014-05-05 18:18     ` Saravana Kannan
  -1 siblings, 0 replies; 10+ messages in thread
From: Saravana Kannan @ 2014-05-05 18:18 UTC (permalink / raw)
  To: myungjoo.ham
  Cc: 박경민,
	linux-pm, linux-kernel, linux-arm-msm, linux-arm-kernel

On 04/29/2014 01:00 PM, Saravana Kannan wrote:
> On 04/27/2014 06:41 PM, MyungJoo Ham wrote:
>> You are hereby changing the semmantics of the original
>> available_frequencies node.
>>
>> When a frequency/voltage pair has been disabled (opp_disable), probably
>> by opp_disable(), the frequency is no more "available".
>> However, when the driver author supplied freq_table as well as OPP
>> in order to see the statistics, the node will behave differently.
>>
>> Please do not affect the current users as long as it does not give
>> additional benefit or fix a bug.
> 
> I was actually trying to stick with the semantics as it was documented.
> The documentation for this file says it'll show frequencies that are not
> allowed by the current min/max settings either. To me, an OPP disable
> seems similar to some frequencies "disabled" by min/max settings.
> 
> Giving preference to OPP is not a hard change to do, but it seems to go
> againsts the documented semantics.
> 
> Thoughts?

I'll send out another patch like you wanted -- with OPP being given
preference over freq_table when listing frequencies.

But I would still like to hear your thoughts. As of today, there's no
clean way to get the complete list of available frequencies that would
give a consistent output irrespective of the temporary limits/conditions
imposed by thermal, current limiting, etc. The round about way is to cat
trans_stat and parse the frequencies from that.

That's why I was trying to give preference to freq_table.

Thanks,
Saravana

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v2] PM / devfreq: Use freq_table for available_frequencies
@ 2014-05-05 18:18     ` Saravana Kannan
  0 siblings, 0 replies; 10+ messages in thread
From: Saravana Kannan @ 2014-05-05 18:18 UTC (permalink / raw)
  To: linux-arm-kernel

On 04/29/2014 01:00 PM, Saravana Kannan wrote:
> On 04/27/2014 06:41 PM, MyungJoo Ham wrote:
>> You are hereby changing the semmantics of the original
>> available_frequencies node.
>>
>> When a frequency/voltage pair has been disabled (opp_disable), probably
>> by opp_disable(), the frequency is no more "available".
>> However, when the driver author supplied freq_table as well as OPP
>> in order to see the statistics, the node will behave differently.
>>
>> Please do not affect the current users as long as it does not give
>> additional benefit or fix a bug.
> 
> I was actually trying to stick with the semantics as it was documented.
> The documentation for this file says it'll show frequencies that are not
> allowed by the current min/max settings either. To me, an OPP disable
> seems similar to some frequencies "disabled" by min/max settings.
> 
> Giving preference to OPP is not a hard change to do, but it seems to go
> againsts the documented semantics.
> 
> Thoughts?

I'll send out another patch like you wanted -- with OPP being given
preference over freq_table when listing frequencies.

But I would still like to hear your thoughts. As of today, there's no
clean way to get the complete list of available frequencies that would
give a consistent output irrespective of the temporary limits/conditions
imposed by thermal, current limiting, etc. The round about way is to cat
trans_stat and parse the frequencies from that.

That's why I was trying to give preference to freq_table.

Thanks,
Saravana

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] PM / devfreq: Use freq_table for available_frequencies
  2014-05-08 10:02 MyungJoo Ham
@ 2014-05-19 23:01   ` Saravana Kannan
  0 siblings, 0 replies; 10+ messages in thread
From: Saravana Kannan @ 2014-05-19 23:01 UTC (permalink / raw)
  To: myungjoo.ham
  Cc: 박경민,
	linux-pm, linux-kernel, linux-arm-msm, linux-arm-kernel

On 05/08/2014 03:02 AM, MyungJoo Ham wrote:
>> On 04/29/2014 01:00 PM, Saravana Kannan wrote:
>>> On 04/27/2014 06:41 PM, MyungJoo Ham wrote:
>>>> You are hereby changing the semmantics of the original
>>>> available_frequencies node.
>>>>
>>>> When a frequency/voltage pair has been disabled (opp_disable), probably
>>>> by opp_disable(), the frequency is no more "available".
>>>> However, when the driver author supplied freq_table as well as OPP
>>>> in order to see the statistics, the node will behave differently.
>>>>
>>>> Please do not affect the current users as long as it does not give
>>>> additional benefit or fix a bug.
>>>
>>> I was actually trying to stick with the semantics as it was documented.
>>> The documentation for this file says it'll show frequencies that are not
>>> allowed by the current min/max settings either. To me, an OPP disable
>>> seems similar to some frequencies "disabled" by min/max settings.
>>>
>>> Giving preference to OPP is not a hard change to do, but it seems to go
>>> againsts the documented semantics.
>>>
>>> Thoughts?
>>
>> I'll send out another patch like you wanted -- with OPP being given
>> preference over freq_table when listing frequencies.
>>
>> But I would still like to hear your thoughts. As of today, there's no
>> clean way to get the complete list of available frequencies that would
>> give a consistent output irrespective of the temporary limits/conditions
>> imposed by thermal, current limiting, etc. The round about way is to cat
>> trans_stat and parse the frequencies from that.
>>
>> That's why I was trying to give preference to freq_table.
>>
>> Thanks,
>> Saravana
> 
> The node, available_frequencies, was suggested before freq_table concept.
> At that time, available_frequencies was supposed to show the list of
> available OPP lists for those who use OPP for devfreq device, excluding
> those disabled by OPP. (OPP lists are external to devfreq and devfreq's
> min/max are internal to devfreq)
> 
> Locally, this node has been used to debug the behavior of a devfreq device.
> With min/max nodes, we know the range while we cannot (easily at shell)
> see which OPP points are available at the moment, where we have been able
> to use available_frequencies.
> 
> We do not want to lose such capavility as long as we do not have OPP sysfs
> automatically assigned to any OPP lists. If I remember correctly, we don't
> have it, yet.
> 
> 
> A. I want to minimize semantics changes in sysfs. Adding another without
> interfering with previous usage is ok.
> B. (more importantly) I don't want to lose the debugging capabilities.
> 

Thanks for the response MyungJoo. Makes sense. I was also discussing
this internally and was considering a "possible_frequencies" similar to
available vs possible CPUs.

I'll make such a patch for that and send it out. In that case, I'll
probably leave "available_frequencies" alone.

-Saravana

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v2] PM / devfreq: Use freq_table for available_frequencies
@ 2014-05-19 23:01   ` Saravana Kannan
  0 siblings, 0 replies; 10+ messages in thread
From: Saravana Kannan @ 2014-05-19 23:01 UTC (permalink / raw)
  To: linux-arm-kernel

On 05/08/2014 03:02 AM, MyungJoo Ham wrote:
>> On 04/29/2014 01:00 PM, Saravana Kannan wrote:
>>> On 04/27/2014 06:41 PM, MyungJoo Ham wrote:
>>>> You are hereby changing the semmantics of the original
>>>> available_frequencies node.
>>>>
>>>> When a frequency/voltage pair has been disabled (opp_disable), probably
>>>> by opp_disable(), the frequency is no more "available".
>>>> However, when the driver author supplied freq_table as well as OPP
>>>> in order to see the statistics, the node will behave differently.
>>>>
>>>> Please do not affect the current users as long as it does not give
>>>> additional benefit or fix a bug.
>>>
>>> I was actually trying to stick with the semantics as it was documented.
>>> The documentation for this file says it'll show frequencies that are not
>>> allowed by the current min/max settings either. To me, an OPP disable
>>> seems similar to some frequencies "disabled" by min/max settings.
>>>
>>> Giving preference to OPP is not a hard change to do, but it seems to go
>>> againsts the documented semantics.
>>>
>>> Thoughts?
>>
>> I'll send out another patch like you wanted -- with OPP being given
>> preference over freq_table when listing frequencies.
>>
>> But I would still like to hear your thoughts. As of today, there's no
>> clean way to get the complete list of available frequencies that would
>> give a consistent output irrespective of the temporary limits/conditions
>> imposed by thermal, current limiting, etc. The round about way is to cat
>> trans_stat and parse the frequencies from that.
>>
>> That's why I was trying to give preference to freq_table.
>>
>> Thanks,
>> Saravana
> 
> The node, available_frequencies, was suggested before freq_table concept.
> At that time, available_frequencies was supposed to show the list of
> available OPP lists for those who use OPP for devfreq device, excluding
> those disabled by OPP. (OPP lists are external to devfreq and devfreq's
> min/max are internal to devfreq)
> 
> Locally, this node has been used to debug the behavior of a devfreq device.
> With min/max nodes, we know the range while we cannot (easily at shell)
> see which OPP points are available at the moment, where we have been able
> to use available_frequencies.
> 
> We do not want to lose such capavility as long as we do not have OPP sysfs
> automatically assigned to any OPP lists. If I remember correctly, we don't
> have it, yet.
> 
> 
> A. I want to minimize semantics changes in sysfs. Adding another without
> interfering with previous usage is ok.
> B. (more importantly) I don't want to lose the debugging capabilities.
> 

Thanks for the response MyungJoo. Makes sense. I was also discussing
this internally and was considering a "possible_frequencies" similar to
available vs possible CPUs.

I'll make such a patch for that and send it out. In that case, I'll
probably leave "available_frequencies" alone.

-Saravana

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v2] PM / devfreq: Use freq_table for available_frequencies
  2014-04-15  6:30 Re: [PATCH] " 함명주
@ 2014-04-15 19:29   ` Saravana Kannan
  0 siblings, 0 replies; 10+ messages in thread
From: Saravana Kannan @ 2014-04-15 19:29 UTC (permalink / raw)
  To: MyungJoo Ham, Kyungmin Park
  Cc: linux-pm, linux-kernel, linux-arm-msm, linux-arm-kernel, Saravana Kannan

Some devices use freq_table instead of OPP. For those devices, the
available_frequencies file shows up empty. Fix that by using freq_table to
generate the available_frequencies data when it's available.

OPP find frequency APIs also skips frequencies that have been temporarily
disabled (say, due to thermal, etc). Since available_frequencies is
supposed to show the entire list of available frequencies without taking
temporary limits into consideration, preference is given to freq_table when
available.

Signed-off-by: Saravana Kannan <skannan@codeaurora.org>
---
 drivers/devfreq/devfreq.c | 29 ++++++++++++++++++-----------
 1 file changed, 18 insertions(+), 11 deletions(-)

diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index 2042ec3..527cbe2 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -912,20 +912,27 @@ static ssize_t available_frequencies_show(struct device *d,
 	struct devfreq *df = to_devfreq(d);
 	struct device *dev = df->dev.parent;
 	struct dev_pm_opp *opp;
+	unsigned int i = 0;
 	ssize_t count = 0;
 	unsigned long freq = 0;
 
-	rcu_read_lock();
-	do {
-		opp = dev_pm_opp_find_freq_ceil(dev, &freq);
-		if (IS_ERR(opp))
-			break;
-
-		count += scnprintf(&buf[count], (PAGE_SIZE - count - 2),
-				   "%lu ", freq);
-		freq++;
-	} while (1);
-	rcu_read_unlock();
+	if (df->profile->freq_table) {
+		for (i = 0; i < df->profile->max_state; i++)
+			count += scnprintf(&buf[count], (PAGE_SIZE - count - 2),
+					   "%u ", df->profile->freq_table[i]);
+	} else {
+		rcu_read_lock();
+		do {
+			opp = dev_pm_opp_find_freq_ceil(dev, &freq);
+			if (IS_ERR(opp))
+				break;
+
+			count += scnprintf(&buf[count], (PAGE_SIZE - count - 2),
+					   "%lu ", freq);
+			freq++;
+		} while (1);
+		rcu_read_unlock();
+	}
 
 	/* Truncate the trailing space */
 	if (count)
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2] PM / devfreq: Use freq_table for available_frequencies
@ 2014-04-15 19:29   ` Saravana Kannan
  0 siblings, 0 replies; 10+ messages in thread
From: Saravana Kannan @ 2014-04-15 19:29 UTC (permalink / raw)
  To: linux-arm-kernel

Some devices use freq_table instead of OPP. For those devices, the
available_frequencies file shows up empty. Fix that by using freq_table to
generate the available_frequencies data when it's available.

OPP find frequency APIs also skips frequencies that have been temporarily
disabled (say, due to thermal, etc). Since available_frequencies is
supposed to show the entire list of available frequencies without taking
temporary limits into consideration, preference is given to freq_table when
available.

Signed-off-by: Saravana Kannan <skannan@codeaurora.org>
---
 drivers/devfreq/devfreq.c | 29 ++++++++++++++++++-----------
 1 file changed, 18 insertions(+), 11 deletions(-)

diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index 2042ec3..527cbe2 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -912,20 +912,27 @@ static ssize_t available_frequencies_show(struct device *d,
 	struct devfreq *df = to_devfreq(d);
 	struct device *dev = df->dev.parent;
 	struct dev_pm_opp *opp;
+	unsigned int i = 0;
 	ssize_t count = 0;
 	unsigned long freq = 0;
 
-	rcu_read_lock();
-	do {
-		opp = dev_pm_opp_find_freq_ceil(dev, &freq);
-		if (IS_ERR(opp))
-			break;
-
-		count += scnprintf(&buf[count], (PAGE_SIZE - count - 2),
-				   "%lu ", freq);
-		freq++;
-	} while (1);
-	rcu_read_unlock();
+	if (df->profile->freq_table) {
+		for (i = 0; i < df->profile->max_state; i++)
+			count += scnprintf(&buf[count], (PAGE_SIZE - count - 2),
+					   "%u ", df->profile->freq_table[i]);
+	} else {
+		rcu_read_lock();
+		do {
+			opp = dev_pm_opp_find_freq_ceil(dev, &freq);
+			if (IS_ERR(opp))
+				break;
+
+			count += scnprintf(&buf[count], (PAGE_SIZE - count - 2),
+					   "%lu ", freq);
+			freq++;
+		} while (1);
+		rcu_read_unlock();
+	}
 
 	/* Truncate the trailing space */
 	if (count)
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

^ permalink raw reply related	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2014-05-19 23:01 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-04-28  1:41 [PATCH v2] PM / devfreq: Use freq_table for available_frequencies MyungJoo Ham
2014-04-28  1:41 ` MyungJoo Ham
2014-04-29 20:00 ` Saravana Kannan
2014-04-29 20:00   ` Saravana Kannan
2014-05-05 18:18   ` Saravana Kannan
2014-05-05 18:18     ` Saravana Kannan
  -- strict thread matches above, loose matches on Subject: below --
2014-05-08 10:02 MyungJoo Ham
2014-05-19 23:01 ` Saravana Kannan
2014-05-19 23:01   ` Saravana Kannan
2014-04-15  6:30 Re: [PATCH] " 함명주
2014-04-15 19:29 ` [PATCH v2] " Saravana Kannan
2014-04-15 19:29   ` Saravana Kannan

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.