Quoting Viresh Kumar (2014-07-01 04:14:04) > On 1 July 2014 00:03, Rob Herring <rob.herring@xxxxxxxxxx> 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. -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
