Add bindings for qcom,kaanapali-camss to support the Camera Subsystem
(CAMSS) on the Qualcomm Kaanapali platform.
The Kaanapali platform provides:
- 3 x VFE, 5 RDI per VFE
- 2 x VFE Lite, 4 RDI per VFE Lite
- 3 x CSID
- 2 x CSID Lite
- 6 x CSIPHY
- 2 x ICP
- 1 x IPE
- 2 x JPEG DMA & Downscaler
- 2 x JPEG Encoder
- 1 x OFE
- 5 x RT CDM
- 3 x TPG
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>
Signed-off-by: Hangxiang Ma <hangxiang.ma@oss.qualcomm.com>
---
.../bindings/media/qcom,kaanapali-camss.yaml | 646 +++++++++++++++++++++
1 file changed, 646 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..3b54620e14c6
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/qcom,kaanapali-camss.yaml
@@ -0,0 +1,646 @@
+# 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:
+ Kaanapali camera subsystem includes submodules such as CSIPHY (CSI Physical layer)
+ and CSID (CSI Decoder), which comply with the MIPI CSI2 protocol.
+
+ The subsystem also integrates a set of real-time image processing engines and their
+ associated configuration modules, as well as non-real-time engines.
+
+ Additionally, it encompasses a test pattern generator (TPG) submodule.
+
+properties:
+ compatible:
+ const: qcom,kaanapali-camss
+
+ reg:
+ items:
+ - description: Registers for CSID 0
+ - description: Registers for CSID 1
+ - description: Registers for CSID 2
+ - description: Registers for CSID Lite 0
+ - description: Registers for CSID Lite 1
+ - description: Registers for CSIPHY 0
+ - description: Registers for CSIPHY 1
+ - description: Registers for CSIPHY 2
+ - description: Registers for CSIPHY 3
+ - description: Registers for CSIPHY 4
+ - description: Registers for CSIPHY 5
+ - description: Registers for VFE (Video Front End) 0
+ - description: Registers for VFE 1
+ - description: Registers for VFE 2
+ - description: Registers for VFE Lite 0
+ - description: Registers for VFE Lite 1
+ - description: Registers for ICP (Imaging Control Processor) 0
+ - description: Registers for ICP 0 SYS
+ - description: Registers for ICP 1
+ - description: Registers for ICP 1 SYS
+ - description: Registers for IPE (Image Processing Engine)
+ - description: Registers for JPEG DMA & Downscaler
+ - description: Registers for JPEG Encoder
+ - description: Registers for OFE (Offline Front End)
+ - description: Registers for RT CDM (Camera Data Mover) 0
+ - description: Registers for RT CDM 1
+ - description: Registers for RT CDM 2
+ - description: Registers for RT CDM 3
+ - description: Registers for RT CDM 4
+ - description: Registers for TPG 0
+ - description: Registers for TPG 1
+ - description: Registers for TPG 2
+
+ 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
+ - const: icp0
+ - const: icp0_sys
+ - const: icp1
+ - const: icp1_sys
+ - const: ipe
+ - const: jpeg_dma
+ - const: jpeg_enc
+ - const: ofe
+ - const: rt_cdm0
+ - const: rt_cdm1
+ - const: rt_cdm2
+ - const: rt_cdm3
+ - const: rt_cdm4
+ - const: tpg0
+ - const: tpg1
+ - const: tpg2
+
+ clocks:
+ maxItems: 60
+
+ 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: cpas_ahb
+ - const: cpas_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_axi_hf
+ - 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
+ - const: qdss_debug_xo
+ - const: camnoc_ipe_nps
+ - const: camnoc_ofe
+ - const: gcc_axi_sf
+ - const: icp0
+ - const: icp0_ahb
+ - const: icp1
+ - const: icp1_ahb
+ - const: ipe_nps
+ - const: ipe_nps_ahb
+ - const: ipe_nps_fast_ahb
+ - const: ipe_pps
+ - const: ipe_pps_fast_ahb
+ - const: jpeg
+ - const: ofe_ahb
+ - const: ofe_anchor
+ - const: ofe_anchor_fast_ahb
+ - const: ofe_hdr
+ - const: ofe_hdr_fast_ahb
+ - const: ofe_main
+ - const: ofe_main_fast_ahb
+ - const: vfe0_bayer
+ - const: vfe0_bayer_fast_ahb
+ - const: vfe1_bayer
+ - const: vfe1_bayer_fast_ahb
+ - const: vfe2_bayer
+ - const: vfe2_bayer_fast_ahb
+
+ interrupts:
+ maxItems: 30
+
+ interrupt-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
+ - const: camnoc_nrt
+ - const: camnoc_rt
+ - const: icp0
+ - const: icp1
+ - const: jpeg_dma
+ - const: jpeg_enc
+ - const: rt_cdm0
+ - const: rt_cdm1
+ - const: rt_cdm2
+ - const: rt_cdm3
+ - const: rt_cdm4
+ - const: tpg0
+ - const: tpg1
+ - const: tpg2
+
+ interconnects:
+ maxItems: 4
+
+ interconnect-names:
+ items:
+ - const: ahb
+ - const: hf_mnoc
+ - const: sf_icp_mnoc
+ - const: sf_mnoc
+
+ iommus:
+ items:
+ - description: VFE non-protected stream
+ - description: ICP0 shared stream
+ - description: ICP1 shared stream
+ - description: IPE CDM non-protected stream
+ - description: IPE non-protected stream
+ - description: JPEG non-protected stream
+ - description: OFE CDM non-protected stream
+ - description: OFE non-protected stream
+ - description: VFE / VFE Lite CDM non-protected stream
+
+ power-domains:
+ items:
+ - description:
+ VFE0 GDSC - Global Distributed Switch Controller for VFE0.
+ - description:
+ VFE1 GDSC - Global Distributed Switch Controller for VFE1.
+ - description:
+ VFE2 GDSC - Global Distributed Switch Controller for VFE2.
+ - description:
+ Titan GDSC - Global Distributed Switch Controller for the entire camss.
+ - description:
+ IPE GDSC - Global Distributed Switch Controller for IPE.
+ - description:
+ OFE GDSC - Block Global Distributed Switch Controller for OFE.
+
+ power-domain-names:
+ items:
+ - const: vfe0
+ - const: vfe1
+ - const: vfe2
+ - const: top
+ - const: ipe
+ - const: ofe
+
+ vdd-csiphy0-0p8-supply:
+ description:
+ Phandle to a 0.8V regulator supply to CSIPHY0 core block.
+
+ vdd-csiphy0-1p2-supply:
+ description:
+ Phandle to a 1.2V regulator supply to CSIPHY0 pll block.
+
+ vdd-csiphy1-0p8-supply:
+ description:
+ Phandle to a 0.8V regulator supply to CSIPHY1 core block.
+
+ vdd-csiphy1-1p2-supply:
+ description:
+ Phandle to a 1.2V regulator supply to CSIPHY1 pll block.
+
+ vdd-csiphy2-0p8-supply:
+ description:
+ Phandle to a 0.8V regulator supply to CSIPHY2 core block.
+
+ vdd-csiphy2-1p2-supply:
+ description:
+ Phandle to a 1.2V regulator supply to CSIPHY2 pll block.
+
+ vdd-csiphy3-0p8-supply:
+ description:
+ Phandle to a 0.8V regulator supply to CSIPHY3 core block.
+
+ vdd-csiphy3-1p2-supply:
+ description:
+ Phandle to a 1.2V regulator supply to CSIPHY3 pll block.
+
+ vdd-csiphy4-0p8-supply:
+ description:
+ Phandle to a 0.8V regulator supply to CSIPHY4 core block.
+
+ vdd-csiphy4-1p2-supply:
+ description:
+ Phandle to a 1.2V regulator supply to CSIPHY4 pll block.
+
+ vdd-csiphy5-0p8-supply:
+ description:
+ Phandle to a 0.8V regulator supply to CSIPHY5 core block.
+
+ vdd-csiphy5-1p2-supply:
+ description:
+ Phandle to a 1.2V regulator supply to CSIPHY5 pll block.
+
+ ports:
+ $ref: /schemas/graph.yaml#/properties/ports
+
+ description:
+ CSI input ports.
+
+patternProperties:
+ "^port@[0-5]$":
+ $ref: /schemas/graph.yaml#/$defs/port-base
+ unevaluatedProperties: false
+ description:
+ Input ports for receiving CSI data on CSIPHY 0-5.
+
+ properties:
+ endpoint:
+ $ref: video-interfaces.yaml#
+ unevaluatedProperties: false
+
+ properties:
+ data-lanes:
+ minItems: 1
+ maxItems: 4
+
+ required:
+ - data-lanes
+
+required:
+ - compatible
+ - reg
+ - reg-names
+ - clocks
+ - clock-names
+ - interrupts
+ - interrupt-names
+ - interconnects
+ - interconnect-names
+ - iommus
+ - power-domains
+ - power-domain-names
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interconnect/qcom,icc.h>
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+ #include <dt-bindings/power/qcom,rpmhpd.h>
+
+ soc {
+ #address-cells = <2>;
+ #size-cells = <2>;
+
+ 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>,
+ <0x0 0x0900e000 0x0 0x1000>,
+ <0x0 0x0900d000 0x0 0x1000>,
+ <0x0 0x0902e000 0x0 0x1000>,
+ <0x0 0x0902d000 0x0 0x1000>,
+ <0x0 0x090d7000 0x0 0x20000>,
+ <0x0 0x0904e000 0x0 0x1000>,
+ <0x0 0x0904d000 0x0 0x1000>,
+ <0x0 0x09057000 0x0 0x40000>,
+ <0x0 0x09147000 0x0 0x580>,
+ <0x0 0x09148000 0x0 0x580>,
+ <0x0 0x09149000 0x0 0x580>,
+ <0x0 0x0914a000 0x0 0x580>,
+ <0x0 0x0914b000 0x0 0x580>,
+ <0x0 0x093fd000 0x0 0x400>,
+ <0x0 0x093fe000 0x0 0x400>,
+ <0x0 0x093ff000 0x0 0x400>;
+ reg-names = "csid0",
+ "csid1",
+ "csid2",
+ "csid_lite0",
+ "csid_lite1",
+ "csiphy0",
+ "csiphy1",
+ "csiphy2",
+ "csiphy3",
+ "csiphy4",
+ "csiphy5",
+ "vfe0",
+ "vfe1",
+ "vfe2",
+ "vfe_lite0",
+ "vfe_lite1",
+ "icp0",
+ "icp0_sys",
+ "icp1",
+ "icp1_sys",
+ "ipe",
+ "jpeg_dma",
+ "jpeg_enc",
+ "ofe",
+ "rt_cdm0",
+ "rt_cdm1",
+ "rt_cdm2",
+ "rt_cdm3",
+ "rt_cdm4",
+ "tpg0",
+ "tpg1",
+ "tpg2";
+
+ clocks = <&camcc_cam_cc_camnoc_nrt_axi_clk>,
+ <&camcc_cam_cc_camnoc_rt_axi_clk>,
+ <&camcc_cam_cc_camnoc_rt_vfe_0_main_clk>,
+ <&camcc_cam_cc_camnoc_rt_vfe_1_main_clk>,
+ <&camcc_cam_cc_camnoc_rt_vfe_2_main_clk>,
+ <&camcc_cam_cc_camnoc_rt_vfe_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_vfe_0_main_clk>,
+ <&camcc_cam_cc_vfe_0_main_fast_ahb_clk>,
+ <&camcc_cam_cc_vfe_1_main_clk>,
+ <&camcc_cam_cc_vfe_1_main_fast_ahb_clk>,
+ <&camcc_cam_cc_vfe_2_main_clk>,
+ <&camcc_cam_cc_vfe_2_main_fast_ahb_clk>,
+ <&camcc_cam_cc_vfe_lite_clk>,
+ <&camcc_cam_cc_vfe_lite_ahb_clk>,
+ <&camcc_cam_cc_vfe_lite_cphy_rx_clk>,
+ <&camcc_cam_cc_vfe_lite_csid_clk>,
+ <&camcc_cam_cc_qdss_debug_xo_clk>,
+ <&camcc_cam_cc_camnoc_nrt_ipe_nps_clk>,
+ <&camcc_cam_cc_camnoc_nrt_ofe_main_clk>,
+ <&gcc_gcc_camera_sf_axi_clk>,
+ <&camcc_cam_cc_icp_0_clk>,
+ <&camcc_cam_cc_icp_0_ahb_clk>,
+ <&camcc_cam_cc_icp_1_clk>,
+ <&camcc_cam_cc_icp_1_ahb_clk>,
+ <&camcc_cam_cc_ipe_nps_clk>,
+ <&camcc_cam_cc_ipe_nps_ahb_clk>,
+ <&camcc_cam_cc_ipe_nps_fast_ahb_clk>,
+ <&camcc_cam_cc_ipe_pps_clk>,
+ <&camcc_cam_cc_ipe_pps_fast_ahb_clk>,
+ <&camcc_cam_cc_jpeg_clk>,
+ <&camcc_cam_cc_ofe_ahb_clk>,
+ <&camcc_cam_cc_ofe_anchor_clk>,
+ <&camcc_cam_cc_ofe_anchor_fast_ahb_clk>,
+ <&camcc_cam_cc_ofe_hdr_clk>,
+ <&camcc_cam_cc_ofe_hdr_fast_ahb_clk>,
+ <&camcc_cam_cc_ofe_main_clk>,
+ <&camcc_cam_cc_ofe_main_fast_ahb_clk>,
+ <&camcc_cam_cc_vfe_0_bayer_clk>,
+ <&camcc_cam_cc_vfe_0_bayer_fast_ahb_clk>,
+ <&camcc_cam_cc_vfe_1_bayer_clk>,
+ <&camcc_cam_cc_vfe_1_bayer_fast_ahb_clk>,
+ <&camcc_cam_cc_vfe_2_bayer_clk>,
+ <&camcc_cam_cc_vfe_2_bayer_fast_ahb_clk>;
+ clock-names = "camnoc_nrt_axi",
+ "camnoc_rt_axi",
+ "camnoc_rt_vfe0",
+ "camnoc_rt_vfe1",
+ "camnoc_rt_vfe2",
+ "camnoc_rt_vfe_lite",
+ "cpas_ahb",
+ "cpas_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_axi_hf",
+ "vfe0",
+ "vfe0_fast_ahb",
+ "vfe1",
+ "vfe1_fast_ahb",
+ "vfe2",
+ "vfe2_fast_ahb",
+ "vfe_lite",
+ "vfe_lite_ahb",
+ "vfe_lite_cphy_rx",
+ "vfe_lite_csid",
+ "qdss_debug_xo",
+ "camnoc_ipe_nps",
+ "camnoc_ofe",
+ "gcc_axi_sf",
+ "icp0",
+ "icp0_ahb",
+ "icp1",
+ "icp1_ahb",
+ "ipe_nps",
+ "ipe_nps_ahb",
+ "ipe_nps_fast_ahb",
+ "ipe_pps",
+ "ipe_pps_fast_ahb",
+ "jpeg",
+ "ofe_ahb",
+ "ofe_anchor",
+ "ofe_anchor_fast_ahb",
+ "ofe_hdr",
+ "ofe_hdr_fast_ahb",
+ "ofe_main",
+ "ofe_main_fast_ahb",
+ "vfe0_bayer",
+ "vfe0_bayer_fast_ahb",
+ "vfe1_bayer",
+ "vfe1_bayer_fast_ahb",
+ "vfe2_bayer",
+ "vfe2_bayer_fast_ahb";
+
+ 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>,
+ <GIC_SPI 271 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 277 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 463 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 657 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 372 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 475 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 456 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 664 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 702 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 348 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 349 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 413 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 416 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 417 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",
+ "camnoc_nrt",
+ "camnoc_rt",
+ "icp0",
+ "icp1",
+ "jpeg_dma",
+ "jpeg_enc",
+ "rt_cdm0",
+ "rt_cdm1",
+ "rt_cdm2",
+ "rt_cdm3",
+ "rt_cdm4",
+ "tpg0",
+ "tpg1",
+ "tpg2";
+
+ 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>,
+ <&mmss_noc_master_camnoc_sf_icp QCOM_ICC_TAG_ALWAYS
+ &mc_virt_slave_ebi1 QCOM_ICC_TAG_ALWAYS>,
+ <&mmss_noc_master_camnoc_sf QCOM_ICC_TAG_ALWAYS
+ &mc_virt_slave_ebi1 QCOM_ICC_TAG_ALWAYS>;
+ interconnect-names = "ahb",
+ "hf_mnoc",
+ "sf_icp_mnoc",
+ "sf_mnoc";
+
+ iommus = <&apps_smmu 0x1c00 0x00>,
+ <&apps_smmu 0x18c0 0x00>,
+ <&apps_smmu 0x1980 0x00>,
+ <&apps_smmu 0x1840 0x00>,
+ <&apps_smmu 0x1800 0x00>,
+ <&apps_smmu 0x18a0 0x00>,
+ <&apps_smmu 0x1880 0x00>,
+ <&apps_smmu 0x1820 0x00>,
+ <&apps_smmu 0x1860 0x00>;
+
+ power-domains = <&camcc_cam_cc_vfe_0_gdsc>,
+ <&camcc_cam_cc_vfe_1_gdsc>,
+ <&camcc_cam_cc_vfe_2_gdsc>,
+ <&camcc_cam_cc_titan_top_gdsc>,
+ <&camcc_cam_cc_ipe_gdsc>,
+ <&camcc_cam_cc_ofe_gdsc>;
+ power-domain-names = "vfe0",
+ "vfe1",
+ "vfe2",
+ "top",
+ "ipe",
+ "ofe";
+
+ vdd-csiphy0-0p8-supply = <&vreg_0p8_supply>;
+ vdd-csiphy0-1p2-supply = <&vreg_1p2_supply>;
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ reg = <0>;
+
+ csiphy_ep0: endpoint {
+ data-lanes = <0 1>;
+ remote-endpoint = <&sensor_ep>;
+ };
+ };
+ };
+ };
+ };
--
2.34.1On Mon, Dec 08, 2025 at 04:39:47AM -0800, Hangxiang Ma wrote: > Add bindings for qcom,kaanapali-camss to support the Camera Subsystem > (CAMSS) on the Qualcomm Kaanapali platform. > > The Kaanapali platform provides: > > - 3 x VFE, 5 RDI per VFE > - 2 x VFE Lite, 4 RDI per VFE Lite > - 3 x CSID > - 2 x CSID Lite > - 6 x CSIPHY > - 2 x ICP > - 1 x IPE > - 2 x JPEG DMA & Downscaler > - 2 x JPEG Encoder > - 1 x OFE > - 5 x RT CDM > - 3 x TPG Please describe the acronyms. > > Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> > Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org> > Signed-off-by: Hangxiang Ma <hangxiang.ma@oss.qualcomm.com> > --- > .../bindings/media/qcom,kaanapali-camss.yaml | 646 +++++++++++++++++++++ > 1 file changed, 646 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..3b54620e14c6 > --- /dev/null > +++ b/Documentation/devicetree/bindings/media/qcom,kaanapali-camss.yaml > @@ -0,0 +1,646 @@ > +# 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: > + Kaanapali camera subsystem includes submodules such as CSIPHY (CSI Physical layer) > + and CSID (CSI Decoder), which comply with the MIPI CSI2 protocol. > + > + The subsystem also integrates a set of real-time image processing engines and their > + associated configuration modules, as well as non-real-time engines. > + > + Additionally, it encompasses a test pattern generator (TPG) submodule. > + > +properties: > + compatible: > + const: qcom,kaanapali-camss > + > + reg: > + items: > + - description: Registers for CSID 0 > + - description: Registers for CSID 1 > + - description: Registers for CSID 2 > + - description: Registers for CSID Lite 0 > + - description: Registers for CSID Lite 1 > + - description: Registers for CSIPHY 0 > + - description: Registers for CSIPHY 1 > + - description: Registers for CSIPHY 2 > + - description: Registers for CSIPHY 3 > + - description: Registers for CSIPHY 4 > + - description: Registers for CSIPHY 5 > + - description: Registers for VFE (Video Front End) 0 > + - description: Registers for VFE 1 > + - description: Registers for VFE 2 > + - description: Registers for VFE Lite 0 > + - description: Registers for VFE Lite 1 > + - description: Registers for ICP (Imaging Control Processor) 0 > + - description: Registers for ICP 0 SYS > + - description: Registers for ICP 1 > + - description: Registers for ICP 1 SYS > + - description: Registers for IPE (Image Processing Engine) > + - description: Registers for JPEG DMA & Downscaler > + - description: Registers for JPEG Encoder > + - description: Registers for OFE (Offline Front End) > + - description: Registers for RT CDM (Camera Data Mover) 0 > + - description: Registers for RT CDM 1 > + - description: Registers for RT CDM 2 > + - description: Registers for RT CDM 3 > + - description: Registers for RT CDM 4 > + - description: Registers for TPG 0 > + - description: Registers for TPG 1 > + - description: Registers for TPG 2 > + > + 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 > + - const: icp0 > + - const: icp0_sys > + - const: icp1 > + - const: icp1_sys > + - const: ipe > + - const: jpeg_dma > + - const: jpeg_enc > + - const: ofe > + - const: rt_cdm0 > + - const: rt_cdm1 > + - const: rt_cdm2 > + - const: rt_cdm3 > + - const: rt_cdm4 > + - const: tpg0 > + - const: tpg1 > + - const: tpg2 > + > + clocks: > + maxItems: 60 > + > + 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: cpas_ahb > + - const: cpas_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_axi_hf This clock (and gcc_axi_sf below) still have the gcc_ prefix and GCC name. Why? It was pointed out in the previous review: clock names should be describing their purpose, not their source. > + - 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 > + - const: qdss_debug_xo > + - const: camnoc_ipe_nps > + - const: camnoc_ofe > + - const: gcc_axi_sf > + - const: icp0 > + - const: icp0_ahb > + - const: icp1 > + - const: icp1_ahb > + - const: ipe_nps > + - const: ipe_nps_ahb > + - const: ipe_nps_fast_ahb > + - const: ipe_pps > + - const: ipe_pps_fast_ahb > + - const: jpeg > + - const: ofe_ahb > + - const: ofe_anchor > + - const: ofe_anchor_fast_ahb > + - const: ofe_hdr > + - const: ofe_hdr_fast_ahb > + - const: ofe_main > + - const: ofe_main_fast_ahb > + - const: vfe0_bayer > + - const: vfe0_bayer_fast_ahb > + - const: vfe1_bayer > + - const: vfe1_bayer_fast_ahb > + - const: vfe2_bayer > + - const: vfe2_bayer_fast_ahb > + > + interrupts: > + maxItems: 30 > + > + interrupt-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 > + - const: camnoc_nrt > + - const: camnoc_rt > + - const: icp0 > + - const: icp1 > + - const: jpeg_dma > + - const: jpeg_enc > + - const: rt_cdm0 > + - const: rt_cdm1 > + - const: rt_cdm2 > + - const: rt_cdm3 > + - const: rt_cdm4 > + - const: tpg0 > + - const: tpg1 > + - const: tpg2 > + > + interconnects: > + maxItems: 4 > + > + interconnect-names: > + items: > + - const: ahb > + - const: hf_mnoc > + - const: sf_icp_mnoc > + - const: sf_mnoc You know... Failure to look around is a sin. What are the names of interconnects used by other devices? What do they actually describe? This is an absolute NAK. > + > + iommus: > + items: > + - description: VFE non-protected stream > + - description: ICP0 shared stream > + - description: ICP1 shared stream > + - description: IPE CDM non-protected stream > + - description: IPE non-protected stream > + - description: JPEG non-protected stream > + - description: OFE CDM non-protected stream > + - description: OFE non-protected stream > + - description: VFE / VFE Lite CDM non-protected stream This will map all IOMMUs to the same domain. Are you sure that this is what we want? Or do we wait for iommu-maps to be fixed? -- With best wishes Dmitry
On 12/8/2025 11:53 AM, Dmitry Baryshkov wrote: > On Mon, Dec 08, 2025 at 04:39:47AM -0800, Hangxiang Ma wrote: >> Add bindings for qcom,kaanapali-camss to support the Camera Subsystem >> (CAMSS) on the Qualcomm Kaanapali platform. >> >> The Kaanapali platform provides: >> >> - 3 x VFE, 5 RDI per VFE >> - 2 x VFE Lite, 4 RDI per VFE Lite >> - 3 x CSID >> - 2 x CSID Lite >> - 6 x CSIPHY >> - 2 x ICP >> - 1 x IPE >> - 2 x JPEG DMA & Downscaler >> - 2 x JPEG Encoder >> - 1 x OFE >> - 5 x RT CDM >> - 3 x TPG > Please describe the acronyms. Ack. >> Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> >> Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org> >> Signed-off-by: Hangxiang Ma <hangxiang.ma@oss.qualcomm.com> >> --- >> .../bindings/media/qcom,kaanapali-camss.yaml | 646 +++++++++++++++++++++ >> 1 file changed, 646 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..3b54620e14c6 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/media/qcom,kaanapali-camss.yaml >> @@ -0,0 +1,646 @@ >> +# 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: >> + Kaanapali camera subsystem includes submodules such as CSIPHY (CSI Physical layer) >> + and CSID (CSI Decoder), which comply with the MIPI CSI2 protocol. >> + >> + The subsystem also integrates a set of real-time image processing engines and their >> + associated configuration modules, as well as non-real-time engines. >> + >> + Additionally, it encompasses a test pattern generator (TPG) submodule. >> + >> +properties: >> + compatible: >> + const: qcom,kaanapali-camss >> + >> + reg: >> + items: >> + - description: Registers for CSID 0 >> + - description: Registers for CSID 1 >> + - description: Registers for CSID 2 >> + - description: Registers for CSID Lite 0 >> + - description: Registers for CSID Lite 1 >> + - description: Registers for CSIPHY 0 >> + - description: Registers for CSIPHY 1 >> + - description: Registers for CSIPHY 2 >> + - description: Registers for CSIPHY 3 >> + - description: Registers for CSIPHY 4 >> + - description: Registers for CSIPHY 5 >> + - description: Registers for VFE (Video Front End) 0 >> + - description: Registers for VFE 1 >> + - description: Registers for VFE 2 >> + - description: Registers for VFE Lite 0 >> + - description: Registers for VFE Lite 1 >> + - description: Registers for ICP (Imaging Control Processor) 0 >> + - description: Registers for ICP 0 SYS >> + - description: Registers for ICP 1 >> + - description: Registers for ICP 1 SYS >> + - description: Registers for IPE (Image Processing Engine) >> + - description: Registers for JPEG DMA & Downscaler >> + - description: Registers for JPEG Encoder >> + - description: Registers for OFE (Offline Front End) >> + - description: Registers for RT CDM (Camera Data Mover) 0 >> + - description: Registers for RT CDM 1 >> + - description: Registers for RT CDM 2 >> + - description: Registers for RT CDM 3 >> + - description: Registers for RT CDM 4 >> + - description: Registers for TPG 0 >> + - description: Registers for TPG 1 >> + - description: Registers for TPG 2 >> + >> + 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 >> + - const: icp0 >> + - const: icp0_sys >> + - const: icp1 >> + - const: icp1_sys >> + - const: ipe >> + - const: jpeg_dma >> + - const: jpeg_enc >> + - const: ofe >> + - const: rt_cdm0 >> + - const: rt_cdm1 >> + - const: rt_cdm2 >> + - const: rt_cdm3 >> + - const: rt_cdm4 >> + - const: tpg0 >> + - const: tpg1 >> + - const: tpg2 >> + >> + clocks: >> + maxItems: 60 >> + >> + 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: cpas_ahb >> + - const: cpas_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_axi_hf > This clock (and gcc_axi_sf below) still have the gcc_ prefix and GCC name. Why? > It was pointed out in the previous review: clock names should be > describing their purpose, not their source. Hi Dmitry, let me add a bit more detail on this clock. As confirmed by the HW team, the logic that runs based on this clock is still inside the CAMNOC_PDX, just that it is on the CX / MMNOC domain side. Do you think "axi_hf_cx" and "axi_sf_cx" makes sense? >> + - 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 >> + - const: qdss_debug_xo >> + - const: camnoc_ipe_nps >> + - const: camnoc_ofe >> + - const: gcc_axi_sf >> + - const: icp0 >> + - const: icp0_ahb >> + - const: icp1 >> + - const: icp1_ahb >> + - const: ipe_nps >> + - const: ipe_nps_ahb >> + - const: ipe_nps_fast_ahb >> + - const: ipe_pps >> + - const: ipe_pps_fast_ahb >> + - const: jpeg >> + - const: ofe_ahb >> + - const: ofe_anchor >> + - const: ofe_anchor_fast_ahb >> + - const: ofe_hdr >> + - const: ofe_hdr_fast_ahb >> + - const: ofe_main >> + - const: ofe_main_fast_ahb >> + - const: vfe0_bayer >> + - const: vfe0_bayer_fast_ahb >> + - const: vfe1_bayer >> + - const: vfe1_bayer_fast_ahb >> + - const: vfe2_bayer >> + - const: vfe2_bayer_fast_ahb >> + >> + interrupts: >> + maxItems: 30 >> + >> + interrupt-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 >> + - const: camnoc_nrt >> + - const: camnoc_rt >> + - const: icp0 >> + - const: icp1 >> + - const: jpeg_dma >> + - const: jpeg_enc >> + - const: rt_cdm0 >> + - const: rt_cdm1 >> + - const: rt_cdm2 >> + - const: rt_cdm3 >> + - const: rt_cdm4 >> + - const: tpg0 >> + - const: tpg1 >> + - const: tpg2 >> + >> + interconnects: >> + maxItems: 4 >> + >> + interconnect-names: >> + items: >> + - const: ahb >> + - const: hf_mnoc >> + - const: sf_icp_mnoc >> + - const: sf_mnoc > You know... Failure to look around is a sin. What are the names of > interconnects used by other devices? What do they actually describe? > > This is an absolute NAK. Please feel free to correct me here but, a couple things. 1. This is consistent with Documentation/devicetree/bindings/media/qcom,qcm2290-camss.yaml. no? 2. If you are referring to some other targets that use, "cam_" prefix, we may not need that , isn't it? If we look at these interconnects from camera side, as you advised for other things like this? > >> + >> + iommus: >> + items: >> + - description: VFE non-protected stream >> + - description: ICP0 shared stream >> + - description: ICP1 shared stream >> + - description: IPE CDM non-protected stream >> + - description: IPE non-protected stream >> + - description: JPEG non-protected stream >> + - description: OFE CDM non-protected stream >> + - description: OFE non-protected stream >> + - description: VFE / VFE Lite CDM non-protected stream > This will map all IOMMUs to the same domain. Are you sure that this is > what we want? Or do we wait for iommu-maps to be fixed?
On Mon, Dec 08, 2025 at 01:03:06PM -0800, Vijay Kumar Tumati wrote:
>
> On 12/8/2025 11:53 AM, Dmitry Baryshkov wrote:
> > On Mon, Dec 08, 2025 at 04:39:47AM -0800, Hangxiang Ma wrote:
> > > Add bindings for qcom,kaanapali-camss to support the Camera Subsystem
> > > (CAMSS) on the Qualcomm Kaanapali platform.
> > >
> > > The Kaanapali platform provides:
> > >
> > > - 3 x VFE, 5 RDI per VFE
> > > - 2 x VFE Lite, 4 RDI per VFE Lite
> > > - 3 x CSID
> > > - 2 x CSID Lite
> > > - 6 x CSIPHY
> > > - 2 x ICP
> > > - 1 x IPE
> > > - 2 x JPEG DMA & Downscaler
> > > - 2 x JPEG Encoder
> > > - 1 x OFE
> > > - 5 x RT CDM
> > > - 3 x TPG
> > Please describe the acronyms.
> Ack.
> > > Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> > > Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>
> > > Signed-off-by: Hangxiang Ma <hangxiang.ma@oss.qualcomm.com>
> > > ---
> > > .../bindings/media/qcom,kaanapali-camss.yaml | 646 +++++++++++++++++++++
> > > 1 file changed, 646 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..3b54620e14c6
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/media/qcom,kaanapali-camss.yaml
> > > @@ -0,0 +1,646 @@
> > > +# 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:
> > > + Kaanapali camera subsystem includes submodules such as CSIPHY (CSI Physical layer)
> > > + and CSID (CSI Decoder), which comply with the MIPI CSI2 protocol.
> > > +
> > > + The subsystem also integrates a set of real-time image processing engines and their
> > > + associated configuration modules, as well as non-real-time engines.
> > > +
> > > + Additionally, it encompasses a test pattern generator (TPG) submodule.
> > > +
> > > +properties:
> > > + compatible:
> > > + const: qcom,kaanapali-camss
> > > +
> > > + reg:
> > > + items:
> > > + - description: Registers for CSID 0
> > > + - description: Registers for CSID 1
> > > + - description: Registers for CSID 2
> > > + - description: Registers for CSID Lite 0
> > > + - description: Registers for CSID Lite 1
> > > + - description: Registers for CSIPHY 0
> > > + - description: Registers for CSIPHY 1
> > > + - description: Registers for CSIPHY 2
> > > + - description: Registers for CSIPHY 3
> > > + - description: Registers for CSIPHY 4
> > > + - description: Registers for CSIPHY 5
> > > + - description: Registers for VFE (Video Front End) 0
> > > + - description: Registers for VFE 1
> > > + - description: Registers for VFE 2
> > > + - description: Registers for VFE Lite 0
> > > + - description: Registers for VFE Lite 1
> > > + - description: Registers for ICP (Imaging Control Processor) 0
> > > + - description: Registers for ICP 0 SYS
> > > + - description: Registers for ICP 1
> > > + - description: Registers for ICP 1 SYS
> > > + - description: Registers for IPE (Image Processing Engine)
> > > + - description: Registers for JPEG DMA & Downscaler
> > > + - description: Registers for JPEG Encoder
> > > + - description: Registers for OFE (Offline Front End)
> > > + - description: Registers for RT CDM (Camera Data Mover) 0
> > > + - description: Registers for RT CDM 1
> > > + - description: Registers for RT CDM 2
> > > + - description: Registers for RT CDM 3
> > > + - description: Registers for RT CDM 4
> > > + - description: Registers for TPG 0
> > > + - description: Registers for TPG 1
> > > + - description: Registers for TPG 2
> > > +
> > > + 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
> > > + - const: icp0
> > > + - const: icp0_sys
> > > + - const: icp1
> > > + - const: icp1_sys
> > > + - const: ipe
> > > + - const: jpeg_dma
> > > + - const: jpeg_enc
> > > + - const: ofe
> > > + - const: rt_cdm0
> > > + - const: rt_cdm1
> > > + - const: rt_cdm2
> > > + - const: rt_cdm3
> > > + - const: rt_cdm4
> > > + - const: tpg0
> > > + - const: tpg1
> > > + - const: tpg2
> > > +
> > > + clocks:
> > > + maxItems: 60
> > > +
> > > + 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: cpas_ahb
> > > + - const: cpas_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_axi_hf
> > This clock (and gcc_axi_sf below) still have the gcc_ prefix and GCC name. Why?
> > It was pointed out in the previous review: clock names should be
> > describing their purpose, not their source.
> Hi Dmitry, let me add a bit more detail on this clock. As confirmed by the
> HW team, the logic that runs based on this clock is still inside the
> CAMNOC_PDX, just that it is on the CX / MMNOC domain side. Do you think
> "axi_hf_cx" and "axi_sf_cx" makes sense?
Why? You are again describing the source. What is the function of?
bus_hf / bus_sf?
> > > + - 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
> > > + - const: qdss_debug_xo
> > > + - const: camnoc_ipe_nps
> > > + - const: camnoc_ofe
> > > + - const: gcc_axi_sf
> > > + - const: icp0
> > > + - const: icp0_ahb
> > > + - const: icp1
> > > + - const: icp1_ahb
> > > + - const: ipe_nps
> > > + - const: ipe_nps_ahb
> > > + - const: ipe_nps_fast_ahb
> > > + - const: ipe_pps
> > > + - const: ipe_pps_fast_ahb
> > > + - const: jpeg
> > > + - const: ofe_ahb
> > > + - const: ofe_anchor
> > > + - const: ofe_anchor_fast_ahb
> > > + - const: ofe_hdr
> > > + - const: ofe_hdr_fast_ahb
> > > + - const: ofe_main
> > > + - const: ofe_main_fast_ahb
> > > + - const: vfe0_bayer
> > > + - const: vfe0_bayer_fast_ahb
> > > + - const: vfe1_bayer
> > > + - const: vfe1_bayer_fast_ahb
> > > + - const: vfe2_bayer
> > > + - const: vfe2_bayer_fast_ahb
> > > +
> > > + interrupts:
> > > + maxItems: 30
> > > +
> > > + interrupt-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
> > > + - const: camnoc_nrt
> > > + - const: camnoc_rt
> > > + - const: icp0
> > > + - const: icp1
> > > + - const: jpeg_dma
> > > + - const: jpeg_enc
> > > + - const: rt_cdm0
> > > + - const: rt_cdm1
> > > + - const: rt_cdm2
> > > + - const: rt_cdm3
> > > + - const: rt_cdm4
> > > + - const: tpg0
> > > + - const: tpg1
> > > + - const: tpg2
> > > +
> > > + interconnects:
> > > + maxItems: 4
> > > +
> > > + interconnect-names:
> > > + items:
> > > + - const: ahb
> > > + - const: hf_mnoc
> > > + - const: sf_icp_mnoc
> > > + - const: sf_mnoc
> > You know... Failure to look around is a sin. What are the names of
> > interconnects used by other devices? What do they actually describe?
> >
> > This is an absolute NAK.
>
> Please feel free to correct me here but, a couple things.
>
> 1. This is consistent with
> Documentation/devicetree/bindings/media/qcom,qcm2290-camss.yaml. no?
I see that nobody noticed an issue with Agatti, Lemans and Monaco
bindings (Krzysztof?)
Usually interconnect names describe the blocks that are connected. Here
are the top results of a quick git grep of interconnect names through
arch/arm64/dts/qcom:
729 "qup-core",
717 "qup-config",
457 "qup-memory",
41 "usb-ddr",
41 "apps-usb",
39 "pcie-mem",
39 "cpu-pcie",
28 "sdhc-ddr",
28 "cpu-sdhc",
28 "cpu-cfg",
24 "mdp0-mem",
17 "memory",
14 "ufs-ddr",
14 "mdp1-mem",
14 "cpu-ufs",
13 "video-mem",
13 "gfx-mem",
I hope this gives you a pointer on how to name the interconnects.
>
> 2. If you are referring to some other targets that use, "cam_" prefix, we
> may not need that , isn't it? If we look at these interconnects from camera
> side, as you advised for other things like this?
See above.
>
> >
> > > +
> > > + iommus:
> > > + items:
> > > + - description: VFE non-protected stream
> > > + - description: ICP0 shared stream
> > > + - description: ICP1 shared stream
> > > + - description: IPE CDM non-protected stream
> > > + - description: IPE non-protected stream
> > > + - description: JPEG non-protected stream
> > > + - description: OFE CDM non-protected stream
> > > + - description: OFE non-protected stream
> > > + - description: VFE / VFE Lite CDM non-protected stream
> > This will map all IOMMUs to the same domain. Are you sure that this is
> > what we want? Or do we wait for iommu-maps to be fixed?
--
With best wishes
Dmitry
On 12/8/2025 2:48 PM, Dmitry Baryshkov wrote: > On Mon, Dec 08, 2025 at 01:03:06PM -0800, Vijay Kumar Tumati wrote: >> On 12/8/2025 11:53 AM, Dmitry Baryshkov wrote: >>> On Mon, Dec 08, 2025 at 04:39:47AM -0800, Hangxiang Ma wrote: >>>> Add bindings for qcom,kaanapali-camss to support the Camera Subsystem >>>> (CAMSS) on the Qualcomm Kaanapali platform. >>>> >>>> The Kaanapali platform provides: >>>> >>>> - 3 x VFE, 5 RDI per VFE >>>> - 2 x VFE Lite, 4 RDI per VFE Lite >>>> - 3 x CSID >>>> - 2 x CSID Lite >>>> - 6 x CSIPHY >>>> - 2 x ICP >>>> - 1 x IPE >>>> - 2 x JPEG DMA & Downscaler >>>> - 2 x JPEG Encoder >>>> - 1 x OFE >>>> - 5 x RT CDM >>>> - 3 x TPG >>> Please describe the acronyms. >> Ack. >>>> Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> >>>> Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org> >>>> Signed-off-by: Hangxiang Ma <hangxiang.ma@oss.qualcomm.com> >>>> --- >>>> .../bindings/media/qcom,kaanapali-camss.yaml | 646 +++++++++++++++++++++ >>>> 1 file changed, 646 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..3b54620e14c6 >>>> --- /dev/null >>>> +++ b/Documentation/devicetree/bindings/media/qcom,kaanapali-camss.yaml >>>> @@ -0,0 +1,646 @@ >>>> +# 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: >>>> + Kaanapali camera subsystem includes submodules such as CSIPHY (CSI Physical layer) >>>> + and CSID (CSI Decoder), which comply with the MIPI CSI2 protocol. >>>> + >>>> + The subsystem also integrates a set of real-time image processing engines and their >>>> + associated configuration modules, as well as non-real-time engines. >>>> + >>>> + Additionally, it encompasses a test pattern generator (TPG) submodule. >>>> + >>>> +properties: >>>> + compatible: >>>> + const: qcom,kaanapali-camss >>>> + >>>> + reg: >>>> + items: >>>> + - description: Registers for CSID 0 >>>> + - description: Registers for CSID 1 >>>> + - description: Registers for CSID 2 >>>> + - description: Registers for CSID Lite 0 >>>> + - description: Registers for CSID Lite 1 >>>> + - description: Registers for CSIPHY 0 >>>> + - description: Registers for CSIPHY 1 >>>> + - description: Registers for CSIPHY 2 >>>> + - description: Registers for CSIPHY 3 >>>> + - description: Registers for CSIPHY 4 >>>> + - description: Registers for CSIPHY 5 >>>> + - description: Registers for VFE (Video Front End) 0 >>>> + - description: Registers for VFE 1 >>>> + - description: Registers for VFE 2 >>>> + - description: Registers for VFE Lite 0 >>>> + - description: Registers for VFE Lite 1 >>>> + - description: Registers for ICP (Imaging Control Processor) 0 >>>> + - description: Registers for ICP 0 SYS >>>> + - description: Registers for ICP 1 >>>> + - description: Registers for ICP 1 SYS >>>> + - description: Registers for IPE (Image Processing Engine) >>>> + - description: Registers for JPEG DMA & Downscaler >>>> + - description: Registers for JPEG Encoder >>>> + - description: Registers for OFE (Offline Front End) >>>> + - description: Registers for RT CDM (Camera Data Mover) 0 >>>> + - description: Registers for RT CDM 1 >>>> + - description: Registers for RT CDM 2 >>>> + - description: Registers for RT CDM 3 >>>> + - description: Registers for RT CDM 4 >>>> + - description: Registers for TPG 0 >>>> + - description: Registers for TPG 1 >>>> + - description: Registers for TPG 2 >>>> + >>>> + 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 >>>> + - const: icp0 >>>> + - const: icp0_sys >>>> + - const: icp1 >>>> + - const: icp1_sys >>>> + - const: ipe >>>> + - const: jpeg_dma >>>> + - const: jpeg_enc >>>> + - const: ofe >>>> + - const: rt_cdm0 >>>> + - const: rt_cdm1 >>>> + - const: rt_cdm2 >>>> + - const: rt_cdm3 >>>> + - const: rt_cdm4 >>>> + - const: tpg0 >>>> + - const: tpg1 >>>> + - const: tpg2 >>>> + >>>> + clocks: >>>> + maxItems: 60 >>>> + >>>> + 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: cpas_ahb >>>> + - const: cpas_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_axi_hf >>> This clock (and gcc_axi_sf below) still have the gcc_ prefix and GCC name. Why? >>> It was pointed out in the previous review: clock names should be >>> describing their purpose, not their source. >> Hi Dmitry, let me add a bit more detail on this clock. As confirmed by the >> HW team, the logic that runs based on this clock is still inside the >> CAMNOC_PDX, just that it is on the CX / MMNOC domain side. Do you think >> "axi_hf_cx" and "axi_sf_cx" makes sense? > Why? You are again describing the source. What is the function of? > bus_hf / bus_sf? In what I proposed, axi - represents that we are talking about the axi bus from camera (against ahb bus). hf - hf wrapper cx - logic on the CX side of the bus in CAMNOC. If you think that 'bus' (even looking from camera client side) by default means AXI bus and 'hf' and 'sf' implicitly represent the CX side (which, kind of, in the current design), then yes, "bus_hf" and "bus_sf" makes sense. Do you advise us to go ahead with these? >>>> + - 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 >>>> + - const: qdss_debug_xo >>>> + - const: camnoc_ipe_nps >>>> + - const: camnoc_ofe >>>> + - const: gcc_axi_sf >>>> + - const: icp0 >>>> + - const: icp0_ahb >>>> + - const: icp1 >>>> + - const: icp1_ahb >>>> + - const: ipe_nps >>>> + - const: ipe_nps_ahb >>>> + - const: ipe_nps_fast_ahb >>>> + - const: ipe_pps >>>> + - const: ipe_pps_fast_ahb >>>> + - const: jpeg >>>> + - const: ofe_ahb >>>> + - const: ofe_anchor >>>> + - const: ofe_anchor_fast_ahb >>>> + - const: ofe_hdr >>>> + - const: ofe_hdr_fast_ahb >>>> + - const: ofe_main >>>> + - const: ofe_main_fast_ahb >>>> + - const: vfe0_bayer >>>> + - const: vfe0_bayer_fast_ahb >>>> + - const: vfe1_bayer >>>> + - const: vfe1_bayer_fast_ahb >>>> + - const: vfe2_bayer >>>> + - const: vfe2_bayer_fast_ahb >>>> + >>>> + interrupts: >>>> + maxItems: 30 >>>> + >>>> + interrupt-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 >>>> + - const: camnoc_nrt >>>> + - const: camnoc_rt >>>> + - const: icp0 >>>> + - const: icp1 >>>> + - const: jpeg_dma >>>> + - const: jpeg_enc >>>> + - const: rt_cdm0 >>>> + - const: rt_cdm1 >>>> + - const: rt_cdm2 >>>> + - const: rt_cdm3 >>>> + - const: rt_cdm4 >>>> + - const: tpg0 >>>> + - const: tpg1 >>>> + - const: tpg2 >>>> + >>>> + interconnects: >>>> + maxItems: 4 >>>> + >>>> + interconnect-names: >>>> + items: >>>> + - const: ahb >>>> + - const: hf_mnoc >>>> + - const: sf_icp_mnoc >>>> + - const: sf_mnoc >>> You know... Failure to look around is a sin. What are the names of >>> interconnects used by other devices? What do they actually describe? >>> >>> This is an absolute NAK. >> Please feel free to correct me here but, a couple things. >> >> 1. This is consistent with >> Documentation/devicetree/bindings/media/qcom,qcm2290-camss.yaml. no? > I see that nobody noticed an issue with Agatti, Lemans and Monaco > bindings (Krzysztof?) > > Usually interconnect names describe the blocks that are connected. Here > are the top results of a quick git grep of interconnect names through > arch/arm64/dts/qcom: > > 729 "qup-core", > 717 "qup-config", > 457 "qup-memory", > 41 "usb-ddr", > 41 "apps-usb", > 39 "pcie-mem", > 39 "cpu-pcie", > 28 "sdhc-ddr", > 28 "cpu-sdhc", > 28 "cpu-cfg", > 24 "mdp0-mem", > 17 "memory", > 14 "ufs-ddr", > 14 "mdp1-mem", > 14 "cpu-ufs", > 13 "video-mem", > 13 "gfx-mem", > > I hope this gives you a pointer on how to name the interconnects. > >> 2. If you are referring to some other targets that use, "cam_" prefix, we >> may not need that , isn't it? If we look at these interconnects from camera >> side, as you advised for other things like this? > See above. I see, so the names cam-cfg, cam-hf-mem, cam-sf-mem, cam-sf-icp-mem should be ok? Or the other option, go exactly like Documentation/devicetree/bindings/media/qcom,sc8280xp-camss.yaml. What would you advise? > >>>> + >>>> + iommus: >>>> + items: >>>> + - description: VFE non-protected stream >>>> + - description: ICP0 shared stream >>>> + - description: ICP1 shared stream >>>> + - description: IPE CDM non-protected stream >>>> + - description: IPE non-protected stream >>>> + - description: JPEG non-protected stream >>>> + - description: OFE CDM non-protected stream >>>> + - description: OFE non-protected stream >>>> + - description: VFE / VFE Lite CDM non-protected stream >>> This will map all IOMMUs to the same domain. Are you sure that this is >>> what we want? Or do we wait for iommu-maps to be fixed? Thanks, Vijay.
On 12/8/2025 3:21 PM, Vijay Kumar Tumati wrote: > > On 12/8/2025 2:48 PM, Dmitry Baryshkov wrote: >> On Mon, Dec 08, 2025 at 01:03:06PM -0800, Vijay Kumar Tumati wrote: >>> On 12/8/2025 11:53 AM, Dmitry Baryshkov wrote: >>>> On Mon, Dec 08, 2025 at 04:39:47AM -0800, Hangxiang Ma wrote: >>>>> Add bindings for qcom,kaanapali-camss to support the Camera Subsystem >>>>> (CAMSS) on the Qualcomm Kaanapali platform. >>>>> >>>>> The Kaanapali platform provides: >>>>> >>>>> - 3 x VFE, 5 RDI per VFE >>>>> - 2 x VFE Lite, 4 RDI per VFE Lite >>>>> - 3 x CSID >>>>> - 2 x CSID Lite >>>>> - 6 x CSIPHY >>>>> - 2 x ICP >>>>> - 1 x IPE >>>>> - 2 x JPEG DMA & Downscaler >>>>> - 2 x JPEG Encoder >>>>> - 1 x OFE >>>>> - 5 x RT CDM >>>>> - 3 x TPG >>>> Please describe the acronyms. >>> Ack. >>>>> Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> >>>>> Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org> >>>>> Signed-off-by: Hangxiang Ma <hangxiang.ma@oss.qualcomm.com> >>>>> --- >>>>> .../bindings/media/qcom,kaanapali-camss.yaml | 646 >>>>> +++++++++++++++++++++ >>>>> 1 file changed, 646 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..3b54620e14c6 >>>>> --- /dev/null >>>>> +++ >>>>> b/Documentation/devicetree/bindings/media/qcom,kaanapali-camss.yaml >>>>> @@ -0,0 +1,646 @@ >>>>> +# 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: >>>>> + Kaanapali camera subsystem includes submodules such as CSIPHY >>>>> (CSI Physical layer) >>>>> + and CSID (CSI Decoder), which comply with the MIPI CSI2 protocol. >>>>> + >>>>> + The subsystem also integrates a set of real-time image >>>>> processing engines and their >>>>> + associated configuration modules, as well as non-real-time >>>>> engines. >>>>> + >>>>> + Additionally, it encompasses a test pattern generator (TPG) >>>>> submodule. >>>>> + >>>>> +properties: >>>>> + compatible: >>>>> + const: qcom,kaanapali-camss >>>>> + >>>>> + reg: >>>>> + items: >>>>> + - description: Registers for CSID 0 >>>>> + - description: Registers for CSID 1 >>>>> + - description: Registers for CSID 2 >>>>> + - description: Registers for CSID Lite 0 >>>>> + - description: Registers for CSID Lite 1 >>>>> + - description: Registers for CSIPHY 0 >>>>> + - description: Registers for CSIPHY 1 >>>>> + - description: Registers for CSIPHY 2 >>>>> + - description: Registers for CSIPHY 3 >>>>> + - description: Registers for CSIPHY 4 >>>>> + - description: Registers for CSIPHY 5 >>>>> + - description: Registers for VFE (Video Front End) 0 >>>>> + - description: Registers for VFE 1 >>>>> + - description: Registers for VFE 2 >>>>> + - description: Registers for VFE Lite 0 >>>>> + - description: Registers for VFE Lite 1 >>>>> + - description: Registers for ICP (Imaging Control Processor) 0 >>>>> + - description: Registers for ICP 0 SYS >>>>> + - description: Registers for ICP 1 >>>>> + - description: Registers for ICP 1 SYS >>>>> + - description: Registers for IPE (Image Processing Engine) >>>>> + - description: Registers for JPEG DMA & Downscaler >>>>> + - description: Registers for JPEG Encoder >>>>> + - description: Registers for OFE (Offline Front End) >>>>> + - description: Registers for RT CDM (Camera Data Mover) 0 >>>>> + - description: Registers for RT CDM 1 >>>>> + - description: Registers for RT CDM 2 >>>>> + - description: Registers for RT CDM 3 >>>>> + - description: Registers for RT CDM 4 >>>>> + - description: Registers for TPG 0 >>>>> + - description: Registers for TPG 1 >>>>> + - description: Registers for TPG 2 >>>>> + >>>>> + 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 >>>>> + - const: icp0 >>>>> + - const: icp0_sys >>>>> + - const: icp1 >>>>> + - const: icp1_sys >>>>> + - const: ipe >>>>> + - const: jpeg_dma >>>>> + - const: jpeg_enc >>>>> + - const: ofe >>>>> + - const: rt_cdm0 >>>>> + - const: rt_cdm1 >>>>> + - const: rt_cdm2 >>>>> + - const: rt_cdm3 >>>>> + - const: rt_cdm4 >>>>> + - const: tpg0 >>>>> + - const: tpg1 >>>>> + - const: tpg2 >>>>> + >>>>> + clocks: >>>>> + maxItems: 60 >>>>> + >>>>> + 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: cpas_ahb >>>>> + - const: cpas_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_axi_hf >>>> This clock (and gcc_axi_sf below) still have the gcc_ prefix and >>>> GCC name. Why? >>>> It was pointed out in the previous review: clock names should be >>>> describing their purpose, not their source. >>> Hi Dmitry, let me add a bit more detail on this clock. As confirmed >>> by the >>> HW team, the logic that runs based on this clock is still inside the >>> CAMNOC_PDX, just that it is on the CX / MMNOC domain side. Do you think >>> "axi_hf_cx" and "axi_sf_cx" makes sense? >> Why? You are again describing the source. What is the function of? >> bus_hf / bus_sf? > > In what I proposed, > > axi - represents that we are talking about the axi bus from camera > (against ahb bus). > > hf - hf wrapper > > cx - logic on the CX side of the bus in CAMNOC. > > If you think that 'bus' (even looking from camera client side) by > default means AXI bus and 'hf' and 'sf' implicitly represent the CX > side (which, kind of, in the current design), then yes, "bus_hf" and > "bus_sf" makes sense. Do you advise us to go ahead with these? > Ok, we will go ahead with "bus_hf" and "bus_sf". Hi @Bryan and others, please let us know if you have any concerns with this. >>>>> + - 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 >>>>> + - const: qdss_debug_xo >>>>> + - const: camnoc_ipe_nps >>>>> + - const: camnoc_ofe >>>>> + - const: gcc_axi_sf >>>>> + - const: icp0 >>>>> + - const: icp0_ahb >>>>> + - const: icp1 >>>>> + - const: icp1_ahb >>>>> + - const: ipe_nps >>>>> + - const: ipe_nps_ahb >>>>> + - const: ipe_nps_fast_ahb >>>>> + - const: ipe_pps >>>>> + - const: ipe_pps_fast_ahb >>>>> + - const: jpeg >>>>> + - const: ofe_ahb >>>>> + - const: ofe_anchor >>>>> + - const: ofe_anchor_fast_ahb >>>>> + - const: ofe_hdr >>>>> + - const: ofe_hdr_fast_ahb >>>>> + - const: ofe_main >>>>> + - const: ofe_main_fast_ahb >>>>> + - const: vfe0_bayer >>>>> + - const: vfe0_bayer_fast_ahb >>>>> + - const: vfe1_bayer >>>>> + - const: vfe1_bayer_fast_ahb >>>>> + - const: vfe2_bayer >>>>> + - const: vfe2_bayer_fast_ahb >>>>> + >>>>> + interrupts: >>>>> + maxItems: 30 >>>>> + >>>>> + interrupt-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 >>>>> + - const: camnoc_nrt >>>>> + - const: camnoc_rt >>>>> + - const: icp0 >>>>> + - const: icp1 >>>>> + - const: jpeg_dma >>>>> + - const: jpeg_enc >>>>> + - const: rt_cdm0 >>>>> + - const: rt_cdm1 >>>>> + - const: rt_cdm2 >>>>> + - const: rt_cdm3 >>>>> + - const: rt_cdm4 >>>>> + - const: tpg0 >>>>> + - const: tpg1 >>>>> + - const: tpg2 >>>>> + >>>>> + interconnects: >>>>> + maxItems: 4 >>>>> + >>>>> + interconnect-names: >>>>> + items: >>>>> + - const: ahb >>>>> + - const: hf_mnoc >>>>> + - const: sf_icp_mnoc >>>>> + - const: sf_mnoc >>>> You know... Failure to look around is a sin. What are the names of >>>> interconnects used by other devices? What do they actually describe? >>>> >>>> This is an absolute NAK. >>> Please feel free to correct me here but, a couple things. >>> >>> 1. This is consistent with >>> Documentation/devicetree/bindings/media/qcom,qcm2290-camss.yaml. no? >> I see that nobody noticed an issue with Agatti, Lemans and Monaco >> bindings (Krzysztof?) >> >> Usually interconnect names describe the blocks that are connected. Here >> are the top results of a quick git grep of interconnect names through >> arch/arm64/dts/qcom: >> >> 729 "qup-core", >> 717 "qup-config", >> 457 "qup-memory", >> 41 "usb-ddr", >> 41 "apps-usb", >> 39 "pcie-mem", >> 39 "cpu-pcie", >> 28 "sdhc-ddr", >> 28 "cpu-sdhc", >> 28 "cpu-cfg", >> 24 "mdp0-mem", >> 17 "memory", >> 14 "ufs-ddr", >> 14 "mdp1-mem", >> 14 "cpu-ufs", >> 13 "video-mem", >> 13 "gfx-mem", >> >> I hope this gives you a pointer on how to name the interconnects. >> >>> 2. If you are referring to some other targets that use, "cam_" >>> prefix, we >>> may not need that , isn't it? If we look at these interconnects from >>> camera >>> side, as you advised for other things like this? >> See above. > > I see, so the names cam-cfg, cam-hf-mem, cam-sf-mem, cam-sf-icp-mem > should be ok? > > Or the other option, go exactly like > Documentation/devicetree/bindings/media/qcom,sc8280xp-camss.yaml. > > What would you advise? > To keep it consistent with the previous generations and still represent the block name, we will go ahead with the style in qcom,sc8280xp-camss.yaml. If anyone has any concerns, please do let us know. >> >>>>> + >>>>> + iommus: >>>>> + items: >>>>> + - description: VFE non-protected stream >>>>> + - description: ICP0 shared stream >>>>> + - description: ICP1 shared stream >>>>> + - description: IPE CDM non-protected stream >>>>> + - description: IPE non-protected stream >>>>> + - description: JPEG non-protected stream >>>>> + - description: OFE CDM non-protected stream >>>>> + - description: OFE non-protected stream >>>>> + - description: VFE / VFE Lite CDM non-protected stream >>>> This will map all IOMMUs to the same domain. Are you sure that this is >>>> what we want? Or do we wait for iommu-maps to be fixed? > Yes, when it is available, we can start using iommu-maps to create separate context banks. > Thanks, > > Vijay. >
On Wed, Dec 10, 2025 at 09:50:51AM -0800, Vijay Kumar Tumati wrote: > > On 12/8/2025 3:21 PM, Vijay Kumar Tumati wrote: > > > > On 12/8/2025 2:48 PM, Dmitry Baryshkov wrote: > > > On Mon, Dec 08, 2025 at 01:03:06PM -0800, Vijay Kumar Tumati wrote: > > > > On 12/8/2025 11:53 AM, Dmitry Baryshkov wrote: > > > > > > + interconnects: > > > > > > + maxItems: 4 > > > > > > + > > > > > > + interconnect-names: > > > > > > + items: > > > > > > + - const: ahb > > > > > > + - const: hf_mnoc > > > > > > + - const: sf_icp_mnoc > > > > > > + - const: sf_mnoc > > > > > You know... Failure to look around is a sin. What are the names of > > > > > interconnects used by other devices? What do they actually describe? > > > > > > > > > > This is an absolute NAK. > > > > Please feel free to correct me here but, a couple things. > > > > > > > > 1. This is consistent with > > > > Documentation/devicetree/bindings/media/qcom,qcm2290-camss.yaml. no? > > > I see that nobody noticed an issue with Agatti, Lemans and Monaco > > > bindings (Krzysztof?) > > > > > > Usually interconnect names describe the blocks that are connected. Here > > > are the top results of a quick git grep of interconnect names through > > > arch/arm64/dts/qcom: > > > > > > 729 "qup-core", > > > 717 "qup-config", > > > 457 "qup-memory", > > > 41 "usb-ddr", > > > 41 "apps-usb", > > > 39 "pcie-mem", > > > 39 "cpu-pcie", > > > 28 "sdhc-ddr", > > > 28 "cpu-sdhc", > > > 28 "cpu-cfg", > > > 24 "mdp0-mem", > > > 17 "memory", > > > 14 "ufs-ddr", > > > 14 "mdp1-mem", > > > 14 "cpu-ufs", > > > 13 "video-mem", > > > 13 "gfx-mem", > > > > > > I hope this gives you a pointer on how to name the interconnects. > > > > > > > 2. If you are referring to some other targets that use, "cam_" > > > > prefix, we > > > > may not need that , isn't it? If we look at these interconnects > > > > from camera > > > > side, as you advised for other things like this? > > > See above. > > > > I see, so the names cam-cfg, cam-hf-mem, cam-sf-mem, cam-sf-icp-mem > > should be ok? > > > > Or the other option, go exactly like > > Documentation/devicetree/bindings/media/qcom,sc8280xp-camss.yaml. > > > > What would you advise? > > > To keep it consistent with the previous generations and still represent the > block name, we will go ahead with the style in qcom,sc8280xp-camss.yaml. If > anyone has any concerns, please do let us know. Krzysztof, Bryan, your opinion? My preference would be to start using sensible names, but I wouldn't enforce that. > > > > > > > > > + > > > > > > + iommus: > > > > > > + items: > > > > > > + - description: VFE non-protected stream > > > > > > + - description: ICP0 shared stream > > > > > > + - description: ICP1 shared stream > > > > > > + - description: IPE CDM non-protected stream > > > > > > + - description: IPE non-protected stream > > > > > > + - description: JPEG non-protected stream > > > > > > + - description: OFE CDM non-protected stream > > > > > > + - description: OFE non-protected stream > > > > > > + - description: VFE / VFE Lite CDM non-protected stream > > > > > This will map all IOMMUs to the same domain. Are you sure that this is > > > > > what we want? Or do we wait for iommu-maps to be fixed? > > > Yes, when it is available, we can start using iommu-maps to create separate > context banks. It would be necessary to justify removing items from the list. Wouldn't it be better to map only necessary SIDs now and add other later once we have iommu-maps? -- With best wishes Dmitry
On 12/10/2025 11:25 AM, Dmitry Baryshkov wrote: > On Wed, Dec 10, 2025 at 09:50:51AM -0800, Vijay Kumar Tumati wrote: >> On 12/8/2025 3:21 PM, Vijay Kumar Tumati wrote: >>> On 12/8/2025 2:48 PM, Dmitry Baryshkov wrote: >>>> On Mon, Dec 08, 2025 at 01:03:06PM -0800, Vijay Kumar Tumati wrote: >>>>> On 12/8/2025 11:53 AM, Dmitry Baryshkov wrote: >>>>>>> + interconnects: >>>>>>> + maxItems: 4 >>>>>>> + >>>>>>> + interconnect-names: >>>>>>> + items: >>>>>>> + - const: ahb >>>>>>> + - const: hf_mnoc >>>>>>> + - const: sf_icp_mnoc >>>>>>> + - const: sf_mnoc >>>>>> You know... Failure to look around is a sin. What are the names of >>>>>> interconnects used by other devices? What do they actually describe? >>>>>> >>>>>> This is an absolute NAK. >>>>> Please feel free to correct me here but, a couple things. >>>>> >>>>> 1. This is consistent with >>>>> Documentation/devicetree/bindings/media/qcom,qcm2290-camss.yaml. no? >>>> I see that nobody noticed an issue with Agatti, Lemans and Monaco >>>> bindings (Krzysztof?) >>>> >>>> Usually interconnect names describe the blocks that are connected. Here >>>> are the top results of a quick git grep of interconnect names through >>>> arch/arm64/dts/qcom: >>>> >>>> 729 "qup-core", >>>> 717 "qup-config", >>>> 457 "qup-memory", >>>> 41 "usb-ddr", >>>> 41 "apps-usb", >>>> 39 "pcie-mem", >>>> 39 "cpu-pcie", >>>> 28 "sdhc-ddr", >>>> 28 "cpu-sdhc", >>>> 28 "cpu-cfg", >>>> 24 "mdp0-mem", >>>> 17 "memory", >>>> 14 "ufs-ddr", >>>> 14 "mdp1-mem", >>>> 14 "cpu-ufs", >>>> 13 "video-mem", >>>> 13 "gfx-mem", >>>> >>>> I hope this gives you a pointer on how to name the interconnects. >>>> >>>>> 2. If you are referring to some other targets that use, "cam_" >>>>> prefix, we >>>>> may not need that , isn't it? If we look at these interconnects >>>>> from camera >>>>> side, as you advised for other things like this? >>>> See above. >>> I see, so the names cam-cfg, cam-hf-mem, cam-sf-mem, cam-sf-icp-mem >>> should be ok? >>> >>> Or the other option, go exactly like >>> Documentation/devicetree/bindings/media/qcom,sc8280xp-camss.yaml. >>> >>> What would you advise? >>> >> To keep it consistent with the previous generations and still represent the >> block name, we will go ahead with the style in qcom,sc8280xp-camss.yaml. If >> anyone has any concerns, please do let us know. > Krzysztof, Bryan, your opinion? My preference would be to start using > sensible names, but I wouldn't enforce that. > >>>>>>> + >>>>>>> + iommus: >>>>>>> + items: >>>>>>> + - description: VFE non-protected stream >>>>>>> + - description: ICP0 shared stream >>>>>>> + - description: ICP1 shared stream >>>>>>> + - description: IPE CDM non-protected stream >>>>>>> + - description: IPE non-protected stream >>>>>>> + - description: JPEG non-protected stream >>>>>>> + - description: OFE CDM non-protected stream >>>>>>> + - description: OFE non-protected stream >>>>>>> + - description: VFE / VFE Lite CDM non-protected stream >>>>>> This will map all IOMMUs to the same domain. Are you sure that this is >>>>>> what we want? Or do we wait for iommu-maps to be fixed? >> Yes, when it is available, we can start using iommu-maps to create separate >> context banks. > It would be necessary to justify removing items from the list. Wouldn't > it be better to map only necessary SIDs now and add other later once we > have iommu-maps? I will let Bryan take the call on this. He was the one who wanted all the SIDs in the bindings. Hi @Bryan, if you can kindly share your thoughts on this and the interconnect naming, we will go ahead and push rev 10 for this. I believe we have taken care of other things. Thank you. >
On 10/12/2025 19:36, Vijay Kumar Tumati wrote: > > On 12/10/2025 11:25 AM, Dmitry Baryshkov wrote: >> On Wed, Dec 10, 2025 at 09:50:51AM -0800, Vijay Kumar Tumati wrote: >>> On 12/8/2025 3:21 PM, Vijay Kumar Tumati wrote: >>>> On 12/8/2025 2:48 PM, Dmitry Baryshkov wrote: >>>>> On Mon, Dec 08, 2025 at 01:03:06PM -0800, Vijay Kumar Tumati wrote: >>>>>> On 12/8/2025 11:53 AM, Dmitry Baryshkov wrote: >>>>>>>> + interconnects: >>>>>>>> + maxItems: 4 >>>>>>>> + >>>>>>>> + interconnect-names: >>>>>>>> + items: >>>>>>>> + - const: ahb >>>>>>>> + - const: hf_mnoc >>>>>>>> + - const: sf_icp_mnoc >>>>>>>> + - const: sf_mnoc >>>>>>> You know... Failure to look around is a sin. What are the names of >>>>>>> interconnects used by other devices? What do they actually describe? >>>>>>> >>>>>>> This is an absolute NAK. >>>>>> Please feel free to correct me here but, a couple things. >>>>>> >>>>>> 1. This is consistent with >>>>>> Documentation/devicetree/bindings/media/qcom,qcm2290-camss.yaml. no? >>>>> I see that nobody noticed an issue with Agatti, Lemans and Monaco >>>>> bindings (Krzysztof?) >>>>> >>>>> Usually interconnect names describe the blocks that are connected. >>>>> Here >>>>> are the top results of a quick git grep of interconnect names through >>>>> arch/arm64/dts/qcom: >>>>> >>>>> 729 "qup-core", >>>>> 717 "qup-config", >>>>> 457 "qup-memory", >>>>> 41 "usb-ddr", >>>>> 41 "apps-usb", >>>>> 39 "pcie-mem", >>>>> 39 "cpu-pcie", >>>>> 28 "sdhc-ddr", >>>>> 28 "cpu-sdhc", >>>>> 28 "cpu-cfg", >>>>> 24 "mdp0-mem", >>>>> 17 "memory", >>>>> 14 "ufs-ddr", >>>>> 14 "mdp1-mem", >>>>> 14 "cpu-ufs", >>>>> 13 "video-mem", >>>>> 13 "gfx-mem", >>>>> >>>>> I hope this gives you a pointer on how to name the interconnects. >>>>> >>>>>> 2. If you are referring to some other targets that use, "cam_" >>>>>> prefix, we >>>>>> may not need that , isn't it? If we look at these interconnects >>>>>> from camera >>>>>> side, as you advised for other things like this? >>>>> See above. >>>> I see, so the names cam-cfg, cam-hf-mem, cam-sf-mem, cam-sf-icp-mem >>>> should be ok? >>>> >>>> Or the other option, go exactly like >>>> Documentation/devicetree/bindings/media/qcom,sc8280xp-camss.yaml. >>>> >>>> What would you advise? >>>> >>> To keep it consistent with the previous generations and still >>> represent the >>> block name, we will go ahead with the style in qcom,sc8280xp- >>> camss.yaml. If >>> anyone has any concerns, please do let us know. >> Krzysztof, Bryan, your opinion? My preference would be to start using >> sensible names, but I wouldn't enforce that. >> >>>>>>>> + >>>>>>>> + iommus: >>>>>>>> + items: >>>>>>>> + - description: VFE non-protected stream >>>>>>>> + - description: ICP0 shared stream >>>>>>>> + - description: ICP1 shared stream >>>>>>>> + - description: IPE CDM non-protected stream >>>>>>>> + - description: IPE non-protected stream >>>>>>>> + - description: JPEG non-protected stream >>>>>>>> + - description: OFE CDM non-protected stream >>>>>>>> + - description: OFE non-protected stream >>>>>>>> + - description: VFE / VFE Lite CDM non-protected stream >>>>>>> This will map all IOMMUs to the same domain. Are you sure that >>>>>>> this is >>>>>>> what we want? Or do we wait for iommu-maps to be fixed? >>> Yes, when it is available, we can start using iommu-maps to create >>> separate >>> context banks. >> It would be necessary to justify removing items from the list. Wouldn't >> it be better to map only necessary SIDs now and add other later once we >> have iommu-maps? > I will let Bryan take the call on this. He was the one who wanted all > the SIDs in the bindings. Hi @Bryan, if you can kindly share your > thoughts on this and the interconnect naming, we will go ahead and push > rev 10 for this. I believe we have taken care of other things. Thank you. >> Since when are we delaying patches for future patches that may land never ? I'm fine with whatever clock name changes you can agree with Krzysztof but it seems a bit ironic to me to be given feedback to "align with previous dts" to then have the result be further change. I'd like a bit of stability and consistency TBH. --- bod
On Wed, Dec 10, 2025 at 09:45:56PM +0000, Bryan O'Donoghue wrote: > On 10/12/2025 19:36, Vijay Kumar Tumati wrote: > > > > On 12/10/2025 11:25 AM, Dmitry Baryshkov wrote: > > > On Wed, Dec 10, 2025 at 09:50:51AM -0800, Vijay Kumar Tumati wrote: > > > > On 12/8/2025 3:21 PM, Vijay Kumar Tumati wrote: > > > > > On 12/8/2025 2:48 PM, Dmitry Baryshkov wrote: > > > > > > On Mon, Dec 08, 2025 at 01:03:06PM -0800, Vijay Kumar Tumati wrote: > > > > > > > On 12/8/2025 11:53 AM, Dmitry Baryshkov wrote: > > > > > > > > > + interconnects: > > > > > > > > > + maxItems: 4 > > > > > > > > > + > > > > > > > > > + interconnect-names: > > > > > > > > > + items: > > > > > > > > > + - const: ahb > > > > > > > > > + - const: hf_mnoc > > > > > > > > > + - const: sf_icp_mnoc > > > > > > > > > + - const: sf_mnoc > > > > > > > > You know... Failure to look around is a sin. What are the names of > > > > > > > > interconnects used by other devices? What do they actually describe? > > > > > > > > > > > > > > > > This is an absolute NAK. > > > > > > > Please feel free to correct me here but, a couple things. > > > > > > > > > > > > > > 1. This is consistent with > > > > > > > Documentation/devicetree/bindings/media/qcom,qcm2290-camss.yaml. no? > > > > > > I see that nobody noticed an issue with Agatti, Lemans and Monaco > > > > > > bindings (Krzysztof?) > > > > > > > > > > > > Usually interconnect names describe the blocks that are > > > > > > connected. Here > > > > > > are the top results of a quick git grep of interconnect names through > > > > > > arch/arm64/dts/qcom: > > > > > > > > > > > > 729 "qup-core", > > > > > > 717 "qup-config", > > > > > > 457 "qup-memory", > > > > > > 41 "usb-ddr", > > > > > > 41 "apps-usb", > > > > > > 39 "pcie-mem", > > > > > > 39 "cpu-pcie", > > > > > > 28 "sdhc-ddr", > > > > > > 28 "cpu-sdhc", > > > > > > 28 "cpu-cfg", > > > > > > 24 "mdp0-mem", > > > > > > 17 "memory", > > > > > > 14 "ufs-ddr", > > > > > > 14 "mdp1-mem", > > > > > > 14 "cpu-ufs", > > > > > > 13 "video-mem", > > > > > > 13 "gfx-mem", > > > > > > > > > > > > I hope this gives you a pointer on how to name the interconnects. > > > > > > > > > > > > > 2. If you are referring to some other targets that use, "cam_" > > > > > > > prefix, we > > > > > > > may not need that , isn't it? If we look at these interconnects > > > > > > > from camera > > > > > > > side, as you advised for other things like this? > > > > > > See above. > > > > > I see, so the names cam-cfg, cam-hf-mem, cam-sf-mem, cam-sf-icp-mem > > > > > should be ok? > > > > > > > > > > Or the other option, go exactly like > > > > > Documentation/devicetree/bindings/media/qcom,sc8280xp-camss.yaml. > > > > > > > > > > What would you advise? > > > > > > > > > To keep it consistent with the previous generations and still > > > > represent the > > > > block name, we will go ahead with the style in qcom,sc8280xp- > > > > camss.yaml. If > > > > anyone has any concerns, please do let us know. > > > Krzysztof, Bryan, your opinion? My preference would be to start using > > > sensible names, but I wouldn't enforce that. > > > > > > > > > > > > + > > > > > > > > > + iommus: > > > > > > > > > + items: > > > > > > > > > + - description: VFE non-protected stream > > > > > > > > > + - description: ICP0 shared stream > > > > > > > > > + - description: ICP1 shared stream > > > > > > > > > + - description: IPE CDM non-protected stream > > > > > > > > > + - description: IPE non-protected stream > > > > > > > > > + - description: JPEG non-protected stream > > > > > > > > > + - description: OFE CDM non-protected stream > > > > > > > > > + - description: OFE non-protected stream > > > > > > > > > + - description: VFE / VFE Lite CDM non-protected stream > > > > > > > > This will map all IOMMUs to the same domain. Are > > > > > > > > you sure that this is > > > > > > > > what we want? Or do we wait for iommu-maps to be fixed? > > > > Yes, when it is available, we can start using iommu-maps to > > > > create separate > > > > context banks. > > > It would be necessary to justify removing items from the list. Wouldn't > > > it be better to map only necessary SIDs now and add other later once we > > > have iommu-maps? > > I will let Bryan take the call on this. He was the one who wanted all > > the SIDs in the bindings. Hi @Bryan, if you can kindly share your > > thoughts on this and the interconnect naming, we will go ahead and push > > rev 10 for this. I believe we have taken care of other things. Thank > > you. > > > > > Since when are we delaying patches for future patches that may land never ? > > I'm fine with whatever clock name changes you can agree with Krzysztof but > it seems a bit ironic to me to be given feedback to "align with previous > dts" to then have the result be further change. > > I'd like a bit of stability and consistency TBH. :-) -- With best wishes Dmitry
On 10/12/2025 21:45, Bryan O'Donoghue wrote: > On 10/12/2025 19:36, Vijay Kumar Tumati wrote: >> >> On 12/10/2025 11:25 AM, Dmitry Baryshkov wrote: >>> On Wed, Dec 10, 2025 at 09:50:51AM -0800, Vijay Kumar Tumati wrote: >>>> On 12/8/2025 3:21 PM, Vijay Kumar Tumati wrote: >>>>> On 12/8/2025 2:48 PM, Dmitry Baryshkov wrote: >>>>>> On Mon, Dec 08, 2025 at 01:03:06PM -0800, Vijay Kumar Tumati wrote: >>>>>>> On 12/8/2025 11:53 AM, Dmitry Baryshkov wrote: >>>>>>>>> + interconnects: >>>>>>>>> + maxItems: 4 >>>>>>>>> + >>>>>>>>> + interconnect-names: >>>>>>>>> + items: >>>>>>>>> + - const: ahb >>>>>>>>> + - const: hf_mnoc >>>>>>>>> + - const: sf_icp_mnoc >>>>>>>>> + - const: sf_mnoc >>>>>>>> You know... Failure to look around is a sin. What are the names of >>>>>>>> interconnects used by other devices? What do they actually describe? >>>>>>>> >>>>>>>> This is an absolute NAK. >>>>>>> Please feel free to correct me here but, a couple things. >>>>>>> >>>>>>> 1. This is consistent with >>>>>>> Documentation/devicetree/bindings/media/qcom,qcm2290-camss.yaml. no? >>>>>> I see that nobody noticed an issue with Agatti, Lemans and Monaco >>>>>> bindings (Krzysztof?) >>>>>> >>>>>> Usually interconnect names describe the blocks that are connected. >>>>>> Here >>>>>> are the top results of a quick git grep of interconnect names through >>>>>> arch/arm64/dts/qcom: >>>>>> >>>>>> 729 "qup-core", >>>>>> 717 "qup-config", >>>>>> 457 "qup-memory", >>>>>> 41 "usb-ddr", >>>>>> 41 "apps-usb", >>>>>> 39 "pcie-mem", >>>>>> 39 "cpu-pcie", >>>>>> 28 "sdhc-ddr", >>>>>> 28 "cpu-sdhc", >>>>>> 28 "cpu-cfg", >>>>>> 24 "mdp0-mem", >>>>>> 17 "memory", >>>>>> 14 "ufs-ddr", >>>>>> 14 "mdp1-mem", >>>>>> 14 "cpu-ufs", >>>>>> 13 "video-mem", >>>>>> 13 "gfx-mem", >>>>>> >>>>>> I hope this gives you a pointer on how to name the interconnects. >>>>>> >>>>>>> 2. If you are referring to some other targets that use, "cam_" >>>>>>> prefix, we >>>>>>> may not need that , isn't it? If we look at these interconnects >>>>>>> from camera >>>>>>> side, as you advised for other things like this? >>>>>> See above. >>>>> I see, so the names cam-cfg, cam-hf-mem, cam-sf-mem, cam-sf-icp-mem >>>>> should be ok? >>>>> >>>>> Or the other option, go exactly like >>>>> Documentation/devicetree/bindings/media/qcom,sc8280xp-camss.yaml. >>>>> >>>>> What would you advise? >>>>> >>>> To keep it consistent with the previous generations and still >>>> represent the >>>> block name, we will go ahead with the style in qcom,sc8280xp- >>>> camss.yaml. If >>>> anyone has any concerns, please do let us know. >>> Krzysztof, Bryan, your opinion? My preference would be to start using >>> sensible names, but I wouldn't enforce that. >>> >>>>>>>>> + >>>>>>>>> + iommus: >>>>>>>>> + items: >>>>>>>>> + - description: VFE non-protected stream >>>>>>>>> + - description: ICP0 shared stream >>>>>>>>> + - description: ICP1 shared stream >>>>>>>>> + - description: IPE CDM non-protected stream >>>>>>>>> + - description: IPE non-protected stream >>>>>>>>> + - description: JPEG non-protected stream >>>>>>>>> + - description: OFE CDM non-protected stream >>>>>>>>> + - description: OFE non-protected stream >>>>>>>>> + - description: VFE / VFE Lite CDM non-protected stream >>>>>>>> This will map all IOMMUs to the same domain. Are you sure that >>>>>>>> this is >>>>>>>> what we want? Or do we wait for iommu-maps to be fixed? >>>> Yes, when it is available, we can start using iommu-maps to create >>>> separate >>>> context banks. >>> It would be necessary to justify removing items from the list. Wouldn't >>> it be better to map only necessary SIDs now and add other later once we >>> have iommu-maps? >> I will let Bryan take the call on this. He was the one who wanted all >> the SIDs in the bindings. Hi @Bryan, if you can kindly share your >> thoughts on this and the interconnect naming, we will go ahead and push >> rev 10 for this. I believe we have taken care of other things. Thank you. >>> > > Since when are we delaying patches for future patches that may land never ? > > I'm fine with whatever clock name changes you can agree with Krzysztof > but it seems a bit ironic to me to be given feedback to "align with > previous dts" to then have the result be further change. > > I'd like a bit of stability and consistency TBH. > > --- > bod > My feedback is - Include the full list of SIDs - Stick to previous clock and interconnect names Your other alternative is to suspend Kaanapali CAMSS unless/until iommu-map is landed. As I say though "change your patch until my other patch is landed" is the opposite of how things are supposed to be done. I recommend you focus on your own series. If iommu-map gets merged first, adapt. If not, don't delay your work to accommodate stuff that is up in the air which for all you know may never land or may take six more months. --- bod
On 12/10/2025 2:05 PM, Bryan O'Donoghue wrote: > On 10/12/2025 21:45, Bryan O'Donoghue wrote: >> On 10/12/2025 19:36, Vijay Kumar Tumati wrote: >>> >>> On 12/10/2025 11:25 AM, Dmitry Baryshkov wrote: >>>> On Wed, Dec 10, 2025 at 09:50:51AM -0800, Vijay Kumar Tumati wrote: >>>>> On 12/8/2025 3:21 PM, Vijay Kumar Tumati wrote: >>>>>> On 12/8/2025 2:48 PM, Dmitry Baryshkov wrote: >>>>>>> On Mon, Dec 08, 2025 at 01:03:06PM -0800, Vijay Kumar Tumati wrote: >>>>>>>> On 12/8/2025 11:53 AM, Dmitry Baryshkov wrote: >>>>>>>>>> + interconnects: >>>>>>>>>> + maxItems: 4 >>>>>>>>>> + >>>>>>>>>> + interconnect-names: >>>>>>>>>> + items: >>>>>>>>>> + - const: ahb >>>>>>>>>> + - const: hf_mnoc >>>>>>>>>> + - const: sf_icp_mnoc >>>>>>>>>> + - const: sf_mnoc >>>>>>>>> You know... Failure to look around is a sin. What are the >>>>>>>>> names of >>>>>>>>> interconnects used by other devices? What do they actually >>>>>>>>> describe? >>>>>>>>> >>>>>>>>> This is an absolute NAK. >>>>>>>> Please feel free to correct me here but, a couple things. >>>>>>>> >>>>>>>> 1. This is consistent with >>>>>>>> Documentation/devicetree/bindings/media/qcom,qcm2290-camss.yaml. >>>>>>>> no? >>>>>>> I see that nobody noticed an issue with Agatti, Lemans and Monaco >>>>>>> bindings (Krzysztof?) >>>>>>> >>>>>>> Usually interconnect names describe the blocks that are connected. >>>>>>> Here >>>>>>> are the top results of a quick git grep of interconnect names >>>>>>> through >>>>>>> arch/arm64/dts/qcom: >>>>>>> >>>>>>> 729 "qup-core", >>>>>>> 717 "qup-config", >>>>>>> 457 "qup-memory", >>>>>>> 41 "usb-ddr", >>>>>>> 41 "apps-usb", >>>>>>> 39 "pcie-mem", >>>>>>> 39 "cpu-pcie", >>>>>>> 28 "sdhc-ddr", >>>>>>> 28 "cpu-sdhc", >>>>>>> 28 "cpu-cfg", >>>>>>> 24 "mdp0-mem", >>>>>>> 17 "memory", >>>>>>> 14 "ufs-ddr", >>>>>>> 14 "mdp1-mem", >>>>>>> 14 "cpu-ufs", >>>>>>> 13 "video-mem", >>>>>>> 13 "gfx-mem", >>>>>>> >>>>>>> I hope this gives you a pointer on how to name the interconnects. >>>>>>> >>>>>>>> 2. If you are referring to some other targets that use, "cam_" >>>>>>>> prefix, we >>>>>>>> may not need that , isn't it? If we look at these interconnects >>>>>>>> from camera >>>>>>>> side, as you advised for other things like this? >>>>>>> See above. >>>>>> I see, so the names cam-cfg, cam-hf-mem, cam-sf-mem, cam-sf-icp-mem >>>>>> should be ok? >>>>>> >>>>>> Or the other option, go exactly like >>>>>> Documentation/devicetree/bindings/media/qcom,sc8280xp-camss.yaml. >>>>>> >>>>>> What would you advise? >>>>>> >>>>> To keep it consistent with the previous generations and still >>>>> represent the >>>>> block name, we will go ahead with the style in qcom,sc8280xp- >>>>> camss.yaml. If >>>>> anyone has any concerns, please do let us know. >>>> Krzysztof, Bryan, your opinion? My preference would be to start using >>>> sensible names, but I wouldn't enforce that. >>>> >>>>>>>>>> + >>>>>>>>>> + iommus: >>>>>>>>>> + items: >>>>>>>>>> + - description: VFE non-protected stream >>>>>>>>>> + - description: ICP0 shared stream >>>>>>>>>> + - description: ICP1 shared stream >>>>>>>>>> + - description: IPE CDM non-protected stream >>>>>>>>>> + - description: IPE non-protected stream >>>>>>>>>> + - description: JPEG non-protected stream >>>>>>>>>> + - description: OFE CDM non-protected stream >>>>>>>>>> + - description: OFE non-protected stream >>>>>>>>>> + - description: VFE / VFE Lite CDM non-protected stream >>>>>>>>> This will map all IOMMUs to the same domain. Are you sure that >>>>>>>>> this is >>>>>>>>> what we want? Or do we wait for iommu-maps to be fixed? >>>>> Yes, when it is available, we can start using iommu-maps to create >>>>> separate >>>>> context banks. >>>> It would be necessary to justify removing items from the list. >>>> Wouldn't >>>> it be better to map only necessary SIDs now and add other later >>>> once we >>>> have iommu-maps? >>> I will let Bryan take the call on this. He was the one who wanted all >>> the SIDs in the bindings. Hi @Bryan, if you can kindly share your >>> thoughts on this and the interconnect naming, we will go ahead and push >>> rev 10 for this. I believe we have taken care of other things. Thank >>> you. >>>> >> >> Since when are we delaying patches for future patches that may land >> never ? >> >> I'm fine with whatever clock name changes you can agree with Krzysztof >> but it seems a bit ironic to me to be given feedback to "align with >> previous dts" to then have the result be further change. >> >> I'd like a bit of stability and consistency TBH. >> >> --- >> bod >> > > My feedback is > > - Include the full list of SIDs > - Stick to previous clock and interconnect names > > Your other alternative is to suspend Kaanapali CAMSS unless/until > iommu-map is landed. > > As I say though "change your patch until my other patch is landed" is > the opposite of how things are supposed to be done. > > I recommend you focus on your own series. If iommu-map gets merged > first, adapt. > > If not, don't delay your work to accommodate stuff that is up in the > air which for all you know may never land or may take six more months. > > --- > bod Makes sense. Thanks Bryan, Dmitry. We will update this to, Interconnect names: cam_ahb, cam_hf_mnoc, cam_sf_mnoc, cam_sf_icp_mnoc, consistent with a set of previous generations. CX domain AXI clock names: gcc_axi_hf, gcc_axi_sf, consistent with other bindings. We will post rev10 with these. Thanks.
© 2016 - 2025 Red Hat, Inc.