[PATCH] media: add V4L2 DT binding documentation

[Guennadi Liakhovetski]

Hi Stephen

On Wed, 12 Sep 2012, Stephen Warren wrote:

> On 09/11/2012 09:51 AM, Guennadi Liakhovetski wrote:
> > This patch adds a document, describing common V4L2 device tree bindings.
> > 
> > Co-authored-by: Sylwester Nawrocki <s.nawrocki at samsung.com>
> > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski at gmx.de>
> 
> Overall, I think this looks pretty reasonable, so:

Good, thanks!

> Acked-by: Stephen Warren <swarren at wwwdotorg.org>
> 
> Just a couple comments:
> 
> > +++ b/Documentation/devicetree/bindings/media/v4l2.txt
> 
> > +	ceu0: ceu at 0xfe910000 {
> 
> > +		mclk: master_clock {
> > +			compatible = "renesas,ceu-clock";
> > +			#clock-cells = <1>;
> 
> Why 1? If there's only 1 clock output from this provider, I don't see a
> need for any cells, unless there are some configuration flags?

Yes, indeed, that's also what's suggested in the clock bindings 
documentation, thanks for pointing out.

> 
> > +			clock-frequency = <50000000>;	/* max clock frequency */
> > +			clock-output-names = "mclk";
> > +		};
> > +
> > +		port {
> ...
> > +			ceu0_0: link at 0 {
> > +				reg = <0>;
> > +				remote = <&csi2_2>;
> > +				immutable;
> 
> Did we decide "immutable" was actually needed? Presumably the driver for
> the HW in question knows the HW isn't configurable, and would simply not
> attempt to apply any configuration even if the .dts author erroneously
> provided some?

Well, I've been thinking about this today. I think, bridge drivers will 
call centrally provided helper functions to enumerate links inside ports. 
While doing that they will want to differentiate between links to external 
devices with explicit configuration, and links to internal devices, whose 
configuration drivers might be able to figure out themselves. How should a 
driver find out what device this link is pointing to? Should it follow the 
"remote" phandle and then check the "compatible" property? The word 
"immutable" is a hint, that this is a link to an internal device, but it 
might either be unneeded or be transformed into something more 
informative.

> > +			};
> > +		};
> > +	};
> > +
> > +	i2c0: i2c at 0xfff20000 {
> ...
> > +		ov772x_1: camera at 0x21 {
> ...
> > +			clocks = <&mclk 0>;
> 
> So presumably that could just be "clocks = <&mclk>;"?

Right.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/