[PATCH v4 0/2] imx8mq-evk and ov5640: Add overlay-based camera support with shared reset handling

[PATCH v4 0/2] imx8mq-evk and ov5640: Add overlay-based camera support with shared reset handling

[robby.cai]

From: Robby Cai <robby.cai@nxp.com>

This series enables OV5640 camera support on the i.MX8MQ EVK using
device tree overlays, including single- and dual-camera configurations.

The DT overlays describe two OV5640 sensors connected to different
MIPI CSI-2 interfaces:

  - OV5640 (I2C2) -> MIPI CSI1 -> CSI1 bridge
  - OV5640 (I2C1) -> MIPI CSI2 -> CSI2 bridge

On this platform, both sensors share a common reset GPIO line, while
each has an independent powerdown (PWDN) GPIO. Due to this hardware
constraint, proper handling of the shared reset line is required when
both cameras are present.

To address this, the OV5640 driver is updated to use the reset controller
framework, allowing it to correctly support shared reset lines. Legacy
reset-gpios support is retained as a fallback when no reset controller
is defined, ensuring compatibility with existing device tree
descriptions without requiring changes.

Note:
1) With commit 8f040b5c5e3a ("leds: class: Use firmware nodes for device lookup"),
the OV5640 driver reports the following errors:

  [   11.373844] ov5640 0-003c: error -EINVAL: getting privacy LED
  [   11.376442] ov5640 0-003c: probe with driver ov5640 failed with error -22
  [   11.906977] ov5640 1-003c: error -EINVAL: getting privacy LED
  [   11.909793] ov5640 1-003c: probe with driver ov5640 failed with error -22

This issue has been reported and discussed in [1] and related threads.
As a temporary workaround for testing OV5640, the patch can be reverted.

Link [1]: https://lore.kernel.org/all/aignTNlK5kCLmQ2A@tom-desktop/

2) The patch at:
     https://lore.kernel.org/imx/20260619073115.3778313-1-robby.cai@oss.nxp.com/
   is also required for OV5640 to function properly on the i.MX8MQ EVK.




Changes in v4:
- Switch EVK camera support to DT overlays for CSI1/CSI2/dual configurations (Kieran Bingham)
- Convert OV5640 driver to use reset controller framework with GPIO fallback (sashiko)
- Ensure correct handling of reset line (sashiko)

Link to v3: https://lore.kernel.org/imx/20260529132334.3333294-1-robby.cai@nxp.com/

Changes in v3:
- Add OV5640 driver changes to use reset control framework for shared reset
- Drop GPIO hog for reset in DTS

Link to v2: https://lore.kernel.org/imx/20260515111143.2980956-1-robby.cai@nxp.com/

Changes in v2:
- Address comments on MIPI clock configuration (Frank, Sebastian):
  drop the first patch and consolidate the correct clock configuration
  into the second patch
- Address comments from sashiko:
  * Use MEDIA_BUS_TYPE_CSI2_DPHY instead of a literal value
  * Fix a probe-order dependency related to reset handling. Switch to
    software reset, as the shared hardware reset line prevents
    independent reset when both cameras are enabled due to a board
    design limitation
  * Fix incorrect voltage value in the reg_2v8 node

Link to v1: https://lore.kernel.org/imx/20260417110200.753678-1-robby.cai@nxp.com/


Signed-off-by: Robby Cai <robby.cai@nxp.com>

Robby Cai (2):
  arm64: dts: imx8mq-evk: Add OV5640 camera support via overlays
  media: i2c: ov5640: Add reset controller support with GPIO fallback

 arch/arm64/boot/dts/freescale/Makefile        |  7 ++
 .../dts/freescale/imx8mq-evk-ov5640-csi1.dtso | 69 ++++++++++++++++
 .../dts/freescale/imx8mq-evk-ov5640-csi2.dtso | 65 +++++++++++++++
 arch/arm64/boot/dts/freescale/imx8mq-evk.dts  | 50 ++++++++++++
 drivers/media/i2c/ov5640.c                    | 80 ++++++++++++++++---
 5 files changed, 261 insertions(+), 10 deletions(-)
 create mode 100644 arch/arm64/boot/dts/freescale/imx8mq-evk-ov5640-csi1.dtso
 create mode 100644 arch/arm64/boot/dts/freescale/imx8mq-evk-ov5640-csi2.dtso

-- 
2.50.1

[PATCH v4 1/2] arm64: dts: imx8mq-evk: Add OV5640 camera support via overlays

[robby.cai]

From: Robby Cai <robby.cai@nxp.com>

Add overlays for single and dual camera setups on CSI1 and CSI2, enabling
the following media pipelines:

  - OV5640 (I2C2) -> MIPI CSI1 -> CSI1 bridge
  - OV5640 (I2C1) -> MIPI CSI2 -> CSI2 bridge

On the i.MX8MQ EVK, both sensors share a common reset GPIO, while each
sensor has an independent powerdown (PWDN) GPIO.

Both sensors also share the same MCLK source (CLKO2), configured
identically as required by the hardware design.

Signed-off-by: Robby Cai <robby.cai@nxp.com>
---
 arch/arm64/boot/dts/freescale/Makefile        |  7 ++
 .../dts/freescale/imx8mq-evk-ov5640-csi1.dtso | 69 +++++++++++++++++++
 .../dts/freescale/imx8mq-evk-ov5640-csi2.dtso | 65 +++++++++++++++++
 arch/arm64/boot/dts/freescale/imx8mq-evk.dts  | 50 ++++++++++++++
 4 files changed, 191 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/Makefile b/arch/arm64/boot/dts/freescale/Makefile
index 8ddaab127ab9..8507cbdb5556 100644
--- a/arch/arm64/boot/dts/freescale/Makefile
+++ b/arch/arm64/boot/dts/freescale/Makefile
@@ -501,6 +501,13 @@ dtb-$(CONFIG_ARCH_MXC) += imx8mq-evk.dtb
 imx8mq-evk-pcie1-ep-dtbs += imx8mq-evk.dtb imx-pcie1-ep.dtbo
 dtb-$(CONFIG_ARCH_MXC) += imx8mq-evk-pcie1-ep.dtb
 
+imx8mq-evk-ov5640-csi1-dtbs := imx8mq-evk.dtb imx8mq-evk-ov5640-csi1.dtbo
+dtb-${CONFIG_ARCH_MXC} += imx8mq-evk-ov5640-csi1.dtb
+imx8mq-evk-ov5640-csi2-dtbs := imx8mq-evk.dtb imx8mq-evk-ov5640-csi2.dtbo
+dtb-${CONFIG_ARCH_MXC} += imx8mq-evk-ov5640-csi2.dtb
+imx8mq-evk-ov5640-dual-dtbs := imx8mq-evk.dtb imx8mq-evk-ov5640-csi1.dtbo imx8mq-evk-ov5640-csi2.dtbo
+dtb-${CONFIG_ARCH_MXC} += imx8mq-evk-ov5640-dual.dtb
+
 dtb-$(CONFIG_ARCH_MXC) += imx8mq-hummingboard-pulse.dtb
 dtb-$(CONFIG_ARCH_MXC) += imx8mq-kontron-pitx-imx8m.dtb
 dtb-$(CONFIG_ARCH_MXC) += imx8mq-librem5-devkit.dtb
diff --git a/arch/arm64/boot/dts/freescale/imx8mq-evk-ov5640-csi1.dtso b/arch/arm64/boot/dts/freescale/imx8mq-evk-ov5640-csi1.dtso
new file mode 100644
index 000000000000..1e9931802cdc
--- /dev/null
+++ b/arch/arm64/boot/dts/freescale/imx8mq-evk-ov5640-csi1.dtso
@@ -0,0 +1,69 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Copyright 2026 NXP
+ */
+
+/dts-v1/;
+/plugin/;
+
+#include <dt-bindings/clock/imx8mq-clock.h>
+#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/media/video-interfaces.h>
+
+&csi1 {
+	status = "okay";
+};
+
+&i2c2 {
+	#address-cells = <1>;
+	#size-cells = <0>;
+	clock-frequency = <100000>;
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_i2c2>;
+	status = "okay";
+
+	camera@3c {
+		compatible = "ovti,ov5640";
+		reg = <0x3c>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&pinctrl_camera1_pwdn>;
+		clocks = <&clk IMX8MQ_CLK_CLKO2>;
+		clock-names = "xclk";
+		assigned-clocks = <&clk IMX8MQ_CLK_CLKO2>;
+		assigned-clock-parents = <&clk IMX8MQ_SYS2_PLL_200M>;
+		assigned-clock-rates = <20000000>;
+		powerdown-gpios = <&gpio1 3 GPIO_ACTIVE_HIGH>;
+		reset-gpios = <&gpio1 6 GPIO_ACTIVE_LOW>;
+		DOVDD-supply = <&sw4_reg>;
+		AVDD-supply = <&reg_2v8>;
+		DVDD-supply = <&reg_1v5>;
+
+		port {
+			camera1_ep: endpoint {
+				remote-endpoint = <&mipi_csi1_in_ep>;
+				clock-lanes = <0>;
+				data-lanes = <1 2>;
+			};
+		};
+	};
+};
+
+&mipi_csi1 {
+	assigned-clock-rates = <266000000>, <200000000>, <66000000>;
+	status = "okay";
+
+	ports {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		port@0 {
+			reg = <0>;
+
+			mipi_csi1_in_ep: endpoint {
+				remote-endpoint = <&camera1_ep>;
+				data-lanes = <1 2>;
+				bus-type = <MEDIA_BUS_TYPE_CSI2_DPHY>;
+			};
+		};
+	};
+};
diff --git a/arch/arm64/boot/dts/freescale/imx8mq-evk-ov5640-csi2.dtso b/arch/arm64/boot/dts/freescale/imx8mq-evk-ov5640-csi2.dtso
new file mode 100644
index 000000000000..fd247b3b5982
--- /dev/null
+++ b/arch/arm64/boot/dts/freescale/imx8mq-evk-ov5640-csi2.dtso
@@ -0,0 +1,65 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Copyright 2026 NXP
+ */
+
+/dts-v1/;
+/plugin/;
+
+#include <dt-bindings/clock/imx8mq-clock.h>
+#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/media/video-interfaces.h>
+
+&csi2 {
+	status = "okay";
+};
+
+&i2c1 {
+	#address-cells = <1>;
+	#size-cells = <0>;
+
+	camera@3c {
+		compatible = "ovti,ov5640";
+		reg = <0x3c>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&pinctrl_camera2_pwdn>;
+		clocks = <&clk IMX8MQ_CLK_CLKO2>;
+		clock-names = "xclk";
+		assigned-clocks = <&clk IMX8MQ_CLK_CLKO2>;
+		assigned-clock-parents = <&clk IMX8MQ_SYS2_PLL_200M>;
+		assigned-clock-rates = <20000000>;
+		powerdown-gpios = <&gpio1 5 GPIO_ACTIVE_HIGH>;
+		reset-gpios = <&gpio1 6 GPIO_ACTIVE_LOW>;
+		DOVDD-supply = <&sw4_reg>;
+		AVDD-supply = <&reg_2v8>;
+		DVDD-supply = <&reg_1v5>;
+
+		port {
+			camera2_ep: endpoint {
+				remote-endpoint = <&mipi_csi2_in_ep>;
+				clock-lanes = <0>;
+				data-lanes = <1 2>;
+			};
+		};
+	};
+};
+
+&mipi_csi2 {
+	assigned-clock-rates = <266000000>, <200000000>, <66000000>;
+	status = "okay";
+
+	ports {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		port@0 {
+			reg = <0>;
+
+			mipi_csi2_in_ep: endpoint {
+				remote-endpoint = <&camera2_ep>;
+				data-lanes = <1 2>;
+				bus-type = <MEDIA_BUS_TYPE_CSI2_DPHY>;
+			};
+		};
+	};
+};
diff --git a/arch/arm64/boot/dts/freescale/imx8mq-evk.dts b/arch/arm64/boot/dts/freescale/imx8mq-evk.dts
index e7d87ea81b69..d8c139c9128d 100644
--- a/arch/arm64/boot/dts/freescale/imx8mq-evk.dts
+++ b/arch/arm64/boot/dts/freescale/imx8mq-evk.dts
@@ -50,6 +50,20 @@ reg_usdhc2_vmmc: regulator-vsd-3v3 {
 		enable-active-high;
 	};
 
+	reg_1v5: regulator-1v5 {
+		compatible = "regulator-fixed";
+		regulator-name = "DVDD_1V5";
+		regulator-min-microvolt = <1500000>;
+		regulator-max-microvolt = <1500000>;
+	};
+
+	reg_2v8: regulator-2v8 {
+		compatible = "regulator-fixed";
+		regulator-name = "AVDD_2V8";
+		regulator-min-microvolt = <2800000>;
+		regulator-max-microvolt = <2800000>;
+	};
+
 	buck2_reg: regulator-buck2 {
 		pinctrl-names = "default";
 		pinctrl-0 = <&pinctrl_buck2>;
@@ -542,12 +556,34 @@ &wdog1 {
 };
 
 &iomuxc {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_mclk>, <&pinctrl_camera_reset>;
+
 	pinctrl_buck2: vddarmgrp {
 		fsl,pins = <
 			MX8MQ_IOMUXC_GPIO1_IO13_GPIO1_IO13		0x19
 		>;
 	};
 
+	pinctrl_camera1_pwdn: camera1pwdngrp {
+		fsl,pins = <
+			MX8MQ_IOMUXC_GPIO1_IO03_GPIO1_IO3		0x19
+		>;
+	};
+
+	pinctrl_camera2_pwdn: camera2pwdngrp {
+		fsl,pins = <
+			MX8MQ_IOMUXC_GPIO1_IO05_GPIO1_IO5		0x19
+		>;
+	};
+
+	/* Shared reset line for cameras on CSI1 and CSI2. */
+	pinctrl_camera_reset: cameraresetgrp {
+		fsl,pins = <
+			MX8MQ_IOMUXC_GPIO1_IO06_GPIO1_IO6		0x19
+		>;
+	};
+
 	pinctrl_fec1: fec1grp {
 		fsl,pins = <
 			MX8MQ_IOMUXC_ENET_MDC_ENET1_MDC			0x3
@@ -575,12 +611,26 @@ MX8MQ_IOMUXC_I2C1_SDA_I2C1_SDA			0x4000007f
 		>;
 	};
 
+	pinctrl_i2c2: i2c2grp {
+		fsl,pins = <
+			MX8MQ_IOMUXC_I2C2_SCL_I2C2_SCL			0x4000007f
+			MX8MQ_IOMUXC_I2C2_SDA_I2C2_SDA			0x4000007f
+		>;
+	};
+
 	pinctrl_ir: irgrp {
 		fsl,pins = <
 			MX8MQ_IOMUXC_GPIO1_IO12_GPIO1_IO12		0x4f
 		>;
 	};
 
+	/* Shared MCLK for cameras on CSI1 and CSI2. */
+	pinctrl_mclk: mclkgrp {
+		fsl,pins = <
+			MX8MQ_IOMUXC_GPIO1_IO15_CCMSRCGPCMIX_CLKO2	0x59
+		>;
+	};
+
 	pinctrl_mipi_dsi: mipidsigrp {
 		fsl,pins = <
 			MX8MQ_IOMUXC_ECSPI1_SCLK_GPIO5_IO6		0x16
-- 
2.50.1

[PATCH v4 2/2] media: i2c: ov5640: Add reset controller support with GPIO fallback

[robby.cai]

From: Robby Cai <robby.cai@nxp.com>

Add support for the reset controller framework by acquiring the reset
line using devm_reset_control_get_optional_shared_deasserted(). This
allows the driver to handle reset lines provided by a reset controller,
including shared ones, while avoiding unbalanced deassert counts.

Retain support for legacy reset-gpios as a fallback when no reset
controller is defined. In that case, request the GPIO and keep it in the
deasserted state as the initial configuration.

This enables the driver to support both reset-controller-backed reset
lines and older GPIO-based descriptions while preserving the existing
power-up sequencing behavior.

Signed-off-by: Robby Cai <robby.cai@nxp.com>
---
 drivers/media/i2c/ov5640.c | 80 +++++++++++++++++++++++++++++++++-----
 1 file changed, 70 insertions(+), 10 deletions(-)

diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index 85ecc23b3587..5e6db8aacb11 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -17,6 +17,7 @@
 #include <linux/module.h>
 #include <linux/pm_runtime.h>
 #include <linux/regulator/consumer.h>
+#include <linux/reset.h>
 #include <linux/slab.h>
 #include <linux/types.h>
 #include <media/v4l2-async.h>
@@ -442,6 +443,7 @@ struct ov5640_dev {
 	u32 xclk_freq;
 
 	struct regulator_bulk_data supplies[OV5640_NUM_SUPPLIES];
+	struct reset_control *reset;
 	struct gpio_desc *reset_gpio;
 	struct gpio_desc *pwdn_gpio;
 	bool   upside_down;
@@ -2431,6 +2433,48 @@ static int ov5640_restore_mode(struct ov5640_dev *sensor)
 	return ov5640_set_framefmt(sensor, &sensor->fmt);
 }
 
+static int ov5640_get_reset(struct device *dev, struct ov5640_dev *sensor)
+{
+	/* use deasserted version to avoid unbalanced deassert counts */
+	sensor->reset =
+	    devm_reset_control_get_optional_shared_deasserted(dev, NULL);
+	if (IS_ERR(sensor->reset))
+		return dev_err_probe(dev, PTR_ERR(sensor->reset),
+				     "Failed to get reset\n");
+	else if (sensor->reset)
+		return 0;
+
+	/*
+	 * fallback to legacy reset-gpios
+	 * GPIOD_OUT_HIGH ensures deasserted state for ACTIVE_LOW reset
+	 */
+	sensor->reset_gpio = devm_gpiod_get_optional(dev, "reset",
+						     GPIOD_OUT_HIGH);
+	if (IS_ERR(sensor->reset_gpio))
+		return dev_err_probe(dev, PTR_ERR(sensor->reset_gpio),
+				     "Failed to get reset gpio");
+
+	return 0;
+}
+
+static int ov5640_reset_assert(struct ov5640_dev *sensor)
+{
+	if (sensor->reset)
+		return reset_control_assert(sensor->reset);
+
+	gpiod_set_value_cansleep(sensor->reset_gpio, 1);
+	return 0;
+}
+
+static int ov5640_reset_deassert(struct ov5640_dev *sensor)
+{
+	if (sensor->reset)
+		return reset_control_deassert(sensor->reset);
+
+	gpiod_set_value_cansleep(sensor->reset_gpio, 0);
+	return 0;
+}
+
 static void ov5640_power(struct ov5640_dev *sensor, bool enable)
 {
 	gpiod_set_value_cansleep(sensor->pwdn_gpio, enable ? 0 : 1);
@@ -2448,12 +2492,19 @@ static void ov5640_power(struct ov5640_dev *sensor, bool enable)
  *
  * In such cases, this gpio should be mapped to pwdn_gpio in the driver, and we
  * should still toggle the pwdn_gpio below with the appropriate delays, while
- * the calls to reset_gpio will be ignored.
+ * reset handling (via reset controller or GPIO) will be ignored.
  */
-static void ov5640_powerup_sequence(struct ov5640_dev *sensor)
+static int ov5640_powerup_sequence(struct ov5640_dev *sensor)
 {
+	int ret;
+
 	if (sensor->pwdn_gpio) {
-		gpiod_set_value_cansleep(sensor->reset_gpio, 1);
+		ret = ov5640_reset_assert(sensor);
+		if (ret) {
+			dev_err(&sensor->i2c_client->dev,
+				"Failed to assert reset: %d\n", ret);
+			return ret;
+		}
 
 		/* camera power cycle */
 		ov5640_power(sensor, false);
@@ -2461,7 +2512,13 @@ static void ov5640_powerup_sequence(struct ov5640_dev *sensor)
 		ov5640_power(sensor, true);
 		usleep_range(1000, 2000);	/* t3 */
 
-		gpiod_set_value_cansleep(sensor->reset_gpio, 0);
+		ret = ov5640_reset_deassert(sensor);
+		if (ret) {
+			dev_err(&sensor->i2c_client->dev,
+				"Failed to deassert reset: %d\n", ret);
+			ov5640_power(sensor, false);
+			return ret;
+		}
 	} else {
 		/* software reset */
 		ov5640_write_reg(sensor, OV5640_REG_SYS_CTRL0,
@@ -2475,6 +2532,8 @@ static void ov5640_powerup_sequence(struct ov5640_dev *sensor)
 	 */
 	ov5640_write_reg(sensor, OV5640_REG_SYS_CTRL0,
 			 OV5640_REG_SYS_CTRL0_SW_PWDN);
+
+	return 0;
 }
 
 static int ov5640_set_power_on(struct ov5640_dev *sensor)
@@ -2497,7 +2556,9 @@ static int ov5640_set_power_on(struct ov5640_dev *sensor)
 		goto xclk_off;
 	}
 
-	ov5640_powerup_sequence(sensor);
+	ret = ov5640_powerup_sequence(sensor);
+	if (ret)
+		goto regulator_off;
 
 	ret = ov5640_init_slave_id(sensor);
 	if (ret)
@@ -2507,6 +2568,7 @@ static int ov5640_set_power_on(struct ov5640_dev *sensor)
 
 power_off:
 	ov5640_power(sensor, false);
+regulator_off:
 	regulator_bulk_disable(OV5640_NUM_SUPPLIES, sensor->supplies);
 xclk_off:
 	clk_disable_unprepare(sensor->xclk);
@@ -3914,11 +3976,9 @@ static int ov5640_probe(struct i2c_client *client)
 	if (IS_ERR(sensor->pwdn_gpio))
 		return PTR_ERR(sensor->pwdn_gpio);
 
-	/* request optional reset pin */
-	sensor->reset_gpio = devm_gpiod_get_optional(dev, "reset",
-						     GPIOD_OUT_HIGH);
-	if (IS_ERR(sensor->reset_gpio))
-		return PTR_ERR(sensor->reset_gpio);
+	ret = ov5640_get_reset(dev, sensor);
+	if (ret)
+		return ret;
 
 	v4l2_i2c_subdev_init(&sensor->sd, client, &ov5640_subdev_ops);
 	sensor->sd.internal_ops = &ov5640_internal_ops;
-- 
2.50.1

Re: [PATCH v4 2/2] media: i2c: ov5640: Add reset controller support with GPIO fallback

[Frank Li]

On Fri, Jun 19, 2026 at 06:05:32PM +0800, robby.cai@oss.nxp.com wrote:
> [You don't often get email from robby.cai@oss.nxp.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>
> From: Robby Cai <robby.cai@nxp.com>
>
> Add support for the reset controller framework by acquiring the reset
> line using devm_reset_control_get_optional_shared_deasserted(). This
> allows the driver to handle reset lines provided by a reset controller,
> including shared ones, while avoiding unbalanced deassert counts.
>
> Retain support for legacy reset-gpios as a fallback when no reset
> controller is defined. In that case, request the GPIO and keep it in the
> deasserted state as the initial configuration.
>
> This enables the driver to support both reset-controller-backed reset
> lines and older GPIO-based descriptions while preserving the existing
> power-up sequencing behavior.
>
> Signed-off-by: Robby Cai <robby.cai@nxp.com>
> ---
>  drivers/media/i2c/ov5640.c | 80 +++++++++++++++++++++++++++++++++-----
>  1 file changed, 70 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
> index 85ecc23b3587..5e6db8aacb11 100644
> --- a/drivers/media/i2c/ov5640.c
> +++ b/drivers/media/i2c/ov5640.c
> @@ -17,6 +17,7 @@
>  #include <linux/module.h>
>  #include <linux/pm_runtime.h>
>  #include <linux/regulator/consumer.h>
> +#include <linux/reset.h>
>  #include <linux/slab.h>
>  #include <linux/types.h>
>  #include <media/v4l2-async.h>
> @@ -442,6 +443,7 @@ struct ov5640_dev {
>         u32 xclk_freq;
>
>         struct regulator_bulk_data supplies[OV5640_NUM_SUPPLIES];
> +       struct reset_control *reset;
>         struct gpio_desc *reset_gpio;
>         struct gpio_desc *pwdn_gpio;
>         bool   upside_down;
> @@ -2431,6 +2433,48 @@ static int ov5640_restore_mode(struct ov5640_dev *sensor)
>         return ov5640_set_framefmt(sensor, &sensor->fmt);
>  }
>
> +static int ov5640_get_reset(struct device *dev, struct ov5640_dev *sensor)
> +{
> +       /* use deasserted version to avoid unbalanced deassert counts */
> +       sensor->reset =
> +           devm_reset_control_get_optional_shared_deasserted(dev, NULL);
> +       if (IS_ERR(sensor->reset))
> +               return dev_err_probe(dev, PTR_ERR(sensor->reset),
> +                                    "Failed to get reset\n");
> +       else if (sensor->reset)
> +               return 0;
> +
> +       /*
> +        * fallback to legacy reset-gpios
> +        * GPIOD_OUT_HIGH ensures deasserted state for ACTIVE_LOW reset
> +        */
> +       sensor->reset_gpio = devm_gpiod_get_optional(dev, "reset",
> +                                                    GPIOD_OUT_HIGH);
> +       if (IS_ERR(sensor->reset_gpio))
> +               return dev_err_probe(dev, PTR_ERR(sensor->reset_gpio),
> +                                    "Failed to get reset gpio");

I think needn't fallback here, NO ABI change, just change to use reset-gpio
driver.

> +
> +       return 0;
> +}
> +
> +static int ov5640_reset_assert(struct ov5640_dev *sensor)
> +{
> +       if (sensor->reset)
> +               return reset_control_assert(sensor->reset);

needn't check sensor->reset, reset_control_assert() is no ops if NULL.

> +
> +       gpiod_set_value_cansleep(sensor->reset_gpio, 1);

Needn't fallback, directly replace.

Frank

> +       return 0;
> +}
> +
> +static int ov5640_reset_deassert(struct ov5640_dev *sensor)
> +{
> +       if (sensor->reset)
> +               return reset_control_deassert(sensor->reset);
> +
> +       gpiod_set_value_cansleep(sensor->reset_gpio, 0);
> +       return 0;
> +}
> +
>  static void ov5640_power(struct ov5640_dev *sensor, bool enable)
>  {
>         gpiod_set_value_cansleep(sensor->pwdn_gpio, enable ? 0 : 1);
> @@ -2448,12 +2492,19 @@ static void ov5640_power(struct ov5640_dev *sensor, bool enable)
>   *
>   * In such cases, this gpio should be mapped to pwdn_gpio in the driver, and we
>   * should still toggle the pwdn_gpio below with the appropriate delays, while
> - * the calls to reset_gpio will be ignored.
> + * reset handling (via reset controller or GPIO) will be ignored.
>   */
> -static void ov5640_powerup_sequence(struct ov5640_dev *sensor)
> +static int ov5640_powerup_sequence(struct ov5640_dev *sensor)
>  {
> +       int ret;
> +
>         if (sensor->pwdn_gpio) {
> -               gpiod_set_value_cansleep(sensor->reset_gpio, 1);
> +               ret = ov5640_reset_assert(sensor);
> +               if (ret) {
> +                       dev_err(&sensor->i2c_client->dev,
> +                               "Failed to assert reset: %d\n", ret);
> +                       return ret;
> +               }
>
>                 /* camera power cycle */
>                 ov5640_power(sensor, false);
> @@ -2461,7 +2512,13 @@ static void ov5640_powerup_sequence(struct ov5640_dev *sensor)
>                 ov5640_power(sensor, true);
>                 usleep_range(1000, 2000);       /* t3 */
>
> -               gpiod_set_value_cansleep(sensor->reset_gpio, 0);
> +               ret = ov5640_reset_deassert(sensor);
> +               if (ret) {
> +                       dev_err(&sensor->i2c_client->dev,
> +                               "Failed to deassert reset: %d\n", ret);
> +                       ov5640_power(sensor, false);
> +                       return ret;
> +               }
>         } else {
>                 /* software reset */
>                 ov5640_write_reg(sensor, OV5640_REG_SYS_CTRL0,
> @@ -2475,6 +2532,8 @@ static void ov5640_powerup_sequence(struct ov5640_dev *sensor)
>          */
>         ov5640_write_reg(sensor, OV5640_REG_SYS_CTRL0,
>                          OV5640_REG_SYS_CTRL0_SW_PWDN);
> +
> +       return 0;
>  }
>
>  static int ov5640_set_power_on(struct ov5640_dev *sensor)
> @@ -2497,7 +2556,9 @@ static int ov5640_set_power_on(struct ov5640_dev *sensor)
>                 goto xclk_off;
>         }
>
> -       ov5640_powerup_sequence(sensor);
> +       ret = ov5640_powerup_sequence(sensor);
> +       if (ret)
> +               goto regulator_off;
>
>         ret = ov5640_init_slave_id(sensor);
>         if (ret)
> @@ -2507,6 +2568,7 @@ static int ov5640_set_power_on(struct ov5640_dev *sensor)
>
>  power_off:
>         ov5640_power(sensor, false);
> +regulator_off:
>         regulator_bulk_disable(OV5640_NUM_SUPPLIES, sensor->supplies);
>  xclk_off:
>         clk_disable_unprepare(sensor->xclk);
> @@ -3914,11 +3976,9 @@ static int ov5640_probe(struct i2c_client *client)
>         if (IS_ERR(sensor->pwdn_gpio))
>                 return PTR_ERR(sensor->pwdn_gpio);
>
> -       /* request optional reset pin */
> -       sensor->reset_gpio = devm_gpiod_get_optional(dev, "reset",
> -                                                    GPIOD_OUT_HIGH);
> -       if (IS_ERR(sensor->reset_gpio))
> -               return PTR_ERR(sensor->reset_gpio);
> +       ret = ov5640_get_reset(dev, sensor);
> +       if (ret)
> +               return ret;
>
>         v4l2_i2c_subdev_init(&sensor->sd, client, &ov5640_subdev_ops);
>         sensor->sd.internal_ops = &ov5640_internal_ops;
> --
> 2.50.1
>
>