Re: [PATCH v9 1/5] media: dt-bindings: Add CAMSS device for Kaanapali
From: Vijay Kumar Tumati
Date: Wed Dec 10 2025 - 12:53:25 EST
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@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?
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.