Quoting Viresh Kumar (2014-07-01 04:14:04)
On 1 July 2014 00:03, Rob Herring rob.herring@linaro.org wrote:
What about comparing "clocks" property in cpu DT nodes?
What if a different clock is selected for some reason.
I don't know why that will happen for CPUs sharing clock line.
I think a clock api function would be better.
@Mike: What do you think? I think we can get a clock API for this.
I can't help but think this is a pretty ugly solution. Why not specify the nature of the cpu clock(s) in DT directly? There was a thread already that discussed adding such a property to the CPU DT binding but it seems to have gone cold[1]. Furthermore my mailer sucks and I see now that my response to that thread never hit the list due to mangled headers. Here is a copy/paste of my response to the aforementioned thread:
""" I'll join the bikeshedding.
The hardware property that matters for cpufreq-cpu0 users is that a multi-core CPU uses a single clock input to scale frequency across all of the cores in that cluster. So an accurate description is:
scaling-method = "clock-ganged"; //hardware-people-speak
Or,
scaling-method = "clock-shared"; //software-people-speak
Versus independently scalable CPUs in an SMP cluster:
scaling-method = "independent"; //x86, Krait, etc.
Or perhaps instead of "independent" at the parent "cpus" node we would put the following in each cpu@N node:
scaling-method = "clock";
Or "psci" or "acpi" or whatever.
Thought exercise: for Hyperthreaded(tm) CPUs with 2 virtual cores for every hard CPU (and multiple CPUs in a cluster):
scaling-method = "paired";
Or more simply, "hyperthreaded". """
Regards, Mike
[1] http://www.spinics.net/lists/cpufreq/msg10034.html
That being said, I don't really have any issue with such a function. Some comments on the implementation.
+static int of_property_match(const struct device_node *np1,
const struct device_node *np2,
const char *list_name)
+{
const __be32 *list1, *list2, *list1_end;
s/list/prop/
Everywhere.
Ok.
int size1, size2;
phandle phandle1, phandle2;
/* Retrieve the list property */
list1 = of_get_property(np1, list_name, &size1);
if (!list1)
return -ENOENT;
list2 = of_get_property(np2, list_name, &size2);
if (!list2)
return -ENOENT;
if (size1 != size2)
return 0;
list1_end = list1 + size1 / sizeof(*list1);
/* Loop over the phandles */
while (list1 < list1_end) {
phandle1 = be32_to_cpup(list1++);
phandle2 = be32_to_cpup(list2++);
if (phandle1 != phandle2)
return 0;
}
You can just do a memcmp here.
Yeah, that would be much better.
This is wrong anyway because you don't know #clock-cells size.
I was actually comparing all the clock-cells, whatever there number is to make sure "clocks" properties are exactly same. Anyway memcmp will still guarantee that.
Thanks for your review.