On 5/14/25 06:59, Olivier Benjamin wrote:

On 5/13/25 20:23, Heiko Stuebner wrote:

quoted

Hi Olivier,

Hello Heiko, thanks for having a look!

Hi again Heiko,

quoted

Am Freitag, 9. Mai 2025, 23:51:39 Mitteleuropäische Sommerzeit schrieb 
Olivier Benjamin:

quoted

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 [off-list ref]
Co-developed-by: Ondrej Jirman <megi@xff.cz>
Signed-off-by: Ondrej Jirman <megi@xff.cz>
Signed-off-by: Olivier Benjamin <redacted>

how independent are the devicetree changes from the binding changes?
As the binding change "only" includes other properties.

They are pretty independent: the binding changes are only needed to 
suppress warnings on the devicetree.
However, the changes to the devicetree are the motivation for the 
changes to the binding: the other properties are not strictly necessary 
otherwise.

Have you had a chance to have a look?
Are any changes needed in your opinion?

quoted

Heiko

quoted

---
  .../boot/dts/rockchip/rk3399-pinephone-pro.dts     | 94 +++++++++++ 
+++++++++++
  1 file changed, 94 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..588e2d8a049cc649aa227c7a885bd494f23fbdf8 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,46 @@ 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 */
+        clock-names = "xvclk";
+        pinctrl-names = "default";
+        pinctrl-0 = <&wcam_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 +524,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 +579,10 @@ mipi_in_panel: endpoint {

      };
  };
+&mipi_dsi1 {
+    status = "okay";
+};
+
  &pmu_io_domains {
      pmu1830-supply = <&vcc_1v8>;
      status = "okay";

@@ -507,6 +595,12 @@ pwrbtn_pin: pwrbtn-pin {

          };
      };
+    camera {
+        wcam_rst: wcam-rst {
+            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>;

-- 
Olivier Benjamin, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com