[PATCH 11/11] of: unify phandle name in struct device_node

2009-11-24 Thread Grant Likely
In struct device_node, the phandle is named 'linux_phandle' for PowerPC
and MicroBlaze, and 'node' for SPARC.  There is no good reason for the
difference, it is just an artifact of the code diverging over a couple
of years.  This patch renames .node to .linux_phandle for SPARC.

Note: the .node also existed in PowerPC/MicroBlaze, but the only user
seems to be arch/powerpc/platforms/powermac/pfunc_core.c.  It doesn't
look like the assignment between .linux_phandle and .node is
significantly different enough to warrant the separate code paths
unless ibm,phandle properties actually appear in Apple device trees.

I think it is safe to eliminate the old .node property and use
linux_phandle everywhere.

Signed-off-by: Grant Likely grant.lik...@secretlab.ca
---

 arch/powerpc/platforms/powermac/pfunc_core.c |2 +-
 arch/sparc/kernel/devices.c  |2 +-
 arch/sparc/kernel/of_device_32.c |2 +-
 arch/sparc/kernel/of_device_64.c |2 +-
 arch/sparc/kernel/prom_common.c  |8 
 arch/sparc/kernel/smp_64.c   |2 +-
 drivers/of/fdt.c |3 +--
 drivers/sbus/char/openprom.c |   10 +-
 drivers/video/aty/atyfb_base.c   |2 +-
 include/linux/of.h   |3 ---
 10 files changed, 16 insertions(+), 20 deletions(-)

diff --git a/arch/powerpc/platforms/powermac/pfunc_core.c 
b/arch/powerpc/platforms/powermac/pfunc_core.c
index 96d5ce5..2004b19 100644
--- a/arch/powerpc/platforms/powermac/pfunc_core.c
+++ b/arch/powerpc/platforms/powermac/pfunc_core.c
@@ -842,7 +842,7 @@ struct pmf_function *__pmf_find_function(struct device_node 
*target,
list_for_each_entry(func, dev-functions, link) {
if (name  strcmp(name, func-name))
continue;
-   if (func-phandle  target-node != func-phandle)
+   if (func-phandle  target-linux_phandle != func-phandle)
continue;
if ((func-flags  flags) == 0)
continue;
diff --git a/arch/sparc/kernel/devices.c b/arch/sparc/kernel/devices.c
index b171ae8..2196e71 100644
--- a/arch/sparc/kernel/devices.c
+++ b/arch/sparc/kernel/devices.c
@@ -59,7 +59,7 @@ static int __cpu_find_by(int (*compare)(int, int, void *), 
void *compare_arg,
 
cur_inst = 0;
for_each_node_by_type(dp, cpu) {
-   int err = check_cpu_node(dp-node, cur_inst,
+   int err = check_cpu_node(dp-linux_phandle, cur_inst,
 compare, compare_arg,
 prom_node, mid);
if (!err) {
diff --git a/arch/sparc/kernel/of_device_32.c b/arch/sparc/kernel/of_device_32.c
index 4c26eb5..27f4c2e 100644
--- a/arch/sparc/kernel/of_device_32.c
+++ b/arch/sparc/kernel/of_device_32.c
@@ -433,7 +433,7 @@ build_resources:
if (!parent)
dev_set_name(op-dev, root);
else
-   dev_set_name(op-dev, %08x, dp-node);
+   dev_set_name(op-dev, %08x, dp-linux_phandle);
 
if (of_device_register(op)) {
printk(%s: Could not register of device.\n,
diff --git a/arch/sparc/kernel/of_device_64.c b/arch/sparc/kernel/of_device_64.c
index 881947e..9a6245d 100644
--- a/arch/sparc/kernel/of_device_64.c
+++ b/arch/sparc/kernel/of_device_64.c
@@ -666,7 +666,7 @@ static struct of_device * __init scan_one_device(struct 
device_node *dp,
if (!parent)
dev_set_name(op-dev, root);
else
-   dev_set_name(op-dev, %08x, dp-node);
+   dev_set_name(op-dev, %08x, dp-linux_phandle);
 
if (of_device_register(op)) {
printk(%s: Could not register of device.\n,
diff --git a/arch/sparc/kernel/prom_common.c b/arch/sparc/kernel/prom_common.c
index d80a65d..2a3e302 100644
--- a/arch/sparc/kernel/prom_common.c
+++ b/arch/sparc/kernel/prom_common.c
@@ -42,7 +42,7 @@ struct device_node *of_find_node_by_phandle(phandle handle)
struct device_node *np;
 
for (np = allnodes; np; np = np-allnext)
-   if (np-node == handle)
+   if (np-linux_phandle == handle)
break;
 
return np;
@@ -89,7 +89,7 @@ int of_set_property(struct device_node *dp, const char *name, 
void *val, int len
void *old_val = prop-value;
int ret;
 
-   ret = prom_setprop(dp-node, name, val, len);
+   ret = prom_setprop(dp-linux_phandle, name, val, len);
 
err = -EINVAL;
if (ret = 0) {
@@ -236,7 +236,7 @@ static struct device_node * __init prom_create_node(phandle 
node,
 
dp-name = get_one_property(node, name);
dp-type = get_one_property(node, device_type);
-   dp-node = node;
+   dp-linux_phandle = node;
 
dp-properties = 

Re: [PATCH 11/11] of: unify phandle name in struct device_node

2009-11-24 Thread David Miller
From: Grant Likely grant.lik...@secretlab.ca
Date: Tue, 24 Nov 2009 01:20:05 -0700

 In struct device_node, the phandle is named 'linux_phandle' for PowerPC
 and MicroBlaze, and 'node' for SPARC.  There is no good reason for the
 difference, it is just an artifact of the code diverging over a couple
 of years.  This patch renames .node to .linux_phandle for SPARC.

I know it's just a name, but there is no reason to put linux in the
member name.  It's the real device phandle value from OpenFirmware not
something invented by Linux's OF layer.

PowerPC uses this for something different, it records the information
here using the special linux,phandle and ibm,phandle properties.

See unflatten_dt_node() in arch/powerpc/kernel/prom.c before your
changes.
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH 11/11] of: unify phandle name in struct device_node

2009-11-24 Thread Grant Likely
On Tue, Nov 24, 2009 at 10:37 AM, David Miller da...@davemloft.net wrote:
 From: Grant Likely grant.lik...@secretlab.ca
 Date: Tue, 24 Nov 2009 01:20:05 -0700

 In struct device_node, the phandle is named 'linux_phandle' for PowerPC
 and MicroBlaze, and 'node' for SPARC.  There is no good reason for the
 difference, it is just an artifact of the code diverging over a couple
 of years.  This patch renames .node to .linux_phandle for SPARC.

 I know it's just a name, but there is no reason to put linux in the
 member name.  It's the real device phandle value from OpenFirmware not
 something invented by Linux's OF layer.

I agree, and I also considered renaming it to simply 'phandle' or to
change the powerpc code back to using .node everywhere (more on .node
usage in powerpc below).  I didn't want to use .node because .node is
pretty generic, and is used in other places for different things.
Basically I wanted to avoid confusion.  In particular, .node points to
a device_node, not a phandle, in struct of_device.  Changing all
references to simply 'phandle' seems to be the best, but it does
require changes to more locations in the code.  In the end I decided
on some constructive lazyness by settling for the already-used
.linux_phandle so that I would need to touch as many files, but still
retain a unique name that is easy to find.  If you prefer, I can
change it all to simply '.phandle'.

 PowerPC uses this for something different, it records the information
 here using the special linux,phandle and ibm,phandle properties.

Agreed, the phandle isn't a real phandle when using the flat tree.
However, though the source of the phandle value is different, the use
case is identical, so it make sense to stuff it into the same member.
Merging them lets me unify more code.  For example,
of_find_node_by_phandle().

 See unflatten_dt_node() in arch/powerpc/kernel/prom.c before your
 changes.

Yup, I looked at this.  The unflatten code stuffs both linux,phandle
and ibm,phandle into .linux_phandle, and linux,phandle also gets
stuffed into .node.  But all the users except one use the
.linux_phandle, not .node, and that remaining user *I think* can
safely use .linux_phandle too.

Thanks for the review,
g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH 11/11] of: unify phandle name in struct device_node

2009-11-24 Thread Benjamin Herrenschmidt
On Tue, 2009-11-24 at 09:37 -0800, David Miller wrote:
 From: Grant Likely grant.lik...@secretlab.ca
 Date: Tue, 24 Nov 2009 01:20:05 -0700
 
  In struct device_node, the phandle is named 'linux_phandle' for PowerPC
  and MicroBlaze, and 'node' for SPARC.  There is no good reason for the
  difference, it is just an artifact of the code diverging over a couple
  of years.  This patch renames .node to .linux_phandle for SPARC.
 
 I know it's just a name, but there is no reason to put linux in the
 member name.  It's the real device phandle value from OpenFirmware not
 something invented by Linux's OF layer.
 
 PowerPC uses this for something different, it records the information
 here using the special linux,phandle and ibm,phandle properties.
 
 See unflatten_dt_node() in arch/powerpc/kernel/prom.c before your
 changes.

Right, this comes from a subtle problem with our firmwares on pSeries.

We have a phandle from OF. But some devices, especially virtual devices,
also have an ibm,phandle which may or may not be the same afaik.

In addition, when the hypervisor hotplugs some devices, it sends us some
new device-tree bits to add. Those don't have a phandle in the formal
sense afaik, but -do- have an ibm,phandle property.

I think we can always just use ibm,phandle instead of the OF one when
the former is present, they should be non overlapping, but of course,
any chance in that area will require plenty of testing on some of those
machines.

Cheers,
Ben.

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH 11/11] of: unify phandle name in struct device_node

2009-11-24 Thread David Miller
From: Grant Likely grant.lik...@secretlab.ca
Date: Tue, 24 Nov 2009 13:33:22 -0700

 If you prefer, I can change it all to simply '.phandle'.

Please do.
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH 11/11] of: unify phandle name in struct device_node

2009-11-24 Thread Grant Likely
On Tue, Nov 24, 2009 at 2:06 PM, Benjamin Herrenschmidt
b...@kernel.crashing.org wrote:
 On Tue, 2009-11-24 at 09:37 -0800, David Miller wrote:
 From: Grant Likely grant.lik...@secretlab.ca
 Date: Tue, 24 Nov 2009 01:20:05 -0700

  In struct device_node, the phandle is named 'linux_phandle' for PowerPC
  and MicroBlaze, and 'node' for SPARC.  There is no good reason for the
  difference, it is just an artifact of the code diverging over a couple
  of years.  This patch renames .node to .linux_phandle for SPARC.

 I know it's just a name, but there is no reason to put linux in the
 member name.  It's the real device phandle value from OpenFirmware not
 something invented by Linux's OF layer.

 PowerPC uses this for something different, it records the information
 here using the special linux,phandle and ibm,phandle properties.

 See unflatten_dt_node() in arch/powerpc/kernel/prom.c before your
 changes.

 Right, this comes from a subtle problem with our firmwares on pSeries.

 We have a phandle from OF. But some devices, especially virtual devices,
 also have an ibm,phandle which may or may not be the same afaik.

 In addition, when the hypervisor hotplugs some devices, it sends us some
 new device-tree bits to add. Those don't have a phandle in the formal
 sense afaik, but -do- have an ibm,phandle property.

 I think we can always just use ibm,phandle instead of the OF one when
 the former is present, they should be non overlapping, but of course,
 any chance in that area will require plenty of testing on some of those
 machines.

If pseries is the troublesome platform, not powermac, then I'm pretty
confident this change is okay.  I cannot find any other users of .node
other than arch/powerpc/platforms/powermac/pfunc_core.c, but I'll
double check with an allmodconfig and an allyesconfig.  This patch
shouldn't change the behaviour of linux_phandle at all.

g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev