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 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 smtp.lore.kernel.org (Postfix) with ESMTPS id BEAB6C433F5 for ; Mon, 7 Feb 2022 09:02:38 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 45A4183C9F; Mon, 7 Feb 2022 10:02:36 +0100 (CET) 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="IYCuumcW"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 551B683C6F; Mon, 7 Feb 2022 10:02:34 +0100 (CET) Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) (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 194B083C94 for ; Mon, 7 Feb 2022 10:02:30 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=kernel.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=pali@kernel.org Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id CAC1361016; Mon, 7 Feb 2022 09:02:28 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id DF35AC004E1; Mon, 7 Feb 2022 09:02:27 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1644224548; bh=deiri9Yxyy7E6DR7YCwNgYBNbX5A+9ZO/bufuiaiY/s=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=IYCuumcWLpS7INAc9RvLE6uYn+gHpHGJmJNecg8KJHEhe2TWDCaULF8s6Sbcg1MhD fqJFo64/9o8duK2wgAJIGaF1RvwcYlSI6QJWH+RHKtER2LQISZKHpMTmx0QRDPVimy 9ppQ5Z1nM4hMtu3/XnUUip4FnLQDot0hp24X69v+n3erhgtGTCPxKTL2UOY2X/NXsI pbiVieX77Qf24BkegqKED0684Ec/LjjJbphqG5Su7UNepdznA+U0prd89U1rRYF/DF AdCcTB/hfRt8oJToo2Hrmze2IWho+txsOHDXJTx9QodPwC6kwaruIzfzfNxHEz6iw8 mrKbMaidozE6g== Received: by pali.im (Postfix) id D1D52C34; Mon, 7 Feb 2022 10:02:25 +0100 (CET) Date: Mon, 7 Feb 2022 10:02:25 +0100 From: Pali =?utf-8?B?Um9ow6Fy?= To: Marcel Ziswiler Cc: Stefan Roese , Marek =?utf-8?B?QmVow7pu?= , u-boot@lists.denx.de, Konstantin Porotchkin , Robert Marko , Sergey Sergeev Subject: Re: [PATCH] tools/mrvl_uart.sh: Remove script Message-ID: <20220207090225.x7gplhvxtdhemcfp@pali> References: <20220203165046.19218-1-pali@kernel.org> <8a13f2672d2484e2b2ea9e2ddf8398fc5e199612.camel@ziswiler.com> <21b00773386780be77b531a0428ec311cddfa83f.camel@ziswiler.com> <20220205002519.2esdmyxjqiqvgvvq@pali> <1c3f6c6988f880231af15c7c30e73f43814f6cf9.camel@ziswiler.com> <20220205005406.lydjjhiyf7z6whgm@pali> <08bc52145cf213ab4b9d64baaa49fd607f7f7e0c.camel@ziswiler.com> <20220205145438.cdyxm3a4f6bosazq@pali> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: User-Agent: NeoMutt/20180716 X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.39 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.5 at phobos.denx.de X-Virus-Status: Clean On Monday 07 February 2022 08:20:54 Marcel Ziswiler wrote: > On Sat, 2022-02-05 at 15:54 +0100, Pali Rohár wrote: > > On Saturday 05 February 2022 03:07:00 Marcel Ziswiler wrote: > > > On Sat, 2022-02-05 at 01:54 +0100, Pali Rohár wrote: > > > > On Saturday 05 February 2022 01:40:23 Marcel Ziswiler wrote: > > > > > On Sat, 2022-02-05 at 01:25 +0100, Pali Rohár wrote: > > > > > > $ kwboot -b /dev/ttyUSB0 > > > > > > > > > > Hm, at least kwboot from today's master does not allow -b without also > > > > > giving it an image. > > > > > > > > This commit is part of master branch and added support for it: > > > > https://source.denx.de/u-boot/u-boot/-/commit/c513fe47dca24de87a904ce7d71cfd8a390e1154 > > > > > > > > If it does not work then there is some bug which should be fixed. I have > > > > tested it and it works on my setup. > > > > > > > > Can you check if you have that commit to ensure that we are not going to > > > > test / debug older version? > > > > > > Yes, sure. I debugged it a little and I believe I found the issue. I guess the intention was for it to be > > > run > > > giving it a dash '-' as image argument for skipping. (Even though that usually would mean using stdin). > > > Anyway, > > > it fails as it then uses such dash as ttypath because optind did not get incremented. The following fixes > > > it > > > for me: > > > > > > diff --git a/tools/kwboot.c b/tools/kwboot.c > > > index 2684f0e75a..d490c026e9 100644 > > > --- a/tools/kwboot.c > > > +++ b/tools/kwboot.c > > > @@ -1768,7 +1768,8 @@ main(int argc, char **argv) > > >                         if (prev_optind == optind) > > >                                 goto usage; > > >                         if (argv[optind] && argv[optind][0] != '-') > > > -                               imgpath = argv[optind++]; > > > +                               imgpath = argv[optind]; > > > +                       optind++; > > >                         break; > > >   > > >                 case 'D': > > > > > > Let me know if that is indeed the intention and I can send a proper fix for it. Maybe I can also update the > > > documentation/usage in that respect. Thanks again. > > > > Now I see where is the issue. Your fix is incorrect because it breaks > > other usage (e.g. specifying other options). > > > > The issue here is that argv[optind] is used also when it is the last > > option on the command line -- which is not getopt() at all, as it is > > parsed after the getopt() processing. > > > > So I think that correct fix should be something like this: > > > >                         bootmsg = kwboot_msg_boot; > >                         if (prev_optind == optind) > >                                 goto usage; > > -                       if (argv[optind] && argv[optind][0] != '-') > > +                       if (optind < argc-1 && argv[optind] && argv[optind][0] != '-') > >                                 imgpath = argv[optind++]; > >                         break; > >   > > > > This should fix usage of all "kwboot -b /dev/tty", "kwboot -b -t /dev/tty" > > and "kwboot -b image.kwb /dev/tty" usage. > > > > Could you test it? > > Yes, this indeed works fine now. > > With that, and if we properly document such "standalone" usage, I would be fine with dropping > tools/mrvl_uart.sh. So you can get my reviewed-by for this one. > > Reviewed-by: Marcel Ziswiler > > And you get my tested-by for such patch fixing this as outlined above. > > Tested-by: Marcel Ziswiler > > > Seems that I tested only -b -t combination (as I wanted to see that > > bootrom correctly start sending NAK xmodem bytes). > > Yep, now I got you. Thanks! Ok, thank you for testing, I will send a proper patch to ML.