On Tue, Mar 18, 2014 at 09:49:12PM +0000, Sebastian Capella wrote: > On 18 March 2014 11:08, Lorenzo Pieralisi <lorenzo.pieralisi@xxxxxxx> wrote: > >> > + * When we reach the max number of CPU idle states or > >> > + * head_idx == curr_idx (empty nodes queue) we are done. > >> > + */ > >> > + head_idx = curr_idx = cnt; > >> > + > >> > + do { > >> > + curr_idx = parse_idle_states_node(curr, curr_idx, cpus); > >> > + if (curr_idx == CPUIDLE_STATE_MAX || head_idx == curr_idx) > >> > + break; > >> > + /* > >> > + * idle_states array is updated by parse_idle_states_node(), > >> > + * we can use the initialized states as a queue of nodes > >> > + * that need to be checked for their idle states siblings. > >> > + * head_idx works as a pointer into the queue to get the > >> > + * next node to be parsed. > >> > + */ > >> > + curr = idle_states[head_idx++].node; > >> > + } while (curr); > >> > >> I still object to index property and this is why. You need to be able > >> to determine state order by actual h/w properties. That is what you > >> are doing in your head when you define the indexes. > >> > >> You really want a linked list here that you can sort as you go and not > >> care what order you parse DT nodes. Not to mention you don't know how > >> many states you will have. > > > > This code does not care about the order of nodes, the index is just there > > to keep track of nodes that have still to be parsed. Sorting is done later, > > using the index property (totally unrelated to the {head/curr}_idx) which I > > understand is frowned upon in DT world (but in this case I think it could be > > accepted, certainly it would make my life easier). > > > > Having said that, I like the idea of implementing it with a linked list and > > sorting states while parsing them. I will remove that index property and > > replace it with an actual hw property: power_consumption ? Or should I just > > use min_residency (the higher the required residency the deeper the idle > > state) ? Defining the power consumption (or better savings) for a state is > > an _outright_ can of worms, that's why using an index is easier. > > > > Thoughts ? > > How about something like rank, power_rank or power_score since it's > neither an index nor a physical value but a value for sorting states > relative to each other? Well, if it is just a renaming issue we can use index just as well. I hope that by now it is understood that the index property is not the state index, it is a value representing relative power consumption. Rob is asking a property describing hw, and honestly unless we define a power consumption property value (defining what it means will be rather complicated though) I'd rather remove index altogether and use min_residency as a comparison value instead. Lorenzo -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html