of: move phandle/ihandle into types.h and export to userspace

PATCH In review Andres Salomon 3 participants 16 messages Oct 8, 2010 – Oct 10, 2010

Subsystems (2)

SPARC + UltraSPARC (sparc/sparc64) Device tree

lore

Share & tools

lore

Andres Salomon

author

2010-10-08 07:42 UTC #0 lore raw

PATCH v2 1/2 of: move phandle/ihandle into types.h and export to userspace

We need phandle for some exported sparc headers; of.h isn't an exported header, and it would be silly to export it when all we really need is one or two types from it. Also, later patches use phandle in structs that are exported to userspace, so export a __kernel_phandle type.

Signed-off-by Andres Salomon <[email protected]>

---
 include/linux/of.h    |    3 ---
 include/linux/types.h |    7 +++++++
 2 files changed, 7 insertions(+), 3 deletions(-)

include/linux/of.h +0 -3

index cad7cf0..db184dc 100644
@@ -25,9 +25,6 @@
 
 #ifdef CONFIG_OF
 
-typedef u32 phandle;
-typedef u32 ihandle;
-
 struct property {
 	char	*name;
 	int	length;

include/linux/types.h +7 -0

index 01a082f..67a034a 100644
@@ -178,10 +178,17 @@ typedef __u64 __bitwise __be64;
 typedef __u16 __bitwise __sum16;
 typedef __u32 __bitwise __wsum;
 
+/* Basic openboot/openfirmware types */
+typedef __u32 __kernel_phandle;
+typedef __u32 __kernel_ihandle;
+
 #ifdef __KERNEL__
 typedef unsigned __bitwise__ gfp_t;
 typedef unsigned __bitwise__ fmode_t;
 
+typedef __kernel_phandle phandle;
+typedef __kernel_ihandle ihandle;
+
 #ifdef CONFIG_PHYS_ADDR_T_64BIT
 typedef u64 phys_addr_t;
 #else

Signature · 1 line

1.5.6.5

Grant Likely

2010-10-08 17:12 UTC #1 lore raw

Quoted 8 lines · Andres Salomon

We need phandle for some exported sparc headers; of.h isn't an exported header, and it would be silly to export it when all we really need is one or two types from it. Also, later patches use phandle in structs that are exported to userspace, so export a __kernel_phandle type.
Signed-off-by Andres Salomon <[email protected]>

Looks okay to me. I'm build testing it now on powerpc and microblaze. If davem agrees, then I'll pick it up into my devicetree-next branch.

Question though; since sparc userspace is the only user of this, should the types.h header avoid exposing it on non-sparc?

Quoted 44 lines

---
 include/linux/of.h    |    3 ---
 include/linux/types.h |    7 +++++++
 2 files changed, 7 insertions(+), 3 deletions(-)

include/linux/of.h +0 -3

index cad7cf0..db184dc 100644
@@ -25,9 +25,6 @@
 
 #ifdef CONFIG_OF
 
-typedef u32 phandle;
-typedef u32 ihandle;
-
 struct property {
 	char	*name;
 	int	length;

include/linux/types.h +7 -0

index 01a082f..67a034a 100644
@@ -178,10 +178,17 @@ typedef __u64 __bitwise __be64;
 typedef __u16 __bitwise __sum16;
 typedef __u32 __bitwise __wsum;
 
+/* Basic openboot/openfirmware types */
+typedef __u32 __kernel_phandle;
+typedef __u32 __kernel_ihandle;
+
 #ifdef __KERNEL__
 typedef unsigned __bitwise__ gfp_t;
 typedef unsigned __bitwise__ fmode_t;
 
+typedef __kernel_phandle phandle;
+typedef __kernel_ihandle ihandle;
+
 #ifdef CONFIG_PHYS_ADDR_T_64BIT
 typedef u64 phys_addr_t;
 #else

Signature · 1 line

1.5.6.5

David Miller

maintainer

2010-10-08 17:17 UTC #2 lore raw

From: Grant Likely <[email protected]>
Date: Fri, 8 Oct 2010 11:12:10 -0600

Quoted 8 lines · Andres Salomon

Quoted 7 lines
We need phandle for some exported sparc headers; of.h isn't an exported header, and it would be silly to export it when all we really need is one or two types from it. Also, later patches use phandle in structs that are exported to userspace, so export a __kernel_phandle type.

Quoted 8 lines

Quoted 1 line
Signed-off-by Andres Salomon <[email protected]>
Looks okay to me. I'm build testing it now on powerpc and microblaze. If davem agrees, then I'll pick it up into my devicetree-next branch.
Question though; since sparc userspace is the only user of this, should the types.h header avoid exposing it on non-sparc?

I can't think of anything in Sparc userspace that needs the phandle_t type.

In fact I was a bit confused when you asked Andres to expose it to userspace.

What I remember from a previous thread is that some other platforms use the type in their DT device-tree building tools or whatever, but they define phandle_t for themselves I remember specifically mentioning sparc has no use for this currently.

Grant Likely

2010-10-08 17:36 UTC #3 lore raw

Quoted 23 lines · David Miller

From: Grant Likely <[email protected]>
Date: Fri, 8 Oct 2010 11:12:10 -0600
Quoted 8 lines · Andres Salomon
Quoted 7 lines
We need phandle for some exported sparc headers; of.h isn't an exported header, and it would be silly to export it when all we really need is one or two types from it. Also, later patches use phandle in structs that are exported to userspace, so export a __kernel_phandle type.
Quoted 8 lines
Quoted 1 line
Signed-off-by Andres Salomon <[email protected]>
Looks okay to me. I'm build testing it now on powerpc and microblaze. If davem agrees, then I'll pick it up into my devicetree-next branch.
Question though; since sparc userspace is the only user of this, should the types.h header avoid exposing it on non-sparc?
I can't think of anything in Sparc userspace that needs the phandle_t type.

Quoted 7 lines

In fact I was a bit confused when you asked Andres to expose it to userspace.
What I remember from a previous thread is that some other platforms use the type in their DT device-tree building tools or whatever, but they define phandle_t for themselves I remember specifically mentioning sparc has no use for this currently.

Weird. Yeah, no other platforms expect to get a phandle type definition from the kernel headers. The only thing driving this conversation is from arch/sparc/include/asm/Kbuild:

header-y += openprom.h

I don't have any background on the history of that file or why it needs to be exported to userspace. There seems to be a lot of stuff in there; but I cannot see the *_funcs structures having any utility outside of the kernel. Perhaps they should be moved out of asm/openprom.h?

Here are the two affected structures:

$ git grep linux_nodeops arch/sparc/include
arch/sparc/include/asm/openprom.h:      struct linux_nodeops *pv_nodeops;
arch/sparc/include/asm/openprom.h:struct linux_nodeops {
arch/sparc/include/asm/oplib_32.h:extern struct linux_nodeops *prom_nodeops;
$ git grep linux_dev_v2_funcs arch/sparc/include
arch/sparc/include/asm/openprom.h:struct linux_dev_v2_funcs {
arch/sparc/include/asm/openprom.h:      struct linux_dev_v2_funcs pv_v2devops;
(devicetree/next) ~/hacking/linux-2.6$

And linux_romvec references linux_nodeops:

$ git grep linux_romvec arch/sparc/include
arch/sparc/include/asm/openprom.h:struct linux_romvec {
arch/sparc/include/asm/oplib_32.h:extern struct linux_romvec *romvec;
arch/sparc/include/asm/oplib_32.h:extern void prom_init(struct linux_romvec *rom_ptr);

oplib_32.h is not exported.

David Miller

maintainer

2010-10-08 17:45 UTC #4 lore raw

From: Grant Likely <[email protected]>
Date: Fri, 8 Oct 2010 11:36:50 -0600

Quoted 5 lines

Weird. Yeah, no other platforms expect to get a phandle type definition from the kernel headers. The only thing driving this conversation is from arch/sparc/include/asm/Kbuild:
header-y += openprom.h

It was probably at one point for the sake of asm/openpromio.h but that header has no dependencies on openprom.h

The only hit I can find in google code search, for non-kernel code, is the SILO bootloader.

But that tree includes it's own copy of include/asm/openprom.h so the actual kernel copy isn't even used.

I'd say we can stop exporting that header and also therefore not worry about making phandle_t visible to userspace.

Grant Likely

2010-10-08 18:27 UTC #5 lore raw

Quoted 12 lines · David Miller

From: Grant Likely <[email protected]>
Date: Fri, 8 Oct 2010 11:36:50 -0600
Quoted 5 lines
Weird. Yeah, no other platforms expect to get a phandle type definition from the kernel headers. The only thing driving this conversation is from arch/sparc/include/asm/Kbuild:
header-y += openprom.h
It was probably at one point for the sake of asm/openpromio.h but that header has no dependencies on openprom.h

Quoted 8 lines

The only hit I can find in google code search, for non-kernel code, is the SILO bootloader.
But that tree includes it's own copy of include/asm/openprom.h so the actual kernel copy isn't even used.
I'd say we can stop exporting that header and also therefore not worry about making phandle_t visible to userspace.

Yay! That simplifies everything, and I believe it also means that the phandle/ihandle definitions can remain where they currently are in linux/of.h

Andres, can you post an updated series that includes removing openprom.h from the header export list?

Thanks,
g.

Andres Salomon

author

2010-10-08 18:37 UTC #6 lore raw

Quoted 22 lines · On Fri, 8 Oct 2010 12:27:45 -0600 Grant Likely <[email protected]>…

Quoted 12 lines · David Miller
From: Grant Likely <[email protected]>
Date: Fri, 8 Oct 2010 11:36:50 -0600
Quoted 5 lines
Weird. Yeah, no other platforms expect to get a phandle type definition from the kernel headers. The only thing driving this conversation is from arch/sparc/include/asm/Kbuild:
header-y += openprom.h
It was probably at one point for the sake of asm/openpromio.h but that header has no dependencies on openprom.h
Quoted 8 lines
The only hit I can find in google code search, for non-kernel code, is the SILO bootloader.
But that tree includes it's own copy of include/asm/openprom.h so the actual kernel copy isn't even used.
I'd say we can stop exporting that header and also therefore not worry about making phandle_t visible to userspace.

Quoted 7 lines

Yay! That simplifies everything, and I believe it also means that the phandle/ihandle definitions can remain where they currently are in linux/of.h
Andres, can you post an updated series that includes removing openprom.h from the header export list?

Sure, I'll just need to rework/retest the build. I've already done that with the openprom.h-removal patch (which was just sent).

Andres Salomon

author

2010-10-08 18:32 UTC #7 lore raw

PATCH 1/1 sparc: stop exporting openprom.h header

In reply to David Miller

It's unknown why openprom.h was being exported; there doesn't seem to be any reason for it currently, and it creates headaches with userspace being able to potentially use the structures in there. So, don't export it anymore.

Signed-off-by Andres Salomon <[email protected]>

---
 arch/sparc/include/asm/Kbuild |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

arch/sparc/include/asm/Kbuild +0 -1

index deeb0fb..3c93f08 100644
@@ -7,7 +7,6 @@ header-y += display7seg.h
 header-y += envctrl.h
 header-y += fbio.h
 header-y += jsflash.h
-header-y += openprom.h
 header-y += openpromio.h
 header-y += perfctr.h
 header-y += psrcompat.h

Signature · 1 line

1.5.6.5

David Miller

maintainer Acked-by on v1 1/1

2010-10-08 18:52 UTC #8 lore raw

Re: [PATCH] sparc: stop exporting openprom.h header

From: Andres Salomon <[email protected]>
Date: Fri, 8 Oct 2010 11:34:24 -0700

Quoted 6 lines

It's unknown why openprom.h was being exported; there doesn't seem to be any reason for it currently, and it creates headaches with userspace being able to potentially use the structures in there. So, don't export it anymore.
Signed-off-by Andres Salomon <[email protected]>

Acked-by David S. Miller <[email protected]>

Grant Likely

2010-10-08 19:00 UTC #9 lore raw

Re: [PATCH] sparc: stop exporting openprom.h header

Quoted 11 lines · David Miller

From: Andres Salomon <[email protected]>
Date: Fri, 8 Oct 2010 11:34:24 -0700
Quoted 6 lines
It's unknown why openprom.h was being exported; there doesn't seem to be any reason for it currently, and it creates headaches with userspace being able to potentially use the structures in there. So, don't export it anymore.
Signed-off-by Andres Salomon <[email protected]>
Acked-by David S. Miller <[email protected]>

I suppose it makes sense for me to pick this one up into my tree so it is grouped with the rest of the pdt patches. I'll pick it up once Andres reposts the series.

g.
--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

David Miller

maintainer

2010-10-08 19:01 UTC #10 lore raw

Re: [PATCH] sparc: stop exporting openprom.h header

From: Grant Likely <[email protected]>
Date: Fri, 8 Oct 2010 13:00:25 -0600

Quoted 9 lines · David Miller

Quoted 8 lines
From: Andres Salomon <[email protected]>
Date: Fri, 8 Oct 2010 11:34:24 -0700
Quoted 5 lines
It's unknown why openprom.h was being exported; there doesn't seem to be any reason for it currently, and it creates headaches with userspace being able to potentially use the structures in there. So, don't export it anymore.

Quoted 7 lines

Quoted 3 lines
Quoted 1 line
Signed-off-by Andres Salomon <[email protected]>
Acked-by David S. Miller <[email protected]>
I suppose it makes sense for me to pick this one up into my tree so it is grouped with the rest of the pdt patches. I'll pick it up once Andres reposts the series.

Yep, that makes sense.

Andres Salomon

author

2010-10-08 21:34 UTC #11 lore raw

Re: [PATCH] sparc: stop exporting openprom.h header

In reply to Grant Likely

Quoted 13 lines · On Fri, 8 Oct 2010 13:00:25 -0600 Grant Likely <[email protected]>…

Quoted 11 lines · David Miller
From: Andres Salomon <[email protected]>
Date: Fri, 8 Oct 2010 11:34:24 -0700
Quoted 7 lines
It's unknown why openprom.h was being exported; there doesn't seem to be any reason for it currently, and it creates headaches with userspace being able to potentially use the structures in there. So, don't export it anymore.
Signed-off-by Andres Salomon <[email protected]>

Quoted 7 lines

Quoted 1 line
Acked-by David S. Miller <[email protected]>
I suppose it makes sense for me to pick this one up into my tree so it is grouped with the rest of the pdt patches. I'll pick it up once Andres reposts the series.
g.

Ok, I sent a new version of the phandle stuff (which was easier than expected, and doesn't affect any other patches).

So to summarize, what's pending is:

1- https://patchwork.kernel.org/patch/242041/ (sparc: stop exporting
openprom.h header)
Acked by Dave

2- https://patchwork.kernel.org/patch/242601/ ([v3] sparc: convert
various prom_* functions to use phandle)
Acked by Dave

3- https://patchwork.kernel.org/patch/141011/ (sparc: break out some
PROM device-tree building code out into drivers/of) Acked by Dave

4- https://patchwork.kernel.org/patch/141021/ (sparc: make
drivers/of/pdt.c no longer sparc-only)
Acked by Dave

5- https://patchwork.kernel.org/patch/141031/ (of: no longer call prom_
functions directly; use an ops structure)
Acked by Dave

6- https://patchwork.kernel.org/patch/141041/ (of: add
of_pdt namespace to pdt code)
Acked by Dave

7- https://patchwork.kernel.org/patch/141051/ (of: add package-to-path
support to pdt)

8- https://patchwork.kernel.org/patch/141071/ (x86: OLPC: add OLPC
device-tree support)

The make-of-build-on-x86 stuff is already in your tree.

Grant Likely

2010-10-09 08:51 UTC #12 lore raw

Re: [PATCH] sparc: stop exporting openprom.h header

Quoted 49 lines · Andres Salomon

Quoted 13 lines · On Fri, 8 Oct 2010 13:00:25 -0600 Grant Likely <[email protected]>…
Quoted 11 lines · David Miller
From: Andres Salomon <[email protected]>
Date: Fri, 8 Oct 2010 11:34:24 -0700
Quoted 7 lines
It's unknown why openprom.h was being exported; there doesn't seem to be any reason for it currently, and it creates headaches with userspace being able to potentially use the structures in there. So, don't export it anymore.
Signed-off-by Andres Salomon <[email protected]>
Quoted 7 lines
Quoted 1 line
Acked-by David S. Miller <[email protected]>
I suppose it makes sense for me to pick this one up into my tree so it is grouped with the rest of the pdt patches. I'll pick it up once Andres reposts the series.
g.
Ok, I sent a new version of the phandle stuff (which was easier than expected, and doesn't affect any other patches).
So to summarize, what's pending is:
1- https://patchwork.kernel.org/patch/242041/ (sparc: stop exporting
openprom.h header)
Acked by Dave
2- https://patchwork.kernel.org/patch/242601/ ([v3] sparc: convert
various prom_* functions to use phandle)
Acked by Dave
3- https://patchwork.kernel.org/patch/141011/ (sparc: break out some
PROM device-tree building code out into drivers/of) Acked by Dave
4- https://patchwork.kernel.org/patch/141021/ (sparc: make
drivers/of/pdt.c no longer sparc-only)
Acked by Dave
5- https://patchwork.kernel.org/patch/141031/ (of: no longer call prom_
functions directly; use an ops structure)
Acked by Dave

Quoted 6 lines

6- https://patchwork.kernel.org/patch/141041/ (of: add
of_pdt namespace to pdt code)
Acked by Dave
7- https://patchwork.kernel.org/patch/141051/ (of: add package-to-path
support to pdt)

I've picked up 1-7 and am build testing now.

Quoted 2 lines

8- https://patchwork.kernel.org/patch/141071/ (x86: OLPC: add OLPC
device-tree support)

I'm not happy about the /proc/devicetree stuff in this patch. I would rather see the proc_device_tree_init() call moved to initcall time so that the need for an of_pdt_init_devicetree() hook goes away, but there are a number of gotchas for dynamic tree users that I need to investigate. Anyway, I'll get 1-7 tested and into linux-next while I think about patch 8.

g.
--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Andres Salomon

author

2010-10-09 20:47 UTC #13 lore raw

Re: [PATCH] sparc: stop exporting openprom.h header

Quoted 62 lines · On Sat, 9 Oct 2010 02:51:43 -0600 Grant Likely <[email protected]>…

Quoted 49 lines · Andres Salomon
Quoted 13 lines · On Fri, 8 Oct 2010 13:00:25 -0600 Grant Likely <[email protected]>…
Quoted 11 lines · David Miller
From: Andres Salomon <[email protected]>
Date: Fri, 8 Oct 2010 11:34:24 -0700
Quoted 7 lines
It's unknown why openprom.h was being exported; there doesn't seem to be any reason for it currently, and it creates headaches with userspace being able to potentially use the structures in there. So, don't export it anymore.
Signed-off-by Andres Salomon <[email protected]>
Quoted 7 lines
Quoted 1 line
Acked-by David S. Miller <[email protected]>
I suppose it makes sense for me to pick this one up into my tree so it is grouped with the rest of the pdt patches. I'll pick it up once Andres reposts the series.
g.
Ok, I sent a new version of the phandle stuff (which was easier than expected, and doesn't affect any other patches).
So to summarize, what's pending is:
1- https://patchwork.kernel.org/patch/242041/ (sparc: stop exporting
openprom.h header)
Acked by Dave
2- https://patchwork.kernel.org/patch/242601/ ([v3] sparc: convert
various prom_* functions to use phandle)
Acked by Dave
3- https://patchwork.kernel.org/patch/141011/ (sparc: break out some
PROM device-tree building code out into drivers/of) Acked by Dave
4- https://patchwork.kernel.org/patch/141021/ (sparc: make
drivers/of/pdt.c no longer sparc-only)
Acked by Dave
5- https://patchwork.kernel.org/patch/141031/ (of: no longer call prom_ functions directly; use an ops structure)
Acked by Dave
Quoted 6 lines
6- https://patchwork.kernel.org/patch/141041/ (of: add
of_pdt namespace to pdt code)
Acked by Dave
7- https://patchwork.kernel.org/patch/141051/ (of: add package-to-path support to pdt)
I've picked up 1-7 and am build testing now.
Quoted 2 lines
8- https://patchwork.kernel.org/patch/141071/ (x86: OLPC: add OLPC
device-tree support)

Quoted 7 lines

I'm not happy about the /proc/devicetree stuff in this patch. I would rather see the proc_device_tree_init() call moved to initcall time so that the need for an of_pdt_init_devicetree() hook goes away, but there are a number of gotchas for dynamic tree users that I need to investigate. Anyway, I'll get 1-7 tested and into linux-next while I think about patch 8.

I'm failing to see why it's a problem to have the hook (which could just as easily be a generic proc_root_init_prepare hook called from proc_root_init(), allowing usage by various other subsystems), but okay.

I'm not that familiar w/ the dynamic tree stuff; do you imagine this being an invasive change, or will it be as simple as just deferring the init call?

Grant Likely

2010-10-10 05:13 UTC #14 lore raw

Re: [PATCH] sparc: stop exporting openprom.h header

Quoted 53 lines · Andres Salomon

Quoted 50 lines · On Sat, 9 Oct 2010 02:51:43 -0600 Grant Likely <[email protected]>…
Quoted 49 lines · Andres Salomon
Quoted 13 lines · On Fri, 8 Oct 2010 13:00:25 -0600 Grant Likely <[email protected]>…
Quoted 11 lines · David Miller
From: Andres Salomon <[email protected]>
Date: Fri, 8 Oct 2010 11:34:24 -0700
Quoted 7 lines
It's unknown why openprom.h was being exported; there doesn't seem to be any reason for it currently, and it creates headaches with userspace being able to potentially use the structures in there. So, don't export it anymore.
Signed-off-by Andres Salomon <[email protected]>
Quoted 7 lines
Quoted 1 line
Acked-by David S. Miller <[email protected]>
I suppose it makes sense for me to pick this one up into my tree so it is grouped with the rest of the pdt patches. I'll pick it up once Andres reposts the series.
g.
Ok, I sent a new version of the phandle stuff (which was easier than expected, and doesn't affect any other patches).
So to summarize, what's pending is:
1- https://patchwork.kernel.org/patch/242041/ (sparc: stop exporting
openprom.h header)
Acked by Dave
2- https://patchwork.kernel.org/patch/242601/ ([v3] sparc: convert
various prom_* functions to use phandle)
Acked by Dave
3- https://patchwork.kernel.org/patch/141011/ (sparc: break out some
PROM device-tree building code out into drivers/of) Acked by Dave
4- https://patchwork.kernel.org/patch/141021/ (sparc: make
drivers/of/pdt.c no longer sparc-only)
Acked by Dave
5- https://patchwork.kernel.org/patch/141031/ (of: no longer call prom_ functions directly; use an ops structure)
Acked by Dave

Quoted 8 lines

Quoted 8 lines
Quoted 6 lines
6- https://patchwork.kernel.org/patch/141041/ (of: add
of_pdt namespace to pdt code)
Acked by Dave
7- https://patchwork.kernel.org/patch/141051/ (of: add package-to-path support to pdt)
I've picked up 1-7 and am build testing now.

Hmmm, series fails to build on sparc32, and doesn't appear to be fully bisectable. Patches 1-3 compile file. Adding patch 4 gives the following build error. Missing include perhaps?

/home/grant/hacking/linux-2.6/drivers/of/pdt.c: In function 'build_one_prop':
/home/grant/hacking/linux-2.6/drivers/of/pdt.c:80: error: implicit declaration of function 'prom_firstprop' /home/grant/hacking/linux-2.6/drivers/of/pdt.c:80: warning: assignment makes pointer from integer without a cast /home/grant/hacking/linux-2.6/drivers/of/pdt.c:82: error: implicit declaration of function 'prom_nextprop' /home/grant/hacking/linux-2.6/drivers/of/pdt.c:82: warning: assignment makes pointer from integer without a cast /home/grant/hacking/linux-2.6/drivers/of/pdt.c:92: error: implicit declaration of function 'prom_getproplen' /home/grant/hacking/linux-2.6/drivers/of/pdt.c:99: error: implicit declaration of function 'prom_getproperty' /home/grant/hacking/linux-2.6/drivers/of/pdt.c: In function 'prom_build_tree':
/home/grant/hacking/linux-2.6/drivers/of/pdt.c:213: error: implicit declaration of function 'prom_getchild' /home/grant/hacking/linux-2.6/drivers/of/pdt.c:218: error: implicit declaration of function 'prom_getsibling' distcc[16086] ERROR: compile /home/grant/hacking/linux-2.6/drivers/of/pdt.c on localhost failed make[3]: *** [drivers/of/pdt.o] Error 1
make[2]: *** [drivers/of] Error 2
make[2]: *** Waiting for unfinished jobs....

And after applying patch 5, I get this instead:

cc1: warnings being treated as errors
/home/grant/hacking/linux-2.6/arch/sparc/kernel/prom_common.c: In function 'prom_common_nextprop':
/home/grant/hacking/linux-2.6/arch/sparc/kernel/prom_common.c:144: error: passing argument 2 of 'prom_nextprop' discards qualifiers from pointer target type /home/grant/hacking/linux-2.6/arch/sparc/include/asm/oplib_32.h:227: note: expected 'char *' but argument is of type 'const char *' make[2]: *** [arch/sparc/kernel/prom_common.o] Error 1
make[2]: *** Waiting for unfinished jobs....

And applying the subsequent patches fails with the same error. Sparc64 builds fine with the entire stack applied, but I haven't bisected and I suspect that patch 4 will still fail there.

So, I'll leave patches 1-3 in my tree, and drop 4-7 until you get it sorted out.

Quoted 11 lines

Quoted 10 lines
Quoted 2 lines
8- https://patchwork.kernel.org/patch/141071/ (x86: OLPC: add OLPC
device-tree support)
I'm not happy about the /proc/devicetree stuff in this patch. I would rather see the proc_device_tree_init() call moved to initcall time so that the need for an of_pdt_init_devicetree() hook goes away, but there are a number of gotchas for dynamic tree users that I need to investigate. Anyway, I'll get 1-7 tested and into linux-next while I think about patch 8.

Quoted 4 lines

I'm failing to see why it's a problem to have the hook (which could just as easily be a generic proc_root_init_prepare hook called from proc_root_init(), allowing usage by various other subsystems), but okay.

I'm not a fan of global hooks and I personally feel that having to resort to them is usually failure in architecture. That said, I'm beginning to worry that the alternative will require a non-trivial refactoring of the proc devicetree code.

Quoted 3 lines

I'm not that familiar w/ the dynamic tree stuff; do you imagine this being an invasive change, or will it be as simple as just deferring the init call?

I had hoped that it would be as simple as deferring the init call, but I started pulling at that thread, and I suspect the proc devicetree has some rather important ordering issues that I don't fully appreciate yet. I also suspect that some of the logic to keep the live tree and the proc tree synchronized is scattered outside of the core OF code. Blech. I'm still investigating, but please include patch 8 when you repost to resolve the build issue.

Andres Salomon

author

2010-10-10 07:17 UTC #15 lore raw

Re: [PATCH] sparc: stop exporting openprom.h header

Quoted 4 lines · On Sat, 9 Oct 2010 23:13:24 -0600 Grant Likely <[email protected]>…

Quoted 3 lines · Andres Salomon
On Sat, 9 Oct 2010 02:51:43 -0600
Grant Likely <[email protected]> wrote:

[...]

Quoted 2 lines

Hmmm, series fails to build on sparc32, and doesn't appear to be fully bisectable. Patches 1-3 compile file. Adding patch 4 gives

Thanks for testing!

Quoted 28 lines

the following build error. Missing include perhaps?
/home/grant/hacking/linux-2.6/drivers/of/pdt.c: In function 'build_one_prop': /home/grant/hacking/linux-2.6/drivers/of/pdt.c:80:
error: implicit declaration of function
'prom_firstprop' /home/grant/hacking/linux-2.6/drivers/of/pdt.c:80:
warning: assignment makes pointer from integer without a cast /home/grant/hacking/linux-2.6/drivers/of/pdt.c:82: error:
implicit declaration of function
'prom_nextprop' /home/grant/hacking/linux-2.6/drivers/of/pdt.c:82:
warning: assignment makes pointer from integer without a cast /home/grant/hacking/linux-2.6/drivers/of/pdt.c:92: error:
implicit declaration of function
'prom_getproplen' /home/grant/hacking/linux-2.6/drivers/of/pdt.c:99:
error: implicit declaration of function
'prom_getproperty' /home/grant/hacking/linux-2.6/drivers/of/pdt.c: In function
'prom_build_tree': /home/grant/hacking/linux-2.6/drivers/of/pdt.c:213:
error: implicit declaration of function
'prom_getchild' /home/grant/hacking/linux-2.6/drivers/of/pdt.c:218:
error: implicit declaration of function 'prom_getsibling' distcc[16086] ERROR:
compile /home/grant/hacking/linux-2.6/drivers/of/pdt.c on localhost failed make[3]: *** [drivers/of/pdt.o] Error 1 make[2]: *** [drivers/of] Error 2 make[2]: *** Waiting for unfinished jobs....
And after applying patch 5, I get this instead:

Quoted 11 lines

cc1: warnings being treated as errors /home/grant/hacking/linux-2.6/arch/sparc/kernel/prom_common.c: In
function
'prom_common_nextprop': /home/grant/hacking/linux-2.6/arch/sparc/kernel/prom_common.c:144:
error: passing argument 2 of 'prom_nextprop' discards qualifiers from pointer target
type /home/grant/hacking/linux-2.6/arch/sparc/include/asm/oplib_32.h:227:
note: expected 'char *' but argument is of type 'const char *'
make[2]: *** [arch/sparc/kernel/prom_common.o] Error 1 make[2]: ***
Waiting for unfinished jobs....

D'oh:

arch/sparc/include/asm/oplib_32.h:extern char *prom_nextprop(phandle node, char *prev_property, char *buffer); arch/sparc/include/asm/oplib_64.h:extern char *prom_nextprop(phandle node, const char *prev_property, char *buf);

Quoted 6 lines

And applying the subsequent patches fails with the same error. Sparc64 builds fine with the entire stack applied, but I haven't bisected and I suspect that patch 4 will still fail there.
So, I'll leave patches 1-3 in my tree, and drop 4-7 until you get it sorted out.

I'll go ahead and resend 4-8 once I fix 'em up; the nextprop fix require modifications to 5 and 8, and I haven't fixed the problem w/ 4 yet.