Re: [PATCH v1 1/6] DT bindings: add bindings for ov965x camera module

[Laurent Pinchart]

Hi Nikolaus,

On Friday 23 Jun 2017 12:59:24 H. Nikolaus Schaller wrote:
> Am 23.06.2017 um 12:46 schrieb Andreas Färber <afaerber@xxxxxxx>:
> > Am 23.06.2017 um 12:25 schrieb H. Nikolaus Schaller:
> >>> diff --git a/Documentation/devicetree/bindings/media/i2c/ov965x.txt
> >>> b/Documentation/devicetree/bindings/media/i2c/ov965x.txt new file mode
> >>> 100644
> >>> index 0000000..0e0de1f
> >>> --- /dev/null
> >>> +++ b/Documentation/devicetree/bindings/media/i2c/ov965x.txt
> >>> @@ -0,0 +1,37 @@
> >>> +* Omnivision OV9650/9652/9655 CMOS sensor
> >>> +
> >>> +The Omnivision OV965x sensor support multiple resolutions output, such
> >>> as
> >>> +CIF, SVGA, UXGA. It also can support YUV422/420, RGB565/555 or raw RGB
> >>> +output format.
> >>> +
> >>> +Required Properties:
> >>> +- compatible: should be one of
> >>> +	"ovti,ov9650"
> >>> +	"ovti,ov9652"
> >>> +	"ovti,ov9655"
> >>> +- clocks: reference to the mclk input clock.
> >> 
> >> I wonder why you have removed the clock-frequency property?
> >> 
> >> In some situations the camera driver must be able to tell the clock
> >> source which frequency it wants to see.
> > 
> > That's what assigned-clock-rates property is for:
> > 
> > https://www.kernel.org/doc/Documentation/devicetree/bindings/clock/clock-b
> > indings.txt
> > 
> > AFAIU clock-frequency on devices is deprecated and equivalent to having
> > a clocks property pointing to a fixed-clock, which is different from a
> > clock with varying rate.
> 
> I am not sure if that helps here. The OMAP3-ISP does not have a fixed clock
> rate so we can only have the driver define what it wants to see.
> 
> And common practise for OMAP3-ISP based camera modules (e.g. N900, N9) is
> that they do it in the driver.
> 
> Maybe ISP developers can comment?

The OMAP3 ISP is a variable-frequency clock provider. The clock frequency is 
controlled by the clock consumer. As such, it's up to the consumer to decide 
whether to compute and request the clock rate dynamically at runtime, or use 
the assigned-clock-rates property in DT.

Some ISPs include a clock generator, others don't. It should make no 
difference whether the clock is provided by the ISP, by a dedicated clock 
source in the SoC or by a discrete on-board adjustable clock source.

-- 
Regards,

Laurent Pinchart