[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <DF291XT4MOYA.3OVO84CAJKJ5R@fairphone.com>
Date: Fri, 19 Dec 2025 15:06:28 +0100
From: "Luca Weiss" <luca.weiss@...rphone.com>
To: "Bryan O'Donoghue" <bryan.odonoghue@...aro.org>, "Vladimir Zapolskiy"
<vladimir.zapolskiy@...aro.org>, "Bryan O'Donoghue" <bod@...nel.org>, "Luca
Weiss" <luca.weiss@...rphone.com>, "Robert Foss" <rfoss@...nel.org>, "Todor
Tomov" <todor.too@...il.com>, "Mauro Carvalho Chehab" <mchehab@...nel.org>,
"Rob Herring" <robh@...nel.org>, "Krzysztof Kozlowski"
<krzk+dt@...nel.org>, "Conor Dooley" <conor+dt@...nel.org>, "Bjorn
Andersson" <andersson@...nel.org>, "Konrad Dybcio" <konradybcio@...nel.org>
Cc: <~postmarketos/upstreaming@...ts.sr.ht>, <phone-devel@...r.kernel.org>,
<linux-arm-msm@...r.kernel.org>, <linux-media@...r.kernel.org>,
<devicetree@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 1/3] dt-bindings: media: camss: Add qcom,sm6350-camss
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@...rphone.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@...rphone.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
Powered by blists - more mailing lists