diff mbox series

media: i2c: ov7740: add media-controller dependency

Message ID 20180108125322.3993808-1-arnd@arndb.de
State New
Headers show
Series media: i2c: ov7740: add media-controller dependency | expand

Commit Message

Arnd Bergmann Jan. 8, 2018, 12:52 p.m. UTC 
Without CONFIG_MEDIA_CONTROLLER, the new driver fails to build:

drivers/perf/arm_dsu_pmu.c: In function 'dsu_pmu_probe_pmu':
drivers/perf/arm_dsu_pmu.c:661:2: error: implicit declaration of function 'bitmap_from_u32array'; did you mean 'bitmap_from_arr32'? [-Werror=implicit-function-declaration]

This adds a dependency similar to what we have for other drivers
like this.

Fixes: 39c5c4471b8d ("media: i2c: Add the ov7740 image sensor driver")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>

---
 drivers/media/i2c/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

-- 
2.9.0

Comments

Yang, Wenyou Jan. 9, 2018, 8:59 a.m. UTC | #1 
Hello Arnd,


On 2018/1/8 20:52, Arnd Bergmann wrote:
> Without CONFIG_MEDIA_CONTROLLER, the new driver fails to build:

>

> drivers/perf/arm_dsu_pmu.c: In function 'dsu_pmu_probe_pmu':

> drivers/perf/arm_dsu_pmu.c:661:2: error: implicit declaration of function 'bitmap_from_u32array'; did you mean 'bitmap_from_arr32'? [-Werror=implicit-function-declaration]

>

> This adds a dependency similar to what we have for other drivers

> like this.

>

> Fixes: 39c5c4471b8d ("media: i2c: Add the ov7740 image sensor driver")

> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

> ---

Indeed.
Thank you for your fix.

Acked-by: Wenyou Yang <wenyou.yang@microchip.com>


>   drivers/media/i2c/Kconfig | 2 +-

>   1 file changed, 1 insertion(+), 1 deletion(-)

>

> diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig

> index 9f18cd296841..03cf3a1a1e06 100644

> --- a/drivers/media/i2c/Kconfig

> +++ b/drivers/media/i2c/Kconfig

> @@ -667,7 +667,7 @@ config VIDEO_OV7670

>   

>   config VIDEO_OV7740

>   	tristate "OmniVision OV7740 sensor support"

> -	depends on I2C && VIDEO_V4L2

> +	depends on I2C && VIDEO_V4L2 && MEDIA_CONTROLLER

>   	depends on MEDIA_CAMERA_SUPPORT

>   	---help---

>   	  This is a Video4Linux2 sensor-level driver for the OmniVision


Best Regards,
Wenyou Yang
Sakari Ailus Jan. 9, 2018, 10:13 a.m. UTC | #2 
Hi Arnd,

On Mon, Jan 08, 2018 at 01:52:28PM +0100, Arnd Bergmann wrote:
> Without CONFIG_MEDIA_CONTROLLER, the new driver fails to build:

> 

> drivers/perf/arm_dsu_pmu.c: In function 'dsu_pmu_probe_pmu':

> drivers/perf/arm_dsu_pmu.c:661:2: error: implicit declaration of function 'bitmap_from_u32array'; did you mean 'bitmap_from_arr32'? [-Werror=implicit-function-declaration]


There seems to be a build error with this driver if Media controller is
disabled, but this is not the error message nor adding a dependency to
Media controller is a sound fix for it.

drivers/media/i2c/ov7740.c: In function ‘ov7740_probe’:
drivers/media/i2c/ov7740.c:1139:38: error: ‘struct v4l2_subdev’ has no member named ‘entity’
  media_entity_cleanup(&ov7740->subdev.entity);

I think it'd be worth adding nop variants for functions that are commonly
used by drivers that can be built with or without the Media controller.

I'll send a patch.

> 

> This adds a dependency similar to what we have for other drivers

> like this.

> 

> Fixes: 39c5c4471b8d ("media: i2c: Add the ov7740 image sensor driver")

> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

> ---

>  drivers/media/i2c/Kconfig | 2 +-

>  1 file changed, 1 insertion(+), 1 deletion(-)

> 

> diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig

> index 9f18cd296841..03cf3a1a1e06 100644

> --- a/drivers/media/i2c/Kconfig

> +++ b/drivers/media/i2c/Kconfig

> @@ -667,7 +667,7 @@ config VIDEO_OV7670

>  

>  config VIDEO_OV7740

>  	tristate "OmniVision OV7740 sensor support"

> -	depends on I2C && VIDEO_V4L2

> +	depends on I2C && VIDEO_V4L2 && MEDIA_CONTROLLER

>  	depends on MEDIA_CAMERA_SUPPORT

>  	---help---

>  	  This is a Video4Linux2 sensor-level driver for the OmniVision


-- 
Kind regards,

Sakari Ailus
sakari.ailus@linux.intel.com
Arnd Bergmann Jan. 9, 2018, 10:36 a.m. UTC | #3 
On Tue, Jan 9, 2018 at 11:13 AM, Sakari Ailus
<sakari.ailus@linux.intel.com> wrote:
> Hi Arnd,

>

> On Mon, Jan 08, 2018 at 01:52:28PM +0100, Arnd Bergmann wrote:

>> Without CONFIG_MEDIA_CONTROLLER, the new driver fails to build:

>>

>> drivers/perf/arm_dsu_pmu.c: In function 'dsu_pmu_probe_pmu':

>> drivers/perf/arm_dsu_pmu.c:661:2: error: implicit declaration of function 'bitmap_from_u32array'; did you mean 'bitmap_from_arr32'? [-Werror=implicit-function-declaration]

>

> There seems to be a build error with this driver if Media controller is

> disabled, but this is not the error message


Oops, bad copy-paste, sorry.

> nor adding a dependency to

> Media controller is a sound fix for it.


> drivers/media/i2c/ov7740.c: In function ‘ov7740_probe’:

> drivers/media/i2c/ov7740.c:1139:38: error: ‘struct v4l2_subdev’ has no member named ‘entity’

>   media_entity_cleanup(&ov7740->subdev.entity);

>

> I think it'd be worth adding nop variants for functions that are commonly

> used by drivers that can be built with or without the Media controller.

>

> I'll send a patch.


Fair enough.

In this case, the problem is not the lack of the wrapper, as we already
have a

static inline void media_entity_cleanup(struct media_entity *entity) {};

helper, but the fact that struct v4l2_subdev contains a struct media_entity
as an optional member that is disabled here.  Since the argument
to media_entity_cleanup is generally this member, we could have
a wrapper around it that takes a v4l2_subdev pointer, like

static inline void media_subdev_entity_cleanup(struct v4l2_subdev* sd)
{
#ifdef CONFIG_MEDIA_CONTROLLER
      media_entity_cleanup(sd->entity);
#endif
}

which seems nicer than making the relatively large media_entity structure
visible unconditionally, or hiding its members instead.

You may also want to revisit the other drivers that have dependencies
on MEDIA_CONTROLLER then:

drivers/media/Kconfig:  depends on MEDIA_CONTROLLER && DVB_CORE
drivers/media/Kconfig:  depends on VIDEO_DEV && MEDIA_CONTROLLER
drivers/media/i2c/Kconfig:      depends on I2C && VIDEO_V4L2 && MEDIA_CONTROLLER
drivers/media/i2c/Kconfig:      depends on I2C && VIDEO_V4L2 && MEDIA_CONTROLLER
drivers/media/i2c/Kconfig:      depends on MEDIA_CONTROLLER
drivers/media/i2c/Kconfig:      depends on I2C && VIDEO_V4L2 && MEDIA_CONTROLLER
drivers/media/i2c/Kconfig:      depends on I2C && VIDEO_V4L2 && MEDIA_CONTROLLER
drivers/media/i2c/Kconfig:      depends on I2C && VIDEO_V4L2 && MEDIA_CONTROLLER
drivers/media/i2c/Kconfig:      depends on I2C && VIDEO_V4L2 && MEDIA_CONTROLLER
drivers/media/pci/intel/ipu3/Kconfig:   depends on MEDIA_CONTROLLER
drivers/media/platform/Kconfig: depends on VIDEO_V4L2 && OF &&
VIDEO_V4L2_SUBDEV_API && MEDIA_CONTROLLER
drivers/media/platform/rcar-vin/Kconfig:        depends on VIDEO_V4L2
&& VIDEO_V4L2_SUBDEV_API && OF && HAS_DMA && MEDIA_CONTROLLER
drivers/staging/media/atomisp/Kconfig:  depends on X86 && EFI &&
MEDIA_CONTROLLER && PCI && ACPI
drivers/staging/media/imx/Kconfig:      depends on MEDIA_CONTROLLER &&
VIDEO_V4L2 && ARCH_MXC && IMX_IPUV3_CORE

When you come up with a patch, I can throw it in my randconfig build
test machine
to see if that causes any unexpected build failures.

Thanks,

       Arnd
diff mbox series

Patch

diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index 9f18cd296841..03cf3a1a1e06 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -667,7 +667,7 @@  config VIDEO_OV7670
 
 config VIDEO_OV7740
 	tristate "OmniVision OV7740 sensor support"
-	depends on I2C && VIDEO_V4L2
+	depends on I2C && VIDEO_V4L2 && MEDIA_CONTROLLER
 	depends on MEDIA_CAMERA_SUPPORT
 	---help---
 	  This is a Video4Linux2 sensor-level driver for the OmniVision