| Message ID | 1348754853-28619-5-git-send-email-g.liakhovetski@gmx.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Guennadi, On 09/27/2012 04:07 PM, Guennadi Liakhovetski wrote: > This patch adds a document, describing common V4L2 device tree bindings. > > Co-authored-by: Sylwester Nawrocki<s.nawrocki@samsung.com> > Signed-off-by: Guennadi Liakhovetski<g.liakhovetski@gmx.de> > --- > Documentation/devicetree/bindings/media/v4l2.txt | 162 ++++++++++++++++++++++ > 1 files changed, 162 insertions(+), 0 deletions(-) > create mode 100644 Documentation/devicetree/bindings/media/v4l2.txt > > diff --git a/Documentation/devicetree/bindings/media/v4l2.txt b/Documentation/devicetree/bindings/media/v4l2.txt > new file mode 100644 > index 0000000..b8b3f41 > --- /dev/null > +++ b/Documentation/devicetree/bindings/media/v4l2.txt > @@ -0,0 +1,162 @@ > +Video4Linux Version 2 (V4L2) > + > +General concept > +--------------- > + > +Video pipelines 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 "port" child DT nodes. > +Configuration of a port depends on other devices participating in the data > +transfer and is described by "link" DT nodes, specified as children of the > +"port" nodes: > + > +/foo { > + port@0 { > + link@0 { ... }; > + link@1 { ... }; > + }; > + port@1 { ... }; > +}; > + > +If a port can be configured to work with more than one other device on the same > +bus, a "link" child DT node must be provided for each of them. If more than one > +port is present on a device or more than one link is connected to a port, a > +common scheme, using "#address-cells," "#size-cells" and "reg" properties is > +used. > + > +Optional link properties: > +- remote: phandle to the other endpoint link DT node. > +- slave-mode: a boolean property, run the link in slave mode. Default is master > + mode. > +- data-shift: on parallel data busses, if data-width is used to specify the > + number of data lines, data-shift can be used to specify which data lines are > + used, e.g. "data-width=<10>; data-shift=<2>;" means, that lines 9:2 are used. > +- hsync-active: 1 or 0 for active-high or -low HSYNC signal polarity > + respectively. > +- vsync-active: ditto for VSYNC. Note, that if HSYNC and VSYNC polarities are > + not specified, embedded synchronisation 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 pin. > +- data-lanes: array of serial, e.g. MIPI CSI-2, data hardware lane numbers in > + the ascending order, beginning with logical lane 0. > +- clock-lanes: hardware lane number, used for the clock lane. It is not very clear we are using common contiguous range of logical indexes for the clock and the data lanes, IMO. Maybe something like this would be more explicit: - data-lanes: an array of hardware data lane indexes. Position of an entry determines logical lane number, while the value of an entry indicates hardware lane number, 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 applicable to serial buses only (e.g. MIPI CSI-2). ? -- 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
On 09/27/2012 09:07 AM, Guennadi Liakhovetski wrote: > This patch adds a document, describing common V4L2 device tree bindings. > > Co-authored-by: Sylwester Nawrocki <s.nawrocki@samsung.com> > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de> > --- > Documentation/devicetree/bindings/media/v4l2.txt | 162 ++++++++++++++++++++++ > 1 files changed, 162 insertions(+), 0 deletions(-) > create mode 100644 Documentation/devicetree/bindings/media/v4l2.txt > > diff --git a/Documentation/devicetree/bindings/media/v4l2.txt b/Documentation/devicetree/bindings/media/v4l2.txt > new file mode 100644 > index 0000000..b8b3f41 > --- /dev/null > +++ b/Documentation/devicetree/bindings/media/v4l2.txt > @@ -0,0 +1,162 @@ > +Video4Linux Version 2 (V4L2) DT describes the h/w, but V4L2 is Linux specific. I think the binding looks pretty good in terms of it is describing the h/w and not V4L2 components or settings. So in this case it's really just the name of the file and title I have issue with. One other comment below: > + > +General concept > +--------------- > + > +Video pipelines 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 "port" child DT nodes. > +Configuration of a port depends on other devices participating in the data > +transfer and is described by "link" DT nodes, specified as children of the > +"port" nodes: > + > +/foo { > + port@0 { > + link@0 { ... }; > + link@1 { ... }; > + }; > + port@1 { ... }; > +}; > + > +If a port can be configured to work with more than one other device on the same > +bus, a "link" child DT node must be provided for each of them. If more than one > +port is present on a device or more than one link is connected to a port, a > +common scheme, using "#address-cells," "#size-cells" and "reg" properties is > +used. > + > +Optional link properties: > +- remote: phandle to the other endpoint link DT node. This name is a little vague. Perhaps "endpoint" would be better. Rob > +- slave-mode: a boolean property, run the link in slave mode. Default is master > + mode. > +- data-shift: on parallel data busses, if data-width is used to specify the > + number of data lines, data-shift can be used to specify which data lines are > + used, e.g. "data-width=<10>; data-shift=<2>;" means, that lines 9:2 are used. > +- hsync-active: 1 or 0 for active-high or -low HSYNC signal polarity > + respectively. > +- vsync-active: ditto for VSYNC. Note, that if HSYNC and VSYNC polarities are > + not specified, embedded synchronisation 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 pin. > +- data-lanes: array of serial, e.g. MIPI CSI-2, data hardware lane numbers in > + the ascending order, beginning with logical lane 0. > +- clock-lanes: hardware lane number, used for the clock lane. > +- clock-noncontinuous: a boolean property to allow MIPI CSI-2 non-continuous > + clock mode. > + > +Example: > + > + 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>; > + }; > + }; > + }; > + > + 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: link { > + clock-lanes = <0>; > + data-lanes = <1>, <2>; > + remote = <&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: 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>; > + }; > + }; > + }; > -- 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 Rob On Tue, 2 Oct 2012, Rob Herring wrote: > On 09/27/2012 09:07 AM, Guennadi Liakhovetski wrote: > > This patch adds a document, describing common V4L2 device tree bindings. > > > > Co-authored-by: Sylwester Nawrocki <s.nawrocki@samsung.com> > > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de> > > --- > > Documentation/devicetree/bindings/media/v4l2.txt | 162 ++++++++++++++++++++++ > > 1 files changed, 162 insertions(+), 0 deletions(-) > > create mode 100644 Documentation/devicetree/bindings/media/v4l2.txt > > > > diff --git a/Documentation/devicetree/bindings/media/v4l2.txt b/Documentation/devicetree/bindings/media/v4l2.txt > > new file mode 100644 > > index 0000000..b8b3f41 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/media/v4l2.txt > > @@ -0,0 +1,162 @@ > > +Video4Linux Version 2 (V4L2) > > DT describes the h/w, but V4L2 is Linux specific. I think the binding > looks pretty good in terms of it is describing the h/w and not V4L2 > components or settings. So in this case it's really just the name of the > file and title I have issue with. Hm, I see your point, then, I guess, you'd also like the file name changed. What should we use then? Just "video?" But there's already a whole directory Documentation/devicetree/bindings/video dedicated to graphics output (drm, fbdev). "video-camera" or "video-capture?" But this file shall also be describing video output. Use "video.txt" and describe inside what exactly this file is for? > > One other comment below: > > > + > > +General concept > > +--------------- > > + > > +Video pipelines 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 "port" child DT nodes. > > +Configuration of a port depends on other devices participating in the data > > +transfer and is described by "link" DT nodes, specified as children of the > > +"port" nodes: > > + > > +/foo { > > + port@0 { > > + link@0 { ... }; > > + link@1 { ... }; > > + }; > > + port@1 { ... }; > > +}; > > + > > +If a port can be configured to work with more than one other device on the same > > +bus, a "link" child DT node must be provided for each of them. If more than one > > +port is present on a device or more than one link is connected to a port, a > > +common scheme, using "#address-cells," "#size-cells" and "reg" properties is > > +used. > > + > > +Optional link properties: > > +- remote: phandle to the other endpoint link DT node. > > This name is a little vague. Perhaps "endpoint" would be better. "endpoint" can also refer to something local like in USB case. Maybe rather the description of the "remote" property should be improved? Thanks Guennadi > > Rob > > > +- slave-mode: a boolean property, run the link in slave mode. Default is master > > + mode. > > +- data-shift: on parallel data busses, if data-width is used to specify the > > + number of data lines, data-shift can be used to specify which data lines are > > + used, e.g. "data-width=<10>; data-shift=<2>;" means, that lines 9:2 are used. > > +- hsync-active: 1 or 0 for active-high or -low HSYNC signal polarity > > + respectively. > > +- vsync-active: ditto for VSYNC. Note, that if HSYNC and VSYNC polarities are > > + not specified, embedded synchronisation 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 pin. > > +- data-lanes: array of serial, e.g. MIPI CSI-2, data hardware lane numbers in > > + the ascending order, beginning with logical lane 0. > > +- clock-lanes: hardware lane number, used for the clock lane. > > +- clock-noncontinuous: a boolean property to allow MIPI CSI-2 non-continuous > > + clock mode. > > + > > +Example: > > + > > + 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>; > > + }; > > + }; > > + }; > > + > > + 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: link { > > + clock-lanes = <0>; > > + data-lanes = <1>, <2>; > > + remote = <&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: 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>; > > + }; > > + }; > > + }; > > > --- 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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 10/02/2012 09:33 AM, Guennadi Liakhovetski wrote: > Hi Rob > > On Tue, 2 Oct 2012, Rob Herring wrote: > >> On 09/27/2012 09:07 AM, Guennadi Liakhovetski wrote: >>> This patch adds a document, describing common V4L2 device tree bindings. >>> >>> Co-authored-by: Sylwester Nawrocki <s.nawrocki@samsung.com> >>> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de> >>> --- >>> Documentation/devicetree/bindings/media/v4l2.txt | 162 ++++++++++++++++++++++ >>> 1 files changed, 162 insertions(+), 0 deletions(-) >>> create mode 100644 Documentation/devicetree/bindings/media/v4l2.txt >>> >>> diff --git a/Documentation/devicetree/bindings/media/v4l2.txt b/Documentation/devicetree/bindings/media/v4l2.txt >>> new file mode 100644 >>> index 0000000..b8b3f41 >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/media/v4l2.txt >>> @@ -0,0 +1,162 @@ >>> +Video4Linux Version 2 (V4L2) >> >> DT describes the h/w, but V4L2 is Linux specific. I think the binding >> looks pretty good in terms of it is describing the h/w and not V4L2 >> components or settings. So in this case it's really just the name of the >> file and title I have issue with. > > Hm, I see your point, then, I guess, you'd also like the file name > changed. What should we use then? Just "video?" But there's already a > whole directory Documentation/devicetree/bindings/video dedicated to > graphics output (drm, fbdev). "video-camera" or "video-capture?" But this > file shall also be describing video output. Use "video.txt" and describe > inside what exactly this file is for? Video output will probably have a lot of overlap with the graphics side. How about video-interfaces.txt? > >> >> One other comment below: >> >>> + >>> +General concept >>> +--------------- >>> + >>> +Video pipelines 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 "port" child DT nodes. >>> +Configuration of a port depends on other devices participating in the data >>> +transfer and is described by "link" DT nodes, specified as children of the >>> +"port" nodes: >>> + >>> +/foo { >>> + port@0 { >>> + link@0 { ... }; >>> + link@1 { ... }; >>> + }; >>> + port@1 { ... }; >>> +}; >>> + >>> +If a port can be configured to work with more than one other device on the same >>> +bus, a "link" child DT node must be provided for each of them. If more than one >>> +port is present on a device or more than one link is connected to a port, a >>> +common scheme, using "#address-cells," "#size-cells" and "reg" properties is >>> +used. >>> + >>> +Optional link properties: >>> +- remote: phandle to the other endpoint link DT node. >> >> This name is a little vague. Perhaps "endpoint" would be better. > > "endpoint" can also refer to something local like in USB case. Maybe > rather the description of the "remote" property should be improved? > remote-endpoint? > Thanks > Guennadi > >> >> Rob >> >>> +- slave-mode: a boolean property, run the link in slave mode. Default is master >>> + mode. >>> +- data-shift: on parallel data busses, if data-width is used to specify the >>> + number of data lines, data-shift can be used to specify which data lines are >>> + used, e.g. "data-width=<10>; data-shift=<2>;" means, that lines 9:2 are used. >>> +- hsync-active: 1 or 0 for active-high or -low HSYNC signal polarity >>> + respectively. >>> +- vsync-active: ditto for VSYNC. Note, that if HSYNC and VSYNC polarities are >>> + not specified, embedded synchronisation 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 pin. >>> +- data-lanes: array of serial, e.g. MIPI CSI-2, data hardware lane numbers in >>> + the ascending order, beginning with logical lane 0. >>> +- clock-lanes: hardware lane number, used for the clock lane. >>> +- clock-noncontinuous: a boolean property to allow MIPI CSI-2 non-continuous >>> + clock mode. >>> + >>> +Example: >>> + >>> + 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>; >>> + }; >>> + }; >>> + }; >>> + >>> + 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: link { >>> + clock-lanes = <0>; >>> + data-lanes = <1>, <2>; >>> + remote = <&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: 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>; >>> + }; >>> + }; >>> + }; >>> >> > > --- > 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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 3 Oct 2012, Rob Herring wrote: > On 10/02/2012 09:33 AM, Guennadi Liakhovetski wrote: > > Hi Rob > > > > On Tue, 2 Oct 2012, Rob Herring wrote: > > > >> On 09/27/2012 09:07 AM, Guennadi Liakhovetski wrote: > >>> This patch adds a document, describing common V4L2 device tree bindings. > >>> > >>> Co-authored-by: Sylwester Nawrocki <s.nawrocki@samsung.com> > >>> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de> > >>> --- > >>> Documentation/devicetree/bindings/media/v4l2.txt | 162 ++++++++++++++++++++++ > >>> 1 files changed, 162 insertions(+), 0 deletions(-) > >>> create mode 100644 Documentation/devicetree/bindings/media/v4l2.txt > >>> > >>> diff --git a/Documentation/devicetree/bindings/media/v4l2.txt b/Documentation/devicetree/bindings/media/v4l2.txt > >>> new file mode 100644 > >>> index 0000000..b8b3f41 > >>> --- /dev/null > >>> +++ b/Documentation/devicetree/bindings/media/v4l2.txt > >>> @@ -0,0 +1,162 @@ > >>> +Video4Linux Version 2 (V4L2) > >> > >> DT describes the h/w, but V4L2 is Linux specific. I think the binding > >> looks pretty good in terms of it is describing the h/w and not V4L2 > >> components or settings. So in this case it's really just the name of the > >> file and title I have issue with. > > > > Hm, I see your point, then, I guess, you'd also like the file name > > changed. What should we use then? Just "video?" But there's already a > > whole directory Documentation/devicetree/bindings/video dedicated to > > graphics output (drm, fbdev). "video-camera" or "video-capture?" But this > > file shall also be describing video output. Use "video.txt" and describe > > inside what exactly this file is for? > > Video output will probably have a lot of overlap with the graphics side. > How about video-interfaces.txt? Hm, that's a bit too vague for me. Somewhere on the outskirts of my mind I'm still considering making just one standard for both V4L2 and fbdev / DRM? Just yesterday we were discussing some common properties with what is being proposed in http://www.mail-archive.com/linux-media@vger.kernel.org/index.html#53322 Still, I think, these two subsystems deserve two separate standards and should just try to re-use properties wherever that makes sense. video-stream seems a bit better, but this too is just a convention - talking about video cameras and TV output as video streaming devices and considering displays more static devices. In principle displays can be considered taking streaming data just as well as TV encoders. What if we just call this camera-tv.txt? > >> One other comment below: > >> > >>> + > >>> +General concept > >>> +--------------- > >>> + > >>> +Video pipelines 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 "port" child DT nodes. > >>> +Configuration of a port depends on other devices participating in the data > >>> +transfer and is described by "link" DT nodes, specified as children of the > >>> +"port" nodes: > >>> + > >>> +/foo { > >>> + port@0 { > >>> + link@0 { ... }; > >>> + link@1 { ... }; > >>> + }; > >>> + port@1 { ... }; > >>> +}; > >>> + > >>> +If a port can be configured to work with more than one other device on the same > >>> +bus, a "link" child DT node must be provided for each of them. If more than one > >>> +port is present on a device or more than one link is connected to a port, a > >>> +common scheme, using "#address-cells," "#size-cells" and "reg" properties is > >>> +used. > >>> + > >>> +Optional link properties: > >>> +- remote: phandle to the other endpoint link DT node. > >> > >> This name is a little vague. Perhaps "endpoint" would be better. > > > > "endpoint" can also refer to something local like in USB case. Maybe > > rather the description of the "remote" property should be improved? > > remote-endpoint? Sorry, I really don't want to pull in yet another term here. We've got ports and links already, now you're proposing to also use "endpoind." Until now everyone was happy with "remote," any more opinions on this? 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 majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri October 5 2012 11:43:27 Guennadi Liakhovetski wrote: > On Wed, 3 Oct 2012, Rob Herring wrote: > > > On 10/02/2012 09:33 AM, Guennadi Liakhovetski wrote: > > > Hi Rob > > > > > > On Tue, 2 Oct 2012, Rob Herring wrote: > > > > > >> On 09/27/2012 09:07 AM, Guennadi Liakhovetski wrote: > > >>> This patch adds a document, describing common V4L2 device tree bindings. > > >>> > > >>> Co-authored-by: Sylwester Nawrocki <s.nawrocki@samsung.com> > > >>> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de> > > >>> --- > > >>> Documentation/devicetree/bindings/media/v4l2.txt | 162 ++++++++++++++++++++++ > > >>> 1 files changed, 162 insertions(+), 0 deletions(-) > > >>> create mode 100644 Documentation/devicetree/bindings/media/v4l2.txt > > >>> > > >>> diff --git a/Documentation/devicetree/bindings/media/v4l2.txt b/Documentation/devicetree/bindings/media/v4l2.txt > > >>> new file mode 100644 > > >>> index 0000000..b8b3f41 > > >>> --- /dev/null > > >>> +++ b/Documentation/devicetree/bindings/media/v4l2.txt > > >>> @@ -0,0 +1,162 @@ > > >>> +Video4Linux Version 2 (V4L2) > > >> > > >> DT describes the h/w, but V4L2 is Linux specific. I think the binding > > >> looks pretty good in terms of it is describing the h/w and not V4L2 > > >> components or settings. So in this case it's really just the name of the > > >> file and title I have issue with. > > > > > > Hm, I see your point, then, I guess, you'd also like the file name > > > changed. What should we use then? Just "video?" But there's already a > > > whole directory Documentation/devicetree/bindings/video dedicated to > > > graphics output (drm, fbdev). "video-camera" or "video-capture?" But this > > > file shall also be describing video output. Use "video.txt" and describe > > > inside what exactly this file is for? > > > > Video output will probably have a lot of overlap with the graphics side. > > How about video-interfaces.txt? > > Hm, that's a bit too vague for me. Somewhere on the outskirts of my mind > I'm still considering making just one standard for both V4L2 and fbdev / > DRM? Just yesterday we were discussing some common properties with what is > being proposed in > > http://www.mail-archive.com/linux-media@vger.kernel.org/index.html#53322 > > Still, I think, these two subsystems deserve two separate standards and > should just try to re-use properties wherever that makes sense. > video-stream seems a bit better, but this too is just a convention - > talking about video cameras and TV output as video streaming devices and > considering displays more static devices. In principle displays can be > considered taking streaming data just as well as TV encoders. What if we > just call this camera-tv.txt? > > > >> One other comment below: > > >> > > >>> + > > >>> +General concept > > >>> +--------------- > > >>> + > > >>> +Video pipelines 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 "port" child DT nodes. > > >>> +Configuration of a port depends on other devices participating in the data > > >>> +transfer and is described by "link" DT nodes, specified as children of the > > >>> +"port" nodes: > > >>> + > > >>> +/foo { > > >>> + port@0 { > > >>> + link@0 { ... }; > > >>> + link@1 { ... }; > > >>> + }; > > >>> + port@1 { ... }; > > >>> +}; > > >>> + > > >>> +If a port can be configured to work with more than one other device on the same > > >>> +bus, a "link" child DT node must be provided for each of them. If more than one > > >>> +port is present on a device or more than one link is connected to a port, a > > >>> +common scheme, using "#address-cells," "#size-cells" and "reg" properties is > > >>> +used. > > >>> + > > >>> +Optional link properties: > > >>> +- remote: phandle to the other endpoint link DT node. > > >> > > >> This name is a little vague. Perhaps "endpoint" would be better. > > > > > > "endpoint" can also refer to something local like in USB case. Maybe > > > rather the description of the "remote" property should be improved? > > > > remote-endpoint? > > Sorry, I really don't want to pull in yet another term here. We've got > ports and links already, now you're proposing to also use "endpoind." > Until now everyone was happy with "remote," any more opinions on this? Actually, when I was reviewing the patch series today I got confused as well by 'remote'. What about 'remote-link'? And v4l2_of_get_remote() can be renamed to v4l2_of_get_remote_link() which I think is a lot clearer. The text can be improved as well since this: - remote: phandle to the other endpoint link DT node. is a bit vague. How about: - remote-link: phandle to the remote end of this link. Regards, Hans -- 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
On Fri, 5 Oct 2012, Hans Verkuil wrote: > On Fri October 5 2012 11:43:27 Guennadi Liakhovetski wrote: > > On Wed, 3 Oct 2012, Rob Herring wrote: > > > > > On 10/02/2012 09:33 AM, Guennadi Liakhovetski wrote: > > > > Hi Rob > > > > > > > > On Tue, 2 Oct 2012, Rob Herring wrote: > > > > > > > >> On 09/27/2012 09:07 AM, Guennadi Liakhovetski wrote: [snip] > > > >>> +Optional link properties: > > > >>> +- remote: phandle to the other endpoint link DT node. > > > >> > > > >> This name is a little vague. Perhaps "endpoint" would be better. > > > > > > > > "endpoint" can also refer to something local like in USB case. Maybe > > > > rather the description of the "remote" property should be improved? > > > > > > remote-endpoint? > > > > Sorry, I really don't want to pull in yet another term here. We've got > > ports and links already, now you're proposing to also use "endpoind." > > Until now everyone was happy with "remote," any more opinions on this? > > Actually, when I was reviewing the patch series today I got confused as > well by 'remote'. What about 'remote-link'? Yes, I was thinking about this one too, it looks a bit clumsy, but it does make it clearer, what is meant. > And v4l2_of_get_remote() can be renamed to v4l2_of_get_remote_link() which > I think is a lot clearer. > > The text can be improved as well since this: > > - remote: phandle to the other endpoint link DT node. > > is a bit vague. How about: > > - remote-link: phandle to the remote end of this link. Looks good to me. 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 majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Guennadi, Some comments inline. On Thu, Sep 27, 2012 at 04:07:23PM +0200, Guennadi Liakhovetski wrote: > This patch adds a document, describing common V4L2 device tree bindings. > > Co-authored-by: Sylwester Nawrocki <s.nawrocki@samsung.com> > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de> > --- > Documentation/devicetree/bindings/media/v4l2.txt | 162 ++++++++++++++++++++++ > 1 files changed, 162 insertions(+), 0 deletions(-) > create mode 100644 Documentation/devicetree/bindings/media/v4l2.txt > > diff --git a/Documentation/devicetree/bindings/media/v4l2.txt b/Documentation/devicetree/bindings/media/v4l2.txt > new file mode 100644 > index 0000000..b8b3f41 > --- /dev/null > +++ b/Documentation/devicetree/bindings/media/v4l2.txt > @@ -0,0 +1,162 @@ > +Video4Linux Version 2 (V4L2) > + > +General concept > +--------------- > + > +Video pipelines 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 "port" child DT nodes. > +Configuration of a port depends on other devices participating in the data > +transfer and is described by "link" DT nodes, specified as children of the > +"port" nodes: > + > +/foo { > + port@0 { > + link@0 { ... }; > + link@1 { ... }; > + }; > + port@1 { ... }; > +}; > + > +If a port can be configured to work with more than one other device on the same > +bus, a "link" child DT node must be provided for each of them. If more than one > +port is present on a device or more than one link is connected to a port, a > +common scheme, using "#address-cells," "#size-cells" and "reg" properties is > +used. > + > +Optional link properties: > +- remote: phandle to the other endpoint link DT node. > +- slave-mode: a boolean property, run the link in slave mode. Default is master > + mode. > +- data-shift: on parallel data busses, if data-width is used to specify the > + number of data lines, data-shift can be used to specify which data lines are > + used, e.g. "data-width=<10>; data-shift=<2>;" means, that lines 9:2 are used. > +- hsync-active: 1 or 0 for active-high or -low HSYNC signal polarity > + respectively. > +- vsync-active: ditto for VSYNC. Note, that if HSYNC and VSYNC polarities are > + not specified, embedded synchronisation 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 pin. > +- data-lanes: array of serial, e.g. MIPI CSI-2, data hardware lane numbers in > + the ascending order, beginning with logical lane 0. > +- clock-lanes: hardware lane number, used for the clock lane. > +- clock-noncontinuous: a boolean property to allow MIPI CSI-2 non-continuous > + clock mode. > + > +Example: > + > + 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? > + 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. 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. I am not sure we really want to have these circular phandles here. I think phandles in the direction of data flow should be enough. I mean the devices probe in arbitrary order anyway and the SoC camera core must keep track of the possible sensor/csi combinations anyway, so it should be enough to make the connections in a single direction. Sascha
Hi Sascha On Fri, 5 Oct 2012, Sascha Hauer wrote: > Hi Guennadi, > > Some comments inline. > > > On Thu, Sep 27, 2012 at 04:07:23PM +0200, Guennadi Liakhovetski wrote: > > This patch adds a document, describing common V4L2 device tree bindings. > > > > Co-authored-by: Sylwester Nawrocki <s.nawrocki@samsung.com> > > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de> > > --- > > Documentation/devicetree/bindings/media/v4l2.txt | 162 ++++++++++++++++++++++ > > 1 files changed, 162 insertions(+), 0 deletions(-) > > create mode 100644 Documentation/devicetree/bindings/media/v4l2.txt > > > > diff --git a/Documentation/devicetree/bindings/media/v4l2.txt b/Documentation/devicetree/bindings/media/v4l2.txt > > new file mode 100644 > > index 0000000..b8b3f41 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/media/v4l2.txt > > @@ -0,0 +1,162 @@ > > +Video4Linux Version 2 (V4L2) > > + > > +General concept > > +--------------- > > + > > +Video pipelines 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 "port" child DT nodes. > > +Configuration of a port depends on other devices participating in the data > > +transfer and is described by "link" DT nodes, specified as children of the > > +"port" nodes: > > + > > +/foo { > > + port@0 { > > + link@0 { ... }; > > + link@1 { ... }; > > + }; > > + port@1 { ... }; > > +}; > > + > > +If a port can be configured to work with more than one other device on the same > > +bus, a "link" child DT node must be provided for each of them. If more than one > > +port is present on a device or more than one link is connected to a port, a > > +common scheme, using "#address-cells," "#size-cells" and "reg" properties is > > +used. > > + > > +Optional link properties: > > +- remote: phandle to the other endpoint link DT node. > > +- slave-mode: a boolean property, run the link in slave mode. Default is master > > + mode. > > +- data-shift: on parallel data busses, if data-width is used to specify the > > + number of data lines, data-shift can be used to specify which data lines are > > + used, e.g. "data-width=<10>; data-shift=<2>;" means, that lines 9:2 are used. > > +- hsync-active: 1 or 0 for active-high or -low HSYNC signal polarity > > + respectively. > > +- vsync-active: ditto for VSYNC. Note, that if HSYNC and VSYNC polarities are > > + not specified, embedded synchronisation 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 pin. > > +- data-lanes: array of serial, e.g. MIPI CSI-2, data hardware lane numbers in > > + the ascending order, beginning with logical lane 0. > > +- clock-lanes: hardware lane number, used for the clock lane. > > +- clock-noncontinuous: a boolean property to allow MIPI CSI-2 non-continuous > > + clock mode. > > + > > +Example: > > + > > + 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. > > + 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. > 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. > 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. > I > think phandles in the direction of data flow should be enough. I mean > the devices probe in arbitrary order anyway and the SoC camera core must > keep track of the possible sensor/csi combinations anyway, so it should > be enough to make the connections in a single direction. 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 majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 > > > 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. > > > 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
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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 10/02/2012 08:33 AM, Guennadi Liakhovetski wrote: > On Tue, 2 Oct 2012, Rob Herring wrote: >> On 09/27/2012 09:07 AM, Guennadi Liakhovetski wrote: >>> This patch adds a document, describing common V4L2 device tree bindings. >>> diff --git a/Documentation/devicetree/bindings/media/v4l2.txt b/Documentation/devicetree/bindings/media/v4l2.txt >> One other comment below: >> >>> + >>> +General concept >>> +--------------- >>> + >>> +Video pipelines 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 "port" child DT nodes. >>> +Configuration of a port depends on other devices participating in the data >>> +transfer and is described by "link" DT nodes, specified as children of the >>> +"port" nodes: >>> + >>> +/foo { >>> + port@0 { >>> + link@0 { ... }; >>> + link@1 { ... }; >>> + }; >>> + port@1 { ... }; >>> +}; >>> + >>> +If a port can be configured to work with more than one other device on the same >>> +bus, a "link" child DT node must be provided for each of them. If more than one >>> +port is present on a device or more than one link is connected to a port, a >>> +common scheme, using "#address-cells," "#size-cells" and "reg" properties is >>> +used. >>> + >>> +Optional link properties: >>> +- remote: phandle to the other endpoint link DT node. >> >> This name is a little vague. Perhaps "endpoint" would be better. > > "endpoint" can also refer to something local like in USB case. Maybe > rather the description of the "remote" property should be improved? The documentation doesn't show up in all the .dts files that use it; it might be useful to try and make the .dts file as obviously readable as possible. Perhaps "remote-port" or "connected-port" would be sufficiently descriptive. (and yes, I know I'm probably bike-shedding now). -- 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
On 09/27/2012 08:07 AM, Guennadi Liakhovetski wrote: > This patch adds a document, describing common V4L2 device tree bindings. > > Co-authored-by: Sylwester Nawrocki <s.nawrocki@samsung.com> > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de> I think this looks reasonable now, touch wood:-) I guess I won't ack the binding until the v4l2 -> something-generic rename happens, but I don't see anything stopping me from doing so once the rename happens. -- 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
On Monday 08 October 2012 14:00:38 Stephen Warren wrote: > On 10/02/2012 08:33 AM, Guennadi Liakhovetski wrote: > > On Tue, 2 Oct 2012, Rob Herring wrote: > >> On 09/27/2012 09:07 AM, Guennadi Liakhovetski wrote: > >>> This patch adds a document, describing common V4L2 device tree bindings. > >>> > >>> diff --git a/Documentation/devicetree/bindings/media/v4l2.txt > >>> b/Documentation/devicetree/bindings/media/v4l2.txt>> > >> One other comment below: > >>> + > >>> +General concept > >>> +--------------- > >>> + > >>> +Video pipelines 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 "port" child DT > >>> nodes. +Configuration of a port depends on other devices participating > >>> in the data +transfer and is described by "link" DT nodes, specified as > >>> children of the +"port" nodes: > >>> + > >>> +/foo { > >>> + port@0 { > >>> + link@0 { ... }; > >>> + link@1 { ... }; > >>> + }; > >>> + port@1 { ... }; > >>> +}; > >>> + > >>> +If a port can be configured to work with more than one other device on > >>> the same +bus, a "link" child DT node must be provided for each of > >>> them. If more than one +port is present on a device or more than one > >>> link is connected to a port, a +common scheme, using "#address-cells," > >>> "#size-cells" and "reg" properties is +used. > >>> + > >>> +Optional link properties: > >>> +- remote: phandle to the other endpoint link DT node. > >> > >> This name is a little vague. Perhaps "endpoint" would be better. > > > > "endpoint" can also refer to something local like in USB case. Maybe > > rather the description of the "remote" property should be improved? > > The documentation doesn't show up in all the .dts files that use it; it > might be useful to try and make the .dts file as obviously readable as > possible. > > Perhaps "remote-port" or "connected-port" would be sufficiently descriptive. I like remote-port better than the already proposed remote-link. > (and yes, I know I'm probably bike-shedding now).
On Mon, 8 Oct 2012, Laurent Pinchart wrote: > On Monday 08 October 2012 14:00:38 Stephen Warren wrote: > > On 10/02/2012 08:33 AM, Guennadi Liakhovetski wrote: > > > On Tue, 2 Oct 2012, Rob Herring wrote: > > >> On 09/27/2012 09:07 AM, Guennadi Liakhovetski wrote: > > >>> This patch adds a document, describing common V4L2 device tree bindings. > > >>> > > >>> diff --git a/Documentation/devicetree/bindings/media/v4l2.txt > > >>> b/Documentation/devicetree/bindings/media/v4l2.txt>> > > >> One other comment below: > > >>> + > > >>> +General concept > > >>> +--------------- > > >>> + > > >>> +Video pipelines 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 "port" child DT > > >>> nodes. +Configuration of a port depends on other devices participating > > >>> in the data +transfer and is described by "link" DT nodes, specified as > > >>> children of the +"port" nodes: > > >>> + > > >>> +/foo { > > >>> + port@0 { > > >>> + link@0 { ... }; > > >>> + link@1 { ... }; > > >>> + }; > > >>> + port@1 { ... }; > > >>> +}; > > >>> + > > >>> +If a port can be configured to work with more than one other device on > > >>> the same +bus, a "link" child DT node must be provided for each of > > >>> them. If more than one +port is present on a device or more than one > > >>> link is connected to a port, a +common scheme, using "#address-cells," > > >>> "#size-cells" and "reg" properties is +used. > > >>> + > > >>> +Optional link properties: > > >>> +- remote: phandle to the other endpoint link DT node. > > >> > > >> This name is a little vague. Perhaps "endpoint" would be better. > > > > > > "endpoint" can also refer to something local like in USB case. Maybe > > > rather the description of the "remote" property should be improved? > > > > The documentation doesn't show up in all the .dts files that use it; it > > might be useful to try and make the .dts file as obviously readable as > > possible. > > > > Perhaps "remote-port" or "connected-port" would be sufficiently descriptive. > > I like remote-port better than the already proposed remote-link. Yes, remote-port sounds better, than remote-link, but might be more difficult to correlate with the fact, that the phandle value of this property points to a link DT node, and not to a port. Thanks Guennadi > > (and yes, I know I'm probably bike-shedding now). > > -- > Regards, > > Laurent Pinchart > --- 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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon 8 October 2012 23:14:01 Guennadi Liakhovetski wrote: > On Mon, 8 Oct 2012, Laurent Pinchart wrote: > > > On Monday 08 October 2012 14:00:38 Stephen Warren wrote: > > > On 10/02/2012 08:33 AM, Guennadi Liakhovetski wrote: > > > > On Tue, 2 Oct 2012, Rob Herring wrote: > > > >> On 09/27/2012 09:07 AM, Guennadi Liakhovetski wrote: > > > >>> This patch adds a document, describing common V4L2 device tree bindings. > > > >>> > > > >>> diff --git a/Documentation/devicetree/bindings/media/v4l2.txt > > > >>> b/Documentation/devicetree/bindings/media/v4l2.txt>> > > > >> One other comment below: > > > >>> + > > > >>> +General concept > > > >>> +--------------- > > > >>> + > > > >>> +Video pipelines 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 "port" child DT > > > >>> nodes. +Configuration of a port depends on other devices participating > > > >>> in the data +transfer and is described by "link" DT nodes, specified as > > > >>> children of the +"port" nodes: > > > >>> + > > > >>> +/foo { > > > >>> + port@0 { > > > >>> + link@0 { ... }; > > > >>> + link@1 { ... }; > > > >>> + }; > > > >>> + port@1 { ... }; > > > >>> +}; > > > >>> + > > > >>> +If a port can be configured to work with more than one other device on > > > >>> the same +bus, a "link" child DT node must be provided for each of > > > >>> them. If more than one +port is present on a device or more than one > > > >>> link is connected to a port, a +common scheme, using "#address-cells," > > > >>> "#size-cells" and "reg" properties is +used. > > > >>> + > > > >>> +Optional link properties: > > > >>> +- remote: phandle to the other endpoint link DT node. > > > >> > > > >> This name is a little vague. Perhaps "endpoint" would be better. > > > > > > > > "endpoint" can also refer to something local like in USB case. Maybe > > > > rather the description of the "remote" property should be improved? > > > > > > The documentation doesn't show up in all the .dts files that use it; it > > > might be useful to try and make the .dts file as obviously readable as > > > possible. > > > > > > Perhaps "remote-port" or "connected-port" would be sufficiently descriptive. > > > > I like remote-port better than the already proposed remote-link. > > Yes, remote-port sounds better, than remote-link, but might be more > difficult to correlate with the fact, that the phandle value of this > property points to a link DT node, and not to a port. I first thought of remote-port as well, but it is just weird that it points to a link node. I seem to remember that 'link' was called 'pad' initially, but people didn't like that due to possible confusion with other meanings of that word. The problem with the word 'link' is that it doesn't describe a link but just one endpoint of a link. Is it an idea to rename 'link' to 'endpoint' and 'remote' to 'remote-endpoint'? So a port has endpoints, and each endpoint has a remote-endpoint property. Regards, Hans -- 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
On Tue, 9 Oct 2012, Hans Verkuil wrote: > On Mon 8 October 2012 23:14:01 Guennadi Liakhovetski wrote: > > On Mon, 8 Oct 2012, Laurent Pinchart wrote: > > > > > On Monday 08 October 2012 14:00:38 Stephen Warren wrote: > > > > On 10/02/2012 08:33 AM, Guennadi Liakhovetski wrote: > > > > > On Tue, 2 Oct 2012, Rob Herring wrote: > > > > >> On 09/27/2012 09:07 AM, Guennadi Liakhovetski wrote: > > > > >>> This patch adds a document, describing common V4L2 device tree bindings. > > > > >>> > > > > >>> diff --git a/Documentation/devicetree/bindings/media/v4l2.txt > > > > >>> b/Documentation/devicetree/bindings/media/v4l2.txt>> > > > > >> One other comment below: > > > > >>> + > > > > >>> +General concept > > > > >>> +--------------- > > > > >>> + > > > > >>> +Video pipelines 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 "port" child DT > > > > >>> nodes. +Configuration of a port depends on other devices participating > > > > >>> in the data +transfer and is described by "link" DT nodes, specified as > > > > >>> children of the +"port" nodes: > > > > >>> + > > > > >>> +/foo { > > > > >>> + port@0 { > > > > >>> + link@0 { ... }; > > > > >>> + link@1 { ... }; > > > > >>> + }; > > > > >>> + port@1 { ... }; > > > > >>> +}; > > > > >>> + > > > > >>> +If a port can be configured to work with more than one other device on > > > > >>> the same +bus, a "link" child DT node must be provided for each of > > > > >>> them. If more than one +port is present on a device or more than one > > > > >>> link is connected to a port, a +common scheme, using "#address-cells," > > > > >>> "#size-cells" and "reg" properties is +used. > > > > >>> + > > > > >>> +Optional link properties: > > > > >>> +- remote: phandle to the other endpoint link DT node. > > > > >> > > > > >> This name is a little vague. Perhaps "endpoint" would be better. > > > > > > > > > > "endpoint" can also refer to something local like in USB case. Maybe > > > > > rather the description of the "remote" property should be improved? > > > > > > > > The documentation doesn't show up in all the .dts files that use it; it > > > > might be useful to try and make the .dts file as obviously readable as > > > > possible. > > > > > > > > Perhaps "remote-port" or "connected-port" would be sufficiently descriptive. > > > > > > I like remote-port better than the already proposed remote-link. > > > > Yes, remote-port sounds better, than remote-link, but might be more > > difficult to correlate with the fact, that the phandle value of this > > property points to a link DT node, and not to a port. > > I first thought of remote-port as well, but it is just weird that it points to > a link node. > > I seem to remember that 'link' was called 'pad' initially, but people didn't > like that due to possible confusion with other meanings of that word. > > The problem with the word 'link' is that it doesn't describe a link but just > one endpoint of a link. > > Is it an idea to rename 'link' to 'endpoint' and 'remote' to 'remote-endpoint'? > > So a port has endpoints, and each endpoint has a remote-endpoint property. I'm fine with that. Thanks Guennadi > Regards, > > Hans --- 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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Guennadi, On Mon, Oct 08, 2012 at 09:58:58AM +0200, Guennadi Liakhovetski wrote: > On Fri, 5 Oct 2012, Sascha Hauer wrote: > > > On Fri, Oct 05, 2012 at 05:41:00PM +0200, Guennadi Liakhovetski wrote: > > > Hi Sascha > > > > > > > > > > > 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? :) Yes, thanks. We thought and disucssed about this devicetree binding quite much the last days. Finally I understood it and I must say that I like it. I think more prosa to explain the general concept would be good in the binding doc. Mark, when do we get the same for aSoC? ;) Sascha
On Wed, Oct 10, 2012 at 10:40:06AM +0200, Sascha Hauer wrote:
> Mark, when do we get the same for aSoC? ;)
Well, I'm unlikely to work on it as I don't have any systems which can
boot with device tree.
The big, big problem you've got doing this is lots of dynamic changes at
runtime and in general complicated systems. It's trivial to describe
the physical links but they don't provide anything like enough
information to use things. Quite frankly I'm not sure device tree is a
useful investment of time for advanced audio systems anyway, it's really
not solving any problems people actually have.
--
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
On Wed, Oct 10, 2012 at 05:51:27PM +0900, Mark Brown wrote: > On Wed, Oct 10, 2012 at 10:40:06AM +0200, Sascha Hauer wrote: > > > Mark, when do we get the same for aSoC? ;) > > Well, I'm unlikely to work on it as I don't have any systems which can > boot with device tree. If it's only that I'm sure we could spare a i.MX53 LOCO ;) > > The big, big problem you've got doing this is lots of dynamic changes at > runtime and in general complicated systems. It's trivial to describe > the physical links but they don't provide anything like enough > information to use things. Quite frankly I'm not sure device tree is a > useful investment of time for advanced audio systems anyway, it's really > not solving any problems people actually have. Right now the i.MX audmux binding is only enough to say which ports should be connected, but not which format should be used. Just today we had the problem where a codec has two DAIs and wanted to add the information on which port our SSI unit is connected to the devicetree. So I think it's worthwile to do, but that would be a big big task... Sascha
On Wed, Oct 10, 2012 at 11:21:15AM +0200, Sascha Hauer wrote: > On Wed, Oct 10, 2012 at 05:51:27PM +0900, Mark Brown wrote: > > Well, I'm unlikely to work on it as I don't have any systems which can > > boot with device tree. > If it's only that I'm sure we could spare a i.MX53 LOCO ;) Well, something with Wolfson hardware would be helpful. > > The big, big problem you've got doing this is lots of dynamic changes at > > runtime and in general complicated systems. It's trivial to describe > > the physical links but they don't provide anything like enough > > information to use things. Quite frankly I'm not sure device tree is a > > useful investment of time for advanced audio systems anyway, it's really > > not solving any problems people actually have. > Right now the i.MX audmux binding is only enough to say which ports > should be connected, but not which format should be used. Just today Why should that be in DT? For most things it's either fixed by the hardware or makes no odds. > we had the problem where a codec has two DAIs and wanted to add the > information on which port our SSI unit is connected to the devicetree. There's nothing stopping you doing that right now, the existing DT > So I think it's worthwile to do, but that would be a big big task... For simple devices there's already stuff there and it's not hard to add new machine bindings, it's trying to cover the general case that's far too much effort. -- 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/v4l2.txt b/Documentation/devicetree/bindings/media/v4l2.txt new file mode 100644 index 0000000..b8b3f41 --- /dev/null +++ b/Documentation/devicetree/bindings/media/v4l2.txt @@ -0,0 +1,162 @@ +Video4Linux Version 2 (V4L2) + +General concept +--------------- + +Video pipelines 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 "port" child DT nodes. +Configuration of a port depends on other devices participating in the data +transfer and is described by "link" DT nodes, specified as children of the +"port" nodes: + +/foo { + port@0 { + link@0 { ... }; + link@1 { ... }; + }; + port@1 { ... }; +}; + +If a port can be configured to work with more than one other device on the same +bus, a "link" child DT node must be provided for each of them. If more than one +port is present on a device or more than one link is connected to a port, a +common scheme, using "#address-cells," "#size-cells" and "reg" properties is +used. + +Optional link properties: +- remote: phandle to the other endpoint link DT node. +- slave-mode: a boolean property, run the link in slave mode. Default is master + mode. +- data-shift: on parallel data busses, if data-width is used to specify the + number of data lines, data-shift can be used to specify which data lines are + used, e.g. "data-width=<10>; data-shift=<2>;" means, that lines 9:2 are used. +- hsync-active: 1 or 0 for active-high or -low HSYNC signal polarity + respectively. +- vsync-active: ditto for VSYNC. Note, that if HSYNC and VSYNC polarities are + not specified, embedded synchronisation 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 pin. +- data-lanes: array of serial, e.g. MIPI CSI-2, data hardware lane numbers in + the ascending order, beginning with logical lane 0. +- clock-lanes: hardware lane number, used for the clock lane. +- clock-noncontinuous: a boolean property to allow MIPI CSI-2 non-continuous + clock mode. + +Example: + + 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>; + }; + }; + }; + + 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: link { + clock-lanes = <0>; + data-lanes = <1>, <2>; + remote = <&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: 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>; + }; + }; + };
This patch adds a document, describing common V4L2 device tree bindings. Co-authored-by: Sylwester Nawrocki <s.nawrocki@samsung.com> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de> --- Documentation/devicetree/bindings/media/v4l2.txt | 162 ++++++++++++++++++++++ 1 files changed, 162 insertions(+), 0 deletions(-) create mode 100644 Documentation/devicetree/bindings/media/v4l2.txt