[PATCH u-boot-marvell v3 00/39] kwboot higher baudrate

Fri Oct 1 11:58:14 CEST 2021

Hi Pali,

On 01.10.21 11:28, Pali Rohár wrote:
> Hello!
> 
> On Friday 01 October 2021 09:46:58 Stefan Roese wrote:
>> Hi Pali,
>>
>> On 30.09.21 20:14, Pali Rohár wrote:
>>> Hello!
>>>
>>> Could you test or review this patch series?
>>>
>>> It is a big improvement for kwboot as it allows to transfer u-boot over
>>> uart into mvebu platforms much faster.
>>
>> I'm testing this series right now on my theadorable target, which
>> is Armada XP based. Here I get this error:
>>
>> $ ./tools/kwboot -B 230400 -b u-boot-spl.kwb -t
>> /dev/serial/by-id/usb-FTDI_FT232R_USB_UART_A1019EGY-if00-port0
>> Patching image boot signature to UART
>> Injecting binary header code for changing baudrate to 230400 Bd
>> Injecting code for changing baudrate back
>> Aligning image header to Xmodem block size
>> Sending boot message. Please reboot the target...|
>> Waiting 2s and flushing tty
>> Sending boot image header (90496 bytes)...
>>    0 %
>> [......................................................................]
>>   10 %
>> [......................................................................]
>>   19 %
>> [......................................................................]
>>   29 %
>> [......................................................................]
>>   39 %
>> [......................................................................]
>>   49 %
>> [......................................................................]
>>   59 %
>> [......................................................................]
>>   69 %
>> [......................................................................]
>>   79 %
>> [......................................................................]
>>   89 %
>> [......................................................................]
>>   99 % [.......       ]
>> Done
>>
>> U-Boot SPL 2021.10-rc5-00228-g5523b4689ff9 (Oct 01 2021 - 08:39:06 +0200)
>> High speed PHY - Version: 2.1.5 (COM-PHY-V20)
>> High speed PHY - Ended Successfully
>> DDR3 Training Sequence - Ver 5.7.4
>> DDR3 Training Sequence - Ended Successfully
>> Trying to boot from BOOTROM
>> Returning to BootROM (return address 0xffff0aa0)...
> 
> At this stage SPL was successfully executed and returned control back to
> BootROM. BootROM now should execute second (injected) header for
> changing baudrate.
> 
>> Changing baudrate to 230400 Bd
> 
> At this stage kwboot received baudrate change magic string which
> indicates that BootROM started executing second injected header, as this
> string is printed at the beginning of the injected code (kwboot_baud_code[])
> 
> But we do not know if whole code was successfully executed or not.
> 
> In case of error (e.g. executing unsupported instructions or touching
> unavailable memory) CPU is reset.
> 
> Can you check if CPU was reset and BootROM started booting from primary
> source (probably SPI) immediately when you saw this message?

I checked by quickly starting a terminal app and it does not seem that
the CPU did go through a reboot. Nothing on the UART at this stage.

BUT:
The tests I did in this mail were done with your 39 patches applied
on top of "master-kwboot" [1]. Here booting with 115k works as explained 
below.

Now I applied the same 39 patches on top of "next" [2]. And here not
even booting with 115k works on my AXP target. It always hangs at this
stage:

[stefan at ryzen u-boot-marvell (next)]$ ./tools/kwboot -b u-boot-spl.kwb 
-t /dev/serial/by-id/usb-FTDI_FT232R_USB_UART_A1019EGY-if00-port0
Patching image boot signature to UART
Sending boot message. Please reboot the target...|
Waiting 2s and flushing tty
Sending boot image header (90112 bytes)...
   0 % 
[......................................................................]
  10 % 
[......................................................................]
  20 % 
[......................................................................]
  29 % 
[......................................................................]
  39 % 
[......................................................................]
  49 % 
[......................................................................]
  59 % 
[......................................................................]
  69 % 
[......................................................................]
  79 % 
[......................................................................]
  89 % 
[......................................................................]
  99 % [.... 
       ]
Done

U-Boot SPL 2021.10-rc5-00431-g6c96332441cf (Oct 01 2021 - 11:56:49 +0200)
High speed PHY - Version: 2.1.5 (COM-PHY-V20)
High speed PHY - Ended Successfully
DDR3 Training Sequence - Ver 5.7.4
DDR3 Training Sequence - Ended Successfully
Trying to boot from BOOTROM
Returning to BootROM (return address 0xffff0aa0)...

xmodem: Connection timed out

This seems to work at least for 115k in the "master-kwboot" branch.

Any ideas?

Thanks,
Stefan

[1] 
https://source.denx.de/u-boot/custodians/u-boot-marvell/-/commits/master-kwboot
[2] https://source.denx.de/u-boot/custodians/u-boot-marvell/-/commits/next

> Or is board even after executing kwboot stucked in some UART execution?
> 
>> Baudrate was not changed
> 
> At this stage kwboot already instructed your FTDI adapter to switch
> baudrate and is ready for receiving data from AXP at higher speed.
> 
> This message indicates that kwboot has not received xmodem ACK for the
> last packet.
> 
> Could you check what is the delay between "Changing baudrate to ..." and
> "Baudrate was not changed" messages? It is in immediately or few seconds
> or minute?
> 
> What is CPU speed of your AXP? Could you try to increase or decrease
> sleep timeout which is in ARM kwboot_baud_code[] bellow comment
> "Sleep 1ms ~~ 600000 cycles at 1200 MHz"
> 
> To debug you can try to also add debug prints in while loop which is in
> function kwboot_xm_sendblock() to check when and how many times it is
> executed.
> 
> And another case for test. Check if AXP really switched to higher
> baudrate. You can do it by commenting code which changes baudrate on
> local PC side in function kwboot_baud_magic_handle(). If AXP does not
> change baudrate then it continue communication at 115200 and so if you
> comment code "Changing baudrate to %d Bd" in kwboot_baud_magic_handle()
> it would also continue communication at 115200.
> 
>>
>> xmodem: Protocol error
>>
>>
>> UART booting currently only seems to work with the default 115kBaud.
> 
> Does it work with lower speed? If not then it indicates that issue is in
> this baudrate change logic.
> 
>> Do you have any idea what might go wrong here?
>>
>> Thanks,
>> Stefan
>>
>>> On Friday 24 September 2021 23:06:37 Marek Behún wrote:
>>>> From: Marek Behún <marek.behun at nic.cz>
>>>>
>>>> Hello Stefan and others,
>>>>
>>>> here's v3 of series adding support for booting Marvell platforms via
>>>> UART (those bootable with kwboot) at higher baudrates.
>>>>
>>>> Tested on Turris Omnia up to 5.15 MBd, which is 44x faster than
>>>> 115200 Bd.
>>>>
>>>> The user can direct kwboot to use higher baudrate via the -B option.
>>>> (BTW this option was there before but did not work with the -b option.)
>>>>
>>>> Only the payload part of the KWB image is uploaded at this higher
>>>> baudrate. The header part is still uploaded at 115200 Bd, since the code
>>>> that changes baudrate is injected into header as a binary extension.
>>>> (The payload part makes up majority of the binary, though. On Turris
>>>>    Omnia the payload currently makes ~87%.)
>>>>
>>>> The series also contains various other fixes, refactors and improvements
>>>> of the code, upon which the main change is done.
>>>>
>>>> Marek & Pali
>>>>
>>>> Changes since v2:
>>>> - fixed integer overflow in patch 15
>>>> - fixed commit title in patch 32
>>>>
>>>> Changes since v1:
>>>> - fixed uploading of older kwb images, now all kwb images should be able
>>>>     to upload at faster baudrate
>>>> - fixed injecting code that changes baudrate back
>>>> - various other fixes and refactors, the best way to compare with v1 is
>>>>     to apply v1 and v2 separately and compare the resulting kwboot.c
>>>>
>>>> Marek Behún (19):
>>>>     tools: kwbimage: Fix printf format warning
>>>>     tools: kwboot: Fix buffer overflow in kwboot_terminal()
>>>>     tools: kwboot: Make the quit sequence buffer const
>>>>     tools: kwboot: Refactor and fix writing buffer
>>>>     tools: kwboot: Fix comparison of integers with different size
>>>>     tools: kwboot: Use a function to check whether received byte is a
>>>>       Xmodem reply
>>>>     tools: kwboot: Print new line after SPL output
>>>>     tools: kwboot: Allow greater timeout when executing header code
>>>>     tools: kwboot: Prevent waiting indefinitely if no xmodem reply is
>>>>       received
>>>>     tools: kwbimage: Simplify iteration over version 1 optional headers
>>>>     tools: kwbimage: Refactor image_version()
>>>>     tools: kwbimage: Refactor kwbimage header size determination
>>>>     tools: kwboot: Explicitly check against size of struct main_hdr_v1
>>>>     tools: kwboot: Check whether baudrate was set to requested value
>>>>     tools: kwboot: Cosmetic fix
>>>>     tools: kwboot: Avoid code repetition in kwboot_img_patch()
>>>>     tools: kwboot: Update file header
>>>>     doc/kwboot.1: Update man page
>>>>     MAINTAINERS: Add entry for kwbimage / kwboot tools
>>>>
>>>> Pali Rohár (20):
>>>>     tools: kwboot: Print version information header
>>>>     tools: kwboot: Fix kwboot_xm_sendblock() function when
>>>>       kwboot_tty_recv() fails
>>>>     tools: kwboot: Fix return type of kwboot_xm_makeblock() function
>>>>     tools: kwboot: Fix printing progress
>>>>     tools: kwboot: Print newline on error when progress was not completed
>>>>     tools: kwboot: Split sending image into header and data stages
>>>>     tools: kwboot: Allow non-xmodem text output from BootROM only in a
>>>>       specific case
>>>>     tools: kwboot: Properly finish xmodem transfer
>>>>     tools: kwboot: Always call kwboot_img_patch_hdr()
>>>>     tools: kwboot: Don't patch image header if signed
>>>>     tools: kwboot: Patch source address in image header
>>>>     tools: kwboot: Patch destination address to DDR area for SPI image
>>>>     tools: kwbimage: Update comments describing kwbimage v1 structures
>>>>     tools: kwboot: Round up header size to 128 B when patching
>>>>     tools: kwboot: Support higher baudrates when booting via UART
>>>>     tools: kwboot: Allow any baudrate on Linux
>>>>     tools: kwboot: Fix initializing tty device
>>>>     tools: kwboot: Disable tty interbyte timeout
>>>>     tools: kwboot: Disable non-blocking mode
>>>>     tools: kwboot: Add Pali and Marek as authors
>>>>
>>>>    MAINTAINERS           |   10 +
>>>>    doc/kwboot.1          |   60 ++-
>>>>    tools/kwbimage.c      |  130 ++---
>>>>    tools/kwbimage.h      |   99 +++-
>>>>    tools/kwboot.c        | 1197 +++++++++++++++++++++++++++++++++++------
>>>>    tools/termios_linux.h |  189 +++++++
>>>>    6 files changed, 1385 insertions(+), 300 deletions(-)
>>>>    create mode 100644 tools/termios_linux.h
>>>>
>>>> -- 
>>>> 2.32.0
>>>>
>>
>>
>> Viele Grüße,
>> Stefan
>>
>> -- 
>> DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
>> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
>> Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr at denx.de

Viele Grüße,
Stefan

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr at denx.de