Patchwork [RFC,v2,3/4] powerpc: refactor of_get_cpu_node to support other architectures

login
register
mail settings
Submitter Sudeep.KarkadaNagesha@arm.com
Date Aug. 16, 2013, 5:39 p.m.
Message ID <1376674791-28244-2-git-send-email-Sudeep.KarkadaNagesha@arm.com>
Download mbox | patch
Permalink /patch/267725/
State Not Applicable
Headers show

Comments

Sudeep.KarkadaNagesha@arm.com - Aug. 16, 2013, 5:39 p.m.
From: Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com>

Currently different drivers requiring to access cpu device node are
parsing the device tree themselves. Since the ordering in the DT need
not match the logical cpu ordering, the parsing logic needs to consider
that. However, this has resulted in lots of code duplication and in some
cases even incorrect logic.

It's better to consolidate them by adding support for getting cpu
device node for a given logical cpu index in DT core library. However
logical to physical index mapping can be architecture specific.

PowerPC has it's own implementation to get the cpu node for a given
logical index.

This patch refactors the current implementation of of_get_cpu_node.
This in preparation to move the implementation to DT core library.
It separates out the logical to physical mapping so that a default
matching of the physical id to the logical cpu index can be added
when moved to common code. Architecture specific code can override it.

Cc: Rob Herring <rob.herring@calxeda.com>
Cc: Grant Likely <grant.likely@linaro.org>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Signed-off-by: Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com>
---
 arch/powerpc/kernel/prom.c | 76 ++++++++++++++++++++++++++++------------------
 1 file changed, 47 insertions(+), 29 deletions(-)
Benjamin Herrenschmidt - Aug. 16, 2013, 10:13 p.m.
On Fri, 2013-08-16 at 18:39 +0100, Sudeep KarkadaNagesha wrote:
> +static bool __of_find_n_match_cpu_property(struct device_node *cpun,
> +                       const char *prop_name, int cpu, unsigned int
> *thread)
> +{
> +       const __be32 *cell;
> +       int ac, prop_len, tid;
> +       u64 hwid;
> +
> +       ac = of_n_addr_cells(cpun);
> +       cell = of_get_property(cpun, prop_name, &prop_len);
> +       if (!cell)
> +               return false;
> +       prop_len /= sizeof(*cell);
> +       for (tid = 0; tid < prop_len; tid++) {
> +               hwid = of_read_number(cell, ac);
> +               if (arch_match_cpu_phys_id(cpu, hwid)) {
> +                       if (thread)
> +                               *thread = tid;
> +                       return true;
> +               }
> +               cell += ac;
> +       }
> +       return false;
> +}

The only problem I can see here is if "ac" is not 1, that will not work
for the ibm,ppc-interrupt-server#s case. IE. The latter is always 1 cell
per entry, only "reg" depends on #address-cells.

However that's only a theorical problem since on ppc #address-cells of
/cpus is always 1...

Cheers,
Ben.
Tomasz Figa - Aug. 17, 2013, 10:50 a.m.
Hi Sudeep,

This looks good to me overall, but I have one more question inline.

On Friday 16 of August 2013 18:39:50 Sudeep KarkadaNagesha wrote:
> From: Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com>
> 
> Currently different drivers requiring to access cpu device node are
> parsing the device tree themselves. Since the ordering in the DT need
> not match the logical cpu ordering, the parsing logic needs to consider
> that. However, this has resulted in lots of code duplication and in some
> cases even incorrect logic.
> 
> It's better to consolidate them by adding support for getting cpu
> device node for a given logical cpu index in DT core library. However
> logical to physical index mapping can be architecture specific.
> 
> PowerPC has it's own implementation to get the cpu node for a given
> logical index.
> 
> This patch refactors the current implementation of of_get_cpu_node.
> This in preparation to move the implementation to DT core library.
> It separates out the logical to physical mapping so that a default
> matching of the physical id to the logical cpu index can be added
> when moved to common code. Architecture specific code can override it.
> 
> Cc: Rob Herring <rob.herring@calxeda.com>
> Cc: Grant Likely <grant.likely@linaro.org>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Signed-off-by: Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com>
> ---
>  arch/powerpc/kernel/prom.c | 76
> ++++++++++++++++++++++++++++------------------ 1 file changed, 47
> insertions(+), 29 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
> index eb23ac9..fb12be6 100644
> --- a/arch/powerpc/kernel/prom.c
> +++ b/arch/powerpc/kernel/prom.c
> @@ -865,45 +865,63 @@ static int __init prom_reconfig_setup(void)
>  __initcall(prom_reconfig_setup);
>  #endif
> 
> +bool arch_match_cpu_phys_id(int cpu, u64 phys_id)
> +{
> +	return (int)phys_id == get_hard_smp_processor_id(cpu);
> +}
> +
> +static bool __of_find_n_match_cpu_property(struct device_node *cpun,
> +			const char *prop_name, int cpu, unsigned int 
*thread)
> +{
> +	const __be32 *cell;
> +	int ac, prop_len, tid;
> +	u64 hwid;
> +
> +	ac = of_n_addr_cells(cpun);
> +	cell = of_get_property(cpun, prop_name, &prop_len);
> +	if (!cell)
> +		return false;

I wonder how would this handle uniprocessor ARM (pre-v7) cores, for which 
the updated bindings[1] define #address-cells = <0> and so no reg 
property.

[1] - http://thread.gmane.org/gmane.linux.ports.arm.kernel/260795

Best regards,
Tomasz
Benjamin Herrenschmidt - Aug. 17, 2013, 10:09 p.m.
On Sat, 2013-08-17 at 12:50 +0200, Tomasz Figa wrote:
> I wonder how would this handle uniprocessor ARM (pre-v7) cores, for
> which 
> the updated bindings[1] define #address-cells = <0> and so no reg 
> property.
> 
> [1] - http://thread.gmane.org/gmane.linux.ports.arm.kernel/260795

Why did you do that in the binding ? That sounds like looking to create
problems ... 

Traditionally, UP setups just used "0" as the "reg" property on other
architectures, why do differently ?

Ben.
Tomasz Figa - Aug. 17, 2013, 10:22 p.m.
On Sunday 18 of August 2013 08:09:36 Benjamin Herrenschmidt wrote:
> On Sat, 2013-08-17 at 12:50 +0200, Tomasz Figa wrote:
> > I wonder how would this handle uniprocessor ARM (pre-v7) cores, for
> > which
> > the updated bindings[1] define #address-cells = <0> and so no reg
> > property.
> > 
> > [1] - http://thread.gmane.org/gmane.linux.ports.arm.kernel/260795
> 
> Why did you do that in the binding ? That sounds like looking to create
> problems ...

[Copying Lorenzo...]

I'm not the author of the change. I was just passing by, while the 
question showed up in my mind. ;)

> Traditionally, UP setups just used "0" as the "reg" property on other
> architectures, why do differently ?

Right, especially since the ARM DT topology parsing code still considers a 
device tree without reg property in cpu node invalid.

Best regards,
Tomasz
Sudeep.KarkadaNagesha@arm.com - Aug. 19, 2013, 10:13 a.m.
On 16/08/13 23:13, Benjamin Herrenschmidt wrote:
> On Fri, 2013-08-16 at 18:39 +0100, Sudeep KarkadaNagesha wrote:
>> +static bool __of_find_n_match_cpu_property(struct device_node *cpun,
>> +                       const char *prop_name, int cpu, unsigned int
>> *thread)
>> +{
>> +       const __be32 *cell;
>> +       int ac, prop_len, tid;
>> +       u64 hwid;
>> +
>> +       ac = of_n_addr_cells(cpun);
>> +       cell = of_get_property(cpun, prop_name, &prop_len);
>> +       if (!cell)
>> +               return false;
>> +       prop_len /= sizeof(*cell);
>> +       for (tid = 0; tid < prop_len; tid++) {
>> +               hwid = of_read_number(cell, ac);
>> +               if (arch_match_cpu_phys_id(cpu, hwid)) {
>> +                       if (thread)
>> +                               *thread = tid;
>> +                       return true;
>> +               }
>> +               cell += ac;
>> +       }
>> +       return false;
>> +}
> 
> The only problem I can see here is if "ac" is not 1, that will not work
> for the ibm,ppc-interrupt-server#s case. IE. The latter is always 1 cell
> per entry, only "reg" depends on #address-cells.
> 
> However that's only a theorical problem since on ppc #address-cells of
> /cpus is always 1...
> 
Ok agreed, but I assume in future if thread ids need 2 ac, it would use
standard 'reg' instead of 'ibm,ppc-interrupt-server#' property.

So I assume the above function is generic and need not be modified to
handle non '1' ac case  with non standard 'ibm,ppc-interrupt-server#'.

Regards,
Sudeep
Mark Rutland - Aug. 19, 2013, 10:19 a.m.
On Sat, Aug 17, 2013 at 11:09:36PM +0100, Benjamin Herrenschmidt wrote:
> On Sat, 2013-08-17 at 12:50 +0200, Tomasz Figa wrote:
> > I wonder how would this handle uniprocessor ARM (pre-v7) cores, for
> > which 
> > the updated bindings[1] define #address-cells = <0> and so no reg 
> > property.
> > 
> > [1] - http://thread.gmane.org/gmane.linux.ports.arm.kernel/260795
> 
> Why did you do that in the binding ? That sounds like looking to create
> problems ... 
> 
> Traditionally, UP setups just used "0" as the "reg" property on other
> architectures, why do differently ?

The decision was taken because we defined our reg property to refer to
the MPIDR register's Aff{2,1,0} bitfields, and on UP cores before v7
there's no MPIDR register at all. Given there can only be a single CPU
in that case, describing a register that wasn't present didn't seem
necessary or helpful.

Thanks,
Mark.
Rob Herring - Aug. 19, 2013, 1:02 p.m.
On 08/19/2013 05:19 AM, Mark Rutland wrote:
> On Sat, Aug 17, 2013 at 11:09:36PM +0100, Benjamin Herrenschmidt wrote:
>> On Sat, 2013-08-17 at 12:50 +0200, Tomasz Figa wrote:
>>> I wonder how would this handle uniprocessor ARM (pre-v7) cores, for
>>> which 
>>> the updated bindings[1] define #address-cells = <0> and so no reg 
>>> property.
>>>
>>> [1] - http://thread.gmane.org/gmane.linux.ports.arm.kernel/260795
>>
>> Why did you do that in the binding ? That sounds like looking to create
>> problems ... 
>>
>> Traditionally, UP setups just used "0" as the "reg" property on other
>> architectures, why do differently ?
> 
> The decision was taken because we defined our reg property to refer to
> the MPIDR register's Aff{2,1,0} bitfields, and on UP cores before v7
> there's no MPIDR register at all. Given there can only be a single CPU
> in that case, describing a register that wasn't present didn't seem
> necessary or helpful.

What exactly reg represents is up to the binding definition, but it
still should be present IMO. I don't see any issue with it being
different for pre-v7.

Rob

> 
> Thanks,
> Mark.
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
Sudeep.KarkadaNagesha@arm.com - Aug. 19, 2013, 1:56 p.m.
On 19/08/13 14:02, Rob Herring wrote:
> On 08/19/2013 05:19 AM, Mark Rutland wrote:
>> On Sat, Aug 17, 2013 at 11:09:36PM +0100, Benjamin Herrenschmidt wrote:
>>> On Sat, 2013-08-17 at 12:50 +0200, Tomasz Figa wrote:
>>>> I wonder how would this handle uniprocessor ARM (pre-v7) cores, for
>>>> which 
>>>> the updated bindings[1] define #address-cells = <0> and so no reg 
>>>> property.
>>>>
>>>> [1] - http://thread.gmane.org/gmane.linux.ports.arm.kernel/260795
>>>
>>> Why did you do that in the binding ? That sounds like looking to create
>>> problems ... 
>>>
>>> Traditionally, UP setups just used "0" as the "reg" property on other
>>> architectures, why do differently ?
>>
>> The decision was taken because we defined our reg property to refer to
>> the MPIDR register's Aff{2,1,0} bitfields, and on UP cores before v7
>> there's no MPIDR register at all. Given there can only be a single CPU
>> in that case, describing a register that wasn't present didn't seem
>> necessary or helpful.
> 
> What exactly reg represents is up to the binding definition, but it
> still should be present IMO. I don't see any issue with it being
> different for pre-v7.
> 
Yes it's better to have 'reg' with value 0 than not having it.
Otherwise this generic of_get_cpu_node implementation would need some
_hack_ to handle that case.

Regards,
Sudeep
Mark Rutland - Aug. 22, 2013, 1:59 p.m.
On Mon, Aug 19, 2013 at 02:56:10PM +0100, Sudeep KarkadaNagesha wrote:
> On 19/08/13 14:02, Rob Herring wrote:
> > On 08/19/2013 05:19 AM, Mark Rutland wrote:
> >> On Sat, Aug 17, 2013 at 11:09:36PM +0100, Benjamin Herrenschmidt wrote:
> >>> On Sat, 2013-08-17 at 12:50 +0200, Tomasz Figa wrote:
> >>>> I wonder how would this handle uniprocessor ARM (pre-v7) cores, for
> >>>> which 
> >>>> the updated bindings[1] define #address-cells = <0> and so no reg 
> >>>> property.
> >>>>
> >>>> [1] - http://thread.gmane.org/gmane.linux.ports.arm.kernel/260795
> >>>
> >>> Why did you do that in the binding ? That sounds like looking to create
> >>> problems ... 
> >>>
> >>> Traditionally, UP setups just used "0" as the "reg" property on other
> >>> architectures, why do differently ?
> >>
> >> The decision was taken because we defined our reg property to refer to
> >> the MPIDR register's Aff{2,1,0} bitfields, and on UP cores before v7
> >> there's no MPIDR register at all. Given there can only be a single CPU
> >> in that case, describing a register that wasn't present didn't seem
> >> necessary or helpful.
> > 
> > What exactly reg represents is up to the binding definition, but it
> > still should be present IMO. I don't see any issue with it being
> > different for pre-v7.
> > 
> Yes it's better to have 'reg' with value 0 than not having it.
> Otherwise this generic of_get_cpu_node implementation would need some
> _hack_ to handle that case.

I'm not sure that having some code to handle a difference in standard
between two architectures is a hack. If anything, I'd argue encoding a
reg of 0 that corresponds to a nonexistent MPIDR value (given that's
what the reg property is defined to map to on ARM) is more of a hack ;)

I'm not averse to having a reg value of 0 for this case, but given that
there are existing devicetrees without it, requiring a reg property will
break compatibility with them.

Mark.
Sudeep.KarkadaNagesha@arm.com - Aug. 22, 2013, 4:51 p.m.
On 22/08/13 14:59, Mark Rutland wrote:
> On Mon, Aug 19, 2013 at 02:56:10PM +0100, Sudeep KarkadaNagesha wrote:
>> On 19/08/13 14:02, Rob Herring wrote:
>>> On 08/19/2013 05:19 AM, Mark Rutland wrote:
>>>> On Sat, Aug 17, 2013 at 11:09:36PM +0100, Benjamin Herrenschmidt wrote:
>>>>> On Sat, 2013-08-17 at 12:50 +0200, Tomasz Figa wrote:
>>>>>> I wonder how would this handle uniprocessor ARM (pre-v7) cores, for
>>>>>> which 
>>>>>> the updated bindings[1] define #address-cells = <0> and so no reg 
>>>>>> property.
>>>>>>
>>>>>> [1] - http://thread.gmane.org/gmane.linux.ports.arm.kernel/260795
>>>>>
>>>>> Why did you do that in the binding ? That sounds like looking to create
>>>>> problems ... 
>>>>>
>>>>> Traditionally, UP setups just used "0" as the "reg" property on other
>>>>> architectures, why do differently ?
>>>>
>>>> The decision was taken because we defined our reg property to refer to
>>>> the MPIDR register's Aff{2,1,0} bitfields, and on UP cores before v7
>>>> there's no MPIDR register at all. Given there can only be a single CPU
>>>> in that case, describing a register that wasn't present didn't seem
>>>> necessary or helpful.
>>>
>>> What exactly reg represents is up to the binding definition, but it
>>> still should be present IMO. I don't see any issue with it being
>>> different for pre-v7.
>>>
>> Yes it's better to have 'reg' with value 0 than not having it.
>> Otherwise this generic of_get_cpu_node implementation would need some
>> _hack_ to handle that case.
> 
> I'm not sure that having some code to handle a difference in standard
> between two architectures is a hack. If anything, I'd argue encoding a
> reg of 0 that corresponds to a nonexistent MPIDR value (given that's
> what the reg property is defined to map to on ARM) is more of a hack ;)
> 
Agreed. But I am more confused.

1. This raises another question as how much do we follow from ePAPR
standard. ePAPR marks the reg property in /cpu as required.
Does that mean we must have all the properties marked as required to be
present in DT ? On the contrary timebase/clock-frequency is some thing
that can be found dynamically and need not be present but they are
marked as required too.

> I'm not averse to having a reg value of 0 for this case, but given that
> there are existing devicetrees without it, requiring a reg property will
> break compatibility with them.
> 

2. What exactly does backward compatibility has to cover ? This can't be
considered as breaking of the binding definition. In continuation to the
above argument that reg property is required, do we need to cover this
as it's clearly a case of missing required property(this holds only if
reg is required always).

Regards,
Sudeep
Grant Likely - Aug. 28, 2013, 7:46 p.m.
On Thu, 22 Aug 2013 14:59:30 +0100, Mark Rutland <mark.rutland@arm.com> wrote:
> On Mon, Aug 19, 2013 at 02:56:10PM +0100, Sudeep KarkadaNagesha wrote:
> > On 19/08/13 14:02, Rob Herring wrote:
> > > On 08/19/2013 05:19 AM, Mark Rutland wrote:
> > >> On Sat, Aug 17, 2013 at 11:09:36PM +0100, Benjamin Herrenschmidt wrote:
> > >>> On Sat, 2013-08-17 at 12:50 +0200, Tomasz Figa wrote:
> > >>>> I wonder how would this handle uniprocessor ARM (pre-v7) cores, for
> > >>>> which 
> > >>>> the updated bindings[1] define #address-cells = <0> and so no reg 
> > >>>> property.
> > >>>>
> > >>>> [1] - http://thread.gmane.org/gmane.linux.ports.arm.kernel/260795
> > >>>
> > >>> Why did you do that in the binding ? That sounds like looking to create
> > >>> problems ... 
> > >>>
> > >>> Traditionally, UP setups just used "0" as the "reg" property on other
> > >>> architectures, why do differently ?
> > >>
> > >> The decision was taken because we defined our reg property to refer to
> > >> the MPIDR register's Aff{2,1,0} bitfields, and on UP cores before v7
> > >> there's no MPIDR register at all. Given there can only be a single CPU
> > >> in that case, describing a register that wasn't present didn't seem
> > >> necessary or helpful.
> > > 
> > > What exactly reg represents is up to the binding definition, but it
> > > still should be present IMO. I don't see any issue with it being
> > > different for pre-v7.
> > > 
> > Yes it's better to have 'reg' with value 0 than not having it.
> > Otherwise this generic of_get_cpu_node implementation would need some
> > _hack_ to handle that case.
> 
> I'm not sure that having some code to handle a difference in standard
> between two architectures is a hack. If anything, I'd argue encoding a
> reg of 0 that corresponds to a nonexistent MPIDR value (given that's
> what the reg property is defined to map to on ARM) is more of a hack ;)
> 
> I'm not averse to having a reg value of 0 for this case, but given that
> there are existing devicetrees without it, requiring a reg property will
> break compatibility with them.

Then special cases those device trees, but you changing existing
convention really needs to be avoided. The referenced documentation
change is brand new, so we're not stuck with it.

g.
Lorenzo Pieralisi - Aug. 29, 2013, 9:50 a.m.
On Wed, Aug 28, 2013 at 08:46:38PM +0100, Grant Likely wrote:
> On Thu, 22 Aug 2013 14:59:30 +0100, Mark Rutland <mark.rutland@arm.com> wrote:
> > On Mon, Aug 19, 2013 at 02:56:10PM +0100, Sudeep KarkadaNagesha wrote:
> > > On 19/08/13 14:02, Rob Herring wrote:
> > > > On 08/19/2013 05:19 AM, Mark Rutland wrote:
> > > >> On Sat, Aug 17, 2013 at 11:09:36PM +0100, Benjamin Herrenschmidt wrote:
> > > >>> On Sat, 2013-08-17 at 12:50 +0200, Tomasz Figa wrote:
> > > >>>> I wonder how would this handle uniprocessor ARM (pre-v7) cores, for
> > > >>>> which 
> > > >>>> the updated bindings[1] define #address-cells = <0> and so no reg 
> > > >>>> property.
> > > >>>>
> > > >>>> [1] - http://thread.gmane.org/gmane.linux.ports.arm.kernel/260795
> > > >>>
> > > >>> Why did you do that in the binding ? That sounds like looking to create
> > > >>> problems ... 
> > > >>>
> > > >>> Traditionally, UP setups just used "0" as the "reg" property on other
> > > >>> architectures, why do differently ?
> > > >>
> > > >> The decision was taken because we defined our reg property to refer to
> > > >> the MPIDR register's Aff{2,1,0} bitfields, and on UP cores before v7
> > > >> there's no MPIDR register at all. Given there can only be a single CPU
> > > >> in that case, describing a register that wasn't present didn't seem
> > > >> necessary or helpful.
> > > > 
> > > > What exactly reg represents is up to the binding definition, but it
> > > > still should be present IMO. I don't see any issue with it being
> > > > different for pre-v7.
> > > > 
> > > Yes it's better to have 'reg' with value 0 than not having it.
> > > Otherwise this generic of_get_cpu_node implementation would need some
> > > _hack_ to handle that case.
> > 
> > I'm not sure that having some code to handle a difference in standard
> > between two architectures is a hack. If anything, I'd argue encoding a
> > reg of 0 that corresponds to a nonexistent MPIDR value (given that's
> > what the reg property is defined to map to on ARM) is more of a hack ;)
> > 
> > I'm not averse to having a reg value of 0 for this case, but given that
> > there are existing devicetrees without it, requiring a reg property will
> > break compatibility with them.
> 
> Then special cases those device trees, but you changing existing
> convention really needs to be avoided. The referenced documentation
> change is brand new, so we're not stuck with it.

I have no problem with changing the bindings and forcing:

#address-cells = <1>;
reg = <0>;

for UP predating v7, my big worry is related to in-kernel dts that we
already patched to follow the #address-cells = <0> rule (and we had to
do it since we got asked that question multiple times on the public
lists).

What do you mean by "special case those device trees" ? I have not
planned to patch them again, unless we really consider that a necessary
evil.

Thanks,
Lorenzo

Patch

diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
index eb23ac9..fb12be6 100644
--- a/arch/powerpc/kernel/prom.c
+++ b/arch/powerpc/kernel/prom.c
@@ -865,45 +865,63 @@  static int __init prom_reconfig_setup(void)
 __initcall(prom_reconfig_setup);
 #endif
 
+bool arch_match_cpu_phys_id(int cpu, u64 phys_id)
+{
+	return (int)phys_id == get_hard_smp_processor_id(cpu);
+}
+
+static bool __of_find_n_match_cpu_property(struct device_node *cpun,
+			const char *prop_name, int cpu, unsigned int *thread)
+{
+	const __be32 *cell;
+	int ac, prop_len, tid;
+	u64 hwid;
+
+	ac = of_n_addr_cells(cpun);
+	cell = of_get_property(cpun, prop_name, &prop_len);
+	if (!cell)
+		return false;
+	prop_len /= sizeof(*cell);
+	for (tid = 0; tid < prop_len; tid++) {
+		hwid = of_read_number(cell, ac);
+		if (arch_match_cpu_phys_id(cpu, hwid)) {
+			if (thread)
+				*thread = tid;
+			return true;
+		}
+		cell += ac;
+	}
+	return false;
+}
+
 /* Find the device node for a given logical cpu number, also returns the cpu
  * local thread number (index in ibm,interrupt-server#s) if relevant and
  * asked for (non NULL)
  */
 struct device_node *of_get_cpu_node(int cpu, unsigned int *thread)
 {
-	int hardid;
-	struct device_node *np;
+	struct device_node *cpun, *cpus;
 
-	hardid = get_hard_smp_processor_id(cpu);
+	cpus = of_find_node_by_path("/cpus");
+	if (!cpus) {
+		pr_warn("Missing cpus node, bailing out\n");
+		return NULL;
+	}
 
-	for_each_node_by_type(np, "cpu") {
-		const u32 *intserv;
-		unsigned int plen, t;
+	for_each_child_of_node(cpus, cpun) {
+		if (of_node_cmp(cpun->type, "cpu"))
+			continue;
 
-		/* Check for ibm,ppc-interrupt-server#s. If it doesn't exist
-		 * fallback to "reg" property and assume no threads
+		/* Check for historical "ibm,ppc-interrupt-server#s" property
+		 * for thread ids on PowerPC. If it doesn't exist fallback to
+		 * standard "reg" property.
 		 */
-		intserv = of_get_property(np, "ibm,ppc-interrupt-server#s",
-				&plen);
-		if (intserv == NULL) {
-			const u32 *reg = of_get_property(np, "reg", NULL);
-			if (reg == NULL)
-				continue;
-			if (*reg == hardid) {
-				if (thread)
-					*thread = 0;
-				return np;
-			}
-		} else {
-			plen /= sizeof(u32);
-			for (t = 0; t < plen; t++) {
-				if (hardid == intserv[t]) {
-					if (thread)
-						*thread = t;
-					return np;
-				}
-			}
-		}
+		if (__of_find_n_match_cpu_property(cpun,
+				"ibm,ppc-interrupt-server#s", cpu, thread))
+			return cpun;
+
+		if (__of_find_n_match_cpu_property(cpun, "reg", cpu, thread))
+			return cpun;
 	}
 	return NULL;
 }