Re: [PATCH v3 3/4] arm64: dts: qcom: add QCS8300 platform

Konrad Dybcio
2024-11-28T20:14:36Z

On 28.11.2024 9:44 AM, Jingyi Wang wrote:
> Add initial DTSI for QCS8300 SoC.
>
> Features added in this revision:
> - CPUs with PSCI idle states
> - Interrupt-controller with PDC wakeup support
> - Timers, TCSR Clock Controllers
> - Reserved Shared memory
> - GCC and RPMHCC
> - TLMM
> - Interconnect
> - QuP with uart
> - SMMU
> - QFPROM
> - Rpmhpd power controller
> - UFS
> - Inter-Processor Communication Controller
> - SRAM
> - Remoteprocs including ADSP,CDSP and GPDSP
> - BWMONs
>
> Written with help from Zhenhua Huang(added the smmu node), Xin Liu(added
> ufs, adsp and gpdsp nodes), Tingguo Cheng(added the rpmhpd node), Kyle
> Deng(added the aoss_qmp node), Raviteja Laggyshetty(added interconnect
> nodes) and Cong Zhang(added the INTID of EL2 non-secure physical timer).
>
> Signed-off-by: Jingyi Wang <quic_jingyw@quicinc.com>
> ---

[...]

> + cpu-map {
> + cluster0 {
> + core0 {
> + cpu = <&cpu0>;
> + };
> +
> + core1 {
> + cpu = <&cpu1>;
> + };
> +
> + core2 {
> + cpu = <&cpu2>;
> + };
> +
> + core3 {
> + cpu = <&cpu3>;
> + };
> +
> + core4 {
> + cpu = <&cpu4>;
> + };

The MPIDR_EL1 register value (CPU node reg) suggests they are not
part of the same cluster (as you confirmed in the psci idle domains
description)

[...]

> +
> + ufs_mem_hc: ufs@1d84000 {
> + compatible = "qcom,qcs8300-ufshc", "qcom,ufshc", "jedec,ufs-2.0";
> + reg = <0x0 0x01d84000 0x0 0x3000>;
> + interrupts = <GIC_SPI 265 IRQ_TYPE_LEVEL_HIGH>;
> + phys = <&ufs_mem_phy>;
> + phy-names = "ufsphy";
> + lanes-per-direction = <2>;
> + #reset-cells = <1>;
> + resets = <&gcc GCC_UFS_PHY_BCR>;
> + reset-names = "rst";
> +
> + power-domains = <&gcc GCC_UFS_PHY_GDSC>;
> + required-opps = <&rpmhpd_opp_nom>;
> +
> + iommus = <&apps_smmu 0x100 0x0>;
> + dma-coherent;
> +
> + interconnects = <&aggre1_noc MASTER_UFS_MEM 0

QCOM_ICC_TAG_ALWAYS, file-wide

[...]

> + ufs_mem_phy: phy@1d87000 {
> + compatible = "qcom,qcs8300-qmp-ufs-phy", "qcom,sa8775p-qmp-ufs-phy";
> + reg = <0x0 0x01d87000 0x0 0xe10>;
> + /*
> + * Yes, GCC_EDP_REF_CLKREF_EN is correct in qref. It
> + * enables the CXO clock to eDP *and* UFS PHY.
> + */
> + clocks = <&rpmhcc RPMH_CXO_CLK>,
> + <&gcc GCC_UFS_PHY_PHY_AUX_CLK>,
> + <&gcc GCC_EDP_REF_CLKREF_EN>;

Are you sure about this, or is this just copypasted from sa8775p?

[...]

> +
> + intc: interrupt-controller@17a00000 {
> + compatible = "arm,gic-v3";
> + reg = <0x0 0x17a00000 0x0 0x10000>, /* GICD */
> + <0x0 0x17a60000 0x0 0x100000>; /* GICR * 8 */

Drop these comments

> + interrupts = <GIC_PPI 9 IRQ_TYPE_LEVEL_HIGH>;
> + #interrupt-cells = <3>;
> + interrupt-controller;
> + #redistributor-regions = <1>;
> + redistributor-stride = <0x0 0x20000>;
> + };
> +
> + memtimer: timer@17c20000 {

Unused label

[...]

> + arch_timer: timer {

Ditto

Konrad