Re: [PATCH v9 1/5] media: dt-bindings: Add CAMSS device for Kaanapali

From: Vijay Kumar Tumati
Date: Mon Dec 08 2025 - 16:03:21 EST

Next message: Sander Vanheule: "Re: [PATCH] ASoC: tegra: Fix uninitialized flat cache warning in tegra210_ahub"
Previous message: Aurabindo Pillai: "Re: [REGRESSION] Black screen on HDMI power-cycle after commit 3471b9a31ce3 (7900XTX + LG C3)"
In reply to: Dmitry Baryshkov: "Re: [PATCH v9 1/5] media: dt-bindings: Add CAMSS device for Kaanapali"
Next in thread: Dmitry Baryshkov: "Re: [PATCH v9 1/5] media: dt-bindings: Add CAMSS device for Kaanapali"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

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@xxxxxxxxxx>
Reviewed-by: Krzysztof Kozlowski <krzk@xxxxxxxxxx>
Signed-off-by: Hangxiang Ma <hangxiang.ma@xxxxxxxxxxxxxxxx>
---
.../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@xxxxxxxxxxxxxxxx>
+
+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?

Next message: Sander Vanheule: "Re: [PATCH] ASoC: tegra: Fix uninitialized flat cache warning in tegra210_ahub"
Previous message: Aurabindo Pillai: "Re: [REGRESSION] Black screen on HDMI power-cycle after commit 3471b9a31ce3 (7900XTX + LG C3)"
In reply to: Dmitry Baryshkov: "Re: [PATCH v9 1/5] media: dt-bindings: Add CAMSS device for Kaanapali"
Next in thread: Dmitry Baryshkov: "Re: [PATCH v9 1/5] media: dt-bindings: Add CAMSS device for Kaanapali"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]