Hi Laurent, Thank you for the review. On Fri, Jul 31, 2020 at 4:27 PM Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx> wrote: > > Hi Prabhakar, > > Thank you for the patch. > > On Fri, Jul 31, 2020 at 10:24:46AM +0100, Lad Prabhakar wrote: <snip> > > + * - 6: VSYNC output enable > > + * - 5: HREF output enable > > + * - 4: PCLK output enable > > + * - [3:0]: D[9:6] output enable > > + */ > > + ret = ov5640_write_reg(sensor, OV5640_REG_PAD_OUTPUT_ENABLE01, 0x7f); > > + if (ret) > > + goto power_off; > > > > - /* Give lanes some time to coax into LP11 state. */ > > - usleep_range(500, 1000); > > + /* > > + * enable D[5:0] DVP data lines > > + * > > + * PAD OUTPUT ENABLE 02 > > + * - [7:2]: D[5:0] output enable > > + */ > > + ret = ov5640_write_reg(sensor, OV5640_REG_PAD_OUTPUT_ENABLE02, 0xfc); > > + if (ret) > > + goto power_off; > > + } > > I'd split this to two separate functions, one for CSI-2, one for > parallel, as this is getting difficult to read. > Sure I'll split this up in v2.. Cheers, Prabhakar > > > > } else { > > if (sensor->ep.bus_type == V4L2_MBUS_CSI2_DPHY) { > > -- > Regards, > > Laurent Pinchart