| Message ID | 1417170507-11172-5-git-send-email-josh.wu@atmel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Josh, Thank you for the patch. On Friday 28 November 2014 18:28:27 Josh Wu wrote: > Add the document for ov2640 dt. > > Cc: devicetree@vger.kernel.org > Signed-off-by: Josh Wu <josh.wu@atmel.com> > --- > .../devicetree/bindings/media/i2c/ov2640.txt | 43 +++++++++++++++++++ > 1 file changed, 43 insertions(+) > create mode 100644 Documentation/devicetree/bindings/media/i2c/ov2640.txt > > diff --git a/Documentation/devicetree/bindings/media/i2c/ov2640.txt > b/Documentation/devicetree/bindings/media/i2c/ov2640.txt new file mode > 100644 > index 0000000..adec147 > --- /dev/null > +++ b/Documentation/devicetree/bindings/media/i2c/ov2640.txt > @@ -0,0 +1,43 @@ > +* Omnivision ov2640 CMOS sensor > + > +The Omnivision OV2640 sensor support multiple resolutions output, such as > +CIF, SVGA, UXGA. It also can support YUV422/420, RGB565/555 or raw RGB > +output format. > + > +Required Properties : > +- compatible : Must be "omnivision,ov2640" The usual practice is to use the company's stock ticker as a prefix. In this case the compatible string would be "ovti,ov2640". You need to add the prefix to Documentation/devicetree/bindings/vendor-prefixes.txt. > +- reset-gpio : reset pin > +- power-down-gpio : power down pin That should be reset-gpios and power-down-gpios, even if there's a single GPIO. Furthermore, given that the power down pin is named PWDN you might want to name the property pwdn-gpios. The reset and pwdn signals won't be connected on all boards, so the two properties should be optional. > +Optional Properties: > +- clocks : reference master clock, if using external fixed clock, > you > + no need to have such property. The clock is required by the chip, so even when using an external fixed clock the property should be present, and reference a fixed clock node. The clocks and clock-names properties should thus be mandatory. > +- clock-names : Must be "mck", it means the master clock for ov2640. The clock input is named xvclk in the datasheet, you should use the same name here. > + > +For further reading of port node refer > Documentation/devicetree/bindings/media/ > +video-interfaces.txt. Even if you reference that document you should still mention what port node(s) these bindings require. Something like the following text should be enough. "The device node must contain one 'port' child node for its digital output video port, in accordance with the video interface bindings defined in Documentation/devicetree/bindings/media/video-interfaces.txt." > + > +Example: > + > + i2c1: i2c@f0018000 { > + ov2640: camera@0x30 { > + compatible = "omnivision,ov2640"; > + reg = <0x30>; > + > + ... ... No need for an ellipsis, what are you trying to hide ? :-) > + > + reset-gpio = <&pioE 24 GPIO_ACTIVE_HIGH>; > + power-down-gpio = <&pioE 29 GPIO_ACTIVE_HIGH>; > + > + /* use pck1 for the master clock of ov2640 */ I think you can drop the comment. > + clocks = <&pck1>; > + clock-names = "mck"; > + > + port { > + ov2640_0: endpoint { > + remote-endpoint = <&isi_0>; > + bus-width = <8>; > + }; > + }; > + }; > + };
Hi, Laurent On 12/2/2014 6:14 AM, Laurent Pinchart wrote: > Hi Josh, > > Thank you for the patch. > > On Friday 28 November 2014 18:28:27 Josh Wu wrote: >> Add the document for ov2640 dt. >> >> Cc: devicetree@vger.kernel.org >> Signed-off-by: Josh Wu <josh.wu@atmel.com> >> --- >> .../devicetree/bindings/media/i2c/ov2640.txt | 43 +++++++++++++++++++ >> 1 file changed, 43 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/media/i2c/ov2640.txt >> >> diff --git a/Documentation/devicetree/bindings/media/i2c/ov2640.txt >> b/Documentation/devicetree/bindings/media/i2c/ov2640.txt new file mode >> 100644 >> index 0000000..adec147 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/media/i2c/ov2640.txt >> @@ -0,0 +1,43 @@ >> +* Omnivision ov2640 CMOS sensor >> + >> +The Omnivision OV2640 sensor support multiple resolutions output, such as >> +CIF, SVGA, UXGA. It also can support YUV422/420, RGB565/555 or raw RGB >> +output format. >> + >> +Required Properties : >> +- compatible : Must be "omnivision,ov2640" > The usual practice is to use the company's stock ticker as a prefix. In this > case the compatible string would be "ovti,ov2640". You need to add the prefix > to Documentation/devicetree/bindings/vendor-prefixes.txt. > >> +- reset-gpio : reset pin >> +- power-down-gpio : power down pin > That should be reset-gpios and power-down-gpios, even if there's a single > GPIO. Furthermore, given that the power down pin is named PWDN you might want > to name the property pwdn-gpios. > > The reset and pwdn signals won't be connected on all boards, so the two > properties should be optional. > >> +Optional Properties: >> +- clocks : reference master clock, if using external fixed clock, >> you >> + no need to have such property. > The clock is required by the chip, so even when using an external fixed clock > the property should be present, and reference a fixed clock node. The clocks > and clock-names properties should thus be mandatory. > >> +- clock-names : Must be "mck", it means the master clock for ov2640. > The clock input is named xvclk in the datasheet, you should use the same name > here. > >> + >> +For further reading of port node refer >> Documentation/devicetree/bindings/media/ >> +video-interfaces.txt. > Even if you reference that document you should still mention what port node(s) > these bindings require. Something like the following text should be enough. > > "The device node must contain one 'port' child node for its digital output > video port, in accordance with the video interface bindings defined in > Documentation/devicetree/bindings/media/video-interfaces.txt." > >> + >> +Example: >> + >> + i2c1: i2c@f0018000 { >> + ov2640: camera@0x30 { >> + compatible = "omnivision,ov2640"; >> + reg = <0x30>; >> + >> + ... ... > No need for an ellipsis, what are you trying to hide ? :-) I just emit the pinctrl property here. Will add it back. > >> + >> + reset-gpio = <&pioE 24 GPIO_ACTIVE_HIGH>; >> + power-down-gpio = <&pioE 29 GPIO_ACTIVE_HIGH>; >> + >> + /* use pck1 for the master clock of ov2640 */ > I think you can drop the comment. I will send a new version which will fix all above. Thanks for your time to review the patch. That help me a lot. Best Regards, Josh Wu > >> + clocks = <&pck1>; >> + clock-names = "mck"; >> + >> + port { >> + ov2640_0: endpoint { >> + remote-endpoint = <&isi_0>; >> + bus-width = <8>; >> + }; >> + }; >> + }; >> + };
diff --git a/Documentation/devicetree/bindings/media/i2c/ov2640.txt b/Documentation/devicetree/bindings/media/i2c/ov2640.txt new file mode 100644 index 0000000..adec147 --- /dev/null +++ b/Documentation/devicetree/bindings/media/i2c/ov2640.txt @@ -0,0 +1,43 @@ +* Omnivision ov2640 CMOS sensor + +The Omnivision OV2640 sensor support multiple resolutions output, such as +CIF, SVGA, UXGA. It also can support YUV422/420, RGB565/555 or raw RGB +output format. + +Required Properties : +- compatible : Must be "omnivision,ov2640" +- reset-gpio : reset pin +- power-down-gpio : power down pin + +Optional Properties: +- clocks : reference master clock, if using external fixed clock, you + no need to have such property. +- clock-names : Must be "mck", it means the master clock for ov2640. + +For further reading of port node refer Documentation/devicetree/bindings/media/ +video-interfaces.txt. + +Example: + + i2c1: i2c@f0018000 { + ov2640: camera@0x30 { + compatible = "omnivision,ov2640"; + reg = <0x30>; + + ... ... + + reset-gpio = <&pioE 24 GPIO_ACTIVE_HIGH>; + power-down-gpio = <&pioE 29 GPIO_ACTIVE_HIGH>; + + /* use pck1 for the master clock of ov2640 */ + clocks = <&pck1>; + clock-names = "mck"; + + port { + ov2640_0: endpoint { + remote-endpoint = <&isi_0>; + bus-width = <8>; + }; + }; + }; + };
Add the document for ov2640 dt. Cc: devicetree@vger.kernel.org Signed-off-by: Josh Wu <josh.wu@atmel.com> --- .../devicetree/bindings/media/i2c/ov2640.txt | 43 ++++++++++++++++++++++ 1 file changed, 43 insertions(+) create mode 100644 Documentation/devicetree/bindings/media/i2c/ov2640.txt