Re: [PATCH u-boot-marvell 00/13] Yet another kwboot improvements

Pali Rohár
2021-10-26T09:06:36Z

On Tuesday 26 October 2021 10:45:09 Stefan Roese wrote:
> On 26.10.21 10:33, Pali Rohár wrote:
> > On Monday 25 October 2021 17:15:14 Stefan Roese wrote:
> > > Hi Pali,
> > >
> > > On 25.10.21 16:42, Pali Rohár wrote:
> > > > On Monday 25 October 2021 16:39:44 Stefan Roese wrote:
> > > > > Hi Marek,
> > > > >
> > > > > On 25.10.21 15:12, Marek Behún wrote:
> > > > > > From: Marek Behún <marek.behun@nic.cz>
> > > > > >
> > > > > > Hello Stefan,
> > > > > >
> > > > > > these are another improvements for kwboot, please apply only after series
> > > > > > arm: mvebu: nandpagesize support for kwbimage v1
> > > > >
> > > > > I'm checking right now and have applied the 3 NAND patches on current
> > > > > master. But this patchset fails at this one:
> > > > >
> > > > > tools: kwboot: Do not modify kwbimage header before increasing its size
> > > > >
> > > > > [stefan@ryzen u-boot-marvell (kwboot-test1)]$ git am -3 ~/tmp/kwboot2/*
> > > > > Applying: tools: kwboot: Initialize rfds to zero
> > > > > Applying: tools: kwboot: Fix initialization of tty device
> > > > > Applying: tools: kwboot: Reserve enough space for patching kwbimage in
> > > > > memory
> > > > > Applying: tools: kwboot: Validate 4-byte image data checksum
> > > > > Applying: tools: kwboot: Inject baudrate change back code after data part
> > > > > Applying: tools: kwboot: Recalculate 4-byte data checksum after injecting
> > > > > baudrate code
> > > > > Applying: tools: kwboot: Correctly set configuration of UART for BootROM
> > > > > messages
> > > > > Applying: tools: kwboot: Show verbose message when waiting for baudrate
> > > > > change magic
> > > > > Applying: tools: kwboot: Simplify code for aligning image header
> > > > > Applying: tools: kwboot: Do not modify kwbimage header before increasing its
> > > > > size
> > > > > error: sha1 information is lacking or useless (tools/kwboot.c).
> > > > > error: could not build fake ancestor
> > > > > Patch failed at 0010 tools: kwboot: Do not modify kwbimage header before
> > > > > increasing its size
> > > > > hint: Use 'git am --show-current-patch=diff' to see the failed patch
> > > > > When you have resolved this problem, run "git am --continue".
> > > > > If you prefer to skip this patch, run "git am --skip" instead.
> > > > > To restore the original branch and stop patching, run "git am --abort".
> > > > >
> > > > > Any idea what's missing here?
> > > >
> > > > Hello! I'm using also this patch:
> > > > https://patchwork.ozlabs.org/project/uboot/patch/20211021144609.9319-2-pali@kernel.org/
> > >
> > > Ah, yes. That does the trick. Now all patches apply clean. Thanks.
> > >
> > > Testing with all these patches on my AXP target does show, it still
> > > does not work with baudrate > 115k:
> > >
> > > [stefan@ryzen u-boot-marvell (kwboot-test1)]$ ./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
> > > 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-00908-gc129aa2f173a (Oct 25 2021 - 17:10:55 +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)...
> > >
> > > Changing baudrate to 230400 Bd
> > > Baudrate was not changed
> > >
> > >
> > > xmodem: Protocol error
> > > [stefan@ryzen u-boot-marvell (kwboot-test1)]$
> > >
> > >
> > > Not changing the baudrate still works. Any idea what I should test? Or
> > > do you have further changes in the queue that I should wait upon?
> >
> > Could you try to boot with baudrate 115201? This value would trigger to
> > use baudrate change code and because it is too close to standard 115200
> > there would be no real change.

Now I found logical error in kwboot code which handles retransmission of
the last header packet. State of "baudrate change" is cleared on every
retransmission. Please apply following diff, so state variables are
initialized only once.

diff --git a/tools/kwboot.c b/tools/kwboot.c
index d38ee0065177..835ccc8c113a 100644
--- a/tools/kwboot.c
+++ b/tools/kwboot.c
@@ -859,11 +859,6 @@ kwboot_xm_recv_reply(int fd, char *c, int nak_on_non_xm,
uint64_t recv_until = _now() + timeout;
int rc;

- if (non_xm_print)
- *non_xm_print = 0;
- if (baud_changed)
- *baud_changed = 0;
-
while (1) {
rc = kwboot_tty_recv(fd, c, 1, timeout);
if (rc) {
@@ -929,6 +924,8 @@ kwboot_xm_sendblock(int fd, struct kwboot_block *block, int allow_non_xm,
char c;

*done_print = 0;
+ non_xm_print = 0;
+ baud_changed = 0;

retries = 0;
do {

> Ok. This seems to work at least partly (SPL):
>
> [stefan@ryzen u-boot-marvell (kwboot-test1)]$ ./tools/kwboot -B 115201 -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 115201 Bd
> Injecting code for changing baudrate back
> 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-00908-gc129aa2f173a (Oct 26 2021 - 10:39:55 +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)...
>
> Changing baudrate to 115201 Bd
>
> Sending boot image data (549892 bytes)...
> 0 %
> [......................................................................]
> 1 %
> [......................................................................]
> ...
> 97 %
> [......................................................................]
> 99 % [........................... ]
> Done
> Finishing transfer
> Waiting 1s for baudrate change magic
>
> Changing baudrate back to 115200 Bd
>
> [Type Ctrl-\ + c to quit]
>
> BootROM 1.20
> Booting from SPI flash
>
>
> So the CPU has run through a reset here.

What kind of CPU core has your AXP board? Maybe there is some arm
instruction in kwboot_baud_code[] array which is not supported by that
core (and so reset occurs)?

And could you send me commands which you are using for compiling U-Boot
for that your AXP board? I would like to examine resulted binary.

> > And what is the output of following command immediately after kwboot?
> >
> > stty -F /dev/serial/by-id/usb-FTDI_FT232R_USB_UART_A1019EGY-if00-port0
>
> Changing baudrate to 230400 Bd
> Baudrate was not changed
>
>
> xmodem: Protocol error
> [stefan@ryzen u-boot-marvell (kwboot-test1)]$ stty -F
> /dev/serial/by-id/usb-FTDI_FT232R_USB_UART_A1019EGY-if00-port0
> speed 230400 baud; line = 0;

... but baudrate _was_ changed.

So it looks like a bug in kwboot code. Try to apply above diff, it is
really possible that you hit that logical error. (Interesting that
neither Marek nor me saw it)

> intr = <undef>; quit = <undef>; erase = <undef>; kill = <undef>; eof =
> <undef>; start = <undef>; stop = <undef>; susp = <undef>; rprnt = <undef>;
> werase = <undef>; lnext = <undef>; discard = <undef>; min = 1; time = 0;
> -brkint -icrnl -imaxbel
> -opost -onlcr
> -isig -icanon -iexten -echo -echoe -echok -echoctl -echoke
>
> > Also, could try to catch whole strace log and send it to me?
>
> I'll try to do this later today.

'strace kwboot ... 1>log 2>&1' should do it...

> Thanks,
> Stefan