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

[Krzysztof Kozlowski]

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@xxxxxxxxxxxx>
> ---
> 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