Messages in this thread | | | Date | Fri, 29 Sep 2023 16:42:15 +0530 | Subject | Re: [PATCH 2/2] arm64: dts: qcom: qcm6490: Add qcm6490 dts file | From | Komal Bajaj <> |
| |
On 9/28/2023 7:16 PM, Konrad Dybcio wrote: > On 28.09.2023 15:33, Komal Bajaj wrote: >> Add qcm6490 devicetree file for QCM6490 SoC and QCM6490 IDP >> platform. QCM6490 is derived from SC7280 meant for various >> form factor including IoT. >> >> Supported features are, as of now: >> * Debug UART >> * eMMC >> * USB >> >> Signed-off-by: Komal Bajaj <quic_kbajaj@quicinc.com> >> --- > [...] > >> +/ { >> + model = "Qualcomm Technologies, Inc. QCM6490 IDP platform"; > Isomething Development Platform platform?
Will drop suffix platform.
> >> + compatible = "qcom,qcm6490-idp", "qcom,qcm6490"; >> + >> + aliases { >> + serial0 = &uart5; >> + }; >> + >> + chosen { >> + stdout-path = "serial0:115200n8"; >> + }; >> + >> +}; > Stray newline above
Noted.
> >> + >> +&apps_rsc { >> + regulators-0 { >> + compatible = "qcom,pm7325-rpmh-regulators"; >> + qcom,pmic-id = "b"; >> + >> + vreg_s1b_1p8: smps1 { >> + regulator-min-microvolt = <1856000>; >> + regulator-max-microvolt = <2040000>; > Hm, you didn't specify regulator-initial-mode on any vregs
Okay, will specify it in next patchset.
> > [...] > >> --- /dev/null >> +++ b/arch/arm64/boot/dts/qcom/qcm6490.dtsi >> @@ -0,0 +1,137 @@ >> +// SPDX-License-Identifier: BSD-3-Clause >> +/* >> + * Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights reserved. >> + */ >> + >> +#include "sc7280.dtsi" >> + >> +/* >> + * Delete sc7280 lpass related nodes as qcm6490 intends to move away from >> + * bypass configuration >> + */ >> +/delete-node/ &lpass_ag_noc; >> +/delete-node/ &lpass_aon; >> +/delete-node/ &lpass_audiocc; >> +/delete-node/ &lpass_core; >> +/delete-node/ &lpass_cpu; >> +/delete-node/ &lpass_hm; >> +/delete-node/ &lpass_rx_macro; >> +/delete-node/ &lpass_tx_macro; >> +/delete-node/ &lpass_va_macro; >> +/delete-node/ &lpass_tlmm; >> +/delete-node/ &lpasscc; >> +/delete-node/ &swr0; >> +/delete-node/ &swr1; > That's very unnecessary, most of these nodes are in use even > when routing audio through ADSP. > > Ones that are not, are set to status = "reserved" and some > will need more work to function based on the configuration. > > There was once a series from somebody at qc to introduce ADSP > audio on 7280, but it was full of hacks and NAKed
Seeking clarification which nodes will be used and which should be disabled. For now, will remove it.
> > >> + >> +/* >> + * Delete unused sc7280 memory nodes and define the memory regions >> + * required by qcm6490 >> + */ > That's specific to your board.
No, these are not board specific, it's soc specific.
> >> +/delete-node/ &rmtfs_mem; >> +/delete-node/ &wlan_ce_mem; >> + >> +/{ >> + reserved-memory { >> + secdata_apss_mem: secdata-apss@808ff000 { >> + reg = <0x0 0x808ff000 0x0 0x1000>; >> + no-map; >> + }; > This is identical to the entry in sc7280.dtsi.
Yeah, right. Will remove it.
> >> + >> + cdsp_secure_heap_mem: cdsp-secure-heap@81800000 { >> + reg = <0x0 0x81800000 0x0 0x1e00000>; >> + no-map; >> + }; >> + >> + camera_mem: camera@84300000 { >> + reg = <0x0 0x84300000 0x0 0x500000>; >> + no-map; >> + }; >> + >> + adsp_mem: adsp@86100000 { >> + reg = <0x0 0x86100000 0x0 0x2800000>; >> + no-map; >> + }; >> + >> + cdsp_mem: cdsp@88900000 { >> + reg = <0x0 0x88900000 0x0 0x1e00000>; >> + no-map; >> + }; >> + >> + cvp_mem: cvp@8ac00000 { >> + reg = <0x0 0x8ac00000 0x0 0x500000>; >> + no-map; >> + }; >> + >> + ipa_gsi_mem: ipa-gsi@8b110000 { >> + reg = <0x0 0x8b110000 0x0 0xa000>; >> + no-map; >> + }; >> + >> + gpu_microcode_mem: gpu-microcode@8b11a000 { >> + reg = <0x0 0x8b11a000 0x0 0x2000>; >> + no-map; >> + }; >> + >> + mpss_mem: mpss@8b800000 { >> + reg = <0x0 0x8b800000 0x0 0xf600000>; >> + no-map; >> + }; >> + >> + wpss_mem: wpss@9ae00000 { >> + reg = <0x0 0x9ae00000 0x0 0x1900000>; >> + no-map; >> + }; > This entry is in both chrome-common and fairphone (meaning all boards > use it), perhaps this one could be moved to 7280.dtsi
Missed to update region for wpss_mem. It's different what's used in chrome-common and fairphone.
> >> + >> + tz_stat_mem: tz-stat@c0000000 { >> + reg = <0x0 0xc0000000 0x0 0x100000>; >> + no-map; >> + }; >> + >> + tags_mem: tags@c0100000 { >> + reg = <0x0 0xc0100000 0x0 0x1200000>; >> + no-map; >> + }; >> + >> + qtee_mem: qtee@c1300000 { >> + reg = <0x0 0xc1300000 0x0 0x500000>; >> + no-map; >> + }; >> + >> + trusted_apps_mem: trusted_apps@c1800000 { >> + reg = <0x0 0xc1800000 0x0 0x3900000>; >> + no-map; >> + }; >> + }; >> +}; >> + >> +&hyp_mem { >> + reg = <0x0 0x80000000 0x0 0x600000>; > This is identical to the entry in sc7280.dtsi.
Will remove it.
> >> +}; >> + >> +&mpss_mem { >> + reg = <0x0 0x8b800000 0x0 0xf600000>; > You're defining it here and overwriting it with an identical > value. > > Looks like CrOS folks don't boot up the modem on non-LTE SKUs. > Weird. Normally it would host some more software.. > >> +}; >> + >> +&remoteproc_mpss { >> + memory-region = <&mpss_mem>; > This is identical to the entry in sc7280.dtsi.
Will remove it.
> >> +}; >> + >> +&video_mem { >> + reg = <0x0 0x8a700000 0x0 0x500000>; >> +}; >> + >> +&wifi { >> + memory-region = <&wlan_fw_mem>; > No CE region?
Yes, no CE region is required here.
> >> +}; >> + >> +&wlan_fw_mem { >> + reg = <0x0 0x80c00000 0x0 0xc00000>; > This is identical to the entry in sc7280.dtsi.
Will remove it.
> > > The memory map generally looks quite different to both chrome > and fairphone.. are you sure it's all correct?
Yes, it's correct, it's different than chrome and fairphone.
Thanks Komal
> > Konrad
| |