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

[Bryan O'Donoghue <bryan.odonoghue@linaro.org>]

On 19/12/2025 14:06, Luca Weiss wrote:
> 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

Looks correct.

>               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>;
> 
These look right to me, consistent with what I see downstream.

---
bod