[PATCH] tools/mrvl_uart.sh: Remove script
marcel at ziswiler.com
Mon Feb 7 08:20:54 CET 2022
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] != '-')
> > - 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] != '-')
> + if (optind < argc-1 && argv[optind] && argv[optind] != '-')
> imgpath = argv[optind++];
> 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 <marcel at ziswiler.com>
And you get my tested-by for such patch fixing this as outlined above.
Tested-by: Marcel Ziswiler <marcel at ziswiler.com>
> 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!
More information about the U-Boot