[PATCH v3 0/2] media: gc2145: add basic dvp bus support

[PATCH v3 0/2] media: gc2145: add basic dvp bus support

[Andrey Skvortsov]

Tested on PinePhone with libcamera-based application GNOME screenshot (45.2).

Andrey Skvortsov (2):
  dt-bindings: media: i2c: add galaxycore,gc2145 DVP bus support
  media: gc2145: implement basic dvp bus support

v3:
 - driver:
  - fixed formatting
  - added short commit description
  - removed unused defines, except GC2145_SYNC_MODE_OPCLK_GATE

v2:
 - bindings:
   - add required bus-type property
   - conditionally require link-frequency property based on bus-type
   - add DVP properties with their default values

 - driver:
  - fix fwnode parsing
  - remove gc2145_is_csi2
  - fix error message for unsupported bus-type

 .../bindings/media/i2c/galaxycore,gc2145.yaml |  65 +++++++++-
 drivers/media/i2c/gc2145.c                    | 112 ++++++++++++++----
 2 files changed, 150 insertions(+), 27 deletions(-)

-- 
2.43.0

[PATCH v3 1/2] dt-bindings: media: i2c: add galaxycore,gc2145 DVP bus support

[Andrey Skvortsov]

Signed-off-by: Andrey Skvortsov <andrej.skvortzov@gmail.com>
Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 .../bindings/media/i2c/galaxycore,gc2145.yaml | 65 ++++++++++++++++++-
 1 file changed, 64 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/media/i2c/galaxycore,gc2145.yaml b/Documentation/devicetree/bindings/media/i2c/galaxycore,gc2145.yaml
index 1726ecca4c77..3ca5bb94502d 100644
--- a/Documentation/devicetree/bindings/media/i2c/galaxycore,gc2145.yaml
+++ b/Documentation/devicetree/bindings/media/i2c/galaxycore,gc2145.yaml
@@ -61,8 +61,38 @@ properties:
         properties:
           link-frequencies: true
 
+          bus-type:
+            enum:
+              - 4 # CSI-2 D-PHY
+              - 5 # Parallel
+            default: 4
+
+          bus-width:
+            const: 8
+
+          hsync-active:
+            enum: [0, 1]
+            default: 1
+
+          vsync-active:
+            enum: [0, 1]
+            default: 1
+
+          pclk-sample:
+            enum: [0, 1]
+            default: 1
+
         required:
-          - link-frequencies
+          - bus-type
+
+        allOf:
+          - if:
+              properties:
+                bus-type:
+                  const: 4
+            then:
+              required:
+                - link-frequencies
 
     required:
       - endpoint
@@ -84,6 +114,7 @@ additionalProperties: false
 
 examples:
   - |
+    #include <dt-bindings/media/video-interfaces.h>
     #include <dt-bindings/gpio/gpio.h>
 
     i2c {
@@ -103,6 +134,7 @@ examples:
             port {
                 endpoint {
                     remote-endpoint = <&mipid02_0>;
+                    bus-type = <MEDIA_BUS_TYPE_CSI2_DPHY>;
                     data-lanes = <1 2>;
                     link-frequencies = /bits/ 64 <120000000 192000000 240000000>;
                 };
@@ -110,4 +142,35 @@ examples:
         };
     };
 
+  - |
+    #include <dt-bindings/media/video-interfaces.h>
+    #include <dt-bindings/gpio/gpio.h>
+
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        camera@3c {
+            compatible = "galaxycore,gc2145";
+            reg = <0x3c>;
+            clocks = <&clk_ext_camera>;
+            iovdd-supply = <&scmi_v3v3_sw>;
+            avdd-supply = <&scmi_v3v3_sw>;
+            dvdd-supply = <&scmi_v3v3_sw>;
+            powerdown-gpios = <&mcp23017 3 (GPIO_ACTIVE_LOW | GPIO_PUSH_PULL)>;
+            reset-gpios = <&mcp23017 4 (GPIO_ACTIVE_LOW | GPIO_PUSH_PULL)>;
+
+            port {
+                endpoint {
+                    remote-endpoint = <&parallel_from_gc2145>;
+                    bus-type = <MEDIA_BUS_TYPE_PARALLEL>;
+                    bus-width = <8>;
+                    hsync-active = <1>;
+                    vsync-active = <1>;
+                    pclk-sample = <1>;
+                };
+            };
+        };
+    };
+
 ...
-- 
2.43.0

Re: [PATCH v3 1/2] dt-bindings: media: i2c: add galaxycore,gc2145 DVP bus support

[Krzysztof Kozlowski]

On 04/05/2024 18:41, Andrey Skvortsov wrote:
> Signed-off-by: Andrey Skvortsov <andrej.skvortzov@gmail.com>
> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Not true. I never acked patch with empty commit and such content.

Drop fake tag and send new version with proper commit msg and proper
changelog under ---.

Best regards,
Krzysztof

Re: [PATCH v3 1/2] dt-bindings: media: i2c: add galaxycore,gc2145 DVP bus support

[Krzysztof Kozlowski]

On 05/05/2024 10:20, Krzysztof Kozlowski wrote:
> On 04/05/2024 18:41, Andrey Skvortsov wrote:
>> Signed-off-by: Andrey Skvortsov <andrej.skvortzov@gmail.com>
>> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> 
> Not true. I never acked patch with empty commit and such content.
> 
> Drop fake tag and send new version with proper commit msg and proper
> changelog under ---.
> 

I just noticed that such tag was added to v2 as well, so you added it to
something entirely else and keep going.

Best regards,
Krzysztof

Re: [PATCH v3 1/2] dt-bindings: media: i2c: add galaxycore,gc2145 DVP bus support

[Andrey Skvortsov]

Hi Krzysztof,

On 24-05-05 10:20, Krzysztof Kozlowski wrote:
> On 04/05/2024 18:41, Andrey Skvortsov wrote:
> > Signed-off-by: Andrey Skvortsov <andrej.skvortzov@gmail.com>
> > Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> 
> Not true. I never acked patch with empty commit and such content.
>
Sorry about that. You've acked v1, so I've kept it. I'll remove it in
v4.

> Drop fake tag and send new version with proper commit msg and proper
> changelog under ---.
There is changelog in cover letter. Should I include corresponding changelog in each
patch as well?

-- 
Best regards,
Andrey Skvortsov

Re: [PATCH v3 1/2] dt-bindings: media: i2c: add galaxycore,gc2145 DVP bus support

[Krzysztof Kozlowski]

On 05/05/2024 11:04, Andrey Skvortsov wrote:
> Hi Krzysztof,
> 
> On 24-05-05 10:20, Krzysztof Kozlowski wrote:
>> On 04/05/2024 18:41, Andrey Skvortsov wrote:
>>> Signed-off-by: Andrey Skvortsov <andrej.skvortzov@gmail.com>
>>> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>>
>> Not true. I never acked patch with empty commit and such content.
>>
> Sorry about that. You've acked v1, so I've kept it. I'll remove it in
> v4.

I acked entirely different version. It had even commit msg. Empty commit
would have never been acked.

> 
>> Drop fake tag and send new version with proper commit msg and proper
>> changelog under ---.
> There is changelog in cover letter. Should I include corresponding changelog in each
> patch as well?

It's fine there although nothing explains why you did these changes or
why commit msg is gone.

Best regards,
Krzysztof

Re: [PATCH v3 1/2] dt-bindings: media: i2c: add galaxycore,gc2145 DVP bus support

[Sakari Ailus]

Hi Andrey,

On Sat, May 04, 2024 at 07:41:14PM +0300, Andrey Skvortsov wrote:
> Signed-off-by: Andrey Skvortsov <andrej.skvortzov@gmail.com>
> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> ---
>  .../bindings/media/i2c/galaxycore,gc2145.yaml | 65 ++++++++++++++++++-
>  1 file changed, 64 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/media/i2c/galaxycore,gc2145.yaml b/Documentation/devicetree/bindings/media/i2c/galaxycore,gc2145.yaml
> index 1726ecca4c77..3ca5bb94502d 100644
> --- a/Documentation/devicetree/bindings/media/i2c/galaxycore,gc2145.yaml
> +++ b/Documentation/devicetree/bindings/media/i2c/galaxycore,gc2145.yaml
> @@ -61,8 +61,38 @@ properties:
>          properties:
>            link-frequencies: true
>  
> +          bus-type:
> +            enum:
> +              - 4 # CSI-2 D-PHY
> +              - 5 # Parallel
> +            default: 4
> +
> +          bus-width:
> +            const: 8
> +
> +          hsync-active:
> +            enum: [0, 1]
> +            default: 1
> +
> +          vsync-active:
> +            enum: [0, 1]
> +            default: 1
> +
> +          pclk-sample:
> +            enum: [0, 1]
> +            default: 1

These are relevant for bus-type 5 only.

> +
>          required:
> -          - link-frequencies
> +          - bus-type
> +
> +        allOf:
> +          - if:
> +              properties:
> +                bus-type:
> +                  const: 4
> +            then:
> +              required:
> +                - link-frequencies

I think I'd keep the link-frequencies for DVP as well.

>  
>      required:
>        - endpoint
> @@ -84,6 +114,7 @@ additionalProperties: false
>  
>  examples:
>    - |
> +    #include <dt-bindings/media/video-interfaces.h>
>      #include <dt-bindings/gpio/gpio.h>
>  
>      i2c {
> @@ -103,6 +134,7 @@ examples:
>              port {
>                  endpoint {
>                      remote-endpoint = <&mipid02_0>;
> +                    bus-type = <MEDIA_BUS_TYPE_CSI2_DPHY>;
>                      data-lanes = <1 2>;
>                      link-frequencies = /bits/ 64 <120000000 192000000 240000000>;
>                  };
> @@ -110,4 +142,35 @@ examples:
>          };
>      };
>  
> +  - |
> +    #include <dt-bindings/media/video-interfaces.h>
> +    #include <dt-bindings/gpio/gpio.h>
> +
> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        camera@3c {
> +            compatible = "galaxycore,gc2145";
> +            reg = <0x3c>;
> +            clocks = <&clk_ext_camera>;
> +            iovdd-supply = <&scmi_v3v3_sw>;
> +            avdd-supply = <&scmi_v3v3_sw>;
> +            dvdd-supply = <&scmi_v3v3_sw>;
> +            powerdown-gpios = <&mcp23017 3 (GPIO_ACTIVE_LOW | GPIO_PUSH_PULL)>;
> +            reset-gpios = <&mcp23017 4 (GPIO_ACTIVE_LOW | GPIO_PUSH_PULL)>;
> +
> +            port {
> +                endpoint {
> +                    remote-endpoint = <&parallel_from_gc2145>;
> +                    bus-type = <MEDIA_BUS_TYPE_PARALLEL>;
> +                    bus-width = <8>;
> +                    hsync-active = <1>;
> +                    vsync-active = <1>;
> +                    pclk-sample = <1>;
> +                };
> +            };
> +        };
> +    };
> +
>  ...

-- 
Kind regards,

Sakari Ailus

[PATCH v3 2/2] media: gc2145: implement basic dvp bus support

[Andrey Skvortsov]

That was tested on PinePhone with libcamera-based GNOME
screenshot (45.2).

Signed-off-by: Andrey Skvortsov <andrej.skvortzov@gmail.com>
---
 drivers/media/i2c/gc2145.c | 112 ++++++++++++++++++++++++++++---------
 1 file changed, 86 insertions(+), 26 deletions(-)

diff --git a/drivers/media/i2c/gc2145.c b/drivers/media/i2c/gc2145.c
index bef7b0e056a8..9808bd0ab6ae 100644
--- a/drivers/media/i2c/gc2145.c
+++ b/drivers/media/i2c/gc2145.c
@@ -39,6 +39,10 @@
 #define GC2145_REG_ANALOG_MODE1	CCI_REG8(0x17)
 #define GC2145_REG_OUTPUT_FMT	CCI_REG8(0x84)
 #define GC2145_REG_SYNC_MODE	CCI_REG8(0x86)
+#define GC2145_SYNC_MODE_VSYNC_POL	BIT(0)
+#define GC2145_SYNC_MODE_HSYNC_POL	BIT(1)
+#define GC2145_SYNC_MODE_OPCLK_POL	BIT(2)
+#define GC2145_SYNC_MODE_OPCLK_GATE	BIT(3)
 #define GC2145_SYNC_MODE_COL_SWITCH	BIT(4)
 #define GC2145_SYNC_MODE_ROW_SWITCH	BIT(5)
 #define GC2145_REG_BYPASS_MODE	CCI_REG8(0x89)
@@ -598,6 +602,7 @@ struct gc2145 {
 	struct v4l2_subdev sd;
 	struct media_pad pad;
 
+	struct v4l2_fwnode_endpoint ep; /* the parsed DT endpoint info */
 	struct regmap *regmap;
 	struct clk *xclk;
 
@@ -773,6 +778,36 @@ static int gc2145_set_pad_format(struct v4l2_subdev *sd,
 	return 0;
 }
 
+static int gc2145_config_dvp_mode(struct gc2145 *gc2145,
+				  const struct gc2145_format *gc2145_format)
+{
+	int ret = 0;
+	u64 sync_mode;
+	int flags = gc2145->ep.bus.parallel.flags;
+
+	ret = cci_read(gc2145->regmap, GC2145_REG_SYNC_MODE, &sync_mode, NULL);
+	if (ret)
+		return ret;
+
+	sync_mode &= ~(GC2145_SYNC_MODE_VSYNC_POL |
+		       GC2145_SYNC_MODE_HSYNC_POL |
+		       GC2145_SYNC_MODE_OPCLK_POL);
+
+	if (flags & V4L2_MBUS_VSYNC_ACTIVE_LOW)
+		sync_mode |= GC2145_SYNC_MODE_VSYNC_POL;
+
+	if (flags & V4L2_MBUS_HSYNC_ACTIVE_LOW)
+		sync_mode |= GC2145_SYNC_MODE_HSYNC_POL;
+
+	if (flags & V4L2_MBUS_PCLK_SAMPLE_FALLING)
+		sync_mode |= GC2145_SYNC_MODE_OPCLK_POL;
+
+	cci_write(gc2145->regmap, GC2145_REG_SYNC_MODE, sync_mode, &ret);
+	cci_write(gc2145->regmap, GC2145_REG_PAD_IO, 0x0f, &ret);
+
+	return ret;
+}
+
 static const struct cci_reg_sequence gc2145_common_mipi_regs[] = {
 	{GC2145_REG_PAGE_SELECT, 0x03},
 	{GC2145_REG_DPHY_ANALOG_MODE1, GC2145_DPHY_MODE_PHY_CLK_EN |
@@ -895,10 +930,13 @@ static int gc2145_start_streaming(struct gc2145 *gc2145,
 		goto err_rpm_put;
 	}
 
-	/* Perform MIPI specific configuration */
-	ret = gc2145_config_mipi_mode(gc2145, gc2145_format);
+	/* Perform interface specific configuration */
+	if (gc2145->ep.bus_type == V4L2_MBUS_CSI2_DPHY)
+		ret = gc2145_config_mipi_mode(gc2145, gc2145_format);
+	else
+		ret = gc2145_config_dvp_mode(gc2145, gc2145_format);
 	if (ret) {
-		dev_err(&client->dev, "%s failed to write mipi conf\n",
+		dev_err(&client->dev, "%s failed to write interface conf\n",
 			__func__);
 		goto err_rpm_put;
 	}
@@ -924,6 +962,9 @@ static void gc2145_stop_streaming(struct gc2145 *gc2145)
 			GC2145_CSI2_MODE_EN | GC2145_CSI2_MODE_MIPI_EN, 0,
 			&ret);
 	cci_write(gc2145->regmap, GC2145_REG_PAGE_SELECT, 0x00, &ret);
+
+	/* Disable dvp streaming */
+	cci_write(gc2145->regmap, GC2145_REG_PAD_IO, 0x00, &ret);
 	if (ret)
 		dev_err(&client->dev, "%s failed to write regs\n", __func__);
 
@@ -1233,9 +1274,8 @@ static int gc2145_init_controls(struct gc2145 *gc2145)
 static int gc2145_check_hwcfg(struct device *dev)
 {
 	struct fwnode_handle *endpoint;
-	struct v4l2_fwnode_endpoint ep_cfg = {
-		.bus_type = V4L2_MBUS_CSI2_DPHY
-	};
+	struct v4l2_subdev *sd = dev_get_drvdata(dev);
+	struct gc2145 *gc2145 = to_gc2145(sd);
 	int ret;
 
 	endpoint = fwnode_graph_get_next_endpoint(dev_fwnode(dev), NULL);
@@ -1244,36 +1284,55 @@ static int gc2145_check_hwcfg(struct device *dev)
 		return -EINVAL;
 	}
 
-	ret = v4l2_fwnode_endpoint_alloc_parse(endpoint, &ep_cfg);
+	gc2145->ep.bus_type = V4L2_MBUS_CSI2_DPHY;
+	ret = v4l2_fwnode_endpoint_alloc_parse(endpoint, &gc2145->ep);
+	if (ret) {
+		gc2145->ep.bus_type = V4L2_MBUS_PARALLEL;
+		ret = v4l2_fwnode_endpoint_alloc_parse(endpoint, &gc2145->ep);
+	}
 	fwnode_handle_put(endpoint);
-	if (ret)
+	if (ret) {
+		dev_err(dev, "could not parse endpoint\n");
 		return ret;
-
-	/* Check the number of MIPI CSI2 data lanes */
-	if (ep_cfg.bus.mipi_csi2.num_data_lanes != 2) {
-		dev_err(dev, "only 2 data lanes are currently supported\n");
-		ret = -EINVAL;
-		goto out;
 	}
 
-	/* Check the link frequency set in device tree */
-	if (!ep_cfg.nr_of_link_frequencies) {
-		dev_err(dev, "link-frequency property not found in DT\n");
+	switch (gc2145->ep.bus_type) {
+	case V4L2_MBUS_CSI2_DPHY:
+		/* Check the number of MIPI CSI2 data lanes */
+		if (gc2145->ep.bus.mipi_csi2.num_data_lanes != 2) {
+			dev_err(dev, "only 2 data lanes are currently supported\n");
+			ret = -EINVAL;
+			goto out;
+		}
+
+		/* Check the link frequency set in device tree */
+		if (!gc2145->ep.nr_of_link_frequencies) {
+			dev_err(dev, "link-frequencies property not found in DT\n");
+			ret = -EINVAL;
+			goto out;
+		}
+
+		if (gc2145->ep.nr_of_link_frequencies != 3 ||
+		    gc2145->ep.link_frequencies[0] != GC2145_640_480_LINKFREQ ||
+		    gc2145->ep.link_frequencies[1] != GC2145_1280_720_LINKFREQ ||
+		    gc2145->ep.link_frequencies[2] != GC2145_1600_1200_LINKFREQ) {
+			dev_err(dev, "Invalid link-frequencies provided\n");
+			ret = -EINVAL;
+			goto out;
+		}
+		break;
+	case V4L2_MBUS_PARALLEL:
+		break;
+	default:
+		dev_err(dev, "unsupported bus type %u\n", gc2145->ep.bus_type);
 		ret = -EINVAL;
 		goto out;
 	}
 
-	if (ep_cfg.nr_of_link_frequencies != 3 ||
-	    ep_cfg.link_frequencies[0] != GC2145_640_480_LINKFREQ ||
-	    ep_cfg.link_frequencies[1] != GC2145_1280_720_LINKFREQ ||
-	    ep_cfg.link_frequencies[2] != GC2145_1600_1200_LINKFREQ) {
-		dev_err(dev, "Invalid link-frequencies provided\n");
-		ret = -EINVAL;
-	}
+	return 0;
 
 out:
-	v4l2_fwnode_endpoint_free(&ep_cfg);
-
+	v4l2_fwnode_endpoint_free(&gc2145->ep);
 	return ret;
 }
 
@@ -1421,6 +1480,7 @@ static void gc2145_remove(struct i2c_client *client)
 	if (!pm_runtime_status_suspended(&client->dev))
 		gc2145_power_off(&client->dev);
 	pm_runtime_set_suspended(&client->dev);
+	v4l2_fwnode_endpoint_free(&gc2145->ep);
 }
 
 static const struct of_device_id gc2145_dt_ids[] = {
-- 
2.43.0

Re: [PATCH v3 2/2] media: gc2145: implement basic dvp bus support

[Krzysztof Kozlowski]

On 04/05/2024 18:41, Andrey Skvortsov wrote:
> That was tested on PinePhone with libcamera-based GNOME
> screenshot (45.2).

This tells me nothing why and what you are doing...


Best regards,
Krzysztof

Re: [PATCH v3 2/2] media: gc2145: implement basic dvp bus support

[Sakari Ailus]

Hi Andrey,

Thanks for the update.

On Sat, May 04, 2024 at 07:41:15PM +0300, Andrey Skvortsov wrote:
> That was tested on PinePhone with libcamera-based GNOME
> screenshot (45.2).
> 
> Signed-off-by: Andrey Skvortsov <andrej.skvortzov@gmail.com>
> ---
>  drivers/media/i2c/gc2145.c | 112 ++++++++++++++++++++++++++++---------
>  1 file changed, 86 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/media/i2c/gc2145.c b/drivers/media/i2c/gc2145.c
> index bef7b0e056a8..9808bd0ab6ae 100644
> --- a/drivers/media/i2c/gc2145.c
> +++ b/drivers/media/i2c/gc2145.c
> @@ -39,6 +39,10 @@
>  #define GC2145_REG_ANALOG_MODE1	CCI_REG8(0x17)
>  #define GC2145_REG_OUTPUT_FMT	CCI_REG8(0x84)
>  #define GC2145_REG_SYNC_MODE	CCI_REG8(0x86)
> +#define GC2145_SYNC_MODE_VSYNC_POL	BIT(0)
> +#define GC2145_SYNC_MODE_HSYNC_POL	BIT(1)
> +#define GC2145_SYNC_MODE_OPCLK_POL	BIT(2)
> +#define GC2145_SYNC_MODE_OPCLK_GATE	BIT(3)
>  #define GC2145_SYNC_MODE_COL_SWITCH	BIT(4)
>  #define GC2145_SYNC_MODE_ROW_SWITCH	BIT(5)
>  #define GC2145_REG_BYPASS_MODE	CCI_REG8(0x89)
> @@ -598,6 +602,7 @@ struct gc2145 {
>  	struct v4l2_subdev sd;
>  	struct media_pad pad;
>  
> +	struct v4l2_fwnode_endpoint ep; /* the parsed DT endpoint info */
>  	struct regmap *regmap;
>  	struct clk *xclk;
>  
> @@ -773,6 +778,36 @@ static int gc2145_set_pad_format(struct v4l2_subdev *sd,
>  	return 0;
>  }
>  
> +static int gc2145_config_dvp_mode(struct gc2145 *gc2145,
> +				  const struct gc2145_format *gc2145_format)
> +{
> +	int ret = 0;
> +	u64 sync_mode;
> +	int flags = gc2145->ep.bus.parallel.flags;
> +
> +	ret = cci_read(gc2145->regmap, GC2145_REG_SYNC_MODE, &sync_mode, NULL);
> +	if (ret)
> +		return ret;
> +
> +	sync_mode &= ~(GC2145_SYNC_MODE_VSYNC_POL |
> +		       GC2145_SYNC_MODE_HSYNC_POL |
> +		       GC2145_SYNC_MODE_OPCLK_POL);
> +
> +	if (flags & V4L2_MBUS_VSYNC_ACTIVE_LOW)
> +		sync_mode |= GC2145_SYNC_MODE_VSYNC_POL;
> +
> +	if (flags & V4L2_MBUS_HSYNC_ACTIVE_LOW)
> +		sync_mode |= GC2145_SYNC_MODE_HSYNC_POL;
> +
> +	if (flags & V4L2_MBUS_PCLK_SAMPLE_FALLING)
> +		sync_mode |= GC2145_SYNC_MODE_OPCLK_POL;
> +
> +	cci_write(gc2145->regmap, GC2145_REG_SYNC_MODE, sync_mode, &ret);
> +	cci_write(gc2145->regmap, GC2145_REG_PAD_IO, 0x0f, &ret);
> +
> +	return ret;
> +}
> +
>  static const struct cci_reg_sequence gc2145_common_mipi_regs[] = {
>  	{GC2145_REG_PAGE_SELECT, 0x03},
>  	{GC2145_REG_DPHY_ANALOG_MODE1, GC2145_DPHY_MODE_PHY_CLK_EN |
> @@ -895,10 +930,13 @@ static int gc2145_start_streaming(struct gc2145 *gc2145,
>  		goto err_rpm_put;
>  	}
>  
> -	/* Perform MIPI specific configuration */
> -	ret = gc2145_config_mipi_mode(gc2145, gc2145_format);
> +	/* Perform interface specific configuration */
> +	if (gc2145->ep.bus_type == V4L2_MBUS_CSI2_DPHY)
> +		ret = gc2145_config_mipi_mode(gc2145, gc2145_format);
> +	else
> +		ret = gc2145_config_dvp_mode(gc2145, gc2145_format);
>  	if (ret) {
> -		dev_err(&client->dev, "%s failed to write mipi conf\n",
> +		dev_err(&client->dev, "%s failed to write interface conf\n",
>  			__func__);
>  		goto err_rpm_put;
>  	}
> @@ -924,6 +962,9 @@ static void gc2145_stop_streaming(struct gc2145 *gc2145)
>  			GC2145_CSI2_MODE_EN | GC2145_CSI2_MODE_MIPI_EN, 0,
>  			&ret);
>  	cci_write(gc2145->regmap, GC2145_REG_PAGE_SELECT, 0x00, &ret);
> +
> +	/* Disable dvp streaming */
> +	cci_write(gc2145->regmap, GC2145_REG_PAD_IO, 0x00, &ret);
>  	if (ret)
>  		dev_err(&client->dev, "%s failed to write regs\n", __func__);
>  
> @@ -1233,9 +1274,8 @@ static int gc2145_init_controls(struct gc2145 *gc2145)
>  static int gc2145_check_hwcfg(struct device *dev)
>  {
>  	struct fwnode_handle *endpoint;
> -	struct v4l2_fwnode_endpoint ep_cfg = {
> -		.bus_type = V4L2_MBUS_CSI2_DPHY
> -	};
> +	struct v4l2_subdev *sd = dev_get_drvdata(dev);
> +	struct gc2145 *gc2145 = to_gc2145(sd);

As the bindings default to bus-type 4, you should reflect that here. I.e.
try parsing the endpoint with bus_type set to 4 first before setting it to
V4L2_MBUS_UNKNOWN.

You'll also need to set the parallel defaults here before parsing the
endpoint.

>  	int ret;
>  
>  	endpoint = fwnode_graph_get_next_endpoint(dev_fwnode(dev), NULL);
> @@ -1244,36 +1284,55 @@ static int gc2145_check_hwcfg(struct device *dev)
>  		return -EINVAL;
>  	}
>  
> -	ret = v4l2_fwnode_endpoint_alloc_parse(endpoint, &ep_cfg);
> +	gc2145->ep.bus_type = V4L2_MBUS_CSI2_DPHY;
> +	ret = v4l2_fwnode_endpoint_alloc_parse(endpoint, &gc2145->ep);
> +	if (ret) {
> +		gc2145->ep.bus_type = V4L2_MBUS_PARALLEL;
> +		ret = v4l2_fwnode_endpoint_alloc_parse(endpoint, &gc2145->ep);
> +	}
>  	fwnode_handle_put(endpoint);
> -	if (ret)
> +	if (ret) {
> +		dev_err(dev, "could not parse endpoint\n");
>  		return ret;
> -
> -	/* Check the number of MIPI CSI2 data lanes */
> -	if (ep_cfg.bus.mipi_csi2.num_data_lanes != 2) {
> -		dev_err(dev, "only 2 data lanes are currently supported\n");
> -		ret = -EINVAL;
> -		goto out;
>  	}
>  
> -	/* Check the link frequency set in device tree */
> -	if (!ep_cfg.nr_of_link_frequencies) {
> -		dev_err(dev, "link-frequency property not found in DT\n");
> +	switch (gc2145->ep.bus_type) {
> +	case V4L2_MBUS_CSI2_DPHY:
> +		/* Check the number of MIPI CSI2 data lanes */
> +		if (gc2145->ep.bus.mipi_csi2.num_data_lanes != 2) {
> +			dev_err(dev, "only 2 data lanes are currently supported\n");
> +			ret = -EINVAL;
> +			goto out;
> +		}
> +
> +		/* Check the link frequency set in device tree */
> +		if (!gc2145->ep.nr_of_link_frequencies) {
> +			dev_err(dev, "link-frequencies property not found in DT\n");
> +			ret = -EINVAL;
> +			goto out;
> +		}
> +
> +		if (gc2145->ep.nr_of_link_frequencies != 3 ||
> +		    gc2145->ep.link_frequencies[0] != GC2145_640_480_LINKFREQ ||
> +		    gc2145->ep.link_frequencies[1] != GC2145_1280_720_LINKFREQ ||
> +		    gc2145->ep.link_frequencies[2] != GC2145_1600_1200_LINKFREQ) {

I guess this works as long as all the ghree frequencies are what the driver
expects but the driver implementation could be improved. Nearly all other
drivers use the available frequencies only.

> +			dev_err(dev, "Invalid link-frequencies provided\n");
> +			ret = -EINVAL;
> +			goto out;
> +		}
> +		break;
> +	case V4L2_MBUS_PARALLEL:
> +		break;
> +	default:
> +		dev_err(dev, "unsupported bus type %u\n", gc2145->ep.bus_type);
>  		ret = -EINVAL;
>  		goto out;
>  	}
>  
> -	if (ep_cfg.nr_of_link_frequencies != 3 ||
> -	    ep_cfg.link_frequencies[0] != GC2145_640_480_LINKFREQ ||
> -	    ep_cfg.link_frequencies[1] != GC2145_1280_720_LINKFREQ ||
> -	    ep_cfg.link_frequencies[2] != GC2145_1600_1200_LINKFREQ) {
> -		dev_err(dev, "Invalid link-frequencies provided\n");
> -		ret = -EINVAL;
> -	}
> +	return 0;
>  
>  out:
> -	v4l2_fwnode_endpoint_free(&ep_cfg);
> -
> +	v4l2_fwnode_endpoint_free(&gc2145->ep);

A newline would be nice here.

>  	return ret;
>  }
>  
> @@ -1421,6 +1480,7 @@ static void gc2145_remove(struct i2c_client *client)
>  	if (!pm_runtime_status_suspended(&client->dev))
>  		gc2145_power_off(&client->dev);
>  	pm_runtime_set_suspended(&client->dev);
> +	v4l2_fwnode_endpoint_free(&gc2145->ep);
>  }
>  
>  static const struct of_device_id gc2145_dt_ids[] = {

-- 
Kind regards,

Sakari Ailus