Dear Guennadi

Thank you for checking patch

> > +static int soc_camera_platform_s_fmt(struct v4l2_subdev *sd,
> > +                                struct v4l2_mbus_framefmt *mf)
> > +{
> > +   return 0;
> 
> This function needs not only return 0, but also fill fmt with the current 
> pixel format.

sorry.
Does this "fill" mean "fill mf->xxxx" ?

mf->code = xxxx;
mf->colorspace = xxx;

> >  static struct v4l2_subdev_video_ops platform_subdev_video_ops = {
> >     .s_stream       = soc_camera_platform_s_stream,
> >     .try_mbus_fmt   = soc_camera_platform_try_fmt,
> >     .enum_mbus_fmt  = soc_camera_platform_enum_fmt,
> > +   .cropcap        = soc_camera_platform_cropcap,
> > +   .g_crop         = soc_camera_platform_g_crop,
> > +   .g_mbus_fmt     = soc_camera_platform_try_fmt,
> > +   .s_mbus_fmt     = soc_camera_platform_s_fmt,
> 
> Wouldn't
> 
> +     .s_mbus_fmt     = soc_camera_platform_try_fmt,
> 
> work here as well?

g_mbus_fmt / try_mbus_fmt are using same argument,
and in this driver, it needs same operation I think.
(same operation mean it fill mf->xxxx)
But should I modify it ?

int (*g_mbus_fmt)(struct v4l2_subdev *sd,  struct v4l2_mbus_framefmt *fmt);
int (*try_mbus_fmt)(struct v4l2_subdev *sd, struct v4l2_mbus_framefmt *fmt);


Best regards
--
Kuninori Morimoto
 
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to