diff mbox series

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

Message ID 20241119221249.539610-2-niklas.soderlund+renesas@ragnatech.se (mailing list archive)
State New
Delegated to: Kieran Bingham
Headers show
Series media: v4l: fwnode: Add support for CSI-2 C-PHY line orders | expand

Commit Message

Niklas Söderlund Nov. 19, 2024, 10:12 p.m. UTC
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(+)

Comments

Rob Herring (Arm) Nov. 19, 2024, 11:26 p.m. UTC | #1
On Tue, 19 Nov 2024 23:12:46 +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(+)
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/media/video-interfaces.yaml: properties:line-orders: 'enum' should not be valid under {'enum': ['const', 'enum', 'exclusiveMaximum', 'exclusiveMinimum', 'minimum', 'maximum', 'multipleOf', 'pattern']}
	hint: Scalar and array keywords cannot be mixed
	from schema $id: http://devicetree.org/meta-schemas/keywords.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/media/video-interfaces.yaml: properties:line-orders: 'enum' should not be valid under {'enum': ['const', 'enum', 'exclusiveMaximum', 'exclusiveMinimum', 'minimum', 'maximum', 'multipleOf', 'pattern']}
	hint: Scalar and array keywords cannot be mixed
	from schema $id: http://devicetree.org/meta-schemas/keywords.yaml#

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20241119221249.539610-2-niklas.soderlund+renesas@ragnatech.se

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.
Sakari Ailus Nov. 20, 2024, 8:06 a.m. UTC | #2
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__ */
Niklas Söderlund Nov. 20, 2024, 9:47 a.m. UTC | #3
Hi Sakari,

Tack för din feedback!

On 2024-11-20 08:06:36 +0000, Sakari Ailus wrote:
> 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.

Yes the R-Car IP core documentation and schematics uses the ABC naming 
for the lines. Unfortunately the documentation is not public.

> 
> > +    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
diff mbox series

Patch

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
+    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
+      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
+      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__ */