Re: [PATCH v2 2/6] dt-bindings: media: camss: Add qcom,kaanapali-camss binding

Vladimir Zapolskiy
2025-10-15T19:45:15Z

On 10/15/25 05:56, Hangxiang Ma wrote:
> Add bindings for qcom,kaanapali-camss in order to support the camera
> subsystem for Kaanapali.
>
> Signed-off-by: Hangxiang Ma <hangxiang.ma@oss.qualcomm.com>
> ---
> .../bindings/media/qcom,kaanapali-camss.yaml | 494 +++++++++++++++++++++
> 1 file changed, 494 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/media/qcom,kaanapali-camss.yaml b/Documentation/devicetree/bindings/media/qcom,kaanapali-camss.yaml
> new file mode 100644
> index 000000000000..d04c21103cfd
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/qcom,kaanapali-camss.yaml
> @@ -0,0 +1,494 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/media/qcom,kaanapali-camss.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Qualcomm Kaanapali Camera Subsystem (CAMSS)
> +
> +maintainers:
> + - Hangxiang Ma <hangxiang.ma@oss.qualcomm.com>
> +
> +description:
> + The CAMSS IP is a CSI decoder and ISP present on Qualcomm platforms.
> +
> +properties:
> + compatible:
> + const: qcom,kaanapali-camss
> +
> + reg:
> + maxItems: 16
> +
> + reg-names:
> + items:
> + - const: csid0
> + - const: csid1
> + - const: csid2
> + - const: csid_lite0
> + - const: csid_lite1
> + - const: csiphy0
> + - const: csiphy1
> + - const: csiphy2
> + - const: csiphy3
> + - const: csiphy4
> + - const: csiphy5
> + - const: vfe0
> + - const: vfe1
> + - const: vfe2
> + - const: vfe_lite0
> + - const: vfe_lite1
> +
> + clocks:
> + maxItems: 34
> +
> + clock-names:
> + items:
> + - const: camnoc_nrt_axi
> + - const: camnoc_rt_axi
> + - const: camnoc_rt_vfe0
> + - const: camnoc_rt_vfe1
> + - const: camnoc_rt_vfe2
> + - const: camnoc_rt_vfe_lite
> + - const: cam_top_ahb
> + - const: cam_top_fast_ahb
> + - const: csid
> + - const: csid_csiphy_rx
> + - const: csiphy0
> + - const: csiphy0_timer
> + - const: csiphy1
> + - const: csiphy1_timer
> + - const: csiphy2
> + - const: csiphy2_timer
> + - const: csiphy3
> + - const: csiphy3_timer
> + - const: csiphy4
> + - const: csiphy4_timer
> + - const: csiphy5
> + - const: csiphy5_timer
> + - const: gcc_hf_axi
> + - const: qdss_debug_xo
> + - const: vfe0
> + - const: vfe0_fast_ahb
> + - const: vfe1
> + - const: vfe1_fast_ahb
> + - const: vfe2
> + - const: vfe2_fast_ahb
> + - const: vfe_lite
> + - const: vfe_lite_ahb
> + - const: vfe_lite_cphy_rx
> + - const: vfe_lite_csid

The list of 'clock-names' values is not alphanumerically sorted.

> +
> + interrupts:
> + maxItems: 16
> + interrupt-names:

Missing empty line to separate properties.

> + items:
> + - const: csid0
> + - const: csid1
> + - const: csid2
> + - const: csid_lite0
> + - const: csid_lite1
> + - const: csiphy0
> + - const: csiphy1
> + - const: csiphy2
> + - const: csiphy3
> + - const: csiphy4
> + - const: csiphy5
> + - const: vfe0
> + - const: vfe1
> + - const: vfe2
> + - const: vfe_lite0
> + - const: vfe_lite1
> +
> + interconnects:
> + maxItems: 2
> +
> + interconnect-names:
> + items:
> + - const: ahb
> + - const: hf_0_mnoc

Please rename "hf_0_mnoc" to "hf_mnoc", see qcom,qcm2290-camss.yaml etc.

> +
> + iommus:
> + maxItems: 1
> +
> + power-domains:
> + items:
> + - description:
> + TFE0 GDSC - Thin Front End, Global Distributed Switch Controller.
> + - description:
> + TFE1 GDSC - Thin Front End, Global Distributed Switch Controller.
> + - description:
> + TFE2 GDSC - Thin Front End, Global Distributed Switch Controller.
> + - description:
> + Titan GDSC - Titan ISP Block Global Distributed Switch Controller.
> +
> + power-domain-names:
> + items:
> + - const: tfe0
> + - const: tfe1
> + - const: tfe2

Please remove all 'tfeX' power domains, they are not going to be utilized
any time soon.

When 'power-domains' list is just a single Titan GDSC, 'power-domain-names'
property is not needed.

> + - const: top
> +
> + vdda-pll-supply:
> + description:
> + Phandle to 1.2V regulator supply to PHY refclk pll block.
> +
> + vdda-phy0-supply:
> + description:
> + Phandle to 0.8V regulator supply to PHY core block.
> +
> + vdda-phy1-supply:
> + description:
> + Phandle to 0.8V regulator supply to PHY core block.
> +
> + vdda-phy2-supply:
> + description:
> + Phandle to 0.8V regulator supply to PHY core block.
> +
> + vdda-phy3-supply:
> + description:
> + Phandle to 0.8V regulator supply to PHY core block.
> +
> + vdda-phy4-supply:
> + description:
> + Phandle to 0.8V regulator supply to PHY core block.
> +
> + vdda-phy5-supply:
> + description:
> + Phandle to 0.8V regulator supply to PHY core block.

What is the difference between vdda-phyX-supply properties, why do you
need so many of them, when their descriptions say they are all the same?

> + ports:
> + $ref: /schemas/graph.yaml#/properties/ports
> +
> + description:
> + CSI input ports.
> +
> + properties:
> + port@0:

Please use

patternProperties:
"^port@[0-3]$":

> + $ref: /schemas/graph.yaml#/$defs/port-base
> + unevaluatedProperties: false
> + description:
> + Input port for receiving CSI data on CSI0.
> +
> + properties:
> + endpoint:
> + $ref: video-interfaces.yaml#
> + unevaluatedProperties: false
> +
> + properties:
> + clock-lanes:
> + maxItems: 1

Please remove 'clock-lanes' property, it is non-configurable, redundant
and tends to store some irrelevant value.

> +
> + data-lanes:
> + minItems: 1
> + maxItems: 4
> +
> + bus-type:
> + enum:
> + - 1 # MEDIA_BUS_TYPE_CSI2_CPHY
> + - 4 # MEDIA_BUS_TYPE_CSI2_DPHY
> +
> + required:
> + - clock-lanes

The 'clock-lanes' property is expected to be removed.

> + - data-lanes
> +
> + port@1:
> + $ref: /schemas/graph.yaml#/$defs/port-base
> + unevaluatedProperties: false
> + description:
> + Input port for receiving CSI data on CSI1.
> +
> + properties:
> + endpoint:
> + $ref: video-interfaces.yaml#
> + unevaluatedProperties: false
> +
> + properties:
> + clock-lanes:
> + maxItems: 1
> +
> + data-lanes:
> + minItems: 1
> + maxItems: 4
> +
> + bus-type:
> + enum:
> + - 1 # MEDIA_BUS_TYPE_CSI2_CPHY
> + - 4 # MEDIA_BUS_TYPE_CSI2_DPHY
> +
> + required:
> + - clock-lanes
> + - data-lanes
> +
> + port@2:
> + $ref: /schemas/graph.yaml#/$defs/port-base
> + unevaluatedProperties: false
> + description:
> + Input port for receiving CSI data on CSI2.
> +
> + properties:
> + endpoint:
> + $ref: video-interfaces.yaml#
> + unevaluatedProperties: false
> +
> + properties:
> + clock-lanes:
> + maxItems: 1
> +
> + data-lanes:
> + minItems: 1
> + maxItems: 4
> +
> + bus-type:
> + enum:
> + - 1 # MEDIA_BUS_TYPE_CSI2_CPHY
> + - 4 # MEDIA_BUS_TYPE_CSI2_DPHY
> +
> + required:
> + - clock-lanes
> + - data-lanes
> +
> + port@3:
> + $ref: /schemas/graph.yaml#/$defs/port-base
> + unevaluatedProperties: false
> + description:
> + Input port for receiving CSI data on CSI3.
> +
> + properties:
> + endpoint:
> + $ref: video-interfaces.yaml#
> + unevaluatedProperties: false
> +
> + properties:
> + clock-lanes:
> + maxItems: 1
> +
> + data-lanes:
> + minItems: 1
> + maxItems: 4
> +
> + bus-type:
> + enum:
> + - 1 # MEDIA_BUS_TYPE_CSI2_CPHY
> + - 4 # MEDIA_BUS_TYPE_CSI2_DPHY
> +
> + required:
> + - clock-lanes
> + - data-lanes
> +
> +required:
> + - compatible
> + - reg
> + - reg-names
> + - clocks
> + - clock-names
> + - interrupts
> + - interrupt-names
> + - interconnects
> + - interconnect-names
> + - iommus
> + - power-domains
> + - power-domain-names
> + - vdda-pll-supply
> + - vdda-phy0-supply
> + - vdda-phy1-supply
> + - vdda-phy2-supply
> + - vdda-phy3-supply
> + - vdda-phy4-supply
> + - vdda-phy5-supply

Please exclude supplies from the list of required properties.

> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/clock/qcom,rpmh.h>
> + #include <dt-bindings/clock/qcom,kaanapali-camcc.h>
> + #include <dt-bindings/clock/qcom,kaanapali-gcc.h>
> + #include <dt-bindings/interconnect/qcom,icc.h>
> + #include <dt-bindings/interconnect/qcom,kaanapali-rpmh.h>
> + #include <dt-bindings/interrupt-controller/arm-gic.h>
> + #include <dt-bindings/power/qcom,rpmhpd.h>

Please keep the list of included headers sorted alphabetically.

> +
> + soc {
> + #address-cells = <2>;
> + #size-cells = <2>;
> +
> + camss: isp@9253000 {
> + compatible = "qcom,kaanapali-camss";
> +
> + reg = <0x0 0x09253000 0x0 0x5e80>,
> + <0x0 0x09263000 0x0 0x5e80>,
> + <0x0 0x09273000 0x0 0x5e80>,
> + <0x0 0x092d3000 0x0 0x3880>,
> + <0x0 0x092e7000 0x0 0x3880>,
> + <0x0 0x09523000 0x0 0x2000>,
> + <0x0 0x09525000 0x0 0x2000>,
> + <0x0 0x09527000 0x0 0x2000>,
> + <0x0 0x09529000 0x0 0x2000>,
> + <0x0 0x0952b000 0x0 0x2000>,
> + <0x0 0x0952d000 0x0 0x2000>,
> + <0x0 0x09151000 0x0 0x20000>,
> + <0x0 0x09171000 0x0 0x20000>,
> + <0x0 0x09191000 0x0 0x20000>,
> + <0x0 0x092dc000 0x0 0x1300>,
> + <0x0 0x092f0000 0x0 0x1300>;
> + reg-names = "csid0",
> + "csid1",
> + "csid2",
> + "csid_lite0",
> + "csid_lite1",
> + "csiphy0",
> + "csiphy1",
> + "csiphy2",
> + "csiphy3",
> + "csiphy4",
> + "csiphy5",
> + "vfe0",
> + "vfe1",
> + "vfe2",
> + "vfe_lite0",
> + "vfe_lite1";
> +
> + clocks = <&camcc CAM_CC_CAMNOC_NRT_AXI_CLK>,
> + <&camcc CAM_CC_CAMNOC_RT_AXI_CLK>,
> + <&camcc CAM_CC_CAMNOC_RT_TFE_0_MAIN_CLK>,
> + <&camcc CAM_CC_CAMNOC_RT_TFE_1_MAIN_CLK>,
> + <&camcc CAM_CC_CAMNOC_RT_TFE_2_MAIN_CLK>,
> + <&camcc CAM_CC_CAMNOC_RT_IFE_LITE_CLK>,
> + <&camcc CAM_CC_CAM_TOP_AHB_CLK>,
> + <&camcc CAM_CC_CAM_TOP_FAST_AHB_CLK>,
> + <&camcc CAM_CC_CSID_CLK>,
> + <&camcc CAM_CC_CSID_CSIPHY_RX_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>,
> + <&camcc CAM_CC_CSIPHY4_CLK>,
> + <&camcc CAM_CC_CSI4PHYTIMER_CLK>,
> + <&camcc CAM_CC_CSIPHY5_CLK>,
> + <&camcc CAM_CC_CSI5PHYTIMER_CLK>,
> + <&gcc GCC_CAMERA_HF_AXI_CLK>,
> + <&camcc CAM_CC_QDSS_DEBUG_XO_CLK>,
> + <&camcc CAM_CC_TFE_0_MAIN_CLK>,
> + <&camcc CAM_CC_TFE_0_MAIN_FAST_AHB_CLK>,
> + <&camcc CAM_CC_TFE_1_MAIN_CLK>,
> + <&camcc CAM_CC_TFE_1_MAIN_FAST_AHB_CLK>,
> + <&camcc CAM_CC_TFE_2_MAIN_CLK>,
> + <&camcc CAM_CC_TFE_2_MAIN_FAST_AHB_CLK>,
> + <&camcc CAM_CC_IFE_LITE_CLK>,
> + <&camcc CAM_CC_IFE_LITE_AHB_CLK>,
> + <&camcc CAM_CC_IFE_LITE_CPHY_RX_CLK>,
> + <&camcc CAM_CC_IFE_LITE_CSID_CLK>;
> + clock-names = "camnoc_nrt_axi",
> + "camnoc_rt_axi",
> + "camnoc_rt_vfe0",
> + "camnoc_rt_vfe1",
> + "camnoc_rt_vfe2",
> + "camnoc_rt_vfe_lite",
> + "cam_top_ahb",
> + "cam_top_fast_ahb",
> + "csid",
> + "csid_csiphy_rx",
> + "csiphy0",
> + "csiphy0_timer",
> + "csiphy1",
> + "csiphy1_timer",
> + "csiphy2",
> + "csiphy2_timer",
> + "csiphy3",
> + "csiphy3_timer",
> + "csiphy4",
> + "csiphy4_timer",
> + "csiphy5",
> + "csiphy5_timer",
> + "gcc_hf_axi",
> + "qdss_debug_xo",
> + "vfe0",
> + "vfe0_fast_ahb",
> + "vfe1",
> + "vfe1_fast_ahb",
> + "vfe2",
> + "vfe2_fast_ahb",
> + "vfe_lite",
> + "vfe_lite_ahb",
> + "vfe_lite_cphy_rx",
> + "vfe_lite_csid";
> +
> + interrupts = <GIC_SPI 601 IRQ_TYPE_EDGE_RISING>,
> + <GIC_SPI 603 IRQ_TYPE_EDGE_RISING>,
> + <GIC_SPI 431 IRQ_TYPE_EDGE_RISING>,
> + <GIC_SPI 605 IRQ_TYPE_EDGE_RISING>,
> + <GIC_SPI 376 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 122 IRQ_TYPE_EDGE_RISING>,
> + <GIC_SPI 89 IRQ_TYPE_EDGE_RISING>,
> + <GIC_SPI 433 IRQ_TYPE_EDGE_RISING>,
> + <GIC_SPI 436 IRQ_TYPE_EDGE_RISING>,
> + <GIC_SPI 457 IRQ_TYPE_EDGE_RISING>,
> + <GIC_SPI 606 IRQ_TYPE_EDGE_RISING>,
> + <GIC_SPI 377 IRQ_TYPE_EDGE_RISING>;
> + interrupt-names = "csid0",
> + "csid1",
> + "csid2",
> + "csid_lite0",
> + "csid_lite1",
> + "csiphy0",
> + "csiphy1",
> + "csiphy2",
> + "csiphy3",
> + "csiphy4",
> + "csiphy5",
> + "vfe0",
> + "vfe1",
> + "vfe2",
> + "vfe_lite0",
> + "vfe_lite1";
> +
> + interconnects = <&gem_noc MASTER_APPSS_PROC QCOM_ICC_TAG_ACTIVE_ONLY
> + &config_noc SLAVE_CAMERA_CFG QCOM_ICC_TAG_ACTIVE_ONLY>,
> + <&mmss_noc MASTER_CAMNOC_HF QCOM_ICC_TAG_ALWAYS
> + &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>;
> + interconnect-names = "ahb",
> + "hf_0_mnoc";
> +
> + iommus = <&apps_smmu 0x1c00 0x00>;
> +
> + power-domains = <&camcc CAM_CC_TFE_0_GDSC>,
> + <&camcc CAM_CC_TFE_1_GDSC>,
> + <&camcc CAM_CC_TFE_2_GDSC>,
> + <&camcc CAM_CC_TITAN_TOP_GDSC>;
> + power-domain-names = "tfe0",
> + "tfe1",
> + "tfe2",
> + "top";
> +
> + vdda-pll-supply = <&vreg_l1d_1p2>;
> + vdda-phy0-supply = <&vreg_l3i_0p8>;
> + vdda-phy1-supply = <&vreg_l3i_0p8>;
> + vdda-phy2-supply = <&vreg_l3d_0p8>;
> + vdda-phy3-supply = <&vreg_l3i_0p8>;
> + vdda-phy4-supply = <&vreg_l3d_0p8>;
> + vdda-phy5-supply = <&vreg_l3i_0p8>;
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port@0 {
> + reg = <0>;
> +
> + csiphy_ep0: endpoint {
> + clock-lanes = <7>;
> + data-lanes = <0 1>;
> + remote-endpoint = <&sensor_ep>;
> + };
> + };
> + };
> + };
> + };
>

--
Best wishes,
Vladimir