Hi Hans,
Thank you for the patch.
On Thursday 04 December 2014 10:54:55 Hans Verkuil wrote:
> From: Hans Verkuil <hans.verkuil@xxxxxxxxx>
>
> If a subdevice pad op is called from a bridge driver, then there is
> no v4l2_subdev_fh struct that can be passed to the subdevice. This
> made it hard to use such subdevs from a bridge driver.
>
> This patch replaces the v4l2_subdev_fh pointer by a v4l2_subdev_pad_config
> pointer in the pad ops. This allows bridge drivers to use the various
> try_ pad ops by creating a v4l2_subdev_pad_config struct and passing it
> along to the pad op.
>
> Signed-off-by: Hans Verkuil <hans.verkuil@xxxxxxxxx>
> ---
> drivers/media/i2c/adv7511.c | 16 +++--
> drivers/media/i2c/adv7604.c | 12 ++--
> drivers/media/i2c/m5mols/m5mols_core.c | 16 ++---
> drivers/media/i2c/mt9m032.c | 34 ++++-----
> drivers/media/i2c/mt9p031.c | 36 +++++-----
> drivers/media/i2c/mt9t001.c | 36 +++++-----
> drivers/media/i2c/mt9v032.c | 36 +++++-----
> drivers/media/i2c/noon010pc30.c | 17 ++---
> drivers/media/i2c/ov9650.c | 16 ++---
> drivers/media/i2c/s5c73m3/s5c73m3-core.c | 51 +++++++-------
> drivers/media/i2c/s5k4ecgx.c | 16 ++---
> drivers/media/i2c/s5k5baf.c | 38 +++++-----
> drivers/media/i2c/s5k6a3.c | 18 ++---
> drivers/media/i2c/s5k6aa.c | 34 ++++-----
> drivers/media/i2c/smiapp/smiapp-core.c | 80 +++++++++----------
> drivers/media/i2c/tvp514x.c | 12 ++--
> drivers/media/i2c/tvp7002.c | 14 ++--
> drivers/media/platform/exynos4-is/fimc-capture.c | 22 +++---
> drivers/media/platform/exynos4-is/fimc-isp.c | 28 ++++----
> drivers/media/platform/exynos4-is/fimc-lite.c | 33 ++++-----
> drivers/media/platform/exynos4-is/mipi-csis.c | 16 ++---
> drivers/media/platform/omap3isp/ispccdc.c | 82 +++++++++---------
> drivers/media/platform/omap3isp/ispccp2.c | 44 ++++++------
> drivers/media/platform/omap3isp/ispcsi2.c | 40 +++++------
> drivers/media/platform/omap3isp/isppreview.c | 70 +++++++++---------
> drivers/media/platform/omap3isp/ispresizer.c | 78 ++++++++++---------
> drivers/media/platform/s3c-camif/camif-capture.c | 18 ++---
> drivers/media/platform/vsp1/vsp1_bru.c | 40 +++++------
> drivers/media/platform/vsp1/vsp1_entity.c | 16 ++---
> drivers/media/platform/vsp1/vsp1_entity.h | 4 +-
> drivers/media/platform/vsp1/vsp1_hsit.c | 16 ++---
> drivers/media/platform/vsp1/vsp1_lif.c | 18 ++---
> drivers/media/platform/vsp1/vsp1_lut.c | 18 ++---
> drivers/media/platform/vsp1/vsp1_rwpf.c | 36 +++++-----
> drivers/media/platform/vsp1/vsp1_rwpf.h | 12 ++--
> drivers/media/platform/vsp1/vsp1_sru.c | 26 +++----
> drivers/media/platform/vsp1/vsp1_uds.c | 26 +++----
> drivers/media/v4l2-core/v4l2-subdev.c | 18 ++---
> drivers/staging/media/davinci_vpfe/dm365_ipipe.c | 49 +++++++------
> drivers/staging/media/davinci_vpfe/dm365_ipipeif.c | 47 ++++++-------
> drivers/staging/media/davinci_vpfe/dm365_isif.c | 79 +++++++++----------
> drivers/staging/media/davinci_vpfe/dm365_resizer.c | 59 ++++++++--------
> drivers/staging/media/omap4iss/iss_csi2.c | 48 ++++++-------
> drivers/staging/media/omap4iss/iss_ipipe.c | 50 ++++++-------
> drivers/staging/media/omap4iss/iss_ipipeif.c | 56 +++++++--------
> drivers/staging/media/omap4iss/iss_resizer.c | 50 ++++++-------
> include/media/v4l2-subdev.h | 58 ++++++++-------
> 47 files changed, 825 insertions(+), 814 deletions(-)
I've quickly reviewed the Aptina sensor drivers and the OMAP3 ISP, VSP1 and
OMAP4 ISS drivers. The changes look good to me.
[snip]
> diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
> index 5beeb87..175dc4f 100644
> --- a/include/media/v4l2-subdev.h
> +++ b/include/media/v4l2-subdev.h
[snip]
> @@ -619,37 +633,31 @@ struct v4l2_subdev {
> #define vdev_to_v4l2_subdev(vdev) \
> ((struct v4l2_subdev *)video_get_drvdata(vdev))
>
> +#define to_v4l2_subdev_fh(fh) \
> + container_of(fh, struct v4l2_subdev_fh, vfh)
> +
Is there a reason to move this macro here ?
> /*
> * Used for storing subdev information per file handle
> */
> struct v4l2_subdev_fh {
> struct v4l2_fh vfh;
> -#if defined(CONFIG_VIDEO_V4L2_SUBDEV_API)
> - struct {
> - struct v4l2_mbus_framefmt try_fmt;
> - struct v4l2_rect try_crop;
> - struct v4l2_rect try_compose;
> - } *pad;
> -#endif
> + struct v4l2_subdev_pad_config *pad;
> };
>
> -#define to_v4l2_subdev_fh(fh) \
> - container_of(fh, struct v4l2_subdev_fh, vfh)
> -
> #if defined(CONFIG_VIDEO_V4L2_SUBDEV_API)
I think you can drop the CONFIG_VIDEO_V4L2_SUBDEV_API dependency. That could
be part of a separate patch though.
> #define __V4L2_SUBDEV_MK_GET_TRY(rtype, fun_name, field_name) \
> static inline struct rtype * \
> - v4l2_subdev_get_try_##fun_name(struct v4l2_subdev_fh *fh, \
> - unsigned int pad) \
> + fun_name(struct v4l2_subdev *sd, \
> + struct v4l2_subdev_pad_config *cfg, \
> + unsigned int pad) \
> { \
> - BUG_ON(pad >= vdev_to_v4l2_subdev( \
> - fh->vfh.vdev)->entity.num_pads); \
> - return &fh->pad[pad].field_name; \
> + BUG_ON((pad) >= (sd)->entity.num_pads); \
No need for the extra parentheses here, pad and sd are function arguments, not
macro arguments.
> + return &cfg[pad].field_name; \
> }
>
> -__V4L2_SUBDEV_MK_GET_TRY(v4l2_mbus_framefmt, format, try_fmt)
> -__V4L2_SUBDEV_MK_GET_TRY(v4l2_rect, crop, try_crop)
> -__V4L2_SUBDEV_MK_GET_TRY(v4l2_rect, compose, try_compose)
> +__V4L2_SUBDEV_MK_GET_TRY(v4l2_mbus_framefmt, v4l2_subdev_get_try_format,
> try_fmt)
> +__V4L2_SUBDEV_MK_GET_TRY(v4l2_rect, v4l2_subdev_get_try_crop,
> try_crop)
> +__V4L2_SUBDEV_MK_GET_TRY(v4l2_rect, v4l2_subdev_get_try_compose,
> try_compose)
> #endif
>
> extern const struct v4l2_file_operations v4l2_subdev_fops;
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
