Messages in this thread | | | From | <> | Subject | RE: [PATCH v5 1/6] dt-bindings: media: platform: visconti: Add Toshiba Visconti Video Input Interface bindings | Date | Mon, 30 Jan 2023 09:06:25 +0000 |
| |
> -----Original Message----- > From: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Sent: Monday, January 23, 2023 4:26 AM > To: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > Cc: ishikawa yuji(石川 悠司 ○RDC□AITC○EA開) > <yuji2.ishikawa@toshiba.co.jp>; Hans Verkuil <hverkuil@xs4all.nl>; Mauro > Carvalho Chehab <mchehab@kernel.org>; iwamatsu nobuhiro(岩松 信洋 □S > WC◯ACT) <nobuhiro1.iwamatsu@toshiba.co.jp>; Rob Herring > <robh+dt@kernel.org>; Krzysztof Kozlowski > <krzysztof.kozlowski+dt@linaro.org>; Rafael J . Wysocki > <rafael.j.wysocki@intel.com>; Mark Brown <broonie@kernel.org>; > linux-media@vger.kernel.org; linux-arm-kernel@lists.infradead.org; > linux-kernel@vger.kernel.org; devicetree@vger.kernel.org > 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
| |