linux-kernel - Re: [PATCH] dt-bindings: media: i2c: imx219: Remove redundant description of data-lanes

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: <20250402-real-enthusiastic-ostrich-dcc243@krzk-bin>
Date: Wed, 2 Apr 2025 10:21:42 +0200
From: Krzysztof Kozlowski <krzk@...nel.org>
To: 
	Niklas Söderlund <niklas.soderlund+renesas@...natech.se>
Cc: Dave Stevenson <dave.stevenson@...pberrypi.com>, 
	Mauro Carvalho Chehab <mchehab@...nel.org>, Rob Herring <robh@...nel.org>, 
	Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>, 
	Shawn Guo <shawnguo@...nel.org>, Sascha Hauer <s.hauer@...gutronix.de>, 
	Pengutronix Kernel Team <kernel@...gutronix.de>, Fabio Estevam <festevam@...il.com>, linux-media@...r.kernel.org, 
	devicetree@...r.kernel.org, imx@...ts.linux.dev, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] dt-bindings: media: i2c: imx219: Remove redundant
 description of data-lanes

On Tue, Apr 01, 2025 at 04:57:58PM +0200, Niklas Söderlund wrote:
> The bindings already reference video-interfaces.yaml in the endpoint
> node, there is no need to duplicate the description of the data-lanes
> property.
> 
>   An array of physical data lane indexes. Position of an entry determines
>   the logical lane number, while the value of an entry indicates physical
>   lane, e.g. for 2-lane MIPI CSI-2 bus we could have "data-lanes = <1 2>;",
>   assuming the clock lane is on hardware lane 0. If the hardware does not
>   support lane reordering, monotonically incremented values shall be used
>   from 0 or 1 onwards, depending on whether or not there is also a clock
>   lane. This property is valid for serial busses only (e.g. MIPI CSI-2).

Please do not quote bindings in commit. It's never helpful.

> 
> What the generic binding do not cover is the behavior if the property
> would be omitted. But the imx219 driver have never agreed with the
> description neither. Before commit ceddfd4493b3 ("media: i2c: imx219:

It did not have to agree. See discussion for v3 of patch adding this binding.

> Support four-lane operation") the driver errored out if not 2 lanes
> where used, and after it if not 2 or 4 lanes where used.

Then... fix the driver?

This property describes hardware, not driver. Why current driver
implementation, e.g. 1 year ago or now, would change the hardware (so
the bindings)?

> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@...natech.se>
> ---
> Hello,
> 
> The data-lanes property is a common property and the driver have always
> operated as the common description, it seemed silly to break the driver
> to adhere to odd specification, then to correct the bindings. However a
> more correct solution would be to do the work on the driver of course.
> 
> This is just a drive-by fix in the hope of sparing others the time to
> discover this oddity themself. This is only tested by using the bindings
> themself and by 'make dt_binding_check'.
> ---
>  Documentation/devicetree/bindings/media/i2c/imx219.yaml | 9 ---------
>  1 file changed, 9 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/media/i2c/imx219.yaml b/Documentation/devicetree/bindings/media/i2c/imx219.yaml
> index 07d088cf66e0..31beeb2be2ea 100644
> --- a/Documentation/devicetree/bindings/media/i2c/imx219.yaml
> +++ b/Documentation/devicetree/bindings/media/i2c/imx219.yaml
> @@ -55,15 +55,6 @@ properties:
>          unevaluatedProperties: false
>  
>          properties:
> -          data-lanes:
> -            description: |-
> -              The sensor supports either two-lane, or four-lane operation.
> -              If this property is omitted four-lane operation is assumed.
> -              For two-lane operation the property must be set to <1 2>.
> -            items:
> -              - const: 1
> -              - const: 2

So 1 lane is also fine? 8 lanes are as well? Previously lack of the
property in DTS meant 4 lanes, now lack of property means anything.

Best regards,
Krzysztof