arm64: dts: qcom: qrb2210-rb1: Add Qualcomm RB1 Vision kit

arm64: dts: qcom: qrb2210-rb1: Add Qualcomm RB1 Vision kit

[Loic Poulain]

This series adds support for Qualcomm RB1 vision kit board/mezzanine.
-  Add support for the missing camera PMIC PM8008, which is present on
   the RB1 core kit.
- Fix the logical state handling of the reset pin in the PM8008 driver.
- Add the RB1 Vision Mezzanine device tree overlay to enable camera.

---
Changes in v2:
- Use correct reset gpio polarity (active-low)
- Remove unecessary pm8008 always-on props
- Address various nits
---

Loic Poulain (3):
  media: i2c: ov9282: Fix reset-gpio logical state
  arm64: dts: qcom: qrb2210-rb1: Add PM8008 node
  arm64: dts: qcom: qrb2210-rb1: Add overlay for vision mezzanine

 arch/arm64/boot/dts/qcom/Makefile             |  5 ++
 .../qcom/qrb2210-rb1-vision-mezzanine.dtso    | 76 +++++++++++++++++++
 arch/arm64/boot/dts/qcom/qrb2210-rb1.dts      | 75 ++++++++++++++++++
 drivers/media/i2c/ov9282.c                    |  8 +-
 4 files changed, 160 insertions(+), 4 deletions(-)
 create mode 100644 arch/arm64/boot/dts/qcom/qrb2210-rb1-vision-mezzanine.dtso

-- 
2.34.1

[PATCH v2 1/3] media: i2c: ov9282: Fix reset-gpio logical state

[Loic Poulain]

Ensure reset state is low in the power-on state and high in the
power-off state (assert reset). Note that the polarity is abstracted
by the GPIO subsystem, so the logic level reflects the intended reset
behavior.

This breaks backward compatibility for any downstream dts using the
wrong polarity.

Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
---
 drivers/media/i2c/ov9282.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/media/i2c/ov9282.c b/drivers/media/i2c/ov9282.c
index c882a021cf18..fb6fcba503c8 100644
--- a/drivers/media/i2c/ov9282.c
+++ b/drivers/media/i2c/ov9282.c
@@ -1127,7 +1127,7 @@ static int ov9282_parse_hw_config(struct ov9282 *ov9282)
 
 	/* Request optional reset pin */
 	ov9282->reset_gpio = devm_gpiod_get_optional(ov9282->dev, "reset",
-						     GPIOD_OUT_LOW);
+						     GPIOD_OUT_HIGH);
 	if (IS_ERR(ov9282->reset_gpio)) {
 		dev_err(ov9282->dev, "failed to get reset gpio %ld",
 			PTR_ERR(ov9282->reset_gpio));
@@ -1238,7 +1238,7 @@ static int ov9282_power_on(struct device *dev)
 
 	usleep_range(400, 600);
 
-	gpiod_set_value_cansleep(ov9282->reset_gpio, 1);
+	gpiod_set_value_cansleep(ov9282->reset_gpio, 0);
 
 	ret = clk_prepare_enable(ov9282->inclk);
 	if (ret) {
@@ -1261,7 +1261,7 @@ static int ov9282_power_on(struct device *dev)
 error_clk:
 	clk_disable_unprepare(ov9282->inclk);
 error_reset:
-	gpiod_set_value_cansleep(ov9282->reset_gpio, 0);
+	gpiod_set_value_cansleep(ov9282->reset_gpio, 1);
 
 	regulator_bulk_disable(OV9282_NUM_SUPPLIES, ov9282->supplies);
 
@@ -1279,7 +1279,7 @@ static int ov9282_power_off(struct device *dev)
 	struct v4l2_subdev *sd = dev_get_drvdata(dev);
 	struct ov9282 *ov9282 = to_ov9282(sd);
 
-	gpiod_set_value_cansleep(ov9282->reset_gpio, 0);
+	gpiod_set_value_cansleep(ov9282->reset_gpio, 1);
 
 	clk_disable_unprepare(ov9282->inclk);
 
-- 
2.34.1

[PATCH v2 2/3] arm64: dts: qcom: qrb2210-rb1: Add PM8008 node

[Loic Poulain]

The PM8008 device is a dedicated camera PMIC integrating all the necessary
camera power management features.

Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
 arch/arm64/boot/dts/qcom/qrb2210-rb1.dts | 75 ++++++++++++++++++++++++
 1 file changed, 75 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/qrb2210-rb1.dts b/arch/arm64/boot/dts/qcom/qrb2210-rb1.dts
index 67ba508e92ba..453e81412c5c 100644
--- a/arch/arm64/boot/dts/qcom/qrb2210-rb1.dts
+++ b/arch/arm64/boot/dts/qcom/qrb2210-rb1.dts
@@ -220,6 +220,81 @@ zap-shader {
 	};
 };
 
+&i2c1 {
+	clock-frequency = <400000>;
+
+	status = "okay";
+
+	pm8008: pmic@8 {
+		compatible = "qcom,pm8008";
+		reg = <0x8>;
+
+		interrupts-extended = <&tlmm 25 IRQ_TYPE_EDGE_RISING>;
+		reset-gpios = <&tlmm 26 GPIO_ACTIVE_LOW>;
+
+		vdd-l1-l2-supply = <&pm4125_s3>;
+		vdd-l3-l4-supply = <&vph_pwr>;
+		vdd-l5-supply = <&vph_pwr>;
+		vdd-l6-supply = <&vph_pwr>;
+		vdd-l7-supply = <&vph_pwr>;
+
+		gpio-controller;
+		#gpio-cells = <2>;
+		gpio-ranges = <&pm8008 0 0 2>;
+
+		interrupt-controller;
+		#interrupt-cells = <2>;
+
+		#thermal-sensor-cells = <0>;
+
+		status = "disabled";
+
+		regulators {
+			vreg_l1p: ldo1 {
+				regulator-name = "vreg_l1p";
+				regulator-min-microvolt = <528000>;
+				regulator-max-microvolt = <1200000>;
+			};
+
+			vreg_l2p: ldo2 {
+				regulator-name = "vreg_l2p";
+				regulator-min-microvolt = <528000>;
+				regulator-max-microvolt = <1200000>;
+			};
+
+			vreg_l3p: ldo3 {
+				regulator-name = "vreg_l3p";
+				regulator-min-microvolt = <1500000>;
+				regulator-max-microvolt = <3400000>;
+			};
+
+			vreg_l4p: ldo4 {
+				regulator-name = "vreg_l4p";
+				regulator-min-microvolt = <1500000>;
+				regulator-max-microvolt = <3404000>;
+			};
+
+			vreg_l5p: ldo5 {
+				regulator-name = "vreg_l5p";
+				regulator-min-microvolt = <1500000>;
+				regulator-max-microvolt = <3400000>;
+			};
+
+			vreg_l6p: ldo6 {
+				regulator-name = "vreg_l6p";
+				regulator-min-microvolt = <1500000>;
+				regulator-max-microvolt = <3400000>;
+			};
+
+			vreg_l7p: ldo7 {
+				regulator-name = "vreg_l7p";
+				regulator-min-microvolt = <1500000>;
+				regulator-max-microvolt = <3400000>;
+			};
+		};
+	};
+};
+
 &i2c2_gpio {
 	clock-frequency = <400000>;
 	status = "okay";
-- 
2.34.1

[PATCH v2 3/3] arm64: dts: qcom: qrb2210-rb1: Add overlay for vision mezzanine

[Loic Poulain]

This initial version includes support for OV9282 camera sensor.

Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
---
 arch/arm64/boot/dts/qcom/Makefile             |  5 ++
 .../qcom/qrb2210-rb1-vision-mezzanine.dtso    | 76 +++++++++++++++++++
 2 files changed, 81 insertions(+)
 create mode 100644 arch/arm64/boot/dts/qcom/qrb2210-rb1-vision-mezzanine.dtso

diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile
index d7f22476d510..bee021efc249 100644
--- a/arch/arm64/boot/dts/qcom/Makefile
+++ b/arch/arm64/boot/dts/qcom/Makefile
@@ -138,6 +138,11 @@ dtb-$(CONFIG_ARCH_QCOM)	+= qcs9100-ride.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= qcs9100-ride-r3.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= qdu1000-idp.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= qrb2210-rb1.dtb
+
+qrb2210-rb1-vision-mezzanine-dtbs	:= qrb2210-rb1.dtb qrb2210-rb1-vision-mezzanine.dtbo
+
+dtb-$(CONFIG_ARCH_QCOM)	+= qrb2210-rb1-vision-mezzanine.dtb
+
 dtb-$(CONFIG_ARCH_QCOM)	+= qrb4210-rb2.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= qrb5165-rb5.dtb
 
diff --git a/arch/arm64/boot/dts/qcom/qrb2210-rb1-vision-mezzanine.dtso b/arch/arm64/boot/dts/qcom/qrb2210-rb1-vision-mezzanine.dtso
new file mode 100644
index 000000000000..3b6261131b75
--- /dev/null
+++ b/arch/arm64/boot/dts/qcom/qrb2210-rb1-vision-mezzanine.dtso
@@ -0,0 +1,76 @@
+// SPDX-License-Identifier: BSD-3-Clause
+/*
+ * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
+ */
+
+/dts-v1/;
+/plugin/;
+
+#include <dt-bindings/clock/qcom,gcc-qcm2290.h>
+#include <dt-bindings/gpio/gpio.h>
+
+&tlmm {
+	cam0a_default: cam0a-default-state {
+		pins = "gpio28";
+		function = "cam_mclk";
+		drive-strength = <16>;
+		bias-disable;
+	};
+};
+
+&pm8008 {
+	status = "okay";
+};
+
+&camss {
+	status = "okay";
+
+	vdd-csiphy-1p2-supply = <&pm4125_l5>;
+	vdd-csiphy-1p8-supply = <&pm4125_l13>;
+
+	ports {
+		port@0 {
+			csiphy0_ep: endpoint {
+				data-lanes = <0 1>;
+				remote-endpoint = <&ov9282_ep>;
+			};
+		};
+	};
+};
+
+&cci {
+	status = "okay";
+};
+
+&cci_i2c1 {
+	#address-cells = <1>;
+	#size-cells = <0>;
+
+	/* Vision Mezzanine DIP3-1 must be ON (Selects camera CAM0A&B) */
+	camera@60 {
+		compatible = "ovti,ov9282";
+		reg = <0x60>;
+
+		/* Note: Reset is active-low but ov9282 driver logic is inverted... */
+		reset-gpios = <&tlmm 18 GPIO_ACTIVE_LOW>;
+
+		pinctrl-0 = <&cam0a_default>;
+		pinctrl-names = "default";
+
+		clocks = <&gcc GCC_CAMSS_MCLK3_CLK>;
+		assigned-clocks = <&gcc GCC_CAMSS_MCLK3_CLK>;
+		assigned-clock-rates = <24000000>;
+
+		avdd-supply = <&vreg_l3p>;
+		dvdd-supply = <&vreg_l1p>;
+		dovdd-supply = <&vreg_l7p>;
+
+		port {
+			ov9282_ep: endpoint {
+				link-frequencies = /bits/ 64 <400000000>;
+				data-lanes = <1 2>;
+				remote-endpoint = <&csiphy0_ep>;
+                        };
+                };
+	};
+};
-- 
2.34.1

Re: [PATCH v2 2/3] arm64: dts: qcom: qrb2210-rb1: Add PM8008 node

[Konrad Dybcio]

On 9/26/25 9:34 AM, Loic Poulain wrote:
> The PM8008 device is a dedicated camera PMIC integrating all the necessary
> camera power management features.
> 
> Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> ---

Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

Konrad

Re: [PATCH v2 3/3] arm64: dts: qcom: qrb2210-rb1: Add overlay for vision mezzanine

[Konrad Dybcio]

On 9/26/25 9:34 AM, Loic Poulain wrote:
> This initial version includes support for OV9282 camera sensor.
> 
> Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
> ---

[...]

> +&cci_i2c1 {
> +	#address-cells = <1>;
> +	#size-cells = <0>;
> +
> +	/* Vision Mezzanine DIP3-1 must be ON (Selects camera CAM0A&B) */
> +	camera@60 {
> +		compatible = "ovti,ov9282";
> +		reg = <0x60>;
> +
> +		/* Note: Reset is active-low but ov9282 driver logic is inverted... */

The comment no longer applies

with it gone:

Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

Konrad

Re: [PATCH v2 1/3] media: i2c: ov9282: Fix reset-gpio logical state

[Bryan O'Donoghue]

On 26/09/2025 08:34, Loic Poulain wrote:
> Ensure reset state is low in the power-on state and high in the
> power-off state (assert reset). Note that the polarity is abstracted
> by the GPIO subsystem, so the logic level reflects the intended reset
> behavior.
> 
> This breaks backward compatibility for any downstream dts using the
> wrong polarity.
> 
> Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
> ---
>   drivers/media/i2c/ov9282.c | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/media/i2c/ov9282.c b/drivers/media/i2c/ov9282.c
> index c882a021cf18..fb6fcba503c8 100644
> --- a/drivers/media/i2c/ov9282.c
> +++ b/drivers/media/i2c/ov9282.c
> @@ -1127,7 +1127,7 @@ static int ov9282_parse_hw_config(struct ov9282 *ov9282)
> 
>   	/* Request optional reset pin */
>   	ov9282->reset_gpio = devm_gpiod_get_optional(ov9282->dev, "reset",
> -						     GPIOD_OUT_LOW);
> +						     GPIOD_OUT_HIGH);
>   	if (IS_ERR(ov9282->reset_gpio)) {
>   		dev_err(ov9282->dev, "failed to get reset gpio %ld",
>   			PTR_ERR(ov9282->reset_gpio));
> @@ -1238,7 +1238,7 @@ static int ov9282_power_on(struct device *dev)
> 
>   	usleep_range(400, 600);
> 
> -	gpiod_set_value_cansleep(ov9282->reset_gpio, 1);
> +	gpiod_set_value_cansleep(ov9282->reset_gpio, 0);
> 
>   	ret = clk_prepare_enable(ov9282->inclk);
>   	if (ret) {
> @@ -1261,7 +1261,7 @@ static int ov9282_power_on(struct device *dev)
>   error_clk:
>   	clk_disable_unprepare(ov9282->inclk);
>   error_reset:
> -	gpiod_set_value_cansleep(ov9282->reset_gpio, 0);
> +	gpiod_set_value_cansleep(ov9282->reset_gpio, 1);
> 
>   	regulator_bulk_disable(OV9282_NUM_SUPPLIES, ov9282->supplies);
> 
> @@ -1279,7 +1279,7 @@ static int ov9282_power_off(struct device *dev)
>   	struct v4l2_subdev *sd = dev_get_drvdata(dev);
>   	struct ov9282 *ov9282 = to_ov9282(sd);
> 
> -	gpiod_set_value_cansleep(ov9282->reset_gpio, 0);
> +	gpiod_set_value_cansleep(ov9282->reset_gpio, 1);
> 
>   	clk_disable_unprepare(ov9282->inclk);
> 
> --
> 2.34.1
> 
> 

Needs a Fixes: tag

---
bod

Re: [PATCH v2 3/3] arm64: dts: qcom: qrb2210-rb1: Add overlay for vision mezzanine

[Bryan O'Donoghue]

On 26/09/2025 08:34, Loic Poulain wrote:
> This initial version includes support for OV9282 camera sensor.
> 
> Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
> ---
>   arch/arm64/boot/dts/qcom/Makefile             |  5 ++
>   .../qcom/qrb2210-rb1-vision-mezzanine.dtso    | 76 +++++++++++++++++++
>   2 files changed, 81 insertions(+)
>   create mode 100644 arch/arm64/boot/dts/qcom/qrb2210-rb1-vision-mezzanine.dtso
> 
> diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile
> index d7f22476d510..bee021efc249 100644
> --- a/arch/arm64/boot/dts/qcom/Makefile
> +++ b/arch/arm64/boot/dts/qcom/Makefile
> @@ -138,6 +138,11 @@ dtb-$(CONFIG_ARCH_QCOM)	+= qcs9100-ride.dtb
>   dtb-$(CONFIG_ARCH_QCOM)	+= qcs9100-ride-r3.dtb
>   dtb-$(CONFIG_ARCH_QCOM)	+= qdu1000-idp.dtb
>   dtb-$(CONFIG_ARCH_QCOM)	+= qrb2210-rb1.dtb
> +
> +qrb2210-rb1-vision-mezzanine-dtbs	:= qrb2210-rb1.dtb qrb2210-rb1-vision-mezzanine.dtbo
> +
> +dtb-$(CONFIG_ARCH_QCOM)	+= qrb2210-rb1-vision-mezzanine.dtb
> +
>   dtb-$(CONFIG_ARCH_QCOM)	+= qrb4210-rb2.dtb
>   dtb-$(CONFIG_ARCH_QCOM)	+= qrb5165-rb5.dtb
> 
> diff --git a/arch/arm64/boot/dts/qcom/qrb2210-rb1-vision-mezzanine.dtso b/arch/arm64/boot/dts/qcom/qrb2210-rb1-vision-mezzanine.dtso
> new file mode 100644
> index 000000000000..3b6261131b75
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/qrb2210-rb1-vision-mezzanine.dtso
> @@ -0,0 +1,76 @@
> +// SPDX-License-Identifier: BSD-3-Clause
> +/*
> + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
> + */
> +
> +/dts-v1/;
> +/plugin/;
> +
> +#include <dt-bindings/clock/qcom,gcc-qcm2290.h>
> +#include <dt-bindings/gpio/gpio.h>
> +
> +&tlmm {
> +	cam0a_default: cam0a-default-state {
> +		pins = "gpio28";
> +		function = "cam_mclk";
> +		drive-strength = <16>;
> +		bias-disable;
> +	};
> +};
> +
> +&pm8008 {
> +	status = "okay";
> +};
> +
> +&camss {
> +	status = "okay";
> +
> +	vdd-csiphy-1p2-supply = <&pm4125_l5>;
> +	vdd-csiphy-1p8-supply = <&pm4125_l13>;
> +
> +	ports {
> +		port@0 {
> +			csiphy0_ep: endpoint {
> +				data-lanes = <0 1>;
> +				remote-endpoint = <&ov9282_ep>;
> +			};
> +		};
> +	};
> +};
> +
> +&cci {
> +	status = "okay";
> +};
> +
> +&cci_i2c1 {
> +	#address-cells = <1>;
> +	#size-cells = <0>;
> +
> +	/* Vision Mezzanine DIP3-1 must be ON (Selects camera CAM0A&B) */
> +	camera@60 {
> +		compatible = "ovti,ov9282";
> +		reg = <0x60>;
> +
> +		/* Note: Reset is active-low but ov9282 driver logic is inverted... */
> +		reset-gpios = <&tlmm 18 GPIO_ACTIVE_LOW>;

This comment is confusing me a bit.

Shouldn't it be that the driver polarity gets fixed and the DTS just 
declares the correct thing ?

> +
> +		pinctrl-0 = <&cam0a_default>;
> +		pinctrl-names = "default";
> +
> +		clocks = <&gcc GCC_CAMSS_MCLK3_CLK>;
> +		assigned-clocks = <&gcc GCC_CAMSS_MCLK3_CLK>;
> +		assigned-clock-rates = <24000000>;
> +
> +		avdd-supply = <&vreg_l3p>;
> +		dvdd-supply = <&vreg_l1p>;
> +		dovdd-supply = <&vreg_l7p>;
> +
> +		port {
> +			ov9282_ep: endpoint {
> +				link-frequencies = /bits/ 64 <400000000>;
> +				data-lanes = <1 2>;
> +				remote-endpoint = <&csiphy0_ep>;
> +                        };
> +                };
> +	};
> +};
> --
> 2.34.1
> 
> 

---
bod

Re: [PATCH v2 3/3] arm64: dts: qcom: qrb2210-rb1: Add overlay for vision mezzanine

[Loic Poulain]

Hi Bryan,

On Sat, Sep 27, 2025 at 12:37 PM Bryan O'Donoghue <bod@kernel.org> wrote:
>
> On 26/09/2025 08:34, Loic Poulain wrote:
> > This initial version includes support for OV9282 camera sensor.
> >
> > Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
> > ---
> >   arch/arm64/boot/dts/qcom/Makefile             |  5 ++
> >   .../qcom/qrb2210-rb1-vision-mezzanine.dtso    | 76 +++++++++++++++++++
> >   2 files changed, 81 insertions(+)
> >   create mode 100644 arch/arm64/boot/dts/qcom/qrb2210-rb1-vision-mezzanine.dtso
> >
> > diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile
> > index d7f22476d510..bee021efc249 100644
> > --- a/arch/arm64/boot/dts/qcom/Makefile
> > +++ b/arch/arm64/boot/dts/qcom/Makefile
> > @@ -138,6 +138,11 @@ dtb-$(CONFIG_ARCH_QCOM)  += qcs9100-ride.dtb
> >   dtb-$(CONFIG_ARCH_QCOM)     += qcs9100-ride-r3.dtb
> >   dtb-$(CONFIG_ARCH_QCOM)     += qdu1000-idp.dtb
> >   dtb-$(CONFIG_ARCH_QCOM)     += qrb2210-rb1.dtb
> > +
> > +qrb2210-rb1-vision-mezzanine-dtbs    := qrb2210-rb1.dtb qrb2210-rb1-vision-mezzanine.dtbo
> > +
> > +dtb-$(CONFIG_ARCH_QCOM)      += qrb2210-rb1-vision-mezzanine.dtb
> > +
> >   dtb-$(CONFIG_ARCH_QCOM)     += qrb4210-rb2.dtb
> >   dtb-$(CONFIG_ARCH_QCOM)     += qrb5165-rb5.dtb
> >
> > diff --git a/arch/arm64/boot/dts/qcom/qrb2210-rb1-vision-mezzanine.dtso b/arch/arm64/boot/dts/qcom/qrb2210-rb1-vision-mezzanine.dtso
> > new file mode 100644
> > index 000000000000..3b6261131b75
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/qcom/qrb2210-rb1-vision-mezzanine.dtso
> > @@ -0,0 +1,76 @@
> > +// SPDX-License-Identifier: BSD-3-Clause
> > +/*
> > + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
> > + */
> > +
> > +/dts-v1/;
> > +/plugin/;
> > +
> > +#include <dt-bindings/clock/qcom,gcc-qcm2290.h>
> > +#include <dt-bindings/gpio/gpio.h>
> > +
> > +&tlmm {
> > +     cam0a_default: cam0a-default-state {
> > +             pins = "gpio28";
> > +             function = "cam_mclk";
> > +             drive-strength = <16>;
> > +             bias-disable;
> > +     };
> > +};
> > +
> > +&pm8008 {
> > +     status = "okay";
> > +};
> > +
> > +&camss {
> > +     status = "okay";
> > +
> > +     vdd-csiphy-1p2-supply = <&pm4125_l5>;
> > +     vdd-csiphy-1p8-supply = <&pm4125_l13>;
> > +
> > +     ports {
> > +             port@0 {
> > +                     csiphy0_ep: endpoint {
> > +                             data-lanes = <0 1>;
> > +                             remote-endpoint = <&ov9282_ep>;
> > +                     };
> > +             };
> > +     };
> > +};
> > +
> > +&cci {
> > +     status = "okay";
> > +};
> > +
> > +&cci_i2c1 {
> > +     #address-cells = <1>;
> > +     #size-cells = <0>;
> > +
> > +     /* Vision Mezzanine DIP3-1 must be ON (Selects camera CAM0A&B) */
> > +     camera@60 {
> > +             compatible = "ovti,ov9282";
> > +             reg = <0x60>;
> > +
> > +             /* Note: Reset is active-low but ov9282 driver logic is inverted... */
> > +             reset-gpios = <&tlmm 18 GPIO_ACTIVE_LOW>;
>
> This comment is confusing me a bit.
>
> Shouldn't it be that the driver polarity gets fixed and the DTS just
> declares the correct thing ?

Yes, as Konrad pointed out, this comment is no longer relevant in V2.
I overlooked removing it when applying his suggested fix to the driver.


>
> > +
> > +             pinctrl-0 = <&cam0a_default>;
> > +             pinctrl-names = "default";
> > +
> > +             clocks = <&gcc GCC_CAMSS_MCLK3_CLK>;
> > +             assigned-clocks = <&gcc GCC_CAMSS_MCLK3_CLK>;
> > +             assigned-clock-rates = <24000000>;
> > +
> > +             avdd-supply = <&vreg_l3p>;
> > +             dvdd-supply = <&vreg_l1p>;
> > +             dovdd-supply = <&vreg_l7p>;
> > +
> > +             port {
> > +                     ov9282_ep: endpoint {
> > +                             link-frequencies = /bits/ 64 <400000000>;
> > +                             data-lanes = <1 2>;
> > +                             remote-endpoint = <&csiphy0_ep>;
> > +                        };
> > +                };
> > +     };
> > +};
> > --
> > 2.34.1
> >
> >
>
> ---
> bod

Re: [PATCH v2 1/3] media: i2c: ov9282: Fix reset-gpio logical state

[Vladimir Zapolskiy]

On 9/26/25 10:34, Loic Poulain wrote:
> Ensure reset state is low in the power-on state and high in the
> power-off state (assert reset). Note that the polarity is abstracted
> by the GPIO subsystem, so the logic level reflects the intended reset
> behavior.
> 
> This breaks backward compatibility for any downstream dts using the
> wrong polarity.
> 
> Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>

Reviewed-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>

I kindly ask you to specify the intended behaviour in the documentation
Documentation/devicetree/bindings/media/i2c/ovti,ov9282.yaml

-- 
Best wishes,
Vladimir

Re: [PATCH v2 1/3] media: i2c: ov9282: Fix reset-gpio logical state

[Loic Poulain]

On Mon, Sep 29, 2025 at 10:46 AM Vladimir Zapolskiy
<vladimir.zapolskiy@linaro.org> wrote:
>
> On 9/26/25 10:34, Loic Poulain wrote:
> > Ensure reset state is low in the power-on state and high in the
> > power-off state (assert reset). Note that the polarity is abstracted
> > by the GPIO subsystem, so the logic level reflects the intended reset
> > behavior.
> >
> > This breaks backward compatibility for any downstream dts using the
> > wrong polarity.
> >
> > Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
>
> Reviewed-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
>
> I kindly ask you to specify the intended behaviour in the documentation
> Documentation/devicetree/bindings/media/i2c/ovti,ov9282.yaml

Sure, I will add this in V3.

Thanks,
Loic

Re: [PATCH v2 3/3] arm64: dts: qcom: qrb2210-rb1: Add overlay for vision mezzanine

[Vladimir Zapolskiy]

On 9/26/25 10:34, Loic Poulain wrote:
> This initial version includes support for OV9282 camera sensor.
> 
> Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
> ---
>   arch/arm64/boot/dts/qcom/Makefile             |  5 ++
>   .../qcom/qrb2210-rb1-vision-mezzanine.dtso    | 76 +++++++++++++++++++
>   2 files changed, 81 insertions(+)
>   create mode 100644 arch/arm64/boot/dts/qcom/qrb2210-rb1-vision-mezzanine.dtso
> 
> diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile
> index d7f22476d510..bee021efc249 100644
> --- a/arch/arm64/boot/dts/qcom/Makefile
> +++ b/arch/arm64/boot/dts/qcom/Makefile
> @@ -138,6 +138,11 @@ dtb-$(CONFIG_ARCH_QCOM)	+= qcs9100-ride.dtb
>   dtb-$(CONFIG_ARCH_QCOM)	+= qcs9100-ride-r3.dtb
>   dtb-$(CONFIG_ARCH_QCOM)	+= qdu1000-idp.dtb
>   dtb-$(CONFIG_ARCH_QCOM)	+= qrb2210-rb1.dtb
> +
> +qrb2210-rb1-vision-mezzanine-dtbs	:= qrb2210-rb1.dtb qrb2210-rb1-vision-mezzanine.dtbo
> +
> +dtb-$(CONFIG_ARCH_QCOM)	+= qrb2210-rb1-vision-mezzanine.dtb
> +
>   dtb-$(CONFIG_ARCH_QCOM)	+= qrb4210-rb2.dtb
>   dtb-$(CONFIG_ARCH_QCOM)	+= qrb5165-rb5.dtb
>   
> diff --git a/arch/arm64/boot/dts/qcom/qrb2210-rb1-vision-mezzanine.dtso b/arch/arm64/boot/dts/qcom/qrb2210-rb1-vision-mezzanine.dtso
> new file mode 100644
> index 000000000000..3b6261131b75
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/qrb2210-rb1-vision-mezzanine.dtso
> @@ -0,0 +1,76 @@
> +// SPDX-License-Identifier: BSD-3-Clause
> +/*
> + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.

Year is missing, please set it.

> + */
> +
> +/dts-v1/;
> +/plugin/;
> +
> +#include <dt-bindings/clock/qcom,gcc-qcm2290.h>
> +#include <dt-bindings/gpio/gpio.h>
> +
> +&tlmm {
> +	cam0a_default: cam0a-default-state {
> +		pins = "gpio28";
> +		function = "cam_mclk";
> +		drive-strength = <16>;
> +		bias-disable;
> +	};
> +};

This is a generic non-changeable MCLK3 pin configuration, which is
specific to the SoC. Like in a number of other cases please consider
to define this and other MCLKx pin configurations in the SoC .dtsi file.

> +
> +&pm8008 {
> +	status = "okay";
> +};
> +
> +&camss {
> +	status = "okay";
> +
> +	vdd-csiphy-1p2-supply = <&pm4125_l5>;
> +	vdd-csiphy-1p8-supply = <&pm4125_l13>;
> +
> +	ports {
> +		port@0 {
> +			csiphy0_ep: endpoint {
> +				data-lanes = <0 1>;
> +				remote-endpoint = <&ov9282_ep>;
> +			};
> +		};
> +	};
> +};
> +
> +&cci {
> +	status = "okay";
> +};
> +
> +&cci_i2c1 {
> +	#address-cells = <1>;
> +	#size-cells = <0>;
> +
> +	/* Vision Mezzanine DIP3-1 must be ON (Selects camera CAM0A&B) */
> +	camera@60 {
> +		compatible = "ovti,ov9282";
> +		reg = <0x60>;
> +
> +		/* Note: Reset is active-low but ov9282 driver logic is inverted... */
> +		reset-gpios = <&tlmm 18 GPIO_ACTIVE_LOW>;
> +
> +		pinctrl-0 = <&cam0a_default>;
> +		pinctrl-names = "default";
> +
> +		clocks = <&gcc GCC_CAMSS_MCLK3_CLK>;
> +		assigned-clocks = <&gcc GCC_CAMSS_MCLK3_CLK>;
> +		assigned-clock-rates = <24000000>;
> +

It makes little sense to split properties with blank lines.

> +		avdd-supply = <&vreg_l3p>;
> +		dvdd-supply = <&vreg_l1p>;
> +		dovdd-supply = <&vreg_l7p>;
> +
> +		port {
> +			ov9282_ep: endpoint {
> +				link-frequencies = /bits/ 64 <400000000>;
> +				data-lanes = <1 2>;
> +				remote-endpoint = <&csiphy0_ep>;

It's quite strange to see CSI0 and MCLK3 in the same boat, but the
schematics says so.

> +                        };
> +                };
> +	};
> +};

Reviewed-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>

-- 
Best wishes,
Vladimir

Re: [PATCH v2 1/3] media: i2c: ov9282: Fix reset-gpio logical state

[Dave Stevenson]

Hi

On Fri, 26 Sept 2025 at 08:34, Loic Poulain
<loic.poulain@oss.qualcomm.com> wrote:
>
> Ensure reset state is low in the power-on state and high in the
> power-off state (assert reset). Note that the polarity is abstracted
> by the GPIO subsystem, so the logic level reflects the intended reset
> behavior.
>
> This breaks backward compatibility for any downstream dts using the
> wrong polarity.

Ouch. That'll be a nasty surprise to some if this lands, and
particularly with a Fixes: tag so it gets backported to stable
kernels.

There are a number of sensor drivers that have been in the tree for
multiple years that do indeed have the reset logic inverted as it
wasn't something being routinely picked up in code review. ov9282,
imx219, imx334, and imx274 for a start. Krzysztof sent [1] recently to
flag that they are wrong and shouldn't be copied, but changing the
behaviour feels unfriendly.

I'll defer to Sakari as to whether this change is acceptable.

Checking all my use cases, they use regulators instead of reset-gpio,
so it doesn't actually have an impact.

  Dave

[1] https://lore.kernel.org/linux-media/20250818150025.247209-2-krzysztof.kozlowski@linaro.org/

> Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
> ---
>  drivers/media/i2c/ov9282.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/media/i2c/ov9282.c b/drivers/media/i2c/ov9282.c
> index c882a021cf18..fb6fcba503c8 100644
> --- a/drivers/media/i2c/ov9282.c
> +++ b/drivers/media/i2c/ov9282.c
> @@ -1127,7 +1127,7 @@ static int ov9282_parse_hw_config(struct ov9282 *ov9282)
>
>         /* Request optional reset pin */
>         ov9282->reset_gpio = devm_gpiod_get_optional(ov9282->dev, "reset",
> -                                                    GPIOD_OUT_LOW);
> +                                                    GPIOD_OUT_HIGH);
>         if (IS_ERR(ov9282->reset_gpio)) {
>                 dev_err(ov9282->dev, "failed to get reset gpio %ld",
>                         PTR_ERR(ov9282->reset_gpio));
> @@ -1238,7 +1238,7 @@ static int ov9282_power_on(struct device *dev)
>
>         usleep_range(400, 600);
>
> -       gpiod_set_value_cansleep(ov9282->reset_gpio, 1);
> +       gpiod_set_value_cansleep(ov9282->reset_gpio, 0);
>
>         ret = clk_prepare_enable(ov9282->inclk);
>         if (ret) {
> @@ -1261,7 +1261,7 @@ static int ov9282_power_on(struct device *dev)
>  error_clk:
>         clk_disable_unprepare(ov9282->inclk);
>  error_reset:
> -       gpiod_set_value_cansleep(ov9282->reset_gpio, 0);
> +       gpiod_set_value_cansleep(ov9282->reset_gpio, 1);
>
>         regulator_bulk_disable(OV9282_NUM_SUPPLIES, ov9282->supplies);
>
> @@ -1279,7 +1279,7 @@ static int ov9282_power_off(struct device *dev)
>         struct v4l2_subdev *sd = dev_get_drvdata(dev);
>         struct ov9282 *ov9282 = to_ov9282(sd);
>
> -       gpiod_set_value_cansleep(ov9282->reset_gpio, 0);
> +       gpiod_set_value_cansleep(ov9282->reset_gpio, 1);
>
>         clk_disable_unprepare(ov9282->inclk);
>
> --
> 2.34.1
>

Re: [PATCH v2 1/3] media: i2c: ov9282: Fix reset-gpio logical state

[Loic Poulain]

Hi Dave,

On Mon, Sep 29, 2025 at 5:18 PM Dave Stevenson
<dave.stevenson@raspberrypi.com> wrote:
>
> Hi
>
> On Fri, 26 Sept 2025 at 08:34, Loic Poulain
> <loic.poulain@oss.qualcomm.com> wrote:
> >
> > Ensure reset state is low in the power-on state and high in the
> > power-off state (assert reset). Note that the polarity is abstracted
> > by the GPIO subsystem, so the logic level reflects the intended reset
> > behavior.
> >
> > This breaks backward compatibility for any downstream dts using the
> > wrong polarity.
>
> Ouch. That'll be a nasty surprise to some if this lands, and
> particularly with a Fixes: tag so it gets backported to stable
> kernels.
>
> There are a number of sensor drivers that have been in the tree for
> multiple years that do indeed have the reset logic inverted as it
> wasn't something being routinely picked up in code review. ov9282,
> imx219, imx334, and imx274 for a start. Krzysztof sent [1] recently to
> flag that they are wrong and shouldn't be copied, but changing the
> behaviour feels unfriendly.
>
> I'll defer to Sakari as to whether this change is acceptable.
>
> Checking all my use cases, they use regulators instead of reset-gpio,
> so it doesn't actually have an impact.
>
>   Dave
>
> [1] https://lore.kernel.org/linux-media/20250818150025.247209-2-krzysztof.kozlowski@linaro.org/

Should I just clone what has been done here (pointed by Konrad) to
keep DTS correctness while ensuring backward compatibility:
https://lore.kernel.org/r/20230102114152.297305-4-krzysztof.kozlowski@linaro.org

Regards,
Loic