[PATCH] media: staging: omap4iss: Include asm/cacheflush.h after generic includes

[PATCH] media: staging: omap4iss: Include asm/cacheflush.h after generic includes

[Guenter Roeck]

Including asm/cacheflush.h first results in the following build error when
trying to build sparc32:allmodconfig.

In file included from arch/sparc/include/asm/page.h:10:0,
                 from arch/sparc/include/asm/string_32.h:13,
                 from arch/sparc/include/asm/string.h:7,
                 from include/linux/string.h:20,
                 from include/linux/bitmap.h:9,
                 from include/linux/cpumask.h:12,
                 from arch/sparc/include/asm/smp_32.h:15,
                 from arch/sparc/include/asm/smp.h:7,
                 from arch/sparc/include/asm/switch_to_32.h:5,
                 from arch/sparc/include/asm/switch_to.h:7,
                 from arch/sparc/include/asm/ptrace.h:120,
                 from arch/sparc/include/asm/thread_info_32.h:19,
                 from arch/sparc/include/asm/thread_info.h:7,
                 from include/linux/thread_info.h:38,
                 from arch/sparc/include/asm/current.h:15,
                 from include/linux/mutex.h:14,
                 from include/linux/notifier.h:14,
                 from include/linux/clk.h:17,
                 from drivers/staging/media/omap4iss/iss_video.c:15:
include/linux/highmem.h: In function 'clear_user_highpage':
include/linux/highmem.h:137:31: error:
	passing argument 1 of 'sparc_flush_page_to_ram' from incompatible
	pointer type

Include generic includes files first to fix the problem.

Fixes: fc96d58c10162 ("[media] v4l: omap4iss: Add support for OMAP4 camera interface - Video devices")
Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
Cc: David Miller <davem@davemloft.net>
Cc: Randy Dunlap <rdunlap@infradead.org>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/staging/media/omap4iss/iss_video.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/media/omap4iss/iss_video.c b/drivers/staging/media/omap4iss/iss_video.c
index a3a83424a926..16478fe9e3f8 100644
--- a/drivers/staging/media/omap4iss/iss_video.c
+++ b/drivers/staging/media/omap4iss/iss_video.c
@@ -11,7 +11,6 @@
  * (at your option) any later version.
  */
 
-#include <asm/cacheflush.h>
 #include <linux/clk.h>
 #include <linux/mm.h>
 #include <linux/pagemap.h>
@@ -24,6 +23,8 @@
 #include <media/v4l2-ioctl.h>
 #include <media/v4l2-mc.h>
 
+#include <asm/cacheflush.h>
+
 #include "iss_video.h"
 #include "iss.h"
 
-- 
2.7.4

Re: [PATCH] media: staging: omap4iss: Include asm/cacheflush.h after generic includes

[David Miller]

From: Guenter Roeck <linux@roeck-us.net>
Date: Mon, 23 Jul 2018 14:39:33 -0700

> Including asm/cacheflush.h first results in the following build error when
> trying to build sparc32:allmodconfig.
 ...
> Include generic includes files first to fix the problem.
> 
> Fixes: fc96d58c10162 ("[media] v4l: omap4iss: Add support for OMAP4 camera interface - Video devices")
> Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
> Cc: David Miller <davem@davemloft.net>
> Cc: Randy Dunlap <rdunlap@infradead.org>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>

Acked-by: David S. Miller <davem@davemloft.net>

Re: [PATCH] media: staging: omap4iss: Include asm/cacheflush.h after generic includes

[Mauro Carvalho Chehab]

Em Mon, 23 Jul 2018 14:39:33 -0700
Guenter Roeck <linux@roeck-us.net> escreveu:

> Including asm/cacheflush.h first results in the following build error when
> trying to build sparc32:allmodconfig.
> 
> In file included from arch/sparc/include/asm/page.h:10:0,
>                  from arch/sparc/include/asm/string_32.h:13,
>                  from arch/sparc/include/asm/string.h:7,
>                  from include/linux/string.h:20,
>                  from include/linux/bitmap.h:9,
>                  from include/linux/cpumask.h:12,
>                  from arch/sparc/include/asm/smp_32.h:15,
>                  from arch/sparc/include/asm/smp.h:7,
>                  from arch/sparc/include/asm/switch_to_32.h:5,
>                  from arch/sparc/include/asm/switch_to.h:7,
>                  from arch/sparc/include/asm/ptrace.h:120,
>                  from arch/sparc/include/asm/thread_info_32.h:19,
>                  from arch/sparc/include/asm/thread_info.h:7,
>                  from include/linux/thread_info.h:38,
>                  from arch/sparc/include/asm/current.h:15,
>                  from include/linux/mutex.h:14,
>                  from include/linux/notifier.h:14,
>                  from include/linux/clk.h:17,
>                  from drivers/staging/media/omap4iss/iss_video.c:15:
> include/linux/highmem.h: In function 'clear_user_highpage':
> include/linux/highmem.h:137:31: error:
> 	passing argument 1 of 'sparc_flush_page_to_ram' from incompatible
> 	pointer type
> 
> Include generic includes files first to fix the problem.
> 
> Fixes: fc96d58c10162 ("[media] v4l: omap4iss: Add support for OMAP4 camera interface - Video devices")
> Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
> Cc: David Miller <davem@davemloft.net>
> Cc: Randy Dunlap <rdunlap@infradead.org>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---
>  drivers/staging/media/omap4iss/iss_video.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/media/omap4iss/iss_video.c b/drivers/staging/media/omap4iss/iss_video.c
> index a3a83424a926..16478fe9e3f8 100644
> --- a/drivers/staging/media/omap4iss/iss_video.c
> +++ b/drivers/staging/media/omap4iss/iss_video.c
> @@ -11,7 +11,6 @@
>   * (at your option) any later version.
>   */
>  
> -#include <asm/cacheflush.h>
>  #include <linux/clk.h>
>  #include <linux/mm.h>
>  #include <linux/pagemap.h>
> @@ -24,6 +23,8 @@
>  #include <media/v4l2-ioctl.h>
>  #include <media/v4l2-mc.h>
>  
> +#include <asm/cacheflush.h>
> +
>  #include "iss_video.h"
>  #include "iss.h"

While I won't be against merging it, IMHO a better fix would be to
add the includes asm/cacheflush.h needs inside it, e. g. something
like adding:

	#include <linux/highmem.h>

at the sparc32 variant of it. Btw, ./arch/sparc/include/asm/cacheflush_64.h
seems to include linux/mm.h... So, I guess the right fix would
be something like:

diff --git a/arch/sparc/include/asm/cacheflush_32.h b/arch/sparc/include/asm/cacheflush_32.h
index fb66094a2c30..daeccbdc371a 100644
--- a/arch/sparc/include/asm/cacheflush_32.h
+++ b/arch/sparc/include/asm/cacheflush_32.h
@@ -2,6 +2,8 @@
 #ifndef _SPARC_CACHEFLUSH_H
 #define _SPARC_CACHEFLUSH_H
 
+#include <linux/mm.h>
+
 #include <asm/cachetlb_32.h>
 
 #define flush_cache_all() \



Thanks,
Mauro

Re: [PATCH] media: staging: omap4iss: Include asm/cacheflush.h after generic includes

[Linus Torvalds]

On Mon, Jul 23, 2018 at 8:41 PM Mauro Carvalho Chehab
<mchehab+samsung@kernel.org> wrote:
>
> While I won't be against merging it, IMHO a better fix would be to
> add the includes asm/cacheflush.h needs inside it, e. g. something
> like adding:

No. The <asm/*> includes really should come after <linux/*>.

This is a media driver bug, plain and simple.

We should strive to avoid adding more header includes, it's one of the
major causes of kernel build slowdown.

             Linus

Re: [PATCH] media: staging: omap4iss: Include asm/cacheflush.h after generic includes

[Mauro Carvalho Chehab]

Em Tue, 24 Jul 2018 10:37:56 -0700
Linus Torvalds <torvalds@linux-foundation.org> escreveu:

> On Mon, Jul 23, 2018 at 8:41 PM Mauro Carvalho Chehab
> <mchehab+samsung@kernel.org> wrote:
> >
> > While I won't be against merging it, IMHO a better fix would be to
> > add the includes asm/cacheflush.h needs inside it, e. g. something
> > like adding:  
> 
> No. The <asm/*> includes really should come after <linux/*>.
> 
> This is a media driver bug, plain and simple.

Works for me. Do you intend to apply it directly? Otherwise I'll
add on my tree - and likely send you during the merge window - this is
just Kconfig random COMPILE_TEST build noise, as this driver
is ARM-only (for an OMAP4 specific IP block). So, probably not worth
sending a pull request just due to that.

> We should strive to avoid adding more header includes, it's one of the
> major causes of kernel build slowdown.

Yeah, some time ago mailing lists got flooded with some janitorial's
patchset adding includes (some claiming to be needed on some archs or
under some random Kconfigs)... Compile-test ended by adding more such
stuff (for a good reason, IMHO). I wonder if are there a better way to
handle includes without slowing builds.

Thanks,
Mauro

Re: [PATCH] media: staging: omap4iss: Include asm/cacheflush.h after generic includes

[Linus Torvalds]

On Tue, Jul 24, 2018 at 11:39 AM Mauro Carvalho Chehab
<mchehab+samsung@kernel.org> wrote:
>
> Works for me. Do you intend to apply it directly?

Yes, I took it and it should be pushed out.

> Yeah, some time ago mailing lists got flooded with some janitorial's
> patchset adding includes (some claiming to be needed on some archs or
> under some random Kconfigs)... Compile-test ended by adding more such
> stuff (for a good reason, IMHO). I wonder if are there a better way to
> handle includes without slowing builds.

It's a nightmare to do by hand, with all the different architectures
having slightly different header file requirements.

The scheduler people did it last year (roughly Feb-2017 timeframe),
and it was painful and involved a lot of build testing. Basically some
<linux/sched.h> was split up into <linux/sched/*.h>

I wouldn't encourage people to do that again without some tooling to
actually look at "what symbols might get defined by header file
collection XYZ, what symbols might I need with any config option" kind
of logic.

But it would be lovely if somebody *could* do tooling like that.

Just having something you can run on C files that says "these headers
are completely unused under all possibly config options and
architectures" might be interesting.

Because right now, most people tend to just copy a big set of headers,
whether they need it or not. And they almost never shrink, but new
ones get added as people add uses.

            Linus