On Tue, Jan 9, 2018 at 2:58 PM, Sakari Ailus
<sakari.ailus@xxxxxxxxxxxxxxx> wrote:
> Add nop variant of media_entity_cleanup. This allows calling
> media_entity_cleanup whether or not Media controller is enabled,
> simplifying driver code.
>
> Also drop #ifdefs on a few drivers around media_entity_cleanup() and drop
> the extra semicolon from media_entity_cleanup prototype.
>
> Signed-off-by: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx>
> ---
> Hi Arnd,
>
> I thought about doing something similar with media_entity_pads_init which is
> equally commonly used in drivers that support MC/non-MC cases. The trouble
> with that is that the drivers set up the struct first before calling
> media_entity_pads_init, requiring the #ifdefs in any case. So the benefit
> would be questionable at least. So just media_entity_cleanup this time.
Looks good overall, just two thoughts:
> diff --git a/include/media/media-entity.h b/include/media/media-entity.h
> index d7a669058b5e..a732af1dbba0 100644
> --- a/include/media/media-entity.h
> +++ b/include/media/media-entity.h
> @@ -634,7 +634,11 @@ int media_entity_pads_init(struct media_entity *entity, u16 num_pads,
> * This function must be called during the cleanup phase after unregistering
> * the entity (currently, it does nothing).
> */
> -static inline void media_entity_cleanup(struct media_entity *entity) {};
> +#if IS_ENABLED(CONFIG_MEDIA_CONTROLLER)
> +static inline void media_entity_cleanup(struct media_entity *entity) {}
> +#else
> +#define media_entity_cleanup(entity) do { } while (false)
> +#endif
This might cause a harmless warning about an unused variable in case
we have a driver that does:
void f(struct i2c_client *client)
{
struct v4l2_subdev *sd = to_subdev(client);
media_entity_cleanup(sd);
}
None of the drivers you changed would have an unused variable after
your change (otherwise they would already have it before your change),
so it's probably fine.
and second, I'm trying the patch below on top of yours now, will
see how far that gets us ;-)
diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index 03cf3a1a1e06..6421da7cb58a 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -310,14 +310,14 @@ config VIDEO_ML86V7667
config VIDEO_AD5820
tristate "AD5820 lens voice coil support"
- depends on I2C && VIDEO_V4L2 && MEDIA_CONTROLLER
+ depends on I2C && VIDEO_V4L2
---help---
This is a driver for the AD5820 camera lens voice coil.
It is used for example in Nokia N900 (RX-51).
config VIDEO_DW9714
tristate "DW9714 lens voice coil support"
- depends on I2C && VIDEO_V4L2 && MEDIA_CONTROLLER
+ depends on I2C && VIDEO_V4L2
depends on VIDEO_V4L2_SUBDEV_API
---help---
This is a driver for the DW9714 camera lens voice coil.
@@ -636,7 +636,6 @@ config VIDEO_OV5670
tristate "OmniVision OV5670 sensor support"
depends on I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API
depends on MEDIA_CAMERA_SUPPORT
- depends on MEDIA_CONTROLLER
select V4L2_FWNODE
---help---
This is a Video4Linux2 sensor-level driver for the OmniVision
@@ -667,7 +666,7 @@ config VIDEO_OV7670
config VIDEO_OV7740
tristate "OmniVision OV7740 sensor support"
- depends on I2C && VIDEO_V4L2 && MEDIA_CONTROLLER
+ depends on I2C && VIDEO_V4L2
depends on MEDIA_CAMERA_SUPPORT
---help---
This is a Video4Linux2 sensor-level driver for the OmniVision
@@ -815,7 +814,7 @@ comment "Flash devices"
config VIDEO_ADP1653
tristate "ADP1653 flash support"
- depends on I2C && VIDEO_V4L2 && MEDIA_CONTROLLER
+ depends on I2C && VIDEO_V4L2
depends on MEDIA_CAMERA_SUPPORT
---help---
This is a driver for the ADP1653 flash controller. It is used for
@@ -823,7 +822,7 @@ config VIDEO_ADP1653
config VIDEO_LM3560
tristate "LM3560 dual flash driver support"
- depends on I2C && VIDEO_V4L2 && MEDIA_CONTROLLER
+ depends on I2C && VIDEO_V4L2
depends on MEDIA_CAMERA_SUPPORT
select REGMAP_I2C
---help---
@@ -832,7 +831,7 @@ config VIDEO_LM3560
config VIDEO_LM3646
tristate "LM3646 dual flash driver support"
- depends on I2C && VIDEO_V4L2 && MEDIA_CONTROLLER
+ depends on I2C && VIDEO_V4L2
depends on MEDIA_CAMERA_SUPPORT
select REGMAP_I2C
---help---
Arnd
