[PATCH] tools/mrvl_uart.sh: Remove script

[Marcel Ziswiler]

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 <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!