-----Original Message-----
From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Sent: Monday, January 23, 2023 4:26 AM
To: Krzysztof Kozlowski <redacted>
Cc: ishikawa yuji(石川 悠司 ○ＲＤＣ□ＡＩＴＣ○ＥＡ開)
[off-list ref]; Hans Verkuil [off-list ref]; Mauro
Carvalho Chehab [off-list ref]; iwamatsu nobuhiro(岩松 信洋 □Ｓ
ＷＣ◯ＡＣＴ) [off-list ref]; Rob Herring
[off-list ref]; Krzysztof Kozlowski
[off-list ref]; Rafael J . Wysocki
[off-list ref]; Mark Brown [off-list ref];
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:

quoted

On 17/01/2023 16:58, Laurent Pinchart wrote:

quoted

On Tue, Jan 17, 2023 at 04:42:51PM +0100, Krzysztof Kozlowski wrote:

quoted

On 17/01/2023 16:26, Laurent Pinchart wrote:

quoted

quoted

+
+          clock-lanes:
+            description: VIIF supports 1 clock line

s/line/lane/

Sorry for a late reply.
I'll fix the description.

quoted

quoted

quoted

quoted

quoted

+            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.

quoted

quoted

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.

quoted

quoted

video-interfaces.yaml defines lots of properties applicable to
endpoints. For a given device, those properties should be required

required:
 - foo

quoted

(easy, that's defined in the bindings), optional,

by default (with unevaluatedProperties:false) or explicitly mention
"foo: true (with additionalProperties:false)

quoted

 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" ?

quoted

quoted

we differentiate between the latter two cases ?

--
Regards,

Laurent Pinchart

Regards,
Yuji Ishikawa
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel