public inbox for linux-media@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] v4l: omap3isp: Enable driver compilation on ARM with COMPILE_TEST
@ 2018-04-07 11:40 Laurent Pinchart
  2018-04-07 13:16 ` Mauro Carvalho Chehab
  2018-04-09  9:35 ` Mauro Carvalho Chehab
  0 siblings, 2 replies; 4+ messages in thread
From: Laurent Pinchart @ 2018-04-07 11:40 UTC (permalink / raw)
  To: linux-media; +Cc: Mauro Carvalho Chehab, Sakari Ailus

The omap3isp driver can't be compiled on non-ARM platforms but has no
compile-time dependency on OMAP. It however requires common clock
framework support, which isn't provided by all ARM platforms.

Drop the OMAP dependency when COMPILE_TEST is set and add ARM and
COMMON_CLK dependencies.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/platform/Kconfig | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

Hi Mauro,

While we continue the discussions on whether the ARM IOMMU functions should be
stubbed in the omap3isp driver itself or not, I propose already merging this
patch that will extend build coverage for the omap3isp driver. Extending
compilation to non-ARM platforms can then be added on top, depending on the
result of the discussion.

You might have noticed the 0-day build bot report reporting that the driver
depends on the common clock framework (build failure on openrisc). The issue
affects ARM as well as not all ARM platforms use the common clock framework.
I've thus also added a dependency on COMMON_CLK. Note that this dependency can
prevent compilation on x86 platforms. If you want to fix that, the
definition of struct clk_hw in include/linux/clk-provider.h will need to be
exposed even when CONFIG_COMMON_CLK isn't selected. I'll let you propose a fix
for that issue to the clock maintainers if you think it should be addressed.

diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
index c7a1cf8a1b01..58aa233d3cf9 100644
--- a/drivers/media/platform/Kconfig
+++ b/drivers/media/platform/Kconfig
@@ -62,7 +62,10 @@ config VIDEO_MUX
 
 config VIDEO_OMAP3
 	tristate "OMAP 3 Camera support"
-	depends on VIDEO_V4L2 && I2C && VIDEO_V4L2_SUBDEV_API && ARCH_OMAP3
+	depends on VIDEO_V4L2 && I2C && VIDEO_V4L2_SUBDEV_API
+	depends on ARCH_OMAP3 || COMPILE_TEST
+	depends on ARM
+	depends on COMMON_CLK
 	depends on HAS_DMA && OF
 	depends on OMAP_IOMMU
 	select ARM_DMA_USE_IOMMU
-- 
Regards,

Laurent Pinchart

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] v4l: omap3isp: Enable driver compilation on ARM with COMPILE_TEST
  2018-04-07 11:40 [PATCH] v4l: omap3isp: Enable driver compilation on ARM with COMPILE_TEST Laurent Pinchart
@ 2018-04-07 13:16 ` Mauro Carvalho Chehab
  2018-04-07 13:39   ` Laurent Pinchart
  2018-04-09  9:35 ` Mauro Carvalho Chehab
  1 sibling, 1 reply; 4+ messages in thread
From: Mauro Carvalho Chehab @ 2018-04-07 13:16 UTC (permalink / raw)
  To: Laurent Pinchart; +Cc: linux-media, Sakari Ailus

Em Sat,  7 Apr 2018 14:40:08 +0300
Laurent Pinchart <laurent.pinchart@ideasonboard.com> escreveu:

> The omap3isp driver can't be compiled on non-ARM platforms but has no
> compile-time dependency on OMAP. It however requires common clock
> framework support, which isn't provided by all ARM platforms.
> 
> Drop the OMAP dependency when COMPILE_TEST is set and add ARM and
> COMMON_CLK dependencies.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  drivers/media/platform/Kconfig | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> Hi Mauro,
> 
> While we continue the discussions on whether the ARM IOMMU functions should be
> stubbed in the omap3isp driver itself or not, I propose already merging this
> patch that will extend build coverage for the omap3isp driver. Extending
> compilation to non-ARM platforms can then be added on top, depending on the
> result of the discussion.
> 
> You might have noticed the 0-day build bot report reporting that the driver
> depends on the common clock framework (build failure on openrisc). The issue
> affects ARM as well as not all ARM platforms use the common clock framework.
> I've thus also added a dependency on COMMON_CLK. Note that this dependency can
> prevent compilation on x86 platforms. If you want to fix that, the
> definition of struct clk_hw in include/linux/clk-provider.h will need to be
> exposed even when CONFIG_COMMON_CLK isn't selected. I'll let you propose a fix
> for that issue to the clock maintainers if you think it should be addressed.

Weird, it built/linked fine on x86 without COMMON_CLK. Perhaps there are
some stubs there that aren't working properly for openrisc arch. I'll
take a look on it.

> 
> diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
> index c7a1cf8a1b01..58aa233d3cf9 100644
> --- a/drivers/media/platform/Kconfig
> +++ b/drivers/media/platform/Kconfig
> @@ -62,7 +62,10 @@ config VIDEO_MUX
>  
>  config VIDEO_OMAP3
>  	tristate "OMAP 3 Camera support"
> -	depends on VIDEO_V4L2 && I2C && VIDEO_V4L2_SUBDEV_API && ARCH_OMAP3
> +	depends on VIDEO_V4L2 && I2C && VIDEO_V4L2_SUBDEV_API
> +	depends on ARCH_OMAP3 || COMPILE_TEST
> +	depends on ARM
> +	depends on COMMON_CLK
>  	depends on HAS_DMA && OF
>  	depends on OMAP_IOMMU
>  	select ARM_DMA_USE_IOMMU



Thanks,
Mauro

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] v4l: omap3isp: Enable driver compilation on ARM with COMPILE_TEST
  2018-04-07 13:16 ` Mauro Carvalho Chehab
@ 2018-04-07 13:39   ` Laurent Pinchart
  0 siblings, 0 replies; 4+ messages in thread
From: Laurent Pinchart @ 2018-04-07 13:39 UTC (permalink / raw)
  To: Mauro Carvalho Chehab; +Cc: linux-media, Sakari Ailus

Hi Mauro,

On Saturday, 7 April 2018 16:16:57 EEST Mauro Carvalho Chehab wrote:
> Em Sat,  7 Apr 2018 14:40:08 +0300 Laurent Pinchart escreveu:
> > The omap3isp driver can't be compiled on non-ARM platforms but has no
> > compile-time dependency on OMAP. It however requires common clock
> > framework support, which isn't provided by all ARM platforms.
> > 
> > Drop the OMAP dependency when COMPILE_TEST is set and add ARM and
> > COMMON_CLK dependencies.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> > 
> >  drivers/media/platform/Kconfig | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> > 
> > Hi Mauro,
> > 
> > While we continue the discussions on whether the ARM IOMMU functions
> > should be stubbed in the omap3isp driver itself or not, I propose already
> > merging this patch that will extend build coverage for the omap3isp
> > driver. Extending compilation to non-ARM platforms can then be added on
> > top, depending on the result of the discussion.
> > 
> > You might have noticed the 0-day build bot report reporting that the
> > driver depends on the common clock framework (build failure on openrisc).
> > The issue affects ARM as well as not all ARM platforms use the common
> > clock framework. I've thus also added a dependency on COMMON_CLK. Note
> > that this dependency can prevent compilation on x86 platforms. If you want
> > to fix that, the definition of struct clk_hw in include/linux/clk-
> > provider.h will need to be exposed even when CONFIG_COMMON_CLK isn't
> > selected. I'll let you propose a fix for that issue to the clock
> > maintainers if you think it should be addressed.
> 
> Weird, it built/linked fine on x86 without COMMON_CLK. Perhaps there are
> some stubs there that aren't working properly for openrisc arch. I'll
> take a look on it.

It's not function stubs this time, but structure definitions. I haven't 
checked why it might build on x86, but please note that some x86 platforms 
enable COMMON_CLK, that might be why you got it to build with an allyesconfig 
or allmodconfig configuration.

Given that this patch extends compile-test coverage from ARCH_OMAP3 to ARM and 
fixes a dependency issue that would affect other architectures as well, could 
you consider taking it in your tree and base your compile-test coverage 
expansion for omap3isp on top of it ?

> > diff --git a/drivers/media/platform/Kconfig
> > b/drivers/media/platform/Kconfig index c7a1cf8a1b01..58aa233d3cf9 100644
> > --- a/drivers/media/platform/Kconfig
> > +++ b/drivers/media/platform/Kconfig
> > @@ -62,7 +62,10 @@ config VIDEO_MUX
> > 
> >  config VIDEO_OMAP3
> >  	tristate "OMAP 3 Camera support"
> > -	depends on VIDEO_V4L2 && I2C && VIDEO_V4L2_SUBDEV_API && ARCH_OMAP3
> > +	depends on VIDEO_V4L2 && I2C && VIDEO_V4L2_SUBDEV_API
> > +	depends on ARCH_OMAP3 || COMPILE_TEST
> > +	depends on ARM
> > +	depends on COMMON_CLK
> >  	depends on HAS_DMA && OF
> >  	depends on OMAP_IOMMU
> >  	select ARM_DMA_USE_IOMMU

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] v4l: omap3isp: Enable driver compilation on ARM with COMPILE_TEST
  2018-04-07 11:40 [PATCH] v4l: omap3isp: Enable driver compilation on ARM with COMPILE_TEST Laurent Pinchart
  2018-04-07 13:16 ` Mauro Carvalho Chehab
@ 2018-04-09  9:35 ` Mauro Carvalho Chehab
  1 sibling, 0 replies; 4+ messages in thread
From: Mauro Carvalho Chehab @ 2018-04-09  9:35 UTC (permalink / raw)
  To: Laurent Pinchart; +Cc: linux-media, Sakari Ailus

Em Sat,  7 Apr 2018 14:40:08 +0300
Laurent Pinchart <laurent.pinchart@ideasonboard.com> escreveu:

> The omap3isp driver can't be compiled on non-ARM platforms but has no
> compile-time dependency on OMAP. It however requires common clock
> framework support, which isn't provided by all ARM platforms.
> 
> Drop the OMAP dependency when COMPILE_TEST is set and add ARM and
> COMMON_CLK dependencies.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  drivers/media/platform/Kconfig | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> Hi Mauro,
> 
> While we continue the discussions on whether the ARM IOMMU functions should be
> stubbed in the omap3isp driver itself or not, I propose already merging this
> patch that will extend build coverage for the omap3isp driver. Extending
> compilation to non-ARM platforms can then be added on top, depending on the
> result of the discussion.

Ok, I'll add it before the patch with the approach proposed by Arnd.

> You might have noticed the 0-day build bot report reporting that the driver
> depends on the common clock framework (build failure on openrisc). The issue
> affects ARM as well as not all ARM platforms use the common clock framework.
> I've thus also added a dependency on COMMON_CLK. Note that this dependency can
> prevent compilation on x86 platforms.

Actually, it doesn't. COMMON_CLK can be selected on x86. It actually
doesn't depend on anything. So, build failures due to that happens
only with randconfigs.

So, it is fine to make OMAP3ISP depending on COMMON_CLK.

Thanks,
Mauro

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2018-04-09  9:35 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-04-07 11:40 [PATCH] v4l: omap3isp: Enable driver compilation on ARM with COMPILE_TEST Laurent Pinchart
2018-04-07 13:16 ` Mauro Carvalho Chehab
2018-04-07 13:39   ` Laurent Pinchart
2018-04-09  9:35 ` Mauro Carvalho Chehab

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox