Hi Guennadi,
Thanks for the patch.
On Thursday 29 September 2011 18:18:56 Guennadi Liakhovetski wrote:
> On Media Controller enabled systems this patch allows the user to
> communicate with the driver directly over /dev/v4l-subdev* device nodes
> using VIDIOC_SUBDEV_* ioctl()s.
>
> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@xxxxxx>
[snip]
> diff --git a/drivers/media/video/mt9t112.c b/drivers/media/video/mt9t112.c
> index 32114a3..bb95ad1 100644
> --- a/drivers/media/video/mt9t112.c
> +++ b/drivers/media/video/mt9t112.c
[snip]
> @@ -739,8 +741,7 @@ static int mt9t112_init_camera(const struct i2c_client
> *client) static int mt9t112_g_chip_ident(struct v4l2_subdev *sd,
> struct v4l2_dbg_chip_ident *id)
> {
> - struct i2c_client *client = v4l2_get_subdevdata(sd);
> - struct mt9t112_priv *priv = to_mt9t112(client);
> + struct mt9t112_priv *priv = container_of(sd, struct mt9t112_priv,
> subdev);
What about modifying to_mt9t112() to take a subdev pointer, or possibly
creating a sd_to_mt9t112() ?
> id->ident = priv->model;
> id->revision = 0;
[snip]
> @@ -1018,14 +1015,67 @@ static struct v4l2_subdev_video_ops
[snip]
> +static int mt9t112_set_fmt(struct v4l2_subdev *sd, struct v4l2_subdev_fh
> *fh, + struct v4l2_subdev_format *sd_fmt)
> +{
> + struct v4l2_mbus_framefmt *mf;
> +
> + if (sd_fmt->which == V4L2_SUBDEV_FORMAT_ACTIVE)
> + return mt9t112_s_fmt(sd, &sd_fmt->format);
> +
> + mf = v4l2_subdev_get_try_format(fh, sd_fmt->pad);
> + *mf = sd_fmt->format;
> + return mt9t112_try_fmt(sd, mf);
I think the code would be clea[nr]er if you split mt9t112_s_fmt() into try and
set, and called try unconditionally in mt9t112_set_fmt().
> +}
> +
> +struct v4l2_subdev_pad_ops mt9t112_subdev_pad_ops = {
> + .enum_mbus_code = mt9t112_enum_mbus_code,
> + .get_fmt = mt9t112_get_fmt,
> + .set_fmt = mt9t112_set_fmt,
Having bot mt9t112_[gs]_fmt and mt9t112_[gs]et_fmt looks confusing to me. What
about renaming the later mt9t112_[gs]et_pad_fmt ?
> +};
> +
> static struct v4l2_subdev_ops mt9t112_subdev_ops = {
> .core = &mt9t112_subdev_core_ops,
> .video = &mt9t112_subdev_video_ops,
> + .pad = &mt9t112_subdev_pad_ops,
> };
>
> +static int mt9t112_open(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh)
> +{
> + struct v4l2_mbus_framefmt *mf = v4l2_subdev_get_try_format(fh, 0);
> + return mf ? mt9t112_try_fmt(sd, mf) : 0;
Can mf be NULL ?
> +}
--
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
