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

[Marek Vasut]

On 4/27/20 3:31 PM, Adam Ford wrote:

[...]

>>>>> +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

And yet, upstream ATF documents support for imx8mm as a PLAT= build
target, but I ran into the same problem with Variscite iMX8MM board,
that ATF is completely broken :-(

> 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.

That's OK, thanks for clarifying.

(I also saw you ran into problems with iMX8MM clock and power domains,
which are basically identical to MX8MQ, but it seems like there is a
push for implementing this through PSCI, which makes the MX8MM far less
appealing)

>> [...]
>>
>>>>> +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

Do you think you can fix the other sites which implement the do_reset()
too ? That would be fantastic.

>> 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.

Except if something calls hang(), the board is stuck instead of
rebooting and trying again. I think you might want to look at the
sysreset_watchdog.c, it implements basically the same thing, except via
DM and without hacks in board files.