Re: [PATCH 1/4] media: dt-bindings: Add property to describe CSI-2 C-PHY line orders

[Sakari Ailus <sakari.ailus@linux.intel.com>]

Hejssan, Niklas!

Tack för de här lapparna!

On Tue, Nov 19, 2024 at 11:12:46PM +0100, Niklas Söderlund wrote:
> Each data lane on a CSI-2 C-PHY bus uses three phase encoding and is
> constructed from three physical wires. The wires are referred to as A, B
> and C and their default order is ABC. However to ease hardware design
> the specification allows for the wires to be switched in any order.
> 
> Add a vendor neutral property to describe the line order used. The
> property name 'line-orders', the possible values it can be assigned and
> there names are taken from the MIPI Discovery and Configuration (DisCo)
> Specification for Imaging.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> ---
>  .../bindings/media/video-interfaces.yaml      | 20 +++++++++++++++++++
>  include/dt-bindings/media/video-interfaces.h  |  7 +++++++
>  2 files changed, 27 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/media/video-interfaces.yaml b/Documentation/devicetree/bindings/media/video-interfaces.yaml
> index 26e3e7d7c67b..95491e5779ba 100644
> --- a/Documentation/devicetree/bindings/media/video-interfaces.yaml
> +++ b/Documentation/devicetree/bindings/media/video-interfaces.yaml
> @@ -210,6 +210,26 @@ properties:
>        lane-polarities property is omitted, the value must be interpreted as 0
>        (normal). This property is valid for serial busses only.
>  
> +  line-orders:
> +    $ref: /schemas/types.yaml#/definitions/uint32-array
> +    minItems: 1
> +    maxItems: 8
> +    enum:
> +      - 0 # ABC
> +      - 1 # ACB
> +      - 2 # BAC
> +      - 3 # BCA
> +      - 4 # CAB
> +      - 5 # CBA

Do you know hardware documentation using lettes for the lines? I do agree
it seems less confusing but I've seen only numbers being used.

> +    description:
> +      An array of line orders of the CSI-2 C-PHY data lanes. The order of the
> +      lanes are the same as in data-lanes property. Valid values are 0-5 as
> +      defined in the MIPI Discovery and Configuration (DisCo) Specification for
> +      Imaging. The length of the array should be the same length as the

s/should/must/

As this is a requirement for DTS authors in particular.

> +      data-lanes property. If the line-orders property is omitted, the value
> +      must be interpreted as 0 (ABC). This property is valid for CSI-2 C-PHY

I would:

s/must/shall/

> +      busses only.
> +
>    strobe:
>      $ref: /schemas/types.yaml#/definitions/uint32
>      enum: [ 0, 1 ]
> diff --git a/include/dt-bindings/media/video-interfaces.h b/include/dt-bindings/media/video-interfaces.h
> index 68ac4e05e37f..88b9d05d8075 100644
> --- a/include/dt-bindings/media/video-interfaces.h
> +++ b/include/dt-bindings/media/video-interfaces.h
> @@ -13,4 +13,11 @@
>  #define MEDIA_BUS_TYPE_PARALLEL			5
>  #define MEDIA_BUS_TYPE_BT656			6
>  
> +#define MEDIA_BUS_CSI2_CPHY_LINE_ORDER_ABC	0
> +#define MEDIA_BUS_CSI2_CPHY_LINE_ORDER_ACB	1
> +#define MEDIA_BUS_CSI2_CPHY_LINE_ORDER_BAC	2
> +#define MEDIA_BUS_CSI2_CPHY_LINE_ORDER_BCA	3
> +#define MEDIA_BUS_CSI2_CPHY_LINE_ORDER_CAB	4
> +#define MEDIA_BUS_CSI2_CPHY_LINE_ORDER_CBA	5
> +
>  #endif /* __DT_BINDINGS_MEDIA_VIDEO_INTERFACES_H__ */

-- 
Med vänliga hälsningar,

Sakari Ailus