| Message ID | 20230129-ov2685-improvements-v4-2-e71985c5c848@z3ntu.xyz |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
| Series | Improvements for OmniVision OV2685 | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | warning | total: 0 errors, 2 warnings, 108 lines checked |
| robh/patch-applied | success | |
| robh/dtbs-check | warning | build log |
| robh/dt-meta-schema | success |
Hi Luca, Thanks for the patch. On Thu, Mar 23, 2023 at 06:57:50PM +0100, Luca Weiss wrote: > Convert the text-based dt-bindings to yaml. > > Changes from original txt: > * Take wording for various properties from other yaml bindings, this > removes e.g. volt amount from schema since it isn't really relevant > and the datasheet is a better source. > * Don't make reset-gpios a required property since it can be tied to > DOVDD instead. > > Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > Signed-off-by: Luca Weiss <luca@z3ntu.xyz> > --- > .../devicetree/bindings/media/i2c/ov2685.txt | 41 --------- > .../devicetree/bindings/media/i2c/ovti,ov2685.yaml | 101 +++++++++++++++++++++ > MAINTAINERS | 1 + > 3 files changed, 102 insertions(+), 41 deletions(-) > > diff --git a/Documentation/devicetree/bindings/media/i2c/ov2685.txt b/Documentation/devicetree/bindings/media/i2c/ov2685.txt > deleted file mode 100644 > index 625c4a8c0d53d..0000000000000 > --- a/Documentation/devicetree/bindings/media/i2c/ov2685.txt > +++ /dev/null > @@ -1,41 +0,0 @@ > -* Omnivision OV2685 MIPI CSI-2 sensor > - > -Required Properties: > -- compatible: shall be "ovti,ov2685" > -- clocks: reference to the xvclk input clock > -- clock-names: shall be "xvclk" > -- avdd-supply: Analog voltage supply, 2.8 volts > -- dovdd-supply: Digital I/O voltage supply, 1.8 volts > -- dvdd-supply: Digital core voltage supply, 1.8 volts > -- reset-gpios: Low active reset gpio > - > -The device node shall contain one 'port' child node with an > -'endpoint' subnode for its digital output video port, > -in accordance with the video interface bindings defined in > -Documentation/devicetree/bindings/media/video-interfaces.txt. > -The endpoint optional property 'data-lanes' shall be "<1>". > - > -Example: > -&i2c7 { > - ov2685: camera-sensor@3c { > - compatible = "ovti,ov2685"; > - reg = <0x3c>; > - pinctrl-names = "default"; > - pinctrl-0 = <&clk_24m_cam>; > - > - clocks = <&cru SCLK_TESTCLKOUT1>; > - clock-names = "xvclk"; > - > - avdd-supply = <&pp2800_cam>; > - dovdd-supply = <&pp1800>; > - dvdd-supply = <&pp1800>; > - reset-gpios = <&gpio2 3 GPIO_ACTIVE_LOW>; > - > - port { > - ucam_out: endpoint { > - remote-endpoint = <&mipi_in_ucam>; > - data-lanes = <1>; > - }; > - }; > - }; > -}; > diff --git a/Documentation/devicetree/bindings/media/i2c/ovti,ov2685.yaml b/Documentation/devicetree/bindings/media/i2c/ovti,ov2685.yaml > new file mode 100644 > index 0000000000000..2ac0ca8a0413b > --- /dev/null > +++ b/Documentation/devicetree/bindings/media/i2c/ovti,ov2685.yaml > @@ -0,0 +1,101 @@ > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/media/i2c/ovti,ov2685.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: OmniVision OV2685 Image Sensor > + > +maintainers: > + - Shunqian Zheng <zhengsq@rock-chips.com> > + > +properties: > + compatible: > + const: ovti,ov2685 > + > + reg: > + maxItems: 1 > + > + clocks: > + items: > + - description: XVCLK clock > + > + clock-names: > + items: > + - const: xvclk > + > + dvdd-supply: > + description: Digital Domain Power Supply > + > + avdd-supply: > + description: Analog Domain Power Supply > + > + dovdd-supply: > + description: I/O Domain Power Supply > + > + reset-gpios: > + maxItems: 1 > + description: Reset Pin GPIO Control (active low) > + > + port: > + description: MIPI CSI-2 transmitter port > + $ref: /schemas/graph.yaml#/$defs/port-base > + additionalProperties: false > + > + properties: > + endpoint: > + $ref: /schemas/media/video-interfaces.yaml# > + unevaluatedProperties: false > + > + properties: > + data-lanes: > + maxItems: 1 This should be 2 --- the sensor supports two lanes (even if the driver doesn't). I can address this when applying if that's ok. > + > + required: > + - data-lanes > + > +required: > + - compatible > + - reg > + - clocks > + - clock-names > + - dvdd-supply > + - avdd-supply > + - dovdd-supply > + - port > + > +additionalProperties: false > + > +examples: > + - | > + #include <dt-bindings/clock/rk3399-cru.h> > + #include <dt-bindings/gpio/gpio.h> > + > + i2c { > + #address-cells = <1>; > + #size-cells = <0>; > + > + ov2685: camera-sensor@3c { > + compatible = "ovti,ov2685"; > + reg = <0x3c>; > + pinctrl-names = "default"; > + pinctrl-0 = <&clk_24m_cam>; > + > + clocks = <&cru SCLK_TESTCLKOUT1>; > + clock-names = "xvclk"; > + > + avdd-supply = <&pp2800_cam>; > + dovdd-supply = <&pp1800>; > + dvdd-supply = <&pp1800>; > + reset-gpios = <&gpio2 3 GPIO_ACTIVE_LOW>; > + > + port { > + ucam_out: endpoint { > + remote-endpoint = <&mipi_in_ucam>; > + data-lanes = <1>; > + }; > + }; > + }; > + }; > + > +... > diff --git a/MAINTAINERS b/MAINTAINERS > index 8ea325040f355..5904f47756fe1 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -15410,6 +15410,7 @@ M: Shunqian Zheng <zhengsq@rock-chips.com> > L: linux-media@vger.kernel.org > S: Maintained > T: git git://linuxtv.org/media_tree.git > +F: Documentation/devicetree/bindings/media/i2c/ovti,ov2685.yaml > F: drivers/media/i2c/ov2685.c > > OMNIVISION OV2740 SENSOR DRIVER >
Hi Sakari, Sakari Ailus <sakari.ailus@iki.fi> schreef op 31 maart 2023 10:39:16 CEST: >Hi Luca, > >Thanks for the patch. > >On Thu, Mar 23, 2023 at 06:57:50PM +0100, Luca Weiss wrote: >> Convert the text-based dt-bindings to yaml. >> >> Changes from original txt: >> * Take wording for various properties from other yaml bindings, this >> removes e.g. volt amount from schema since it isn't really relevant >> and the datasheet is a better source. >> * Don't make reset-gpios a required property since it can be tied to >> DOVDD instead. >> >> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> >> Signed-off-by: Luca Weiss <luca@z3ntu.xyz> >> --- >> .../devicetree/bindings/media/i2c/ov2685.txt | 41 --------- >> .../devicetree/bindings/media/i2c/ovti,ov2685.yaml | 101 +++++++++++++++++++++ >> MAINTAINERS | 1 + >> 3 files changed, 102 insertions(+), 41 deletions(-) >> >> diff --git a/Documentation/devicetree/bindings/media/i2c/ov2685.txt b/Documentation/devicetree/bindings/media/i2c/ov2685.txt >> deleted file mode 100644 >> index 625c4a8c0d53d..0000000000000 >> --- a/Documentation/devicetree/bindings/media/i2c/ov2685.txt >> +++ /dev/null >> @@ -1,41 +0,0 @@ >> -* Omnivision OV2685 MIPI CSI-2 sensor >> - >> -Required Properties: >> -- compatible: shall be "ovti,ov2685" >> -- clocks: reference to the xvclk input clock >> -- clock-names: shall be "xvclk" >> -- avdd-supply: Analog voltage supply, 2.8 volts >> -- dovdd-supply: Digital I/O voltage supply, 1.8 volts >> -- dvdd-supply: Digital core voltage supply, 1.8 volts >> -- reset-gpios: Low active reset gpio >> - >> -The device node shall contain one 'port' child node with an >> -'endpoint' subnode for its digital output video port, >> -in accordance with the video interface bindings defined in >> -Documentation/devicetree/bindings/media/video-interfaces.txt. >> -The endpoint optional property 'data-lanes' shall be "<1>". >> - >> -Example: >> -&i2c7 { >> - ov2685: camera-sensor@3c { >> - compatible = "ovti,ov2685"; >> - reg = <0x3c>; >> - pinctrl-names = "default"; >> - pinctrl-0 = <&clk_24m_cam>; >> - >> - clocks = <&cru SCLK_TESTCLKOUT1>; >> - clock-names = "xvclk"; >> - >> - avdd-supply = <&pp2800_cam>; >> - dovdd-supply = <&pp1800>; >> - dvdd-supply = <&pp1800>; >> - reset-gpios = <&gpio2 3 GPIO_ACTIVE_LOW>; >> - >> - port { >> - ucam_out: endpoint { >> - remote-endpoint = <&mipi_in_ucam>; >> - data-lanes = <1>; >> - }; >> - }; >> - }; >> -}; >> diff --git a/Documentation/devicetree/bindings/media/i2c/ovti,ov2685.yaml b/Documentation/devicetree/bindings/media/i2c/ovti,ov2685.yaml >> new file mode 100644 >> index 0000000000000..2ac0ca8a0413b >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/media/i2c/ovti,ov2685.yaml >> @@ -0,0 +1,101 @@ >> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) >> +%YAML 1.2 >> +--- >> +$id: http://devicetree.org/schemas/media/i2c/ovti,ov2685.yaml# >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: OmniVision OV2685 Image Sensor >> + >> +maintainers: >> + - Shunqian Zheng <zhengsq@rock-chips.com> >> + >> +properties: >> + compatible: >> + const: ovti,ov2685 >> + >> + reg: >> + maxItems: 1 >> + >> + clocks: >> + items: >> + - description: XVCLK clock >> + >> + clock-names: >> + items: >> + - const: xvclk >> + >> + dvdd-supply: >> + description: Digital Domain Power Supply >> + >> + avdd-supply: >> + description: Analog Domain Power Supply >> + >> + dovdd-supply: >> + description: I/O Domain Power Supply >> + >> + reset-gpios: >> + maxItems: 1 >> + description: Reset Pin GPIO Control (active low) >> + >> + port: >> + description: MIPI CSI-2 transmitter port >> + $ref: /schemas/graph.yaml#/$defs/port-base >> + additionalProperties: false >> + >> + properties: >> + endpoint: >> + $ref: /schemas/media/video-interfaces.yaml# >> + unevaluatedProperties: false >> + >> + properties: >> + data-lanes: >> + maxItems: 1 > >This should be 2 --- the sensor supports two lanes (even if the driver >doesn't). Right, for some reason the product brief mentions that it features "a single-lane MIPI interface" but the datasheet I have writes that it has a 2-lane MIPI serial output, so I guess it does support two lanes? > >I can address this when applying if that's ok. That would be nice, thanks! Regards Luca > >> + >> + required: >> + - data-lanes >> + >> +required: >> + - compatible >> + - reg >> + - clocks >> + - clock-names >> + - dvdd-supply >> + - avdd-supply >> + - dovdd-supply >> + - port >> + >> +additionalProperties: false >> + >> +examples: >> + - | >> + #include <dt-bindings/clock/rk3399-cru.h> >> + #include <dt-bindings/gpio/gpio.h> >> + >> + i2c { >> + #address-cells = <1>; >> + #size-cells = <0>; >> + >> + ov2685: camera-sensor@3c { >> + compatible = "ovti,ov2685"; >> + reg = <0x3c>; >> + pinctrl-names = "default"; >> + pinctrl-0 = <&clk_24m_cam>; >> + >> + clocks = <&cru SCLK_TESTCLKOUT1>; >> + clock-names = "xvclk"; >> + >> + avdd-supply = <&pp2800_cam>; >> + dovdd-supply = <&pp1800>; >> + dvdd-supply = <&pp1800>; >> + reset-gpios = <&gpio2 3 GPIO_ACTIVE_LOW>; >> + >> + port { >> + ucam_out: endpoint { >> + remote-endpoint = <&mipi_in_ucam>; >> + data-lanes = <1>; >> + }; >> + }; >> + }; >> + }; >> + >> +... >> diff --git a/MAINTAINERS b/MAINTAINERS >> index 8ea325040f355..5904f47756fe1 100644 >> --- a/MAINTAINERS >> +++ b/MAINTAINERS >> @@ -15410,6 +15410,7 @@ M: Shunqian Zheng <zhengsq@rock-chips.com> >> L: linux-media@vger.kernel.org >> S: Maintained >> T: git git://linuxtv.org/media_tree.git >> +F: Documentation/devicetree/bindings/media/i2c/ovti,ov2685.yaml >> F: drivers/media/i2c/ov2685.c >> >> OMNIVISION OV2740 SENSOR DRIVER >> >
Hi Luca, On Fri, Mar 31, 2023 at 11:18:29AM +0200, Luca Weiss wrote: > >> + properties: > >> + data-lanes: > >> + maxItems: 1 > > > >This should be 2 --- the sensor supports two lanes (even if the driver > >doesn't). > > Right, for some reason the product brief mentions that it features "a > single-lane MIPI interface" but the datasheet I have writes that it has a > 2-lane MIPI serial output, so I guess it does support two lanes? I suppose the datasheet is right. Well, if someone proves otherwise, we can always change this. > > > > >I can address this when applying if that's ok. > > That would be nice, thanks! Done, thanks!
On Thu, Mar 23, 2023 at 12:58 PM Luca Weiss <luca@z3ntu.xyz> wrote: > > Convert the text-based dt-bindings to yaml. > > Changes from original txt: > * Take wording for various properties from other yaml bindings, this > removes e.g. volt amount from schema since it isn't really relevant > and the datasheet is a better source. > * Don't make reset-gpios a required property since it can be tied to > DOVDD instead. > > Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > Signed-off-by: Luca Weiss <luca@z3ntu.xyz> > --- > .../devicetree/bindings/media/i2c/ov2685.txt | 41 --------- > .../devicetree/bindings/media/i2c/ovti,ov2685.yaml | 101 +++++++++++++++++++++ > MAINTAINERS | 1 + > 3 files changed, 102 insertions(+), 41 deletions(-) Now warning in linux-next: /builds/robherring/linux-dt/Documentation/devicetree/bindings/media/rockchip-isp1.example.dtb: camera@3c: port:endpoint:data-lanes: [[1]] is too short From schema: /builds/robherring/linux-dt/Documentation/devicetree/bindings/media/i2c/ovti,ov2685.yaml /builds/robherring/linux-dt/Documentation/devicetree/bindings/media/i2c/ovti,ov2685.example.dtb: camera-sensor@3c: port:endpoint:data-lanes: [[1]] is too short From schema: /builds/robherring/linux-dt/Documentation/devicetree/bindings/media/i2c/ovti,ov2685.yaml
On Mittwoch, 19. April 2023 01:54:18 CEST Rob Herring wrote: > On Thu, Mar 23, 2023 at 12:58 PM Luca Weiss <luca@z3ntu.xyz> wrote: > > Convert the text-based dt-bindings to yaml. > > > > Changes from original txt: > > * Take wording for various properties from other yaml bindings, this > > > > removes e.g. volt amount from schema since it isn't really relevant > > and the datasheet is a better source. > > > > * Don't make reset-gpios a required property since it can be tied to > > > > DOVDD instead. > > > > Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > > Signed-off-by: Luca Weiss <luca@z3ntu.xyz> > > --- > > > > .../devicetree/bindings/media/i2c/ov2685.txt | 41 --------- > > .../devicetree/bindings/media/i2c/ovti,ov2685.yaml | 101 > > +++++++++++++++++++++ MAINTAINERS > > | 1 + > > 3 files changed, 102 insertions(+), 41 deletions(-) > > Now warning in linux-next: > > /builds/robherring/linux-dt/Documentation/devicetree/bindings/media/rockchip > -isp1.example.dtb: camera@3c: port:endpoint:data-lanes: [[1]] is too short > From schema: > /builds/robherring/linux-dt/Documentation/devicetree/bindings/media/i2c/ovti > ,ov2685.yaml > /builds/robherring/linux-dt/Documentation/devicetree/bindings/media/i2c/ovt > i,ov2685.example.dtb: camera-sensor@3c: port:endpoint:data-lanes: [[1]] is > too short > From schema: > /builds/robherring/linux-dt/Documentation/devicetree/bindings/media/i2c/ovti > ,ov2685.yaml Right, since Sakari changed maxItems=1 to maxItems=2, now minItems is also 2 but it should be 1. I'll send a patch to fix this. Regards Luca
diff --git a/Documentation/devicetree/bindings/media/i2c/ov2685.txt b/Documentation/devicetree/bindings/media/i2c/ov2685.txt deleted file mode 100644 index 625c4a8c0d53d..0000000000000 --- a/Documentation/devicetree/bindings/media/i2c/ov2685.txt +++ /dev/null @@ -1,41 +0,0 @@ -* Omnivision OV2685 MIPI CSI-2 sensor - -Required Properties: -- compatible: shall be "ovti,ov2685" -- clocks: reference to the xvclk input clock -- clock-names: shall be "xvclk" -- avdd-supply: Analog voltage supply, 2.8 volts -- dovdd-supply: Digital I/O voltage supply, 1.8 volts -- dvdd-supply: Digital core voltage supply, 1.8 volts -- reset-gpios: Low active reset gpio - -The device node shall contain one 'port' child node with an -'endpoint' subnode for its digital output video port, -in accordance with the video interface bindings defined in -Documentation/devicetree/bindings/media/video-interfaces.txt. -The endpoint optional property 'data-lanes' shall be "<1>". - -Example: -&i2c7 { - ov2685: camera-sensor@3c { - compatible = "ovti,ov2685"; - reg = <0x3c>; - pinctrl-names = "default"; - pinctrl-0 = <&clk_24m_cam>; - - clocks = <&cru SCLK_TESTCLKOUT1>; - clock-names = "xvclk"; - - avdd-supply = <&pp2800_cam>; - dovdd-supply = <&pp1800>; - dvdd-supply = <&pp1800>; - reset-gpios = <&gpio2 3 GPIO_ACTIVE_LOW>; - - port { - ucam_out: endpoint { - remote-endpoint = <&mipi_in_ucam>; - data-lanes = <1>; - }; - }; - }; -}; diff --git a/Documentation/devicetree/bindings/media/i2c/ovti,ov2685.yaml b/Documentation/devicetree/bindings/media/i2c/ovti,ov2685.yaml new file mode 100644 index 0000000000000..2ac0ca8a0413b --- /dev/null +++ b/Documentation/devicetree/bindings/media/i2c/ovti,ov2685.yaml @@ -0,0 +1,101 @@ +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/media/i2c/ovti,ov2685.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: OmniVision OV2685 Image Sensor + +maintainers: + - Shunqian Zheng <zhengsq@rock-chips.com> + +properties: + compatible: + const: ovti,ov2685 + + reg: + maxItems: 1 + + clocks: + items: + - description: XVCLK clock + + clock-names: + items: + - const: xvclk + + dvdd-supply: + description: Digital Domain Power Supply + + avdd-supply: + description: Analog Domain Power Supply + + dovdd-supply: + description: I/O Domain Power Supply + + reset-gpios: + maxItems: 1 + description: Reset Pin GPIO Control (active low) + + port: + description: MIPI CSI-2 transmitter port + $ref: /schemas/graph.yaml#/$defs/port-base + additionalProperties: false + + properties: + endpoint: + $ref: /schemas/media/video-interfaces.yaml# + unevaluatedProperties: false + + properties: + data-lanes: + maxItems: 1 + + required: + - data-lanes + +required: + - compatible + - reg + - clocks + - clock-names + - dvdd-supply + - avdd-supply + - dovdd-supply + - port + +additionalProperties: false + +examples: + - | + #include <dt-bindings/clock/rk3399-cru.h> + #include <dt-bindings/gpio/gpio.h> + + i2c { + #address-cells = <1>; + #size-cells = <0>; + + ov2685: camera-sensor@3c { + compatible = "ovti,ov2685"; + reg = <0x3c>; + pinctrl-names = "default"; + pinctrl-0 = <&clk_24m_cam>; + + clocks = <&cru SCLK_TESTCLKOUT1>; + clock-names = "xvclk"; + + avdd-supply = <&pp2800_cam>; + dovdd-supply = <&pp1800>; + dvdd-supply = <&pp1800>; + reset-gpios = <&gpio2 3 GPIO_ACTIVE_LOW>; + + port { + ucam_out: endpoint { + remote-endpoint = <&mipi_in_ucam>; + data-lanes = <1>; + }; + }; + }; + }; + +... diff --git a/MAINTAINERS b/MAINTAINERS index 8ea325040f355..5904f47756fe1 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -15410,6 +15410,7 @@ M: Shunqian Zheng <zhengsq@rock-chips.com> L: linux-media@vger.kernel.org S: Maintained T: git git://linuxtv.org/media_tree.git +F: Documentation/devicetree/bindings/media/i2c/ovti,ov2685.yaml F: drivers/media/i2c/ov2685.c OMNIVISION OV2740 SENSOR DRIVER