> -----Original Message----- > From: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx> > Sent: Monday, January 23, 2023 4:26 AM > To: Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx> > Cc: ishikawa yuji(石川 悠司 ○RDC□AITC○EA開) > <yuji2.ishikawa@xxxxxxxxxxxxx>; Hans Verkuil <hverkuil@xxxxxxxxx>; Mauro > Carvalho Chehab <mchehab@xxxxxxxxxx>; iwamatsu nobuhiro(岩松 信洋 □S > WC◯ACT) <nobuhiro1.iwamatsu@xxxxxxxxxxxxx>; Rob Herring > <robh+dt@xxxxxxxxxx>; Krzysztof Kozlowski > <krzysztof.kozlowski+dt@xxxxxxxxxx>; Rafael J . Wysocki > <rafael.j.wysocki@xxxxxxxxx>; Mark Brown <broonie@xxxxxxxxxx>; > linux-media@xxxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; > linux-kernel@xxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx > Subject: Re: [PATCH v5 1/6] dt-bindings: media: platform: visconti: Add Toshiba > Visconti Video Input Interface bindings > > Hi Krzysztof, > > On Tue, Jan 17, 2023 at 06:01:27PM +0100, Krzysztof Kozlowski wrote: > > On 17/01/2023 16:58, Laurent Pinchart wrote: > > > On Tue, Jan 17, 2023 at 04:42:51PM +0100, Krzysztof Kozlowski wrote: > > >> On 17/01/2023 16:26, Laurent Pinchart wrote: > > >>> > > >>>> + > > >>>> + clock-lanes: > > >>>> + description: VIIF supports 1 clock line > > >>> > > >>> s/line/lane/ Sorry for a late reply. I'll fix the description. > > >>> > > >>>> + const: 0 > > >>> > > >>> I would also add > > >>> > > >>> clock-noncontinuous: true > > >>> link-frequencies: true > > >>> > > >>> to indicate that the above two properties are used by this device. > > >> > > >> No, these are coming from other schema and there is never need to > > >> mention some property to indicate it is more used than other case. > > >> None of the bindings are created such way, so this should not be > exception. > > > > > > There are some bindings that do so, but that may not be a good > > > enough reason, as there's a chance I wrote those myself :-) > > > > > > I would have sworn that at some point in the past the schema > > > wouldn't have validated the example with this omitted. I'm not sure > > > if something changed or if I got this wrong. > > > > You probably think about case when using additionalProperties:false, > > where one has to explicitly list all valid properties. But not for > > unevaluatedProperties:false. > > Possibly, yes. > > > > video-interfaces.yaml defines lots of properties applicable to > > > endpoints. For a given device, those properties should be required > > > > required: > > - foo > > > > > (easy, that's defined in the bindings), optional, > > > > by default (with unevaluatedProperties:false) or explicitly mention > > "foo: true (with additionalProperties:false) > > > > > or forbidden. How do > > > > foo: false (with unevaluatedProperties:false) or by default (with > > additionalProperties:false) > > I think we should default to the latter. video-interfaces.yaml contains lots of > properties endpoint properties, most bindings will use less than half of them, so > having to explicitly list all the ones that are not used with "foo: false" would be > quite inconvenient. Furthermore, I expect more properties to be added to > video-interfaces.yaml over time, and those shouldn't be accepted by default in > existing bindings. > I caught up with this discussion after some exercise on JSON schema validator. I'll remove "unevaluatedProperties: false" at the "endpoint" and add "aditionalProperties: false" instead. Furthermore, I'll explicitly declare required properties (required: ["foo"]) and optional properties (properties: {foo: true}) for Visconti. Is this correct understanding? Are these changes also applied to "port", which is the parent node of the "endpoint" ? > > > we differentiate between the latter two cases ? > > -- > Regards, > > Laurent Pinchart Regards, Yuji Ishikawa