Re: [PATCH 05/16] media: fsl-viu: allow building it with COMPILE_TEST
Hi Mauro, I love your patch! Perhaps something to improve: [auto build test WARNING on linuxtv-media/master] [also build test WARNING on v4.16 next-20180406] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Mauro-Carvalho-Chehab/Make-all-drivers-under-drivers-media-to-build-with-COMPILE_TEST/20180406-164215 base: git://linuxtv.org/media_tree.git master config: m68k-allyesconfig (attached as .config) compiler: m68k-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0 reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree make.cross ARCH=m68k All warnings (new ones prefixed by >>): >> drivers/media//platform/fsl-viu.c:43:0: warning: "out_be32" redefined #define out_be32(v, a) writel(a, v) In file included from arch/m68k/include/asm/io_mm.h:27:0, from arch/m68k/include/asm/io.h:5, from include/linux/io.h:25, from drivers/media//platform/fsl-viu.c:23: arch/m68k/include/asm/raw_io.h:46:0: note: this is the location of the previous definition #define out_be32(addr,l) (void)((*(__force volatile u32 *) (addr)) = (l)) >> drivers/media//platform/fsl-viu.c:44:0: warning: "in_be32" redefined #define in_be32(a) readl(a) In file included from arch/m68k/include/asm/io_mm.h:27:0, from arch/m68k/include/asm/io.h:5, from include/linux/io.h:25, from drivers/media//platform/fsl-viu.c:23: arch/m68k/include/asm/raw_io.h:37:0: note: this is the location of the previous definition #define in_be32(addr) \ vim +/out_be32 +43 drivers/media//platform/fsl-viu.c > 23 #include 24 #include 25 #include 26 #include 27 #include 28 #include 29 #include 30 #include 31 #include 32 #include 33 #include 34 #include 35 36 #define DRV_NAME"fsl_viu" 37 #define VIU_VERSION "0.5.1" 38 39 /* Allow building this driver with COMPILE_TEST */ 40 #ifndef CONFIG_PPC_MPC512x 41 #define NO_IRQ 0 42 > 43 #define out_be32(v, a) writel(a, v) > 44 #define in_be32(a) readl(a) 45 #endif 46 --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: application/gzip
Re: [PATCH 05/16] media: fsl-viu: allow building it with COMPILE_TEST
Em Fri, 6 Apr 2018 16:37:15 +0200 Arnd Bergmann escreveu: > On Fri, Apr 6, 2018 at 4:26 PM, Mauro Carvalho Chehab > wrote: > > Em Fri, 6 Apr 2018 16:16:46 +0200 > > Arnd Bergmann escreveu: > > > >> On Fri, Apr 6, 2018 at 4:15 PM, Mauro Carvalho Chehab > >> wrote: > >> > Em Fri, 6 Apr 2018 11:51:16 +0200 > >> > Arnd Bergmann escreveu: > >> > > >> >> On Fri, Apr 6, 2018 at 11:47 AM, Mauro Carvalho Chehab > >> >> wrote: > >> >> > >> >> > [PATCH] media: fsl-viu: allow building it with COMPILE_TEST > >> >> > > >> >> > There aren't many things that would be needed to allow it > >> >> > to build with compile test. > >> >> > > >> >> > Add the needed bits. > >> >> > > >> >> > Signed-off-by: Mauro Carvalho Chehab > >> >> > >> >> Reviewed-by: Arnd Bergmann > >> > > >> > Actually, in order to avoid warnings with smatch, the COMPILE_TEST > >> > macros should be declared as: > >> > > >> > +#define out_be32(v, a) iowrite32be(a, (void __iomem *)v) > >> > +#define in_be32(a) ioread32be((void __iomem *)a) > >> > >> I would just add the correct annotations, I think they've always been > >> missing. > >> 2 patches coming in a few minutes. > > > > I corrected the annotations too. Now, it gives the same results > > building for both arm and x86. > > > > If you want to double check, the full tree is at: > > > > > > https://git.linuxtv.org/mchehab/experimental.git/log/?h=compile_test > > The __iomem annotations look good, my other patch is still needed to > get a clean build with "make C=1" but doesn't apply cleanly on top of your > version. I assume you'll just fix it up accordingly. Heh, another duplicated patch: https://git.linuxtv.org/mchehab/experimental.git/commit/?h=compile_test&id=687520dc31a88c82c694492423c5d9c503cbdebb That's why it didn't apply cleanly: $ patch -p1 -i /tmp/media\:\ platform\:\ fsl-viu\:\ mark\ local\ functions\ \'static\'.patch --merge patching file drivers/media/platform/fsl-viu.c Hunk #1 already applied at 238. Hunk #2 already applied at 251. Hunk #3 already applied at 262. Hunk #4 already applied at 806. Hunk #5 already applied at 817. Hunk #6 already applied at 1305. Great minds think alike :-) Thanks, Mauro
Re: [PATCH 05/16] media: fsl-viu: allow building it with COMPILE_TEST
On Fri, Apr 6, 2018 at 4:26 PM, Mauro Carvalho Chehab wrote: > Em Fri, 6 Apr 2018 16:16:46 +0200 > Arnd Bergmann escreveu: > >> On Fri, Apr 6, 2018 at 4:15 PM, Mauro Carvalho Chehab >> wrote: >> > Em Fri, 6 Apr 2018 11:51:16 +0200 >> > Arnd Bergmann escreveu: >> > >> >> On Fri, Apr 6, 2018 at 11:47 AM, Mauro Carvalho Chehab >> >> wrote: >> >> >> >> > [PATCH] media: fsl-viu: allow building it with COMPILE_TEST >> >> > >> >> > There aren't many things that would be needed to allow it >> >> > to build with compile test. >> >> > >> >> > Add the needed bits. >> >> > >> >> > Signed-off-by: Mauro Carvalho Chehab >> >> >> >> Reviewed-by: Arnd Bergmann >> > >> > Actually, in order to avoid warnings with smatch, the COMPILE_TEST >> > macros should be declared as: >> > >> > +#define out_be32(v, a) iowrite32be(a, (void __iomem *)v) >> > +#define in_be32(a) ioread32be((void __iomem *)a) >> >> I would just add the correct annotations, I think they've always been >> missing. >> 2 patches coming in a few minutes. > > I corrected the annotations too. Now, it gives the same results > building for both arm and x86. > > If you want to double check, the full tree is at: > > https://git.linuxtv.org/mchehab/experimental.git/log/?h=compile_test The __iomem annotations look good, my other patch is still needed to get a clean build with "make C=1" but doesn't apply cleanly on top of your version. I assume you'll just fix it up accordingly. Arnd
Re: [PATCH 05/16] media: fsl-viu: allow building it with COMPILE_TEST
Em Fri, 6 Apr 2018 16:16:46 +0200 Arnd Bergmann escreveu: > On Fri, Apr 6, 2018 at 4:15 PM, Mauro Carvalho Chehab > wrote: > > Em Fri, 6 Apr 2018 11:51:16 +0200 > > Arnd Bergmann escreveu: > > > >> On Fri, Apr 6, 2018 at 11:47 AM, Mauro Carvalho Chehab > >> wrote: > >> > >> > [PATCH] media: fsl-viu: allow building it with COMPILE_TEST > >> > > >> > There aren't many things that would be needed to allow it > >> > to build with compile test. > >> > > >> > Add the needed bits. > >> > > >> > Signed-off-by: Mauro Carvalho Chehab > >> > >> Reviewed-by: Arnd Bergmann > > > > Actually, in order to avoid warnings with smatch, the COMPILE_TEST > > macros should be declared as: > > > > +#define out_be32(v, a) iowrite32be(a, (void __iomem *)v) > > +#define in_be32(a) ioread32be((void __iomem *)a) > > I would just add the correct annotations, I think they've always been missing. > 2 patches coming in a few minutes. I corrected the annotations too. Now, it gives the same results building for both arm and x86. If you want to double check, the full tree is at: https://git.linuxtv.org/mchehab/experimental.git/log/?h=compile_test > > Arnd Thanks, Mauro
Re: [PATCH 05/16] media: fsl-viu: allow building it with COMPILE_TEST
On Fri, Apr 6, 2018 at 4:15 PM, Mauro Carvalho Chehab wrote: > Em Fri, 6 Apr 2018 11:51:16 +0200 > Arnd Bergmann escreveu: > >> On Fri, Apr 6, 2018 at 11:47 AM, Mauro Carvalho Chehab >> wrote: >> >> > [PATCH] media: fsl-viu: allow building it with COMPILE_TEST >> > >> > There aren't many things that would be needed to allow it >> > to build with compile test. >> > >> > Add the needed bits. >> > >> > Signed-off-by: Mauro Carvalho Chehab >> >> Reviewed-by: Arnd Bergmann > > Actually, in order to avoid warnings with smatch, the COMPILE_TEST > macros should be declared as: > > +#define out_be32(v, a) iowrite32be(a, (void __iomem *)v) > +#define in_be32(a) ioread32be((void __iomem *)a) I would just add the correct annotations, I think they've always been missing. 2 patches coming in a few minutes. Arnd
Re: [PATCH 05/16] media: fsl-viu: allow building it with COMPILE_TEST
Em Fri, 6 Apr 2018 11:51:16 +0200 Arnd Bergmann escreveu: > On Fri, Apr 6, 2018 at 11:47 AM, Mauro Carvalho Chehab > wrote: > > > [PATCH] media: fsl-viu: allow building it with COMPILE_TEST > > > > There aren't many things that would be needed to allow it > > to build with compile test. > > > > Add the needed bits. > > > > Signed-off-by: Mauro Carvalho Chehab > > Reviewed-by: Arnd Bergmann Actually, in order to avoid warnings with smatch, the COMPILE_TEST macros should be declared as: +#define out_be32(v, a) iowrite32be(a, (void __iomem *)v) +#define in_be32(a) ioread32be((void __iomem *)a) Thanks, Mauro [PATCH] media: fsl-viu: allow building it with COMPILE_TEST There aren't many things that would be needed to allow it to build with compile test. Add the needed bits. Signed-off-by: Mauro Carvalho Chehab diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig index 03c9dfeb7781..e6eb1eb776e1 100644 --- a/drivers/media/platform/Kconfig +++ b/drivers/media/platform/Kconfig @@ -42,7 +42,7 @@ config VIDEO_SH_VOU config VIDEO_VIU tristate "Freescale VIU Video Driver" - depends on VIDEO_V4L2 && PPC_MPC512x + depends on VIDEO_V4L2 && (PPC_MPC512x || COMPILE_TEST) select VIDEOBUF_DMA_CONTIG default y ---help--- diff --git a/drivers/media/platform/fsl-viu.c b/drivers/media/platform/fsl-viu.c index 9abe79779659..6fd1c8f66047 100644 --- a/drivers/media/platform/fsl-viu.c +++ b/drivers/media/platform/fsl-viu.c @@ -36,6 +36,12 @@ #define DRV_NAME "fsl_viu" #define VIU_VERSION"0.5.1" +/* Allow building this driver with COMPILE_TEST */ +#ifndef CONFIG_PPC +#define out_be32(v, a) iowrite32be(a, (void __iomem *)v) +#define in_be32(a) ioread32be((void __iomem *)a) +#endif + #define BUFFER_TIMEOUT msecs_to_jiffies(500) /* 0.5 seconds */ #defineVIU_VID_MEM_LIMIT 4 /* Video memory limit, in Mb */ @@ -1407,7 +1413,7 @@ static int viu_of_probe(struct platform_device *op) } viu_irq = irq_of_parse_and_map(op->dev.of_node, 0); - if (viu_irq == NO_IRQ) { + if (!viu_irq) { dev_err(&op->dev, "Error while mapping the irq\n"); return -EINVAL; }
Re: [PATCH 05/16] media: fsl-viu: allow building it with COMPILE_TEST
On Fri, Apr 6, 2018 at 11:47 AM, Mauro Carvalho Chehab wrote: > [PATCH] media: fsl-viu: allow building it with COMPILE_TEST > > There aren't many things that would be needed to allow it > to build with compile test. > > Add the needed bits. > > Signed-off-by: Mauro Carvalho Chehab Reviewed-by: Arnd Bergmann
Re: [PATCH 05/16] media: fsl-viu: allow building it with COMPILE_TEST
Em Thu, 5 Apr 2018 23:35:06 +0200 Arnd Bergmann escreveu: > On Thu, Apr 5, 2018 at 7:54 PM, Mauro Carvalho Chehab > wrote: > > There aren't many things that would be needed to allow it > > to build with compile test. > > > +/* Allow building this driver with COMPILE_TEST */ > > +#ifndef CONFIG_PPC_MPC512x > > +#define NO_IRQ 0 > > The NO_IRQ usage here really needs to die. The portable way to do this > is the simpler > > diff --git a/drivers/media/platform/fsl-viu.c > b/drivers/media/platform/fsl-viu.c > index 200c47c69a75..707bda89b4f7 100644 > --- a/drivers/media/platform/fsl-viu.c > +++ b/drivers/media/platform/fsl-viu.c > @@ -1407,7 +1407,7 @@ static int viu_of_probe(struct platform_device *op) > } > > viu_irq = irq_of_parse_and_map(op->dev.of_node, 0); > - if (viu_irq == NO_IRQ) { > + if (!viu_irq) { > dev_err(&op->dev, "Error while mapping the irq\n"); > return -EINVAL; > } > > > +#define out_be32(v, a) writel(a, v) > > +#define in_be32(a) readl(a) > > This does get it to compile, but looks confusing because it mixes up the > endianess. I'd suggest doing it like > > #ifndef CONFIG_PPC > #define out_be32(v, a) iowrite32be(a, v) > #define in_be32(a) ioread32be(a) > #endif > > Arnd Thanks for the review. Yeah, that looks better. Patch enclosed. Thanks, Mauro [PATCH] media: fsl-viu: allow building it with COMPILE_TEST There aren't many things that would be needed to allow it to build with compile test. Add the needed bits. Signed-off-by: Mauro Carvalho Chehab diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig index 03c9dfeb7781..e6eb1eb776e1 100644 --- a/drivers/media/platform/Kconfig +++ b/drivers/media/platform/Kconfig @@ -42,7 +42,7 @@ config VIDEO_SH_VOU config VIDEO_VIU tristate "Freescale VIU Video Driver" - depends on VIDEO_V4L2 && PPC_MPC512x + depends on VIDEO_V4L2 && (PPC_MPC512x || COMPILE_TEST) select VIDEOBUF_DMA_CONTIG default y ---help--- diff --git a/drivers/media/platform/fsl-viu.c b/drivers/media/platform/fsl-viu.c index 9abe79779659..f54592c431d3 100644 --- a/drivers/media/platform/fsl-viu.c +++ b/drivers/media/platform/fsl-viu.c @@ -36,6 +36,12 @@ #define DRV_NAME "fsl_viu" #define VIU_VERSION"0.5.1" +/* Allow building this driver with COMPILE_TEST */ +#ifndef CONFIG_PPC +#define out_be32(v, a) iowrite32be(a, v) +#define in_be32(a) ioread32be(a) +#endif + #define BUFFER_TIMEOUT msecs_to_jiffies(500) /* 0.5 seconds */ #defineVIU_VID_MEM_LIMIT 4 /* Video memory limit, in Mb */ @@ -1407,7 +1413,7 @@ static int viu_of_probe(struct platform_device *op) } viu_irq = irq_of_parse_and_map(op->dev.of_node, 0); - if (viu_irq == NO_IRQ) { + if (!viu_irq) { dev_err(&op->dev, "Error while mapping the irq\n"); return -EINVAL; }
Re: [PATCH 05/16] media: fsl-viu: allow building it with COMPILE_TEST
On Thu, Apr 5, 2018 at 7:54 PM, Mauro Carvalho Chehab wrote: > There aren't many things that would be needed to allow it > to build with compile test. > +/* Allow building this driver with COMPILE_TEST */ > +#ifndef CONFIG_PPC_MPC512x > +#define NO_IRQ 0 The NO_IRQ usage here really needs to die. The portable way to do this is the simpler diff --git a/drivers/media/platform/fsl-viu.c b/drivers/media/platform/fsl-viu.c index 200c47c69a75..707bda89b4f7 100644 --- a/drivers/media/platform/fsl-viu.c +++ b/drivers/media/platform/fsl-viu.c @@ -1407,7 +1407,7 @@ static int viu_of_probe(struct platform_device *op) } viu_irq = irq_of_parse_and_map(op->dev.of_node, 0); - if (viu_irq == NO_IRQ) { + if (!viu_irq) { dev_err(&op->dev, "Error while mapping the irq\n"); return -EINVAL; } > +#define out_be32(v, a) writel(a, v) > +#define in_be32(a) readl(a) This does get it to compile, but looks confusing because it mixes up the endianess. I'd suggest doing it like #ifndef CONFIG_PPC #define out_be32(v, a) iowrite32be(a, v) #define in_be32(a) ioread32be(a) #endif Arnd