[RFC PATCH] drm.h: Fix DRM compilation with bare-metal toolchain.

[RFC PATCH] drm.h: Fix DRM compilation with bare-metal toolchain.

[Nishanth Menon]

From: Paul Sokolovsky <paul.sokolovsky@linaro.org>

An ifdef in drm.h expects to be compiled with full-fledged Linux
toolchain, but it's common to compile kernel with just bare-metal
toolchain which doesn't define __linux__. So, also add __KERNEL__
check.

[nm@ti.com: port forward to 3.9-rc6 and post to dri devel for feedback as RFC]
Signed-off-by: Paul Sokolovsky <paul.sokolovsky@linaro.org>
---
Paul, Dri devel list,
I picked up this patch from linaro tree:
https://git.linaro.org/gitweb?p=people/asac/android/kernel/lt-ti.git;a=patch;h=719fbc876740cf75e82dd082ae5a00dfcf6fff7a
Discussion thread: http://lists.linaro.org/pipermail/linaro-dev/2011-June/thread.html#4874
Seems to me as a valid fix even for upstream perhaps?
Regards,
Nishanth Menon

 include/uapi/drm/drm.h |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
index 8d1e2bb..73a99e4 100644
--- a/include/uapi/drm/drm.h
+++ b/include/uapi/drm/drm.h
@@ -36,7 +36,7 @@
 #ifndef _DRM_H_
 #define _DRM_H_
 
-#if defined(__linux__)
+#if defined(__KERNEL__) || defined(__linux__)
 
 #include <linux/types.h>
 #include <asm/ioctl.h>
-- 
1.7.9.5

Re: [RFC PATCH] drm.h: Fix DRM compilation with bare-metal toolchain.

[Dave Airlie]

> An ifdef in drm.h expects to be compiled with full-fledged Linux
> toolchain, but it's common to compile kernel with just bare-metal
> toolchain which doesn't define __linux__. So, also add __KERNEL__
> check.
Seems okay, pushed.

Dave.

Re: [RFC PATCH] drm.h: Fix DRM compilation with bare-metal toolchain.

[Paul Sokolovsky]

Hello,

On Fri, 12 Apr 2013 18:28:26 -0500
Nishanth Menon <nm-l0cyMroinI0@public.gmane.org> wrote:

> From: Paul Sokolovsky <paul.sokolovsky-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> 
> An ifdef in drm.h expects to be compiled with full-fledged Linux
> toolchain, but it's common to compile kernel with just bare-metal
> toolchain which doesn't define __linux__. So, also add __KERNEL__
> check.
> 
> [nm-l0cyMroinI0@public.gmane.org: port forward to 3.9-rc6 and post to dri devel for
> feedback as RFC] Signed-off-by: Paul Sokolovsky
> <paul.sokolovsky-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> ---
> Paul, Dri devel list,
> I picked up this patch from linaro tree:
> https://git.linaro.org/gitweb?p=people/asac/android/kernel/lt-ti.git;a=patch;h=719fbc876740cf75e82dd082ae5a00dfcf6fff7a
> Discussion thread:
> http://lists.linaro.org/pipermail/linaro-dev/2011-June/thread.html#4874
> Seems to me as a valid fix even for upstream perhaps? 

Yes, IIRC, per the discussion you quote above, I sent this patch for
review of our (Linaro's) kernel folks to see if it's ok (the patch is
simple, story why it's needed may be not such, though I was positive
it's needed). It might be forgotten somehow, thanks for picking it up!


> Regards, Nishanth Menon
> 
>  include/uapi/drm/drm.h |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
> index 8d1e2bb..73a99e4 100644
> --- a/include/uapi/drm/drm.h
> +++ b/include/uapi/drm/drm.h
> @@ -36,7 +36,7 @@
>  #ifndef _DRM_H_
>  #define _DRM_H_
>  
> -#if defined(__linux__)
> +#if defined(__KERNEL__) || defined(__linux__)
>  
>  #include <linux/types.h>
>  #include <asm/ioctl.h>



-- 
Best Regards,
Paul

Linaro.org | Open source software for ARM SoCs
Follow Linaro: http://www.facebook.com/pages/Linaro
http://twitter.com/#!/linaroorg - http://www.linaro.org/linaro-blog

Re: [RFC PATCH] drm.h: Fix DRM compilation with bare-metal toolchain.

[Arnd Bergmann]

On Tuesday 16 April 2013 12:48:28 Paul Sokolovsky wrote:
> > diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
> > index 8d1e2bb..73a99e4 100644
> > --- a/include/uapi/drm/drm.h
> > +++ b/include/uapi/drm/drm.h
> > @@ -36,7 +36,7 @@
> >  #ifndef _DRM_H_
> >  #define _DRM_H_
> >  
> > -#if defined(__linux__)
> > +#if defined(__KERNEL__) || defined(__linux__)
> >  
> >  #include <linux/types.h>
> >  #include <asm/ioctl.h>

This is still completely bogus, the __KERNEL__ symbol has no significance here.
Either make the compiler define __linux__, or remove this #ifdef completely.

	Arnd

Re: [RFC PATCH] drm.h: Fix DRM compilation with bare-metal toolchain.

[Nishanth Menon]

On 12:50-20130416, Arnd Bergmann wrote:
> On Tuesday 16 April 2013 12:48:28 Paul Sokolovsky wrote:
> > > diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
> > > index 8d1e2bb..73a99e4 100644
> > > --- a/include/uapi/drm/drm.h
> > > +++ b/include/uapi/drm/drm.h
> > > @@ -36,7 +36,7 @@
> > >  #ifndef _DRM_H_
> > >  #define _DRM_H_
> > >  
> > > -#if defined(__linux__)
> > > +#if defined(__KERNEL__) || defined(__linux__)
> > >  
> > >  #include <linux/types.h>
> > >  #include <asm/ioctl.h>
> 
> This is still completely bogus, the __KERNEL__ symbol has no significance here.
> Either make the compiler define __linux__, or remove this #ifdef completely.
> 
Searching the v.39-rc7 tag, and greping for _linux_ a few interesting
list pops up. (pruned):
arch/arc/Makefile:cflags-y	+= -mA7 -fno-common -pipe -fno-builtin -D__linux__
arch/h8300/Makefile:KBUILD_CFLAGS += -D__linux__
arch/hexagon/Makefile:KBUILD_CFLAGS += -ffixed-$(TIR_NAME) -DTHREADINFO_REG=$(TIR_NAME) -D__linux__
arch/score/Makefile:	-D__linux__ -ffunction-sections -ffreestanding
arch/xtensa/Makefile:KBUILD_CFLAGS += -ffreestanding -D__linux__
^^ these architectures seem to bypass the pain entirely by defining
__linux__

arch/mips/include/uapi/asm/sgidefs.h:#ifndef __linux__
drivers/gpu/drm/radeon/radeon_cp.c:#ifdef __linux__
{snip}
drivers/scsi/aic7xxx/aic7770.c:#ifdef __linux__
drivers/scsi/aic7xxx/aic79xx.h:#ifndef __linux__
{snip}
drivers/scsi/aic7xxx/aic79xx_core.c:#ifdef __linux__
{snip}
drivers/scsi/aic7xxx/aic79xx_pci.c:#ifdef __linux__
drivers/scsi/aic7xxx/aic7xxx.h:#ifndef __linux__
drivers/scsi/aic7xxx/aic7xxx.h:#ifndef __linux__
drivers/scsi/aic7xxx/aic7xxx_93cx6.c:#ifdef __linux__
drivers/scsi/aic7xxx/aic7xxx_core.c:#ifdef __linux__
{snip}
drivers/scsi/aic7xxx/aic7xxx_pci.c:#ifdef __linux__
drivers/scsi/aic7xxx/aicasm/aicasm.h:#ifdef __linux__
drivers/scsi/aic7xxx/aicasm/aicasm_gram.y:#ifdef __linux__
drivers/scsi/aic7xxx/aicasm/aicasm_macro_scan.l:#ifdef __linux__
drivers/scsi/aic7xxx/aicasm/aicasm_scan.l:#ifdef __linux__
drivers/scsi/aic7xxx/aicasm/aicasm_symbol.c:#ifdef __linux__
drivers/scsi/aic7xxx/aicasm/aicasm_symbol.h:#ifdef __linux__
drivers/scsi/dpt/osd_defs.h:#if (defined(__linux__))
drivers/staging/ced1401/machine.h:#if (defined(__linux__) || defined(_linux) || defined(__linux)) && !defined(LINUX)
include/acpi/platform/acenv.h:#if defined(_LINUX) || defined(__linux__)
include/linux/coda.h:#if defined(__linux__)
include/uapi/drm/drm.h:#if defined(__linux__)
include/uapi/linux/coda.h:#if defined(__linux__)
include/uapi/linux/fuse.h:#ifdef __linux__

And then we have the following as well..
fs/ext4/ext4.h:#if defined(__KERNEL__) || defined(__linux__)

Trying out a few different prebuilt compilers I had around, I see:
http://pastebin.com/bTVDLTb1

So, is our approach just to use __linux__ for builds? I am trying to
understand rationale behind why #include <linux/types.h> #include <asm/ioctl.h>
would want __linux__ and why __KERNEL__ check is un-wanted.
Ofcourse, I cant comment about the "One of the BSDs" in else options..
and why we'd like to keep it around in kernel :)

-- 
Regards,
Nishanth Menon

Re: [RFC PATCH] drm.h: Fix DRM compilation with bare-metal toolchain.

[Arnd Bergmann]

On Tuesday 16 April 2013, Nishanth Menon wrote:
> On 12:50-20130416, Arnd Bergmann wrote:
> > On Tuesday 16 April 2013 12:48:28 Paul Sokolovsky wrote:
> > > > diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
> > > > index 8d1e2bb..73a99e4 100644
> > > > --- a/include/uapi/drm/drm.h
> > > > +++ b/include/uapi/drm/drm.h
> > > > @@ -36,7 +36,7 @@
> > > >  #ifndef _DRM_H_
> > > >  #define _DRM_H_
> > > >  
> > > > -#if defined(__linux__)
> > > > +#if defined(__KERNEL__) || defined(__linux__)
> > > >  
> > > >  #include <linux/types.h>
> > > >  #include <asm/ioctl.h>
> > 
> > This is still completely bogus, the __KERNEL__ symbol has no significance here.
> > Either make the compiler define __linux__, or remove this #ifdef completely.
> > 
> Searching the v.39-rc7 tag, and greping for _linux_ a few interesting
> list pops up. (pruned):
> arch/arc/Makefile:cflags-y	+= -mA7 -fno-common -pipe -fno-builtin -D__linux__
> arch/h8300/Makefile:KBUILD_CFLAGS += -D__linux__
> arch/hexagon/Makefile:KBUILD_CFLAGS += -ffixed-$(TIR_NAME) -DTHREADINFO_REG=$(TIR_NAME) -D__linux__
> arch/score/Makefile:	-D__linux__ -ffunction-sections -ffreestanding
> arch/xtensa/Makefile:KBUILD_CFLAGS += -ffreestanding -D__linux__
> ^^ these architectures seem to bypass the pain entirely by defining
> __linux__

Right, that seems like a reasonable approach when the compilers are actually known
to be compatible.

> arch/mips/include/uapi/asm/sgidefs.h:#ifndef __linux__

On MIPS, they are not. If you are building a Linux kernel with a gcc that
was targetted at another ABI, the system call interface may change, so they
forbid that here.

> drivers/gpu/drm/radeon/radeon_cp.c:#ifdef __linux__
> {snip}
> drivers/scsi/aic7xxx/aic7770.c:#ifdef __linux__
> drivers/scsi/aic7xxx/aic79xx.h:#ifndef __linux__
> {snip}
> drivers/scsi/aic7xxx/aic79xx_core.c:#ifdef __linux__
> {snip}
> drivers/scsi/aic7xxx/aic79xx_pci.c:#ifdef __linux__
> drivers/scsi/aic7xxx/aic7xxx.h:#ifndef __linux__
> drivers/scsi/aic7xxx/aic7xxx.h:#ifndef __linux__
> drivers/scsi/aic7xxx/aic7xxx_93cx6.c:#ifdef __linux__
> drivers/scsi/aic7xxx/aic7xxx_core.c:#ifdef __linux__
> {snip}
> drivers/scsi/aic7xxx/aic7xxx_pci.c:#ifdef __linux__
> drivers/scsi/aic7xxx/aicasm/aicasm.h:#ifdef __linux__
> drivers/scsi/aic7xxx/aicasm/aicasm_macro_scan.l:#ifdef __linux__
> drivers/scsi/aic7xxx/aicasm/aicasm_scan.l:#ifdef __linux__
> drivers/scsi/aic7xxx/aicasm/aicasm_symbol.c:#ifdef __linux__
> drivers/scsi/aic7xxx/aicasm/aicasm_symbol.h:#ifdef __linux__
> drivers/scsi/dpt/osd_defs.h:#if (defined(__linux__))
> drivers/staging/ced1401/machine.h:#if (defined(__linux__) || defined(_linux) || defined(__linux)) && !defined(LINUX)

These are all drivers that are shared with another OS, or used
to be. In most of them, we can probably just remove the
#else path, since I don't think they are getting synchronized
any more.

> include/acpi/platform/acenv.h:#if defined(_LINUX) || defined(__linux__)

The acpi header files are maintained outside of Linux and are kept
OS-independent.

> include/linux/coda.h:#if defined(__linux__)
> include/uapi/drm/drm.h:#if defined(__linux__)
> include/uapi/linux/coda.h:#if defined(__linux__)
> include/uapi/linux/fuse.h:#ifdef __linux__

In case of coda, we should not need to care any more, that header just
got broken by the uapi-split for other operating systems.

The drm.h and fuse.h header files are in theory still kept
OS-agnostic and are actively maintained.

> And then we have the following as well..
> fs/ext4/ext4.h:#if defined(__KERNEL__) || defined(__linux__)

This seems to have been copied from the ext2 utils. The ext2/ext3
versions of this file don't have support for other operating systems.

> Trying out a few different prebuilt compilers I had around, I see:
> http://pastebin.com/bTVDLTb1
> 
> So, is our approach just to use __linux__ for builds? I am trying to
> understand rationale behind why #include <linux/types.h> #include <asm/ioctl.h>
> would want __linux__ and why __KERNEL__ check is un-wanted.
> Ofcourse, I cant comment about the "One of the BSDs" in else options..
> and why we'd like to keep it around in kernel :)

We might be in a similar situation on ARM that we are in on MIPS. For
instance, there are some compilers that are targetting (old) Android
that have a slightly different ABI, and building a kernel with those
results in a system call ABI that is incompatible with user space built
with a standard compiler. The safest approach is probably to bail
out early if __linux__ is not set, and force anyone that wants to use
a strange compiler to set the macro manually.

	Arnd

Re: [RFC PATCH] drm.h: Fix DRM compilation with bare-metal toolchain.

[Rob Clark]

On Wed, Apr 17, 2013 at 6:43 AM, Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org> wrote:
>> drivers/scsi/aic7xxx/aicasm/aicasm_symbol.c:#ifdef __linux__
>> drivers/scsi/aic7xxx/aicasm/aicasm_symbol.h:#ifdef __linux__
>> drivers/scsi/dpt/osd_defs.h:#if (defined(__linux__))
>> drivers/staging/ced1401/machine.h:#if (defined(__linux__) || defined(_linux) || defined(__linux)) && !defined(LINUX)
>
> These are all drivers that are shared with another OS, or used
> to be. In most of them, we can probably just remove the
> #else path, since I don't think they are getting synchronized
> any more.


note that DRM gets periodically ported to the *BSD's, so possibly the
same applies here..

BR,
-R

Re: [RFC PATCH] drm.h: Fix DRM compilation with bare-metal toolchain.

[Jon Medhurst (Tixy)]

On Tue, 2013-04-16 at 12:50 +0200, Arnd Bergmann wrote:
> On Tuesday 16 April 2013 12:48:28 Paul Sokolovsky wrote:
> > > diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
> > > index 8d1e2bb..73a99e4 100644
> > > --- a/include/uapi/drm/drm.h
> > > +++ b/include/uapi/drm/drm.h
> > > @@ -36,7 +36,7 @@
> > >  #ifndef _DRM_H_
> > >  #define _DRM_H_
> > >  
> > > -#if defined(__linux__)
> > > +#if defined(__KERNEL__) || defined(__linux__)
> > >  
> > >  #include <linux/types.h>
> > >  #include <asm/ioctl.h>
> 
> This is still completely bogus, the __KERNEL__ symbol has no significance here.
> Either make the compiler define __linux__, or remove this #ifdef completely.

I don't have the full background here but thought I would point out that
a similar issue seems to have just cropped up with fuse.h ...
http://lkml.org/lkml/2013/4/15/487

-- 
Tixy