| Message ID | 20230126165909.121302-2-jacopo.mondi@ideasonboard.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v6,1/9] media: dt-bindings: Add OV5670 | expand |
On 26/01/2023 17:59, Jacopo Mondi wrote: > Add the bindings documentation for Omnivision OV5670 image sensor. > > Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> > --- (...) > + > + dovdd-supply: > + description: Digital I/O circuit power. Typically 2.8V or 1.8V. > + > + port: > + $ref: /schemas/graph.yaml#/$defs/port-base > + additionalProperties: false > + > + properties: > + endpoint: > + $ref: /schemas/media/video-interfaces.yaml# > + unevaluatedProperties: false > + > + properties: > + data-lanes: > + minItems: 1 > + maxItems: 2 > + items: > + enum: [1, 2] > + > + clock-noncontinuous: true You do not need this. Drop. > + Best regards, Krzysztof
Hi Krzysztof On Fri, Jan 27, 2023 at 03:19:08PM +0100, Krzysztof Kozlowski wrote: > On 26/01/2023 17:59, Jacopo Mondi wrote: > > Add the bindings documentation for Omnivision OV5670 image sensor. > > > > Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> > > --- > > (...) > > > + > > + dovdd-supply: > > + description: Digital I/O circuit power. Typically 2.8V or 1.8V. > > + > > + port: > > + $ref: /schemas/graph.yaml#/$defs/port-base > > + additionalProperties: false > > + > > + properties: > > + endpoint: > > + $ref: /schemas/media/video-interfaces.yaml# > > + unevaluatedProperties: false > > + > > + properties: > > + data-lanes: > > + minItems: 1 > > + maxItems: 2 > > + items: > > + enum: [1, 2] > > + > > + clock-noncontinuous: true > > You do not need this. Drop. > Is this due to "unevaluatedProperties: false" ? I read you recent explanation to a similar question on the Visconti bindings. Let me summarize my understanding: For a given schema a property could be - required required: - foo - optional by default with "unevaluatedProperties: false" "foo: true" with "additionalProperties: false" - forbidden "foo: false" with "unevaluatedProperties: false" by default wiht "additionalProperties: false" clock-noncontinuous is defined in video-interfaces.yaml. as I specify "unevaluatedProperties: false" does it mean all the properties defined in video-interfaces.yaml are optionally accepted ? If that's the case that's not what I want as clock-noncontinuous is -the only- property from that file we want to accept here (and data-lanes ofc). Should I change "unevaluatedProperties: false" to "additionalProperties: false" and keep "clock-noncontinuous: true" ? Thanks j > > + > > Best regards, > Krzysztof >
On 27/01/2023 19:14, Jacopo Mondi wrote: > Hi Krzysztof > > On Fri, Jan 27, 2023 at 03:19:08PM +0100, Krzysztof Kozlowski wrote: >> On 26/01/2023 17:59, Jacopo Mondi wrote: >>> Add the bindings documentation for Omnivision OV5670 image sensor. >>> >>> Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> >>> --- >> >> (...) >> >>> + >>> + dovdd-supply: >>> + description: Digital I/O circuit power. Typically 2.8V or 1.8V. >>> + >>> + port: >>> + $ref: /schemas/graph.yaml#/$defs/port-base >>> + additionalProperties: false >>> + >>> + properties: >>> + endpoint: >>> + $ref: /schemas/media/video-interfaces.yaml# >>> + unevaluatedProperties: false >>> + >>> + properties: >>> + data-lanes: >>> + minItems: 1 >>> + maxItems: 2 >>> + items: >>> + enum: [1, 2] >>> + >>> + clock-noncontinuous: true >> >> You do not need this. Drop. >> > > Is this due to "unevaluatedProperties: false" ? > > I read you recent explanation to a similar question on the Visconti > bindings. Let me summarize my understanding: > > For a given schema a property could be > - required > required: > - foo > > - optional > by default with "unevaluatedProperties: false" > "foo: true" with "additionalProperties: false" > > - forbidden > "foo: false" with "unevaluatedProperties: false" > by default wiht "additionalProperties: false" > > clock-noncontinuous is defined in video-interfaces.yaml. as I specify > "unevaluatedProperties: false" does it mean > all the properties defined in video-interfaces.yaml are optionally > accepted ? If that's the case that's not what I want as > clock-noncontinuous is -the only- property from that file we want to > accept here (and data-lanes ofc). > > Should I change "unevaluatedProperties: false" to > "additionalProperties: false" and keep "clock-noncontinuous: true" ? > Why would you disallow other properties? Just because driver does not use them? That's not correct, driver change but bindings should stay the same. Best regards, Krzysztof
Hi Krzysztof, On Fri, Jan 27, 2023 at 08:58:20PM +0100, Krzysztof Kozlowski wrote: > On 27/01/2023 19:14, Jacopo Mondi wrote: > > Hi Krzysztof > > > > On Fri, Jan 27, 2023 at 03:19:08PM +0100, Krzysztof Kozlowski wrote: > >> On 26/01/2023 17:59, Jacopo Mondi wrote: > >>> Add the bindings documentation for Omnivision OV5670 image sensor. > >>> > >>> Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> > >>> --- > >> > >> (...) > >> > >>> + > >>> + dovdd-supply: > >>> + description: Digital I/O circuit power. Typically 2.8V or 1.8V. > >>> + > >>> + port: > >>> + $ref: /schemas/graph.yaml#/$defs/port-base > >>> + additionalProperties: false > >>> + > >>> + properties: > >>> + endpoint: > >>> + $ref: /schemas/media/video-interfaces.yaml# > >>> + unevaluatedProperties: false > >>> + > >>> + properties: > >>> + data-lanes: > >>> + minItems: 1 > >>> + maxItems: 2 > >>> + items: > >>> + enum: [1, 2] > >>> + > >>> + clock-noncontinuous: true > >> > >> You do not need this. Drop. > >> > > > > Is this due to "unevaluatedProperties: false" ? > > > > I read you recent explanation to a similar question on the Visconti > > bindings. Let me summarize my understanding: > > > > For a given schema a property could be > > - required > > required: > > - foo > > > > - optional > > by default with "unevaluatedProperties: false" > > "foo: true" with "additionalProperties: false" > > > > - forbidden > > "foo: false" with "unevaluatedProperties: false" > > by default wiht "additionalProperties: false" > > > > clock-noncontinuous is defined in video-interfaces.yaml. as I specify > > "unevaluatedProperties: false" does it mean > > all the properties defined in video-interfaces.yaml are optionally > > accepted ? If that's the case that's not what I want as > > clock-noncontinuous is -the only- property from that file we want to > > accept here (and data-lanes ofc). > > > > Should I change "unevaluatedProperties: false" to > > "additionalProperties: false" and keep "clock-noncontinuous: true" ? > > > > Why would you disallow other properties? Just because driver does not > use them? That's not correct, driver change but bindings should stay the > same. The clock-noncontinuous property is relevant for the hardware. There are some properties not listed here that might be relevant (for all camera sensors) but most properties in video-interfaces.yaml are not applicable to this device. I also think is be useful to say what is relevant in DT bindings, as the other sources of information left are hardware datasheets (if you have access to them) or the driver (which is supposed not to be relevant for the bindings).
On 27/01/2023 21:38, Sakari Ailus wrote: > Hi Krzysztof, > > On Fri, Jan 27, 2023 at 08:58:20PM +0100, Krzysztof Kozlowski wrote: >> On 27/01/2023 19:14, Jacopo Mondi wrote: >>> Hi Krzysztof >>> >>> On Fri, Jan 27, 2023 at 03:19:08PM +0100, Krzysztof Kozlowski wrote: >>>> On 26/01/2023 17:59, Jacopo Mondi wrote: >>>>> Add the bindings documentation for Omnivision OV5670 image sensor. >>>>> >>>>> Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> >>>>> --- >>>> >>>> (...) >>>> >>>>> + >>>>> + dovdd-supply: >>>>> + description: Digital I/O circuit power. Typically 2.8V or 1.8V. >>>>> + >>>>> + port: >>>>> + $ref: /schemas/graph.yaml#/$defs/port-base >>>>> + additionalProperties: false >>>>> + >>>>> + properties: >>>>> + endpoint: >>>>> + $ref: /schemas/media/video-interfaces.yaml# >>>>> + unevaluatedProperties: false >>>>> + >>>>> + properties: >>>>> + data-lanes: >>>>> + minItems: 1 >>>>> + maxItems: 2 >>>>> + items: >>>>> + enum: [1, 2] >>>>> + >>>>> + clock-noncontinuous: true >>>> >>>> You do not need this. Drop. >>>> >>> >>> Is this due to "unevaluatedProperties: false" ? >>> >>> I read you recent explanation to a similar question on the Visconti >>> bindings. Let me summarize my understanding: >>> >>> For a given schema a property could be >>> - required >>> required: >>> - foo >>> >>> - optional >>> by default with "unevaluatedProperties: false" >>> "foo: true" with "additionalProperties: false" >>> >>> - forbidden >>> "foo: false" with "unevaluatedProperties: false" >>> by default wiht "additionalProperties: false" >>> >>> clock-noncontinuous is defined in video-interfaces.yaml. as I specify >>> "unevaluatedProperties: false" does it mean >>> all the properties defined in video-interfaces.yaml are optionally >>> accepted ? If that's the case that's not what I want as >>> clock-noncontinuous is -the only- property from that file we want to >>> accept here (and data-lanes ofc). >>> >>> Should I change "unevaluatedProperties: false" to >>> "additionalProperties: false" and keep "clock-noncontinuous: true" ? >>> >> >> Why would you disallow other properties? Just because driver does not >> use them? That's not correct, driver change but bindings should stay the >> same. > > The clock-noncontinuous property is relevant for the hardware. There are > some properties not listed here that might be relevant (for all camera > sensors) but most properties in video-interfaces.yaml are not applicable to > this device. > > I also think is be useful to say what is relevant in DT bindings, as the > other sources of information left are hardware datasheets (if you have > access to them) or the driver (which is supposed not to be relevant for the > bindings). > Then it might be meaningful to list all allowed properties - even if not currently supported by the driver - and use additionalProperties:false. This has drawback - whenever video-interfaces gets something new, the bindings here (and other such devices) will have to be explicitly enabled. Best regards, Krzysztof
Hi Krzysztof On Fri, Jan 27, 2023 at 09:44:25PM +0100, Krzysztof Kozlowski wrote: > On 27/01/2023 21:38, Sakari Ailus wrote: > > Hi Krzysztof, > > > > On Fri, Jan 27, 2023 at 08:58:20PM +0100, Krzysztof Kozlowski wrote: > >> On 27/01/2023 19:14, Jacopo Mondi wrote: > >>> Hi Krzysztof > >>> > >>> On Fri, Jan 27, 2023 at 03:19:08PM +0100, Krzysztof Kozlowski wrote: > >>>> On 26/01/2023 17:59, Jacopo Mondi wrote: > >>>>> Add the bindings documentation for Omnivision OV5670 image sensor. > >>>>> > >>>>> Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> > >>>>> --- > >>>> > >>>> (...) > >>>> > >>>>> + > >>>>> + dovdd-supply: > >>>>> + description: Digital I/O circuit power. Typically 2.8V or 1.8V. > >>>>> + > >>>>> + port: > >>>>> + $ref: /schemas/graph.yaml#/$defs/port-base > >>>>> + additionalProperties: false > >>>>> + > >>>>> + properties: > >>>>> + endpoint: > >>>>> + $ref: /schemas/media/video-interfaces.yaml# > >>>>> + unevaluatedProperties: false > >>>>> + > >>>>> + properties: > >>>>> + data-lanes: > >>>>> + minItems: 1 > >>>>> + maxItems: 2 > >>>>> + items: > >>>>> + enum: [1, 2] > >>>>> + > >>>>> + clock-noncontinuous: true > >>>> > >>>> You do not need this. Drop. > >>>> > >>> > >>> Is this due to "unevaluatedProperties: false" ? > >>> > >>> I read you recent explanation to a similar question on the Visconti > >>> bindings. Let me summarize my understanding: > >>> > >>> For a given schema a property could be > >>> - required > >>> required: > >>> - foo > >>> > >>> - optional > >>> by default with "unevaluatedProperties: false" > >>> "foo: true" with "additionalProperties: false" > >>> > >>> - forbidden > >>> "foo: false" with "unevaluatedProperties: false" > >>> by default wiht "additionalProperties: false" > >>> > >>> clock-noncontinuous is defined in video-interfaces.yaml. as I specify > >>> "unevaluatedProperties: false" does it mean > >>> all the properties defined in video-interfaces.yaml are optionally > >>> accepted ? If that's the case that's not what I want as > >>> clock-noncontinuous is -the only- property from that file we want to > >>> accept here (and data-lanes ofc). > >>> > >>> Should I change "unevaluatedProperties: false" to > >>> "additionalProperties: false" and keep "clock-noncontinuous: true" ? > >>> > >> > >> Why would you disallow other properties? Just because driver does not > >> use them? That's not correct, driver change but bindings should stay the > >> same. > > > > The clock-noncontinuous property is relevant for the hardware. There are > > some properties not listed here that might be relevant (for all camera > > sensors) but most properties in video-interfaces.yaml are not applicable to > > this device. > > > > I also think is be useful to say what is relevant in DT bindings, as the > > other sources of information left are hardware datasheets (if you have > > access to them) or the driver (which is supposed not to be relevant for the > > bindings). > > > > Then it might be meaningful to list all allowed properties - even if not > currently supported by the driver - and use additionalProperties:false. Have a look at what properties video-interfaces.yaml lists. Some of them only apply to CSI-2 sensors (data lanes, link-frequencies etc), some of them only to parallel sensors (lines polarities, bus-width etc). I see most of the bindings in media/i2c reporting $ref: /schemas/media/video-interfaces.yaml# unevaluatedProperties: false I think that's actually wrong as there's no way all the properties in video-interfaces.yaml can apply to a single device (with the exception of a few sensors that support both bus types). > This has drawback - whenever video-interfaces gets something new, the > bindings here (and other such devices) will have to be explicitly enabled. video-interfaces is rarely updated, and when it happes it's to add properties required by a newly supported device, so this doesn't concern me much personally. > > Best regards, > Krzysztof >
diff --git a/Documentation/devicetree/bindings/media/i2c/ovti,ov5670.yaml b/Documentation/devicetree/bindings/media/i2c/ovti,ov5670.yaml new file mode 100644 index 000000000000..fa264255b5eb --- /dev/null +++ b/Documentation/devicetree/bindings/media/i2c/ovti,ov5670.yaml @@ -0,0 +1,92 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/media/i2c/ovti,ov5670.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Omnivision OV5670 5 Megapixels raw image sensor + +maintainers: + - Jacopo Mondi <jacopo.mondi@ideasonboard.com> + +description: |- + The OV5670 is a 5 Megapixels raw image sensor which provides images in 10-bits + RAW BGGR Bayer format on a 2 data lanes MIPI CSI-2 serial interface and is + controlled through an I2C compatible control bus. + +properties: + compatible: + const: ovti,ov5670 + + reg: + maxItems: 1 + + clocks: + description: System clock. From 6 to 27 MHz. + maxItems: 1 + + powerdown-gpios: + description: Reference to the GPIO connected to the PWDNB pin. Active low. + + reset-gpios: + description: Reference to the GPIO connected to the XSHUTDOWN pin. Active low. + maxItems: 1 + + avdd-supply: + description: Analog circuit power. Typically 2.8V. + + dvdd-supply: + description: Digital circuit power. Typically 1.2V. + + dovdd-supply: + description: Digital I/O circuit power. Typically 2.8V or 1.8V. + + port: + $ref: /schemas/graph.yaml#/$defs/port-base + additionalProperties: false + + properties: + endpoint: + $ref: /schemas/media/video-interfaces.yaml# + unevaluatedProperties: false + + properties: + data-lanes: + minItems: 1 + maxItems: 2 + items: + enum: [1, 2] + + clock-noncontinuous: true + +required: + - compatible + - reg + - clocks + - port + +additionalProperties: false + +examples: + - | + i2c { + #address-cells = <1>; + #size-cells = <0>; + + ov5670: sensor@36 { + compatible = "ovti,ov5670"; + reg = <0x36>; + + clocks = <&sensor_xclk>; + + port { + ov5670_ep: endpoint { + remote-endpoint = <&csi_ep>; + data-lanes = <1 2>; + clock-noncontinuous; + }; + }; + }; + }; + +... diff --git a/MAINTAINERS b/MAINTAINERS index f61eb221415b..38d8d1d5d536 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -15468,6 +15468,7 @@ M: Chiranjeevi Rapolu <chiranjeevi.rapolu@intel.com> L: linux-media@vger.kernel.org S: Maintained T: git git://linuxtv.org/media_tree.git +F: Documentation/devicetree/bindings/media/i2c/ovti,ov5670.yaml F: drivers/media/i2c/ov5670.c OMNIVISION OV5675 SENSOR DRIVER
Add the bindings documentation for Omnivision OV5670 image sensor. Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> --- .../bindings/media/i2c/ovti,ov5670.yaml | 92 +++++++++++++++++++ MAINTAINERS | 1 + 2 files changed, 93 insertions(+) create mode 100644 Documentation/devicetree/bindings/media/i2c/ovti,ov5670.yaml -- 2.39.0