On Tue, Apr 24, 2018 at 09:36:44PM +0200, Maxime Ripard wrote: > Hi Sakari, > > On Tue, Apr 24, 2018 at 10:21:47AM +0300, Sakari Ailus wrote: > > > /* download ov5640 settings to sensor through i2c */ > > > static int ov5640_load_regs(struct ov5640_dev *sensor, > > > const struct ov5640_mode_info *mode) > > > @@ -1620,6 +1830,14 @@ static int ov5640_set_mode(struct ov5640_dev *sensor, > > > if (ret) > > > return ret; > > > > > > + if (sensor->ep.bus_type == V4L2_MBUS_CSI2) > > > + ret = ov5640_set_mipi_pclk(sensor, mode->clock); > > > > What is the value of the mode->clock expected to signify? It'd seem like > > that this changes from this patch to the next. Which one is correct? > > It doesn't, this is the clock rate computed through the formula > described above (and that might be incorrect for MIPI-CSI, given > Samuel feedback) from the way the registers are initialized in the > arrays. > > This shouldn't bring any change to the clock rate, but instead of > hardcoding it, we now have the infrastructure to calculate the factors > for any given rate. > > The subsequent patch will remove that hardcoded clock rate and > generate it dynamically from the timings / format. > > Does that make sense? Or maybe I should split this some other way? I guess it's ok as it is. I think I must have misread a chunk in the following patch --- sorry about the noise. -- Sakari Ailus sakari.ailus@xxxxxxxxxxxxxxx