Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932423Ab3HNMxJ (ORCPT ); Wed, 14 Aug 2013 08:53:09 -0400 Received: from mail-oa0-f51.google.com ([209.85.219.51]:51400 "EHLO mail-oa0-f51.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932315Ab3HNMxG (ORCPT ); Wed, 14 Aug 2013 08:53:06 -0400 Message-ID: <520B7DAE.3060501@gmail.com> Date: Wed, 14 Aug 2013 07:53:02 -0500 From: Rob Herring User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130623 Thunderbird/17.0.7 MIME-Version: 1.0 To: Sudeep KarkadaNagesha , Benjamin Herrenschmidt CC: "linuxppc-dev@lists.ozlabs.org" , "Rafael J. Wysocki" , Viresh Kumar , Olof Johansson , "linux-pm@vger.kernel.org" , "linux-arm-kernel@lists.infradead.org" , "linux-kernel@vger.kernel.org" , "devicetree@vger.kernel.org" Subject: Re: [GIT PULL] DT/core: cpu_ofnode updates for v3.12 References: <5208E2D3.7060005@arm.com> <3356439.a21MloFP7n@vostro.rjw.lan> <520A536C.3030600@arm.com> <520A7B00.8060405@arm.com> <1376428024.4255.14.camel@pasglop> <520B5584.7030608@arm.com> In-Reply-To: <520B5584.7030608@arm.com> X-Enigmail-Version: 1.5.2 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4714 Lines: 137 On 08/14/2013 05:01 AM, Sudeep KarkadaNagesha wrote: > On 13/08/13 22:07, Benjamin Herrenschmidt wrote: >> On Tue, 2013-08-13 at 19:29 +0100, Sudeep KarkadaNagesha wrote: >>> I don't understand completely the use of ibm,ppc-interrupt-server#s and >>> its implications on generic of_get_cpu_node implementation. >>> I see the PPC specific definition of of_get_cpu_node uses thread id only >>> in 2 instances. Based on that, I have tried to move all the other >>> instances to use generic definition. >>> >>> Let me know if the idea is correct. >> >> No. The device-tree historically only represents cores, not HW threads, so >> it makes sense to retrieve also the thread number corresponding to the CPU. >> > Ok > >> However, the mechanism to represent HW threads in the device-tree is currently >> somewhat platform specific (the ibm,ppc-interrupt-server#s). > I see most of the callers pass NULL to thread id argument except 2 > instances in entire tree. If that's the case why can't we move to use > generic of_get_cpu_node in most of those cases and have PPC specific > implementation for the ones using thread id. > >> >> So what you could do for now is: >> >> - Have a generic version that always returns 0 as the thread, which is weak > I would prefer to move to generic of_get_cpu_node where ever possible > and rename the function that takes thread id rather than making generic > one weak. > >> >> - powerpc keeps its own implementation > How about only in cases where it needs thread_id. > >> >> - Start a discussion on the bindings (if not already there) to define threads >> in a better way at which point the generic function can be updated. >> > I am not sure if we need to define any new bindings. Excerpts from ePAPR > (v1.1): > "3.7.1 General Properties of CPU nodes > The value of "reg" is a that defines a unique > CPU/thread id for the CPU/threads represented by the CPU node. > If a CPU supports more than one thread (i.e. multiple streams of > execution) the reg property is an array with 1 element per thread. The > #address-cells on the /cpus node specifies how many cells each element > of the array takes. Software can determine the number of threads by > dividing the size of reg by the parent node's #address-cells." > > And this is exactly in agreement to what's implemented in the generic > of_get_cpu_node: > > for_each_child_of_node(cpus, cpun) { > if (of_node_cmp(cpun->type, "cpu")) > continue; > cell = of_get_property(cpun, "reg", &prop_len); > if (!cell) { > pr_warn("%s: missing reg property\n", cpun->full_name); > continue; > } > prop_len /= sizeof(*cell); > while (prop_len) { > hwid = of_read_number(cell, ac); > prop_len -= ac; > if (arch_match_cpu_phys_id(cpu, hwid)) > return cpun; > } > } How about something like this: for_each_child_of_node(cpus, cpun) { if (of_node_cmp(cpun->type, "cpu")) continue; if (arch_of_get_cpu_node(cpun, thread)) return cpun; cell = of_get_property(cpun, "reg", &prop_len); if (!cell) { pr_warn("%s: missing reg property\n", cpun->full_name); continue; } prop_len /= sizeof(*cell); while (prop_len) { hwid = of_read_number(cell, ac); prop_len -= ac; if (arch_match_cpu_phys_id(cpu, hwid)) return cpun; } } For PPC: arch_of_get_cpu_node() { const u32 *intserv; unsigned int plen, t; /* Check for ibm,ppc-interrupt-server#s. */ intserv = of_get_property(np, "ibm,ppc-interrupt-server#s", &plen); if (!intserv) return false; hardid = get_hard_smp_processor_id(cpu); plen /= sizeof(u32); for (t = 0; t < plen; t++) { if (hardid == intserv[t]) { if (thread) *thread = t; return true; } } return false; } > > Yes this doesn't cover the historical "ibm,ppc-interrupt-server#s", for > which we can have PPC specific wrapper above the generic one i.e. get > the cpu node and then parse for thread id under custom property. > > Let me know your thoughts. > > Regards, > Sudeep > > > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/