Hi Hans, On Wednesday 03 December 2014 12:17:57 Hans Verkuil wrote: > On 12/03/14 12:14, Sylwester Nawrocki wrote: > > On 02/12/14 13:21, Hans Verkuil wrote: > >> -static int s5k6aa_set_crop(struct v4l2_subdev *sd, struct v4l2_subdev_fh > >> *fh, > >> - struct v4l2_subdev_crop *crop) > >> +static int s5k6aa_set_selection(struct v4l2_subdev *sd, > >> + struct v4l2_subdev_fh *fh, > >> + struct v4l2_subdev_selection *sel) > >> { > >> struct s5k6aa *s5k6aa = to_s5k6aa(sd); > >> struct v4l2_mbus_framefmt *mf; > >> unsigned int max_x, max_y; > >> struct v4l2_rect *crop_r; > >> > >> + if (sel->pad || sel->target != V4L2_SEL_TGT_CROP) > >> + return -EINVAL; > >> + > > > > Isn't checking sel->pad redundant here ? There is already the pad index > > validation in check_selection() in v4l2-subdev.c and this driver has only > > one pad. > > If it is called from a bridge driver, then it hasn't gone through > check_selection(). > > That said, if it is called from a bridge driver, then one might expect > correct usage of pad. > > Laurent, do you have an opinion on this? I would expect the pad to be valid when called from a bridge driver. We could double-check that in subdev drivers as a debugging help, but I'm not sure if it's worth it. -- 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