Re: [PATCH v2 1/3] dt-bindings: media: camss: Add qcom,sm6350-camss

["Luca Weiss" <luca.weiss@fairphone.com>]

Hi Bryan,

On Sun Nov 16, 2025 at 3:05 PM CET, Bryan O'Donoghue wrote:
> On 14/11/2025 17:06, Vladimir Zapolskiy wrote:
>> On 11/14/25 18:09, Bryan O'Donoghue wrote:
>>> On 14/11/2025 13:06, Luca Weiss wrote:
>>>> Hi Vladimir,
>>>>
>>>> On Fri Nov 14, 2025 at 1:40 PM CET, Vladimir Zapolskiy wrote:
>>>>> Hi Luca.
>>>>>
>>>>> On 11/14/25 13:15, Luca Weiss wrote:
>>>>>> Add bindings for the Camera Subsystem on the SM6350 SoC.
>>>>>>
>>>>>> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
>>>>>> ---
>>>>>>     .../bindings/media/qcom,sm6350-camss.yaml          | 349 ++++++ 
>>>>>> +++++++++++++++
>>>>>>     1 file changed, 349 insertions(+)
>>>>>>
>>>>>> diff --git a/Documentation/devicetree/bindings/media/qcom,sm6350- 
>>>>>> camss.yaml b/Documentation/devicetree/bindings/media/qcom,sm6350- 
>>>>>> camss.yaml
>>>>>> new file mode 100644
>>>>>> index 000000000000..d812b5b50c05
>>>>>> --- /dev/null
>>>>>> +++ b/Documentation/devicetree/bindings/media/qcom,sm6350-camss.yaml
>>>>>> @@ -0,0 +1,349 @@
>>>>>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>>>>>> +%YAML 1.2
>>>>>> +---
>>>>>> +$id: http://devicetree.org/schemas/media/qcom,sm6350-camss.yaml#
>>>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>>>> +
>>>>>> +title: Qualcomm SM6350 Camera Subsystem (CAMSS)
>>>>>> +
>>>>>> +maintainers:
>>>>>> +  - Luca Weiss <luca.weiss@fairphone.com>
>>>>>> +
>>>>>> +description:
>>>>>> +  The CAMSS IP is a CSI decoder and ISP present on Qualcomm 
>>>>>> platforms.
>>>>>> +
>>>>>> +properties:
>>>>>> +  compatible:
>>>>>> +    const: qcom,sm6350-camss
>>>>>> +
>>>>>> +  reg:
>>>>>> +    maxItems: 12
>>>>>> +
>>>>>> +  reg-names:
>>>>>> +    items:
>>>>>> +      - const: csid0
>>>>>> +      - const: csid1
>>>>>> +      - const: csid2
>>>>>> +      - const: csid_lite
>>>>>> +      - const: csiphy0
>>>>>> +      - const: csiphy1
>>>>>> +      - const: csiphy2
>>>>>> +      - const: csiphy3
>>>>>> +      - const: vfe0
>>>>>> +      - const: vfe1
>>>>>> +      - const: vfe2
>>>>>> +      - const: vfe_lite
>>>>>> +
>>>>>> +  clocks:
>>>>>> +    maxItems: 30
>>>>>> +
>>>>>> +  clock-names:
>>>>>> +    items:
>>>>>> +      - const: cam_ahb_clk
>>>>>> +      - const: cam_axi
>>>>>> +      - const: soc_ahb
>>>>>> +      - const: camnoc_axi
>>>>>> +      - const: core_ahb
>>>>>> +      - const: cpas_ahb
>>>>>> +      - const: csiphy0
>>>>>> +      - const: csiphy0_timer
>>>>>> +      - const: csiphy1
>>>>>> +      - const: csiphy1_timer
>>>>>> +      - const: csiphy2
>>>>>> +      - const: csiphy2_timer
>>>>>> +      - const: csiphy3
>>>>>> +      - const: csiphy3_timer
>>>>>> +      - const: slow_ahb_src
>>>>>> +      - const: vfe0_axi
>>>>>> +      - const: vfe0
>>>>>> +      - const: vfe0_cphy_rx
>>>>>> +      - const: vfe0_csid
>>>>>> +      - const: vfe1_axi
>>>>>> +      - const: vfe1
>>>>>> +      - const: vfe1_cphy_rx
>>>>>> +      - const: vfe1_csid
>>>>>> +      - const: vfe2_axi
>>>>>> +      - const: vfe2
>>>>>> +      - const: vfe2_cphy_rx
>>>>>> +      - const: vfe2_csid
>>>>>> +      - const: vfe_lite
>>>>>> +      - const: vfe_lite_cphy_rx
>>>>>> +      - const: vfe_lite_csid
>>>>>
>>>>> The sorting order of this list does not follow the sorting order 
>>>>> accepted
>>>>> in the past.
>>>>
>>>> What file should I best reference?
>>>
>>> Documentation/devicetree/bindings/media/qcom,sdm845-camss.yaml
>>>
>>>>>
>>>>> I'm very sorry for the vagueness, but I can not pronounce the accepted
>>>>> sorting order name, because it triggers people.
>>>>>
>>>>>> +
>>>>>> +  interrupts:
>>>>>> +    maxItems: 12
>>>>>> +
>>>>>> +  interrupt-names:
>>>>>> +    items:
>>>>>> +      - const: csid0
>>>>>> +      - const: csid1
>>>>>> +      - const: csid2
>>>>>> +      - const: csid_lite
>>>>>> +      - const: csiphy0
>>>>>> +      - const: csiphy1
>>>>>> +      - const: csiphy2
>>>>>> +      - const: csiphy3
>>>>>> +      - const: vfe0
>>>>>> +      - const: vfe1
>>>>>> +      - const: vfe2
>>>>>> +      - const: vfe_lite
>>>>>> +
>>>>>> +  interconnects:
>>>>>> +    maxItems: 4
>>>>>> +
>>>>>> +  interconnect-names:
>>>>>> +    items:
>>>>>> +      - const: ahb
>>>>>> +      - const: hf_mnoc
>>>>>> +      - const: sf_mnoc
>>>>>> +      - const: sf_icp_mnoc
>>>>>
>>>>> Please remove sf_mnoc and sf_icp_mnoc, they are not needed for enabling
>>>>> IP to produce raw images, and one day you may use them somewhere else.
>>>>
>>>> Ack, will give it a try.
>>>
>>> Disagree with this.
>>>
>>> See the Kanaapali patches. I'm asking new submissions to be as complete
>>> as possible, instead of limiting the hardware description to the RDI.
>>>
>>> So listing the ICP noc is the right thing to do.
>>>
>>> So please include register banks for
>>>
>>> - bps
>>> - cdm
>>> - icp
>>> - ipe
>>> - jpeg
>>> - lrme
>> 
>> This suggests to get a DT backward compatibility lock forever, 
>
> I'm not entirely sure what this comment means.
>
> The objective here is to get a full and complete DT describing camera 
> hardware that can be consumed by
>
> - CAMSS RDI
> - CAMSS RDI + future goodness
> - FreeBSD
> - Any other DT consumer of upstream data perhaps even CamX
> which
>> makes it
>> a very bad advice, which is favourable for just the single interested 
>> party,
>> which offers an alternative to the upstream CAMSS.
>> 
>> Anybody who wants to get support of any CAMSS ISP functionality above RAW
>> images shall not add anything unrelated and unsupported.
>> 
>> The asked inclusion shall be done later on safely, if ever needed.
>
> As I say the objective is to fully describe the hardware in DT, not to 
> describe only the RDI path.

So far I've gotten this diff on top of v2, not quite sure how to
continue further...

Please advice how to turn these resources into a usable dt-binding.

I'm also a bit worried that it'll be subtly wrong due to all the
additions being impossible to test for me.

Regards
Luca


diff --git a/Documentation/devicetree/bindings/media/qcom,sm6350-camss.yaml b/Documentation/devicetree/bindings/media/qcom,sm6350-camss.yaml
index d812b5b50c05..340bf03ffe91 100644
--- a/Documentation/devicetree/bindings/media/qcom,sm6350-camss.yaml
+++ b/Documentation/devicetree/bindings/media/qcom,sm6350-camss.yaml
@@ -203,6 +202,23 @@ examples:
                   <0x0 0x0acb6000 0x0 0x4000>,
                   <0x0 0x0acbd000 0x0 0x4000>,
                   <0x0 0x0acc4000 0x0 0x4000>;
+
+                  // ICP ?? == qcom,cam-a5 ?
+               // <0x0 0x0ac00000 0x0 0x6000>, // ? a5_qgic
+               // <0x0 0x0ac10000 0x0 0x8000>, // ? a5_sierra
+               // <0x0 0x0ac18000 0x0 0x3000>; // ? a5_csr
+
+               // <0x0 0x0ac87000 0x0 0xa000> // "ipe" IPE (Image Processing Engine)
+               // <0x0 0x0ac6f000 0x0 0x8000> // "bps"? BPS
+               // <0x0 0x0ac6b000 0x0 0xa00> // "lrme"? LRME
+
+               // <0x0 0x0ac40000 0x0 0x1000>, // ? CAM CPAS: cam_cpas_top
+               // <0x0 0x0ac42000 0x0 0x4600>, // ? CAM CPAS: cam_camnoc
+               // <0x0 0x01fc0000 0x0 0x40000>; // ? CAM CPAS: core_top_csr_tcsr
+
+               // <0x0 0x0ac52000 0x0 0x4000> // "jpeg_dma" JPEG DMA & Downscaler
+               // <0x0 0x0ac4e000 0x0 0x4000> // "jpeg_enc" JPEG Encoder
+               // <0x0 0x0ac48000 0x0 0x1000> // ? CPAS CDM
             reg-names = "csid0",
                         "csid1",
                         "csid2",
@@ -216,8 +232,12 @@ examples:
                         "vfe2",
                         "vfe_lite";
 
-            clocks = <&gcc GCC_CAMERA_AHB_CLK>,
-                     <&gcc GCC_CAMERA_AXI_CLK>,
+            // CAMCC_FAST_AHB_CLK_SRC + CAMCC_ICP_CLK for qcom,cam-a5
+            // CAMCC_IPE_0_AHB_CLK + CAMCC_IPE_0_AREG_CLK + CAMCC_IPE_0_AXI_CLK + CAMCC_IPE_0_CLK for qcom,cam-ipe
+            // CAMCC_BPS_AHB_CLK + CAMCC_BPS_AREG_CLK + CAMCC_BPS_AXI_CLK + CAMCC_BPS_CLK for qcom,cam-bps
+            // CAMCC_JPEG_CLK for qcom,cam_jpeg_enc / qcom,cam_jpeg_dma
+            // CAMCC_LRME_CLK for qcom,lrme
+            clocks = <&gcc GCC_CAMERA_AXI_CLK>,
                      <&camcc CAMCC_SOC_AHB_CLK>,
                      <&camcc CAMCC_CAMNOC_AXI_CLK>,
                      <&camcc CAMCC_CORE_AHB_CLK>,
@@ -246,8 +266,7 @@ examples:
                      <&camcc CAMCC_IFE_LITE_CLK>,
                      <&camcc CAMCC_IFE_LITE_CPHY_RX_CLK>,
                      <&camcc CAMCC_IFE_LITE_CSID_CLK>;
-            clock-names = "cam_ahb_clk",
-                          "cam_axi",
+            clock-names = "cam_axi",
                           "soc_ahb",
                           "camnoc_axi",
                           "core_ahb",
@@ -277,18 +296,24 @@ examples:
                           "vfe_lite_cphy_rx",
                           "vfe_lite_csid";
 
-            interrupts = <GIC_SPI 464 IRQ_TYPE_LEVEL_HIGH>,
-                         <GIC_SPI 466 IRQ_TYPE_LEVEL_HIGH>,
-                         <GIC_SPI 717 IRQ_TYPE_LEVEL_HIGH>,
-                         <GIC_SPI 473 IRQ_TYPE_LEVEL_HIGH>,
-                         <GIC_SPI 477 IRQ_TYPE_LEVEL_HIGH>,
-                         <GIC_SPI 478 IRQ_TYPE_LEVEL_HIGH>,
-                         <GIC_SPI 479 IRQ_TYPE_LEVEL_HIGH>,
-                         <GIC_SPI 461 IRQ_TYPE_LEVEL_HIGH>,
-                         <GIC_SPI 465 IRQ_TYPE_LEVEL_HIGH>,
-                         <GIC_SPI 467 IRQ_TYPE_LEVEL_HIGH>,
-                         <GIC_SPI 718 IRQ_TYPE_LEVEL_HIGH>,
-                         <GIC_SPI 472 IRQ_TYPE_LEVEL_HIGH>;
+            // 469 for cpas-cdm
+            // 463 for qcom,cam-a5
+            // 474 for qcom,cam_jpeg_enc
+            // 475 for qcom,cam_jpeg_dma
+            // 476 for qcom,lrme
+            // 459 for qcom,cam-cpas
+            interrupts = <GIC_SPI 464 IRQ_TYPE_EDGE_RISING>,
+                         <GIC_SPI 466 IRQ_TYPE_EDGE_RISING>,
+                         <GIC_SPI 717 IRQ_TYPE_EDGE_RISING>,
+                         <GIC_SPI 473 IRQ_TYPE_EDGE_RISING>,
+                         <GIC_SPI 477 IRQ_TYPE_EDGE_RISING>,
+                         <GIC_SPI 478 IRQ_TYPE_EDGE_RISING>,
+                         <GIC_SPI 479 IRQ_TYPE_EDGE_RISING>,
+                         <GIC_SPI 461 IRQ_TYPE_EDGE_RISING>,
+                         <GIC_SPI 465 IRQ_TYPE_EDGE_RISING>,
+                         <GIC_SPI 467 IRQ_TYPE_EDGE_RISING>,
+                         <GIC_SPI 718 IRQ_TYPE_EDGE_RISING>,
+                         <GIC_SPI 472 IRQ_TYPE_EDGE_RISING>;
             interrupt-names = "csid0",
                               "csid1",
                               "csid2",
@@ -302,6 +327,9 @@ examples:
                               "vfe2",
                               "vfe_lite";
 
+            // MASTER_CAMNOC_HF0_UNCOMP -> SLAVE_CAMNOC_UNCOMP ?
+            // MASTER_CAMNOC_SF_UNCOMP -> SLAVE_CAMNOC_UNCOMP ?
+            // MASTER_CAMNOC_ICP_UNCOMP -> SLAVE_CAMNOC_UNCOMP ?
             interconnects = <&gem_noc MASTER_AMPSS_M0 QCOM_ICC_TAG_ACTIVE_ONLY
                              &config_noc SLAVE_CAMERA_CFG QCOM_ICC_TAG_ACTIVE_ONLY>,
                             <&mmss_noc MASTER_CAMNOC_HF QCOM_ICC_TAG_ALWAYS
@@ -315,12 +343,18 @@ examples:
                                  "sf_mnoc",
                                  "sf_icp_mnoc";
 
+            // <&apps_smmu 0xd40 0x20> & <&apps_smmu 0xd60 0x20> for lrme
+            // <&apps_smmu 0xd00 0x20> & <&apps_smmu 0xd20 0x20> for jpeg
+            // <&apps_smmu 0xca2 0x0> & <&apps_smmu 0xc40 0x20> & <&apps_smmu 0xc60 0x20> & <&apps_smmu 0xcc0 0x20> & <&apps_smmu 0xce0 0x20> for icp
+            // <&apps_smmu 0xc80 0x0> for cpas-cdm0
             iommus = <&apps_smmu 0x820 0xc0>,
                      <&apps_smmu 0x840 0x0>,
                      <&apps_smmu 0x860 0xc0>,
                      <&apps_smmu 0x880 0x0>;
 
-            power-domains = <&camcc TITAN_TOP_GDSC>
+            // ipe_0_gdsc for qcom,cam-ipe
+            // bps_gdsc for qcom,cam-bps
+            power-domains = <&camcc TITAN_TOP_GDSC>,
                             <&camcc IFE_0_GDSC>,
                             <&camcc IFE_1_GDSC>,
                             <&camcc IFE_2_GDSC>;
@@ -329,8 +363,14 @@ examples:
                                  "ife1",
                                  "ife2";
 
-            vdd-csiphy-0p9-supply = <&vreg_l18a>;
-            vdd-csiphy-1p25-supply = <&vreg_l22a>;
+            vdd-csiphy0-0p9-supply = <&vreg_l18a>;
+            vdd-csiphy0-1p25-supply = <&vreg_l22a>;
+            vdd-csiphy1-0p9-supply = <&vreg_l18a>;
+            vdd-csiphy1-1p25-supply = <&vreg_l22a>;
+            vdd-csiphy2-0p9-supply = <&vreg_l18a>;
+            vdd-csiphy2-1p25-supply = <&vreg_l22a>;
+            vdd-csiphy3-0p9-supply = <&vreg_l18a>;
+            vdd-csiphy3-1p25-supply = <&vreg_l22a>;
 
             ports {
                 #address-cells = <1>;

>
>>>>> I believe this clock is critical, and it is set so in the SM6350 GCC 
>>>>> driver,
>>>>> therefore it should not be added here.
>>>>
>>>> True, gcc_camera_ahb_clk has CLK_IS_CRITICAL in gcc-sm6350.c
>>>
>>> DT describes hardware, not the happenstance of Linux driver setup.
>>>
>>> On that basis omitting <&gcc GCC_CAMERA_AHB_CLK> from the clock list is
>>> not correct.
>> 
>> This has been already discussed, an enumerous amount of Qualcomm/MSM
>> specific resourced like clocks enabled in ABL, Linux etc. are considered
>> critical and not described in the dtb.
>
> I still think the argument for that is tenuous wrong in fact but, I know 
> you are right, this is a lost argument.
>
> @Luca please _do_ include the full array of registers/clocks/nocs as you 
> find them and yeah drop the AXI clock from that description because reasons.
>
> https://lore.kernel.org/linux-arm-msm/20251113-add-support-for-camss-on-kaanapali-v6-1-1e6038785a8e@oss.qualcomm.com/
>
>   ---
> bod