Re: [PATCH v2] v4l2-ctrls: Add v4l2_ctrl_[gs]_ctrl_int64()

[Hans Verkuil <hverkuil@xxxxxxxxx>]

On Mon July 23 2012 19:16:03 Laurent Pinchart wrote:
> Hi Hans,
> 
> On Monday 23 July 2012 17:05:35 Hans Verkuil wrote:
> > On Mon July 23 2012 16:02:40 Laurent Pinchart wrote:
> > > These helper functions get and set a 64-bit control's value from within
> > > a driver. They are similar to v4l2_ctrl_[gs]_ctrl() but operate on
> > > 64-bit integer controls instead of 32-bit controls.
> > > 
> > > Signed-off-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> 
> [snip]
> 
> > > -static int get_ctrl(struct v4l2_ctrl *ctrl, s32 *val)
> > > +static int get_ctrl(struct v4l2_ctrl *ctrl, struct v4l2_ext_control *c)
> > > 
> > >  {
> > >  
> > >  	struct v4l2_ctrl *master = ctrl->cluster[0];
> > >  	int ret = 0;
> > >  	int i;
> > > 
> > > +	/* String controls are not supported. The new_to_user() and
> > > +	 * cur_to_user() calls below would need to be fixed not to access
> > > +	 * userspace memory.
> > 
> > Just one small suggestion: change this comment to:
> > 
> > 	/* String controls are not supported. The new_to_user() and
> > 	 * cur_to_user() calls below would need to be modified not to access
> > 	 * userspace memory when called from get_ctrl().
> > 	 */
> > 
> > And a similar change in the comment with set_ctrl.
> > 
> > The word 'fixed' suggested that new_to_user etc. were broken, which isn't
> > the case. We are just using it in a special situation.
> 
> OK. Fixed.
> 
> > With the above change to get/set_ctrl you can add my
> > 
> > Acked-by: Hans Verkuil <hans.verkuil@xxxxxxxxx>
> 
> Thank you. I'll resubmit the patch (along with a driver patch that uses it). 
> Would you like to push it through your tree ?

You take it. I assume you have a driver that needs it, so it's better if it
goes in together with that driver.

Regards,

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