diff mbox

v4l: omap3isp: Enable driver compilation on ARM with COMPILE_TEST

Message ID 20180407114008.6707-1-laurent.pinchart@ideasonboard.com (mailing list archive)
State New, archived
Headers show

Commit Message

Laurent Pinchart April 7, 2018, 11:40 a.m. UTC
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.

Comments

Mauro Carvalho Chehab April 7, 2018, 1:16 p.m. UTC | #1
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
Laurent Pinchart April 7, 2018, 1:39 p.m. UTC | #2
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
Mauro Carvalho Chehab April 9, 2018, 9:35 a.m. UTC | #3
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
diff mbox

Patch

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