| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <1d6a20d8-b011-4608-a722-a1996b366a56@oss.qualcomm.com>
Date: Thu, 16 Oct 2025 16:53:37 -0700
From: Vijay Kumar Tumati <vijay.tumati@....qualcomm.com>
To: Vladimir Zapolskiy <vladimir.zapolskiy@...aro.org>,
Hangxiang Ma <hangxiang.ma@....qualcomm.com>,
Loic Poulain <loic.poulain@....qualcomm.com>,
Robert Foss
<rfoss@...nel.org>, Andi Shyti <andi.shyti@...nel.org>,
Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley <conor+dt@...nel.org>, Todor Tomov <todor.too@...il.com>,
Mauro Carvalho Chehab <mchehab@...nel.org>,
Bryan O'Donoghue <bryan.odonoghue@...aro.org>
Cc: linux-i2c@...r.kernel.org, linux-arm-msm@...r.kernel.org,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-media@...r.kernel.org, aiqun.yu@....qualcomm.com,
tingwei.zhang@....qualcomm.com, trilok.soni@....qualcomm.com,
yijie.yang@....qualcomm.com,
Jingyi Wang <jingyi.wang@....qualcomm.com>,
Atiya Kailany <atiya.kailany@....qualcomm.com>
Subject: Re: [PATCH v2 2/6] dt-bindings: media: camss: Add
qcom,kaanapali-camss binding
On 10/15/2025 12:45 PM, Vladimir Zapolskiy wrote:
> On 10/15/25 05:56, Hangxiang Ma wrote:
>> Add bindings for qcom,kaanapali-camss in order to support the camera
>> subsystem for Kaanapali.
>>
>> Signed-off-by: Hangxiang Ma <hangxiang.ma@....qualcomm.com>
>> ---
>> .../bindings/media/qcom,kaanapali-camss.yaml | 494
>> +++++++++++++++++++++
>> 1 file changed, 494 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..d04c21103cfd
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/media/qcom,kaanapali-camss.yaml
>> @@ -0,0 +1,494 @@
>> +# 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@....qualcomm.com>
>> +
>> +description:
>> + The CAMSS IP is a CSI decoder and ISP present on Qualcomm platforms.
>> +
>> +properties:
>> + compatible:
>> + const: qcom,kaanapali-camss
>> +
>> + reg:
>> + maxItems: 16
>> +
>> + 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
>> +
>> + clocks:
>> + maxItems: 34
>> +
>> + 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: cam_top_ahb
>> + - const: cam_top_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_hf_axi
>> + - const: qdss_debug_xo
>> + - 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
>
> The list of 'clock-names' values is not alphanumerically sorted.
>
>> +
>> + interrupts:
>> + maxItems: 16
>> + interrupt-names:
>
> Missing empty line to separate properties.
>
>> + 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
>> +
>> + interconnects:
>> + maxItems: 2
>> +
>> + interconnect-names:
>> + items:
>> + - const: ahb
>> + - const: hf_0_mnoc
>
> Please rename "hf_0_mnoc" to "hf_mnoc", see qcom,qcm2290-camss.yaml etc.
>
>> +
>> + iommus:
>> + maxItems: 1
>> +
>> + power-domains:
>> + items:
>> + - description:
>> + TFE0 GDSC - Thin Front End, Global Distributed Switch
>> Controller.
>> + - description:
>> + TFE1 GDSC - Thin Front End, Global Distributed Switch
>> Controller.
>> + - description:
>> + TFE2 GDSC - Thin Front End, Global Distributed Switch
>> Controller.
>> + - description:
>> + Titan GDSC - Titan ISP Block Global Distributed Switch
>> Controller.
>> +
>> + power-domain-names:
>> + items:
>> + - const: tfe0
>> + - const: tfe1
>> + - const: tfe2
>
> Please remove all 'tfeX' power domains, they are not going to be utilized
> any time soon.
>
> When 'power-domains' list is just a single Titan GDSC,
> 'power-domain-names'
> property is not needed.
>
>> + - const: top
>> +
>> + vdda-pll-supply:
>> + description:
>> + Phandle to 1.2V regulator supply to PHY refclk pll block.
>> +
>> + vdda-phy0-supply:
>> + description:
>> + Phandle to 0.8V regulator supply to PHY core block.
>> +
>> + vdda-phy1-supply:
>> + description:
>> + Phandle to 0.8V regulator supply to PHY core block.
>> +
>> + vdda-phy2-supply:
>> + description:
>> + Phandle to 0.8V regulator supply to PHY core block.
>> +
>> + vdda-phy3-supply:
>> + description:
>> + Phandle to 0.8V regulator supply to PHY core block.
>> +
>> + vdda-phy4-supply:
>> + description:
>> + Phandle to 0.8V regulator supply to PHY core block.
>> +
>> + vdda-phy5-supply:
>> + description:
>> + Phandle to 0.8V regulator supply to PHY core block.
>
> What is the difference between vdda-phyX-supply properties, why do you
> need so many of them, when their descriptions say they are all the same?
Each of these supply power to a specific CSIPHY and could be different
based on the board architecture. But I agree that the description should
probably capture that than just relying on the name.
>
>> + ports:
>> + $ref: /schemas/graph.yaml#/properties/ports
>> +
>> + description:
>> + CSI input ports.
>> +
>> + properties:
>> + port@0:
>
> Please use
>
> patternProperties:
> "^port@[0-3]$":
>
>> + $ref: /schemas/graph.yaml#/$defs/port-base
>> + unevaluatedProperties: false
>> + description:
>> + Input port for receiving CSI data on CSI0.
>> +
>> + properties:
>> + endpoint:
>> + $ref: video-interfaces.yaml#
>> + unevaluatedProperties: false
>> +
>> + properties:
>> + clock-lanes:
>> + maxItems: 1
>
> Please remove 'clock-lanes' property, it is non-configurable, redundant
> and tends to store some irrelevant value.
>
>> +
>> + data-lanes:
>> + minItems: 1
>> + maxItems: 4
>> +
>> + bus-type:
>> + enum:
>> + - 1 # MEDIA_BUS_TYPE_CSI2_CPHY
>> + - 4 # MEDIA_BUS_TYPE_CSI2_DPHY
>> +
>> + required:
>> + - clock-lanes
>
> The 'clock-lanes' property is expected to be removed.
>
>> + - data-lanes
>> +
>> + port@1:
>> + $ref: /schemas/graph.yaml#/$defs/port-base
>> + unevaluatedProperties: false
>> + description:
>> + Input port for receiving CSI data on CSI1.
>> +
>> + properties:
>> + endpoint:
>> + $ref: video-interfaces.yaml#
>> + unevaluatedProperties: false
>> +
>> + properties:
>> + clock-lanes:
>> + maxItems: 1
>> +
>> + data-lanes:
>> + minItems: 1
>> + maxItems: 4
>> +
>> + bus-type:
>> + enum:
>> + - 1 # MEDIA_BUS_TYPE_CSI2_CPHY
>> + - 4 # MEDIA_BUS_TYPE_CSI2_DPHY
>> +
>> + required:
>> + - clock-lanes
>> + - data-lanes
>> +
>> + port@2:
>> + $ref: /schemas/graph.yaml#/$defs/port-base
>> + unevaluatedProperties: false
>> + description:
>> + Input port for receiving CSI data on CSI2.
>> +
>> + properties:
>> + endpoint:
>> + $ref: video-interfaces.yaml#
>> + unevaluatedProperties: false
>> +
>> + properties:
>> + clock-lanes:
>> + maxItems: 1
>> +
>> + data-lanes:
>> + minItems: 1
>> + maxItems: 4
>> +
>> + bus-type:
>> + enum:
>> + - 1 # MEDIA_BUS_TYPE_CSI2_CPHY
>> + - 4 # MEDIA_BUS_TYPE_CSI2_DPHY
>> +
>> + required:
>> + - clock-lanes
>> + - data-lanes
>> +
>> + port@3:
>> + $ref: /schemas/graph.yaml#/$defs/port-base
>> + unevaluatedProperties: false
>> + description:
>> + Input port for receiving CSI data on CSI3.
>> +
>> + properties:
>> + endpoint:
>> + $ref: video-interfaces.yaml#
>> + unevaluatedProperties: false
>> +
>> + properties:
>> + clock-lanes:
>> + maxItems: 1
>> +
>> + data-lanes:
>> + minItems: 1
>> + maxItems: 4
>> +
>> + bus-type:
>> + enum:
>> + - 1 # MEDIA_BUS_TYPE_CSI2_CPHY
>> + - 4 # MEDIA_BUS_TYPE_CSI2_DPHY
>> +
>> + required:
>> + - clock-lanes
>> + - data-lanes
>> +
>> +required:
>> + - compatible
>> + - reg
>> + - reg-names
>> + - clocks
>> + - clock-names
>> + - interrupts
>> + - interrupt-names
>> + - interconnects
>> + - interconnect-names
>> + - iommus
>> + - power-domains
>> + - power-domain-names
>> + - vdda-pll-supply
>> + - vdda-phy0-supply
>> + - vdda-phy1-supply
>> + - vdda-phy2-supply
>> + - vdda-phy3-supply
>> + - vdda-phy4-supply
>> + - vdda-phy5-supply
>
> Please exclude supplies from the list of required properties.
One of these supplies is required based which PHY the use case is being
run. Can you please advise how to handle that? Thanks.
>
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> + - |
>> + #include <dt-bindings/clock/qcom,rpmh.h>
>> + #include <dt-bindings/clock/qcom,kaanapali-camcc.h>
>> + #include <dt-bindings/clock/qcom,kaanapali-gcc.h>
>> + #include <dt-bindings/interconnect/qcom,icc.h>
>> + #include <dt-bindings/interconnect/qcom,kaanapali-rpmh.h>
>> + #include <dt-bindings/interrupt-controller/arm-gic.h>
>> + #include <dt-bindings/power/qcom,rpmhpd.h>
>
> Please keep the list of included headers sorted alphabetically.
>
>> +
>> + soc {
>> + #address-cells = <2>;
>> + #size-cells = <2>;
>> +
>> + camss: isp@...3000 {
>> + compatible = "qcom,kaanapali-camss";
>> +
>> + reg = <0x0 0x09253000 0x0 0x5e80>,
>> + <0x0 0x09263000 0x0 0x5e80>,
>> + <0x0 0x09273000 0x0 0x5e80>,
>> + <0x0 0x092d3000 0x0 0x3880>,
>> + <0x0 0x092e7000 0x0 0x3880>,
>> + <0x0 0x09523000 0x0 0x2000>,
>> + <0x0 0x09525000 0x0 0x2000>,
>> + <0x0 0x09527000 0x0 0x2000>,
>> + <0x0 0x09529000 0x0 0x2000>,
>> + <0x0 0x0952b000 0x0 0x2000>,
>> + <0x0 0x0952d000 0x0 0x2000>,
>> + <0x0 0x09151000 0x0 0x20000>,
>> + <0x0 0x09171000 0x0 0x20000>,
>> + <0x0 0x09191000 0x0 0x20000>,
>> + <0x0 0x092dc000 0x0 0x1300>,
>> + <0x0 0x092f0000 0x0 0x1300>;
>> + reg-names = "csid0",
>> + "csid1",
>> + "csid2",
>> + "csid_lite0",
>> + "csid_lite1",
>> + "csiphy0",
>> + "csiphy1",
>> + "csiphy2",
>> + "csiphy3",
>> + "csiphy4",
>> + "csiphy5",
>> + "vfe0",
>> + "vfe1",
>> + "vfe2",
>> + "vfe_lite0",
>> + "vfe_lite1";
>> +
>> + clocks = <&camcc CAM_CC_CAMNOC_NRT_AXI_CLK>,
>> + <&camcc CAM_CC_CAMNOC_RT_AXI_CLK>,
>> + <&camcc CAM_CC_CAMNOC_RT_TFE_0_MAIN_CLK>,
>> + <&camcc CAM_CC_CAMNOC_RT_TFE_1_MAIN_CLK>,
>> + <&camcc CAM_CC_CAMNOC_RT_TFE_2_MAIN_CLK>,
>> + <&camcc CAM_CC_CAMNOC_RT_IFE_LITE_CLK>,
>> + <&camcc CAM_CC_CAM_TOP_AHB_CLK>,
>> + <&camcc CAM_CC_CAM_TOP_FAST_AHB_CLK>,
>> + <&camcc CAM_CC_CSID_CLK>,
>> + <&camcc CAM_CC_CSID_CSIPHY_RX_CLK>,
>> + <&camcc CAM_CC_CSIPHY0_CLK>,
>> + <&camcc CAM_CC_CSI0PHYTIMER_CLK>,
>> + <&camcc CAM_CC_CSIPHY1_CLK>,
>> + <&camcc CAM_CC_CSI1PHYTIMER_CLK>,
>> + <&camcc CAM_CC_CSIPHY2_CLK>,
>> + <&camcc CAM_CC_CSI2PHYTIMER_CLK>,
>> + <&camcc CAM_CC_CSIPHY3_CLK>,
>> + <&camcc CAM_CC_CSI3PHYTIMER_CLK>,
>> + <&camcc CAM_CC_CSIPHY4_CLK>,
>> + <&camcc CAM_CC_CSI4PHYTIMER_CLK>,
>> + <&camcc CAM_CC_CSIPHY5_CLK>,
>> + <&camcc CAM_CC_CSI5PHYTIMER_CLK>,
>> + <&gcc GCC_CAMERA_HF_AXI_CLK>,
>> + <&camcc CAM_CC_QDSS_DEBUG_XO_CLK>,
>> + <&camcc CAM_CC_TFE_0_MAIN_CLK>,
>> + <&camcc CAM_CC_TFE_0_MAIN_FAST_AHB_CLK>,
>> + <&camcc CAM_CC_TFE_1_MAIN_CLK>,
>> + <&camcc CAM_CC_TFE_1_MAIN_FAST_AHB_CLK>,
>> + <&camcc CAM_CC_TFE_2_MAIN_CLK>,
>> + <&camcc CAM_CC_TFE_2_MAIN_FAST_AHB_CLK>,
>> + <&camcc CAM_CC_IFE_LITE_CLK>,
>> + <&camcc CAM_CC_IFE_LITE_AHB_CLK>,
>> + <&camcc CAM_CC_IFE_LITE_CPHY_RX_CLK>,
>> + <&camcc CAM_CC_IFE_LITE_CSID_CLK>;
>> + clock-names = "camnoc_nrt_axi",
>> + "camnoc_rt_axi",
>> + "camnoc_rt_vfe0",
>> + "camnoc_rt_vfe1",
>> + "camnoc_rt_vfe2",
>> + "camnoc_rt_vfe_lite",
>> + "cam_top_ahb",
>> + "cam_top_fast_ahb",
>> + "csid",
>> + "csid_csiphy_rx",
>> + "csiphy0",
>> + "csiphy0_timer",
>> + "csiphy1",
>> + "csiphy1_timer",
>> + "csiphy2",
>> + "csiphy2_timer",
>> + "csiphy3",
>> + "csiphy3_timer",
>> + "csiphy4",
>> + "csiphy4_timer",
>> + "csiphy5",
>> + "csiphy5_timer",
>> + "gcc_hf_axi",
>> + "qdss_debug_xo",
>> + "vfe0",
>> + "vfe0_fast_ahb",
>> + "vfe1",
>> + "vfe1_fast_ahb",
>> + "vfe2",
>> + "vfe2_fast_ahb",
>> + "vfe_lite",
>> + "vfe_lite_ahb",
>> + "vfe_lite_cphy_rx",
>> + "vfe_lite_csid";
>> +
>> + interrupts = <GIC_SPI 601 IRQ_TYPE_EDGE_RISING>,
>> + <GIC_SPI 603 IRQ_TYPE_EDGE_RISING>,
>> + <GIC_SPI 431 IRQ_TYPE_EDGE_RISING>,
>> + <GIC_SPI 605 IRQ_TYPE_EDGE_RISING>,
>> + <GIC_SPI 376 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 448 IRQ_TYPE_EDGE_RISING>,
>> + <GIC_SPI 122 IRQ_TYPE_EDGE_RISING>,
>> + <GIC_SPI 89 IRQ_TYPE_EDGE_RISING>,
>> + <GIC_SPI 433 IRQ_TYPE_EDGE_RISING>,
>> + <GIC_SPI 436 IRQ_TYPE_EDGE_RISING>,
>> + <GIC_SPI 457 IRQ_TYPE_EDGE_RISING>,
>> + <GIC_SPI 606 IRQ_TYPE_EDGE_RISING>,
>> + <GIC_SPI 377 IRQ_TYPE_EDGE_RISING>;
>> + interrupt-names = "csid0",
>> + "csid1",
>> + "csid2",
>> + "csid_lite0",
>> + "csid_lite1",
>> + "csiphy0",
>> + "csiphy1",
>> + "csiphy2",
>> + "csiphy3",
>> + "csiphy4",
>> + "csiphy5",
>> + "vfe0",
>> + "vfe1",
>> + "vfe2",
>> + "vfe_lite0",
>> + "vfe_lite1";
>> +
>> + interconnects = <&gem_noc MASTER_APPSS_PROC
>> QCOM_ICC_TAG_ACTIVE_ONLY
>> + &config_noc SLAVE_CAMERA_CFG
>> QCOM_ICC_TAG_ACTIVE_ONLY>,
>> + <&mmss_noc MASTER_CAMNOC_HF
>> QCOM_ICC_TAG_ALWAYS
>> + &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>;
>> + interconnect-names = "ahb",
>> + "hf_0_mnoc";
>> +
>> + iommus = <&apps_smmu 0x1c00 0x00>;
>> +
>> + power-domains = <&camcc CAM_CC_TFE_0_GDSC>,
>> + <&camcc CAM_CC_TFE_1_GDSC>,
>> + <&camcc CAM_CC_TFE_2_GDSC>,
>> + <&camcc CAM_CC_TITAN_TOP_GDSC>;
>> + power-domain-names = "tfe0",
>> + "tfe1",
>> + "tfe2",
>> + "top";
>> +
>> + vdda-pll-supply = <&vreg_l1d_1p2>;
>> + vdda-phy0-supply = <&vreg_l3i_0p8>;
>> + vdda-phy1-supply = <&vreg_l3i_0p8>;
>> + vdda-phy2-supply = <&vreg_l3d_0p8>;
>> + vdda-phy3-supply = <&vreg_l3i_0p8>;
>> + vdda-phy4-supply = <&vreg_l3d_0p8>;
>> + vdda-phy5-supply = <&vreg_l3i_0p8>;
>> +
>> + ports {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> +
>> + port@0 {
>> + reg = <0>;
>> +
>> + csiphy_ep0: endpoint {
>> + clock-lanes = <7>;
>> + data-lanes = <0 1>;
>> + remote-endpoint = <&sensor_ep>;
>> + };
>> + };
>> + };
>> + };
>> + };
>>
>
Powered by blists - more mailing lists