| Message ID | 20101008004354.13b5b254@debxo |
|---|---|
| State | Not Applicable |
| Delegated to: | David Miller |
| Headers | show |
On Fri, Oct 08, 2010 at 12:43:54AM -0700, Andres Salomon wrote: > > 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 <dilinger@queued.net> 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? g. > --- > include/linux/of.h | 3 --- > include/linux/types.h | 7 +++++++ > 2 files changed, 7 insertions(+), 3 deletions(-) > > diff --git a/include/linux/of.h b/include/linux/of.h > index cad7cf0..db184dc 100644 > --- a/include/linux/of.h > +++ b/include/linux/of.h > @@ -25,9 +25,6 @@ > > #ifdef CONFIG_OF > > -typedef u32 phandle; > -typedef u32 ihandle; > - > struct property { > char *name; > int length; > diff --git a/include/linux/types.h b/include/linux/types.h > index 01a082f..67a034a 100644 > --- a/include/linux/types.h > +++ b/include/linux/types.h > @@ -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 > -- > 1.5.6.5 > -- To unsubscribe from this list: send the line "unsubscribe sparclinux" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Grant Likely <grant.likely@secretlab.ca> Date: Fri, 8 Oct 2010 11:12:10 -0600 > On Fri, Oct 08, 2010 at 12:43:54AM -0700, Andres Salomon wrote: >> >> 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 <dilinger@queued.net> > > 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. -- To unsubscribe from this list: send the line "unsubscribe sparclinux" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Oct 08, 2010 at 10:17:59AM -0700, David Miller wrote: > From: Grant Likely <grant.likely@secretlab.ca> > Date: Fri, 8 Oct 2010 11:12:10 -0600 > > > On Fri, Oct 08, 2010 at 12:43:54AM -0700, Andres Salomon wrote: > >> > >> 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 <dilinger@queued.net> > > > > 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. 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. g. -- To unsubscribe from this list: send the line "unsubscribe sparclinux" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Grant Likely <grant.likely@secretlab.ca> Date: Fri, 8 Oct 2010 11:36:50 -0600 > 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. -- To unsubscribe from this list: send the line "unsubscribe sparclinux" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Oct 08, 2010 at 10:45:57AM -0700, David Miller wrote: > From: Grant Likely <grant.likely@secretlab.ca> > Date: Fri, 8 Oct 2010 11:36:50 -0600 > > > 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. 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. -- To unsubscribe from this list: send the line "unsubscribe sparclinux" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, 8 Oct 2010 12:27:45 -0600 Grant Likely <grant.likely@secretlab.ca> wrote: > On Fri, Oct 08, 2010 at 10:45:57AM -0700, David Miller wrote: > > From: Grant Likely <grant.likely@secretlab.ca> > > Date: Fri, 8 Oct 2010 11:36:50 -0600 > > > > > 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. > > 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). -- To unsubscribe from this list: send the line "unsubscribe sparclinux" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/include/linux/of.h b/include/linux/of.h index cad7cf0..db184dc 100644 --- a/include/linux/of.h +++ b/include/linux/of.h @@ -25,9 +25,6 @@ #ifdef CONFIG_OF -typedef u32 phandle; -typedef u32 ihandle; - struct property { char *name; int length; diff --git a/include/linux/types.h b/include/linux/types.h index 01a082f..67a034a 100644 --- a/include/linux/types.h +++ b/include/linux/types.h @@ -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
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 <dilinger@queued.net> --- include/linux/of.h | 3 --- include/linux/types.h | 7 +++++++ 2 files changed, 7 insertions(+), 3 deletions(-)