Hi all,
On 1/16/25 3:47 PM, Quentin Schulz wrote:
This adds minimal support for the Pre-ICT tester adapter for RK3588
Jaguar.
GPIO3A3, GPIO3A4, GPIO3B2 and GPIO3D2 to GPIO3D5 are all routed to power
rails and can only be used as input and their bias are important to be
able to properly detect soldering issues.
Additionally, this adds build-time overlay application tests for (some)
Rockchip overlays to try to avoid future regressions.
Notably, the Device Trees from Wolfvision PF5 aren't migrated (but
should) as I do not own the device and couldn't figure out from the
introducing commit logs what the possible valid combinations are.
+Cc Michael Riesch for awareness
I'm wondering if we shouldn't backport patches 1 and 2 to stable? In
which case, it would make sense to try to have the Wolfvision PF5
overlay tests merged before the addition of the Pre-ICT tester adapter
support for RK3588 Jaguar as that one won't be backported to stable and
backporting the Wolfvision overlay test would incur an unnecessary
(though not difficult) git conflict to resolve.
I also do not know what kind of tests we should have when overlay
combinations are possible (let's say there are A, B and C overlays that
can all be applied, should we have base + A, base + B, base + C,
base + A + B, base + A + C, base + B + C and base + A + B + C tests?
maybe even base + B + A, base + C + B, base A + C + B, base + B + A + C,
base + B + C + A, base + C + B + A and base + C + A + B tests?).
Signed-off-by: Quentin Schulz <quentin.schulz@xxxxxxxxx>
---
Changes in v2:
- add overlay application tests for Edgeble NCM6A WiFi and Rock 5B PCIe
Endpoint+SNRS
- add overlay application test for RK3588 Jaguar + Pre-ICT tester
adapter,
This actually has a side effect.
Rockchip DTBs are compiled without symbols today for backward
compatibility reasons. Indeed, having symbols increases the size of the
DTB and by a rather non-negligible amount.
With this series applied (plus the change for the wolvision that was
intended for v3):
115K arch/arm64/boot/dts/rockchip/rk3568-wolfvision-pf5.dtb
162K arch/arm64/boot/dts/rockchip/rk3588-edgeble-neu6a-io.dtb
165K arch/arm64/boot/dts/rockchip/rk3588-jaguar.dtb
165K arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dtb
Without it:
57K arch/arm64/boot/dts/rockchip/rk3568-wolfvision-pf5.dtb
83K arch/arm64/boot/dts/rockchip/rk3588-edgeble-neu6a-io.dtb
87K arch/arm64/boot/dts/rockchip/rk3588-jaguar.dtb
86K arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dtb
As far as I remember, the issue is that we want to make sure that such a
bloated binary is not going to break things. Considering that U-Boot
passes the full DTB to TF-A and that upstream TF-A has support for
loading it on Rockchip since v2.1, this would be one piece of software
potentially impacted by the size increase.
TF-A v2.1 to v2.3 (both included) only had 64KiB available for loading
the DTB passed by the previous stage (U-Boot most likely).
https://review.trustedfirmware.org/plugins/gitiles/TF-A/trusted-firmware-a/+/7029e806833b94f729d9117bd35d488476b0e27e%5E%21/#F0
the commit introducing support for parsing the FDT, see the static array
of 0x10000 bytes (64KiB).
The buffer size got increased to 0x20000 bytes (128KiB) in v2.4, c.f.
https://review.trustedfirmware.org/plugins/gitiles/TF-A/trusted-firmware-a/+/8109f738ffa79a63735cba29da26e7c2859977b5%5E%21/#F0
Additionally, before v2.4, passing a DT too big would result in TF-A
crashing, c.f.
https://review.trustedfirmware.org/plugins/gitiles/TF-A/trusted-firmware-a/+/e7b586987c0a46660aa8402f19d626a5489fe449%5E%21/#F0
Unfortunately, Rockchip has seemingly decided v2.2 will be forever their
base version for their blobs. This means that we are forced to pass a DT
below 64KiB in size at the risk of crashing TF-A otherwise. Considering
that a memory misalignment can also make fdt_open_into() fail and thus
crash TF-A <= 2.3, c.f.
https://review.trustedfirmware.org/plugins/gitiles/TF-A/trusted-firmware-a/+/621acbd055d712ab8bf79054911155598fdb74d0%5E%21/#F0,
there's essentially too much risk to use DT with TF-A <= 2.3.
Rockchip being stuck on v2.2 for the binary blob is the reason why most
Rockchip boards supported by U-Boot do NOT actually pass the DT to TF-A,
c.f. SPL_ATF_NO_PLATFORM_PARAM symbol:
https://elixir.bootlin.com/u-boot/v2024.10/source/common/spl/Kconfig#L1446
So essentially, if SPL_ATF_NO_PLATFORM_PARAM is selected (the default
for RK356x, RK3588, forced on on RK3308, enabled for the majority of
RK3399 boards, enabled for all RK3328 boards) the DT won't be passed to
TF-A so no issue in terms of size on that side.
If it is not selected, for TF-A < 2.4 (released 20201117, 4 years ago),
a DTB bigger than 64KiB will crash TF-A.
If it is not selected, for TF-A >= 2.4, a DTB bigger than 128KiB will
result in TF-A not being able to read the DTB (for Rockchip, that means
not being able to derive the UART settings (controller and baudrate) to
use, and will use the compile-time default instead).
RPi seems to be loading it into a 1MiB buffer, Xilinx into a 2MiB
buffer, 64KiB for ARM FPGA targets and Allwinner.
We could/should increase the size of the buffer for the DTB passed to
TF-A but there's a limit. Indeed, there are many assumptions all over
U-Boot that TF-A only operates in the first 2MiB of DRAM and reserves it
for that purpose.
If I didn't misread the code, it seems
PX30/RK3328/RK3368/RK3399/RK356x/RK3588 upstream support only uses the
last 768KiB of the first 1MiB of DRAM, c.f. BL31_BASE and BL31_LIMIT. No
clue if that's a proper interpretation of the code or if I missed
something, that's a rather odd choice, considering Rockchip is adamant
we need to reserve 2MiB for their downstream blob.
Regardless of what can be done for TF-A in the future, the fact is that
it currently is limited to 128KiB in the best case scenario. This limit
is already reached by adding symbols to the DTB, which is a thing that
this patch series does.
Note that U-Boot typically does not use the kernel's DTB as its own, at
least for Rockchip, for now. It does compile from the same source (+
some additions in arch/arm/dts/*-u-boot.dtsi files) but without symbols
except if OF_OVERLAY_LIST symbol is defined, in which case the DTB
symbols are kept. This symbol is currently not enabled by any Rockchip
board.
In short, I don't know where to go with that additional piece of
information, but this is a bit bigger than simply moving things around
and adding compile-time tests for overlay application.
Cheers,
Quentin
