Date:   Sun, 22 Jan 2023 21:25:40 +0200
From:   Laurent Pinchart <laurent.pinchart@ideasonboard.com>
To:     Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Cc:     Yuji Ishikawa <yuji2.ishikawa@toshiba.co.jp>,
        Hans Verkuil <hverkuil@xs4all.nl>,
        Mauro Carvalho Chehab <mchehab@kernel.org>,
        Nobuhiro Iwamatsu <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
Message-ID: <Y82NtJCtr+CZgS9k@pendragon.ideasonboard.com>
References: <20230111022433.25950-1-yuji2.ishikawa@toshiba.co.jp>
 <20230111022433.25950-2-yuji2.ishikawa@toshiba.co.jp>
 <Y8a+Hk2jFOjbkIvZ@pendragon.ideasonboard.com>
 <d1bb1148-e273-f5bb-bccc-ceca82bb6836@linaro.org>
 <Y8bFjhHkbNAKQK3t@pendragon.ideasonboard.com>
 <c2740d66-b51f-efc2-6583-a69bde950c68@linaro.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <c2740d66-b51f-efc2-6583-a69bde950c68@linaro.org>
Precedence: bulk

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

> > we differentiate between the latter two cases ?

-- 
Regards,

Laurent Pinchart