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:-)
On Mon, 31 Dec 2012, Sylwester Nawrocki wrote: > From: Guennadi Liakhovetski <g.liakhovet...@gmx.de> > > 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.liakhovet...@gmx.de> > Signed-off-by: Sylwester Nawrocki <s.nawro...@samsung.com> > Reviewed-by: Stephen Warren <swar...@nvidia.com> > > --- > > 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. > +- 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. > +- 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?" 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-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
