Hi all,
I've sent a few patches for fixing some compilation warnings or issues on Rockchip platforms [1][2][3][4][5] but I'm left with a few still...
[1] https://review.trustedfirmware.org/c/TF-A/trusted-firmware-a/+/33080 [2] https://review.trustedfirmware.org/c/TF-A/trusted-firmware-a/+/33081 [3] https://review.trustedfirmware.org/c/TF-A/trusted-firmware-a/+/33082 [4] https://review.trustedfirmware.org/c/TF-A/trusted-firmware-a/+/33083 [5] https://review.trustedfirmware.org/c/TF-A/trusted-firmware-a/+/33084
1) RK3588 symbol redefinition in assembly
======================================================================== commit fc3b98b2124c2b773322da0201e9ad7a697e6323 Author: Quentin Schulz quentin.schulz@cherry.de Date: Mon Oct 28 12:40:18 2024 +0100
fix(rk3588): pmu: fix assembly symbol redefinition
Somehow cpus_pd_req_enter_wfi() gets called multiple times and clang isn't happy about redefining the same label multiple times (it is an inline function).
An option could be to force the code to not be inlined (with __attribute__((noinline))) as removing the explicit inline still made the compiler inline the code.
Recreate the while loop without a label to fix the clang build issue: plat/rockchip/rk3588/drivers/pmu/pmu.c:763:7: error: symbol 'wfi_loop' is already defined 763 | "wfi_loop:\n" | ^ <inline asm>:5:1: note: instantiated into assembly here 5 | wfi_loop: | ^
Signed-off-by: Quentin Schulz quentin.schulz@cherry.de Change-Id: Ie9f55135b2f95a78deb7cbb94f9a62d3ba61e808
diff --git a/plat/rockchip/rk3588/drivers/pmu/pmu.c b/plat/rockchip/rk3588/drivers/pmu/pmu.c index f693dbdf0..f05d3e58f 100644 --- a/plat/rockchip/rk3588/drivers/pmu/pmu.c +++ b/plat/rockchip/rk3588/drivers/pmu/pmu.c @@ -760,10 +760,10 @@ static inline void cpus_pd_req_enter_wfi(void) "mrs x0, S3_0_C15_C2_7\n" "orr x0, x0, #0x1\n" "msr S3_0_C15_C2_7, x0\n" - "wfi_loop:\n" + "ldr x0, .+4\n" "isb\n" "wfi\n" - "b wfi_loop\n"); + "br x0\n"); }
static void nonboot_cpus_off(void) =========================================================================
This only happens for clang and I'm a bit confused as to why. Applying the above patch fixing that issue but inspecting the assembly code reveals something really odd.
**With** that patch, with gcc: 25c8c: d510149f msr dbgprcr_el1, xzr 25c90: d538f2e0 mrs x0, s3_0_c15_c2_7 25c94: b2400000 orr x0, x0, #0x1 25c98: d518f2e0 msr s3_0_c15_c2_7, x0 25c9c: 58000020 ldr x0, 0x25ca0 25ca0: d5033fdf isb 25ca4: d503207f wfi 25ca8: d61f0000 br x0
**with** that patch, with clang: 26d2c: d510149f msr dbgprcr_el1, xzr 26d30: d538f2e0 mrs x0, s3_0_c15_c2_7 26d34: b2400000 orr x0, x0, #0x1 26d38: d518f2e0 msr s3_0_c15_c2_7, x0 26d3c: 58000020 ldr x0, 0x26d40 <--- "my" ldr 26d40: d5033fdf isb 26d44: d503207f wfi 26d48: d61f0000 br x0 <--- "my" br 26d4c: 94000e76 bl 0x2a724 26d50: d510149f msr dbgprcr_el1, xzr 26d54: d538f2e0 mrs x0, s3_0_c15_c2_7 26d58: b2400000 orr x0, x0, #0x1 26d5c: d518f2e0 msr s3_0_c15_c2_7, x0 26d60: 58000020 ldr x0, 0x26d64 <--- "my" ldr 26d64: d5033fdf isb 26d68: d503207f wfi 26d6c: d61f0000 br x0 <--- "my" br
So I think the above patch is just a band-aid on what the actually is but I have no idea what could be going on.
If I use this patch instead, to match the other assembly instructions in this file
========================================================================= diff --git a/plat/rockchip/rk3588/drivers/pmu/pmu.c b/plat/rockchip/rk3588/drivers/pmu/pmu.c index f693dbdf0..a4128b214 100644 --- a/plat/rockchip/rk3588/drivers/pmu/pmu.c +++ b/plat/rockchip/rk3588/drivers/pmu/pmu.c @@ -760,10 +760,10 @@ static inline void cpus_pd_req_enter_wfi(void) "mrs x0, S3_0_C15_C2_7\n" "orr x0, x0, #0x1\n" "msr S3_0_C15_C2_7, x0\n" - "wfi_loop:\n" + "1:\n" "isb\n" "wfi\n" - "b wfi_loop\n"); + "b 1b\n"); }
static void nonboot_cpus_off(void) =========================================================================
It happily compiles, though the binary still seems to contain this loop twice.
Using __attribute__((noinline)) the code is not duplicated, but I assume this is expected as it would now simply be a function.
2) RK3399 warning array subscript 0 is outside array bounds of 'volatile unsigned int[0]' [-Warray-bounds=]
"""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""" In file included from src/dram.c:12: src/dram.c: In function 'm0_main': include/rk3399_mcu.h:15:34: warning: array subscript 0 is outside array bounds of 'volatile unsigned int[0]' [-Warray-bounds=] 15 | (*(volatile unsigned int *)(c)); __v; }) | ~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ include/rk3399_mcu.h:16:69: note: in definition of macro 'mmio_write_32' 16 | #define mmio_write_32(c, v) ((*(volatile unsigned int *)(c)) = (v)) | ^ src/dram.c:67:23: note: in expansion of macro 'mmio_read_32' 67 | mmio_read_32(PARAM_ADDR + PARAM_FREQ_SELECT)); | ^~~~~~~~~~~~ cc1: note: source object is likely at address zero In function 'ddr_set_pll', inlined from 'm0_main' at src/dram.c:71:2: include/rk3399_mcu.h:14:40: warning: array subscript 0 is outside array bounds of 'volatile unsigned int[0]' [-Warray-bounds=] 14 | #define mmio_read_32(c) ({unsigned int __v = \ | ^~~ include/rk3399_mcu.h:16:69: note: in definition of macro 'mmio_write_32' 16 | #define mmio_write_32(c, v) ((*(volatile unsigned int *)(c)) = (v)) | ^ src/dram.c:47:23: note: in expansion of macro 'mmio_read_32' 47 | mmio_read_32(PARAM_ADDR + PARAM_DPLL_CON0)); | ^~~~~~~~~~~~ In function 'm0_main': cc1: note: source object is likely at address zero In function 'ddr_set_pll', inlined from 'm0_main' at src/dram.c:71:2: include/rk3399_mcu.h:14:40: warning: array subscript 0 is outside array bounds of 'volatile unsigned int[0]' [-Warray-bounds=] 14 | #define mmio_read_32(c) ({unsigned int __v = \ | ^~~ include/rk3399_mcu.h:16:69: note: in definition of macro 'mmio_write_32' 16 | #define mmio_write_32(c, v) ((*(volatile unsigned int *)(c)) = (v)) | ^ src/dram.c:49:23: note: in expansion of macro 'mmio_read_32' 49 | mmio_read_32(PARAM_ADDR + PARAM_DPLL_CON1)); | ^~~~~~~~~~~~ In function 'm0_main': cc1: note: source object is likely at address zero include/rk3399_mcu.h:16:35: warning: array subscript 0 is outside array bounds of 'volatile unsigned int[0]' [-Warray-bounds=] 16 | #define mmio_write_32(c, v) ((*(volatile unsigned int *)(c)) = (v)) | ~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ src/dram.c:80:9: note: in expansion of macro 'mmio_write_32' 80 | mmio_write_32(PARAM_ADDR + PARAM_M0_DONE, M0_DONE_FLAG); | ^~~~~~~~~~~~~ cc1: note: source object is likely at address zero """""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""
This function is used in many places, but only in very specific places is it triggering a warning, when PARAM_ADDR is used, but only when accessed from m0 C files.
PARAM_ADDR is set to 0xc0. From !m0 files, access to PARAM_ADDR seems to be done through the M0_PARAM_ADDR which is M0_BINCODE_BASE + PARAM_ADDR. For m0 files, it's directly accessing PARAM_ADDR. I honestly don't really know why it's complaining for m0 files but not the !m0 ones.
3) RK3399 expected string in '.incbin' directive
This is only for clang.
""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""" <instantiation>:6:10: error: expected string in '.incbin' directive .incbin /home/qschulz/work/upstream/trusted-firmware-a/build/rk3399/release/m0/rk3399m0.bin ^ plat/rockchip/rk3399/drivers/pmu/pmu_fw.S:20:1: note: while in macro instantiation INCBIN """/home/qschulz/work/upstream/trusted-firmware-a/build/rk3399/release/m0/rk3399m0.bin""", "rk3399m0_bin", ".sram.incbin" ^ <instantiation>:6:10: error: expected string in '.incbin' directive .incbin /home/qschulz/work/upstream/trusted-firmware-a/build/rk3399/release/m0/rk3399m0pmu.bin ^ plat/rockchip/rk3399/drivers/pmu/pmu_fw.S:21:1: note: while in macro instantiation INCBIN """/home/qschulz/work/upstream/trusted-firmware-a/build/rk3399/release/m0/rk3399m0pmu.bin""", "rk3399m0pmu_bin", ".pmusram.incbin" ^ """""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""
With the following patch:
========================================================================= diff --git a/plat/rockchip/rk3399/drivers/pmu/pmu_fw.S b/plat/rockchip/rk3399/drivers/pmu/pmu_fw.S index 26f331317..ed70cba1f 100644 --- a/plat/rockchip/rk3399/drivers/pmu/pmu_fw.S +++ b/plat/rockchip/rk3399/drivers/pmu/pmu_fw.S @@ -11,7 +11,7 @@ .type \sym, @object .align 4 \sym : - .incbin \file + .incbin "\file" .size \sym , .-\sym .global \sym()_end \sym()_end : =========================================================================
clang seems happy with that error but the linker triggers another one:
ld.lld: warning: ignoring memory region assignment for non-allocatable section '.incbin_sram' ld.lld: error: section '.pmusram' will not fit in region 'PMUSRAM': overflowed by 3928 bytes
Would appreciate some guidance there :)
Cheers, Quentin