[PATCH 0/6] media: qcom: camss: Add sc7180 support

[PATCH 0/6] media: qcom: camss: Add sc7180 support

[George Chan via B4 Relay]

SM7125 is the SoC found in the Xiaomi Redmi Note 9 Pro(joyeuse) cellphone.
This series adds support to bring up the CSIPHY, CSID, VFE/RDI interfaces.

Since SM7125 is a low-speed variant of SC7180, SC7180 testers please
take a look and have a test as well.

sc7180 provides

- 2 x VFE
- 1 x VFE Lite
- 2 x CSID
- 1 x CSID Lite
- 4 x CSI PHY

The sc7180-camss binding should be comaptible with sdm845 yaml.
I've copied a new yaml from sdm845-camss.yaml, strip all _src clk and leave
original maintainer. If this is not desirable then i can add binding to
existing sdm845 yaml instead.

In addition, a bootable tree of sm7125/joyeuse is availble at:
https://github.com/99degree/linux/tree/camss
  

Signed-off-by: George Chan <gchan9527@gmail.com>
---
George Chan (6):
      media: dt-bindings: media: camss: Add qcom,sc7180-camss binding
      media: qcom: camss: Add CAMSS_SC7180 enum
      media: qcom: camss: csiphy-3ph: Add Gen2 v1.2.2 two-phase MIPI CSI-2 DPHY init
      media: qcom: camss: Add sc7180 support
      media: qcom: camss: Add sc7180 resources
      [RFT]arm64: dts: qcom: sc7180: Add support for camss subsys

 .../bindings/media/qcom,sc7180-camss.yaml          | 324 +++++++++++++++++++++
 arch/arm64/boot/dts/qcom/sc7180.dtsi               | 134 +++++++++
 .../platform/qcom/camss/camss-csiphy-3ph-1-0.c     | 120 ++++++++
 drivers/media/platform/qcom/camss/camss-csiphy.c   |   1 +
 drivers/media/platform/qcom/camss/camss-vfe.c      |   3 +
 drivers/media/platform/qcom/camss/camss-video.c    |   1 +
 drivers/media/platform/qcom/camss/camss.c          | 218 +++++++++++++-
 drivers/media/platform/qcom/camss/camss.h          |   1 +
 8 files changed, 801 insertions(+), 1 deletion(-)
---
base-commit: 2102cb0d050d34d50b9642a3a50861787527e922
change-id: 20240621-b4-sc7180-camss-cddc6b60a9b4

Best regards,
-- 
George Chan <gchan9527@gmail.com>

[PATCH 1/6] media: dt-bindings: media: camss: Add qcom,sc7180-camss binding

[George Chan via B4 Relay]

From: George Chan <gchan9527@gmail.com>

Add bindings for qcom,sc7180-camss in order to support the camera
subsystem for sm7125 as found in the Xiaomi Redmi 9 Pro cellphone.

Signed-off-by: George Chan <gchan9527@gmail.com>
---
 .../bindings/media/qcom,sc7180-camss.yaml          | 324 +++++++++++++++++++++
 1 file changed, 324 insertions(+)

diff --git a/Documentation/devicetree/bindings/media/qcom,sc7180-camss.yaml b/Documentation/devicetree/bindings/media/qcom,sc7180-camss.yaml
new file mode 100644
index 000000000000..4dc10c32ee9c
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/qcom,sc7180-camss.yaml
@@ -0,0 +1,324 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/media/qcom,sc7180-camss.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm CAMSS ISP
+
+maintainers:
+  - Robert Foss <robert.foss@linaro.org>
+
+description: |
+  The CAMSS IP is a CSI decoder and ISP present on Qualcomm platforms
+
+properties:
+  compatible:
+    const: qcom,sc7180-camss
+
+  clocks:
+    minItems: 25
+    maxItems: 25
+
+  clock-names:
+    items:
+      - const: camnoc_axi
+      - const: cpas_ahb
+      - const: cphy_rx_src
+      - const: csi0
+      - const: csi1
+      - const: csi2
+      - const: csiphy0
+      - const: csiphy0_timer
+      - const: csiphy1
+      - const: csiphy1_timer
+      - const: csiphy2
+      - const: csiphy2_timer
+      - const: csiphy3
+      - const: csiphy3_timer
+      - const: gcc_camera_ahb
+      - const: gcc_camera_axi
+      - const: soc_ahb
+      - const: vfe0_axi
+      - const: vfe0
+      - const: vfe0_cphy_rx
+      - const: vfe1_axi
+      - const: vfe1
+      - const: vfe1_cphy_rx
+      - const: vfe_lite
+      - const: vfe_lite_cphy_rx
+
+  interrupts:
+    minItems: 10
+    maxItems: 10
+
+  interrupt-names:
+    items:
+      - const: csid0
+      - const: csid1
+      - const: csid2
+      - const: csiphy0
+      - const: csiphy1
+      - const: csiphy2
+      - const: csiphy3
+      - const: vfe0
+      - const: vfe1
+      - const: vfe_lite
+
+  iommus:
+    maxItems: 4
+
+  power-domains:
+    items:
+      - description: IFE0 GDSC - Image Front End, Global Distributed Switch Controller.
+      - description: IFE1 GDSC - Image Front End, Global Distributed Switch Controller.
+      - description: Titan GDSC - Titan ISP Block, Global Distributed Switch Controller.
+
+  ports:
+    $ref: /schemas/graph.yaml#/properties/ports
+
+    description:
+      CSI input ports.
+
+    properties:
+      port@0:
+        $ref: /schemas/graph.yaml#/$defs/port-base
+        unevaluatedProperties: false
+        description:
+          Input port for receiving CSI data.
+
+        properties:
+          endpoint:
+            $ref: video-interfaces.yaml#
+            unevaluatedProperties: false
+
+            properties:
+              data-lanes:
+                minItems: 1
+                maxItems: 4
+
+            required:
+              - data-lanes
+
+      port@1:
+        $ref: /schemas/graph.yaml#/$defs/port-base
+        unevaluatedProperties: false
+        description:
+          Input port for receiving CSI data.
+
+        properties:
+          endpoint:
+            $ref: video-interfaces.yaml#
+            unevaluatedProperties: false
+
+            properties:
+              data-lanes:
+                minItems: 1
+                maxItems: 4
+
+            required:
+              - data-lanes
+
+      port@2:
+        $ref: /schemas/graph.yaml#/$defs/port-base
+        unevaluatedProperties: false
+        description:
+          Input port for receiving CSI data.
+
+        properties:
+          endpoint:
+            $ref: video-interfaces.yaml#
+            unevaluatedProperties: false
+
+            properties:
+              data-lanes:
+                minItems: 1
+                maxItems: 4
+
+            required:
+              - data-lanes
+
+      port@3:
+        $ref: /schemas/graph.yaml#/$defs/port-base
+        unevaluatedProperties: false
+        description:
+          Input port for receiving CSI data.
+
+        properties:
+          endpoint:
+            $ref: video-interfaces.yaml#
+            unevaluatedProperties: false
+
+            properties:
+              data-lanes:
+                minItems: 1
+                maxItems: 4
+
+            required:
+              - data-lanes
+
+  reg:
+    minItems: 10
+    maxItems: 10
+
+  reg-names:
+    items:
+      - const: csid0
+      - const: csid1
+      - const: csid2
+      - const: csiphy0
+      - const: csiphy1
+      - const: csiphy2
+      - const: csiphy3
+      - const: vfe0
+      - const: vfe1
+      - const: vfe_lite
+
+  vdda-phy-supply:
+    description:
+      Phandle to a regulator supply to PHY core block.
+
+  vdda-pll-supply:
+    description:
+      Phandle to 1.8V regulator supply to PHY refclk pll block.
+
+required:
+  - clock-names
+  - clocks
+  - compatible
+  - interrupt-names
+  - interrupts
+  - iommus
+  - power-domains
+  - reg
+  - reg-names
+  - vdda-phy-supply
+  - vdda-pll-supply
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/clock/qcom,camcc-sc7180.h>
+    #include <dt-bindings/clock/qcom,gcc-sc7180.h>
+
+    soc {
+      #address-cells = <2>;
+      #size-cells = <2>;
+
+      camss: camss@acb3000 {
+        compatible = "qcom,sc7180-camss";
+
+        clocks = <&clock_camcc CAM_CC_CAMNOC_AXI_CLK>,
+          <&clock_camcc CAM_CC_CPAS_AHB_CLK>,
+          <&clock_camcc CAM_CC_IFE_0_CSID_CLK>,
+          <&clock_camcc CAM_CC_IFE_1_CSID_CLK>,
+          <&clock_camcc CAM_CC_IFE_LITE_CSID_CLK>,
+          <&clock_camcc CAM_CC_CSIPHY0_CLK>,
+          <&clock_camcc CAM_CC_CSI0PHYTIMER_CLK>,
+          <&clock_camcc CAM_CC_CSIPHY1_CLK>,
+          <&clock_camcc CAM_CC_CSI1PHYTIMER_CLK>,
+          <&clock_camcc CAM_CC_CSIPHY2_CLK>,
+          <&clock_camcc CAM_CC_CSI2PHYTIMER_CLK>,
+          <&clock_camcc CAM_CC_CSIPHY3_CLK>,
+          <&clock_camcc CAM_CC_CSI3PHYTIMER_CLK>,
+          <&gcc GCC_CAMERA_AHB_CLK>,
+          <&gcc GCC_CAMERA_HF_AXI_CLK>,
+          <&clock_camcc CAM_CC_SOC_AHB_CLK>,
+          <&clock_camcc CAM_CC_IFE_0_AXI_CLK>,
+          <&clock_camcc CAM_CC_IFE_0_CLK>,
+          <&clock_camcc CAM_CC_IFE_0_CPHY_RX_CLK>,
+          <&clock_camcc CAM_CC_IFE_1_AXI_CLK>,
+          <&clock_camcc CAM_CC_IFE_1_CLK>,
+          <&clock_camcc CAM_CC_IFE_1_CPHY_RX_CLK>,
+          <&clock_camcc CAM_CC_IFE_LITE_CLK>,
+          <&clock_camcc CAM_CC_IFE_LITE_CPHY_RX_CLK>;
+
+        clock-names = "camnoc_axi",
+          "cpas_ahb",
+          "csi0",
+          "csi1",
+          "csi2",
+          "csiphy0",
+          "csiphy0_timer",
+          "csiphy1",
+          "csiphy1_timer",
+          "csiphy2",
+          "csiphy2_timer",
+          "csiphy3",
+          "csiphy3_timer",
+          "gcc_camera_ahb",
+          "gcc_camera_axi",
+          "soc_ahb",
+          "vfe0_axi",
+          "vfe0",
+          "vfe0_cphy_rx",
+          "vfe1_axi",
+          "vfe1",
+          "vfe1_cphy_rx",
+          "vfe_lite",
+          "vfe_lite_cphy_rx";
+
+        interrupts = <GIC_SPI 464 IRQ_TYPE_LEVEL_HIGH>,
+          <GIC_SPI 466 IRQ_TYPE_LEVEL_HIGH>,
+          <GIC_SPI 473 IRQ_TYPE_LEVEL_HIGH>,
+          <GIC_SPI 477 IRQ_TYPE_LEVEL_HIGH>,
+          <GIC_SPI 478 IRQ_TYPE_LEVEL_HIGH>,
+          <GIC_SPI 479 IRQ_TYPE_LEVEL_HIGH>,
+          <GIC_SPI 448 IRQ_TYPE_LEVEL_HIGH>,
+          <GIC_SPI 465 IRQ_TYPE_LEVEL_HIGH>,
+          <GIC_SPI 467 IRQ_TYPE_LEVEL_HIGH>,
+          <GIC_SPI 472 IRQ_TYPE_LEVEL_HIGH>;
+
+        interrupt-names = "csid0",
+          "csid1",
+          "csid2",
+          "csiphy0",
+          "csiphy1",
+          "csiphy2",
+          "csiphy3",
+          "vfe0",
+          "vfe1",
+          "vfe_lite";
+
+        iommus = <&apps_smmu 0x820 0x0>,
+          <&apps_smmu 0x840 0x0>,
+          <&apps_smmu 0x860 0x0>;
+
+        power-domains = <&camcc IFE_0_GDSC>,
+          <&camcc IFE_1_GDSC>,
+          <&camcc TITAN_TOP_GDSC>;
+
+        reg = <0 0xacb3000 0 0x1000>,
+          <0 0xacba000 0 0x1000>,
+          <0 0xacc8000 0 0x1000>,
+          <0 0xac65000 0 0x1000>,
+          <0 0xac66000 0 0x1000>,
+          <0 0xac67000 0 0x1000>,
+          <0 0xac68000 0 0x1000>,
+          <0 0xacaf000 0 0x4000>,
+          <0 0xacb6000 0 0x4000>,
+          <0 0xacc4000 0 0x4000>;
+
+        reg-names = "csid0",
+          "csid1",
+          "csid2",
+          "csiphy0",
+          "csiphy1",
+          "csiphy2",
+          "csiphy3",
+          "vfe0",
+          "vfe1",
+          "vfe_lite";
+
+        vdda-phy-supply = <&vreg_l1a_0p875>;
+        vdda-pll-supply = <&vreg_l26a_1p2>;
+
+        ports {
+          #address-cells = <1>;
+          #size-cells = <0>;
+        };
+      };
+    };

-- 
2.34.1

Re: [PATCH 1/6] media: dt-bindings: media: camss: Add qcom,sc7180-camss binding

[Krzysztof Kozlowski]

On 21/06/2024 11:40, George Chan via B4 Relay wrote:
> From: George Chan <gchan9527@gmail.com>
> 
> Add bindings for qcom,sc7180-camss in order to support the camera
> subsystem for sm7125 as found in the Xiaomi Redmi 9 Pro cellphone.
> 
> Signed-off-by: George Chan <gchan9527@gmail.com>

Subject: just one media (first). No need to write media: media: ...


A nit, subject: drop second/last, redundant "binding". The "dt-bindings"
prefix is already stating that these are bindings.
See also:
https://elixir.bootlin.com/linux/v6.7-rc8/source/Documentation/devicetree/bindings/submitting-patches.rst#L18


> ---
>  .../bindings/media/qcom,sc7180-camss.yaml          | 324 +++++++++++++++++++++
>  1 file changed, 324 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/media/qcom,sc7180-camss.yaml b/Documentation/devicetree/bindings/media/qcom,sc7180-camss.yaml
> new file mode 100644
> index 000000000000..4dc10c32ee9c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/qcom,sc7180-camss.yaml
> @@ -0,0 +1,324 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/media/qcom,sc7180-camss.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Qualcomm CAMSS ISP

What is CAMSS?

> +
> +maintainers:
> +  - Robert Foss <robert.foss@linaro.org>

For sure this is not true. Robert does not work in Linaro and I doubt he
cares that much about camss.

> +
> +description: |

Do not need '|' unless you need to preserve formatting.

> +  The CAMSS IP is a CSI decoder and ISP present on Qualcomm platforms
> +
> +properties:
> +  compatible:
> +    const: qcom,sc7180-camss
> +
> +  clocks:
> +    minItems: 25

Drop minItems

> +    maxItems: 25
> +
> +  clock-names:
> +    items:
> +      - const: camnoc_axi
> +      - const: cpas_ahb
> +      - const: cphy_rx_src
> +      - const: csi0
> +      - const: csi1
> +      - const: csi2
> +      - const: csiphy0
> +      - const: csiphy0_timer
> +      - const: csiphy1
> +      - const: csiphy1_timer
> +      - const: csiphy2
> +      - const: csiphy2_timer
> +      - const: csiphy3
> +      - const: csiphy3_timer
> +      - const: gcc_camera_ahb
> +      - const: gcc_camera_axi
> +      - const: soc_ahb
> +      - const: vfe0_axi
> +      - const: vfe0
> +      - const: vfe0_cphy_rx
> +      - const: vfe1_axi
> +      - const: vfe1
> +      - const: vfe1_cphy_rx
> +      - const: vfe_lite
> +      - const: vfe_lite_cphy_rx
> +
> +  interrupts:
> +    minItems: 10

Drop minItems

> +    maxItems: 10
> +
> +  interrupt-names:
> +    items:
> +      - const: csid0
> +      - const: csid1
> +      - const: csid2
> +      - const: csiphy0
> +      - const: csiphy1
> +      - const: csiphy2
> +      - const: csiphy3
> +      - const: vfe0
> +      - const: vfe1
> +      - const: vfe_lite
> +
> +  iommus:
> +    maxItems: 4
> +
> +  power-domains:
> +    items:
> +      - description: IFE0 GDSC - Image Front End, Global Distributed Switch Controller.
> +      - description: IFE1 GDSC - Image Front End, Global Distributed Switch Controller.
> +      - description: Titan GDSC - Titan ISP Block, Global Distributed Switch Controller.
> +
> +  ports:
> +    $ref: /schemas/graph.yaml#/properties/ports
> +
> +    description:
> +      CSI input ports.
> +
> +    properties:
> +      port@0:
> +        $ref: /schemas/graph.yaml#/$defs/port-base
> +        unevaluatedProperties: false
> +        description:
> +          Input port for receiving CSI data.
> +
> +        properties:
> +          endpoint:
> +            $ref: video-interfaces.yaml#
> +            unevaluatedProperties: false
> +
> +            properties:
> +              data-lanes:
> +                minItems: 1
> +                maxItems: 4
> +
> +            required:
> +              - data-lanes
> +
> +      port@1:
> +        $ref: /schemas/graph.yaml#/$defs/port-base
> +        unevaluatedProperties: false
> +        description:
> +          Input port for receiving CSI data.
> +
> +        properties:
> +          endpoint:
> +            $ref: video-interfaces.yaml#
> +            unevaluatedProperties: false
> +
> +            properties:
> +              data-lanes:
> +                minItems: 1
> +                maxItems: 4
> +
> +            required:
> +              - data-lanes
> +
> +      port@2:
> +        $ref: /schemas/graph.yaml#/$defs/port-base
> +        unevaluatedProperties: false
> +        description:
> +          Input port for receiving CSI data.
> +
> +        properties:
> +          endpoint:
> +            $ref: video-interfaces.yaml#
> +            unevaluatedProperties: false
> +
> +            properties:
> +              data-lanes:
> +                minItems: 1
> +                maxItems: 4
> +
> +            required:
> +              - data-lanes
> +
> +      port@3:
> +        $ref: /schemas/graph.yaml#/$defs/port-base
> +        unevaluatedProperties: false
> +        description:
> +          Input port for receiving CSI data.
> +
> +        properties:
> +          endpoint:
> +            $ref: video-interfaces.yaml#
> +            unevaluatedProperties: false
> +
> +            properties:
> +              data-lanes:
> +                minItems: 1
> +                maxItems: 4
> +
> +            required:
> +              - data-lanes
> +
> +  reg:
> +    minItems: 10

Drop minItems

> +    maxItems: 10
> +
> +  reg-names:
> +    items:
> +      - const: csid0
> +      - const: csid1
> +      - const: csid2
> +      - const: csiphy0
> +      - const: csiphy1
> +      - const: csiphy2
> +      - const: csiphy3
> +      - const: vfe0
> +      - const: vfe1
> +      - const: vfe_lite
> +
> +  vdda-phy-supply:
> +    description:
> +      Phandle to a regulator supply to PHY core block.
> +
> +  vdda-pll-supply:
> +    description:
> +      Phandle to 1.8V regulator supply to PHY refclk pll block.
> +
> +required:
> +  - clock-names
> +  - clocks
> +  - compatible

Keep the list ordered, the same as list properties.

> +  - interrupt-names
> +  - interrupts
> +  - iommus
> +  - power-domains
> +  - reg
> +  - reg-names
> +  - vdda-phy-supply
> +  - vdda-pll-supply
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/clock/qcom,camcc-sc7180.h>
> +    #include <dt-bindings/clock/qcom,gcc-sc7180.h>
> +
> +    soc {
> +      #address-cells = <2>;
> +      #size-cells = <2>;
> +
> +      camss: camss@acb3000 {
> +        compatible = "qcom,sc7180-camss";
> +
> +        clocks = <&clock_camcc CAM_CC_CAMNOC_AXI_CLK>,
> +          <&clock_camcc CAM_CC_CPAS_AHB_CLK>,

Missed alignment with previous <.

> +          <&clock_camcc CAM_CC_IFE_0_CSID_CLK>,
> +          <&clock_camcc CAM_CC_IFE_1_CSID_CLK>,
> +          <&clock_camcc CAM_CC_IFE_LITE_CSID_CLK>,
> +          <&clock_camcc CAM_CC_CSIPHY0_CLK>,
> +          <&clock_camcc CAM_CC_CSI0PHYTIMER_CLK>,
> +          <&clock_camcc CAM_CC_CSIPHY1_CLK>,
> +          <&clock_camcc CAM_CC_CSI1PHYTIMER_CLK>,
> +          <&clock_camcc CAM_CC_CSIPHY2_CLK>,
> +          <&clock_camcc CAM_CC_CSI2PHYTIMER_CLK>,
> +          <&clock_camcc CAM_CC_CSIPHY3_CLK>,
> +          <&clock_camcc CAM_CC_CSI3PHYTIMER_CLK>,
> +          <&gcc GCC_CAMERA_AHB_CLK>,
> +          <&gcc GCC_CAMERA_HF_AXI_CLK>,
> +          <&clock_camcc CAM_CC_SOC_AHB_CLK>,
> +          <&clock_camcc CAM_CC_IFE_0_AXI_CLK>,
> +          <&clock_camcc CAM_CC_IFE_0_CLK>,
> +          <&clock_camcc CAM_CC_IFE_0_CPHY_RX_CLK>,
> +          <&clock_camcc CAM_CC_IFE_1_AXI_CLK>,
> +          <&clock_camcc CAM_CC_IFE_1_CLK>,
> +          <&clock_camcc CAM_CC_IFE_1_CPHY_RX_CLK>,
> +          <&clock_camcc CAM_CC_IFE_LITE_CLK>,
> +          <&clock_camcc CAM_CC_IFE_LITE_CPHY_RX_CLK>;
> +
> +        clock-names = "camnoc_axi",
> +          "cpas_ahb",

Same problem.

> +          "csi0",
> +          "csi1",
> +          "csi2",
> +          "csiphy0",
> +          "csiphy0_timer",
> +          "csiphy1",
> +          "csiphy1_timer",
> +          "csiphy2",
> +          "csiphy2_timer",
> +          "csiphy3",
> +          "csiphy3_timer",
> +          "gcc_camera_ahb",
> +          "gcc_camera_axi",
> +          "soc_ahb",
> +          "vfe0_axi",
> +          "vfe0",
> +          "vfe0_cphy_rx",
> +          "vfe1_axi",
> +          "vfe1",
> +          "vfe1_cphy_rx",
> +          "vfe_lite",
> +          "vfe_lite_cphy_rx";
> +
> +        interrupts = <GIC_SPI 464 IRQ_TYPE_LEVEL_HIGH>,
> +          <GIC_SPI 466 IRQ_TYPE_LEVEL_HIGH>,
> +          <GIC_SPI 473 IRQ_TYPE_LEVEL_HIGH>,
> +          <GIC_SPI 477 IRQ_TYPE_LEVEL_HIGH>,
> +          <GIC_SPI 478 IRQ_TYPE_LEVEL_HIGH>,
> +          <GIC_SPI 479 IRQ_TYPE_LEVEL_HIGH>,
> +          <GIC_SPI 448 IRQ_TYPE_LEVEL_HIGH>,
> +          <GIC_SPI 465 IRQ_TYPE_LEVEL_HIGH>,
> +          <GIC_SPI 467 IRQ_TYPE_LEVEL_HIGH>,
> +          <GIC_SPI 472 IRQ_TYPE_LEVEL_HIGH>;
> +
> +        interrupt-names = "csid0",
> +          "csid1",
> +          "csid2",
> +          "csiphy0",
> +          "csiphy1",
> +          "csiphy2",
> +          "csiphy3",
> +          "vfe0",
> +          "vfe1",
> +          "vfe_lite";
> +
> +        iommus = <&apps_smmu 0x820 0x0>,
> +          <&apps_smmu 0x840 0x0>,
> +          <&apps_smmu 0x860 0x0>;
> +
> +        power-domains = <&camcc IFE_0_GDSC>,
> +          <&camcc IFE_1_GDSC>,
> +          <&camcc TITAN_TOP_GDSC>;
> +
> +        reg = <0 0xacb3000 0 0x1000>,

reg is always the second property. See DTS coding style.

> +          <0 0xacba000 0 0x1000>,
> +          <0 0xacc8000 0 0x1000>,
> +          <0 0xac65000 0 0x1000>,
> +          <0 0xac66000 0 0x1000>,
> +          <0 0xac67000 0 0x1000>,
> +          <0 0xac68000 0 0x1000>,
> +          <0 0xacaf000 0 0x4000>,
> +          <0 0xacb6000 0 0x4000>,
> +          <0 0xacc4000 0 0x4000>;
> +
> +        reg-names = "csid0",

So this will be the third property.



Best regards,
Krzysztof

Re: [PATCH 1/6] media: dt-bindings: media: camss: Add qcom,sc7180-camss binding

[george chan]

On Fri, Jun 21, 2024 at 6:02 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On 21/06/2024 11:40, George Chan via B4 Relay wrote:
> > From: George Chan <gchan9527@gmail.com>
> >
> > Add bindings for qcom,sc7180-camss in order to support the camera
> > subsystem for sm7125 as found in the Xiaomi Redmi 9 Pro cellphone.
> >
> > Signed-off-by: George Chan <gchan9527@gmail.com>
>
> Subject: just one media (first). No need to write media: media: ...
>
>
> A nit, subject: drop second/last, redundant "binding". The "dt-bindings"
> prefix is already stating that these are bindings.
> See also:
> https://elixir.bootlin.com/linux/v6.7-rc8/source/Documentation/devicetree/bindings/submitting-patches.rst#L18

I found the cause of this, see if I can fix it next round.

> > +title: Qualcomm CAMSS ISP
>
> What is CAMSS?
>

No idea from an outsider, i can suggest one like "title: Qualcomm
Camera SubSystem"

> > +
> > +maintainers:
> > +  - Robert Foss <robert.foss@linaro.org>
>
> For sure this is not true. Robert does not work in Linaro and I doubt he
> cares that much about camss.
>
Well, i might suggest to be like below if no objection

 maintainers:
-  - Robert Foss <robert.foss@linaro.org>
+  - Bryan O'Donoghue <bryan.odonoghue@linaro.org>
...
> > +
> > +description: |
>
> Do not need '|' unless you need to preserve formatting.

How about this? I have no idea what this is, just modify it blindly below.
-description: |
+description:

Then drop all "minItems"
...
> > +required:
> > +  - clock-names
> > +  - clocks
> > +  - compatible
>
> Keep the list ordered, the same as list properties.

Sure for this "required" list
...
>
> Missed alignment with previous <.
>
Sure
...
> > +        reg = <0 0xacb3000 0 0x1000>,
>
> reg is always the second property. See DTS coding style.
>
...
> > +        reg-names = "csid0",
>
> So this will be the third property.
>
>
>
> Best regards,
> Krzysztof
>
Best regards,
George

Re: [PATCH 1/6] media: dt-bindings: media: camss: Add qcom,sc7180-camss binding

[Rob Herring (Arm)]

On Fri, 21 Jun 2024 17:40:53 +0800, George Chan wrote:
> Add bindings for qcom,sc7180-camss in order to support the camera
> subsystem for sm7125 as found in the Xiaomi Redmi 9 Pro cellphone.
> 
> Signed-off-by: George Chan <gchan9527@gmail.com>
> ---
>  .../bindings/media/qcom,sc7180-camss.yaml          | 324 +++++++++++++++++++++
>  1 file changed, 324 insertions(+)
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/media/qcom,sc7180-camss.example.dtb: camss@acb3000: clocks: [[4294967295, 6], [4294967295, 12], [4294967295, 37], [4294967295, 44], [4294967295, 50], [4294967295, 22], [4294967295, 14], [4294967295, 23], [4294967295, 16], [4294967295, 24], [4294967295, 18], [4294967295, 25], [4294967295, 20], [4294967295, 10], [4294967295, 11], [4294967295, 77], [4294967295, 33], [4294967295, 34], [4294967295, 36], [4294967295, 40], [4294967295, 41], [4294967295, 43], [4294967295, 47], [4294967295, 49]] is too short
	from schema $id: http://devicetree.org/schemas/media/qcom,sc7180-camss.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/media/qcom,sc7180-camss.example.dtb: camss@acb3000: clock-names:2: 'cphy_rx_src' was expected
	from schema $id: http://devicetree.org/schemas/media/qcom,sc7180-camss.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/media/qcom,sc7180-camss.example.dtb: camss@acb3000: clock-names:3: 'csi0' was expected
	from schema $id: http://devicetree.org/schemas/media/qcom,sc7180-camss.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/media/qcom,sc7180-camss.example.dtb: camss@acb3000: clock-names:4: 'csi1' was expected
	from schema $id: http://devicetree.org/schemas/media/qcom,sc7180-camss.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/media/qcom,sc7180-camss.example.dtb: camss@acb3000: clock-names:5: 'csi2' was expected
	from schema $id: http://devicetree.org/schemas/media/qcom,sc7180-camss.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/media/qcom,sc7180-camss.example.dtb: camss@acb3000: clock-names:6: 'csiphy0' was expected
	from schema $id: http://devicetree.org/schemas/media/qcom,sc7180-camss.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/media/qcom,sc7180-camss.example.dtb: camss@acb3000: clock-names:7: 'csiphy0_timer' was expected
	from schema $id: http://devicetree.org/schemas/media/qcom,sc7180-camss.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/media/qcom,sc7180-camss.example.dtb: camss@acb3000: clock-names:8: 'csiphy1' was expected
	from schema $id: http://devicetree.org/schemas/media/qcom,sc7180-camss.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/media/qcom,sc7180-camss.example.dtb: camss@acb3000: clock-names:9: 'csiphy1_timer' was expected
	from schema $id: http://devicetree.org/schemas/media/qcom,sc7180-camss.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/media/qcom,sc7180-camss.example.dtb: camss@acb3000: clock-names:10: 'csiphy2' was expected
	from schema $id: http://devicetree.org/schemas/media/qcom,sc7180-camss.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/media/qcom,sc7180-camss.example.dtb: camss@acb3000: clock-names:11: 'csiphy2_timer' was expected
	from schema $id: http://devicetree.org/schemas/media/qcom,sc7180-camss.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/media/qcom,sc7180-camss.example.dtb: camss@acb3000: clock-names:12: 'csiphy3' was expected
	from schema $id: http://devicetree.org/schemas/media/qcom,sc7180-camss.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/media/qcom,sc7180-camss.example.dtb: camss@acb3000: clock-names:13: 'csiphy3_timer' was expected
	from schema $id: http://devicetree.org/schemas/media/qcom,sc7180-camss.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/media/qcom,sc7180-camss.example.dtb: camss@acb3000: clock-names:14: 'gcc_camera_ahb' was expected
	from schema $id: http://devicetree.org/schemas/media/qcom,sc7180-camss.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/media/qcom,sc7180-camss.example.dtb: camss@acb3000: clock-names:15: 'gcc_camera_axi' was expected
	from schema $id: http://devicetree.org/schemas/media/qcom,sc7180-camss.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/media/qcom,sc7180-camss.example.dtb: camss@acb3000: clock-names:16: 'soc_ahb' was expected
	from schema $id: http://devicetree.org/schemas/media/qcom,sc7180-camss.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/media/qcom,sc7180-camss.example.dtb: camss@acb3000: clock-names:17: 'vfe0_axi' was expected
	from schema $id: http://devicetree.org/schemas/media/qcom,sc7180-camss.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/media/qcom,sc7180-camss.example.dtb: camss@acb3000: clock-names:18: 'vfe0' was expected
	from schema $id: http://devicetree.org/schemas/media/qcom,sc7180-camss.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/media/qcom,sc7180-camss.example.dtb: camss@acb3000: clock-names:19: 'vfe0_cphy_rx' was expected
	from schema $id: http://devicetree.org/schemas/media/qcom,sc7180-camss.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/media/qcom,sc7180-camss.example.dtb: camss@acb3000: clock-names:20: 'vfe1_axi' was expected
	from schema $id: http://devicetree.org/schemas/media/qcom,sc7180-camss.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/media/qcom,sc7180-camss.example.dtb: camss@acb3000: clock-names:21: 'vfe1' was expected
	from schema $id: http://devicetree.org/schemas/media/qcom,sc7180-camss.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/media/qcom,sc7180-camss.example.dtb: camss@acb3000: clock-names:22: 'vfe1_cphy_rx' was expected
	from schema $id: http://devicetree.org/schemas/media/qcom,sc7180-camss.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/media/qcom,sc7180-camss.example.dtb: camss@acb3000: clock-names:23: 'vfe_lite' was expected
	from schema $id: http://devicetree.org/schemas/media/qcom,sc7180-camss.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/media/qcom,sc7180-camss.example.dtb: camss@acb3000: clock-names: ['camnoc_axi', 'cpas_ahb', 'csi0', 'csi1', 'csi2', 'csiphy0', 'csiphy0_timer', 'csiphy1', 'csiphy1_timer', 'csiphy2', 'csiphy2_timer', 'csiphy3', 'csiphy3_timer', 'gcc_camera_ahb', 'gcc_camera_axi', 'soc_ahb', 'vfe0_axi', 'vfe0', 'vfe0_cphy_rx', 'vfe1_axi', 'vfe1', 'vfe1_cphy_rx', 'vfe_lite', 'vfe_lite_cphy_rx'] is too short
	from schema $id: http://devicetree.org/schemas/media/qcom,sc7180-camss.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/media/qcom,sc7180-camss.example.dtb: camss@acb3000: iommus: [[4294967295, 2080, 0], [4294967295, 2112, 0], [4294967295, 2144, 0]] is too short
	from schema $id: http://devicetree.org/schemas/media/qcom,sc7180-camss.yaml#

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20240621-b4-sc7180-camss-v1-1-14937929f30e@gmail.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.

Re: [PATCH 1/6] media: dt-bindings: media: camss: Add qcom,sc7180-camss binding

[Bryan O'Donoghue]

On 21/06/2024 10:40, George Chan via B4 Relay wrote:
> +  power-domains:
> +    items:
> +      - description: IFE0 GDSC - Image Front End, Global Distributed Switch Controller.
> +      - description: IFE1 GDSC - Image Front End, Global Distributed Switch Controller.
> +      - description: Titan GDSC - Titan ISP Block, Global Distributed Switch Controller.

Please name these power-domains and require them in your yaml, 
remembering to add them into the DTS.

See

Commit: d89751c61279 ("media: qcom: camss: Add support for named 
power-domains")

---
bod

Re: [PATCH 1/6] media: dt-bindings: media: camss: Add qcom,sc7180-camss binding

[george chan]

On Fri, Jun 21, 2024 at 7:24 PM Bryan O'Donoghue
<bryan.odonoghue@linaro.org> wrote:
>
> On 21/06/2024 10:40, George Chan via B4 Relay wrote:
> > +  power-domains:
> > +    items:
> > +      - description: IFE0 GDSC - Image Front End, Global Distributed Switch Controller.
> > +      - description: IFE1 GDSC - Image Front End, Global Distributed Switch Controller.
> > +      - description: Titan GDSC - Titan ISP Block, Global Distributed Switch Controller.
>
> Please name these power-domains and require them in your yaml,
> remembering to add them into the DTS.

I can borrow the idea from sdm845-venus-v2

@@ -191,6 +190,7 @@ required:
   - interrupts
   - iommus
   - power-domains
+  - power-domain-names
   - reg
...
+        power-domain-names = "ife0",
+                             "ife1",
+                             "top";

I can confirm sc7180 will be the first one to have
"power-domain-names" property on camss node amongst SOCs.
>
> ---
> bod

Re: [PATCH 1/6] media: dt-bindings: media: camss: Add qcom,sc7180-camss binding

[Bryan O'Donoghue]

On 22/06/2024 16:31, george chan wrote:
> On Fri, Jun 21, 2024 at 7:24 PM Bryan O'Donoghue
> <bryan.odonoghue@linaro.org> wrote:
>>
>> On 21/06/2024 10:40, George Chan via B4 Relay wrote:
>>> +  power-domains:
>>> +    items:
>>> +      - description: IFE0 GDSC - Image Front End, Global Distributed Switch Controller.
>>> +      - description: IFE1 GDSC - Image Front End, Global Distributed Switch Controller.
>>> +      - description: Titan GDSC - Titan ISP Block, Global Distributed Switch Controller.
>>
>> Please name these power-domains and require them in your yaml,
>> remembering to add them into the DTS.
> 
> I can borrow the idea from sdm845-venus-v2
> 
> @@ -191,6 +190,7 @@ required:
>     - interrupts
>     - iommus
>     - power-domains
> +  - power-domain-names
>     - reg
> ...
> +        power-domain-names = "ife0",
> +                             "ife1",
> +                             "top";

Something like this

https://git.codelinaro.org/bryan.odonoghue/kernel/-/commit/7626cd2a2a509832c214e538827b91c5dbf1bf09

---
bod

Re: [PATCH 1/6] media: dt-bindings: media: camss: Add qcom,sc7180-camss binding

[george chan]

On Sun, Jun 23, 2024 at 7:10 PM Bryan O'Donoghue
<bryan.odonoghue@linaro.org> wrote:
>
> Something like this
>
> https://git.codelinaro.org/bryan.odonoghue/kernel/-/commit/7626cd2a2a509832c214e538827b91c5dbf1bf09
>
Nice, see if sc7180 is the first-runner-up patch hitting the mainline.

[PATCH 2/6] media: qcom: camss: Add CAMSS_SC7180 enum

[George Chan via B4 Relay]

From: George Chan <gchan9527@gmail.com>

Adds a CAMSS SoC identifier for the SC7180.

Signed-off-by: George Chan <gchan9527@gmail.com>
---
 drivers/media/platform/qcom/camss/camss.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/media/platform/qcom/camss/camss.h b/drivers/media/platform/qcom/camss/camss.h
index ac15fe23a702..5e750c481b74 100644
--- a/drivers/media/platform/qcom/camss/camss.h
+++ b/drivers/media/platform/qcom/camss/camss.h
@@ -76,6 +76,7 @@ enum camss_version {
 	CAMSS_8x96,
 	CAMSS_660,
 	CAMSS_845,
+	CAMSS_7180,
 	CAMSS_8250,
 	CAMSS_8280XP,
 };

-- 
2.34.1

Re: [PATCH 2/6] media: qcom: camss: Add CAMSS_SC7180 enum

[Krzysztof Kozlowski]

On 21/06/2024 11:40, George Chan via B4 Relay wrote:
> From: George Chan <gchan9527@gmail.com>
> 
> Adds a CAMSS SoC identifier for the SC7180.
> 
> Signed-off-by: George Chan <gchan9527@gmail.com>
> ---
>  drivers/media/platform/qcom/camss/camss.h | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/media/platform/qcom/camss/camss.h b/drivers/media/platform/qcom/camss/camss.h
> index ac15fe23a702..5e750c481b74 100644
> --- a/drivers/media/platform/qcom/camss/camss.h
> +++ b/drivers/media/platform/qcom/camss/camss.h
> @@ -76,6 +76,7 @@ enum camss_version {
>  	CAMSS_8x96,
>  	CAMSS_660,
>  	CAMSS_845,
> +	CAMSS_7180,

This patch on its own makes no sense. Squash it with patch adding 7180.

Best regards,
Krzysztof

Re: [PATCH 2/6] media: qcom: camss: Add CAMSS_SC7180 enum

[Bryan O'Donoghue]

On 21/06/2024 11:03, Krzysztof Kozlowski wrote:
> On 21/06/2024 11:40, George Chan via B4 Relay wrote:
>> From: George Chan <gchan9527@gmail.com>
>>
>> Adds a CAMSS SoC identifier for the SC7180.
>>
>> Signed-off-by: George Chan <gchan9527@gmail.com>
>> ---
>>   drivers/media/platform/qcom/camss/camss.h | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/media/platform/qcom/camss/camss.h b/drivers/media/platform/qcom/camss/camss.h
>> index ac15fe23a702..5e750c481b74 100644
>> --- a/drivers/media/platform/qcom/camss/camss.h
>> +++ b/drivers/media/platform/qcom/camss/camss.h
>> @@ -76,6 +76,7 @@ enum camss_version {
>>   	CAMSS_8x96,
>>   	CAMSS_660,
>>   	CAMSS_845,
>> +	CAMSS_7180,
> 
> This patch on its own makes no sense. Squash it with patch adding 7180.
> 
> Best regards,
> Krzysztof
> 

Agreed you can put it into the PHY init sequence in patch #3

---
bod

[PATCH 3/6] media: qcom: camss: csiphy-3ph: Add Gen2 v1.2.2 two-phase MIPI CSI-2 DPHY init

[George Chan via B4 Relay]

From: George Chan <gchan9527@gmail.com>

Add a PHY configuration sequence for the sc7180 which uses a Qualcomm
Gen 2 version 1.2.2 CSI-2 PHY.

The PHY can be configured as two phase or three phase in C-PHY or D-PHY
mode. This configuration supports two-phase D-PHY mode.

Signed-off-by: George Chan <gchan9527@gmail.com>
---
 .../platform/qcom/camss/camss-csiphy-3ph-1-0.c     | 120 +++++++++++++++++++++
 1 file changed, 120 insertions(+)

diff --git a/drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c b/drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c
index df7e93a5a4f6..181bb7f7c300 100644
--- a/drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c
+++ b/drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c
@@ -348,6 +348,121 @@ csiphy_reg_t lane_regs_sm8250[5][20] = {
 	},
 };
 
+/* GEN2 1.2.2 2PH */
+struct
+csiphy_reg_t lane_regs_sc7180[5][20] = {
+	{
+		{0x0030, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
+		{0x0900, 0x05, 0x00, CSIPHY_DEFAULT_PARAMS},
+		{0x0908, 0x10, 0x00, CSIPHY_DEFAULT_PARAMS},
+		{0x0904, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
+		{0x0904, 0x03, 0x00, CSIPHY_DEFAULT_PARAMS},
+		{0x0004, 0x0C, 0x00, CSIPHY_DEFAULT_PARAMS},
+		{0x002C, 0x01, 0x00, CSIPHY_DEFAULT_PARAMS},
+		{0x0034, 0x07, 0x00, CSIPHY_DEFAULT_PARAMS},
+		{0x0010, 0x02, 0x00, CSIPHY_DEFAULT_PARAMS},
+		{0x001C, 0x08, 0x00, CSIPHY_DEFAULT_PARAMS},
+		{0x003C, 0xB8, 0x00, CSIPHY_DEFAULT_PARAMS},
+		{0x0008, 0x10, 0x00, CSIPHY_SETTLE_CNT_LOWER_BYTE},
+		{0x0000, 0x8D, 0x00, CSIPHY_DEFAULT_PARAMS},
+		{0x000c, 0x00, 0x00, CSIPHY_DNP_PARAMS},
+		{0x0038, 0xFE, 0x00, CSIPHY_DEFAULT_PARAMS},
+		{0x0014, 0x60, 0x00, CSIPHY_DEFAULT_PARAMS},
+		{0x0028, 0x00, 0x00, CSIPHY_DNP_PARAMS},
+		{0x0024, 0x00, 0x00, CSIPHY_DNP_PARAMS},
+		{0x0800, 0x02, 0x00, CSIPHY_DEFAULT_PARAMS},
+		{0x0884, 0x01, 0x00, CSIPHY_DEFAULT_PARAMS},
+	},
+	{
+		{0x0730, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
+		{0x0C80, 0x05, 0x00, CSIPHY_DEFAULT_PARAMS},
+		{0x0C88, 0x10, 0x00, CSIPHY_DEFAULT_PARAMS},
+		{0x0C84, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
+		{0x0C84, 0x03, 0x00, CSIPHY_DEFAULT_PARAMS},
+		{0x0704, 0x0C, 0x00, CSIPHY_DEFAULT_PARAMS},
+		{0x072C, 0x01, 0x00, CSIPHY_DEFAULT_PARAMS},
+		{0x0734, 0x07, 0x00, CSIPHY_DEFAULT_PARAMS},
+		{0x0710, 0x02, 0x00, CSIPHY_DEFAULT_PARAMS},
+		{0x071C, 0x08, 0x00, CSIPHY_DEFAULT_PARAMS},
+		{0x073C, 0xB8, 0x00, CSIPHY_DEFAULT_PARAMS},
+		{0x0708, 0x10, 0x00, CSIPHY_SETTLE_CNT_LOWER_BYTE},
+		{0x0700, 0x80, 0x00, CSIPHY_DEFAULT_PARAMS},
+		{0x070c, 0xA5, 0x00, CSIPHY_DEFAULT_PARAMS},
+		{0x0738, 0x1F, 0x00, CSIPHY_DEFAULT_PARAMS},
+		{0x0714, 0x60, 0x00, CSIPHY_DEFAULT_PARAMS},
+		{0x0728, 0x04, 0x00, CSIPHY_DEFAULT_PARAMS},
+		{0x0724, 0x00, 0x00, CSIPHY_DNP_PARAMS},
+		{0x0800, 0x02, 0x00, CSIPHY_DEFAULT_PARAMS},
+		{0x0884, 0x01, 0x00, CSIPHY_DEFAULT_PARAMS},
+	},
+	{
+		{0x0230, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
+		{0x0A00, 0x05, 0x00, CSIPHY_DEFAULT_PARAMS},
+		{0x0A08, 0x10, 0x00, CSIPHY_DEFAULT_PARAMS},
+		{0x0A04, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
+		{0x0A04, 0x03, 0x00, CSIPHY_DEFAULT_PARAMS},
+		{0x0204, 0x0C, 0x00, CSIPHY_DEFAULT_PARAMS},
+		{0x022C, 0x01, 0x00, CSIPHY_DEFAULT_PARAMS},
+		{0x0234, 0x07, 0x00, CSIPHY_DEFAULT_PARAMS},
+		{0x0210, 0x02, 0x00, CSIPHY_DEFAULT_PARAMS},
+		{0x021C, 0x08, 0x00, CSIPHY_DEFAULT_PARAMS},
+		{0x023C, 0xB8, 0x00, CSIPHY_DEFAULT_PARAMS},
+		{0x0208, 0x10, 0x00, CSIPHY_SETTLE_CNT_LOWER_BYTE},
+		{0x0200, 0x8D, 0x00, CSIPHY_DEFAULT_PARAMS},
+		{0x020c, 0x00, 0x00, CSIPHY_DNP_PARAMS},
+		{0x0238, 0xFE, 0x00, CSIPHY_DEFAULT_PARAMS},
+		{0x0214, 0x60, 0x00, CSIPHY_DEFAULT_PARAMS},
+		{0x0228, 0x00, 0x00, CSIPHY_DNP_PARAMS},
+		{0x0224, 0x00, 0x00, CSIPHY_DNP_PARAMS},
+		{0x0800, 0x02, 0x00, CSIPHY_DEFAULT_PARAMS},
+		{0x0884, 0x01, 0x00, CSIPHY_DEFAULT_PARAMS},
+	},
+	{
+		{0x0430, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
+		{0x0B00, 0x05, 0x00, CSIPHY_DEFAULT_PARAMS},
+		{0x0B08, 0x10, 0x00, CSIPHY_DEFAULT_PARAMS},
+		{0x0B04, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
+		{0x0B04, 0x03, 0x00, CSIPHY_DEFAULT_PARAMS},
+		{0x0404, 0x0C, 0x00, CSIPHY_DEFAULT_PARAMS},
+		{0x042C, 0x01, 0x00, CSIPHY_DEFAULT_PARAMS},
+		{0x0434, 0x07, 0x00, CSIPHY_DEFAULT_PARAMS},
+		{0x0410, 0x02, 0x00, CSIPHY_DEFAULT_PARAMS},
+		{0x041C, 0x08, 0x00, CSIPHY_DEFAULT_PARAMS},
+		{0x043C, 0xB8, 0x00, CSIPHY_DEFAULT_PARAMS},
+		{0x0408, 0x10, 0x00, CSIPHY_SETTLE_CNT_LOWER_BYTE},
+		{0x0400, 0x8D, 0x00, CSIPHY_DEFAULT_PARAMS},
+		{0x040c, 0x00, 0x00, CSIPHY_DNP_PARAMS},
+		{0x0438, 0xFE, 0x00, CSIPHY_DEFAULT_PARAMS},
+		{0x0414, 0x60, 0x00, CSIPHY_DEFAULT_PARAMS},
+		{0x0428, 0x00, 0x00, CSIPHY_DNP_PARAMS},
+		{0x0424, 0x00, 0x00, CSIPHY_DNP_PARAMS},
+		{0x0800, 0x02, 0x00, CSIPHY_DEFAULT_PARAMS},
+		{0x0884, 0x01, 0x00, CSIPHY_DEFAULT_PARAMS},
+	},
+	{
+		{0x0630, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
+		{0x0C00, 0x05, 0x00, CSIPHY_DEFAULT_PARAMS},
+		{0x0C08, 0x10, 0x00, CSIPHY_DEFAULT_PARAMS},
+		{0x0C04, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
+		{0x0C04, 0x03, 0x00, CSIPHY_DEFAULT_PARAMS},
+		{0x0604, 0x0C, 0x00, CSIPHY_DEFAULT_PARAMS},
+		{0x062C, 0x01, 0x00, CSIPHY_DEFAULT_PARAMS},
+		{0x0634, 0x07, 0x00, CSIPHY_DEFAULT_PARAMS},
+		{0x0610, 0x02, 0x00, CSIPHY_DEFAULT_PARAMS},
+		{0x061C, 0x08, 0x00, CSIPHY_DEFAULT_PARAMS},
+		{0x063C, 0xB8, 0x00, CSIPHY_DEFAULT_PARAMS},
+		{0x0608, 0x10, 0x00, CSIPHY_SETTLE_CNT_LOWER_BYTE},
+		{0x0600, 0x8D, 0x00, CSIPHY_DEFAULT_PARAMS},
+		{0x060c, 0x00, 0x00, CSIPHY_DNP_PARAMS},
+		{0x0638, 0xFE, 0x00, CSIPHY_DEFAULT_PARAMS},
+		{0x0614, 0x60, 0x00, CSIPHY_DEFAULT_PARAMS},
+		{0x0628, 0x00, 0x00, CSIPHY_DNP_PARAMS},
+		{0x0624, 0x00, 0x00, CSIPHY_DNP_PARAMS},
+		{0x0800, 0x02, 0x00, CSIPHY_DEFAULT_PARAMS},
+		{0x0884, 0x01, 0x00, CSIPHY_DEFAULT_PARAMS},
+	},
+};
+
 static void csiphy_hw_version_read(struct csiphy_device *csiphy,
 				   struct device *dev)
 {
@@ -509,6 +624,10 @@ static void csiphy_gen2_config_lanes(struct csiphy_device *csiphy,
 		r = &lane_regs_sdm845[0][0];
 		array_size = ARRAY_SIZE(lane_regs_sdm845[0]);
 		break;
+	case CAMSS_7180:
+		r = &lane_regs_sc7180[0][0];
+		array_size = ARRAY_SIZE(lane_regs_sc7180[0]);
+		break;
 	case CAMSS_8250:
 		r = &lane_regs_sm8250[0][0];
 		array_size = ARRAY_SIZE(lane_regs_sm8250[0]);
@@ -558,6 +677,7 @@ static bool csiphy_is_gen2(u32 version)
 
 	switch (version) {
 	case CAMSS_845:
+	case CAMSS_7180:
 	case CAMSS_8250:
 	case CAMSS_8280XP:
 		ret = true;

-- 
2.34.1

Re: [PATCH 3/6] media: qcom: camss: csiphy-3ph: Add Gen2 v1.2.2 two-phase MIPI CSI-2 DPHY init

[Bryan O'Donoghue]

On 21/06/2024 10:40, George Chan via B4 Relay wrote:
> From: George Chan <gchan9527@gmail.com>
> 
> Add a PHY configuration sequence for the sc7180 which uses a Qualcomm
> Gen 2 version 1.2.2 CSI-2 PHY.
> 
> The PHY can be configured as two phase or three phase in C-PHY or D-PHY
> mode. This configuration supports two-phase D-PHY mode.
> 
> Signed-off-by: George Chan <gchan9527@gmail.com>
> ---
>   .../platform/qcom/camss/camss-csiphy-3ph-1-0.c     | 120 +++++++++++++++++++++
>   1 file changed, 120 insertions(+)
> 
> diff --git a/drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c b/drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c
> index df7e93a5a4f6..181bb7f7c300 100644
> --- a/drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c
> +++ b/drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c
> @@ -348,6 +348,121 @@ csiphy_reg_t lane_regs_sm8250[5][20] = {
>   	},
>   };
>   
> +/* GEN2 1.2.2 2PH */

This is the init sequence for 1_2_1 not 1_2_2

https://review.lineageos.org/c/LineageOS/android_kernel_xiaomi_sm8250/+/311931/10/techpack/camera/drivers/cam_sensor_module/cam_csiphy/include/cam_csiphy_1_2_1_hwreg.h

https://review.lineageos.org/c/LineageOS/android_kernel_xiaomi_sm8250/+/311931/10/techpack/camera/drivers/cam_sensor_module/cam_csiphy/include/cam_csiphy_1_2_2_hwreg.h

Please fix.

---
bod

Re: [PATCH 3/6] media: qcom: camss: csiphy-3ph: Add Gen2 v1.2.2 two-phase MIPI CSI-2 DPHY init

[Konrad Dybcio]

On 21.06.2024 1:25 PM, Bryan O'Donoghue wrote:
> On 21/06/2024 10:40, George Chan via B4 Relay wrote:
>> From: George Chan <gchan9527@gmail.com>
>>
>> Add a PHY configuration sequence for the sc7180 which uses a Qualcomm
>> Gen 2 version 1.2.2 CSI-2 PHY.
>>
>> The PHY can be configured as two phase or three phase in C-PHY or D-PHY
>> mode. This configuration supports two-phase D-PHY mode.
>>
>> Signed-off-by: George Chan <gchan9527@gmail.com>
>> ---
>>   .../platform/qcom/camss/camss-csiphy-3ph-1-0.c     | 120 +++++++++++++++++++++
>>   1 file changed, 120 insertions(+)
>>
>> diff --git a/drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c b/drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c
>> index df7e93a5a4f6..181bb7f7c300 100644
>> --- a/drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c
>> +++ b/drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c
>> @@ -348,6 +348,121 @@ csiphy_reg_t lane_regs_sm8250[5][20] = {
>>       },
>>   };
>>   +/* GEN2 1.2.2 2PH */
> 
> This is the init sequence for 1_2_1 not 1_2_2
> 
> https://review.lineageos.org/c/LineageOS/android_kernel_xiaomi_sm8250/+/311931/10/techpack/camera/drivers/cam_sensor_module/cam_csiphy/include/cam_csiphy_1_2_1_hwreg.h
> 
> https://review.lineageos.org/c/LineageOS/android_kernel_xiaomi_sm8250/+/311931/10/techpack/camera/drivers/cam_sensor_module/cam_csiphy/include/cam_csiphy_1_2_2_hwreg.h

FWIW 1.2.2 seems to be the desired one: [1]

Konrad

[1] https://git.codelinaro.org/clo/la/kernel/msm-4.14/-/blob/UC.UM.1.0.r1-02500-sa8155.0/arch/arm64/boot/dts/qcom/atoll-camera.dtsi#L22

Re: [PATCH 3/6] media: qcom: camss: csiphy-3ph: Add Gen2 v1.2.2 two-phase MIPI CSI-2 DPHY init

[george chan]

resend with plain text

On Sat, Jun 22, 2024 at 7:20 PM Konrad Dybcio <konrad.dybcio@linaro.org> wrote:
>
> On 21.06.2024 1:25 PM, Bryan O'Donoghue wrote:
> > On 21/06/2024 10:40, George Chan via B4 Relay wrote:
> >> From: George Chan <gchan9527@gmail.com>
> >>
> >> Add a PHY configuration sequence for the sc7180 which uses a Qualcomm
> >> Gen 2 version 1.2.2 CSI-2 PHY.
> >>
> >> The PHY can be configured as two phase or three phase in C-PHY or D-PHY
> >> mode. This configuration supports two-phase D-PHY mode.
> >>
> >> Signed-off-by: George Chan <gchan9527@gmail.com>
> >> ---
> >>   .../platform/qcom/camss/camss-csiphy-3ph-1-0.c     | 120 +++++++++++++++++++++
> >>   1 file changed, 120 insertions(+)
> >>
> >> diff --git a/drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c b/drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c
> >> index df7e93a5a4f6..181bb7f7c300 100644
> >> --- a/drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c
> >> +++ b/drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c
> >> @@ -348,6 +348,121 @@ csiphy_reg_t lane_regs_sm8250[5][20] = {
> >>       },
> >>   };
> >>   +/* GEN2 1.2.2 2PH */
> >
> > This is the init sequence for 1_2_1 not 1_2_2

Yes, undesirable copy-n-paste result.

> >
> > https://review.lineageos.org/c/LineageOS/android_kernel_xiaomi_sm8250/+/311931/10/techpack/camera/drivers/cam_sensor_module/cam_csiphy/include/cam_csiphy_1_2_1_hwreg.h
> >
> > https://review.lineageos.org/c/LineageOS/android_kernel_xiaomi_sm8250/+/311931/10/techpack/camera/drivers/cam_sensor_module/cam_csiphy/include/cam_csiphy_1_2_2_hwreg.h
>
> FWIW 1.2.2 seems to be the desired one: [1]
>
> Konrad
>
> [1] https://git.codelinaro.org/clo/la/kernel/msm-4.14/-/blob/UC.UM.1.0.r1-02500-sa8155.0/arch/arm64/boot/dts/qcom/atoll-camera.dtsi#L22

Here is the log from sm7125 joyeuse phone, not sure if it helps or not.
[  204.034767] qcom-camss acb3000.camss: CSIPHY 3PH HW Version = 0x01000000

I carefully looked into this csiphy_2ph_v1_2_2_reg of various trees,
and concluded below version:
(1)atoll, sdm845[1]
(2)surya[2], sa8155, factory-trogdor-13443.B-chromeos-5.4[3]

I was tempted to use (1)atoll one but it looked like (2) is newer. Is
it worthy to create CAMSS_7125 specially for SM7125. Please give me
some advice about it.

Regards,
George

[1] https://github.com/LineageOS/android_kernel_xiaomi_sm6250/blob/lineage-21/drivers/media/platform/msm/camera/cam_sensor_module/cam_csiphy/include/cam_csiphy_1_2_2_hwreg.h
[2] https://github.com/LineageOS/android_kernel_xiaomi_surya/blob/lineage-21/drivers/media/platform/msm/camera/cam_sensor_module/cam_csiphy/include/cam_csiphy_1_2_2_hwreg.h
[3] https://chromium.googlesource.com/chromiumos/third_party/kernel/+/refs/heads/factory-trogdor-13443.B-chromeos-5.4/drivers/media/platform/camx/cam_sensor_module/cam_csiphy/include/cam_csiphy_1_2_2_hwreg.h

Re: [PATCH 3/6] media: qcom: camss: csiphy-3ph: Add Gen2 v1.2.2 two-phase MIPI CSI-2 DPHY init

[Bryan O'Donoghue]

On 22/06/2024 14:43, george chan wrote:
>     FWIW 1.2.2 seems to be the desired one: [1]
> 
>     Konrad
> 
>     [1]
>     https://git.codelinaro.org/clo/la/kernel/msm-4.14/-/blob/UC.UM.1.0.r1-02500-sa8155.0/arch/arm64/boot/dts/qcom/atoll-camera.dtsi#L22 <https://git.codelinaro.org/clo/la/kernel/msm-4.14/-/blob/UC.UM.1.0.r1-02500-sa8155.0/arch/arm64/boot/dts/qcom/atoll-camera.dtsi#L22>
> 
> 
> Here is the log from sm7125 joyeuse phone, not sure if it helps or not.
> [  204.034767] qcom-camss acb3000.camss: CSIPHY 3PH HW Version = 0x01000000
> 
> I carefully looked into this csiphy_2ph_v1_2_2_reg of various trees, and 
> concluded below version:
> (1)atoll, sdm845[1]
> (2)surya[2], sa8155, factory-trogdor-13443.B-chromeos-5.4[3]
> 
> I was tempted to use (1)atoll one but it looked like (2) is newer. Is it 
> worthy to create CAMSS_7125 specially for SM7125. Please give me some 
> advice about it.

So, which have you tested with as verified and working ?

My assumption here is that this series has been tested and is proven to 
work.

Version 1.2.1 and version 1.2.2 don't indicate different versions of the 
init sequence but different versions of the PHY.

For example - the CSI decoder is "just" digital logic, the "source code" 
for the at logic can be "recompiled" for a different process node.

But the PHYs translate analogue signals into the digital domain and 
therefore will vary with different process nodes - 3nm v 4nm v 28nm.

So it is virtually impossible - or highly improbable that init sequence 
1.2.1 and init sequence 1.2.2 will work on the same piece of hardware.

So its not a question of choosing the newer version - only one version 
will work - the version that is specifically tuned to the PHY for the 
given process node and RTL version.

Err, so TL;DR you _have_ tested this and gotten data delivered to you in 
user-space - right ?

---
bod

Re: [PATCH 3/6] media: qcom: camss: csiphy-3ph: Add Gen2 v1.2.2 two-phase MIPI CSI-2 DPHY init

[george chan]

On Sun, Jun 23, 2024 at 7:17 PM Bryan O'Donoghue
<bryan.odonoghue@linaro.org> wrote:
>
> On 22/06/2024 14:43, george chan wrote:
> >     FWIW 1.2.2 seems to be the desired one: [1]
> >
> >     Konrad
> >
> >     [1]
> >     https://git.codelinaro.org/clo/la/kernel/msm-4.14/-/blob/UC.UM.1.0.r1-02500-sa8155.0/arch/arm64/boot/dts/qcom/atoll-camera.dtsi#L22 <https://git.codelinaro.org/clo/la/kernel/msm-4.14/-/blob/UC.UM.1.0.r1-02500-sa8155.0/arch/arm64/boot/dts/qcom/atoll-camera.dtsi#L22>
> >
> >
> > Here is the log from sm7125 joyeuse phone, not sure if it helps or not.
> > [  204.034767] qcom-camss acb3000.camss: CSIPHY 3PH HW Version = 0x01000000
> >
> > I carefully looked into this csiphy_2ph_v1_2_2_reg of various trees, and
> > concluded below version:
> > (1)atoll, sdm845[1]
> > (2)surya[2], sa8155, factory-trogdor-13443.B-chromeos-5.4[3]
> >
> > I was tempted to use (1)atoll one but it looked like (2) is newer. Is it
> > worthy to create CAMSS_7125 specially for SM7125. Please give me some
> > advice about it.
>
> So, which have you tested with as verified and working ?
>

Tests show me, under my sm7125 test phone test case, no matter v1.2.1,
or atoll's v1.2.2 even surya and trogdor tree v1.2.2 are all
surprisingly works. Thanks for telling me or I won't be able to spot
this out. These results are quite funny :-)

> My assumption here is that this series has been tested and is proven to
> work.
>
> Version 1.2.1 and version 1.2.2 don't indicate different versions of the
> init sequence but different versions of the PHY.
>
> For example - the CSI decoder is "just" digital logic, the "source code"
> for the at logic can be "recompiled" for a different process node.
>
> But the PHYs translate analogue signals into the digital domain and
> therefore will vary with different process nodes - 3nm v 4nm v 28nm.
>
> So it is virtually impossible - or highly improbable that init sequence
> 1.2.1 and init sequence 1.2.2 will work on the same piece of hardware.
>

Yes, agreed. I also have the feeling of sc7180(10nm) vs sm7125(8nm) fab.

> So its not a question of choosing the newer version - only one version
> will work - the version that is specifically tuned to the PHY for the
> given process node and RTL version.
>
> Err, so TL;DR you _have_ tested this and gotten data delivered to you in
> user-space - right ?

User-space tool can't tell so I made some guesses.

A side note, atoll's reg sequence is a bit shorter and, unlike
trogdor, it does not write upto 0x9xx reg. That seemed to me, using
atoll's init sequence for sc7180 is nothing wrong initially. Later on
today, I am wondering if writing those extra regs(> 0x9xx) is to
stabilize the phy, so applying trogdor init sequence to atoll might be
more desirable.

As the trogdor tree with kernel 5.4 so this is a side proof that
trogdor init sequence is newer than atoll's. So I will use the
trodgor's init sequence for CAMSS_7180.

So here is the plan. Let's treat CAMSS_7180 to both sm7125 and sc7180
SOCs, and apply the idea to all others sharing v1.2.2 phy atm.

If somebody knowledgeable could confirm some real difference, then I
can prepare another CAMSS_7125 patch-set afterward.

>
> ---
> bod

Re: [PATCH 3/6] media: qcom: camss: csiphy-3ph: Add Gen2 v1.2.2 two-phase MIPI CSI-2 DPHY init

[Bryan O'Donoghue]

On 23/06/2024 22:37, george chan wrote:
> User-space tool can't tell so I made some guesses.

So how are you testing ?

Libcamera on your target rootfs ?

# example 1
cam -c 1 --capture=10 --file

Should deliver up ten frames to userpsace.

For me working means either

1. Sensor data delivered to user-space or
2. Minimum test pattern generator (TPG) data delivered to userspace

Here's an example of the TPG on the rb3/sdm845

# example 2
media-ctl --reset
yavta --no-query -w '0x009f0903 9' /dev/v4l-subdev4
yavta --list /dev/v4l-subdev4
media-ctl -d /dev/media0 -V '"msm_csid0":0[fmt:SGRBG10_1X10/3280x2464]'
media-ctl -d /dev/media0 -V '"msm_vfe0_rdi0":0[fmt:SGRBG10_1X10/3280x2464]'
media-ctl -l '"msm_csid0":1->"msm_vfe0_rdi0":0[1]'
media-ctl -d /dev/media0 -p
yavta -B capture-mplane --capture=5 -n 5 -I -f SGRBG10P -s 3280x2464 
--file=TPG-SGRBG10-3280x2464-000-#.bin /dev/video2

If you can't use libcamera to do the v4l pipeline setup you can do so 
yourself manually again here's rb3 setting up the pipeline and reading 
from ov8856.

# example 3
media-ctl --reset
media-ctl -d /dev/media0 -V '"ov8856 
'16-0010'":0[fmt:SGRBG10_1X10/3280x2464 field:none]'
media-ctl -d /dev/media0 -V '"msm_csiphy0":0[fmt:SGRBG10_1X10/3280x2464]'
media-ctl -d /dev/media0 -V '"msm_csid0":0[fmt:SGRBG10_1X10/3280x2464]'
media-ctl -d /dev/media0 -V '"msm_vfe0_rdi0":0[fmt:SGRBG10_1X10/3280x2464]'
media-ctl -l 
'"msm_csiphy0":1->"msm_csid0":0[1],"msm_csid0":1->"msm_vfe0_rdi0":0[1]'
media-ctl -d /dev/media0 -p
yavta -B capture-mplane --capture=5 -n 5 -I -f SGRBG10P -s 3280x2464 
--file=ov8856-SGRBG10-3280x2464-000-#.bin /dev/video0

Maybe its an obvious question but, are you currently able to read from 
either

1. The sensor - thus proving the PHY init sequence you have or
2. The TPG ?

as illustrated with one of the examples [1, 2, 3] above ?

---
bod

Re: [PATCH 3/6] media: qcom: camss: csiphy-3ph: Add Gen2 v1.2.2 two-phase MIPI CSI-2 DPHY init

[george chan]

On Mon, Jun 24, 2024 at 6:13 AM Bryan O'Donoghue
<bryan.odonoghue@linaro.org> wrote:
>
> On 23/06/2024 22:37, george chan wrote:
> > User-space tool can't tell so I made some guesses.

Sorry for misleading, actually i mean user-space too can't tell the
difference. As all 3 kinds of init sequences are working, I can't get
a strong conclusion of "correct" init sequence between atoll's and
trodger's.

> So how are you testing ?
>
> Libcamera on your target rootfs ?

Yes, a similar test was carried out early days with the "wrong" v1.2.1
init sequence, on pmOS qcam installed into xiaomi redmi note 9 pro
(sm7125). It showed nice output. And I was excited so I took a video
recording too:
https://www.youtube.com/watch?v=U_do11pSf1s

After your indication, I replaced the v1.2.1 init sequence with
atoll's and trodger's and carried some simple test with below cmd and
both are outputting files.

media-ctl --reset
media-ctl -V '"msm_csid0":0[fmt:SRGGB10/2592x1944 field:none]'
media-ctl -V '"msm_vfe0_rdi0":0[fmt:SRGGB10/2592x1944 field:none]'
media-ctl -l '"msm_csid0":1->"msm_vfe0_rdi0":0[1]'
v4l2-ctl -d /dev/v4l-subdev4 -c test_pattern=0
v4l2-ctl -d /dev/v4l-subdev5 -c test_pattern=0
v4l2-ctl -d /dev/v4l-subdev6 -c test_pattern=0
v4l2-ctl -d /dev/v4l-subdev19 -c test_pattern=$1

media-ctl -V '"s5k5e9 13-002d":0[fmt:SRGGB10/2592x1944 field:none]'
media-ctl -V '"msm_csiphy2":0[fmt:SRGGB10/2592x1944 field:none]'
media-ctl -l '"msm_csiphy2":1->"msm_csid0":0[1]'
yavta -B capture-mplane --capture=3 -n 3 -f SRGGB10P -s 2592x1944 /dev/video0 -F

As you can see the cmos named s5k5e9. and this time simply do yavta
dump, no pmOS qcam test.

Since this test is carried out in sm7125 SOC, in theory, it is better
to test with sc7180 (less likely form-factor available in the market)
so I will send out v2 with trogdor init sequence for other dev have
sc7180 board to have a test.

Stay tuned.

Re: [PATCH 3/6] media: qcom: camss: csiphy-3ph: Add Gen2 v1.2.2 two-phase MIPI CSI-2 DPHY init

[Bryan O'Donoghue]

On 24/06/2024 00:16, george chan wrote:
>> So how are you testing ?
>>
>> Libcamera on your target rootfs ?
> Yes, a similar test was carried out early days with the "wrong" v1.2.1
> init sequence, on pmOS qcam installed into xiaomi redmi note 9 pro
> (sm7125). It showed nice output. And I was excited so I took a video
> recording too:
> https://www.youtube.com/watch?v=U_do11pSf1s

Grand so.

---
bod

[PATCH 4/6] media: qcom: camss: Add sc7180 support

[George Chan via B4 Relay]

From: George Chan <gchan9527@gmail.com>

Add in functional logic throughout the code to support the sc7180.

Signed-off-by: George Chan <gchan9527@gmail.com>
---
 drivers/media/platform/qcom/camss/camss-csiphy.c | 1 +
 drivers/media/platform/qcom/camss/camss-vfe.c    | 3 +++
 drivers/media/platform/qcom/camss/camss-video.c  | 1 +
 3 files changed, 5 insertions(+)

diff --git a/drivers/media/platform/qcom/camss/camss-csiphy.c b/drivers/media/platform/qcom/camss/camss-csiphy.c
index 45b3a8e5dea4..1fb08b5ee01b 100644
--- a/drivers/media/platform/qcom/camss/camss-csiphy.c
+++ b/drivers/media/platform/qcom/camss/camss-csiphy.c
@@ -577,6 +577,7 @@ int msm_csiphy_subdev_init(struct camss *camss,
 		csiphy->nformats = ARRAY_SIZE(csiphy_formats_8x96);
 		break;
 	case CAMSS_845:
+	case CAMSS_7180:
 	case CAMSS_8250:
 	case CAMSS_8280XP:
 		csiphy->formats = csiphy_formats_sdm845;
diff --git a/drivers/media/platform/qcom/camss/camss-vfe.c b/drivers/media/platform/qcom/camss/camss-vfe.c
index d875237cf244..05fa1adc1661 100644
--- a/drivers/media/platform/qcom/camss/camss-vfe.c
+++ b/drivers/media/platform/qcom/camss/camss-vfe.c
@@ -224,6 +224,7 @@ static u32 vfe_src_pad_code(struct vfe_line *line, u32 sink_code,
 	case CAMSS_8x96:
 	case CAMSS_660:
 	case CAMSS_845:
+	case CAMSS_7180:
 	case CAMSS_8250:
 	case CAMSS_8280XP:
 		switch (sink_code) {
@@ -1518,6 +1519,7 @@ int msm_vfe_subdev_init(struct camss *camss, struct vfe_device *vfe,
 			}
 			break;
 		case CAMSS_845:
+		case CAMSS_7180:
 		case CAMSS_8250:
 		case CAMSS_8280XP:
 			l->formats = formats_rdi_845;
@@ -1603,6 +1605,7 @@ static int vfe_bpl_align(struct vfe_device *vfe)
 
 	switch (vfe->camss->res->version) {
 	case CAMSS_845:
+	case CAMSS_7180:
 	case CAMSS_8250:
 	case CAMSS_8280XP:
 		ret = 16;
diff --git a/drivers/media/platform/qcom/camss/camss-video.c b/drivers/media/platform/qcom/camss/camss-video.c
index 54cd82f74115..5b7f3fba938c 100644
--- a/drivers/media/platform/qcom/camss/camss-video.c
+++ b/drivers/media/platform/qcom/camss/camss-video.c
@@ -1027,6 +1027,7 @@ int msm_video_register(struct camss_video *video, struct v4l2_device *v4l2_dev,
 		}
 		break;
 	case CAMSS_845:
+	case CAMSS_7180:
 	case CAMSS_8250:
 	case CAMSS_8280XP:
 		video->formats = formats_rdi_845;

-- 
2.34.1

[PATCH 5/6] media: qcom: camss: Add sc7180 resources

[George Chan via B4 Relay]

From: George Chan <gchan9527@gmail.com>

This commit describes the hardware layout for the sc7180 for the
following hardware blocks:

- 2 x VFE
- 1 x VFE Lite
- 2 x CSID
- 1 x CSID Lite
- 4 x CSI PHY

Signed-off-by: George Chan <gchan9527@gmail.com>
---
 drivers/media/platform/qcom/camss/camss.c | 218 +++++++++++++++++++++++++++++-
 1 file changed, 217 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c
index 1923615f0eea..d50f98565531 100644
--- a/drivers/media/platform/qcom/camss/camss.c
+++ b/drivers/media/platform/qcom/camss/camss.c
@@ -713,6 +713,210 @@ static const struct camss_subdev_resources vfe_res_845[] = {
 	}
 };
 
+static const struct camss_subdev_resources csiphy_res_7180[] = {
+	/* CSIPHY0 */
+	{
+		.regulators = {},
+		.clock = {
+			"csiphy0",
+			 "csiphy0_timer"
+		},
+		.clock_rate = {
+			{ 150000000, 270000000, 360000000 },
+			{ 300000000 },
+		},
+		.reg = { "csiphy0" },
+		.interrupt = { "csiphy0" },
+		.ops = &csiphy_ops_3ph_1_0
+	},
+	/* CSIPHY1 */
+	{
+		.regulators = {},
+		.clock = {
+			"csiphy1",
+			"csiphy1_timer"
+		},
+		.clock_rate = {
+			{ 150000000, 270000000, 360000000 },
+			{ 300000000 },
+		},
+		.reg = { "csiphy1" },
+		.interrupt = { "csiphy1" },
+		.ops = &csiphy_ops_3ph_1_0
+	},
+	/* CSIPHY2 */
+	{
+		.regulators = {},
+		.clock = {
+			"csiphy2",
+			"csiphy2_timer"
+		},
+		.clock_rate = {
+			{ 150000000, 270000000, 360000000 },
+			{ 300000000 },
+		},
+		.reg = { "csiphy2" },
+		.interrupt = { "csiphy2" },
+		.ops = &csiphy_ops_3ph_1_0
+	},
+	/* CSIPHY3 */
+	{
+		.regulators = {},
+		.clock = {
+			"csiphy3",
+			"csiphy3_timer"
+		},
+		.clock_rate = {
+			{ 150000000, 270000000, 360000000 },
+			{ 300000000 },
+		},
+		.reg = { "csiphy3" },
+		.interrupt = { "csiphy3" },
+		.ops = &csiphy_ops_3ph_1_0
+	}
+};
+
+static const struct camss_subdev_resources csid_res_7180[] = {
+	/* CSID0 */
+	{
+		.regulators = { "vdda-phy", "vdda-pll" },
+		.clock = {
+			"soc_ahb",
+			"vfe0",
+			"vfe0_cphy_rx",
+			"csi0"
+		},
+		.clock_rate = {
+			{ 0 },
+			{ 240000000, 360000000, 432000000, 600000000 },
+			{ 150000000, 270000000, 360000000 },
+			{ 150000000, 270000000, 360000000, 480000000 },
+		},
+		.reg = { "csid0" },
+		.interrupt = { "csid0" },
+		.ops = &csid_ops_gen2
+	},
+
+	/* CSID1 */
+	{
+		.regulators = { "vdda-phy", "vdda-pll" },
+		.clock = {
+			"soc_ahb",
+			"vfe1",
+			"vfe1_cphy_rx",
+			"csi1",
+		},
+		.clock_rate = {
+			{ 0 },
+			{ 240000000, 360000000, 432000000, 600000000 },
+			{ 150000000, 270000000, 360000000 },
+			{ 150000000, 270000000, 360000000, 480000000 },
+		},
+		.reg = { "csid1" },
+		.interrupt = { "csid1" },
+		.ops = &csid_ops_gen2
+	},
+
+	/* CSID2 */
+	{
+		.regulators = { "vdda-phy", "vdda-pll" },
+		.clock = {
+			"soc_ahb",
+			"vfe_lite",
+			"vfe_lite_cphy_rx",
+			"csi2",
+		},
+		.clock_rate = {
+			{ 0 },
+			{ 240000000, 360000000, 432000000, 600000000 },
+			{ 150000000, 270000000, 360000000 },
+			{ 150000000, 270000000, 360000000, 480000000 },
+		},
+		.reg = { "csid2" },
+		.interrupt = { "csid2" },
+		.is_lite = true,
+		.ops = &csid_ops_gen2
+	}
+};
+
+static const struct camss_subdev_resources vfe_res_7180[] = {
+	/* VFE0 */
+	{
+		.regulators = {},
+		.clock = {
+			"camnoc_axi",
+			"cpas_ahb",
+			"soc_ahb",
+			"vfe0",
+			"vfe0_axi",
+			"csi0",
+		},
+		.clock_rate = {
+			{ 0 },
+			{ 0 },
+			{ 0 },
+			{ 19200000, 240000000, 360000000, 432000000, 600000000 },
+			{ 0 },
+			{ 150000000, 270000000, 360000000, 480000000 },
+		},
+		.reg = { "vfe0" },
+		.interrupt = { "vfe0" },
+		.pd_name = "ife0",
+		.line_num = 4,
+		.has_pd = true,
+		.ops = &vfe_ops_170
+	},
+	/* VFE1 */
+	{
+		.regulators = {},
+		.clock = {
+			"camnoc_axi",
+			"cpas_ahb",
+			"soc_ahb",
+			"vfe1",
+			"vfe1_axi",
+			"csi1",
+		},
+		.clock_rate = {
+			{ 0 },
+			{ 0 },
+			{ 0 },
+			{ 19200000, 240000000, 360000000, 432000000, 600000000 },
+			{ 0 },
+			{ 150000000, 270000000, 360000000, 480000000 },
+		},
+		.reg = { "vfe1" },
+		.interrupt = { "vfe1" },
+		.pd_name = "ife1",
+		.line_num = 4,
+		.has_pd = true,
+		.ops = &vfe_ops_170
+	},
+	/* VFE-lite */
+	{
+		.regulators = {},
+		.clock = {
+			"camnoc_axi",
+			"cpas_ahb",
+			"soc_ahb",
+			"vfe_lite",
+			"csi2",
+		},
+		.clock_rate = {
+			{ 0 },
+			{ 0 },
+			{ 0 },
+			{ 19200000, 240000000, 360000000, 432000000, 600000000 },
+			{ 150000000, 270000000, 360000000, 480000000 },
+		},
+		.reg = { "vfe_lite" },
+		.interrupt = { "vfe_lite" },
+		.is_lite = true,
+		.line_num = 4,
+		.ops = &vfe_ops_170
+	}
+};
+
 static const struct camss_subdev_resources csiphy_res_8250[] = {
 	/* CSIPHY0 */
 	{
@@ -1263,7 +1467,7 @@ int camss_enable_clocks(int nclocks, struct camss_clock *clock,
 	for (i = 0; i < nclocks; i++) {
 		ret = clk_prepare_enable(clock[i].clk);
 		if (ret) {
-			dev_err(dev, "clock enable failed: %d\n", ret);
+			dev_err(dev, "clock enable failed: %s %d\n", clock[i].name, ret);
 			goto error;
 		}
 	}
@@ -2105,6 +2309,17 @@ static const struct camss_resources sdm845_resources = {
 	.vfe_num = ARRAY_SIZE(vfe_res_845),
 };
 
+static const struct camss_resources sc7180_resources = {
+	.version = CAMSS_7180,
+	.pd_name = "top",
+	.csiphy_res = csiphy_res_7180,
+	.csid_res = csid_res_7180,
+	.vfe_res = vfe_res_7180,
+	.csiphy_num = ARRAY_SIZE(csiphy_res_7180),
+	.csid_num = ARRAY_SIZE(csid_res_7180),
+	.vfe_num = ARRAY_SIZE(vfe_res_7180),
+};
+
 static const struct camss_resources sm8250_resources = {
 	.version = CAMSS_8250,
 	.pd_name = "top",
@@ -2137,6 +2352,7 @@ static const struct of_device_id camss_dt_match[] = {
 	{ .compatible = "qcom,msm8996-camss", .data = &msm8996_resources },
 	{ .compatible = "qcom,sdm660-camss", .data = &sdm660_resources },
 	{ .compatible = "qcom,sdm845-camss", .data = &sdm845_resources },
+	{ .compatible = "qcom,sc7180-camss", .data = &sc7180_resources },
 	{ .compatible = "qcom,sm8250-camss", .data = &sm8250_resources },
 	{ .compatible = "qcom,sc8280xp-camss", .data = &sc8280xp_resources },
 	{ }

-- 
2.34.1

Re: [PATCH 5/6] media: qcom: camss: Add sc7180 resources

[Konrad Dybcio]

On 21.06.2024 11:40 AM, George Chan via B4 Relay wrote:
> From: George Chan <gchan9527@gmail.com>
> 
> This commit describes the hardware layout for the sc7180 for the
> following hardware blocks:
> 
> - 2 x VFE
> - 1 x VFE Lite
> - 2 x CSID
> - 1 x CSID Lite
> - 4 x CSI PHY
> 
> Signed-off-by: George Chan <gchan9527@gmail.com>
> ---

[...]

>  		if (ret) {
> -			dev_err(dev, "clock enable failed: %d\n", ret);
> +			dev_err(dev, "clock enable failed: %s %d\n", clock[i].name, ret);

This is a good change, but should be separate


Generally this looks quite in line with [1], although I wasn't able to find
the matching clock rates

Konrad

[1] https://git.codelinaro.org/clo/la/kernel/msm-4.14/-/blob/UC.UM.1.0.r1-02500-sa8155.0/arch/arm64/boot/dts/qcom/atoll-camera.dtsi

Re: [PATCH 5/6] media: qcom: camss: Add sc7180 resources

[george chan]

On Sat, Jun 22, 2024 at 7:18 PM Konrad Dybcio <konrad.dybcio@linaro.org> wrote:
>
> although I wasn't able to find
> the matching clock rates
The clk rate is in the camcc clk driver, isn't it? ;-)

Re: [PATCH 5/6] media: qcom: camss: Add sc7180 resources

[Bryan O'Donoghue]

On 23/06/2024 22:48, george chan wrote:
> On Sat, Jun 22, 2024 at 7:18 PM Konrad Dybcio <konrad.dybcio@linaro.org> wrote:
>>
>> although I wasn't able to find
>> the matching clock rates
> The clk rate is in the camcc clk driver, isn't it? ;-)
> 

Yes but what we are saying here is you should have two patches. One 
patch to add the resources and a separate patch to add extended error 
messaging.

---
bod

Re: [PATCH 5/6] media: qcom: camss: Add sc7180 resources

[george chan]

On Mon, Jun 24, 2024 at 6:14 AM Bryan O'Donoghue
<bryan.odonoghue@linaro.org> wrote:
> Yes but what we are saying here is you should have two patches. One
> patch to add the resources and a separate patch to add extended error
> messaging.

Sure, v2 is available with split patches for sure.

[PATCH RFT 6/6] arm64: dts: qcom: sc7180: Add support for camss subsys

[George Chan via B4 Relay]

From: George Chan <gchan9527@gmail.com>

Introduce camss subsys support to sc7180 family soc.

Signed-off-by: George Chan <gchan9527@gmail.com>
---
 arch/arm64/boot/dts/qcom/sc7180.dtsi | 134 +++++++++++++++++++++++++++++++++++
 1 file changed, 134 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi b/arch/arm64/boot/dts/qcom/sc7180.dtsi
index b5ebf8980325..6ed4caafbe98 100644
--- a/arch/arm64/boot/dts/qcom/sc7180.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi
@@ -5,6 +5,7 @@
  * Copyright (c) 2019-2020, The Linux Foundation. All rights reserved.
  */
 
+#include <dt-bindings/clock/qcom,camcc-sc7180.h>
 #include <dt-bindings/clock/qcom,dispcc-sc7180.h>
 #include <dt-bindings/clock/qcom,gcc-sc7180.h>
 #include <dt-bindings/clock/qcom,gpucc-sc7180.h>
@@ -3150,6 +3151,139 @@ camnoc_virt: interconnect@ac00000 {
 			qcom,bcm-voters = <&apps_bcm_voter>;
 		};
 
+		camss: camss@acb3000 {
+			compatible = "qcom,sc7180-camss";
+
+			reg = <0 0x0acb3000 0 0x1000>,
+				<0 0x0acba000 0 0x1000>,
+				<0 0x0acc8000 0 0x1000>,
+				<0 0x0ac65000 0 0x1000>,
+				<0 0x0ac66000 0 0x1000>,
+				<0 0x0ac67000 0 0x1000>,
+				<0 0x0ac68000 0 0x1000>,
+				<0 0x0acaf000 0 0x4000>,
+				<0 0x0acb6000 0 0x4000>,
+				<0 0x0acc4000 0 0x4000>;
+			reg-names = "csid0",
+				"csid1",
+				"csid2",
+				"csiphy0",
+				"csiphy1",
+				"csiphy2",
+				"csiphy3",
+				"vfe0",
+				"vfe1",
+				"vfe_lite";
+
+			interrupts = <GIC_SPI 464 IRQ_TYPE_EDGE_RISING>,
+				<GIC_SPI 466 IRQ_TYPE_EDGE_RISING>,
+				<GIC_SPI 473 IRQ_TYPE_EDGE_RISING>,
+				<GIC_SPI 477 IRQ_TYPE_EDGE_RISING>,
+				<GIC_SPI 478 IRQ_TYPE_EDGE_RISING>,
+				<GIC_SPI 479 IRQ_TYPE_EDGE_RISING>,
+				<GIC_SPI 448 IRQ_TYPE_EDGE_RISING>,
+				<GIC_SPI 465 IRQ_TYPE_EDGE_RISING>,
+				<GIC_SPI 467 IRQ_TYPE_EDGE_RISING>,
+				<GIC_SPI 472 IRQ_TYPE_EDGE_RISING>;
+			interrupt-names = "csid0",
+				"csid1",
+				"csid2",
+				"csiphy0",
+				"csiphy1",
+				"csiphy2",
+				"csiphy3",
+				"vfe0",
+				"vfe1",
+				"vfe_lite";
+
+			power-domains = <&camcc IFE_0_GDSC>,
+				<&camcc IFE_1_GDSC>,
+				<&camcc TITAN_TOP_GDSC>;
+
+			power-domain-names = "ife0",
+				"ife1",
+				"top";
+
+			required-opps = <&rpmhpd_opp_low_svs>;
+
+			clocks = <&camcc CAM_CC_CAMNOC_AXI_CLK>,
+				<&camcc CAM_CC_CPAS_AHB_CLK>,
+				<&camcc CAM_CC_IFE_0_CSID_CLK>,
+				<&camcc CAM_CC_IFE_1_CSID_CLK>,
+				<&camcc CAM_CC_IFE_LITE_CSID_CLK>,
+				<&camcc CAM_CC_CSIPHY0_CLK>,
+				<&camcc CAM_CC_CSI0PHYTIMER_CLK>,
+				<&camcc CAM_CC_CSIPHY1_CLK>,
+				<&camcc CAM_CC_CSI1PHYTIMER_CLK>,
+				<&camcc CAM_CC_CSIPHY2_CLK>,
+				<&camcc CAM_CC_CSI2PHYTIMER_CLK>,
+				<&camcc CAM_CC_CSIPHY3_CLK>,
+				<&camcc CAM_CC_CSI3PHYTIMER_CLK>,
+				<&gcc GCC_CAMERA_AHB_CLK>,
+				<&gcc GCC_CAMERA_HF_AXI_CLK>,
+				<&camcc CAM_CC_SOC_AHB_CLK>,
+				<&camcc CAM_CC_IFE_0_AXI_CLK>,
+				<&camcc CAM_CC_IFE_0_CLK>,
+				<&camcc CAM_CC_IFE_0_CPHY_RX_CLK>,
+				<&camcc CAM_CC_IFE_1_AXI_CLK>,
+				<&camcc CAM_CC_IFE_1_CLK>,
+				<&camcc CAM_CC_IFE_1_CPHY_RX_CLK>,
+				<&camcc CAM_CC_IFE_LITE_CLK>,
+				<&camcc CAM_CC_IFE_LITE_CPHY_RX_CLK>;
+
+			clock-names = "camnoc_axi",
+				"cpas_ahb",
+				"csi0",
+				"csi1",
+				"csi2",
+				"csiphy0",
+				"csiphy0_timer",
+				"csiphy1",
+				"csiphy1_timer",
+				"csiphy2",
+				"csiphy2_timer",
+				"csiphy3",
+				"csiphy3_timer",
+				"gcc_camera_ahb",
+				"gcc_camera_axi",
+				"soc_ahb",
+				"vfe0_axi",
+				"vfe0",
+				"vfe0_cphy_rx",
+				"vfe1_axi",
+				"vfe1",
+				"vfe1_cphy_rx",
+				"vfe_lite",
+				"vfe_lite_cphy_rx";
+
+			iommus = <&apps_smmu 0x820 0x0>,
+				<&apps_smmu 0x840 0x0>,
+				<&apps_smmu 0x860 0x0>;
+
+			status = "disabled";
+
+			ports {
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				port@0 {
+					reg = <0>;
+				};
+
+				port@1 {
+					reg = <1>;
+				};
+
+				port@2 {
+					reg = <2>;
+				};
+
+				port@3 {
+					reg = <3>;
+				};
+			};
+		};
+
 		camcc: clock-controller@ad00000 {
 			compatible = "qcom,sc7180-camcc";
 			reg = <0 0x0ad00000 0 0x10000>;

-- 
2.34.1

Re: [PATCH RFT 6/6] arm64: dts: qcom: sc7180: Add support for camss subsys

[Krzysztof Kozlowski]

On 21/06/2024 11:40, George Chan via B4 Relay wrote:
> From: George Chan <gchan9527@gmail.com>
> 
> Introduce camss subsys support to sc7180 family soc.
> 
> Signed-off-by: George Chan <gchan9527@gmail.com>
> ---
>  arch/arm64/boot/dts/qcom/sc7180.dtsi | 134 +++++++++++++++++++++++++++++++++++
>  1 file changed, 134 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi b/arch/arm64/boot/dts/qcom/sc7180.dtsi
> index b5ebf8980325..6ed4caafbe98 100644
> --- a/arch/arm64/boot/dts/qcom/sc7180.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi
> @@ -5,6 +5,7 @@
>   * Copyright (c) 2019-2020, The Linux Foundation. All rights reserved.
>   */
>  
> +#include <dt-bindings/clock/qcom,camcc-sc7180.h>
>  #include <dt-bindings/clock/qcom,dispcc-sc7180.h>
>  #include <dt-bindings/clock/qcom,gcc-sc7180.h>
>  #include <dt-bindings/clock/qcom,gpucc-sc7180.h>
> @@ -3150,6 +3151,139 @@ camnoc_virt: interconnect@ac00000 {
>  			qcom,bcm-voters = <&apps_bcm_voter>;
>  		};
>  
> +		camss: camss@acb3000 {
> +			compatible = "qcom,sc7180-camss";
> +
> +			reg = <0 0x0acb3000 0 0x1000>,
> +				<0 0x0acba000 0 0x1000>,

All this looks misaligned.



Best regards,
Krzysztof