Hi Jacopo, Krzysztof, On Sat, Jan 28, 2023 at 10:58:31AM +0100, Jacopo Mondi wrote: > 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@xxxxxxxxxxxxxxxx> > > >>>>> --- > > >>>> > > >>>> (...) > > >>>> > > >>>>> + > > >>>>> + 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). It's been in my plan to split this into multiple files so you could refer to fewer than all the properties. I have no schedule for this though. > > > 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. Me neither. -- Kind regards, Sakari Ailus
