On Wed, Sep 25, 2013 at 3:29 AM, Rob Herring <robherring2@xxxxxxxxx> wrote: > On Sun, Sep 22, 2013 at 3:29 PM, David Miller <davem@xxxxxxxxxxxxx> wrote: >> From: Rob Herring <robherring2@xxxxxxxxx> >> Date: Sun, 22 Sep 2013 15:13:35 -0500 >> >>> On 09/20/2013 11:44 AM, Sudeep KarkadaNagesha wrote: >>>> On 19/09/13 18:38, David Miller wrote: >>>>> From: Rob Herring <robherring2@xxxxxxxxx> >>>>> Date: Thu, 19 Sep 2013 08:26:04 -0500 >>>>> >>>>>> The simple solution here is to just remove the warning. It doesn't >>>>>> appear to me that sparc ever sets the cpu device of_node pointer. >>>>> >>>>> Why wouldn't I want sparc to have this functionality now that the >>>>> code is generically available. >>>>> >>>> Makes sense, but as you mentioned before we need to match other property >>>> names namely "upa-portid", "portid" or "cpuid" for cpu physical id right >>>> ? Are all these 32-bit values ? >>>> >>>>>> Are there any cases on where sparc does have a /cpus node? If so we'd >>>>>> need to make of_get_cpu_node a weak function or depend on a kconfig option. >>>>> >>>>> I already gave a solution to this problem, make the loop iterator be: >>>>> >>>>> for_each_node_by_type(dp, "cpu") >>>>> >>>> >>>> Does it make sense use this only when /cpus is not found ? >>>> IMHO as the number of node in DT increases(which is the case on ARM >>>> platforms) parsing entire tree may be expensive(which can be avoided in >>>> case /cpus is found) >>> >>> Can't you simply do something like this for the search: >>> >>> cpus = of_find_node_by_path("/cpus"); >>> if (!cpus && IS_ENABLED(CONFIG_SPARC)) >>> cpus = of_find_node_by_path("/"); >>> if (!cpus) { >>> pr_warn("Missing cpus node, bailing out\n"); >>> return NULL; >>> } >>> >>> for_each_child_of_node(cpus, cpun) { >> >> This doesn't work either Rob. Some UltraSPARC-IV systems put the cpu >> nodes under directories representing the cores. > > Okay. Good to know. > >> For the third time, the only thing which will work unilaterally is >> the type base iterator I proposed above. > > Now that I understand all the variations, I agree. > >> Why is there so much resistence to something which has an extremely >> high likelyhood of working everywhere and not requiring any ifdef >> crapola? > > Simplifying Sudeep's code a bit while maintaining some level of > validation of the DT layout was my only goal. I guess that validation > really needs to be at build time rather than boot time and there is > some work going on in that area. Still need a solution here in short order. Sundeep, are you working on a fixup patch? I can remove the warning as a short term fix, but to make the function useful generally then there needs to be a way for the architecture to tell the core what property name to use for CPU ids. The ugly powerpc exception that is in there should be reworked at the same time. g. -- 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