| 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: <3c35c36d-c116-4a1d-91c8-ae1ee2e1f840@linaro.org> Date: Fri, 31 Oct 2025 20:03:09 +0000 From: Bryan O'Donoghue <bryan.odonoghue@...aro.org> To: Vijay Kumar Tumati <vijay.tumati@....qualcomm.com>, 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 31/10/2025 17:39, Vijay Kumar Tumati wrote: > > 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@....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? TPG is in process of being upstreamed by qcom. I think the list of registers above should be included in the dts because the DTS is a description of hardware, not a description of camss/rdi. The point of DTS is to do that, describe hardware and to be consumable outside of the upstream linux kernel. u-boot, BSD, potentially even a downstream Linux kernel or driver. >>> + >>> + 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. > We've run into trouble with that in Iris. https://lore.kernel.org/linux-arm-msm/c9d8f76a-513f-4a09-bba4-cb8f0df1d2fe@kernel.org/ The right thing to do is to describe everything that targets the HLOS - main CPU. For non CPU targets - like say setting up the SMMU for the ICP - we could add those mappings in with iommu-map later. The CPU side SID map should be complete. It doesn't divulge any propitiatory information or secret sauce, it just makes our lives easier in the end. --- bod
Powered by blists - more mailing lists