On Mon, Apr 27, 2020 at 2:06 AM Marek Vasut marex@denx.de wrote:
On 4/27/20 4:26 AM, Adam Ford wrote:
Hi,
[...]
diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile index 59a2713cb2..249d446f69 100644 --- a/arch/arm/dts/Makefile +++ b/arch/arm/dts/Makefile @@ -730,7 +730,8 @@ dtb-$(CONFIG_ARCH_IMX8M) += \ imx8mm-verdin.dtb \ imx8mn-ddr4-evk.dtb \ imx8mq-evk.dtb \
imx8mp-evk.dtb
imx8mp-evk.dtb \imx8mm-beacon-kit.dtbKeep the list sorted.
Before verdin or before ddr4-evk? It's already out of order.
Alphabetical (and if it's not sorted, it should be. can you send a patch to sort it ?)
[...]
diff --git a/board/beacon/imx8mm/README b/board/beacon/imx8mm/README new file mode 100644 index 0000000000..6dc916fc10 --- /dev/null +++ b/board/beacon/imx8mm/README @@ -0,0 +1,37 @@ +U-Boot for the Beacon EmbeddedWorks Devkit
+Quick Start +=========== +- Build the ARM Trusted firmware binary +- Get ddr fimware +- Build U-Boot +- Boot
+Get and Build the ARM Trusted firmware +====================================== +Note: srctree is U-Boot source directory
+$ git clone https://source.codeaurora.org/external/imx/imx-atf +$ git checkout imx_4.19.35_1.0.0 +$ make PLAT=imx8mm bl31 ARCH=arm CROSS_COMPILE=aarch64-linux-gnu- +$ cp build/imx8mm/release/bl31.bin $(srctree)
Why not use mainline TFA instead of this NXP one ?
The testing of upstream kernel code appears to require the NXP ATF instead of upstream ATF. Some of the DDRC stuff doesnt' appear to work with upstream ATF.
Are there any more details on this ?
Yes,
There was some dialog between me and one of the NXP developers adding DDRC support to i.MX8MM.
https://lore.kernel.org/linux-arm-kernel/CAHCN7xJc8yMe683wsB1e1TdE26FX1oMFT_...
In his message in the above link, he explitly wrote: You need a recent version of TF-A from nxp ( upstream). Try this:
https://source.codeaurora.org/external/imx/imx-atf/log/?h=imx_4.19.35_1.1.0
Or this: https://github.com/cdleonard/arm-trusted-firmware/commits/imx_2.0.y_busfreq
Support on upstream ATF is not yet available
Until that is available upstream, I was going to reference the NXP version because that's also what NXP distributes, and it's consistent with their BSP. Once the support is available upstream for ATF, I was planning on pointing there.
[...]
+int do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) +{
puts("resetting ...\n");reset_cpu(WDOG1_BASE_ADDR);return 0;+}
Please do not reimplement reset in SPL, just use some reset driver. This must also be fixed on the NXP devkits, because they are broken and wrong.
What should they be doing instead? I am just modeling it after the NXP dev kit and it appears to work.
Sure, it "works", but it's not implemented correctly. This NXP devkit is literally the only board which hacks around missing do_reset() in their SPL by implementing it in board files and proliferation of this bug must be stopped.
I think I found a solution which removes the need for this completely based on doc/README.SPL
Either arch/arm/lib/reset.c or drivers/sysreset/sysreset_watchdog.c should be used, in this case it's likely the later.
Since I don't know what the 'right' was is, I don't know what I need to do. Ideally, it would be nice if NXP had platform-common code that would do it
Reset implementation is not platform code, it shouldn't be there, see drivers/sysreset/ .
For the benefit of anyone else with an i.MX8M Mini, I found that that if I enable CONFIG_PANIC_HANG, the need for this function disappears, and it still appears to boot normally from microSD.
instead of a series of board files with nearly identical code.
I agree the amount of duplication in board files should be reduced.