On Wed, Mar 18, 2026 at 07:12:46PM +0200, Laurent Pinchart wrote:

Hello Krzysztof,

On Mon, Mar 16, 2026 at 07:02:41PM +0100, Krzysztof Kozlowski wrote:

quoted

On 16/03/2026 18:19, Sakari Ailus wrote:

quoted

On Mon, Mar 16, 2026 at 03:42:09PM +0100, Krzysztof Kozlowski wrote:

quoted

On 16/03/2026 14:53, Laurent Pinchart wrote:

quoted

On Mon, Mar 16, 2026 at 02:45:34PM +0100, Krzysztof Kozlowski wrote:

quoted

The "endpoint" node references video-interfaces.yaml schema with
"unevaluatedProperties: false" which means that all properties from
referenced schema apply.  Listing some of them with ": true" is simply
redundant and does not make this code easier to read.

I think you know my opinion on this topic. I believe we would be better
off by turning "unevaluatedProperties: false" into
"additionalProperties: false" here, and keeping the list of applicable
properties. It brings value to device tree authors by telling which
properties are applicable to the device at hand. For instance ... (see
below)

(let me trim)

quoted

quoted

diff --git a/Documentation/devicetree/bindings/media/i2c/ovti,ov5647.yaml b/Documentation/devicetree/bindings/media/i2c/ovti,ov5647.yaml
index 2d7937a372a2..7a05a1eda58d 100644
--- a/Documentation/devicetree/bindings/media/i2c/ovti,ov5647.yaml
+++ b/Documentation/devicetree/bindings/media/i2c/ovti,ov5647.yaml

@@ -51,9 +51,6 @@ properties:
         $ref: /schemas/media/video-interfaces.yaml#
         unevaluatedProperties: false
 
-        properties:
-          clock-noncontinuous: true
-

... Without this, an integrator will need to dive into driver code to
know if non-continuous clock is usable for the device.

I see. Our usual interpretation of common schema, expressed by @Rob in
few places, that eventually all properties might be applicable. This
applies especially for ABI tied to the core Linux specifics, e.g.
rotation and orientation from video-interface-devices.yaml.

Absolutely every sensor can be mounted rotated, therefore every binding
referencing video-interface-devices should allow it, even if driver is
not using it. Because basically that's the ABI we want to define for
each sensor, thus each binding referencing common schema should have
"unevaluatedProps: true" without listing them.

Yes, that's totally fine. I agree that properties defined in
video-interface-devices.yaml should not be listed in individual
bindings. They're generic, and all of them are applicable to each image
sensor device. That part is fine, I think we have no disagreement.

quoted

quoted

quoted

Similarly touchscreen.yaml.

OTOH, second option, properties which are strictly hardware, e.g. name
of power supply or whether clock has or has not non-continuous mode,
should be allowed only when they match the hardware. Such bindings
should use "additionalProperties: false" so the hardware description is
constrained/fixed/specific.

The patch may be technically correct but I'm afraid it won't improve the
bindings but rather the opposite: it removes information telling whether a
property is relevant for a given device.

I bet there are a lot of possibilities to write invalid DTS while the
checker says it's fine (missing data-lanes or link-frequencies, for
instance). That may have been the case before the patch but I'd make
properties a driver needs to function mandatory rather than removing them
from bindings altogether.

That's pretty different problem and I am not removing any mandatory
properties. I changed absolutely nothing from functional point of view.

quoted

It'd been on my to-do list to split the current video-interfaces.yaml into
several files: generic camera sensor properties, CSI-2 interface
properties, DVP/Bt.656 interface properties and the rest (full list
probably requires more thought). That way we could only include properties
that are relevant for the device without necessarily listing each one for
all bindings.

I'd also continue to list boolean properties relevant for devices as well
as other properties that are relevant for a device but not mandatory.

I don't think there is such goal and particular subsystem does not get
exception here. What is relevant for device comes either from the
hardware or implemented ABI, as I explained. Bindings arbitrarily
choosing "I think this might be relevant" from some big schema with
irrelevant pieces is not manageable and not correct.

But that's not what we're discussing. The properties you're dropping
here are not "arbitrarily" choosen as being relevant. Whether it is
possible or not to use a non-continuous clock is a hardware property,
it's not an arbitrary choice.

Your patch will not change anything when it comes to validation of DT by
tools using the schema, but it drops important information relevant to
DT writers. What I recommend instead is to switch from
"unevaluatedProperties: false" to "additionalProperties: false". Not
only will we keep the information, but it will also be enforced properly
by tools.

I agree with keeping the information. Really, I'm indifferent, so if 
anyone finds it useful then let's leave it. I'm also not going to care 
in reviews either, so it's up to the media maintainers to care and 
ensure consistency.

Also, I don't think we can switch to additionalProperties here because 
then we have to list all the standard graph properties, too. That I do 
care about and don't care to see.

Rob