| Message ID | 20250116-pre-ict-jaguar-v2-0-157d319004fc@cherry.de (mailing list archive) |
|---|---|
| Headers | show |
| Series | arm64: dts: rockchip: minimal support for Pre-ICT tester adapter for RK3588 Jaguar + add overlay tests | expand |
Hi Quentin, On 1/16/25 15:47, 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 Thanks for the heads up! Just to make sure I understood correctly: By migrated you mean that the overlay entries are moved to a separate section in the Makefile and there are explicit combinations of base DTS and overlays for compile-time testing purposes? If so, I don't consider the PF5 migration as *that* urgent. I don't think you can break anything on our side. Or am I missing something? Maybe you can move the lines dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-wolfvision-pf5-display-vz.dtbo dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-wolfvision-pf5-io-expander.dtbo to the overlay section as well? This should not cause any functional changes. > 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?). I have never been good at combinatorics, but I feel this has the potential to explode :-) My two cents: The overlays *should* be orthogonal to each other, i.e., no dependencies between them in the sense that overlay A creates a node that is used by overlay B and that sort of thing. Then - Permutation can be ignored. (base + A + C = base + C + A) - Composition (base + A + B + C) may be ignored in favor of individual tests. - Individual tests may be ignored in favor of (a) composition(s) that cover(s) all individual tests. But of course this is likely to vary from case to case. In our case, in the composition rk3568-wolfvision-pf5-vz-2-uhd := rk3568-wolfvision-pf5.dtb \ rk3568-wolfvision-pf5-io-expander.dtbo \ rk3568-wolfvision-pf5-display-vz.dtbo would do the trick. Thanks and regards, Michael > > Signed-off-by: Quentin Schulz <quentin.schulz@cherry.de> > --- > 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, > - Link to v1: https://lore.kernel.org/r/20241206-pre-ict-jaguar-v1-1-7f660bd4b70c@cherry.de > > --- > Quentin Schulz (3): > arm64: dts: rockchip: add overlay test for Edgeble NCM6A > arm64: dts: rockchip: add overlay tests for Rock 5B PCIe overlays > arm64: dts: rockchip: minimal support for Pre-ICT tester adapter for RK3588 Jaguar > > arch/arm64/boot/dts/rockchip/Makefile | 14 +- > .../dts/rockchip/rk3588-jaguar-pre-ict-tester.dtso | 171 +++++++++++++++++++++ > 2 files changed, 182 insertions(+), 3 deletions(-) > --- > base-commit: 619f0b6fad524f08d493a98d55bac9ab8895e3a6 > change-id: 20241206-pre-ict-jaguar-b90fafee8bd8 > > Best regards,
Hi Michael, On 1/20/25 10:06 AM, Michael Riesch wrote: > Hi Quentin, > > On 1/16/25 15:47, 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 > > Thanks for the heads up! > > Just to make sure I understood correctly: By migrated you mean that the > overlay entries are moved to a separate section in the Makefile and > there are explicit combinations of base DTS and overlays for > compile-time testing purposes? If so, I don't consider the PF5 migration > as *that* urgent. I don't think you can break anything on our side. Or > am I missing something? > I think it makes sense to backport the patches in this series (except the one adding support for the Pre-ICT tester adapter on RK3588 Jaguar). Because we cannot backport the patch for Pre-ICT tester adapter (it's an addition, not a bug fix), any patch that is applied after that one will result in a git conflict when backporting to stable releases. I believe it makes sense to backport build time overlay application tests. The git conflict will likely be no big deal but if we can avoid it, better avoid it :) > Maybe you can move the lines > > dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-wolfvision-pf5-display-vz.dtbo > dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-wolfvision-pf5-io-expander.dtbo > > to the overlay section as well? This should not cause any functional > changes. > The overlay section would only support the syntax which allows build time testing. I would like to avoid confusion around what to do when adding a new overlay. Specifically note the missing o in the extension with the build time tests. >> 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?). > > I have never been good at combinatorics, but I feel this has the > potential to explode :-) My two cents: The overlays *should* be > orthogonal to each other, i.e., no dependencies between them in the > sense that overlay A creates a node that is used by overlay B and that > sort of thing. Then I disagree. I can already tell you the following usecase: our Pre-ICT tester adapter for RK3588 Jaguar has two proprietary camera connectors. We already have two camera modules working with it, so the following would make sense: pre-ict-tester.dtbo pre-ict-tester-con1-camX.dtbo pre-ict-tester-con1-camY.dtbo pre-ict-tester-con2-camX.dtbo pre-ict-tester-con2-camY.dtbo You would then apply pre-ict-tester.dtbo followed by one or two cam dtbos. The pre-ict-tester can be used without the camera modules (c.f. this patch :) ). > - Permutation can be ignored. (base + A + C = base + C + A) I think that's fair. It would anyway be an issue with dtso which are using /delete-node/ or /delete-property/. > - Composition (base + A + B + C) may be ignored in favor of individual > tests. Not sure this is ideal either. Our RK3588 Jaguar main PCBA also has two proprietary camera connectors. It would make sense to test that applying a dtso for main PCBA is not messing with applying a dtso for the camera module on the adapter. This is a bit theoretical at the moment though since there's no camera stack available for RK3588. > - Individual tests may be ignored in favor of (a) composition(s) that > cover(s) all individual tests. > > But of course this is likely to vary from case to case. In our case, in > the composition > > rk3568-wolfvision-pf5-vz-2-uhd := rk3568-wolfvision-pf5.dtb \ > rk3568-wolfvision-pf5-io-expander.dtbo \ > rk3568-wolfvision-pf5-display-vz.dtbo > > would do the trick. > Thanks, will send a patch for that in v3. Cheers, Quentin
Hi Quentin, On 1/20/25 10:20, Quentin Schulz wrote: > Hi Michael, > > On 1/20/25 10:06 AM, Michael Riesch wrote: >> Hi Quentin, >> >> On 1/16/25 15:47, 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 >> >> Thanks for the heads up! >> >> Just to make sure I understood correctly: By migrated you mean that the >> overlay entries are moved to a separate section in the Makefile and >> there are explicit combinations of base DTS and overlays for >> compile-time testing purposes? If so, I don't consider the PF5 migration >> as *that* urgent. I don't think you can break anything on our side. Or >> am I missing something? >> > > I think it makes sense to backport the patches in this series (except > the one adding support for the Pre-ICT tester adapter on RK3588 Jaguar). > Because we cannot backport the patch for Pre-ICT tester adapter (it's an > addition, not a bug fix), any patch that is applied after that one will > result in a git conflict when backporting to stable releases. > > I believe it makes sense to backport build time overlay application tests. > > The git conflict will likely be no big deal but if we can avoid it, > better avoid it :) OK. > >> Maybe you can move the lines >> >> dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-wolfvision-pf5-display-vz.dtbo >> dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-wolfvision-pf5-io-expander.dtbo >> >> to the overlay section as well? This should not cause any functional >> changes. >> > > The overlay section would only support the syntax which allows build > time testing. I would like to avoid confusion around what to do when > adding a new overlay. Specifically note the missing o in the extension > with the build time tests. > >>> 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?). >> >> I have never been good at combinatorics, but I feel this has the >> potential to explode :-) My two cents: The overlays *should* be >> orthogonal to each other, i.e., no dependencies between them in the >> sense that overlay A creates a node that is used by overlay B and that >> sort of thing. Then > > I disagree. I can already tell you the following usecase: > > our Pre-ICT tester adapter for RK3588 Jaguar has two proprietary camera > connectors. We already have two camera modules working with it, so the > following would make sense: > > pre-ict-tester.dtbo > pre-ict-tester-con1-camX.dtbo > pre-ict-tester-con1-camY.dtbo > pre-ict-tester-con2-camX.dtbo > pre-ict-tester-con2-camY.dtbo > > You would then apply pre-ict-tester.dtbo followed by one or two cam > dtbos. The pre-ict-tester can be used without the camera modules (c.f. > this patch :) ). Most probably there is no general answer. In the end the use cases decide what tests do make sense. > >> - Permutation can be ignored. (base + A + C = base + C + A) > > I think that's fair. It would anyway be an issue with dtso which are > using /delete-node/ or /delete-property/. > >> - Composition (base + A + B + C) may be ignored in favor of individual >> tests. > > Not sure this is ideal either. > > Our RK3588 Jaguar main PCBA also has two proprietary camera connectors. > It would make sense to test that applying a dtso for main PCBA is not > messing with applying a dtso for the camera module on the adapter. > > This is a bit theoretical at the moment though since there's no camera > stack available for RK3588. > >> - Individual tests may be ignored in favor of (a) composition(s) that >> cover(s) all individual tests. >> >> But of course this is likely to vary from case to case. In our case, in >> the composition >> >> rk3568-wolfvision-pf5-vz-2-uhd := rk3568-wolfvision-pf5.dtb \ >> rk3568-wolfvision-pf5-io-expander.dtbo \ >> rk3568-wolfvision-pf5-display-vz.dtbo >> >> would do the trick. >> > > Thanks, will send a patch for that in v3. That'd be great, thanks! Regards, Michael > > Cheers, > Quentin
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@cherry.de> > --- > 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
On Wed, Jan 22, 2025 at 04:38:16PM +0100, Quentin Schulz wrote: > 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). Not everyone is using binary blobs from Rockchip. On my rock5b (rk3588), I'm building the bootloader using buildroot, which is using upstream TrustedFirmware-A (v2.12). > 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. This is significant information indeed. Kind regards, Niklas
Am Mittwoch, 22. Januar 2025, 17:12:26 CET schrieb Niklas Cassel: > On Wed, Jan 22, 2025 at 04:38:16PM +0100, Quentin Schulz wrote: > > 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). > > Not everyone is using binary blobs from Rockchip. > On my rock5b (rk3588), I'm building the bootloader using buildroot, > which is using upstream TrustedFirmware-A (v2.12). > > > > 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. > > This is significant information indeed. I guess the question is, can this hurt existing devices? As Quentin mentioned, this only affects DTs that get handed over from U-Boot to TF-A (and maybe OP-TEE). So the whole range of things loading their DT from extlinux.conf or whatever are not really affected. DTs U-Boot can hand over are 2 types, (1) built from within u-boot and (2) stored somewhere centrally (SPI flash). Case (1) is again not affected, as U-Boot (and other bootloaders) may very well sync the DTS files, but generally not the build-system, so if U-Boot (or any other bootloader) creates DTBs with symbols is completely their own choice. And for case (2) I see the manufacturer being responsible. Having the DT in central storage makes it somewhat part of a "bios"-level in the hirarchy and the general guarantee is that new software _will work_ with older DTs, but the other way around is more a nice to have (old SW with new DTB). So if some manufacturer has a centrally located DTB this does not matter until they upgrade, and when that happens I do expect testing to happen at the manufacturers side, before rolling out a "bios update" Heiko
On Thu, Jan 23, 2025 at 03:13:01PM +0100, Heiko Stübner wrote: > Am Mittwoch, 22. Januar 2025, 17:12:26 CET schrieb Niklas Cassel: > > On Wed, Jan 22, 2025 at 04:38:16PM +0100, Quentin Schulz wrote: > > > 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). > > > > Not everyone is using binary blobs from Rockchip. > > On my rock5b (rk3588), I'm building the bootloader using buildroot, > > which is using upstream TrustedFirmware-A (v2.12). > > > > > > > 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. > > > > This is significant information indeed. > > I guess the question is, can this hurt existing devices? > > As Quentin mentioned, this only affects DTs that get handed over from > U-Boot to TF-A (and maybe OP-TEE). > > So the whole range of things loading their DT from extlinux.conf or > whatever are not really affected. > > > DTs U-Boot can hand over are 2 types, > (1) built from within u-boot and > (2) stored somewhere centrally (SPI flash). > > > Case (1) is again not affected, as U-Boot (and other bootloaders) may > very well sync the DTS files, but generally not the build-system, so if > U-Boot (or any other bootloader) creates DTBs with symbols is completely > their own choice. > > > And for case (2) I see the manufacturer being responsible. Having the DT > in central storage makes it somewhat part of a "bios"-level in the hirarchy > and the general guarantee is that new software _will work_ with older DTs, > but the other way around is more a nice to have (old SW with new DTB). > > So if some manufacturer has a centrally located DTB this does not matter > until they upgrade, and when that happens I do expect testing to happen > at the manufacturers side, before rolling out a "bios update" Personally, I'm all for letting the kernel build the DTBs with symbols. (I have a patch that I keep rebasing on my tree only for that purpose. Sure, I could modify my build scripts to build the DTB separately, but with this patch, I do not need to do anything since the kernel builds the DTBs already.) Other platforms, e.g. TI already build many boards with symbols: https://github.com/torvalds/linux/blob/v6.13/arch/arm64/boot/dts/ti/Makefile#L242-L261 You seems to have been against enabling symbols before: https://lore.kernel.org/linux-rockchip/171941553475.921128.9467465539299233735.b4-ty@sntech.de/ https://lore.kernel.org/linux-rockchip/1952472.6tgchFWduM@diego/ But if you have changed you mind, and you are no longer concerned about doing so, then in my own self-interest I'm all for it :) Kind regards, Niklas
Am Freitag, 24. Januar 2025, 11:21:00 CET schrieb Niklas Cassel: > On Thu, Jan 23, 2025 at 03:13:01PM +0100, Heiko Stübner wrote: > > Am Mittwoch, 22. Januar 2025, 17:12:26 CET schrieb Niklas Cassel: > > > On Wed, Jan 22, 2025 at 04:38:16PM +0100, Quentin Schulz wrote: > > > > 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). > > > > > > Not everyone is using binary blobs from Rockchip. > > > On my rock5b (rk3588), I'm building the bootloader using buildroot, > > > which is using upstream TrustedFirmware-A (v2.12). > > > > > > > > > > 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. > > > > > > This is significant information indeed. > > > > I guess the question is, can this hurt existing devices? > > > > As Quentin mentioned, this only affects DTs that get handed over from > > U-Boot to TF-A (and maybe OP-TEE). > > > > So the whole range of things loading their DT from extlinux.conf or > > whatever are not really affected. > > > > > > DTs U-Boot can hand over are 2 types, > > (1) built from within u-boot and > > (2) stored somewhere centrally (SPI flash). > > > > > > Case (1) is again not affected, as U-Boot (and other bootloaders) may > > very well sync the DTS files, but generally not the build-system, so if > > U-Boot (or any other bootloader) creates DTBs with symbols is completely > > their own choice. > > > > > > And for case (2) I see the manufacturer being responsible. Having the DT > > in central storage makes it somewhat part of a "bios"-level in the hirarchy > > and the general guarantee is that new software _will work_ with older DTs, > > but the other way around is more a nice to have (old SW with new DTB). > > > > So if some manufacturer has a centrally located DTB this does not matter > > until they upgrade, and when that happens I do expect testing to happen > > at the manufacturers side, before rolling out a "bios update" > > Personally, I'm all for letting the kernel build the DTBs with symbols. > > (I have a patch that I keep rebasing on my tree only for that purpose. > Sure, I could modify my build scripts to build the DTB separately, > but with this patch, I do not need to do anything since the kernel > builds the DTBs already.) > > Other platforms, e.g. TI already build many boards with symbols: > https://github.com/torvalds/linux/blob/v6.13/arch/arm64/boot/dts/ti/Makefile#L242-L261 > > > You seems to have been against enabling symbols before: > https://lore.kernel.org/linux-rockchip/171941553475.921128.9467465539299233735.b4-ty@sntech.de/ > https://lore.kernel.org/linux-rockchip/1952472.6tgchFWduM@diego/ > > But if you have changed you mind, and you are no longer concerned about > doing so, then in my own self-interest I'm all for it :) I'm all for keeping compatibility as good as possible and that issue came on the table way too often already ;-) . In the past it was essentially easy to go with "just don't enable symbols" and not go down the nitty-gritty detail route - because that whole mesh of different firmware combinations gives me a headache ;-) [0] So finally going through those possible affected variants gave me those thoughts of "is there even an actual problem with existing boards?". Especially wrt forward<->backwards compatibility. Outcome is, I'm definitly not sure about myself, but also could not come up with an actual scenario. But that compile-time testing of applying DTBOs is way too great to pass up on :-) Heiko [0] If just some vendor would directly work on upstream TF-A from the beginning, instead of hacking up some half-decade old ATF ... ;-)
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@cherry.de> --- 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, - Link to v1: https://lore.kernel.org/r/20241206-pre-ict-jaguar-v1-1-7f660bd4b70c@cherry.de --- Quentin Schulz (3): arm64: dts: rockchip: add overlay test for Edgeble NCM6A arm64: dts: rockchip: add overlay tests for Rock 5B PCIe overlays arm64: dts: rockchip: minimal support for Pre-ICT tester adapter for RK3588 Jaguar arch/arm64/boot/dts/rockchip/Makefile | 14 +- .../dts/rockchip/rk3588-jaguar-pre-ict-tester.dtso | 171 +++++++++++++++++++++ 2 files changed, 182 insertions(+), 3 deletions(-) --- base-commit: 619f0b6fad524f08d493a98d55bac9ab8895e3a6 change-id: 20241206-pre-ict-jaguar-b90fafee8bd8 Best regards,