[U-Boot] db-88f6820-amc board fails to boot with latest u-boot master

[Stefan Roese]

On 15.02.2017 01:57, Chris Packham wrote:
> On Wed, Feb 15, 2017 at 11:56 AM, Chris Packham <judge.packham at gmail.com> wrote:
>> On Tue, Feb 14, 2017 at 11:01 PM, Mario Six <mario.six at gdsys.cc> wrote:
>>> On Tue, Feb 14, 2017 at 10:26 AM, Chris Packham <judge.packham at gmail.com> wrote:
>>>> On Tue, Feb 14, 2017 at 7:56 PM, Mario Six <mario.six at gdsys.cc> wrote:
>>>>> On Tue, Feb 14, 2017 at 7:36 AM, Stefan Roese <sr at denx.de> wrote:
>>>>>> Hi Chris,
>>>>>>
>>>>>> On 14.02.2017 02:48, Chris Packham wrote:
>>>>>>>
>>>>>>> I just tried UART booting db-88f6820-amc with the latest u-boot master
>>>>>>> and it fails to reach the normal u-boot prompt. I don't know if this
>>>>>>> affects the normal SPI booting.
>>>>>>>
>>>>>>> I've bisected down to commit 94084eea3bd3 ("tools: kwbimage: Fix dest
>>>>>>> addr").
>>>>>>
>>>>>>
>>>>>> Thanks for checking and reporting.
>>>>>>
>>>>>>> Reverting this gets things working again, but I'm guessing it'll break
>>>>>>> secure boot. Should the size adjustment be conditional on something?
>>>>>>
>>>>>>
>>>>>> Might be, not sure.
>>>>>>
>>>>>>> Does db-88f6820-amc_defconfig need something that the other armada
>>>>>>> configs have enabled?
>>>>>>
>>>>>>
>>>>>> I don't think so.
>>>>>>
>>>>>> Mario, could you please take a look at this so that we find a solution
>>>>>> for the upcoming release?
>>>>>>
>>>>>
>>>>> I'll take a look. The quick-fix solution would be to only do the
>>>>> address adjustment if secure boot is active.
>>>>>
>>>>> But the weird thing is that the address should not matter for the
>>>>> non-secure boot images, since for them the SPL (which is run from a
>>>>> BIN header) directly loads and starts the main U-Boot, hence the
>>>>> BootROM's image loading process is completely bypassed, and the
>>>>> destination address from the header is never used. Does the
>>>>> db-88f6820-amc do anything different in that regard?
>>>>>
>>>>
>>>> No the amc board is pretty standard. Just doesn't have the extra SATA
>>>> and PCI-e stuff you get on the bigger GP board.
>>>>
>>>
>>> OK, thanks for clarifying.
>>>
>>> Well, I just tried it with our GP board, and it works with and without
>>> the address adjustment, as expected.
>>>
>>> Caveat: I'm booting from an SD card (I changed the U-Boot config
>>> around to build a suitable image). There definitely are *some*
>>> differences between the boot flows on different boot mediums (I never
>>> got the jump back into the BootROM to work on SD boot, for example).
>>> So if you boot from another medium, this might be the problem.
>>
>> For the AMC board my only option is SPI (still haven't tried that with
>> a "broken" build) and UART. UART definitely fails for me.
>>
>>
>>>> I notice in that commit series you talk about the .img file. But the
>>>> default build target is u-boot-spl.kwb[1]. That's what I've always
>>>> loaded onto the armada boards.
>>>
>>> The u-boot-spl.kwb is the correct image. But this image contains the
>>> u-boot.img as a payload. And since this image has the header
>>> prepended, we have to subtract the header size from the destination
>>> address, so that the entry point lines up correctly if we want to jump
>>> back into the BootROM (which is necessary for secure boot to work).
>>> But for boards that don't jump back into the BootROM, this doesn't
>>> matter, since the BootROM doesn't actually load the main U-Boot in
>>> this case, but the SPL does (and the SPL doesn't care about some
>>> address in a SoC-specific header; hence my consternation).
>>
>> Maybe this is UART boot specific. The addressing is probably a bit
>> special and secure boot probably does not apply. Up to now it's been
>> neat trick that kwboot can patch the image on the fly. Maybe it needs
>> to be a bit smarter. I note that Marvell's build process actually
>> produces a separate -uart version of the image, maybe this is why.
>
> I think the problem is commit f4db6c976cf8 ("arm: mvebu: Add runtime
> detection of UART (xmodem) boot-mode") and commit 94084eea3bd3
> ("tools: kwbimage: Fix dest
> addr") conflict and end up taking off too much when using kwboot. I'll
> try removing the destaddr adjustment from kwboot and see where I get
> to.

Thanks for working on this, Chris. I'll pull your kwboot patch shortly
so that its included in the upcoming release.

Thanks,
Stefan