linux-kernel - Re: [PATCH 2/2] media: dt-bindings: Use additionalProperties: false for endpoint: properties:

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20241014202920.GE5522@pendragon.ideasonboard.com>
Date: Mon, 14 Oct 2024 23:29:20 +0300
From: Laurent Pinchart <laurent.pinchart@...asonboard.com>
To: Krzysztof Kozlowski <krzk@...nel.org>
Cc: Bryan O'Donoghue <bryan.odonoghue@...aro.org>,
	Mauro Carvalho Chehab <mchehab@...nel.org>,
	Rob Herring <robh@...nel.org>,
	Krzysztof Kozlowski <krzk+dt@...nel.org>,
	Conor Dooley <conor+dt@...nel.org>,
	Dave Stevenson <dave.stevenson@...pberrypi.com>,
	Sakari Ailus <sakari.ailus@...ux.intel.com>,
	Shawn Guo <shawnguo@...nel.org>,
	Sascha Hauer <s.hauer@...gutronix.de>,
	Pengutronix Kernel Team <kernel@...gutronix.de>,
	Fabio Estevam <festevam@...il.com>,
	Martin Kepplinger <martink@...teo.de>,
	Paul Kocialkowski <paul.kocialkowski@...tlin.com>,
	"Paul J. Murphy" <paul.j.murphy@...el.com>,
	Daniele Alessandrelli <daniele.alessandrelli@...el.com>,
	Tommaso Merciai <tomm.merciai@...il.com>,
	Martin Hecht <martin.hecht@...et.eu>,
	Zhi Mao <zhi.mao@...iatek.com>,
	Alain Volmat <alain.volmat@...s.st.com>,
	Mikhail Rudenko <mike.rudenko@...il.com>,
	Ricardo Ribalda <ribalda@...nel.org>,
	Kieran Bingham <kieran.bingham@...asonboard.com>,
	Umang Jain <umang.jain@...asonboard.com>,
	Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>,
	Vladimir Zapolskiy <vladimir.zapolskiy@...aro.org>,
	Dongchun Zhu <dongchun.zhu@...iatek.com>,
	Quentin Schulz <quentin.schulz@...obroma-systems.com>,
	Todor Tomov <todor.too@...il.com>, linux-media@...r.kernel.org,
	devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
	imx@...ts.linux.dev, linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH 2/2] media: dt-bindings: Use additionalProperties: false
 for endpoint: properties:

On Mon, Oct 14, 2024 at 10:47:31AM +0200, Krzysztof Kozlowski wrote:
> On 14/10/2024 10:31, Bryan O'Donoghue wrote:
> > On 14/10/2024 08:45, Krzysztof Kozlowski wrote:
> >> I do not understand the reasoning behind this change at all. I don't
> >> think DT maintainers ever suggested it (in fact, rather opposite:
> >> suggested using unevaluatedProps) and I think is not a consensus of any
> >> talks.
> > 
> > No there is not but then, how do you give consistent feedback except 
> > proposing something to be a baseline.
> > 
> > On the one hand you have upstream additionalProperties: false and 
> > unevaluatedProperites: false - it'd be better to have a consistent 
> > message on which is to be used.
> 
> Well, I am afraid that push towards additionalProps will lead to grow
> common schema (video-interface-devices or video-interfaces) into huge
> one-fit-all binding. And that's not good.
> 
> If a common binding for a group of devices encourages you to list its
> subset, then it is not that common.
> 
> Solution is to fix that, e.g. split it per classes of devices.

I think splitting large schemas per class is a good idea, but the
problem will still exist. For instance, if we were to move the
CSI-2-specific properties to a separate schema, that schema would define
clock-lanes, data-lanes and clock-noncontinuous. The clock-lanes and
clock-noncontinuous properties do not apply to every device, how would
we then handle that ? I see three options:

- Use "additionalProperties: false" and explicitly list the properties that apply
  to the device with "$prop: true". This is what this series does.

- Use "unevaluatedProperites: false" and explicitly list the properties
  that do not apply to the device with "$prop: false". The drawback is
  that any property being added to the common schema will require
  modifications to all bindings that use the schema.

- Use "unevaluatedProperites: false" and don't list any property with
  "$prop: false". This is what is being done today in many bindings. The
  drawback is that device tree sources that specify invalid properties
  for the device will validate.

Among those options, my preference goes to the first one. It catches the
most issues in device tree sources, while not having the drawback of the
second option. It requires explicitly listing the valid properties, but
I don't consider that as a drawback, I think it's actually a good thing
as it clearly shows to the developers which properties are valid.

Are there other options I haven't considered ?

> Or don't care and use unevaluatedProps because it makes people's life
> easier and is still correct. If it is not correct, then this should be
> used as an argument.

-- 
Regards,

Laurent Pinchart