On 5/14/2024 12:37 AM, Trilok Soni wrote: > On 5/13/2024 2:07 AM, Tengfei Fan wrote: >> QCS8550 is derived from SM8550. The differnece between SM8550 and > > spellcheck s/difference/difference > >> QCS8550 is QCS8550 doesn't have modem RF system. QCS8550 is mainly used >> in IoT scenarios. > > IoT products and not scenarios. > >> QCS8550 firmware has different memory map with SM8550 firmware. > > "QCS8550 firmware has different memory map compared to SM8550" > > > The >> memory map will be runtime added through bootloader. > > >> There are 3 types of reserved memory regions here: >> 1. Firmware related regions which aren't shared with kernel. >> The device tree source in kernel doesn't need to have node to indicate >> the firmware related reserved information. OS bootloader conveys the > > Just "Bootloader conveys the information by updating devicetree at runtime" ? > >> information by update device tree in runtime. >> This will be described as: UEFI saves the physical address of the >> UEFI System Table to dts file's chosen node. Kernel read this table and >> add reserved memory regions to efi config table. Current reserved memory >> region may have reserved region which was not yet used, release note of >> the firmware have such kind of information. > > I understand what you are trying to explain below, but can we simplify further? I > had to read multiple times to understand what you are trying to convey above. > >> 2. Firmware related memory regions which are shared with Kernel >> Each region has a specific node with specific label name for later >> phandle reference from other driver dt node. How about like this: 2. Firmware related memory regions which are shared with Kernel The device tree source in the kernel needs to include nodes that indicate firmware-related shared information. A label name is suggested because this type of shared information needs to be referenced by specific drivers for handling purposes. >> 3. PIL regions. > > Do we use the PIL - peripheral image loader in the upstream kernel or just remoteproc? > I am fine w/ PIL if it is used at other places in Qualcomm remoteproc. > >> PIL regions will be reserved and then assigned to subsystem firmware >> later. >> Here is a reserved memory map for this platform: >> 0x100000000 +------------------+ >> | | >> | Firmware Related | >> | | >> 0xd4d00000 +------------------+ >> | | >> | Kernel Available | > > What is "kernel available" means? It means not reserved memory, normal available memory from kernel point of view. > >> | | >> 0xa7000000 +------------------+ >> | | >> | PIL Region | >> | | >> 0x8a800000 +------------------+ >> | | >> | Firmware Related | >> | | >> 0x80000000 +------------------+ > >> Note that: > > Do we need to write "Note that:" ? > >> 0xa7000000..0xA8000000 is used by bootloader, when kernel boot up, >> it is available for kernel usage. This region is not suggested to be >> used by kernel features like ramoops, suspend resume etc. >> >> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx> >> Signed-off-by: Tengfei Fan <quic_tengfan@xxxxxxxxxxx> >> --- >> arch/arm64/boot/dts/qcom/qcs8550.dtsi | 169 ++++++++++++++++++++++++++ >> 1 file changed, 169 insertions(+) >> create mode 100644 arch/arm64/boot/dts/qcom/qcs8550.dtsi >> >> diff --git a/arch/arm64/boot/dts/qcom/qcs8550.dtsi b/arch/arm64/boot/dts/qcom/qcs8550.dtsi >> new file mode 100644 >> index 000000000000..a3ebf3d4e16d >> --- /dev/null >> +++ b/arch/arm64/boot/dts/qcom/qcs8550.dtsi >> @@ -0,0 +1,169 @@ >> +// SPDX-License-Identifier: BSD-3-Clause >> +/* >> + * Copyright (c) 2023-2024, Qualcomm Innovation Center, Inc. All rights reserved. >> + */ >> + >> +#include "sm8550.dtsi" >> + >> +/delete-node/ &reserved_memory; >> + >> +/ { >> + reserved_memory: reserved-memory { >> + #address-cells = <2>; >> + #size-cells = <2>; >> + ranges; >> + >> + >> + /* These are 3 types of reserved memory regions here: >> + * 1. Firmware related regions which aren't shared with kernel. >> + * The device tree source in kernel doesn't need to have node to >> + * indicate the firmware related reserved information. OS bootloader >> + * conveys the information by update device tree in runtime. >> + * This will be described as: UEFI saves the physical address of >> + * the UEFI System Table to dts file's chosen node. Kernel read this >> + * table and add reserved memory regions to efi config table. Current >> + * reserved memory region may have reserved region which was not yet >> + * used, release note of the firmware have such kind of information. >> + * 2. Firmware related memory regions which are shared with Kernel. >> + * Each region has a specific node with specific label name for >> + * later phandle reference from other driver dt node. >> + * 3. PIL regions. >> + * PIL regions will be reserved and then assigned to subsystem >> + * firmware later. >> + * Here is a reserved memory map for this platform: > > Just check the comment above and it will apply here. > >> + * 0x100000000 +------------------+ >> + * | | >> + * | Firmware Related | >> + * | | >> + * 0xd4d00000 +------------------+ >> + * | | >> + * | Kernel Available | >> + * | | >> + * 0xa7000000 +------------------+ >> + * | | >> + * | PIL Region | >> + * | | >> + * 0x8a800000 +------------------+ >> + * | | >> + * | Firmware Related | >> + * | | >> + * 0x80000000 +------------------+ >> + * Note that: >> + * 0xa7000000..0xA8000000 is used by bootloader, when kernel boot up, >> + * it is available for kernel usage. This region is not suggested to >> + * be used by kernel features like ramoops, suspend resume etc. >> + */ >> + >> + /* >> + * Firmware related regions, bootlader will possible reserve parts of > > spellcheck s/bootlader/bootloader > >> + * region from 0x80000000..0x8a800000. >> + */ >> + aop_image_mem: aop-image-region@81c00000 { >> + reg = <0x0 0x81c00000 0x0 0x60000>; >> + no-map; >> + }; >> + >> + aop_cmd_db_mem: aop-cmd-db-region@81c60000 { >> + compatible = "qcom,cmd-db"; >> + reg = <0x0 0x81c60000 0x0 0x20000>; >> + no-map; >> + }; >> + >> + aop_config_mem: aop-config-region@81c80000 { >> + no-map; >> + reg = <0x0 0x81c80000 0x0 0x20000>; >> + }; >> + >> + smem_mem: smem-region@81d00000 { >> + compatible = "qcom,smem"; >> + reg = <0x0 0x81d00000 0x0 0x200000>; >> + hwlocks = <&tcsr_mutex 3>; >> + no-map; >> + }; >> + >> + adsp_mhi_mem: adsp-mhi-region@81f00000 { >> + reg = <0x0 0x81f00000 0x0 0x20000>; >> + no-map; >> + }; >> + >> + /* PIL region */ >> + mpss_mem: mpss-region@8a800000 { >> + reg = <0x0 0x8a800000 0x0 0x10800000>; >> + no-map; >> + }; >> + >> + q6_mpss_dtb_mem: q6-mpss-dtb-region@9b000000 { >> + reg = <0x0 0x9b000000 0x0 0x80000>; >> + no-map; >> + }; >> + >> + ipa_fw_mem: ipa-fw-region@9b080000 { >> + reg = <0x0 0x9b080000 0x0 0x10000>; >> + no-map; >> + }; >> + >> + ipa_gsi_mem: ipa-gsi-region@9b090000 { >> + reg = <0x0 0x9b090000 0x0 0xa000>; >> + no-map; >> + }; >> + >> + gpu_micro_code_mem: gpu-micro-code-region@9b09a000 { >> + reg = <0x0 0x9b09a000 0x0 0x2000>; >> + no-map; >> + }; >> + >> + spss_region_mem: spss-region@9b100000 { >> + reg = <0x0 0x9b100000 0x0 0x180000>; >> + no-map; >> + }; >> + >> + spu_secure_shared_memory_mem: spu-secure-shared-memory-region@9b280000 { >> + reg = <0x0 0x9b280000 0x0 0x80000>; >> + no-map; >> + }; >> + >> + camera_mem: camera-region@9b300000 { >> + reg = <0x0 0x9b300000 0x0 0x800000>; >> + no-map; >> + }; >> + >> + video_mem: video-region@9bb00000 { >> + reg = <0x0 0x9bb00000 0x0 0x700000>; >> + no-map; >> + }; >> + >> + cvp_mem: cvp-region@9c200000 { >> + reg = <0x0 0x9c200000 0x0 0x700000>; >> + no-map; >> + }; >> + >> + cdsp_mem: cdsp-region@9c900000 { >> + reg = <0x0 0x9c900000 0x0 0x2000000>; >> + no-map; >> + }; >> + >> + q6_cdsp_dtb_mem: q6-cdsp-dtb-region@9e900000 { >> + reg = <0x0 0x9e900000 0x0 0x80000>; >> + no-map; >> + }; >> + >> + q6_adsp_dtb_mem: q6-adsp-dtb-region@9e980000 { >> + reg = <0x0 0x9e980000 0x0 0x80000>; >> + no-map; >> + }; >> + >> + adspslpi_mem: adspslpi-region@9ea00000 { >> + reg = <0x0 0x9ea00000 0x0 0x4080000>; >> + no-map; >> + }; >> + >> + /* >> + * Firmware related regions, bootlader will possible reserve parts of > > Ditto. > >> + * region from 0xd8000000..0x100000000. >> + */ >> + mpss_dsm_mem: mpss_dsm_region@d4d00000 { >> + reg = <0x0 0xd4d00000 0x0 0x3300000>; >> + no-map; >> + }; >> + }; >> +}; -- Thx and BRs, Aiqun(Maria) Yu