On Wed, 27 Oct 2021 16:10:21 +0200 Pali Rohár pali@kernel.org wrote:
On Wednesday 27 October 2021 15:52:47 Pali Rohár wrote:
On Wednesday 27 October 2021 07:09:42 Stefan Roese wrote:
On 26.10.21 20:48, Pali Rohár wrote:
On Tuesday 26 October 2021 16:21:02 Stefan Roese wrote:
On 26.10.21 14:40, Pali Rohár wrote:
My another guess there could be a problem is usage of stack. Maybe it is possible that register with stack pointer is not initialized after the full transfer when going to execute main image. Same arm baudrate change code is used after the SPL and before main U-Boot, and the first instruction is "push". Maybe modifying the arm code to not use stack when switching the baudrate back to 115200 could also help. I will look at it.
Thanks.
Here is dirty hack patch which completely disable calling code for changing baudrate back to 115200 on ARM side:
diff --git a/tools/kwboot.c b/tools/kwboot.c index 5f4ff673972e..00d58a239c71 100644 --- a/tools/kwboot.c +++ b/tools/kwboot.c @@ -1070,17 +1070,17 @@ kwboot_xmodem(int tty, const void *_img, size_t size, int baudrate) return rc; if (baudrate) {
char buf[sizeof(kwb_baud_magic)];kwboot_printv("Waiting 1s for baudrate change magic\n");rc = kwboot_tty_recv(tty, buf, sizeof(buf), 1000);if (rc)return rc;if (memcmp(buf, kwb_baud_magic, sizeof(buf))) {errno = EPROTO;return -1;}+// char buf[sizeof(kwb_baud_magic)]; +// +// kwboot_printv("Waiting 1s for baudrate change magic\n"); +// rc = kwboot_tty_recv(tty, buf, sizeof(buf), 1000); +// if (rc) +// return rc; +// +// if (memcmp(buf, kwb_baud_magic, sizeof(buf))) { +// errno = EPROTO; +// return -1; +// } kwboot_printv("\nChanging baudrate back to 115200 Bd\n\n"); rc = kwboot_tty_change_baudrate(tty, 115200); @@ -1551,8 +1551,8 @@ kwboot_img_patch(void *img, size_t *size, int baudrate) * This code is appended after the data part of the image and * execaddr is changed to execute this code before U-Boot proper. */
kwboot_printv("Injecting code for changing baudrate back\n");_copy_baudrate_change_code(hdr, size, 1, baudrate, 115200);+// kwboot_printv("Injecting code for changing baudrate back\n"); +// _copy_baudrate_change_code(hdr, size, 1, baudrate, 115200);
I do have this here in my version as well:
/* Update the 32-bit data checksum */ *kwboot_img_csum32_ptr(img) = kwboot_img_csum32(img); /* recompute header size */ hdrsz = kwbheader_size(hdr);So I'm using the newer version, just to be sure.
Ok, I probably generated diff against older version, but you have figured out how to apply it.
/* recompute header size */ hdrsz = kwbheader_size(hdr);As main U-Boot binary on ARM resets UART, it means that baudrate is properly set to 115200. Probably beginning of the U-Boot output could be lost but at least console should start.
Could you try this patch if it starts working now?
Okay, applied this patch and and booting with different baudrates works on this board again (tested with 230400):
96 % [......................................................................] 98 % [......................................................................] 99 % [................ ] Done Finishing transfer
Changing baudrate back to 115200 Bd
[Type Ctrl-\ + c to quit]
U-Boot 2021.10-00908-gc129aa2f173a-dirty (Oct 27 2021 - 07:05:39 +0200)
SoC: MV78260-B0 at 1333 MHz I2C: ready DRAM: 2 GiB (667 MHz, 64-bit, ECC not enabled) Loading Environment from SPIFlash... SF: Detected m25p128 with page size 256 Bytes, erase size 256 KiB, total 16 MiB OK Model: Marvell Armada XP theadorable ...
Thanks, Stefan
Perfect! So it really looks like that issue is in the code which resets baudrate back to the value 115200.
I have there another diff which removes usage of the stack in code which resets baudrate back to default value:
diff --git a/tools/kwboot.c b/tools/kwboot.c index b56c9a0c8104..8f0e50501398 100644 --- a/tools/kwboot.c +++ b/tools/kwboot.c @@ -1444,6 +1444,11 @@ _inject_baudrate_change_code(void *img, size_t *size, int pre, memcpy(code, kwboot_baud_code, codesz - 8); *(uint32_t *)(code + codesz - 8) = cpu_to_le32(old_baud); *(uint32_t *)(code + codesz - 4) = cpu_to_le32(new_baud);
- if (!pre) {
Ou, there is a mistake, it should be "if (pre) {"
*(uint32_t *)code = cpu_to_le32(0xe1a00000); /* arm nop */*(uint32_t *)(code + codesz - 4*7) = cpu_to_le32(0xe12fff1e); /* bx lr */- }
This fixes it for me as well, for that one Omnia which didn't work which I was debugging a few days ago.
I guess this can't be done in the binhdr? So we need 2 different versions? I don't quite like this ad-hoc change, but I also don't like two copies with just a small change between them...
Marek