Skip to contentSkip to conversation

PM / OPP: Use list_for_each_entry_reverse instead of list_for_each_entry

PATCH In review Jonghwan Choi 2 participants 4 messages Apr 29, 2014

Subsystems (4)
Driver core Suspend to RAM Hibernation Power management core
lore
Share & tools
lore

Jonghwan Choi

author
#0 lore raw

PATCH 1/2 PM / OPP: Use list_for_each_entry_reverse instead of list_for_each_entry

Most of the cpufreq table is sorted in descending order. But when cpufreq table is made from dev_pm_opp_init_cpufreq_table, it is sorted in ascending order. So in some case to make cpufreq table with descending order it needs an extra work.

Signed-off-by Jonghwan Choi <[email protected]>
---
 drivers/base/power/opp.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/base/power/opp.c +1 -1
index 2553867..8d81a46 100644
@@ -649,7 +649,7 @@ int dev_pm_opp_init_cpufreq_table(struct device *dev,
 		return -ENOMEM;
 	}
 
-	list_for_each_entry(opp, &dev_opp->opp_list, node) {
+	list_for_each_entry_reverse(opp, &dev_opp->opp_list, node) {
 		if (opp->available) {
 			freq_table[i].driver_data = i;
 			freq_table[i].frequency = opp->rate / 1000;
Signature · 1 line
1.7.10.4
#1 lore raw
Quoted 1 line · Jonghwan Choi

Most of the cpufreq table is sorted in descending order.

Which part of kernel says that?

Quoted 3 lines

But when cpufreq table is made from dev_pm_opp_init_cpufreq_table, it is sorted in ascending order. So in some case to make cpufreq table with descending order it needs an extra work.

Order of this table shouldn't matter at all in cpufreq layer..

author
#2 lore raw

Hi~

Quoted 4 lines
Quoted 1 line

Most of the cpufreq table is sorted in descending order.

Which part of kernel says that?

-> I couldn't check all. Sorry.

Quoted 5 lines
Quoted 3 lines

But when cpufreq table is made from dev_pm_opp_init_cpufreq_table, it is sorted in ascending order. So in some case to make cpufreq table with descending order it needs an extra work.

Order of this table shouldn't matter at all in cpufreq layer..

-> You are right. But 5440 cpufreq driver write an index number instead of clk divider value
for change DVFS. And our another(will submit) also write an index number for changing DVFS.
As you said, order of table shouldn't matter at all in cpufreq layer. Then, could this can be applied?
In our case, We want to use index 0 for P0 and index 1 for P1.....

Thanks

Best Ragrds.

Quoted 20 lines

-----Original Message-----
From: [email protected] [mailto:[email protected]] On Behalf Of Viresh Kumar
Sent: Tuesday, April 29, 2014 2:33 PM
To: Jonghwan Choi
Cc: open list; Rafael J. Wysocki; Len Brown
Subject: Re: [PATCH 1/2] PM / OPP: Use list_for_each_entry_reverse instead of list_for_each_entry

Quoted 1 line · Jonghwan Choi

Most of the cpufreq table is sorted in descending order.

Which part of kernel says that?

Quoted 3 lines

But when cpufreq table is made from dev_pm_opp_init_cpufreq_table, it is sorted in ascending order. So in some case to make cpufreq table with descending order it needs an extra work.

Order of this table shouldn't matter at all in cpufreq layer..

#3 lore raw
Quoted 6 lines · Jonghwan Choi

-> You are right. But 5440 cpufreq driver write an index number instead of clk divider value
for change DVFS. And our another(will submit) also write an index number for changing DVFS.
As you said, order of table shouldn't matter at all in cpufreq layer. Then, could this can be applied?

These two patches aren't going to fly I believe. Depending on the order of table for setting hardware is inviting trouble.

Quoted 1 line

In our case, We want to use index 0 for P0 and index 1 for P1.....

What I would recommend is, use .driver_data field to hold what has to be written to hardware for any frequency. And then simply use driver_data instead of index.