Hi Dongchun, On Tue, Jun 30, 2020 at 10:49:42AM +0800, Dongchun Zhu wrote: > Add a V4L2 sub-device driver for OV02A10 image sensor. > > Signed-off-by: Dongchun Zhu <dongchun.zhu@xxxxxxxxxxxx> > --- > MAINTAINERS | 1 + > drivers/media/i2c/Kconfig | 13 + > drivers/media/i2c/Makefile | 1 + > drivers/media/i2c/ov02a10.c | 1052 +++++++++++++++++++++++++++++++++++++++++++ > 4 files changed, 1067 insertions(+) > create mode 100644 drivers/media/i2c/ov02a10.c Thank you for the patch. Please see my comments inline. [snip] > +static int ov02a10_entity_init_cfg(struct v4l2_subdev *sd, > + struct v4l2_subdev_pad_config *cfg) > +{ > + struct v4l2_subdev_format fmt = { > + .which = cfg ? V4L2_SUBDEV_FORMAT_TRY : V4L2_SUBDEV_FORMAT_ACTIVE, As we discussed before, this function is never called with cfg == NULL. Perhaps what we need here is to call ov02a10_set_fmt() twice, once for V4L2_SUBDEV_FORMAT_ACTIVE and then for V4L2_SUBDEV_FORMAT_TRY? Sakari, would that be a proper implementation of this function? > + .format = { > + .width = 1600, > + .height = 1200, > + } > + }; > + > + ov02a10_set_fmt(sd, cfg, &fmt); > + > + return 0; [snip] With this and Sakari's comment about the initial state of the reset pin fixed, feel free to add my Reviewed-by: Tomasz Figa <tfiga@xxxxxxxxxxxx> Best regards, Tomasz