linux-kernel - Re: [PATCH v5 2/6] media: dt-bindings: Add CAMSS device for Kaanapali

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
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <e9da04ab-5119-4bfd-a25c-50e7b2ef05d3@oss.qualcomm.com>
Date: Fri, 31 Oct 2025 10:39:44 -0700
From: Vijay Kumar Tumati <vijay.tumati@....qualcomm.com>
To: Bryan O'Donoghue <bryan.odonoghue@...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>,
        Vladimir Zapolskiy <vladimir.zapolskiy@...aro.org>,
        Mauro Carvalho Chehab <mchehab@...nel.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 v5 2/6] media: dt-bindings: Add CAMSS device for Kaanapali


On 10/31/2025 6:50 AM, Bryan O'Donoghue wrote:
> On 31/10/2025 02:59, Hangxiang Ma wrote:
>> Add the compatible string "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
>>
>> Signed-off-by: Hangxiang Ma <hangxiang.ma@....qualcomm.com>
>> ---
>>   .../bindings/media/qcom,kaanapali-camss.yaml       | 406 
>> +++++++++++++++++++++
>>   1 file changed, 406 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..c34867022fd1
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/media/qcom,kaanapali-camss.yaml
>> @@ -0,0 +1,406 @@
>> +# 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
>
> No test pattern generator on this part ?
>
> We have patches in-flight to add TPG so it makes no sense to omit 
> these registers from current or new submissions.
>
> https://lore.kernel.org/linux-media/20251017-camss_tpg-v5-1-cafe3ad42163@oss.qualcomm.com/ 
>
>
> While we're at it we should consider adding in the other key 
> functional blocks.
>
> OFE, IPE etc, there's no harm in including the registers even if the 
> intention and outcome is never switching that functionality on.
>
Hi Bryan, we have quite a few register spaces on Kaanapali or any other 
target that are not required for the RDI only CAMSS driver, including 
ICP, JPEG, OFE, IPE, CDMs and some custom modules like CRE along with 
the TPG. So do I understand your suggestion correctly that you advise 
all of those are enlisted in the DTSI and the bindings although the 
driver doesn't make use of or map them?
>> +
>> +  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: 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
>> +
>> +  interrupts:
>> +    maxItems: 16
>> +
>> +  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
>> +
>> +  interconnects:
>> +    maxItems: 2
>> +
>> +  interconnect-names:
>> +    items:
>> +      - const: ahb
>> +      - const: hf_mnoc
>> +
>> +  iommus:
>> +    maxItems: 1
>
>
> This can't be right.
>
> The experience we are having with Iris for example shows that 
> restricting the iommus is wrong.
>
> For this and future bindings I'm expecting to see the full list of 
> AC_VM_HLOS S2 VMID targets.
>
> The second we try to switch on say something like the JPEG encoder 
> this list and its upstream binding becomes a problem.
>
> - S1_IFE_HLOS        @ 0x1c00
> - S1_CDM_BPS_IPS_HLOS    @ 0x1820
> - S1_CDM_BPS_IPS_HLOS    @ 0x18c0
> - S1_CDM_BPS_IPS_HLOS    @ 0x1980
> - S1_CDM_BPS_IPS_HLOS    @ 0x1800
> - S1_JPEG_HLOS        @ 0x18a0
> - S1_RT_CDM_HLOS    @ 0x1860
> - S1_CDM_BPS_IPE_HLOS    @ 0x1840
> - S1_CDM_BPS_IPE_HLOS    @ 0x1880
> - S1_CRE_HLOS        @ 0x18e0
>
> The ICP mappings can come later if ever via iommu-maps..
>
> ---
> bod
>
Similar to the above, You are advising to declare all the S2 HLOS mapped 
streams in the bindings and the DTSI? If we do that in the DTSI, I 
wonder how we can specifically map the RDI output buffers to the IFE 
context bank only, for instance, going by the current CAMSS driver 
implementation. Perhaps, IFE should be the first one in the list for now 
and the driver will be extended later when we support more devices? I 
will explore on that. Good to understand these details and practices. 
Thank you.