| Message ID | 20180108125322.3993808-1-arnd@arndb.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
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
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 --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
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(-)