On Tue, 11 May 2010, Kuninori Morimoto wrote:

> 
> 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;

Exactly, sorry for being unclear.

> > >  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 ?

My expectation is, that you don't need to modify anything, just 
soc_camera_platform_try_fmt() for all three methods: .try_mbus_fmt, 
.g_mbus_fmt and .s_mbus_fmt. Please, verify, whether or not I am right.

> 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);

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to