Hi Russell, On Tue, Aug 13, 2019 at 12:38:46PM +0100, Russell King - ARM Linux admin wrote: > On Tue, Aug 06, 2019 at 06:39:36PM +0530, Manivannan Sadhasivam wrote: > > +Required Properties: > > +- compatible: Should be "sony,imx290" > > +- reg: I2C bus address of the device > > +- clocks: Reference to the xclk clock. > > +- clock-names: Should be "xclk". > > +- clock-frequency: Frequency of the xclk clock. > > Driver code: > > + ret = of_property_read_u32(dev->of_node, "clock-frequency", &xclk_freq);+ if (ret) { > + dev_err(dev, "Could not get xclk frequency\n"); > + return ret; > + } > + > + /* external clock must be 37.125 MHz */ > + if (xclk_freq != 37125000) { > + dev_err(dev, "External clock frequency %u is not supported\n", > + xclk_freq); > + return -EINVAL; > + } > > So, only 37125000 is supported - is that not worth mentioning in this > description? Is this a hard requirement of the sensor? If so, why > does it need to be mentioned in the DT binding? The driver only supports a particular frequency, but the sensor is not limited to that. Unfortunately this is not uncommon for camera sensors, for the vendors often provide register settings for a given configuration only (external clock frequency, number of CSI-2 lanes, CSI-2 bus frequency, image cropping, binning etc.). That still doesn't mean there are no alternative configurations for different external clock frequencies, or that there could not be a driver that was able to configure the sensor to use a given frequency. -- Regards, Sakari Ailus