On Fri, 5 Oct 2012, Sascha Hauer wrote: > On Fri, Oct 05, 2012 at 05:41:00PM +0200, Guennadi Liakhovetski wrote: > > Hi Sascha > > > > > > + > > > > + ceu0: ceu@0xfe910000 { > > > > + compatible = "renesas,sh-mobile-ceu"; > > > > + reg = <0xfe910000 0xa0>; > > > > + interrupts = <0x880>; > > > > + > > > > + mclk: master_clock { > > > > + compatible = "renesas,ceu-clock"; > > > > + #clock-cells = <1>; > > > > + clock-frequency = <50000000>; /* max clock frequency */ > > > > + clock-output-names = "mclk"; > > > > + }; > > > > + > > > > + port { > > > > + #address-cells = <1>; > > > > + #size-cells = <0>; > > > > + > > > > + ceu0_1: link@1 { > > > > + reg = <1>; /* local link # */ > > > > + remote = <&ov772x_1_1>; /* remote phandle */ > > > > + bus-width = <8>; /* used data lines */ > > > > + data-shift = <0>; /* lines 7:0 are used */ > > > > + > > > > + /* If [hv]sync-active are missing, embedded bt.605 sync is used */ > > > > + hsync-active = <1>; /* active high */ > > > > + vsync-active = <1>; /* active high */ > > > > + data-active = <1>; /* active high */ > > > > + pclk-sample = <1>; /* rising */ > > > > + }; > > > > + > > > > + ceu0_0: link@0 { > > > > + reg = <0>; > > > > + remote = <&csi2_2>; > > > > + immutable; > > > > + }; > > > > + }; > > > > + }; > > > > + > > > > + i2c0: i2c@0xfff20000 { > > > > + ... > > > > + ov772x_1: camera@0x21 { > > > > + compatible = "omnivision,ov772x"; > > > > + reg = <0x21>; > > > > + vddio-supply = <®ulator1>; > > > > + vddcore-supply = <®ulator2>; > > > > + > > > > + clock-frequency = <20000000>; > > > > + clocks = <&mclk 0>; > > > > + clock-names = "xclk"; > > > > + > > > > + port { > > > > + /* With 1 link per port no need in addresses */ > > > > + ov772x_1_1: link { > > > > + bus-width = <8>; > > > > + remote = <&ceu0_1>; > > > > + hsync-active = <1>; > > > > + vsync-active = <0>; /* who came up with an inverter here?... */ > > > > + data-active = <1>; > > > > + pclk-sample = <1>; > > > > + }; > > > > > > I currently do not understand why these properties are both in the sensor > > > and in the link. What happens if they conflict? Are inverters assumed > > > like suggested above? I think the bus can only have a single bus-width, > > > why allow multiple bus widths here? > > > > Yes, these nodes represent port configuration of each party on a certain > > link. And they can differ in certain properties, like - as you correctly > > notice - in the case, when there's an inverter on a line. As for other > > properties, some of them must be identical, like bus-width, still, they > > have to be provided on both ends, because generally drivers have to be > > able to perform all the required configuration based only on the > > information from their own nodes, without looking at "remote" partner node > > properties. > > So the port associated to the ov772x_1 only describes how to configure > the ov772x and it's up to me to make sure that this configuration > matches the partner device. If I don't then it won't work but soc-camera > will happily continue. > Ok, that's good, I thought there would be some kind of matching > mechanism take place here. It may be worth to make this more explicit in > the docs. > > > > > > > + reg = <0xffc90000 0x1000>; > > > > + interrupts = <0x17a0>; > > > > + #address-cells = <1>; > > > > + #size-cells = <0>; > > > > + > > > > + port@1 { > > > > + compatible = "renesas,csi2c"; /* one of CSI2I and CSI2C */ > > > > + reg = <1>; /* CSI-2 PHY #1 of 2: PHY_S, PHY_M has port address 0, is unused */ > > > > + > > > > + csi2_1: link { > > > > + clock-lanes = <0>; > > > > + data-lanes = <2>, <1>; > > > > + remote = <&imx074_1>; > > > > + }; > > > > + }; > > > > + port@2 { > > > > + reg = <2>; /* port 2: link to the CEU */ > > > > + > > > > + csi2_2: link { > > > > + immutable; > > > > + remote = <&ceu0_0>; > > > > + }; > > > > + }; > > > > > > Maybe the example would be clearer if you split it up in two, one simple > > > case with the csi2_1 <-> imx074_1 and a more advanced with the link in > > > between. > > > > With no link between two ports no connection is possible, so, only > > examples with links make sense. > > I should have said with the renesas,sh-mobile-ceu in between. > > So simple example: csi2_1 <-l-> imx074_1 > advanced: csi2_2 <-l-> ceu <-l-> ov772x No, CEU is the DMA engine with some image processing, so, it's always present. The CSI-2 is just a MIPI CSI-2 interface, that in the above case is used with the CEU too. So, it's like ,-l- ov772x / ceu0 < \ `-l- csi2 -l- imx074 > > > It took me some time until I figured out that these are two > > > separate camera/sensor pairs. Somehow I was looking for a multiplexer > > > between them. > > > > Maybe I can add more comments to the file, perhaps, add an ASCII-art > > chart. > > That would be good. Is the above good enough? :) Thanks Guennadi > > > I am not sure we really want to have these circular phandles here. > > > > It has been suggested and accepted during a discussion at the KS / LPC a > > month ago. The original version only had phandle referencing in one > > direction. > > Ok. > > Sascha --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html