Re: [PATCH] media: omap3isp: Shuffle cacheflush.h and include mm.h
From: Nathan Chancellor <hidden>
Date: 2020-05-27 08:13:50
Also in:
linux-alpha, linux-arch, linux-fsdevel, linux-m68k, linux-mips, linux-mm, linux-riscv, linux-sh, linux-um, linuxppc-dev, lkml, sparclinux
Hi Geert, On Wed, May 27, 2020 at 09:02:51AM +0200, Geert Uytterhoeven wrote:
Hi Nathan, CC Laurent On Wed, May 27, 2020 at 6:37 AM Nathan Chancellor [off-list ref] wrote:quoted
After mm.h was removed from the asm-generic version of cacheflush.h, s390 allyesconfig shows several warnings of the following nature: In file included from ./arch/s390/include/generated/asm/cacheflush.h:1, from drivers/media/platform/omap3isp/isp.c:42: ./include/asm-generic/cacheflush.h:16:42: warning: 'struct mm_struct' declared inside parameter list will not be visible outside of this definition or declaration cacheflush.h does not include mm.h nor does it include any forward declaration of these structures hence the warning. To avoid this, include mm.h explicitly in this file and shuffle cacheflush.h below it. Fixes: 19c0054597a0 ("asm-generic: don't include <linux/mm.h> in cacheflush.h") Signed-off-by: Nathan Chancellor <redacted>Thanks for your patch!quoted
I am aware the fixes tag is kind of irrelevant because that SHA will change in the next linux-next revision and this will probably get folded into the original patch anyways but still. The other solution would be to add forward declarations of these structs to the top of cacheflush.h, I just chose to do what Christoph did in the original patch. I am happy to do that instead if you all feel that is better.That actually looks like a better solution to me, as it would address the problem for all users.quoted
drivers/media/platform/omap3isp/isp.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)diff --git a/drivers/media/platform/omap3isp/isp.c b/drivers/media/platform/omap3isp/isp.c index a4ee6b86663e..54106a768e54 100644 --- a/drivers/media/platform/omap3isp/isp.c +++ b/drivers/media/platform/omap3isp/isp.c@@ -39,8 +39,6 @@ * Troy Laramy <t-laramy@ti.com> */ -#include <asm/cacheflush.h> - #include <linux/clk.h> #include <linux/clkdev.h> #include <linux/delay.h>@@ -49,6 +47,7 @@ #include <linux/i2c.h> #include <linux/interrupt.h> #include <linux/mfd/syscon.h> +#include <linux/mm.h> #include <linux/module.h> #include <linux/omap-iommu.h> #include <linux/platform_device.h>@@ -58,6 +57,8 @@ #include <linux/sched.h> #include <linux/vmalloc.h> +#include <asm/cacheflush.h> + #ifdef CONFIG_ARM_DMA_USE_IOMMU #include <asm/dma-iommu.h> #endifWhy does this file need <asm/cacheflush.h> at all? It doesn't call any of the flush_*() functions, and seems to compile fine without (on arm32). Perhaps it was included at the top intentionally, to override the definitions of copy_{to,from}_user_page()? Fortunately that doesn't seem to be the case, from a quick look at the assembler output. So let's just remove the #include instead?
Sounds good to me. I can send a patch if needed or I suppose Andrew can just make a small fixup patch for it. Let me know what I should do. Cheers, Nathan _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel