From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 705BEC433EF for ; Wed, 27 Oct 2021 15:08:49 +0000 (UTC) Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id D69D960EFF for ; Wed, 27 Oct 2021 15:08:48 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org D69D960EFF Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=lists.denx.de Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 0DFD180D1C; Wed, 27 Oct 2021 17:08:47 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=kernel.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (2048-bit key; unprotected) header.d=kernel.org header.i=@kernel.org header.b="cGrMLnvk"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 8CE4380D1C; Wed, 27 Oct 2021 17:08:45 +0200 (CEST) Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id E32678029E for ; Wed, 27 Oct 2021 17:08:40 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=kernel.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=kabel@kernel.org Received: by mail.kernel.org (Postfix) with ESMTPSA id 7E9CD60F5A; Wed, 27 Oct 2021 15:08:38 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1635347319; bh=rhpOLe/7bluvFT2iF+Kf5CoYNdcPuS86KwtQwCIsuno=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=cGrMLnvk/ddqUorrNAu7j5sTk3UJdPklJcKaDNU04lL0CKLuj/zwaCXfTjUS7wOZQ zk4m/eHQGi1iptAwWkS0jk1I2EzZYFo5MZCMjasDi7aaluywNqGidGat9IFA7bvaBn wiqkizOEMGBVaSuq3x4o2e3by8MgX0ccyPpButKzuu0xUoIM6IRU37R9L96+4Vk6jb G0IeulmJmpzq9JZlgfuj+tsC3q8usoyoHkoZJ/DvobfY3K4KPEsujFZAvkaRLQu3CT 3ogqAuXCLqcqkYeK9nAvQodMHRMo/nyzO1eNDdijwxJvIoiWJb29HMiydUJKw+RzsY DqOOtIRf9W6sQ== Date: Wed, 27 Oct 2021 17:08:35 +0200 From: Marek =?UTF-8?B?QmVow7pu?= To: Pali =?UTF-8?B?Um9ow6Fy?= Cc: Stefan Roese , u-boot@lists.denx.de, Marek =?UTF-8?B?QmVo?= =?UTF-8?B?w7pu?= Subject: Re: [PATCH u-boot-marvell 00/13] Yet another kwboot improvements Message-ID: <20211027170835.7c584981@thinkpad> In-Reply-To: <20211027141021.kgtby7g3dukmf225@pali> References: <20211026083305.bm3lvoevgp62y4fe@pali> <1d9e63ba-bb86-8a86-d7f0-02fdf6e1da52@denx.de> <20211026090620.eilcjqkc33wavm5b@pali> <872e26ac-77ff-f19b-b3c9-debff1af58b0@denx.de> <20211026124048.lprqsoqow63cwqom@pali> <20211026184824.3g2bup2cwpnxk6a3@pali> <2eae7502-2fb9-cefd-57e8-f14786a626bc@denx.de> <20211027135247.kusaph47hrvieoiq@pali> <20211027141021.kgtby7g3dukmf225@pali> X-Mailer: Claws Mail 3.18.0 (GTK+ 2.24.33; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.34 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.2 at phobos.denx.de X-Virus-Status: Clean On Wed, 27 Oct 2021 16:10:21 +0200 Pali Roh=C3=A1r wrote: > On Wednesday 27 October 2021 15:52:47 Pali Roh=C3=A1r wrote: > > On Wednesday 27 October 2021 07:09:42 Stefan Roese wrote: =20 > > > On 26.10.21 20:48, Pali Roh=C3=A1r wrote: =20 > > > > On Tuesday 26 October 2021 16:21:02 Stefan Roese wrote: =20 > > > > > On 26.10.21 14:40, Pali Roh=C3=A1r wrote: =20 > > > > > > My another guess there could be a problem is usage of stack. Ma= ybe it is > > > > > > possible that register with stack pointer is not initialized af= ter the > > > > > > full transfer when going to execute main image. Same arm baudra= te 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 w= ill look > > > > > > at it. =20 > > > > >=20 > > > > > Thanks. =20 > > > >=20 > > > > Here is dirty hack patch which completely disable calling code for > > > > changing baudrate back to 115200 on ARM side: > > > >=20 > > > > 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, si= ze_t size, int baudrate) > > > > return rc; > > > > if (baudrate) { > > > > - char buf[sizeof(kwb_baud_magic)]; > > > > - > > > > - kwboot_printv("Waiting 1s for baudrate change magic\n"); > > > > - rc =3D kwboot_tty_recv(tty, buf, sizeof(buf), 1000); > > > > - if (rc) > > > > - return rc; > > > > - > > > > - if (memcmp(buf, kwb_baud_magic, sizeof(buf))) { > > > > - errno =3D EPROTO; > > > > - return -1; > > > > - } > > > > +// char buf[sizeof(kwb_baud_magic)]; > > > > +// > > > > +// kwboot_printv("Waiting 1s for baudrate change magic\n"); > > > > +// rc =3D kwboot_tty_recv(tty, buf, sizeof(buf), 1000); > > > > +// if (rc) > > > > +// return rc; > > > > +// > > > > +// if (memcmp(buf, kwb_baud_magic, sizeof(buf))) { > > > > +// errno =3D EPROTO; > > > > +// return -1; > > > > +// } > > > > kwboot_printv("\nChanging baudrate back to 115200 Bd\n\n"); > > > > rc =3D 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 prope= r. > > > > */ > > > > - 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); =20 > > >=20 > > > I do have this here in my version as well: > > >=20 > > > /* Update the 32-bit data checksum */ > > > *kwboot_img_csum32_ptr(img) =3D kwboot_img_csum32(img); > > >=20 > > > /* recompute header size */ > > > hdrsz =3D kwbheader_size(hdr); > > >=20 > > > So I'm using the newer version, just to be sure. =20 > >=20 > > Ok, I probably generated diff against older version, but you have > > figured out how to apply it. > > =20 > > > > /* recompute header size */ > > > > hdrsz =3D kwbheader_size(hdr); > > > >=20 > > > > 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 cou= ld be > > > > lost but at least console should start. > > > >=20 > > > > Could you try this patch if it starts working now? =20 > > >=20 > > > Okay, applied this patch and and booting with different baudrates wor= ks > > > on this board again (tested with 230400): > > >=20 > > > 96 % > > > [....................................................................= ..] > > > 98 % > > > [....................................................................= ..] > > > 99 % [................ ] > > > Done > > > Finishing transfer > > >=20 > > > Changing baudrate back to 115200 Bd > > >=20 > > > [Type Ctrl-\ + c to quit] > > >=20 > > >=20 > > > U-Boot 2021.10-00908-gc129aa2f173a-dirty (Oct 27 2021 - 07:05:39 +020= 0) > > >=20 > > > 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 s= ize 256 > > > Bytes, erase size 256 KiB, total 16 MiB > > > OK > > > Model: Marvell Armada XP theadorable > > > ... > > >=20 > > >=20 > > > Thanks, > > > Stefan =20 > >=20 > > Perfect! So it really looks like that issue is in the code which resets > > baudrate back to the value 115200. > >=20 > > I have there another diff which removes usage of the stack in code which > > resets baudrate back to default value: > >=20 > > 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) =3D cpu_to_le32(old_baud); > > *(uint32_t *)(code + codesz - 4) =3D cpu_to_le32(new_baud); > > + > > + if (!pre) { =20 >=20 > Ou, there is a mistake, it should be "if (pre) {" >=20 > > + *(uint32_t *)code =3D cpu_to_le32(0xe1a00000); /* arm nop */ > > + *(uint32_t *)(code + codesz - 4*7) =3D cpu_to_le32(0xe12fff1e); /* b= x 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