[PATCH v4 0/4] Describe the cameras in the PinePhone Pro dts

[PATCH v4 0/4] Describe the cameras in the PinePhone Pro dts

[Olivier Benjamin]

This series adds support for the Pine64 PinePhone Pro's rear and front
cameras in Device Tree.
This is based on some of Ondrej Jirman's patches hosted in his tree at
https://codeberg.org/megi/linux, but I have also fully reviewed and
re-written the code from the RK3399 datasheet, the PinePhone Pro
schematic, and the IMX258-0AQH5 software reference manual.

I have tested these changes on my PinePhone Pro and am able to take
photos from both cameras using libcamera's cam.

This series has raised a question about the proper label name for the
front/user camera and rear/world camera for phones.
This series is using "ucam" and "wcam", which is used in a few other
Rockship DTBs:
 - arch/arm64/boot/dts/rockchip/px30-evb.dts
 - rk3399-gru-scarlet.dtsi

Signed-off-by: Olivier Benjamin <olivier.benjamin@bootlin.com>
---
Changes in v4:
- Dropped "clock-names" property from IM258 as it is absent from the binding.
- Renamed pinctrl definitions to match the names in the PinePhone Pro datasheet.
- Collected Rob Herring's acked-by for patches 1 and 2
- Link to v3: https://lore.kernel.org/r/20250509-camera-v3-0-dab2772d229a@bootlin.com

Changes in v3:
- Fixed new DTB warnings reported by Rob Herring's bot
- Link to v2: https://lore.kernel.org/r/20250302-camera-v2-0-312b44b4a89c@bootlin.com

Changes in v2:
- Rebase on mainline
- Change patch subject to arm64: dts: rockchip
- Rename new regulators to fit preferred form for fixed regulators
- Link to v1: https://lore.kernel.org/r/20250228-camera-v1-0-c51869f94e97@bootlin.com

---
Olivier Benjamin (4):
      dt-bindings: media: ov8858: inherit video-interface-devices properties
      dt-bindings: media: imx258: inherit video-interface-devices properties
      arm64: dts: rockchip: describe I2c Bus 1 and IMX258 world camera on PinePhone Pro
      arm64: dts: rockchip: describe the OV8858 user camera on PinePhone Pro

 .../devicetree/bindings/media/i2c/ovti,ov8858.yaml |   4 +-
 .../devicetree/bindings/media/i2c/sony,imx258.yaml |   4 +-
 .../boot/dts/rockchip/rk3399-pinephone-pro.dts     | 138 +++++++++++++++++++++
 3 files changed, 144 insertions(+), 2 deletions(-)
---
-- 
Olivier Benjamin <olivier.benjamin@bootlin.com>

[PATCH v4 1/4] dt-bindings: media: ov8858: inherit video-interface-devices properties

[Olivier Benjamin]

Update the OV8858 binding to inherit properties defined in the
video-interface-devices binding.

Acked-by: Rob Herring (Arm) <robh@kernel.org>
Signed-off-by: Olivier Benjamin <olivier.benjamin@bootlin.com>
---
 Documentation/devicetree/bindings/media/i2c/ovti,ov8858.yaml | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/media/i2c/ovti,ov8858.yaml b/Documentation/devicetree/bindings/media/i2c/ovti,ov8858.yaml
index a65f921ec0fd24e449331772e39a16cadd73a868..491f2931e6bcdf5a1ad2cd4f9eb187055c3a7a2d 100644
--- a/Documentation/devicetree/bindings/media/i2c/ovti,ov8858.yaml
+++ b/Documentation/devicetree/bindings/media/i2c/ovti,ov8858.yaml
@@ -15,6 +15,8 @@ description: |
   controlled through an I2C-compatible SCCB bus. The sensor transmits images
   on a MIPI CSI-2 output interface with up to 4 data lanes.
 
+$ref: /schemas/media/video-interface-devices.yaml#
+
 properties:
   compatible:
     const: ovti,ov8858
@@ -69,7 +71,7 @@ required:
   - clocks
   - port
 
-additionalProperties: false
+unevaluatedProperties: false
 
 examples:
   - |

-- 
2.49.0

[PATCH v4 2/4] dt-bindings: media: imx258: inherit video-interface-devices properties

[Olivier Benjamin]

Update the IMX258 binding to inherit properties defined in the
video-interface-devices binding.

Acked-by: Rob Herring (Arm) <robh@kernel.org>
Signed-off-by: Olivier Benjamin <olivier.benjamin@bootlin.com>
---
 Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml b/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml
index 975c1d77c8e5d24179e9cb8d92fe7b6798c4d2ec..421b935b52bcaafbee0aff5b5cebc6332409bd8b 100644
--- a/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml
+++ b/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml
@@ -18,6 +18,8 @@ description: |-
   The camera module does not expose the model through registers, so the
   exact model needs to be specified.
 
+$ref: /schemas/media/video-interface-devices.yaml#
+
 properties:
   compatible:
     enum:
@@ -81,7 +83,7 @@ required:
   - reg
   - port
 
-additionalProperties: false
+unevaluatedProperties: false
 
 examples:
   - |

-- 
2.49.0

[PATCH v4 3/4] arm64: dts: rockchip: describe I2c Bus 1 and IMX258 world camera on PinePhone Pro

[Olivier Benjamin]

Add the description of the rear/world camera (IMX258) on the PinePhone Pro
to the device dts file.
It receives commands on the I2C Bus 1 at address 0x1a and transmits data
over CSI-MIPI.

The I2C address for IMX258 can be found in the IMX258-0AQH5 Software
Reference Manual, page 24, section 2.3.1: 0b0011010 = 0x1a.
Section 3 indicates the module has 4 pairs of data lines. While 4-lane
mode is nominal, 2-lane mode should also be supported.

The pin muxing info was extracted from the PinePhone Pro schematic v1.0
as well as the RK3399 datasheet revision 1.8.

Table 2-3 in section 2.8 of the RK3399 datasheet contains the mapping
of IO functions for the SoC pins. Page 52 shows GPIO1_A0, page 54 shows
GPIO2_D4.

For I2C power, the PinePhone Pro schematic page 11 quadrants A4 and A5:
RK3399_J.AA8 and RK3399_J.Y8 get power from vcaa1v8_codec, so turn it on

The IMX258 also uses the following regulators, expected by its driver:
 - vana (2.8V analog), called AVDD2V8_DVP on P.18 q.C1 and derived from
   VCC1V8_S3 on P.13 q.B2
 - vdig (1.2V digital core), called DVDD_DVP on P.18 q.C1 and shown on
   P.18 q.D3 to be equivalent to VCC1V2_DVP derived from VCC3V3_SYS on
   P.13 q.B3. Note that this regulator's voltage is inconsistently
   labeled either 1.2V or 1.5V

RK3399_J.AG1 is GPIO4_A1/I2C1_SDA, RK3399_J.Y6 is GPIO4_A2/I2C1_SCL
This is the default pinctrl "i2c1_xfer" for i2c1 from rk3399-base.

For the reset (RESET) signal:
page 11 quadrant D2             | p.18 q.C3-4 | p.18 q.C2
RK3399_E.R25 -> GPIO1_A0 -> Camera_RST -> MIPI_RST0 -> IMX258.12

For the powerdown (PWDN) signal:
page 11 quadrants B4-5          | p.18 q.C2
RK3399_G.AF8 -> GPIO2_D4 -> DVP_PDN1_H -> IMX258.14

Helped-by: Dragan Simic <dsimic@manjaro.org>
Co-developed-by: Ondrej Jirman <megi@xff.cz>
Signed-off-by: Ondrej Jirman <megi@xff.cz>
Signed-off-by: Olivier Benjamin <olivier.benjamin@bootlin.com>
---
 .../boot/dts/rockchip/rk3399-pinephone-pro.dts     | 93 ++++++++++++++++++++++
 1 file changed, 93 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3399-pinephone-pro.dts b/arch/arm64/boot/dts/rockchip/rk3399-pinephone-pro.dts
index 04ba4c4565d0a205e2e46d7535c6a3190993621d..b335d044138b413359ba54269ab646142254a55f 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-pinephone-pro.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3399-pinephone-pro.dts
@@ -114,6 +114,16 @@ vcc3v3_sys: regulator-vcc3v3-sys {
 		vin-supply = <&vcc_sys>;
 	};
 
+	avdd2v8_dvp: regulator-avdd2v8-dvp {
+		compatible = "regulator-fixed";
+		regulator-name = "avdd2v8_dvp";
+		regulator-always-on;
+		regulator-boot-on;
+		regulator-min-microvolt = <2800000>;
+		regulator-max-microvolt = <2800000>;
+		vin-supply = <&vcc3v3_sys>;
+	};
+
 	vcca1v8_s3: regulator-vcc1v8-s3 {
 		compatible = "regulator-fixed";
 		regulator-name = "vcca1v8_s3";
@@ -136,6 +146,16 @@ vcc1v8_codec: regulator-vcc1v8-codec {
 		vin-supply = <&vcc3v3_sys>;
 	};
 
+	vcc1v2_dvp: regulator-vcc1v2-dvp {
+		compatible = "regulator-fixed";
+		regulator-name = "vcc1v2_dvp";
+		regulator-always-on;
+		regulator-boot-on;
+		regulator-min-microvolt = <1200000>;
+		regulator-max-microvolt = <1200000>;
+		vin-supply = <&vcca1v8_s3>;
+	};
+
 	wifi_pwrseq: sdio-wifi-pwrseq {
 		compatible = "mmc-pwrseq-simple";
 		clocks = <&rk818 1>;
@@ -312,6 +332,8 @@ vcc3v0_touch: LDO_REG2 {
 
 			vcca1v8_codec: LDO_REG3 {
 				regulator-name = "vcca1v8_codec";
+				regulator-always-on;
+				regulator-boot-on;
 				regulator-min-microvolt = <1800000>;
 				regulator-max-microvolt = <1800000>;
 			};
@@ -420,6 +442,45 @@ regulator-state-mem {
 	};
 };
 
+&i2c1 {
+	clock-frequency = <400000>;
+	pinctrl-0 = <&i2c1_xfer &cif_clkouta>;
+	assigned-clocks = <&cru SCLK_CIF_OUT>;
+	assigned-clock-rates = <24000000>;
+	status = "okay";
+
+	wcam: camera@1a {
+		compatible = "sony,imx258";
+		reg = <0x1a>;
+		clocks = <&cru SCLK_CIF_OUT>; /* MIPI_MCLK0, derived from CIF_CLKO */
+		pinctrl-names = "default";
+		pinctrl-0 = <&camera_rst>;
+		/* Note: both cameras also depend on vcca1v8_codec to power the I2C bus. */
+		vif-supply = <&vcc1v8_dvp>;
+		vana-supply = <&avdd2v8_dvp>;
+		vdig-supply = <&vcc1v2_dvp>; /* DVDD_DVP is the same as VCC1V2_DVP */
+		reset-gpios = <&gpio1 RK_PA0 GPIO_ACTIVE_LOW>;
+		orientation = <1>; /* V4L2_CAMERA_ORIENTATION_BACK */
+		rotation = <270>;
+		lens-focus = <&wcam_lens>;
+
+		port {
+			wcam_out: endpoint {
+				remote-endpoint = <&mipi_in_wcam>;
+				data-lanes = <1 2 3 4>;
+				link-frequencies = /bits/ 64 <636000000>;
+			};
+		};
+	};
+
+	wcam_lens: camera-lens@c {
+		compatible = "dongwoon,dw9714";
+		reg = <0x0c>;
+		/* Same I2c bus as both cameras, depends on vcca1v8_codec for power. */
+		vcc-supply = <&vcc1v8_dvp>;
+	};
+};
+
 &i2c3 {
 	i2c-scl-rising-time-ns = <450>;
 	i2c-scl-falling-time-ns = <15>;
@@ -462,6 +523,28 @@ &io_domains {
 	status = "okay";
 };
 
+&isp1 {
+	status = "okay";
+
+	ports {
+		port@0 {
+			mipi_in_wcam: endpoint@0 {
+				reg = <0>;
+				remote-endpoint = <&wcam_out>;
+				data-lanes = <1 2 3 4>;
+			};
+		};
+	};
+};
+
+&mipi_dphy_rx0 {
+	status = "okay";
+};
+
+&isp1_mmu {
+	status = "okay";
+};
+
 &mipi_dsi {
 	status = "okay";
 	clock-master;
@@ -495,6 +578,10 @@ mipi_in_panel: endpoint {
 	};
 };
 
+&mipi_dsi1 {
+	status = "okay";
+};
+
 &pmu_io_domains {
 	pmu1830-supply = <&vcc_1v8>;
 	status = "okay";
@@ -507,6 +594,12 @@ pwrbtn_pin: pwrbtn-pin {
 		};
 	};
 
+	camera {
+		camera_rst_l: camera-rst-l {
+			rockchip,pins = <1 RK_PA0 RK_FUNC_GPIO &pcfg_pull_none>;
+		};
+	};
+
 	leds {
 		red_led_pin: red-led-pin {
 			rockchip,pins = <4 RK_PD2 RK_FUNC_GPIO &pcfg_pull_none>;

-- 
2.49.0

Re: [PATCH v4 3/4] arm64: dts: rockchip: describe I2c Bus 1 and IMX258 world camera on PinePhone Pro

[Kieran Bingham]

Hi Olivier,

I'm very pleased to see this support getting upstreamed!

Quoting Olivier Benjamin (2025-06-20 16:21:34)
> Add the description of the rear/world camera (IMX258) on the PinePhone Pro
> to the device dts file.
> It receives commands on the I2C Bus 1 at address 0x1a and transmits data
> over CSI-MIPI.
> 
> The I2C address for IMX258 can be found in the IMX258-0AQH5 Software
> Reference Manual, page 24, section 2.3.1: 0b0011010 = 0x1a.
> Section 3 indicates the module has 4 pairs of data lines. While 4-lane
> mode is nominal, 2-lane mode should also be supported.
> 
> The pin muxing info was extracted from the PinePhone Pro schematic v1.0
> as well as the RK3399 datasheet revision 1.8.
> 
> Table 2-3 in section 2.8 of the RK3399 datasheet contains the mapping
> of IO functions for the SoC pins. Page 52 shows GPIO1_A0, page 54 shows
> GPIO2_D4.
> 
> For I2C power, the PinePhone Pro schematic page 11 quadrants A4 and A5:
> RK3399_J.AA8 and RK3399_J.Y8 get power from vcaa1v8_codec, so turn it on
> 
> The IMX258 also uses the following regulators, expected by its driver:
>  - vana (2.8V analog), called AVDD2V8_DVP on P.18 q.C1 and derived from
>    VCC1V8_S3 on P.13 q.B2
>  - vdig (1.2V digital core), called DVDD_DVP on P.18 q.C1 and shown on
>    P.18 q.D3 to be equivalent to VCC1V2_DVP derived from VCC3V3_SYS on
>    P.13 q.B3. Note that this regulator's voltage is inconsistently
>    labeled either 1.2V or 1.5V
> 
> RK3399_J.AG1 is GPIO4_A1/I2C1_SDA, RK3399_J.Y6 is GPIO4_A2/I2C1_SCL
> This is the default pinctrl "i2c1_xfer" for i2c1 from rk3399-base.
> 
> For the reset (RESET) signal:
> page 11 quadrant D2             | p.18 q.C3-4 | p.18 q.C2
> RK3399_E.R25 -> GPIO1_A0 -> Camera_RST -> MIPI_RST0 -> IMX258.12
> 
> For the powerdown (PWDN) signal:
> page 11 quadrants B4-5          | p.18 q.C2
> RK3399_G.AF8 -> GPIO2_D4 -> DVP_PDN1_H -> IMX258.14
> 
> Helped-by: Dragan Simic <dsimic@manjaro.org>
> Co-developed-by: Ondrej Jirman <megi@xff.cz>
> Signed-off-by: Ondrej Jirman <megi@xff.cz>
> Signed-off-by: Olivier Benjamin <olivier.benjamin@bootlin.com>
> ---
>  .../boot/dts/rockchip/rk3399-pinephone-pro.dts     | 93 ++++++++++++++++++++++
>  1 file changed, 93 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-pinephone-pro.dts b/arch/arm64/boot/dts/rockchip/rk3399-pinephone-pro.dts
> index 04ba4c4565d0a205e2e46d7535c6a3190993621d..b335d044138b413359ba54269ab646142254a55f 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3399-pinephone-pro.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3399-pinephone-pro.dts
> @@ -114,6 +114,16 @@ vcc3v3_sys: regulator-vcc3v3-sys {
>                 vin-supply = <&vcc_sys>;
>         };
>  
> +       avdd2v8_dvp: regulator-avdd2v8-dvp {
> +               compatible = "regulator-fixed";
> +               regulator-name = "avdd2v8_dvp";
> +               regulator-always-on;
> +               regulator-boot-on;
> +               regulator-min-microvolt = <2800000>;
> +               regulator-max-microvolt = <2800000>;
> +               vin-supply = <&vcc3v3_sys>;
> +       };
> +
>         vcca1v8_s3: regulator-vcc1v8-s3 {
>                 compatible = "regulator-fixed";
>                 regulator-name = "vcca1v8_s3";
> @@ -136,6 +146,16 @@ vcc1v8_codec: regulator-vcc1v8-codec {
>                 vin-supply = <&vcc3v3_sys>;
>         };
>  
> +       vcc1v2_dvp: regulator-vcc1v2-dvp {
> +               compatible = "regulator-fixed";
> +               regulator-name = "vcc1v2_dvp";
> +               regulator-always-on;
> +               regulator-boot-on;
> +               regulator-min-microvolt = <1200000>;
> +               regulator-max-microvolt = <1200000>;
> +               vin-supply = <&vcca1v8_s3>;
> +       };
> +
>         wifi_pwrseq: sdio-wifi-pwrseq {
>                 compatible = "mmc-pwrseq-simple";
>                 clocks = <&rk818 1>;
> @@ -312,6 +332,8 @@ vcc3v0_touch: LDO_REG2 {
>  
>                         vcca1v8_codec: LDO_REG3 {
>                                 regulator-name = "vcca1v8_codec";
> +                               regulator-always-on;
> +                               regulator-boot-on;

Is regulator-boot-on needed when regulator-always-on is set? Doesn't
-always-on take precedence and ensure it's enabled if it's not on
anyway?

It seems odd to have (need?) both ...? but I'm no DT expert on this part
- it just stands out to me.


>                                 regulator-min-microvolt = <1800000>;
>                                 regulator-max-microvolt = <1800000>;
>                         };
> @@ -420,6 +442,45 @@ regulator-state-mem {
>         };
>  };
>  
> +&i2c1 {
> +       clock-frequency = <400000>;
> +       pinctrl-0 = <&i2c1_xfer &cif_clkouta>;
> +       assigned-clocks = <&cru SCLK_CIF_OUT>;
> +       assigned-clock-rates = <24000000>;
> +       status = "okay";
> +
> +       wcam: camera@1a {
> +               compatible = "sony,imx258";
> +               reg = <0x1a>;
> +               clocks = <&cru SCLK_CIF_OUT>; /* MIPI_MCLK0, derived from CIF_CLKO */
> +               pinctrl-names = "default";
> +               pinctrl-0 = <&camera_rst>;
> +               /* Note: both cameras also depend on vcca1v8_codec to power the I2C bus. */

Should that be represented by the i2c1 bus then?

> +               vif-supply = <&vcc1v8_dvp>;
> +               vana-supply = <&avdd2v8_dvp>;
> +               vdig-supply = <&vcc1v2_dvp>; /* DVDD_DVP is the same as VCC1V2_DVP */
> +               reset-gpios = <&gpio1 RK_PA0 GPIO_ACTIVE_LOW>;
> +               orientation = <1>; /* V4L2_CAMERA_ORIENTATION_BACK */
> +               rotation = <270>;
> +               lens-focus = <&wcam_lens>;
> +
> +               port {
> +                       wcam_out: endpoint {
> +                               remote-endpoint = <&mipi_in_wcam>;
> +                               data-lanes = <1 2 3 4>;
> +                               link-frequencies = /bits/ 64 <636000000>;
> +                       };
> +               };
> +       };
> +
> +       wcam_lens: camera-lens@c {
> +               compatible = "dongwoon,dw9714";
> +               reg = <0x0c>;
> +               /* Same I2c bus as both cameras, depends on vcca1v8_codec for power. */
> +               vcc-supply = <&vcc1v8_dvp>;
> +       };
> +};
> +
>  &i2c3 {
>         i2c-scl-rising-time-ns = <450>;
>         i2c-scl-falling-time-ns = <15>;
> @@ -462,6 +523,28 @@ &io_domains {
>         status = "okay";
>  };
>  
> +&isp1 {
> +       status = "okay";
> +
> +       ports {
> +               port@0 {
> +                       mipi_in_wcam: endpoint@0 {
> +                               reg = <0>;
> +                               remote-endpoint = <&wcam_out>;
> +                               data-lanes = <1 2 3 4>;
> +                       };
> +               };
> +       };
> +};
> +
> +&mipi_dphy_rx0 {
> +       status = "okay";
> +};
> +
> +&isp1_mmu {
> +       status = "okay";
> +};
> +
>  &mipi_dsi {
>         status = "okay";
>         clock-master;
> @@ -495,6 +578,10 @@ mipi_in_panel: endpoint {
>         };
>  };
>  
> +&mipi_dsi1 {
> +       status = "okay";
> +};
> +

Is this an unrelated change? The Camera should be on CSI - not DSI?

>  &pmu_io_domains {
>         pmu1830-supply = <&vcc_1v8>;
>         status = "okay";
> @@ -507,6 +594,12 @@ pwrbtn_pin: pwrbtn-pin {
>                 };
>         };
>  
> +       camera {
> +               camera_rst_l: camera-rst-l {
> +                       rockchip,pins = <1 RK_PA0 RK_FUNC_GPIO &pcfg_pull_none>;
> +               };
> +       };
> +
>         leds {
>                 red_led_pin: red-led-pin {
>                         rockchip,pins = <4 RK_PD2 RK_FUNC_GPIO &pcfg_pull_none>;
> 
> -- 
> 2.49.0
> 
> 
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH v4 3/4] arm64: dts: rockchip: describe I2c Bus 1 and IMX258 world camera on PinePhone Pro

[Kieran Bingham]

Quoting Kieran Bingham (2025-08-02 09:40:57)
> Hi Olivier,
> 
> I'm very pleased to see this support getting upstreamed!
> 
> Quoting Olivier Benjamin (2025-06-20 16:21:34)
> > Add the description of the rear/world camera (IMX258) on the PinePhone Pro
> > to the device dts file.
> > It receives commands on the I2C Bus 1 at address 0x1a and transmits data
> > over CSI-MIPI.
> > 
> > The I2C address for IMX258 can be found in the IMX258-0AQH5 Software
> > Reference Manual, page 24, section 2.3.1: 0b0011010 = 0x1a.
> > Section 3 indicates the module has 4 pairs of data lines. While 4-lane
> > mode is nominal, 2-lane mode should also be supported.
> > 
> > The pin muxing info was extracted from the PinePhone Pro schematic v1.0
> > as well as the RK3399 datasheet revision 1.8.
> > 
> > Table 2-3 in section 2.8 of the RK3399 datasheet contains the mapping
> > of IO functions for the SoC pins. Page 52 shows GPIO1_A0, page 54 shows
> > GPIO2_D4.
> > 
> > For I2C power, the PinePhone Pro schematic page 11 quadrants A4 and A5:
> > RK3399_J.AA8 and RK3399_J.Y8 get power from vcaa1v8_codec, so turn it on
> > 
> > The IMX258 also uses the following regulators, expected by its driver:
> >  - vana (2.8V analog), called AVDD2V8_DVP on P.18 q.C1 and derived from
> >    VCC1V8_S3 on P.13 q.B2
> >  - vdig (1.2V digital core), called DVDD_DVP on P.18 q.C1 and shown on
> >    P.18 q.D3 to be equivalent to VCC1V2_DVP derived from VCC3V3_SYS on
> >    P.13 q.B3. Note that this regulator's voltage is inconsistently
> >    labeled either 1.2V or 1.5V
> > 
> > RK3399_J.AG1 is GPIO4_A1/I2C1_SDA, RK3399_J.Y6 is GPIO4_A2/I2C1_SCL
> > This is the default pinctrl "i2c1_xfer" for i2c1 from rk3399-base.
> > 
> > For the reset (RESET) signal:
> > page 11 quadrant D2             | p.18 q.C3-4 | p.18 q.C2
> > RK3399_E.R25 -> GPIO1_A0 -> Camera_RST -> MIPI_RST0 -> IMX258.12
> > 
> > For the powerdown (PWDN) signal:
> > page 11 quadrants B4-5          | p.18 q.C2
> > RK3399_G.AF8 -> GPIO2_D4 -> DVP_PDN1_H -> IMX258.14
> > 
> > Helped-by: Dragan Simic <dsimic@manjaro.org>
> > Co-developed-by: Ondrej Jirman <megi@xff.cz>
> > Signed-off-by: Ondrej Jirman <megi@xff.cz>
> > Signed-off-by: Olivier Benjamin <olivier.benjamin@bootlin.com>
> > ---
> >  .../boot/dts/rockchip/rk3399-pinephone-pro.dts     | 93 ++++++++++++++++++++++
> >  1 file changed, 93 insertions(+)
> > 
> > diff --git a/arch/arm64/boot/dts/rockchip/rk3399-pinephone-pro.dts b/arch/arm64/boot/dts/rockchip/rk3399-pinephone-pro.dts
> > index 04ba4c4565d0a205e2e46d7535c6a3190993621d..b335d044138b413359ba54269ab646142254a55f 100644
> > --- a/arch/arm64/boot/dts/rockchip/rk3399-pinephone-pro.dts
> > +++ b/arch/arm64/boot/dts/rockchip/rk3399-pinephone-pro.dts
> > @@ -114,6 +114,16 @@ vcc3v3_sys: regulator-vcc3v3-sys {
> >                 vin-supply = <&vcc_sys>;
> >         };
> >  
> > +       avdd2v8_dvp: regulator-avdd2v8-dvp {
> > +               compatible = "regulator-fixed";
> > +               regulator-name = "avdd2v8_dvp";
> > +               regulator-always-on;
> > +               regulator-boot-on;
> > +               regulator-min-microvolt = <2800000>;
> > +               regulator-max-microvolt = <2800000>;
> > +               vin-supply = <&vcc3v3_sys>;
> > +       };
> > +
> >         vcca1v8_s3: regulator-vcc1v8-s3 {
> >                 compatible = "regulator-fixed";
> >                 regulator-name = "vcca1v8_s3";
> > @@ -136,6 +146,16 @@ vcc1v8_codec: regulator-vcc1v8-codec {
> >                 vin-supply = <&vcc3v3_sys>;
> >         };
> >  
> > +       vcc1v2_dvp: regulator-vcc1v2-dvp {
> > +               compatible = "regulator-fixed";
> > +               regulator-name = "vcc1v2_dvp";
> > +               regulator-always-on;
> > +               regulator-boot-on;
> > +               regulator-min-microvolt = <1200000>;
> > +               regulator-max-microvolt = <1200000>;
> > +               vin-supply = <&vcca1v8_s3>;
> > +       };
> > +
> >         wifi_pwrseq: sdio-wifi-pwrseq {
> >                 compatible = "mmc-pwrseq-simple";
> >                 clocks = <&rk818 1>;
> > @@ -312,6 +332,8 @@ vcc3v0_touch: LDO_REG2 {
> >  
> >                         vcca1v8_codec: LDO_REG3 {
> >                                 regulator-name = "vcca1v8_codec";
> > +                               regulator-always-on;
> > +                               regulator-boot-on;
> 
> Is regulator-boot-on needed when regulator-always-on is set? Doesn't
> -always-on take precedence and ensure it's enabled if it's not on
> anyway?
> 
> It seems odd to have (need?) both ...? but I'm no DT expert on this part
> - it just stands out to me.
> 
> 
> >                                 regulator-min-microvolt = <1800000>;
> >                                 regulator-max-microvolt = <1800000>;
> >                         };
> > @@ -420,6 +442,45 @@ regulator-state-mem {
> >         };
> >  };
> >  
> > +&i2c1 {
> > +       clock-frequency = <400000>;
> > +       pinctrl-0 = <&i2c1_xfer &cif_clkouta>;
> > +       assigned-clocks = <&cru SCLK_CIF_OUT>;
> > +       assigned-clock-rates = <24000000>;
> > +       status = "okay";
> > +
> > +       wcam: camera@1a {
> > +               compatible = "sony,imx258";
> > +               reg = <0x1a>;
> > +               clocks = <&cru SCLK_CIF_OUT>; /* MIPI_MCLK0, derived from CIF_CLKO */
> > +               pinctrl-names = "default";
> > +               pinctrl-0 = <&camera_rst>;
> > +               /* Note: both cameras also depend on vcca1v8_codec to power the I2C bus. */
> 
> Should that be represented by the i2c1 bus then?
> 
> > +               vif-supply = <&vcc1v8_dvp>;
> > +               vana-supply = <&avdd2v8_dvp>;
> > +               vdig-supply = <&vcc1v2_dvp>; /* DVDD_DVP is the same as VCC1V2_DVP */
> > +               reset-gpios = <&gpio1 RK_PA0 GPIO_ACTIVE_LOW>;
> > +               orientation = <1>; /* V4L2_CAMERA_ORIENTATION_BACK */
> > +               rotation = <270>;
> > +               lens-focus = <&wcam_lens>;
> > +
> > +               port {
> > +                       wcam_out: endpoint {
> > +                               remote-endpoint = <&mipi_in_wcam>;
> > +                               data-lanes = <1 2 3 4>;
> > +                               link-frequencies = /bits/ 64 <636000000>;
> > +                       };
> > +               };
> > +       };
> > +
> > +       wcam_lens: camera-lens@c {
> > +               compatible = "dongwoon,dw9714";
> > +               reg = <0x0c>;
> > +               /* Same I2c bus as both cameras, depends on vcca1v8_codec for power. */
> > +               vcc-supply = <&vcc1v8_dvp>;
> > +       };
> > +};
> > +
> >  &i2c3 {
> >         i2c-scl-rising-time-ns = <450>;
> >         i2c-scl-falling-time-ns = <15>;
> > @@ -462,6 +523,28 @@ &io_domains {
> >         status = "okay";
> >  };
> >  
> > +&isp1 {
> > +       status = "okay";
> > +
> > +       ports {
> > +               port@0 {
> > +                       mipi_in_wcam: endpoint@0 {
> > +                               reg = <0>;
> > +                               remote-endpoint = <&wcam_out>;
> > +                               data-lanes = <1 2 3 4>;
> > +                       };
> > +               };
> > +       };
> > +};
> > +
> > +&mipi_dphy_rx0 {
> > +       status = "okay";
> > +};
> > +
> > +&isp1_mmu {
> > +       status = "okay";
> > +};
> > +
> >  &mipi_dsi {
> >         status = "okay";
> >         clock-master;
> > @@ -495,6 +578,10 @@ mipi_in_panel: endpoint {
> >         };
> >  };
> >  
> > +&mipi_dsi1 {
> > +       status = "okay";
> > +};
> > +
> 
> Is this an unrelated change? The Camera should be on CSI - not DSI?

Never mind - it seems these are probably already applied ;-)

--
Kieran


> 
> >  &pmu_io_domains {
> >         pmu1830-supply = <&vcc_1v8>;
> >         status = "okay";
> > @@ -507,6 +594,12 @@ pwrbtn_pin: pwrbtn-pin {
> >                 };
> >         };
> >  
> > +       camera {
> > +               camera_rst_l: camera-rst-l {
> > +                       rockchip,pins = <1 RK_PA0 RK_FUNC_GPIO &pcfg_pull_none>;
> > +               };
> > +       };
> > +
> >         leds {
> >                 red_led_pin: red-led-pin {
> >                         rockchip,pins = <4 RK_PD2 RK_FUNC_GPIO &pcfg_pull_none>;
> > 
> > -- 
> > 2.49.0
> > 
> > 
> > _______________________________________________
> > Linux-rockchip mailing list
> > Linux-rockchip@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-rockchip

[PATCH v4 4/4] arm64: dts: rockchip: describe the OV8858 user camera on PinePhone Pro

[Olivier Benjamin]

Add the description of the front/user camera (OV8858) on the PinePhone Pro
to the device dts file.
It receives commands over SCCB, an I2C-compatible protocol, at
I2C address 0x36 and transmits data over CSI-MIPI.
I confirmed this address experimentally.

The pin control mapping was again extracted from the PinePhone Pro
schematic v1.0 as well as the RK3399 datasheet revision 1.8.

Table 2-3 in section 2.8 of the RK3399 datasheet contains the mapping
of IO functions for the SoC pins. Page 52 shows GPIO1_A4, page 54 shows
GPIO2_B4.

For the reset (RESET) signal:
page 11 quadrant D2             | p.18 q.B3-4 | p.18 q.C2
RK3399_E.R28 -> GPIO1_A4 -> Camera2_RST -> MIPI_RST1 -> OV8858.12

For the powerdown (PWDN) signal:
page 9 quadrants D4-5          | p.18 q.B2
RK3399_L.F31 -> GPIO2_B4 -> DVP_PDN0_H -> OV8858.14

Helped-by: Dragan Simic <dsimic@manjaro.org>
Co-developed-by: Ondrej Jirman <megi@xff.cz>
Signed-off-by: Ondrej Jirman <megi@xff.cz>
Signed-off-by: Olivier Benjamin <olivier.benjamin@bootlin.com>
---
 .../boot/dts/rockchip/rk3399-pinephone-pro.dts     | 49 +++++++++++++++++++++-
 1 file changed, 47 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk3399-pinephone-pro.dts b/arch/arm64/boot/dts/rockchip/rk3399-pinephone-pro.dts
index b335d044138b413359ba54269ab646142254a55f..c1a151ed4215638895ef5c9872d746dca38cb671 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-pinephone-pro.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3399-pinephone-pro.dts
@@ -454,7 +454,7 @@ wcam: camera@1a {
 		reg = <0x1a>;
 		clocks = <&cru SCLK_CIF_OUT>; /* MIPI_MCLK0, derived from CIF_CLKO */
 		pinctrl-names = "default";
-		pinctrl-0 = <&camera_rst>;
+		pinctrl-0 = <&camera_rst_l>;
 		/* Note: both cameras also depend on vcca1v8_codec to power the I2C bus. */
 		vif-supply = <&vcc1v8_dvp>;
 		vana-supply = <&avdd2v8_dvp>;
@@ -479,6 +479,27 @@ wcam_lens: camera-lens@c {
 		/* Same I2c bus as both cameras, depends on vcca1v8_codec for power. */
 		vcc-supply = <&vcc1v8_dvp>;
 	};
+
+	ucam: camera@36 {
+		compatible = "ovti,ov8858";
+		reg = <0x36>;
+		clocks = <&cru SCLK_CIF_OUT>; /* MIPI_MCLK1, derived from CIF_CLK0 */
+		clock-names = "xvclk";
+		pinctrl-names = "default";
+		pinctrl-0 = <&camera2_rst_l &dvp_pdn0_h>;
+		dovdd-supply = <&vcc1v8_dvp>;
+		reset-gpios = <&gpio1 RK_PA4 GPIO_ACTIVE_LOW>;
+		powerdown-gpios = <&gpio2 RK_PB4 GPIO_ACTIVE_LOW>;
+		orientation = <0>; /* V4L2_CAMERA_ORIENTATION_FRONT */
+		rotation = <90>;
+
+		port {
+			ucam_out: endpoint {
+				remote-endpoint = <&mipi_in_ucam>;
+				data-lanes = <1 2 3 4>;
+			};
+		};
+	};
 };
 
 &i2c3 {
@@ -523,6 +544,24 @@ &io_domains {
 	status = "okay";
 };
 
+&isp0 {
+	status = "okay";
+
+	ports {
+		port@0 {
+			mipi_in_ucam: endpoint@0 {
+				reg = <0>;
+				remote-endpoint = <&ucam_out>;
+				data-lanes = <1 2 3 4>;
+			};
+		};
+	};
+};
+
+&isp0_mmu {
+	status = "okay";
+};
+
 &isp1 {
 	status = "okay";
 
@@ -594,10 +633,16 @@ pwrbtn_pin: pwrbtn-pin {
 		};
 	};
 
-	camera {
+	cameras {
 		camera_rst_l: camera-rst-l {
 			rockchip,pins = <1 RK_PA0 RK_FUNC_GPIO &pcfg_pull_none>;
 		};
+		camera2_rst_l: camera2-rst-l {
+			rockchip,pins = <1 RK_PA4 RK_FUNC_GPIO &pcfg_pull_none>;
+		};
+		dvp_pdn0_h: dvp-pdn0-h {
+			rockchip,pins = <2 RK_PB4 RK_FUNC_GPIO &pcfg_pull_none>;
+		};
 	};
 
 	leds {

-- 
2.49.0

Re: [PATCH v4 0/4] Describe the cameras in the PinePhone Pro dts

[Sakari Ailus]

Hi Olivier,

On Fri, Jun 20, 2025 at 05:21:31PM +0200, Olivier Benjamin wrote:
> This series adds support for the Pine64 PinePhone Pro's rear and front
> cameras in Device Tree.
> This is based on some of Ondrej Jirman's patches hosted in his tree at
> https://codeberg.org/megi/linux, but I have also fully reviewed and
> re-written the code from the RK3399 datasheet, the PinePhone Pro
> schematic, and the IMX258-0AQH5 software reference manual.
> 
> I have tested these changes on my PinePhone Pro and am able to take
> photos from both cameras using libcamera's cam.
> 
> This series has raised a question about the proper label name for the
> front/user camera and rear/world camera for phones.
> This series is using "ucam" and "wcam", which is used in a few other
> Rockship DTBs:
>  - arch/arm64/boot/dts/rockchip/px30-evb.dts
>  - rk3399-gru-scarlet.dtsi
> 
> Signed-off-by: Olivier Benjamin <olivier.benjamin@bootlin.com>

Thanks for the patches.

I've picked the first two in the set, presumably the rest will be merged
via another tree?

-- 
Regards,

Sakari Ailus

Re: [PATCH v4 0/4] Describe the cameras in the PinePhone Pro dts

[Heiko Stuebner]

Hi,

Am Montag, 23. Juni 2025, 08:47:44 Mitteleuropäische Sommerzeit schrieb Sakari Ailus:
> Hi Olivier,
> 
> On Fri, Jun 20, 2025 at 05:21:31PM +0200, Olivier Benjamin wrote:
> > This series adds support for the Pine64 PinePhone Pro's rear and front
> > cameras in Device Tree.
> > This is based on some of Ondrej Jirman's patches hosted in his tree at
> > https://codeberg.org/megi/linux, but I have also fully reviewed and
> > re-written the code from the RK3399 datasheet, the PinePhone Pro
> > schematic, and the IMX258-0AQH5 software reference manual.
> > 
> > I have tested these changes on my PinePhone Pro and am able to take
> > photos from both cameras using libcamera's cam.
> > 
> > This series has raised a question about the proper label name for the
> > front/user camera and rear/world camera for phones.
> > This series is using "ucam" and "wcam", which is used in a few other
> > Rockship DTBs:
> >  - arch/arm64/boot/dts/rockchip/px30-evb.dts
> >  - rk3399-gru-scarlet.dtsi
> > 
> > Signed-off-by: Olivier Benjamin <olivier.benjamin@bootlin.com>
> 
> Thanks for the patches.
> 
> I've picked the first two in the set, presumably the rest will be merged
> via another tree?

correct, and with the first two being applied, I can now also safely pick
the other two :-)

Thanks a lot
Heiko

Re: [PATCH v4 0/4] Describe the cameras in the PinePhone Pro dts

[Sakari Ailus]

Hi Heiko,

On Mon, Jun 23, 2025 at 12:05:22PM +0200, Heiko Stuebner wrote:
> Hi,
> 
> Am Montag, 23. Juni 2025, 08:47:44 Mitteleuropäische Sommerzeit schrieb Sakari Ailus:
> > Hi Olivier,
> > 
> > On Fri, Jun 20, 2025 at 05:21:31PM +0200, Olivier Benjamin wrote:
> > > This series adds support for the Pine64 PinePhone Pro's rear and front
> > > cameras in Device Tree.
> > > This is based on some of Ondrej Jirman's patches hosted in his tree at
> > > https://codeberg.org/megi/linux, but I have also fully reviewed and
> > > re-written the code from the RK3399 datasheet, the PinePhone Pro
> > > schematic, and the IMX258-0AQH5 software reference manual.
> > > 
> > > I have tested these changes on my PinePhone Pro and am able to take
> > > photos from both cameras using libcamera's cam.
> > > 
> > > This series has raised a question about the proper label name for the
> > > front/user camera and rear/world camera for phones.
> > > This series is using "ucam" and "wcam", which is used in a few other
> > > Rockship DTBs:
> > >  - arch/arm64/boot/dts/rockchip/px30-evb.dts
> > >  - rk3399-gru-scarlet.dtsi
> > > 
> > > Signed-off-by: Olivier Benjamin <olivier.benjamin@bootlin.com>
> > 
> > Thanks for the patches.
> > 
> > I've picked the first two in the set, presumably the rest will be merged
> > via another tree?
> 
> correct, and with the first two being applied, I can now also safely pick
> the other two :-)

Once in my tree, they'll next end up to the media committers' tree and
after some time you should be able to find them in linux-next. This process
will take some time. Just FYI.

-- 
Sakari Ailus

Re: [PATCH v4 0/4] Describe the cameras in the PinePhone Pro dts

[Heiko Stübner]

Am Montag, 23. Juni 2025, 12:08:07 Mitteleuropäische Sommerzeit schrieb Sakari Ailus:
> Hi Heiko,
> 
> On Mon, Jun 23, 2025 at 12:05:22PM +0200, Heiko Stuebner wrote:
> > Hi,
> > 
> > Am Montag, 23. Juni 2025, 08:47:44 Mitteleuropäische Sommerzeit schrieb Sakari Ailus:
> > > Hi Olivier,
> > > 
> > > On Fri, Jun 20, 2025 at 05:21:31PM +0200, Olivier Benjamin wrote:
> > > > This series adds support for the Pine64 PinePhone Pro's rear and front
> > > > cameras in Device Tree.
> > > > This is based on some of Ondrej Jirman's patches hosted in his tree at
> > > > https://codeberg.org/megi/linux, but I have also fully reviewed and
> > > > re-written the code from the RK3399 datasheet, the PinePhone Pro
> > > > schematic, and the IMX258-0AQH5 software reference manual.
> > > > 
> > > > I have tested these changes on my PinePhone Pro and am able to take
> > > > photos from both cameras using libcamera's cam.
> > > > 
> > > > This series has raised a question about the proper label name for the
> > > > front/user camera and rear/world camera for phones.
> > > > This series is using "ucam" and "wcam", which is used in a few other
> > > > Rockship DTBs:
> > > >  - arch/arm64/boot/dts/rockchip/px30-evb.dts
> > > >  - rk3399-gru-scarlet.dtsi
> > > > 
> > > > Signed-off-by: Olivier Benjamin <olivier.benjamin@bootlin.com>
> > > 
> > > Thanks for the patches.
> > > 
> > > I've picked the first two in the set, presumably the rest will be merged
> > > via another tree?
> > 
> > correct, and with the first two being applied, I can now also safely pick
> > the other two :-)
> 
> Once in my tree, they'll next end up to the media committers' tree and
> after some time you should be able to find them in linux-next. This process
> will take some time. Just FYI.

thanks for the heads up :-) .

So I'll give that a bit more time for the bindings to appear.

Thanks
Heiko

Re: (subset) [PATCH v4 0/4] Describe the cameras in the PinePhone Pro dts

[Heiko Stuebner]

On Fri, 20 Jun 2025 17:21:31 +0200, Olivier Benjamin wrote:
> This series adds support for the Pine64 PinePhone Pro's rear and front
> cameras in Device Tree.
> This is based on some of Ondrej Jirman's patches hosted in his tree at
> https://codeberg.org/megi/linux, but I have also fully reviewed and
> re-written the code from the RK3399 datasheet, the PinePhone Pro
> schematic, and the IMX258-0AQH5 software reference manual.
> 
> [...]

Applied, thanks!

[3/4] arm64: dts: rockchip: describe I2c Bus 1 and IMX258 world camera on PinePhone Pro
      commit: b610dc822f38010508b54c8f556a3de28556d273
[4/4] arm64: dts: rockchip: describe the OV8858 user camera on PinePhone Pro
      commit: 9259c81a185663c2a7cd6a5301df4b9645980528

So I found the binding-patched from the media-tree in linux-next now,
so picked up the dts patches.

I fixed up some issues, please double check:
- the pinctrl-handle in patch3 did not match the created one
  (which patch 4 fixed) but I adapted patch 3 instead
- sorting of the avdd regulator (alphabetical by node-name please)
- sorting of properties, broadly speaking, alphabetical too
  (caveats apply, see dt documentation)

Best regards,
-- 
Heiko Stuebner <heiko@sntech.de>

Re: [PATCH v4 0/4] Describe the cameras in the PinePhone Pro dts

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 928 bytes --]

On Fri 2025-06-20 17:21:31, Olivier Benjamin wrote:
> This series adds support for the Pine64 PinePhone Pro's rear and front
> cameras in Device Tree.
> This is based on some of Ondrej Jirman's patches hosted in his tree at
> https://codeberg.org/megi/linux, but I have also fully reviewed and
> re-written the code from the RK3399 datasheet, the PinePhone Pro
> schematic, and the IMX258-0AQH5 software reference manual.
> 
> I have tested these changes on my PinePhone Pro and am able to take
> photos from both cameras using libcamera's cam.
> 
> This series has raised a question about the proper label name for the
> front/user camera and rear/world camera for phones.

I am not familiar with "user" / "world" cameras, but "selfie" and
"main" is something I've seen before.

Best regards,
								Pavel
-- 
I don't work for Nazis and criminals, and neither should you.
Boycott Putin, Trump, and Musk!

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]