| Message ID | 20241001-b4-master-24-11-25-ov08x40-v2-2-e478976b20c1@linaro.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | ov08x40: Enable use of ov08x40 on Qualcomm X1E80100 CRD | expand |
Hi Bryan, On Tue, Oct 01, 2024 at 02:15:50PM +0100, Bryan O'Donoghue wrote: > Add bindings for the already upstream OV08X40 to enable usage of this > sensor on DT based systems. > > Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> > --- > .../bindings/media/i2c/ovti,ov08x40.yaml | 120 +++++++++++++++++++++ > 1 file changed, 120 insertions(+) > > diff --git a/Documentation/devicetree/bindings/media/i2c/ovti,ov08x40.yaml b/Documentation/devicetree/bindings/media/i2c/ovti,ov08x40.yaml > new file mode 100644 > index 0000000000000000000000000000000000000000..bc499e4b5d48ed57250dec33a91c92552f137cf9 > --- /dev/null > +++ b/Documentation/devicetree/bindings/media/i2c/ovti,ov08x40.yaml > @@ -0,0 +1,120 @@ > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > +# Copyright (c) 2024 Linaro Ltd. > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/media/i2c/ovti,ov08x40.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Omnivision OV08X40 CMOS Sensor > + > +maintainers: > + - Bryan O'Donoghue <bryan.odonoghue@linaro.org> > + > +description: | > + The Omnivision OV08X40 is a 9.2 megapixel, CMOS image sensor which supports: > + - Automatic black level calibration (ABLC) > + - Programmable controls for frame rate, mirror and flip, binning, cropping > + and windowing > + - Output formats 10-bit 4C RGB RAW, 10-bit Bayer RAW > + - 4-lane MIPI D-PHY TX @ 1 Gbps per lane > + - 2-lane MPIP D-PHY TX @ 2 Gbps per lane > + - Dynamic defect pixel cancellation > + - Standard SCCB command interface > + > +properties: > + compatible: > + const: ovti,ov08x40 > + > + reg: > + maxItems: 1 > + > + assigned-clocks: true > + assigned-clock-parents: true > + assigned-clock-rates: true As much as I'd like to see these mandatory, there seem to be cases where they can't be used. Therefore I'd leave them in the example only. If that turns out to be the only change to do, I can also handle that while applying. > + > + clocks: > + maxItems: 1 > + > + avdd-supply: > + description: Analogue circuit voltage supply. > + > + dovdd-supply: > + description: I/O circuit voltage supply. > + > + dvdd-supply: > + description: Digital circuit voltage supply. > + > + reset-gpios: > + description: Active low GPIO connected to XSHUTDOWN pad of the sensor. > + > + port: > + $ref: /schemas/graph.yaml#/$defs/port-base > + additionalProperties: false > + > + properties: > + endpoint: > + $ref: /schemas/media/video-interfaces.yaml# > + unevaluatedProperties: false > + > + properties: > + data-lanes: > + oneOf: > + - items: > + - const: 1 > + - const: 2 > + - items: > + - const: 1 > + - const: 2 > + - const: 3 > + - const: 4 > + > + link-frequencies: true > + > + required: > + - data-lanes > + - link-frequencies > + > +required: > + - compatible > + - reg > + - clocks > + - port > + > +additionalProperties: false > + > +examples: > + - | > + #include <dt-bindings/gpio/gpio.h> > + > + i2c { > + #address-cells = <1>; > + #size-cells = <0>; > + > + ov08x40: camera@36 { > + compatible = "ovti,ov08x40"; > + reg = <0x36>; > + > + reset-gpios = <&tlmm 111 GPIO_ACTIVE_LOW>; > + pinctrl-names = "default"; > + pinctrl-0 = <&cam_rgb_defaultt>; > + > + clocks = <&ov08x40_clk>; > + > + assigned-clocks = <&ov9282_clk>; > + assigned-clock-parents = <&ov9282_clk_parent>; > + assigned-clock-rates = <19200000>; > + > + avdd-supply = <&vreg_l7b_2p8>; > + dvdd-supply = <&vreg_l7b_1p8>; > + dovdd-supply = <&vreg_l3m_1p8>; > + > + port { > + ov08x40_ep: endpoint { > + remote-endpoint = <&csiphy4_ep>; > + data-lanes = <1 2 3 4>; > + link-frequencies = /bits/ 64 <400000000>; > + }; > + }; > + }; > + }; > +... >
On 01/10/2024 14:22, Sakari Ailus wrote: >> + assigned-clocks: true >> + assigned-clock-parents: true >> + assigned-clock-rates: true > As much as I'd like to see these mandatory, there seem to be cases where > they can't be used. Therefore I'd leave them in the example only. > > If that turns out to be the only change to do, I can also handle that while > applying. So I took Documentation/devicetree/bindings/media/i2c/ovti,ov9282.yaml as the reference for this. Without listing "assigned-clock*" in the required: field I believe the dts checkers will not require those. So instead of saying assigned-clocks: maxItems: 1 we write assigned-clocks: true omit from "required:" and get the desired effect. For example this passes the checker for me. &cci1_i2c1 { camera@36 { compatible = "ovti,ov08x40"; reg = <0x36>; reset-gpios = <&tlmm 237 GPIO_ACTIVE_LOW>; pinctrl-names = "default"; pinctrl-0 = <&cam_rgb_default>; clocks = <&camcc CAM_CC_MCLK4_CLK>; assigned-clocks = <&camcc CAM_CC_MCLK4_CLK>; assigned-clock-rates = <19200000>; orientation = <0>; /* front facing */ avdd-supply = <&vreg_l7b_2p8>; dvdd-supply = <&vreg_l7b_2p8>; dovdd-supply = <&vreg_l3m_1p8>; port { ov08x40_ep: endpoint { clock-lanes = <0>; data-lanes = <1 2 3 4>; link-frequencies = /bits/ 64 <400000000>; remote-endpoint = <&csiphy4_ep>; }; }; }; }; Eh.. at least that's how I think this works. Krzysztof/Rob ? --- bod
On Tue, Oct 01, 2024 at 02:15:50PM +0100, Bryan O'Donoghue wrote: > Add bindings for the already upstream OV08X40 to enable usage of this > sensor on DT based systems. > > Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> > --- > .../bindings/media/i2c/ovti,ov08x40.yaml | 120 +++++++++++++++++++++ > 1 file changed, 120 insertions(+) ... > +properties: > + compatible: > + const: ovti,ov08x40 > + > + reg: > + maxItems: 1 > + > + assigned-clocks: true > + assigned-clock-parents: true > + assigned-clock-rates: true Drop all three, redundant. > + > + clocks: > + maxItems: 1 > + > + avdd-supply: > + description: Analogue circuit voltage supply. > + > + dovdd-supply: > + description: I/O circuit voltage supply. > + > + dvdd-supply: > + description: Digital circuit voltage supply. > + > + reset-gpios: > + description: Active low GPIO connected to XSHUTDOWN pad of the sensor. > + > + port: > + $ref: /schemas/graph.yaml#/$defs/port-base > + additionalProperties: false > + > + properties: > + endpoint: > + $ref: /schemas/media/video-interfaces.yaml# > + unevaluatedProperties: false > + > + properties: > + data-lanes: > + oneOf: > + - items: > + - const: 1 > + - const: 2 > + - items: > + - const: 1 > + - const: 2 > + - const: 3 > + - const: 4 > + > + link-frequencies: true Drop, also redundant, unless you provide here constraints from datasheet. > + > + required: > + - data-lanes > + - link-frequencies Best regards, Krzysztof
On Tue, Oct 01, 2024 at 02:47:43PM +0100, Bryan O'Donoghue wrote: > On 01/10/2024 14:22, Sakari Ailus wrote: > > > + assigned-clocks: true > > > + assigned-clock-parents: true > > > + assigned-clock-rates: true > > As much as I'd like to see these mandatory, there seem to be cases where > > they can't be used. Therefore I'd leave them in the example only. > > > > If that turns out to be the only change to do, I can also handle that while > > applying. > > So I took Documentation/devicetree/bindings/media/i2c/ovti,ov9282.yaml as > the reference for this. > > Without listing "assigned-clock*" in the required: field I believe the dts > checkers will not require those. > > So instead of saying > assigned-clocks: > maxItems: 1 > > we write > assigned-clocks: true > > omit from "required:" and get the desired effect. > > For example this passes the checker for me. > > &cci1_i2c1 { > camera@36 { > compatible = "ovti,ov08x40"; > reg = <0x36>; > > reset-gpios = <&tlmm 237 GPIO_ACTIVE_LOW>; > pinctrl-names = "default"; > pinctrl-0 = <&cam_rgb_default>; > > clocks = <&camcc CAM_CC_MCLK4_CLK>; > assigned-clocks = <&camcc CAM_CC_MCLK4_CLK>; > assigned-clock-rates = <19200000>; > > orientation = <0>; /* front facing */ > > avdd-supply = <&vreg_l7b_2p8>; > dvdd-supply = <&vreg_l7b_2p8>; > dovdd-supply = <&vreg_l3m_1p8>; > > port { > ov08x40_ep: endpoint { > clock-lanes = <0>; > data-lanes = <1 2 3 4>; > link-frequencies = /bits/ 64 <400000000>; > remote-endpoint = <&csiphy4_ep>; > }; > }; > }; > }; > > Eh.. at least that's how I think this works. > > Krzysztof/Rob ? What is the question? You should not have assigned-* properties in the schema. Example is fine. Best regards, Krzysztof
On 02/10/2024 07:20, Krzysztof Kozlowski wrote: >> + assigned-clocks: true >> + assigned-clock-parents: true >> + assigned-clock-rates: true > Drop all three, redundant. Ah, I understand. Ack. --- bod
diff --git a/Documentation/devicetree/bindings/media/i2c/ovti,ov08x40.yaml b/Documentation/devicetree/bindings/media/i2c/ovti,ov08x40.yaml new file mode 100644 index 0000000000000000000000000000000000000000..bc499e4b5d48ed57250dec33a91c92552f137cf9 --- /dev/null +++ b/Documentation/devicetree/bindings/media/i2c/ovti,ov08x40.yaml @@ -0,0 +1,120 @@ +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) +# Copyright (c) 2024 Linaro Ltd. +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/media/i2c/ovti,ov08x40.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Omnivision OV08X40 CMOS Sensor + +maintainers: + - Bryan O'Donoghue <bryan.odonoghue@linaro.org> + +description: | + The Omnivision OV08X40 is a 9.2 megapixel, CMOS image sensor which supports: + - Automatic black level calibration (ABLC) + - Programmable controls for frame rate, mirror and flip, binning, cropping + and windowing + - Output formats 10-bit 4C RGB RAW, 10-bit Bayer RAW + - 4-lane MIPI D-PHY TX @ 1 Gbps per lane + - 2-lane MPIP D-PHY TX @ 2 Gbps per lane + - Dynamic defect pixel cancellation + - Standard SCCB command interface + +properties: + compatible: + const: ovti,ov08x40 + + reg: + maxItems: 1 + + assigned-clocks: true + assigned-clock-parents: true + assigned-clock-rates: true + + clocks: + maxItems: 1 + + avdd-supply: + description: Analogue circuit voltage supply. + + dovdd-supply: + description: I/O circuit voltage supply. + + dvdd-supply: + description: Digital circuit voltage supply. + + reset-gpios: + description: Active low GPIO connected to XSHUTDOWN pad of the sensor. + + port: + $ref: /schemas/graph.yaml#/$defs/port-base + additionalProperties: false + + properties: + endpoint: + $ref: /schemas/media/video-interfaces.yaml# + unevaluatedProperties: false + + properties: + data-lanes: + oneOf: + - items: + - const: 1 + - const: 2 + - items: + - const: 1 + - const: 2 + - const: 3 + - const: 4 + + link-frequencies: true + + required: + - data-lanes + - link-frequencies + +required: + - compatible + - reg + - clocks + - port + +additionalProperties: false + +examples: + - | + #include <dt-bindings/gpio/gpio.h> + + i2c { + #address-cells = <1>; + #size-cells = <0>; + + ov08x40: camera@36 { + compatible = "ovti,ov08x40"; + reg = <0x36>; + + reset-gpios = <&tlmm 111 GPIO_ACTIVE_LOW>; + pinctrl-names = "default"; + pinctrl-0 = <&cam_rgb_defaultt>; + + clocks = <&ov08x40_clk>; + + assigned-clocks = <&ov9282_clk>; + assigned-clock-parents = <&ov9282_clk_parent>; + assigned-clock-rates = <19200000>; + + avdd-supply = <&vreg_l7b_2p8>; + dvdd-supply = <&vreg_l7b_1p8>; + dovdd-supply = <&vreg_l3m_1p8>; + + port { + ov08x40_ep: endpoint { + remote-endpoint = <&csiphy4_ep>; + data-lanes = <1 2 3 4>; + link-frequencies = /bits/ 64 <400000000>; + }; + }; + }; + }; +...
Add bindings for the already upstream OV08X40 to enable usage of this sensor on DT based systems. Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> --- .../bindings/media/i2c/ovti,ov08x40.yaml | 120 +++++++++++++++++++++ 1 file changed, 120 insertions(+)