diff mbox

[media] v4l2-pci-skeleton: Only build if PCI is available

Message ID 1409073919-27336-1-git-send-email-broonie@kernel.org
State New
Headers show

Commit Message

Mark Brown Aug. 26, 2014, 5:25 p.m. UTC 
From: Mark Brown <broonie@linaro.org>

Currently arm64 does not support PCI but it does support v4l2. Since the
PCI skeleton driver is built unconditionally as a module with no dependency
on PCI this causes build failures for arm64 allmodconfig. Fix this by
defining a symbol VIDEO_PCI_SKELETON for the skeleton and conditionalising
the build on that.

Signed-off-by: Mark Brown <broonie@linaro.org>
---

The patch adding the Makefile was added to the Documentation tree,
either it should be reverted or something like this added on top.

 Documentation/video4linux/Makefile | 2 +-
 drivers/media/v4l2-core/Kconfig    | 7 +++++++
 2 files changed, 8 insertions(+), 1 deletion(-)

Comments

Randy Dunlap Aug. 26, 2014, 6:58 p.m. UTC | #1 
On 08/26/14 10:25, Mark Brown wrote:
> From: Mark Brown <broonie@linaro.org>
> 
> Currently arm64 does not support PCI but it does support v4l2. Since the
> PCI skeleton driver is built unconditionally as a module with no dependency
> on PCI this causes build failures for arm64 allmodconfig. Fix this by
> defining a symbol VIDEO_PCI_SKELETON for the skeleton and conditionalising
> the build on that.

Looks good.  Applied.  Thanks.

I'm pretty sure that it also needs "depends on VIDEOBUF2_CORE" or
something in that family.  I'll check that and add it.

> Signed-off-by: Mark Brown <broonie@linaro.org>
> ---
> 
> The patch adding the Makefile was added to the Documentation tree,
> either it should be reverted or something like this added on top.
> 
>  Documentation/video4linux/Makefile | 2 +-
>  drivers/media/v4l2-core/Kconfig    | 7 +++++++
>  2 files changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/video4linux/Makefile b/Documentation/video4linux/Makefile
> index d58101e788fc..65a351d75c95 100644
> --- a/Documentation/video4linux/Makefile
> +++ b/Documentation/video4linux/Makefile
> @@ -1 +1 @@
> -obj-m := v4l2-pci-skeleton.o
> +obj-$(CONFIG_VIDEO_PCI_SKELETON) := v4l2-pci-skeleton.o
> diff --git a/drivers/media/v4l2-core/Kconfig b/drivers/media/v4l2-core/Kconfig
> index 9ca0f8d59a14..2b368f711c9e 100644
> --- a/drivers/media/v4l2-core/Kconfig
> +++ b/drivers/media/v4l2-core/Kconfig
> @@ -25,6 +25,13 @@ config VIDEO_FIXED_MINOR_RANGES
>  
>  	  When in doubt, say N.
>  
> +config VIDEO_PCI_SKELETON
> +	tristate "Skeleton PCI V4L2 driver"
> +	depends on PCI && COMPILE_TEST
> +	---help---
> +	  Enable build of the skeleton PCI driver, used as a reference
> +	  when developign new drivers.
> +
>  # Used by drivers that need tuner.ko
>  config VIDEO_TUNER
>  	tristate
>
Randy Dunlap Aug. 26, 2014, 7:20 p.m. UTC | #2 
On 08/26/14 10:25, Mark Brown wrote:
> From: Mark Brown <broonie@linaro.org>
> 
> Currently arm64 does not support PCI but it does support v4l2. Since the
> PCI skeleton driver is built unconditionally as a module with no dependency
> on PCI this causes build failures for arm64 allmodconfig. Fix this by
> defining a symbol VIDEO_PCI_SKELETON for the skeleton and conditionalising
> the build on that.
> 
> Signed-off-by: Mark Brown <broonie@linaro.org>
> ---
> 
> The patch adding the Makefile was added to the Documentation tree,
> either it should be reverted or something like this added on top.
> 
>  Documentation/video4linux/Makefile | 2 +-
>  drivers/media/v4l2-core/Kconfig    | 7 +++++++
>  2 files changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/video4linux/Makefile b/Documentation/video4linux/Makefile
> index d58101e788fc..65a351d75c95 100644
> --- a/Documentation/video4linux/Makefile
> +++ b/Documentation/video4linux/Makefile
> @@ -1 +1 @@
> -obj-m := v4l2-pci-skeleton.o
> +obj-$(CONFIG_VIDEO_PCI_SKELETON) := v4l2-pci-skeleton.o
> diff --git a/drivers/media/v4l2-core/Kconfig b/drivers/media/v4l2-core/Kconfig
> index 9ca0f8d59a14..2b368f711c9e 100644
> --- a/drivers/media/v4l2-core/Kconfig
> +++ b/drivers/media/v4l2-core/Kconfig
> @@ -25,6 +25,13 @@ config VIDEO_FIXED_MINOR_RANGES
>  
>  	  When in doubt, say N.
>  
> +config VIDEO_PCI_SKELETON
> +	tristate "Skeleton PCI V4L2 driver"
> +	depends on PCI && COMPILE_TEST

	               && ??  No, don't require COMPILE_TEST.
		However, PCI || COMPILE_TEST would allow it to build on arm64
		if COMPILE_TEST is enabled, guaranteeing build errors.
		Is that what should happen?  I suppose so...


> +	---help---
> +	  Enable build of the skeleton PCI driver, used as a reference
> +	  when developign new drivers.
> +
>  # Used by drivers that need tuner.ko
>  config VIDEO_TUNER
>  	tristate
>
Mark Brown Aug. 26, 2014, 7:26 p.m. UTC | #3 
On Tue, Aug 26, 2014 at 12:20:54PM -0700, Randy Dunlap wrote:
> On 08/26/14 10:25, Mark Brown wrote:

> > index d58101e788fc..65a351d75c95 100644
> > --- a/Documentation/video4linux/Makefile
> > +++ b/Documentation/video4linux/Makefile
> > @@ -1 +1 @@
> > -obj-m := v4l2-pci-skeleton.o
> > +obj-$(CONFIG_VIDEO_PCI_SKELETON) := v4l2-pci-skeleton.o
> > diff --git a/drivers/media/v4l2-core/Kconfig b/drivers/media/v4l2-core/Kconfig

> > +config VIDEO_PCI_SKELETON
> > +	tristate "Skeleton PCI V4L2 driver"
> > +	depends on PCI && COMPILE_TEST

> 	               && ??  No, don't require COMPILE_TEST.

That's a very deliberate choice.  There's no reason I can see to build
this code other than to check that it builds, it's reference code rather
than something that someone is expected to actually use in their system.  
This seems like a perfect candidate for COMPILE_TEST.

> 		However, PCI || COMPILE_TEST would allow it to build on arm64
> 		if COMPILE_TEST is enabled, guaranteeing build errors.
> 		Is that what should happen?  I suppose so...

No, it's not - if it's going to depend on COMPILE_TEST at all it need to
be a hard dependency.
Randy Dunlap Aug. 26, 2014, 7:59 p.m. UTC | #4 
On 08/26/14 12:26, Mark Brown wrote:
> On Tue, Aug 26, 2014 at 12:20:54PM -0700, Randy Dunlap wrote:
>> On 08/26/14 10:25, Mark Brown wrote:
> 
>>> index d58101e788fc..65a351d75c95 100644
>>> --- a/Documentation/video4linux/Makefile
>>> +++ b/Documentation/video4linux/Makefile
>>> @@ -1 +1 @@
>>> -obj-m := v4l2-pci-skeleton.o
>>> +obj-$(CONFIG_VIDEO_PCI_SKELETON) := v4l2-pci-skeleton.o
>>> diff --git a/drivers/media/v4l2-core/Kconfig b/drivers/media/v4l2-core/Kconfig
> 
>>> +config VIDEO_PCI_SKELETON
>>> +	tristate "Skeleton PCI V4L2 driver"
>>> +	depends on PCI && COMPILE_TEST
> 
>> 	               && ??  No, don't require COMPILE_TEST.
> 
> That's a very deliberate choice.  There's no reason I can see to build
> this code other than to check that it builds, it's reference code rather
> than something that someone is expected to actually use in their system.  
> This seems like a perfect candidate for COMPILE_TEST.
> 
>> 		However, PCI || COMPILE_TEST would allow it to build on arm64
>> 		if COMPILE_TEST is enabled, guaranteeing build errors.
>> 		Is that what should happen?  I suppose so...
> 
> No, it's not - if it's going to depend on COMPILE_TEST at all it need to
> be a hard dependency.

How about just drop COMPILE_TEST?  This code only builds if someone enabled
BUILD_DOCSRC.  That should be enough (along with PCI and some VIDEO kconfig
symbols) to qualify it.
Randy Dunlap Aug. 26, 2014, 8:08 p.m. UTC | #5 
On 08/26/14 12:59, Randy Dunlap wrote:
> On 08/26/14 12:26, Mark Brown wrote:
>> On Tue, Aug 26, 2014 at 12:20:54PM -0700, Randy Dunlap wrote:
>>> On 08/26/14 10:25, Mark Brown wrote:
>>
>>>> index d58101e788fc..65a351d75c95 100644
>>>> --- a/Documentation/video4linux/Makefile
>>>> +++ b/Documentation/video4linux/Makefile
>>>> @@ -1 +1 @@
>>>> -obj-m := v4l2-pci-skeleton.o
>>>> +obj-$(CONFIG_VIDEO_PCI_SKELETON) := v4l2-pci-skeleton.o
>>>> diff --git a/drivers/media/v4l2-core/Kconfig b/drivers/media/v4l2-core/Kconfig
>>
>>>> +config VIDEO_PCI_SKELETON
>>>> +	tristate "Skeleton PCI V4L2 driver"
>>>> +	depends on PCI && COMPILE_TEST
>>
>>> 	               && ??  No, don't require COMPILE_TEST.
>>
>> That's a very deliberate choice.  There's no reason I can see to build
>> this code other than to check that it builds, it's reference code rather
>> than something that someone is expected to actually use in their system.  
>> This seems like a perfect candidate for COMPILE_TEST.
>>
>>> 		However, PCI || COMPILE_TEST would allow it to build on arm64
>>> 		if COMPILE_TEST is enabled, guaranteeing build errors.
>>> 		Is that what should happen?  I suppose so...
>>
>> No, it's not - if it's going to depend on COMPILE_TEST at all it need to
>> be a hard dependency.
> 
> How about just drop COMPILE_TEST?  This code only builds if someone enabled
> BUILD_DOCSRC.  That should be enough (along with PCI and some VIDEO kconfig
> symbols) to qualify it.

I'll add BUILD_DOCSRC to the depends list in the Kconfig file...
Mark Brown Aug. 27, 2014, 7:09 a.m. UTC | #6 
On Tue, Aug 26, 2014 at 01:08:39PM -0700, Randy Dunlap wrote:
> On 08/26/14 12:59, Randy Dunlap wrote:
> > On 08/26/14 12:26, Mark Brown wrote:

> >> No, it's not - if it's going to depend on COMPILE_TEST at all it need to
> >> be a hard dependency.

> > How about just drop COMPILE_TEST?  This code only builds if someone enabled
> > BUILD_DOCSRC.  That should be enough (along with PCI and some VIDEO kconfig
> > symbols) to qualify it.

> I'll add BUILD_DOCSRC to the depends list in the Kconfig file...

OK, that symbol probably does the job - I wasn't aware of it previously.
diff mbox

Patch

diff --git a/Documentation/video4linux/Makefile b/Documentation/video4linux/Makefile
index d58101e788fc..65a351d75c95 100644
--- a/Documentation/video4linux/Makefile
+++ b/Documentation/video4linux/Makefile
@@ -1 +1 @@ 
-obj-m := v4l2-pci-skeleton.o
+obj-$(CONFIG_VIDEO_PCI_SKELETON) := v4l2-pci-skeleton.o
diff --git a/drivers/media/v4l2-core/Kconfig b/drivers/media/v4l2-core/Kconfig
index 9ca0f8d59a14..2b368f711c9e 100644
--- a/drivers/media/v4l2-core/Kconfig
+++ b/drivers/media/v4l2-core/Kconfig
@@ -25,6 +25,13 @@  config VIDEO_FIXED_MINOR_RANGES
 
 	  When in doubt, say N.
 
+config VIDEO_PCI_SKELETON
+	tristate "Skeleton PCI V4L2 driver"
+	depends on PCI && COMPILE_TEST
+	---help---
+	  Enable build of the skeleton PCI driver, used as a reference
+	  when developign new drivers.
+
 # Used by drivers that need tuner.ko
 config VIDEO_TUNER
 	tristate