| Message ID | 1358969489-20420-2-git-send-email-s.nawrocki@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Sylwester, Thanks for the patch. On Wednesday 23 January 2013 20:31:16 Sylwester Nawrocki wrote: > From: Guennadi Liakhovetski <g.liakhovetski@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.liakhovetski@gmx.de> > Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com> > Reviewed-by: Stephen Warren <swarren@nvidia.com> > Acked-by: Rob Herring <rob.herring@calxeda.com> > --- > > Changes since v3: > - improved clock-lanes property description, > - grammar corrections of the example dts snippet description. > --- > .../devicetree/bindings/media/video-interfaces.txt | 204 +++++++++++++++++ > 1 file changed, 204 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..0da126f > --- /dev/null > +++ b/Documentation/devicetree/bindings/media/video-interfaces.txt > @@ -0,0 +1,204 @@ > +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. Wouldn't this cause problems if the device has both video ports and a child bus ? Using #address-cells and #size-cells for the video ports would prevent the child bus from being handled in the usual way. A possible solution would be to number ports with a dash instead of a @, as done in pinctrl for instance. We would then get port-0 { endpoint-0 { ... }; endpoint-1 { ... }; }; port-1 { ... }; > +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. I wonder if we should specify whether a port is a data sink or data source. A source can be connected to multiple sinks at the same time, but a sink can only be connected to a single source. If we want to perform automatic sanity checks in the core knowing the direction might help. > +Optional endpoint properties > +---------------------------- > + > +- remote-endpoint: phandle to an 'endpoint' subnode of the other device > + node. > +- slave-mode: a boolean property, run the link in slave mode. > + Default is master mode. What are master and slave modes ? It might be worth it describing them. > +- bus-width: number of data lines, valid for parallel busses. > +- 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: sample data on rising (1) or falling (0) edge of the pixel > + clock signal. > +- data-lanes: an array of physical data lane indexes. Position of an entry > + determines the logical lane number, while the value of an entry indicates > + physical lane, e.g. for 2-lane MIPI CSI-2 bus we could have > + "data-lanes = <1>, <2>;", assuming the clock lane is on hardware lane 0. > + This property is valid for serial busses only (e.g. MIPI CSI-2). > +- clock-lanes: an array of physical clock lane indexes. Position of an > + entry determines the logical lane number, while the value of an entry > + indicates physical lane, e.g. for a MIPI CSI-2 bus we could have > + "clock-lanes = <0>;", which places the clock lane on hardware lane 0. > + This property is valid for serial busses only (e.g. MIPI CSI-2). Note > + that for the MIPI CSI-2 bus this array contains only one entry. > +- clock-noncontinuous: a boolean property to allow MIPI CSI-2 > + non-continuous clock mode. > + > +Example > +------- > + > +The example snippet below describes two data pipelines. ov772x and imx074 > +are camera sensors with a parallel and serial (MIPI CSI-2) video bus > +respectively. Both sensors are on the I2C control bus corresponding to the > +i2c0 controller node. ov772x sensor is linked directly to the ceu0 video > +host interface. imx074 is linked to ceu0 through the MIPI CSI-2 receiver > +(csi2). ceu0 has a (single) DMA engine writing captured data to memory. > +ceu0 node has a single 'port' node which indicates that at any time only > +one of the following data pipelines can be active: ov772x -> ceu0 or > +imx074 -> csi2 -> ceu0. > + > + 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: endpoint@1 { > + reg = <1>; /* Local endpoint # */ > + remote = <&ov772x_1_1>; /* Remote phandle */ > + bus-width = <8>; /* Used data lines */ > + data-shift = <0>; /* Lines 7:0 are used */ As data-shift is optional, shouldn't it be left out when equal to 0 ? It would, however, be nice to have a non-zero data-shift somewhere in the example. > + > + /* If hsync-active/vsync-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: endpoint@0 { > + reg = <0>; > + remote = <&csi2_2>; > + immutable; What is the immutable property for her e? > + }; > + }; > + }; > + > + 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 endpoint per port no need in addresses. */ s/in/for/ ? > + ov772x_1_1: endpoint { > + bus-width = <8>; > + remote-endpoint = <&ceu0_1>; > + hsync-active = <1>; > + vsync-active = <0>; /* Who came up with an > + inverter here ?... */ > + data-active = <1>; > + pclk-sample = <1>; > + }; > + }; > + }; > + > + imx074: camera@0x1a { > + compatible = "sony,imx074"; > + reg = <0x1a>; > + vddio-supply = <®ulator1>; > + vddcore-supply = <®ulator2>; > + > + clock-frequency = <30000000>; /* Shared clock with ov772x_1 */ > + clocks = <&mclk 0>; > + clock-names = "sysclk"; /* Assuming this is the > + name in the datasheet */ > + port { > + imx074_1: endpoint { > + clock-lanes = <0>; > + data-lanes = <1>, <2>; > + remote-endpoint = <&csi2_1>; > + }; > + }; > + }; > + }; > + > + csi2: csi2@0xffc90000 { > + compatible = "renesas,sh-mobile-csi2"; > + 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: endpoint { > + clock-lanes = <0>; > + data-lanes = <2>, <1>; > + remote-endpoint = <&imx074_1>; > + }; > + }; > + port@2 { > + reg = <2>; /* port 2: link to the CEU */ > + > + csi2_2: endpoint { > + immutable; > + remote-endpoint = <&ceu0_0>; > + }; > + }; > + };
Hi Laurent, Thanks for the review. On 01/24/2013 11:16 AM, Laurent Pinchart wrote: [...] >> +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. > > Wouldn't this cause problems if the device has both video ports and a child > bus ? Using #address-cells and #size-cells for the video ports would prevent > the child bus from being handled in the usual way. Indeed, it looks like a serious issue in these bindings. > A possible solution would be to number ports with a dash instead of a @, as > done in pinctrl for instance. We would then get > > port-0 { > endpoint-0 { ... }; > endpoint-1 { ... }; > }; > port-1 { ... }; Sounds like a good alternative, I can't think of any better solution at the moment. >> +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. > > I wonder if we should specify whether a port is a data sink or data source. A > source can be connected to multiple sinks at the same time, but a sink can > only be connected to a single source. If we want to perform automatic sanity > checks in the core knowing the direction might help. Multiple sources can be linked to a single sink, but only one link can be active at any time. So I'm not sure if knowing if a DT port is a data source or data sink would let us to validate device tree structure statically in general. Such source/sink property could be useful later at runtime, when data pipeline is set up for streaming. How do you think this could be represented ? By just having boolean properties like: 'source' and 'sink' in the port nodes ? Or perhaps in the endpoint nodes, since some devices might be bidirectional ? I don't recall any at the moment though. >> +Optional endpoint properties >> +---------------------------- >> + >> +- remote-endpoint: phandle to an 'endpoint' subnode of the other device >> + node. >> +- slave-mode: a boolean property, run the link in slave mode. >> + Default is master mode. > > What are master and slave modes ? It might be worth it describing them. This was originally proposed by Guennadi, I think he knows exactly what's the meaning of this property. I'll dig into relevant documentation to find out and provide more detailed description. >> +- bus-width: number of data lines, valid for parallel busses. >> +- 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: sample data on rising (1) or falling (0) edge of the pixel >> + clock signal. >> +- data-lanes: an array of physical data lane indexes. Position of an entry >> + determines the logical lane number, while the value of an entry indicates >> + physical lane, e.g. for 2-lane MIPI CSI-2 bus we could have >> + "data-lanes = <1>, <2>;", assuming the clock lane is on hardware lane 0. >> + This property is valid for serial busses only (e.g. MIPI CSI-2). >> +- clock-lanes: an array of physical clock lane indexes. Position of an >> + entry determines the logical lane number, while the value of an entry >> + indicates physical lane, e.g. for a MIPI CSI-2 bus we could have >> + "clock-lanes = <0>;", which places the clock lane on hardware lane 0. >> + This property is valid for serial busses only (e.g. MIPI CSI-2). Note >> + that for the MIPI CSI-2 bus this array contains only one entry. >> +- clock-noncontinuous: a boolean property to allow MIPI CSI-2 >> + non-continuous clock mode. >> + >> +Example >> +------- >> + >> +The example snippet below describes two data pipelines. ov772x and imx074 >> +are camera sensors with a parallel and serial (MIPI CSI-2) video bus >> +respectively. Both sensors are on the I2C control bus corresponding to the >> +i2c0 controller node. ov772x sensor is linked directly to the ceu0 video >> +host interface. imx074 is linked to ceu0 through the MIPI CSI-2 receiver >> +(csi2). ceu0 has a (single) DMA engine writing captured data to memory. >> +ceu0 node has a single 'port' node which indicates that at any time only >> +one of the following data pipelines can be active: ov772x -> ceu0 or >> +imx074 -> csi2 -> ceu0. >> + >> + 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: endpoint@1 { >> + reg = <1>; /* Local endpoint # */ >> + remote = <&ov772x_1_1>; /* Remote phandle */ >> + bus-width = <8>; /* Used data lines */ >> + data-shift = <0>; /* Lines 7:0 are used */ > > As data-shift is optional, shouldn't it be left out when equal to 0 ? It > would, however, be nice to have a non-zero data-shift somewhere in the > example. Yes, good point. data-shift could be ommited. I'm going to increase the bus-width and make data-shit non-zero. >> + >> + /* If hsync-active/vsync-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: endpoint@0 { >> + reg = <0>; >> + remote = <&csi2_2>; >> + immutable; > > What is the immutable property for her e? I was staring at this yesterday and finally I forgot to remove it. It is undocumented and I think it's not supposed to be here. Guennadi, would you have any comments on that ? >> + }; >> + }; >> + }; >> + >> + 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 endpoint per port no need in addresses. */ > > s/in/for/ ? I proposed same change to Guennadi, but he argued that "in" is also commonly used. I agreed even though 'for' seemed more natural to me. I would change it, unless there is a strong opposition. :) -- Regards, Sylwester -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Sylwester, On Thursday 24 January 2013 19:30:10 Sylwester Nawrocki wrote: > On 01/24/2013 11:16 AM, Laurent Pinchart wrote: > [...] > > >> +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. > > > > Wouldn't this cause problems if the device has both video ports and a > > child bus ? Using #address-cells and #size-cells for the video ports would > > prevent the child bus from being handled in the usual way. > > Indeed, it looks like a serious issue in these bindings. > > > A possible solution would be to number ports with a dash instead of a @, > > as done in pinctrl for instance. We would then get > > > > port-0 { > > endpoint-0 { ... }; > > endpoint-1 { ... }; > > }; > > port-1 { ... }; > > Sounds like a good alternative, I can't think of any better solution at the > moment. > > >> +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. > > > > I wonder if we should specify whether a port is a data sink or data > > source. A source can be connected to multiple sinks at the same time, but > > a sink can only be connected to a single source. If we want to perform > > automatic sanity checks in the core knowing the direction might help. > > Multiple sources can be linked to a single sink, but only one link can be > active at any time. > > So I'm not sure if knowing if a DT port is a data source or data sink would > let us to validate device tree structure statically in general. > > Such source/sink property could be useful later at runtime, when data > pipeline is set up for streaming. Yes, I was mostly thinking about runtime. > How do you think this could be represented ? By just having boolean > properties like: 'source' and 'sink' in the port nodes ? Or perhaps in the > endpoint nodes, since some devices might be bidirectional ? I don't recall > any at the moment though. Source and sink properties would do. We could also use a direction property that could take sink, source and bidirectional values, but that might be more complex. I don't think we will have bidirectional link (as that would most probably involve a very different kind of bus, and thus new bindings). > >> +Optional endpoint properties > >> +---------------------------- > >> + > >> +- remote-endpoint: phandle to an 'endpoint' subnode of the other device > >> + node. > >> +- slave-mode: a boolean property, run the link in slave mode. > >> + Default is master mode. > > > > What are master and slave modes ? It might be worth it describing them. > > This was originally proposed by Guennadi, I think he knows exactly what's > the meaning of this property. I'll dig into relevant documentation to > find out and provide more detailed description. Thank you. > >> +- bus-width: number of data lines, valid for parallel busses. > >> +- 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: sample data on rising (1) or falling (0) edge of the > >> + pixel clock signal. > >> +- data-lanes: an array of physical data lane indexes. Position of an > >> + entry determines the logical lane number, while the value of an entry > >> + indicates physical lane, e.g. for 2-lane MIPI CSI-2 bus we could have > >> + "data-lanes = <1>, <2>;", assuming the clock lane is on hardware lane > >> + 0. This property is valid for serial busses only (e.g. MIPI CSI-2). > >> +- clock-lanes: an array of physical clock lane indexes. Position of an > >> + entry determines the logical lane number, while the value of an entry > >> + indicates physical lane, e.g. for a MIPI CSI-2 bus we could have > >> + "clock-lanes = <0>;", which places the clock lane on hardware lane 0. > >> + This property is valid for serial busses only (e.g. MIPI CSI-2). Note > >> + that for the MIPI CSI-2 bus this array contains only one entry. > >> +- clock-noncontinuous: a boolean property to allow MIPI CSI-2 > >> + non-continuous clock mode. > >> + > >> +Example > >> +------- > >> + > >> +The example snippet below describes two data pipelines. ov772x and > >> +imx074 are camera sensors with a parallel and serial (MIPI CSI-2) video > >> +bus respectively. Both sensors are on the I2C control bus corresponding > >> +to the i2c0 controller node. ov772x sensor is linked directly to the > >> +ceu0 video host interface. imx074 is linked to ceu0 through the MIPI > >> +CSI-2 receiver (csi2). ceu0 has a (single) DMA engine writing captured > >> +data to memory. ceu0 node has a single 'port' node which indicates that > >> +at any time only one of the following data pipelines can be active: > >> +ov772x -> ceu0 or imx074 -> csi2 -> ceu0. > >> + > >> + 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: endpoint@1 { > >> + reg = <1>; /* Local endpoint # */ > >> + remote = <&ov772x_1_1>; /* Remote phandle */ > >> + bus-width = <8>; /* Used data lines */ > >> + data-shift = <0>; /* Lines 7:0 are used */ > > > > As data-shift is optional, shouldn't it be left out when equal to 0 ? It > > would, however, be nice to have a non-zero data-shift somewhere in the > > example. > > Yes, good point. data-shift could be ommited. I'm going to increase the > bus-width and make data-shit non-zero. > > >> + > >> + /* If hsync-active/vsync-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: endpoint@0 { > >> + reg = <0>; > >> + remote = <&csi2_2>; > >> + immutable; > > > > What is the immutable property for her e? > > I was staring at this yesterday and finally I forgot to remove it. It is > undocumented and I think it's not supposed to be here. Guennadi, would > you have any comments on that ? > > >> + }; > >> + }; > >> + }; > >> + > >> + 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 endpoint per port no need in addresses. */ > > > > s/in/for/ ? > > I proposed same change to Guennadi, but he argued that "in" is also > commonly used. I agreed even though 'for' seemed more natural to me. > I would change it, unless there is a strong opposition. :)
Hi Laurent, On 01/25/2013 02:52 AM, Laurent Pinchart wrote: >>>> +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. >>> >>> Wouldn't this cause problems if the device has both video ports and a >>> child bus ? Using #address-cells and #size-cells for the video ports would >>> prevent the child bus from being handled in the usual way. >> >> Indeed, it looks like a serious issue in these bindings. >> >>> A possible solution would be to number ports with a dash instead of a @, >>> as done in pinctrl for instance. We would then get >>> >>> port-0 { >>> endpoint-0 { ... }; >>> endpoint-1 { ... }; >>> }; >>> port-1 { ... }; One problem here is that index of the port or the endpoint node can have random value and don't need to start with 0, which is the case for the pinctrl properties. It makes iterating over those nodes more difficult, instead of using standard functions like of_node_cmp() we would need to search for sub-strings in the node name. >> Sounds like a good alternative, I can't think of any better solution at the >> moment. >> >>>> +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. >>> >>> I wonder if we should specify whether a port is a data sink or data >>> source. A source can be connected to multiple sinks at the same time, but >>> a sink can only be connected to a single source. If we want to perform >>> automatic sanity checks in the core knowing the direction might help. >> >> Multiple sources can be linked to a single sink, but only one link can be >> active at any time. >> >> So I'm not sure if knowing if a DT port is a data source or data sink would >> let us to validate device tree structure statically in general. >> >> Such source/sink property could be useful later at runtime, when data >> pipeline is set up for streaming. > > Yes, I was mostly thinking about runtime. > >> How do you think this could be represented ? By just having boolean >> properties like: 'source' and 'sink' in the port nodes ? Or perhaps in the >> endpoint nodes, since some devices might be bidirectional ? I don't recall >> any at the moment though. > > Source and sink properties would do. We could also use a direction property > that could take sink, source and bidirectional values, but that might be more > complex. Since we're going to allow multiple endpoints at a port to be active at any time, for the reasons we discussed in IRC [1], I assume it's no longer possible to perform sanity checks mentioned above in the core. Should we then keep the 'source', 'sink' properties in the port nodes ? [1] http://linuxtv.org/irc/v4l/index.php?date=2013-01-29 > I don't think we will have bidirectional link (as that would most probably > involve a very different kind of bus, and thus new bindings). -- Thanks, Sylwester -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, On 01/30/2013 01:40 PM, Sylwester Nawrocki wrote: > On 01/25/2013 02:52 AM, Laurent Pinchart wrote: >>>>> +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. >>>> >>>> Wouldn't this cause problems if the device has both video ports and a >>>> child bus ? Using #address-cells and #size-cells for the video ports would >>>> prevent the child bus from being handled in the usual way. >>> >>> Indeed, it looks like a serious issue in these bindings. >>> >>>> A possible solution would be to number ports with a dash instead of a @, >>>> as done in pinctrl for instance. We would then get >>>> >>>> port-0 { >>>> endpoint-0 { ... }; >>>> endpoint-1 { ... }; >>>> }; >>>> port-1 { ... }; Another possible solution could be putting the port nodes under a ports node, for these cases where a device has a child bus. Where there is no conflict, the ports node could be omitted for simplicity. Does it sound reasonable ? device { ports { #address-cells = <1>; #size-cells = <0>; port@0 { endpoint@0 { reg = <0>; }; endpoint@1 { ... }; }; port@1 { ... }; }; }; > One problem here is that index of the port or the endpoint node can have > random value and don't need to start with 0, which is the case for the pinctrl > properties. It makes iterating over those nodes more difficult, instead > of using standard functions like of_node_cmp() we would need to search for > sub-strings in the node name. -- Thanks, Sylwester -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/Documentation/devicetree/bindings/media/video-interfaces.txt b/Documentation/devicetree/bindings/media/video-interfaces.txt new file mode 100644 index 0000000..0da126f --- /dev/null +++ b/Documentation/devicetree/bindings/media/video-interfaces.txt @@ -0,0 +1,204 @@ +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. +- slave-mode: a boolean property, run the link in slave mode. Default is master + mode. +- bus-width: number of data lines, valid for parallel busses. +- 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: sample data on rising (1) or falling (0) edge of the pixel clock + signal. +- data-lanes: an array of physical data lane indexes. Position of an entry + determines the logical lane number, while the value of an entry indicates + physical lane, e.g. for 2-lane MIPI CSI-2 bus we could have + "data-lanes = <1>, <2>;", assuming the clock lane is on hardware lane 0. + This property is valid for serial busses only (e.g. MIPI CSI-2). +- clock-lanes: an array of physical clock lane indexes. Position of an entry + determines the logical lane number, while the value of an entry indicates + physical lane, e.g. for a MIPI CSI-2 bus we could have "clock-lanes = <0>;", + which places the clock lane on hardware lane 0. This property is valid for + serial busses only (e.g. MIPI CSI-2). Note that for the MIPI CSI-2 bus this + array contains only one entry. +- clock-noncontinuous: a boolean property to allow MIPI CSI-2 non-continuous + clock mode. + +Example +------- + +The example snippet below describes two data pipelines. ov772x and imx074 are +camera sensors with a parallel and serial (MIPI CSI-2) video bus respectively. +Both sensors are on the I2C control bus corresponding to the i2c0 controller +node. ov772x sensor is linked directly to the ceu0 video host interface. +imx074 is linked to ceu0 through the MIPI CSI-2 receiver (csi2). ceu0 has a +(single) DMA engine writing captured data to memory. ceu0 node has a single +'port' node which indicates that at any time only one of the following data +pipelines can be active: ov772x -> ceu0 or imx074 -> csi2 -> ceu0. + + 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: endpoint@1 { + reg = <1>; /* Local endpoint # */ + remote = <&ov772x_1_1>; /* Remote phandle */ + bus-width = <8>; /* Used data lines */ + data-shift = <0>; /* Lines 7:0 are used */ + + /* If hsync-active/vsync-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: endpoint@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 endpoint per port no need in addresses. */ + ov772x_1_1: endpoint { + bus-width = <8>; + remote-endpoint = <&ceu0_1>; + hsync-active = <1>; + vsync-active = <0>; /* Who came up with an + inverter here ?... */ + data-active = <1>; + pclk-sample = <1>; + }; + }; + }; + + imx074: camera@0x1a { + compatible = "sony,imx074"; + reg = <0x1a>; + vddio-supply = <®ulator1>; + vddcore-supply = <®ulator2>; + + clock-frequency = <30000000>; /* Shared clock with ov772x_1 */ + clocks = <&mclk 0>; + clock-names = "sysclk"; /* Assuming this is the + name in the datasheet */ + port { + imx074_1: endpoint { + clock-lanes = <0>; + data-lanes = <1>, <2>; + remote-endpoint = <&csi2_1>; + }; + }; + }; + }; + + csi2: csi2@0xffc90000 { + compatible = "renesas,sh-mobile-csi2"; + 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: endpoint { + clock-lanes = <0>; + data-lanes = <2>, <1>; + remote-endpoint = <&imx074_1>; + }; + }; + port@2 { + reg = <2>; /* port 2: link to the CEU */ + + csi2_2: endpoint { + immutable; + remote-endpoint = <&ceu0_0>; + }; + }; + };