On Wed, Oct 03, 2018 at 10:24:17PM +0200, Ricardo Ribalda Delgado wrote: ... > > > +static int imx214_enum_frame_size(struct v4l2_subdev *subdev, > > > + struct v4l2_subdev_pad_config *cfg, > > > + struct v4l2_subdev_frame_size_enum *fse) > > > +{ > > > + if (fse->code != IMX214_MBUS_CODE) > > > + return -EINVAL; > > > + > > > + if (fse->index >= ARRAY_SIZE(imx214_modes)) > > > > array_index_nospec() ?? I find it scary that you'd need that in drivers. > > :-o Uh... not needed. The value is just sent back to the user as such so this is fine AFAICT. > > > > > + return -EINVAL; > > > + > > > + fse->min_width = fse->max_width = imx214_modes[fse->index].width; > > > + fse->min_height = fse->max_height = imx214_modes[fse->index].height; > > > + > > > + return 0; > > > +} ... > > > + /* > > > + * WARNING! > > > + * Values obtained reverse engineering blobs and/or devices. > > > + * Ranges and functionality might be wrong. > > > + * > > > + * Sony, please release some register set documentation for the > > > + * device. > > > + * > > > + * Yours sincerely, Ricardo. > > > + */ > > > + imx214->exposure = v4l2_ctrl_new_std(&imx214->ctrls, &imx214_ctrl_ops, > > > + V4L2_CID_EXPOSURE, > > > + 0, 0xffff, 1, 0x0c70); > > > > The exposure is in lines so it can't exceed frame height + blanking. > > There's a marginal, too. I don't know what it might be for this sensor > > though. Usually it's small, such as 8 or 16. The image will almost > > certainly be garbled if you exceed the allowed value. > >Seems that this sensor > > On this sensor what I am experiencing instead of garbage is that the > fps gets reduced. So I believe it is fine to > set it up this way. That's rather confusing as well. The user should explicitly need to change fps first, rather than it happening as a side effect of a seemingly unrelated control. -- Regards, Sakari Ailus sakari.ailus@xxxxxxxxxxxxxxx