Hi Sylwester On Wed, 2 Jan 2013, Sylwester Nawrocki wrote: > Hi Guennadi, > > On 01/02/2013 12:31 PM, Guennadi Liakhovetski wrote: > > Hi Sylwester > > > > Thanks for picking up these patches! In general both look good to me, just > > a couple of nit-picks, that I couldn't help remarking:-) > > Sure, thanks again for the feedback. > > > On Mon, 31 Dec 2012, Sylwester Nawrocki wrote: > > > > > From: Guennadi Liakhovetski<g.liakhovetski@xxxxxx> > > > > > > This patch adds a document describing common OF bindings for video > > > capture, output and video processing devices. It is curently mainly > > > focused on video capture devices, with data busses defined by > > > standards like ITU-R BT.656 or MIPI-CSI2. > > > It also documents a method of describing data links between devices. > > > > > > Signed-off-by: Guennadi Liakhovetski<g.liakhovetski@xxxxxx> > > > Signed-off-by: Sylwester Nawrocki<s.nawrocki@xxxxxxxxxxx> > > > Reviewed-by: Stephen Warren<swarren@xxxxxxxxxx> > > > > > > --- > > > > > > This is basically a resend of my previous version of this patch [1], > > > with just a few typo/grammar issue corrections. > > > > > > [1] http://patchwork.linuxtv.org/patch/15911/ > > > --- > > > .../devicetree/bindings/media/video-interfaces.txt | 198 > > > ++++++++++++++++++++ > > > 1 file changed, 198 insertions(+) > > > create mode 100644 > > > Documentation/devicetree/bindings/media/video-interfaces.txt > > > > > > diff --git a/Documentation/devicetree/bindings/media/video-interfaces.txt > > > b/Documentation/devicetree/bindings/media/video-interfaces.txt > > > new file mode 100644 > > > index 0000000..d1eea35 > > > --- /dev/null > > > +++ b/Documentation/devicetree/bindings/media/video-interfaces.txt > > > @@ -0,0 +1,198 @@ > > > +Common bindings for video data receiver and transmitter interfaces > > > + > > > +General concept > > > +--------------- > > > + > > > +Video data pipelines usually consist of external devices, e.g. camera > > > sensors, > > > +controlled over an I2C, SPI or UART bus, and SoC internal IP blocks, > > > including > > > +video DMA engines and video data processors. > > > + > > > +SoC internal blocks are described by DT nodes, placed similarly to other > > > SoC > > > +blocks. External devices are represented as child nodes of their > > > respective > > > +bus controller nodes, e.g. I2C. > > > + > > > +Data interfaces on all video devices are described by their child 'port' > > > nodes. > > > +Configuration of a port depends on other devices participating in the > > > data > > > +transfer and is described by 'endpoint' subnodes. > > > + > > > +dev { > > > + #address-cells =<1>; > > > + #size-cells =<0>; > > > + port@0 { > > > + endpoint@0 { ... }; > > > + endpoint@1 { ... }; > > > + }; > > > + port@1 { ... }; > > > +}; > > > + > > > +If a port can be configured to work with more than one other device on > > > the same > > > +bus, an 'endpoint' child node must be provided for each of them. If more > > > than > > > +one port is present in a device node or there is more than one endpoint > > > at a > > > +port, a common scheme, using '#address-cells', '#size-cells' and 'reg' > > > properties > > > +is used. > > > + > > > +Two 'endpoint' nodes are linked with each other through their > > > 'remote-endpoint' > > > +phandles. An endpoint subnode of a device contains all properties needed > > > for > > > +configuration of this device for data exchange with the other device. In > > > most > > > +cases properties at the peer 'endpoint' nodes will be identical, however > > > +they might need to be different when there is any signal modifications on > > > the > > > +bus between two devices, e.g. there are logic signal inverters on the > > > lines. > > > + > > > +Required properties > > > +------------------- > > > + > > > +If there is more than one 'port' or more than one 'endpoint' node > > > following > > > +properties are required in relevant parent node: > > > + > > > +- #address-cells : number of cells required to define port number, should > > > be 1. > > > +- #size-cells : should be zero. > > > + > > > +Optional endpoint properties > > > +---------------------------- > > > + > > > +- remote-endpoint : phandle to an 'endpoint' subnode of the other device > > > node. > > > > This spacing before ":" looks strange to me. I personally prefer the > > normal English rule - "x: y," i.e. no space before and a space after, but > > I wouldn't remark on your choice of a space on each side in this specific > > case, if it was consistent. Whereas sometimes having one space and > > sometimes having none looks weird to me. I would go for "no space before > > ':'" throughout this document. > > Gah, it was so close! ;) Sorry about it, it looked more readable to me that > way. > And I've checked other bindings' documentation and there was many files having > space on both sides of a colon. Nevertheless, I will change it back to the > original form. > > > > +- slave-mode : a boolean property, run the link in slave mode. Default is > > > master > > > + mode. > > > +- bus-width : number of data lines, valid for parallel buses. > > > > As we discussed before, both "busses" and "buses" spellings are commonly > > used at different locations around the world, but I think we should stick > > to only one of them in a single document. It looks weird to have "buses" > > in one line and "busses" in the following one. > > True, I think that was the one occurrence I'd noticed and have forgotten to > correct then. I'll fix it, thanks for pointing out. I think there were at least 2 of them, but I might be wrong, a grep will tell with certainty :-) > > > +- data-shift: on parallel data busses, if bus-width is used to specify > > > the > > > + number of data lines, data-shift can be used to specify which data > > > lines are > > > + used, e.g. "bus-width=<10>; data-shift=<2>;" means, that lines 9:2 are > > > used. > > > +- hsync-active : active state of HSYNC signal, 0/1 for LOW/HIGH > > > respectively. > > > +- vsync-active : active state of VSYNC signal, 0/1 for LOW/HIGH > > > respectively. > > > + Note, that if HSYNC and VSYNC polarities are not specified, embedded > > > + synchronization may be required, where supported. > > > +- data-active : similar to HSYNC and VSYNC, specifies data line polarity. > > > +- field-even-active: field signal level during the even field data > > > transmission. > > > +- pclk-sample : rising (1) or falling (0) edge to sample the pixel clock > > > signal. > > > > Yes, it was in my original document too, but don't we mean "sample data on > > rising (1) or falling (0) edge of the pixel clock signal?" > > Oops, I've managed to overlooked this. Certainly, it wasn't supposed to mean > sampling the clock signal. BTW, I had some doubts about this property. On the > transmitter side we more care about driving, rather than sampling data. And > usually when a transmitter drives data line at one clock edge type (e.g. > rising) > then the receiver samples data on the other edge (e.g. falling). > > In the display timing bindings there is a definitions like: > > + - pixelclk-active: with > + - active high = drive pixel data on rising edge/ > + sample data on falling edge > + - active low = drive pixel data on falling edge/ > + sample data on rising edge > + - ignored = ignored > > where: > > + <1>: high active > + <0>: low active > + omitted: not used on hardware > > > Then in our case, e.g. pclk-sample = <1>; on the transmitter side would mean > the receiver, which also has same pclk-sample = <1>; specified in its node, > has to sample data on rising clock edge and the transmitter is driving data > on falling edge, right ? That's also what seems logical to me. We can rephrase it to "should be sampled (by the receiver) on the rising edge." Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
 |