Re: [PATCH v2 1/2] media: soc_camera: pxa_camera documentation device-tree support

[Mark Rutland <mark.rutland@arm.com>]

On Wed, Jun 25, 2014 at 08:44:31PM +0100, Robert Jarzmik wrote:
> Mark Rutland <mark.rutland@arm.com> writes:
> 
> > On Sat, Jun 21, 2014 at 11:21:46PM +0100, Robert Jarzmik wrote:
> >> +Required properties:
> >> + - compatible: Should be "marvell,pxa27x-qci"
> >
> > Is that x a wildcard? Or is 'x' part of the name of a particular unit?
> It's kind of a wildcard for a group of platforms
> It stands for the 3 PXA27x SoCs I'm aware of : PXA270, PXA271, and PXA272. The
> difference between them is different core frequency range and embedded RAM.
> 
> > We prefer not to have wildcard compatible strings in DT.
> OK, then let's go for "marvell,pxa270-qci".

That sounds fine to me.

> >
> >> + - reg: register base and size
> >> + - interrupts: the interrupt number
> >> + - any required generic properties defined in video-interfaces.txt
> >> +
> >> +Optional properties:
> >> + - clock-frequency: host interface is driving MCLK, and MCLK rate is this rate
> >
> > Is MCLK an input or an output of this block?
> An output clock.
> 
> > If the former, why isn't this described as a clock?
> It's a good point. I'll try to add that too. The little trouble I have is that
> the PXA clocks are not _yet_ in device-tree. Putting a clock description will
> make this patch dependant on the clock framework patches [1], right ?

Yes, this will.

> 
> >> 
> >> +Example:
> >> +
> >> +	pxa_camera: pxa_camera@50000000 {
> >> +		compatible = "marvell,pxa27x-qci";
> >> +		reg = <0x50000000 0x1000>;
> >> +		interrupts = <33>;
> >> +
> >> +		clocks = <&pxa2xx_clks 24>;
> >> +		clock-names = "camera";
> >
> > These weren't mentioned above. Is the clock input line really called
> > "camera"?
> This is another clock, an input clock, independant of the former one. This is
> the clock actually fed to make this IP block work. This is dependant on the
> clock framework patches [1].

Ok. This clock should be mentioned in the properties description above.

Thanks,
Mark.