[PATCH 0/4] media: v4l: fwnode: Add support for CSI-2 C-PHY line orders

[PATCH 0/4] media: v4l: fwnode: Add support for CSI-2 C-PHY line orders

[Niklas Söderlund]

Hello,

This series aims to extend the video interface bindings with a new 
property to describe the CSI-2 C-PHY line orders. In comparison with 
CSI-2 D-PHY where each data-lane is constructed from two lines making up 
a differential pair the C-PHY uses three phase encoding constructed from 
three lines to create a single data lane.

The three lines making up the C-PHY data lane are labeled A, B and C in 
the specification and can be ordered in any combination (ABC, ACB, BAC, 
BCA, CAB and CBA). This can be compared to the D-PHY model where the two 
lines in the differential pair can be "normal" or "inverted" to do the 
same thing.

For the D-PHY uses-case the exists a property 'lane-polarities' can be 
used to describe this line wiring. However there are no property to 
describe this ordering of the C-PHY lines in the video interfaces 
definition nor in the V4L2 fwnode structures or parser.

Patch 1/4 extends the video bindings with a new property 'line-orders' 
which can describe this property of the C-PHY. The property name and the 
const values used for different line configurations are taken from the 
MIPI Discovery and Configuration (DisCo) Specification for Imaging 
document.

Patch 2/4 extends the V4L2 fwnode data structure and parser to consume 
and exposes this property to drivers.

While patch 3/4 and 4/4 adds an example use of the property both in the 
bindings and in the driver using the R-Car CSI-2 receiver driver on V4H.

A note on the changes to the R-Car driver not relevant to the core V4L2 
or bindings work. The V4H WhiteHawk development platform is the only 
model where the CSI-2 bus is used in a C-PHY configuration. Early 
datasheets where used to add support for it and at that time the line 
order registers where not documented so magic values where used as-is.  
This have been addressed in later versions of the datasheet and this can 
now be done properly.

The magic values used however configured one of the data lanes used in a 
BCA configuration, which is required for proper operation on that 
development platform. Thus the change in patch 4/4 breaks proper 
operation with older DTS files lacking the new line-orders property.

I think this is fine as the only known use-case for this platform is 
together with the MAX96712 CSI-2 transmitter and for this we only have a 
staging driver capable of generating test patterns. To extend this to a 
capture pipeline capable of capturing frames from a real source DTS 
changes are needed to describe the video source, so an updated DTS are
need anyhow.

Niklas Söderlund (4):
  media: dt-bindings: Add property to describe CSI-2 C-PHY line orders
  media: v4l: fwnode: Parse MiPI DisCo for C-PHY line-orders
  arm64: dts: renesas: white-hawk-csi-dsi: Define CSI-2 data line orders
  media: rcar-csi2: Allow specifying C-PHY line order

 .../bindings/media/video-interfaces.yaml      | 20 +++++
 .../boot/dts/renesas/white-hawk-csi-dsi.dtsi  |  6 ++
 drivers/media/platform/renesas/rcar-csi2.c    | 74 +++++++++++++++++--
 drivers/media/v4l2-core/v4l2-fwnode.c         | 56 +++++++++++++-
 include/dt-bindings/media/video-interfaces.h  |  7 ++
 include/media/v4l2-mediabus.h                 | 21 ++++++
 6 files changed, 176 insertions(+), 8 deletions(-)

-- 
2.47.0

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

[Niklas Söderlund]

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

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

[Rob Herring (Arm)]

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.

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

[Sakari Ailus]

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

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

[Niklas Söderlund]

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

-- 
Kind Regards,
Niklas Söderlund

[PATCH 2/4] media: v4l: fwnode: Parse MiPI DisCo for C-PHY line-orders

[Niklas Söderlund]

Extend the fwnode parsing to validate and fill in the CSI-2 C-PHY
line-orders order properties as defined in MIPI Discovery and
Configuration (DisCo) Specification for Imaging.

Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
---
 drivers/media/v4l2-core/v4l2-fwnode.c | 56 ++++++++++++++++++++++++++-
 include/media/v4l2-mediabus.h         | 21 ++++++++++
 2 files changed, 76 insertions(+), 1 deletion(-)

diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c
index f19c8adf2c61..b8b2b7fb685e 100644
--- a/drivers/media/v4l2-core/v4l2-fwnode.c
+++ b/drivers/media/v4l2-core/v4l2-fwnode.c
@@ -127,7 +127,7 @@ static int v4l2_fwnode_endpoint_parse_csi2_bus(struct fwnode_handle *fwnode,
 {
 	struct v4l2_mbus_config_mipi_csi2 *bus = &vep->bus.mipi_csi2;
 	bool have_clk_lane = false, have_data_lanes = false,
-		have_lane_polarities = false;
+		have_lane_polarities = false, have_line_orders = false;
 	unsigned int flags = 0, lanes_used = 0;
 	u32 array[1 + V4L2_MBUS_CSI2_MAX_DATA_LANES];
 	u32 clock_lane = 0;
@@ -197,6 +197,17 @@ static int v4l2_fwnode_endpoint_parse_csi2_bus(struct fwnode_handle *fwnode,
 		have_lane_polarities = true;
 	}
 
+	rval = fwnode_property_count_u32(fwnode, "line-orders");
+	if (rval > 0) {
+		if (rval != num_data_lanes) {
+			pr_warn("invalid number of line-orders entries (need %u, got %u)\n",
+				num_data_lanes, rval);
+			return -EINVAL;
+		}
+
+		have_line_orders = true;
+	}
+
 	if (!fwnode_property_read_u32(fwnode, "clock-lanes", &v)) {
 		clock_lane = v;
 		pr_debug("clock lane position %u\n", v);
@@ -250,6 +261,49 @@ static int v4l2_fwnode_endpoint_parse_csi2_bus(struct fwnode_handle *fwnode,
 		} else {
 			pr_debug("no lane polarities defined, assuming not inverted\n");
 		}
+
+		if (have_line_orders) {
+			fwnode_property_read_u32_array(fwnode,
+						       "line-orders", array,
+						       num_data_lanes);
+
+			for (i = 0; i < num_data_lanes; i++) {
+				const char *order;
+
+				switch (array[i]) {
+				case 0:
+					order = "ABC";
+					break;
+				case 1:
+					order = "ACB";
+					break;
+				case 2:
+					order = "BAC";
+					break;
+				case 3:
+					order = "BCA";
+					break;
+				case 4:
+					order = "CAB";
+					break;
+				case 5:
+					order = "CBA";
+					break;
+				default:
+					pr_warn("lane %u invalid line-order assuming ABC (got %u)\n",
+						i, array[i]);
+					bus->line_orders[i] = V4L2_MBUS_CSI2_CPHY_LINE_ORDER_ABC;
+					continue;
+				}
+				bus->line_orders[i] = array[i];
+				pr_debug("lane %u line order %s", i, order);
+			}
+		} else {
+			for (i = 0; i < num_data_lanes; i++)
+				bus->line_orders[i] = V4L2_MBUS_CSI2_CPHY_LINE_ORDER_ABC;
+
+			pr_debug("no line orders defined, assuming ABC\n");
+		}
 	}
 
 	return 0;
diff --git a/include/media/v4l2-mediabus.h b/include/media/v4l2-mediabus.h
index 5bce6e423e94..e7f019f68c8d 100644
--- a/include/media/v4l2-mediabus.h
+++ b/include/media/v4l2-mediabus.h
@@ -73,6 +73,24 @@
 
 #define V4L2_MBUS_CSI2_MAX_DATA_LANES		8
 
+/**
+ * enum v4l2_mbus_csi2_cphy_line_orders_type - CSI-2 C-PHY line order
+ * @V4L2_MBUS_CSI2_CPHY_LINE_ORDER_ABC: C-PHY line order ABC (default)
+ * @V4L2_MBUS_CSI2_CPHY_LINE_ORDER_ACB: C-PHY line order ACB
+ * @V4L2_MBUS_CSI2_CPHY_LINE_ORDER_BAC: C-PHY line order BAC
+ * @V4L2_MBUS_CSI2_CPHY_LINE_ORDER_BCA: C-PHY line order BCA
+ * @V4L2_MBUS_CSI2_CPHY_LINE_ORDER_CAB: C-PHY line order CAB
+ * @V4L2_MBUS_CSI2_CPHY_LINE_ORDER_CBA: C-PHY line order CBA
+ */
+enum v4l2_mbus_csi2_cphy_line_orders_type {
+	V4L2_MBUS_CSI2_CPHY_LINE_ORDER_ABC,
+	V4L2_MBUS_CSI2_CPHY_LINE_ORDER_ACB,
+	V4L2_MBUS_CSI2_CPHY_LINE_ORDER_BAC,
+	V4L2_MBUS_CSI2_CPHY_LINE_ORDER_BCA,
+	V4L2_MBUS_CSI2_CPHY_LINE_ORDER_CAB,
+	V4L2_MBUS_CSI2_CPHY_LINE_ORDER_CBA,
+};
+
 /**
  * struct v4l2_mbus_config_mipi_csi2 - MIPI CSI-2 data bus configuration
  * @flags: media bus (V4L2_MBUS_*) flags
@@ -81,6 +99,8 @@
  * @num_data_lanes: number of data lanes
  * @lane_polarities: polarity of the lanes. The order is the same of
  *		   the physical lanes.
+ * @line_orders: line order of the data lanes. The order is the same of the
+ *		   physical lanes.
  */
 struct v4l2_mbus_config_mipi_csi2 {
 	unsigned int flags;
@@ -88,6 +108,7 @@ struct v4l2_mbus_config_mipi_csi2 {
 	unsigned char clock_lane;
 	unsigned char num_data_lanes;
 	bool lane_polarities[1 + V4L2_MBUS_CSI2_MAX_DATA_LANES];
+	enum v4l2_mbus_csi2_cphy_line_orders_type line_orders[V4L2_MBUS_CSI2_MAX_DATA_LANES];
 };
 
 /**
-- 
2.47.0

Re: [PATCH 2/4] media: v4l: fwnode: Parse MiPI DisCo for C-PHY line-orders

[Sakari Ailus]

Hejssan,

On Tue, Nov 19, 2024 at 11:12:47PM +0100, Niklas Söderlund wrote:
> Extend the fwnode parsing to validate and fill in the CSI-2 C-PHY
> line-orders order properties as defined in MIPI Discovery and
> Configuration (DisCo) Specification for Imaging.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> ---
>  drivers/media/v4l2-core/v4l2-fwnode.c | 56 ++++++++++++++++++++++++++-
>  include/media/v4l2-mediabus.h         | 21 ++++++++++
>  2 files changed, 76 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c
> index f19c8adf2c61..b8b2b7fb685e 100644
> --- a/drivers/media/v4l2-core/v4l2-fwnode.c
> +++ b/drivers/media/v4l2-core/v4l2-fwnode.c
> @@ -127,7 +127,7 @@ static int v4l2_fwnode_endpoint_parse_csi2_bus(struct fwnode_handle *fwnode,
>  {
>  	struct v4l2_mbus_config_mipi_csi2 *bus = &vep->bus.mipi_csi2;
>  	bool have_clk_lane = false, have_data_lanes = false,
> -		have_lane_polarities = false;
> +		have_lane_polarities = false, have_line_orders = false;
>  	unsigned int flags = 0, lanes_used = 0;
>  	u32 array[1 + V4L2_MBUS_CSI2_MAX_DATA_LANES];
>  	u32 clock_lane = 0;
> @@ -197,6 +197,17 @@ static int v4l2_fwnode_endpoint_parse_csi2_bus(struct fwnode_handle *fwnode,
>  		have_lane_polarities = true;
>  	}
>  
> +	rval = fwnode_property_count_u32(fwnode, "line-orders");
> +	if (rval > 0) {
> +		if (rval != num_data_lanes) {
> +			pr_warn("invalid number of line-orders entries (need %u, got %u)\n",
> +				num_data_lanes, rval);
> +			return -EINVAL;
> +		}
> +
> +		have_line_orders = true;
> +	}
> +
>  	if (!fwnode_property_read_u32(fwnode, "clock-lanes", &v)) {
>  		clock_lane = v;
>  		pr_debug("clock lane position %u\n", v);
> @@ -250,6 +261,49 @@ static int v4l2_fwnode_endpoint_parse_csi2_bus(struct fwnode_handle *fwnode,
>  		} else {
>  			pr_debug("no lane polarities defined, assuming not inverted\n");
>  		}
> +
> +		if (have_line_orders) {
> +			fwnode_property_read_u32_array(fwnode,
> +						       "line-orders", array,
> +						       num_data_lanes);
> +
> +			for (i = 0; i < num_data_lanes; i++) {
> +				const char *order;
> +
> +				switch (array[i]) {
> +				case 0:
> +					order = "ABC";
> +					break;
> +				case 1:
> +					order = "ACB";
> +					break;
> +				case 2:
> +					order = "BAC";
> +					break;
> +				case 3:
> +					order = "BCA";
> +					break;
> +				case 4:
> +					order = "CAB";
> +					break;
> +				case 5:
> +					order = "CBA";
> +					break;

Please use an array instead.

> +				default:
> +					pr_warn("lane %u invalid line-order assuming ABC (got %u)\n",
> +						i, array[i]);
> +					bus->line_orders[i] = V4L2_MBUS_CSI2_CPHY_LINE_ORDER_ABC;
> +					continue;
> +				}
> +				bus->line_orders[i] = array[i];
> +				pr_debug("lane %u line order %s", i, order);
> +			}
> +		} else {
> +			for (i = 0; i < num_data_lanes; i++)
> +				bus->line_orders[i] = V4L2_MBUS_CSI2_CPHY_LINE_ORDER_ABC;

A few lines could be wrapped above.

> +
> +			pr_debug("no line orders defined, assuming ABC\n");
> +		}
>  	}
>  
>  	return 0;
> diff --git a/include/media/v4l2-mediabus.h b/include/media/v4l2-mediabus.h
> index 5bce6e423e94..e7f019f68c8d 100644
> --- a/include/media/v4l2-mediabus.h
> +++ b/include/media/v4l2-mediabus.h
> @@ -73,6 +73,24 @@
>  
>  #define V4L2_MBUS_CSI2_MAX_DATA_LANES		8
>  
> +/**
> + * enum v4l2_mbus_csi2_cphy_line_orders_type - CSI-2 C-PHY line order
> + * @V4L2_MBUS_CSI2_CPHY_LINE_ORDER_ABC: C-PHY line order ABC (default)
> + * @V4L2_MBUS_CSI2_CPHY_LINE_ORDER_ACB: C-PHY line order ACB
> + * @V4L2_MBUS_CSI2_CPHY_LINE_ORDER_BAC: C-PHY line order BAC
> + * @V4L2_MBUS_CSI2_CPHY_LINE_ORDER_BCA: C-PHY line order BCA
> + * @V4L2_MBUS_CSI2_CPHY_LINE_ORDER_CAB: C-PHY line order CAB
> + * @V4L2_MBUS_CSI2_CPHY_LINE_ORDER_CBA: C-PHY line order CBA
> + */
> +enum v4l2_mbus_csi2_cphy_line_orders_type {
> +	V4L2_MBUS_CSI2_CPHY_LINE_ORDER_ABC,
> +	V4L2_MBUS_CSI2_CPHY_LINE_ORDER_ACB,
> +	V4L2_MBUS_CSI2_CPHY_LINE_ORDER_BAC,
> +	V4L2_MBUS_CSI2_CPHY_LINE_ORDER_BCA,
> +	V4L2_MBUS_CSI2_CPHY_LINE_ORDER_CAB,
> +	V4L2_MBUS_CSI2_CPHY_LINE_ORDER_CBA,
> +};
> +
>  /**
>   * struct v4l2_mbus_config_mipi_csi2 - MIPI CSI-2 data bus configuration
>   * @flags: media bus (V4L2_MBUS_*) flags
> @@ -81,6 +99,8 @@
>   * @num_data_lanes: number of data lanes
>   * @lane_polarities: polarity of the lanes. The order is the same of
>   *		   the physical lanes.
> + * @line_orders: line order of the data lanes. The order is the same of the
> + *		   physical lanes.
>   */
>  struct v4l2_mbus_config_mipi_csi2 {
>  	unsigned int flags;
> @@ -88,6 +108,7 @@ struct v4l2_mbus_config_mipi_csi2 {
>  	unsigned char clock_lane;
>  	unsigned char num_data_lanes;
>  	bool lane_polarities[1 + V4L2_MBUS_CSI2_MAX_DATA_LANES];
> +	enum v4l2_mbus_csi2_cphy_line_orders_type line_orders[V4L2_MBUS_CSI2_MAX_DATA_LANES];
>  };
>  
>  /**

-- 
Med vänliga hälsningar,

Sakari Ailus

Re: [PATCH 2/4] media: v4l: fwnode: Parse MiPI DisCo for C-PHY line-orders

[Niklas Söderlund]

Hello Sakari,

On 2024-11-20 08:10:42 +0000, Sakari Ailus wrote:
> Hejssan,
> 
> On Tue, Nov 19, 2024 at 11:12:47PM +0100, Niklas Söderlund wrote:
> > Extend the fwnode parsing to validate and fill in the CSI-2 C-PHY
> > line-orders order properties as defined in MIPI Discovery and
> > Configuration (DisCo) Specification for Imaging.
> > 
> > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> > ---
> >  drivers/media/v4l2-core/v4l2-fwnode.c | 56 ++++++++++++++++++++++++++-
> >  include/media/v4l2-mediabus.h         | 21 ++++++++++
> >  2 files changed, 76 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c
> > index f19c8adf2c61..b8b2b7fb685e 100644
> > --- a/drivers/media/v4l2-core/v4l2-fwnode.c
> > +++ b/drivers/media/v4l2-core/v4l2-fwnode.c
> > @@ -127,7 +127,7 @@ static int v4l2_fwnode_endpoint_parse_csi2_bus(struct fwnode_handle *fwnode,
> >  {
> >  	struct v4l2_mbus_config_mipi_csi2 *bus = &vep->bus.mipi_csi2;
> >  	bool have_clk_lane = false, have_data_lanes = false,
> > -		have_lane_polarities = false;
> > +		have_lane_polarities = false, have_line_orders = false;
> >  	unsigned int flags = 0, lanes_used = 0;
> >  	u32 array[1 + V4L2_MBUS_CSI2_MAX_DATA_LANES];
> >  	u32 clock_lane = 0;
> > @@ -197,6 +197,17 @@ static int v4l2_fwnode_endpoint_parse_csi2_bus(struct fwnode_handle *fwnode,
> >  		have_lane_polarities = true;
> >  	}
> >  
> > +	rval = fwnode_property_count_u32(fwnode, "line-orders");
> > +	if (rval > 0) {
> > +		if (rval != num_data_lanes) {
> > +			pr_warn("invalid number of line-orders entries (need %u, got %u)\n",
> > +				num_data_lanes, rval);
> > +			return -EINVAL;
> > +		}
> > +
> > +		have_line_orders = true;
> > +	}
> > +
> >  	if (!fwnode_property_read_u32(fwnode, "clock-lanes", &v)) {
> >  		clock_lane = v;
> >  		pr_debug("clock lane position %u\n", v);
> > @@ -250,6 +261,49 @@ static int v4l2_fwnode_endpoint_parse_csi2_bus(struct fwnode_handle *fwnode,
> >  		} else {
> >  			pr_debug("no lane polarities defined, assuming not inverted\n");
> >  		}
> > +
> > +		if (have_line_orders) {
> > +			fwnode_property_read_u32_array(fwnode,
> > +						       "line-orders", array,
> > +						       num_data_lanes);
> > +
> > +			for (i = 0; i < num_data_lanes; i++) {
> > +				const char *order;
> > +
> > +				switch (array[i]) {
> > +				case 0:
> > +					order = "ABC";
> > +					break;
> > +				case 1:
> > +					order = "ACB";
> > +					break;
> > +				case 2:
> > +					order = "BAC";
> > +					break;
> > +				case 3:
> > +					order = "BCA";
> > +					break;
> > +				case 4:
> > +					order = "CAB";
> > +					break;
> > +				case 5:
> > +					order = "CBA";
> > +					break;
> 
> Please use an array instead.
> 
> > +				default:
> > +					pr_warn("lane %u invalid line-order assuming ABC (got %u)\n",
> > +						i, array[i]);
> > +					bus->line_orders[i] = V4L2_MBUS_CSI2_CPHY_LINE_ORDER_ABC;
> > +					continue;
> > +				}
> > +				bus->line_orders[i] = array[i];
> > +				pr_debug("lane %u line order %s", i, order);
> > +			}
> > +		} else {
> > +			for (i = 0; i < num_data_lanes; i++)
> > +				bus->line_orders[i] = V4L2_MBUS_CSI2_CPHY_LINE_ORDER_ABC;
> 
> A few lines could be wrapped above.

I'm not sure I understand this comment. Do you mean I could loop over 
num_data_lanes and initialize all lines to ABC before checking 
have_line_orders and that way avoid having to loop here and set the 
default ABC if we are out-of bounds in the switch?

> 
> > +
> > +			pr_debug("no line orders defined, assuming ABC\n");
> > +		}
> >  	}
> >  
> >  	return 0;
> > diff --git a/include/media/v4l2-mediabus.h b/include/media/v4l2-mediabus.h
> > index 5bce6e423e94..e7f019f68c8d 100644
> > --- a/include/media/v4l2-mediabus.h
> > +++ b/include/media/v4l2-mediabus.h
> > @@ -73,6 +73,24 @@
> >  
> >  #define V4L2_MBUS_CSI2_MAX_DATA_LANES		8
> >  
> > +/**
> > + * enum v4l2_mbus_csi2_cphy_line_orders_type - CSI-2 C-PHY line order
> > + * @V4L2_MBUS_CSI2_CPHY_LINE_ORDER_ABC: C-PHY line order ABC (default)
> > + * @V4L2_MBUS_CSI2_CPHY_LINE_ORDER_ACB: C-PHY line order ACB
> > + * @V4L2_MBUS_CSI2_CPHY_LINE_ORDER_BAC: C-PHY line order BAC
> > + * @V4L2_MBUS_CSI2_CPHY_LINE_ORDER_BCA: C-PHY line order BCA
> > + * @V4L2_MBUS_CSI2_CPHY_LINE_ORDER_CAB: C-PHY line order CAB
> > + * @V4L2_MBUS_CSI2_CPHY_LINE_ORDER_CBA: C-PHY line order CBA
> > + */
> > +enum v4l2_mbus_csi2_cphy_line_orders_type {
> > +	V4L2_MBUS_CSI2_CPHY_LINE_ORDER_ABC,
> > +	V4L2_MBUS_CSI2_CPHY_LINE_ORDER_ACB,
> > +	V4L2_MBUS_CSI2_CPHY_LINE_ORDER_BAC,
> > +	V4L2_MBUS_CSI2_CPHY_LINE_ORDER_BCA,
> > +	V4L2_MBUS_CSI2_CPHY_LINE_ORDER_CAB,
> > +	V4L2_MBUS_CSI2_CPHY_LINE_ORDER_CBA,
> > +};
> > +
> >  /**
> >   * struct v4l2_mbus_config_mipi_csi2 - MIPI CSI-2 data bus configuration
> >   * @flags: media bus (V4L2_MBUS_*) flags
> > @@ -81,6 +99,8 @@
> >   * @num_data_lanes: number of data lanes
> >   * @lane_polarities: polarity of the lanes. The order is the same of
> >   *		   the physical lanes.
> > + * @line_orders: line order of the data lanes. The order is the same of the
> > + *		   physical lanes.
> >   */
> >  struct v4l2_mbus_config_mipi_csi2 {
> >  	unsigned int flags;
> > @@ -88,6 +108,7 @@ struct v4l2_mbus_config_mipi_csi2 {
> >  	unsigned char clock_lane;
> >  	unsigned char num_data_lanes;
> >  	bool lane_polarities[1 + V4L2_MBUS_CSI2_MAX_DATA_LANES];
> > +	enum v4l2_mbus_csi2_cphy_line_orders_type line_orders[V4L2_MBUS_CSI2_MAX_DATA_LANES];
> >  };
> >  
> >  /**
> 
> -- 
> Med vänliga hälsningar,
> 
> Sakari Ailus

-- 
Kind Regards,
Niklas Söderlund

Re: [PATCH 2/4] media: v4l: fwnode: Parse MiPI DisCo for C-PHY line-orders

[Sakari Ailus]

Hejssan, Niklas!

On Wed, Nov 20, 2024 at 10:50:30AM +0100, Niklas Söderlund wrote:
> Hello Sakari,
> 
> On 2024-11-20 08:10:42 +0000, Sakari Ailus wrote:
> > Hejssan,
> > 
> > On Tue, Nov 19, 2024 at 11:12:47PM +0100, Niklas Söderlund wrote:
> > > Extend the fwnode parsing to validate and fill in the CSI-2 C-PHY
> > > line-orders order properties as defined in MIPI Discovery and
> > > Configuration (DisCo) Specification for Imaging.
> > > 
> > > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> > > ---
> > >  drivers/media/v4l2-core/v4l2-fwnode.c | 56 ++++++++++++++++++++++++++-
> > >  include/media/v4l2-mediabus.h         | 21 ++++++++++
> > >  2 files changed, 76 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c
> > > index f19c8adf2c61..b8b2b7fb685e 100644
> > > --- a/drivers/media/v4l2-core/v4l2-fwnode.c
> > > +++ b/drivers/media/v4l2-core/v4l2-fwnode.c
> > > @@ -127,7 +127,7 @@ static int v4l2_fwnode_endpoint_parse_csi2_bus(struct fwnode_handle *fwnode,
> > >  {
> > >  	struct v4l2_mbus_config_mipi_csi2 *bus = &vep->bus.mipi_csi2;
> > >  	bool have_clk_lane = false, have_data_lanes = false,
> > > -		have_lane_polarities = false;
> > > +		have_lane_polarities = false, have_line_orders = false;
> > >  	unsigned int flags = 0, lanes_used = 0;
> > >  	u32 array[1 + V4L2_MBUS_CSI2_MAX_DATA_LANES];
> > >  	u32 clock_lane = 0;
> > > @@ -197,6 +197,17 @@ static int v4l2_fwnode_endpoint_parse_csi2_bus(struct fwnode_handle *fwnode,
> > >  		have_lane_polarities = true;
> > >  	}
> > >  
> > > +	rval = fwnode_property_count_u32(fwnode, "line-orders");
> > > +	if (rval > 0) {
> > > +		if (rval != num_data_lanes) {
> > > +			pr_warn("invalid number of line-orders entries (need %u, got %u)\n",
> > > +				num_data_lanes, rval);
> > > +			return -EINVAL;
> > > +		}
> > > +
> > > +		have_line_orders = true;
> > > +	}
> > > +
> > >  	if (!fwnode_property_read_u32(fwnode, "clock-lanes", &v)) {
> > >  		clock_lane = v;
> > >  		pr_debug("clock lane position %u\n", v);
> > > @@ -250,6 +261,49 @@ static int v4l2_fwnode_endpoint_parse_csi2_bus(struct fwnode_handle *fwnode,
> > >  		} else {
> > >  			pr_debug("no lane polarities defined, assuming not inverted\n");
> > >  		}
> > > +
> > > +		if (have_line_orders) {
> > > +			fwnode_property_read_u32_array(fwnode,
> > > +						       "line-orders", array,
> > > +						       num_data_lanes);
> > > +
> > > +			for (i = 0; i < num_data_lanes; i++) {
> > > +				const char *order;
> > > +
> > > +				switch (array[i]) {
> > > +				case 0:
> > > +					order = "ABC";
> > > +					break;
> > > +				case 1:
> > > +					order = "ACB";
> > > +					break;
> > > +				case 2:
> > > +					order = "BAC";
> > > +					break;
> > > +				case 3:
> > > +					order = "BCA";
> > > +					break;
> > > +				case 4:
> > > +					order = "CAB";
> > > +					break;
> > > +				case 5:
> > > +					order = "CBA";
> > > +					break;
> > 
> > Please use an array instead.
> > 
> > > +				default:
> > > +					pr_warn("lane %u invalid line-order assuming ABC (got %u)\n",
> > > +						i, array[i]);
> > > +					bus->line_orders[i] = V4L2_MBUS_CSI2_CPHY_LINE_ORDER_ABC;
> > > +					continue;
> > > +				}
> > > +				bus->line_orders[i] = array[i];
> > > +				pr_debug("lane %u line order %s", i, order);
> > > +			}
> > > +		} else {
> > > +			for (i = 0; i < num_data_lanes; i++)
> > > +				bus->line_orders[i] = V4L2_MBUS_CSI2_CPHY_LINE_ORDER_ABC;
> > 
> > A few lines could be wrapped above.
> 
> I'm not sure I understand this comment. Do you mean I could loop over 
> num_data_lanes and initialize all lines to ABC before checking 
> have_line_orders and that way avoid having to loop here and set the 
> default ABC if we are out-of bounds in the switch?

No, just that you'd wrap lines that are over 80 characters per line, unless
there's some tangible reason to have them like that.

-- 
Med vänliga hälsningar,

Sakari Ailus

[PATCH 3/4] arm64: dts: renesas: white-hawk-csi-dsi: Define CSI-2 data line orders

[Niklas Söderlund]

The second CSI-2 C-PHY data-lane have a different line order (BCA) then
the two other data-lanes (ABC) for both connected CSI-2 receivers,
describe this in the device tree.

This have worked in the past as the R-Car CSI-2 driver did not have
documentation for the line order configuration and a magic value was
written to the register for this specific setup. Now the registers
involved are documented and the hardware description as well as the
driver needs to be corrected.

Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
---
 arch/arm64/boot/dts/renesas/white-hawk-csi-dsi.dtsi | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/white-hawk-csi-dsi.dtsi b/arch/arm64/boot/dts/renesas/white-hawk-csi-dsi.dtsi
index 3006b0a64f41..a5d1c1008e7e 100644
--- a/arch/arm64/boot/dts/renesas/white-hawk-csi-dsi.dtsi
+++ b/arch/arm64/boot/dts/renesas/white-hawk-csi-dsi.dtsi
@@ -21,6 +21,9 @@ csi40_in: endpoint {
 				bus-type = <MEDIA_BUS_TYPE_CSI2_CPHY>;
 				clock-lanes = <0>;
 				data-lanes = <1 2 3>;
+				line-orders = <MEDIA_BUS_CSI2_CPHY_LINE_ORDER_ABC
+					       MEDIA_BUS_CSI2_CPHY_LINE_ORDER_BCA
+					       MEDIA_BUS_CSI2_CPHY_LINE_ORDER_ABC>;
 				remote-endpoint = <&max96712_out0>;
 			};
 		};
@@ -41,6 +44,9 @@ csi41_in: endpoint {
 				bus-type = <MEDIA_BUS_TYPE_CSI2_CPHY>;
 				clock-lanes = <0>;
 				data-lanes = <1 2 3>;
+				line-orders = <MEDIA_BUS_CSI2_CPHY_LINE_ORDER_ABC
+					       MEDIA_BUS_CSI2_CPHY_LINE_ORDER_BCA
+					       MEDIA_BUS_CSI2_CPHY_LINE_ORDER_ABC>;
 				remote-endpoint = <&max96712_out1>;
 			};
 		};
-- 
2.47.0

[PATCH 4/4] media: rcar-csi2: Allow specifying C-PHY line order

[Niklas Söderlund]

Later versions of the V4H datasheet adds documentation for the line
order register needed to support all possible configurations. Extend the
driver to take the line order for each data line into account when
configuring the device.

Unfortunately not all registers initially thought to be involved in line
order configuration where directly related. One magic value is still in
the driver and left as-is, but it is not related to line order as that
procedure have now been documented.

Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
---
 drivers/media/platform/renesas/rcar-csi2.c | 74 ++++++++++++++++++++--
 1 file changed, 67 insertions(+), 7 deletions(-)

diff --git a/drivers/media/platform/renesas/rcar-csi2.c b/drivers/media/platform/renesas/rcar-csi2.c
index 27ffdd28cbf7..8a0b6a68e194 100644
--- a/drivers/media/platform/renesas/rcar-csi2.c
+++ b/drivers/media/platform/renesas/rcar-csi2.c
@@ -183,17 +183,19 @@ struct rcar_csi2;
 #define V4H_CORE_DIG_IOCTRL_RW_AFE_CB_CTRL_2_REG(n)	(0x23840 + ((n) * 2)) /* n = 0 - 11 */
 #define V4H_CORE_DIG_RW_COMMON_REG(n)			(0x23880 + ((n) * 2)) /* n = 0 - 15 */
 #define V4H_CORE_DIG_ANACTRL_RW_COMMON_ANACTRL_REG(n)	(0x239e0 + ((n) * 2)) /* n = 0 - 3 */
-#define V4H_CORE_DIG_CLANE_1_RW_CFG_0_REG		0x2a400
 #define V4H_CORE_DIG_CLANE_1_RW_HS_TX_6_REG		0x2a60c
 
 /* V4H C-PHY */
 #define V4H_CORE_DIG_RW_TRIO0_REG(n)			(0x22100 + ((n) * 2)) /* n = 0 - 3 */
 #define V4H_CORE_DIG_RW_TRIO1_REG(n)			(0x22500 + ((n) * 2)) /* n = 0 - 3 */
 #define V4H_CORE_DIG_RW_TRIO2_REG(n)			(0x22900 + ((n) * 2)) /* n = 0 - 3 */
+#define V4H_CORE_DIG_CLANE_0_RW_CFG_0_REG		0x2a000
 #define V4H_CORE_DIG_CLANE_0_RW_LP_0_REG		0x2a080
 #define V4H_CORE_DIG_CLANE_0_RW_HS_RX_REG(n)		(0x2a100 + ((n) * 2)) /* n = 0 - 6 */
+#define V4H_CORE_DIG_CLANE_1_RW_CFG_0_REG		0x2a400
 #define V4H_CORE_DIG_CLANE_1_RW_LP_0_REG		0x2a480
 #define V4H_CORE_DIG_CLANE_1_RW_HS_RX_REG(n)		(0x2a500 + ((n) * 2)) /* n = 0 - 6 */
+#define V4H_CORE_DIG_CLANE_2_RW_CFG_0_REG		0x2a800
 #define V4H_CORE_DIG_CLANE_2_RW_LP_0_REG		0x2a880
 #define V4H_CORE_DIG_CLANE_2_RW_HS_RX_REG(n)		(0x2a900 + ((n) * 2)) /* n = 0 - 6 */
 
@@ -672,6 +674,21 @@ static const struct rcar_csi2_format *rcsi2_code_to_fmt(unsigned int code)
 	return NULL;
 }
 
+struct rcsi2_cphy_line_order {
+	enum v4l2_mbus_csi2_cphy_line_orders_type order;
+	u16 cfg;
+	u16 ctrl29;
+};
+
+static const struct rcsi2_cphy_line_order rcsi2_cphy_line_orders[] = {
+	{ .order = V4L2_MBUS_CSI2_CPHY_LINE_ORDER_ABC, .cfg = 0x0, .ctrl29 = 0x0 },
+	{ .order = V4L2_MBUS_CSI2_CPHY_LINE_ORDER_ACB, .cfg = 0xa, .ctrl29 = 0x1 },
+	{ .order = V4L2_MBUS_CSI2_CPHY_LINE_ORDER_BAC, .cfg = 0xc, .ctrl29 = 0x1 },
+	{ .order = V4L2_MBUS_CSI2_CPHY_LINE_ORDER_BCA, .cfg = 0x5, .ctrl29 = 0x0 },
+	{ .order = V4L2_MBUS_CSI2_CPHY_LINE_ORDER_CAB, .cfg = 0x3, .ctrl29 = 0x0 },
+	{ .order = V4L2_MBUS_CSI2_CPHY_LINE_ORDER_CBA, .cfg = 0x9, .ctrl29 = 0x1 }
+};
+
 enum rcar_csi2_pads {
 	RCAR_CSI2_SINK,
 	RCAR_CSI2_SOURCE_VC0,
@@ -722,6 +739,7 @@ struct rcar_csi2 {
 	bool cphy;
 	unsigned short lanes;
 	unsigned char lane_swap[4];
+	enum v4l2_mbus_csi2_cphy_line_orders_type line_orders[3];
 };
 
 static inline struct rcar_csi2 *sd_to_csi2(struct v4l2_subdev *sd)
@@ -754,11 +772,24 @@ static void rcsi2_write(struct rcar_csi2 *priv, unsigned int reg, u32 data)
 	iowrite32(data, priv->base + reg);
 }
 
+static u16 rcsi2_read16(struct rcar_csi2 *priv, unsigned int reg)
+{
+	return ioread16(priv->base + reg);
+}
+
 static void rcsi2_write16(struct rcar_csi2 *priv, unsigned int reg, u16 data)
 {
 	iowrite16(data, priv->base + reg);
 }
 
+static void rcsi2_modify16(struct rcar_csi2 *priv, unsigned int reg, u16 data, u16 mask)
+{
+	u16 val;
+
+	val = rcsi2_read16(priv, reg) & ~mask;
+	rcsi2_write16(priv, reg, val | data);
+}
+
 static int rcsi2_phtw_write(struct rcar_csi2 *priv, u8 data, u8 code)
 {
 	unsigned int timeout;
@@ -1112,6 +1143,26 @@ static int rcsi2_start_receiver_gen3(struct rcar_csi2 *priv,
 	return 0;
 }
 
+static void rsci2_set_line_order(struct rcar_csi2 *priv,
+				 enum v4l2_mbus_csi2_cphy_line_orders_type order,
+				 unsigned int cfgreg, unsigned int ctrlreg)
+{
+	const struct rcsi2_cphy_line_order *info = NULL;
+
+	for (unsigned int i = 0; i < ARRAY_SIZE(rcsi2_cphy_line_orders); i++) {
+		if (rcsi2_cphy_line_orders[i].order == order) {
+			info = &rcsi2_cphy_line_orders[i];
+			break;
+		}
+	}
+
+	if (!info)
+		return;
+
+	rcsi2_modify16(priv, cfgreg, info->cfg, 0x000f);
+	rcsi2_modify16(priv, ctrlreg, info->ctrl29, 0x0100);
+}
+
 static int rcsi2_wait_phy_start_v4h(struct rcar_csi2 *priv, u32 match)
 {
 	unsigned int timeout;
@@ -1189,12 +1240,18 @@ static int rcsi2_c_phy_setting_v4h(struct rcar_csi2 *priv, int msps)
 	rcsi2_write16(priv, V4H_CORE_DIG_RW_TRIO1_REG(1), conf->trio1);
 	rcsi2_write16(priv, V4H_CORE_DIG_RW_TRIO2_REG(1), conf->trio1);
 
-	/*
-	 * Configure pin-swap.
-	 * TODO: This registers is not documented yet, the values should depend
-	 * on the 'clock-lanes' and 'data-lanes' devicetree properties.
-	 */
-	rcsi2_write16(priv, V4H_CORE_DIG_CLANE_1_RW_CFG_0_REG, 0xf5);
+	/* Configure data line order. */
+	rsci2_set_line_order(priv, priv->line_orders[0],
+			     V4H_CORE_DIG_CLANE_0_RW_CFG_0_REG,
+			     V4H_CORE_DIG_IOCTRL_RW_AFE_LANE0_CTRL_2_REG(9));
+	rsci2_set_line_order(priv, priv->line_orders[1],
+			     V4H_CORE_DIG_CLANE_1_RW_CFG_0_REG,
+			     V4H_CORE_DIG_IOCTRL_RW_AFE_LANE1_CTRL_2_REG(9));
+	rsci2_set_line_order(priv, priv->line_orders[2],
+			     V4H_CORE_DIG_CLANE_2_RW_CFG_0_REG,
+			     V4H_CORE_DIG_IOCTRL_RW_AFE_LANE2_CTRL_2_REG(9));
+
+	/* TODO: This registers is not documented. */
 	rcsi2_write16(priv, V4H_CORE_DIG_CLANE_1_RW_HS_TX_6_REG, 0x5000);
 
 	/* Leave Shutdown mode */
@@ -1732,6 +1789,9 @@ static int rcsi2_parse_v4l2(struct rcar_csi2 *priv,
 		}
 	}
 
+	for (i = 0; i < ARRAY_SIZE(priv->line_orders); i++)
+		priv->line_orders[i] = vep->bus.mipi_csi2.line_orders[i];
+
 	return 0;
 }
 
-- 
2.47.0