[PATCH] imx: Add support for i.MX8MM Beacon EmbeddedWorks devkit.

[Adam Ford]

On Mon, Apr 27, 2020 at 2:06 AM Marek Vasut <marex at 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.dtb
> >>
> >> Keep 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_i_hshkEECrJ52oHg@mail.gmail.com/T/

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.