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? > Please also add a comment or two documenting this; it'll be otherwise > difficult to find out later on. I'm not sure there's a point in documenting that intermediate step that is just there for a single commit. Maybe I should expand the commit log to make it clearer? Maxime -- Maxime Ripard, Bootlin (formerly Free Electrons) Embedded Linux and Kernel engineering https://bootlin.com
