All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 1/3] tools: kwboot: Allow to specify custom baudrate only in supported operations
@ 2022-03-07 18:03 Pali Rohár
  2022-03-07 18:03 ` [PATCH v2 2/3] tools: kwboot: Check if baudrate value is supported before sending image Pali Rohár
                   ` (3 more replies)
  0 siblings, 4 replies; 12+ messages in thread
From: Pali Rohár @ 2022-03-07 18:03 UTC (permalink / raw)
  To: Stefan Roese, Marek Behún, Marcel Ziswiler, Tony Dinh; +Cc: u-boot

Custom baudrate different than 115200 may be specified only when kwboot is
not going to send boot/debug message pattern or when it is going to send
boot message pattern with image file (in which case baudrate change happens
after sending kwbimage header). BootROM detects boot/debug message pattern
only at baudrate 115200.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 tools/kwboot.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/tools/kwboot.c b/tools/kwboot.c
index 69d1be0f4823..986f27c2012a 100644
--- a/tools/kwboot.c
+++ b/tools/kwboot.c
@@ -2133,6 +2133,12 @@ main(int argc, char **argv)
 	if (optind != argc)
 		goto usage;
 
+	/* boot and debug message use baudrate 115200 */
+	if (((bootmsg && !imgpath) || debugmsg) && baudrate != 115200) {
+		fprintf(stderr, "Baudrate other than 115200 cannot be used for this operation.\n");
+		goto usage;
+	}
+
 	tty = kwboot_open_tty(ttypath, imgpath ? 115200 : baudrate);
 	if (tty < 0) {
 		perror(ttypath);
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 2/3] tools: kwboot: Check if baudrate value is supported before sending image
  2022-03-07 18:03 [PATCH v2 1/3] tools: kwboot: Allow to specify custom baudrate only in supported operations Pali Rohár
@ 2022-03-07 18:03 ` Pali Rohár
  2022-03-08 12:15   ` Stefan Roese
  2022-03-07 18:03 ` [PATCH v2 3/3] tools: kwboot: Allow to mix positional arguments with option -b Pali Rohár
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 12+ messages in thread
From: Pali Rohár @ 2022-03-07 18:03 UTC (permalink / raw)
  To: Stefan Roese, Marek Behún, Marcel Ziswiler, Tony Dinh; +Cc: u-boot

Call kwboot_open_tty() which baudrate value which was specified at the
command line by option -B. This function returns error if baudrate is not
supported by selected tty device.

Initial baudrate for image transfer is always 115200, so call
kwboot_tty_change_baudrate() with value 115200 immediately after
kwboot_open_tty() if baudrate specified by option -B is different than
115200.

This makes kwboot fail immediately, informing that baudrate is unsupported,
instead of failing only after the first part of image is already sent.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 tools/kwboot.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/tools/kwboot.c b/tools/kwboot.c
index 986f27c2012a..3b45e19a30aa 100644
--- a/tools/kwboot.c
+++ b/tools/kwboot.c
@@ -2139,12 +2139,24 @@ main(int argc, char **argv)
 		goto usage;
 	}
 
-	tty = kwboot_open_tty(ttypath, imgpath ? 115200 : baudrate);
+	tty = kwboot_open_tty(ttypath, baudrate);
 	if (tty < 0) {
 		perror(ttypath);
 		goto out;
 	}
 
+	/*
+	 * initial baudrate for image transfer is always 115200,
+	 * the change to different baudrate is done only after the header is sent
+	 */
+	if (imgpath && baudrate != 115200) {
+		rc = kwboot_tty_change_baudrate(tty, 115200);
+		if (rc) {
+			perror(ttypath);
+			goto out;
+		}
+	}
+
 	if (baudrate == 115200)
 		/* do not change baudrate during Xmodem to the same value */
 		baudrate = 0;
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 3/3] tools: kwboot: Allow to mix positional arguments with option -b
  2022-03-07 18:03 [PATCH v2 1/3] tools: kwboot: Allow to specify custom baudrate only in supported operations Pali Rohár
  2022-03-07 18:03 ` [PATCH v2 2/3] tools: kwboot: Check if baudrate value is supported before sending image Pali Rohár
@ 2022-03-07 18:03 ` Pali Rohár
  2022-03-08  5:23   ` Tony Dinh
                     ` (2 more replies)
  2022-03-08 12:14 ` [PATCH v2 1/3] tools: kwboot: Allow to specify custom baudrate only in supported operations Stefan Roese
  2022-03-14 15:19 ` Stefan Roese
  3 siblings, 3 replies; 12+ messages in thread
From: Pali Rohár @ 2022-03-07 18:03 UTC (permalink / raw)
  To: Stefan Roese, Marek Behún, Marcel Ziswiler, Tony Dinh; +Cc: u-boot

Commit 9e6d71d2b55f ("tools: kwboot: Allow to use -b without image path as
the last getopt() option") broke usage of kwboot with following arguments:

  kwboot -t -B 115200 /dev/ttyUSB0 -b u-boot-spl.kwb

Fix parsing of option -b with optional argument again.

Fixes: 9e6d71d2b55f ("tools: kwboot: Allow to use -b without image path as the last getopt() option")
Signed-off-by: Pali Rohár <pali@kernel.org>
Reported-by: Tony Dinh <mibodhi@gmail.com>
---
Tony and Marcel, could you test this change if all your kwboot use cases
still work correctly?
---
 tools/kwboot.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/tools/kwboot.c b/tools/kwboot.c
index 3b45e19a30aa..9f2dd2de4ef5 100644
--- a/tools/kwboot.c
+++ b/tools/kwboot.c
@@ -2073,7 +2073,8 @@ main(int argc, char **argv)
 			bootmsg = 1;
 			if (prev_optind == optind)
 				goto usage;
-			if (optind < argc - 1 && argv[optind] && argv[optind][0] != '-')
+			/* Option -b could have optional argument which specify image path */
+			if (optind < argc && argv[optind] && argv[optind][0] != '-')
 				imgpath = argv[optind++];
 			break;
 
@@ -2128,10 +2129,19 @@ main(int argc, char **argv)
 	if (!bootmsg && !term && !debugmsg && !imgpath)
 		goto usage;
 
-	ttypath = argv[optind++];
-
-	if (optind != argc)
+	/*
+	 * If there is no remaining argument but optional imgpath was parsed
+	 * then it means that optional imgpath was eaten by getopt parser.
+	 * Reassing imgpath to required ttypath argument.
+	 */
+	if (optind == argc && imgpath) {
+		ttypath = imgpath;
+		imgpath = NULL;
+	} else if (optind + 1 == argc) {
+		ttypath = argv[optind];
+	} else {
 		goto usage;
+	}
 
 	/* boot and debug message use baudrate 115200 */
 	if (((bootmsg && !imgpath) || debugmsg) && baudrate != 115200) {
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 3/3] tools: kwboot: Allow to mix positional arguments with option -b
  2022-03-07 18:03 ` [PATCH v2 3/3] tools: kwboot: Allow to mix positional arguments with option -b Pali Rohár
@ 2022-03-08  5:23   ` Tony Dinh
  2022-03-08 19:22     ` Pali Rohár
  2022-03-08 12:15   ` Stefan Roese
  2022-03-14 15:23   ` Stefan Roese
  2 siblings, 1 reply; 12+ messages in thread
From: Tony Dinh @ 2022-03-08  5:23 UTC (permalink / raw)
  To: Pali Rohár
  Cc: Stefan Roese, Marek Behún, Marcel Ziswiler, U-Boot Mailing List

Hi Pali,

I've tested this patch series and it's all working fine!

Thanks,
Tony

Tested-by: Tony Dinh <mibodhi at gmail.com>


On Mon, Mar 7, 2022 at 10:04 AM Pali Rohár <pali@kernel.org> wrote:
>
> Commit 9e6d71d2b55f ("tools: kwboot: Allow to use -b without image path as
> the last getopt() option") broke usage of kwboot with following arguments:
>
>   kwboot -t -B 115200 /dev/ttyUSB0 -b u-boot-spl.kwb
>
> Fix parsing of option -b with optional argument again.
>
> Fixes: 9e6d71d2b55f ("tools: kwboot: Allow to use -b without image path as the last getopt() option")
> Signed-off-by: Pali Rohár <pali@kernel.org>
> Reported-by: Tony Dinh <mibodhi@gmail.com>
> ---
> Tony and Marcel, could you test this change if all your kwboot use cases
> still work correctly?
> ---
>  tools/kwboot.c | 18 ++++++++++++++----
>  1 file changed, 14 insertions(+), 4 deletions(-)
>
> diff --git a/tools/kwboot.c b/tools/kwboot.c
> index 3b45e19a30aa..9f2dd2de4ef5 100644
> --- a/tools/kwboot.c
> +++ b/tools/kwboot.c
> @@ -2073,7 +2073,8 @@ main(int argc, char **argv)
>                         bootmsg = 1;
>                         if (prev_optind == optind)
>                                 goto usage;
> -                       if (optind < argc - 1 && argv[optind] && argv[optind][0] != '-')
> +                       /* Option -b could have optional argument which specify image path */
> +                       if (optind < argc && argv[optind] && argv[optind][0] != '-')
>                                 imgpath = argv[optind++];
>                         break;
>
> @@ -2128,10 +2129,19 @@ main(int argc, char **argv)
>         if (!bootmsg && !term && !debugmsg && !imgpath)
>                 goto usage;
>
> -       ttypath = argv[optind++];
> -
> -       if (optind != argc)
> +       /*
> +        * If there is no remaining argument but optional imgpath was parsed
> +        * then it means that optional imgpath was eaten by getopt parser.
> +        * Reassing imgpath to required ttypath argument.
> +        */
> +       if (optind == argc && imgpath) {
> +               ttypath = imgpath;
> +               imgpath = NULL;
> +       } else if (optind + 1 == argc) {
> +               ttypath = argv[optind];
> +       } else {
>                 goto usage;
> +       }
>
>         /* boot and debug message use baudrate 115200 */
>         if (((bootmsg && !imgpath) || debugmsg) && baudrate != 115200) {
> --
> 2.20.1
>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 1/3] tools: kwboot: Allow to specify custom baudrate only in supported operations
  2022-03-07 18:03 [PATCH v2 1/3] tools: kwboot: Allow to specify custom baudrate only in supported operations Pali Rohár
  2022-03-07 18:03 ` [PATCH v2 2/3] tools: kwboot: Check if baudrate value is supported before sending image Pali Rohár
  2022-03-07 18:03 ` [PATCH v2 3/3] tools: kwboot: Allow to mix positional arguments with option -b Pali Rohár
@ 2022-03-08 12:14 ` Stefan Roese
  2022-03-14 15:19 ` Stefan Roese
  3 siblings, 0 replies; 12+ messages in thread
From: Stefan Roese @ 2022-03-08 12:14 UTC (permalink / raw)
  To: Pali Rohár, Marek Behún, Marcel Ziswiler, Tony Dinh; +Cc: u-boot

On 3/7/22 19:03, Pali Rohár wrote:
> Custom baudrate different than 115200 may be specified only when kwboot is
> not going to send boot/debug message pattern or when it is going to send
> boot message pattern with image file (in which case baudrate change happens
> after sending kwbimage header). BootROM detects boot/debug message pattern
> only at baudrate 115200.
> 
> Signed-off-by: Pali Rohár <pali@kernel.org>

Reviewed-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan

> ---
>   tools/kwboot.c | 6 ++++++
>   1 file changed, 6 insertions(+)
> 
> diff --git a/tools/kwboot.c b/tools/kwboot.c
> index 69d1be0f4823..986f27c2012a 100644
> --- a/tools/kwboot.c
> +++ b/tools/kwboot.c
> @@ -2133,6 +2133,12 @@ main(int argc, char **argv)
>   	if (optind != argc)
>   		goto usage;
>   
> +	/* boot and debug message use baudrate 115200 */
> +	if (((bootmsg && !imgpath) || debugmsg) && baudrate != 115200) {
> +		fprintf(stderr, "Baudrate other than 115200 cannot be used for this operation.\n");
> +		goto usage;
> +	}
> +
>   	tty = kwboot_open_tty(ttypath, imgpath ? 115200 : baudrate);
>   	if (tty < 0) {
>   		perror(ttypath);

Viele Grüße,
Stefan Roese

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 2/3] tools: kwboot: Check if baudrate value is supported before sending image
  2022-03-07 18:03 ` [PATCH v2 2/3] tools: kwboot: Check if baudrate value is supported before sending image Pali Rohár
@ 2022-03-08 12:15   ` Stefan Roese
  0 siblings, 0 replies; 12+ messages in thread
From: Stefan Roese @ 2022-03-08 12:15 UTC (permalink / raw)
  To: Pali Rohár, Marek Behún, Marcel Ziswiler, Tony Dinh; +Cc: u-boot

On 3/7/22 19:03, Pali Rohár wrote:
> Call kwboot_open_tty() which baudrate value which was specified at the
> command line by option -B. This function returns error if baudrate is not
> supported by selected tty device.
> 
> Initial baudrate for image transfer is always 115200, so call
> kwboot_tty_change_baudrate() with value 115200 immediately after
> kwboot_open_tty() if baudrate specified by option -B is different than
> 115200.
> 
> This makes kwboot fail immediately, informing that baudrate is unsupported,
> instead of failing only after the first part of image is already sent.
> 
> Signed-off-by: Pali Rohár <pali@kernel.org>

Reviewed-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan


> ---
>   tools/kwboot.c | 14 +++++++++++++-
>   1 file changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/kwboot.c b/tools/kwboot.c
> index 986f27c2012a..3b45e19a30aa 100644
> --- a/tools/kwboot.c
> +++ b/tools/kwboot.c
> @@ -2139,12 +2139,24 @@ main(int argc, char **argv)
>   		goto usage;
>   	}
>   
> -	tty = kwboot_open_tty(ttypath, imgpath ? 115200 : baudrate);
> +	tty = kwboot_open_tty(ttypath, baudrate);
>   	if (tty < 0) {
>   		perror(ttypath);
>   		goto out;
>   	}
>   
> +	/*
> +	 * initial baudrate for image transfer is always 115200,
> +	 * the change to different baudrate is done only after the header is sent
> +	 */
> +	if (imgpath && baudrate != 115200) {
> +		rc = kwboot_tty_change_baudrate(tty, 115200);
> +		if (rc) {
> +			perror(ttypath);
> +			goto out;
> +		}
> +	}
> +
>   	if (baudrate == 115200)
>   		/* do not change baudrate during Xmodem to the same value */
>   		baudrate = 0;

Viele Grüße,
Stefan Roese

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 3/3] tools: kwboot: Allow to mix positional arguments with option -b
  2022-03-07 18:03 ` [PATCH v2 3/3] tools: kwboot: Allow to mix positional arguments with option -b Pali Rohár
  2022-03-08  5:23   ` Tony Dinh
@ 2022-03-08 12:15   ` Stefan Roese
  2022-03-14 15:23   ` Stefan Roese
  2 siblings, 0 replies; 12+ messages in thread
From: Stefan Roese @ 2022-03-08 12:15 UTC (permalink / raw)
  To: Pali Rohár, Marek Behún, Marcel Ziswiler, Tony Dinh; +Cc: u-boot

On 3/7/22 19:03, Pali Rohár wrote:
> Commit 9e6d71d2b55f ("tools: kwboot: Allow to use -b without image path as
> the last getopt() option") broke usage of kwboot with following arguments:
> 
>    kwboot -t -B 115200 /dev/ttyUSB0 -b u-boot-spl.kwb
> 
> Fix parsing of option -b with optional argument again.
> 
> Fixes: 9e6d71d2b55f ("tools: kwboot: Allow to use -b without image path as the last getopt() option")
> Signed-off-by: Pali Rohár <pali@kernel.org>
> Reported-by: Tony Dinh <mibodhi@gmail.com>

Reviewed-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan

> ---
> Tony and Marcel, could you test this change if all your kwboot use cases
> still work correctly?
> ---
>   tools/kwboot.c | 18 ++++++++++++++----
>   1 file changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/kwboot.c b/tools/kwboot.c
> index 3b45e19a30aa..9f2dd2de4ef5 100644
> --- a/tools/kwboot.c
> +++ b/tools/kwboot.c
> @@ -2073,7 +2073,8 @@ main(int argc, char **argv)
>   			bootmsg = 1;
>   			if (prev_optind == optind)
>   				goto usage;
> -			if (optind < argc - 1 && argv[optind] && argv[optind][0] != '-')
> +			/* Option -b could have optional argument which specify image path */
> +			if (optind < argc && argv[optind] && argv[optind][0] != '-')
>   				imgpath = argv[optind++];
>   			break;
>   
> @@ -2128,10 +2129,19 @@ main(int argc, char **argv)
>   	if (!bootmsg && !term && !debugmsg && !imgpath)
>   		goto usage;
>   
> -	ttypath = argv[optind++];
> -
> -	if (optind != argc)
> +	/*
> +	 * If there is no remaining argument but optional imgpath was parsed
> +	 * then it means that optional imgpath was eaten by getopt parser.
> +	 * Reassing imgpath to required ttypath argument.
> +	 */
> +	if (optind == argc && imgpath) {
> +		ttypath = imgpath;
> +		imgpath = NULL;
> +	} else if (optind + 1 == argc) {
> +		ttypath = argv[optind];
> +	} else {
>   		goto usage;
> +	}
>   
>   	/* boot and debug message use baudrate 115200 */
>   	if (((bootmsg && !imgpath) || debugmsg) && baudrate != 115200) {

Viele Grüße,
Stefan Roese

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 3/3] tools: kwboot: Allow to mix positional arguments with option -b
  2022-03-08  5:23   ` Tony Dinh
@ 2022-03-08 19:22     ` Pali Rohár
  2022-03-09  5:51       ` Stefan Roese
  0 siblings, 1 reply; 12+ messages in thread
From: Pali Rohár @ 2022-03-08 19:22 UTC (permalink / raw)
  To: Tony Dinh
  Cc: Stefan Roese, Marek Behún, Marcel Ziswiler, U-Boot Mailing List

On Monday 07 March 2022 21:23:29 Tony Dinh wrote:
> Hi Pali,
> 
> I've tested this patch series and it's all working fine!

Perfect!

Stefan, I think that this change should go into 2022.04 as it is fixing
regression introduced during 2022.04 development.

Could you do more checks/testing of kwboot if everything is also working
fine for you?

> Thanks,
> Tony
> 
> Tested-by: Tony Dinh <mibodhi at gmail.com>
> 
> 
> On Mon, Mar 7, 2022 at 10:04 AM Pali Rohár <pali@kernel.org> wrote:
> >
> > Commit 9e6d71d2b55f ("tools: kwboot: Allow to use -b without image path as
> > the last getopt() option") broke usage of kwboot with following arguments:
> >
> >   kwboot -t -B 115200 /dev/ttyUSB0 -b u-boot-spl.kwb
> >
> > Fix parsing of option -b with optional argument again.
> >
> > Fixes: 9e6d71d2b55f ("tools: kwboot: Allow to use -b without image path as the last getopt() option")
> > Signed-off-by: Pali Rohár <pali@kernel.org>
> > Reported-by: Tony Dinh <mibodhi@gmail.com>
> > ---
> > Tony and Marcel, could you test this change if all your kwboot use cases
> > still work correctly?
> > ---
> >  tools/kwboot.c | 18 ++++++++++++++----
> >  1 file changed, 14 insertions(+), 4 deletions(-)
> >
> > diff --git a/tools/kwboot.c b/tools/kwboot.c
> > index 3b45e19a30aa..9f2dd2de4ef5 100644
> > --- a/tools/kwboot.c
> > +++ b/tools/kwboot.c
> > @@ -2073,7 +2073,8 @@ main(int argc, char **argv)
> >                         bootmsg = 1;
> >                         if (prev_optind == optind)
> >                                 goto usage;
> > -                       if (optind < argc - 1 && argv[optind] && argv[optind][0] != '-')
> > +                       /* Option -b could have optional argument which specify image path */
> > +                       if (optind < argc && argv[optind] && argv[optind][0] != '-')
> >                                 imgpath = argv[optind++];
> >                         break;
> >
> > @@ -2128,10 +2129,19 @@ main(int argc, char **argv)
> >         if (!bootmsg && !term && !debugmsg && !imgpath)
> >                 goto usage;
> >
> > -       ttypath = argv[optind++];
> > -
> > -       if (optind != argc)
> > +       /*
> > +        * If there is no remaining argument but optional imgpath was parsed
> > +        * then it means that optional imgpath was eaten by getopt parser.
> > +        * Reassing imgpath to required ttypath argument.
> > +        */
> > +       if (optind == argc && imgpath) {
> > +               ttypath = imgpath;
> > +               imgpath = NULL;
> > +       } else if (optind + 1 == argc) {
> > +               ttypath = argv[optind];
> > +       } else {
> >                 goto usage;
> > +       }
> >
> >         /* boot and debug message use baudrate 115200 */
> >         if (((bootmsg && !imgpath) || debugmsg) && baudrate != 115200) {
> > --
> > 2.20.1
> >

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 3/3] tools: kwboot: Allow to mix positional arguments with option -b
  2022-03-08 19:22     ` Pali Rohár
@ 2022-03-09  5:51       ` Stefan Roese
  0 siblings, 0 replies; 12+ messages in thread
From: Stefan Roese @ 2022-03-09  5:51 UTC (permalink / raw)
  To: Pali Rohár, Tony Dinh
  Cc: Marek Behún, Marcel Ziswiler, U-Boot Mailing List

On 3/8/22 20:22, Pali Rohár wrote:
> On Monday 07 March 2022 21:23:29 Tony Dinh wrote:
>> Hi Pali,
>>
>> I've tested this patch series and it's all working fine!
> 
> Perfect!
> 
> Stefan, I think that this change should go into 2022.04 as it is fixing
> regression introduced during 2022.04 development.

I agree.

> Could you do more checks/testing of kwboot if everything is also working
> fine for you?

I'll try to do this later this week, yes.

Thanks,
Stefan

>> Thanks,
>> Tony
>>
>> Tested-by: Tony Dinh <mibodhi at gmail.com>
>>
>>
>> On Mon, Mar 7, 2022 at 10:04 AM Pali Rohár <pali@kernel.org> wrote:
>>>
>>> Commit 9e6d71d2b55f ("tools: kwboot: Allow to use -b without image path as
>>> the last getopt() option") broke usage of kwboot with following arguments:
>>>
>>>    kwboot -t -B 115200 /dev/ttyUSB0 -b u-boot-spl.kwb
>>>
>>> Fix parsing of option -b with optional argument again.
>>>
>>> Fixes: 9e6d71d2b55f ("tools: kwboot: Allow to use -b without image path as the last getopt() option")
>>> Signed-off-by: Pali Rohár <pali@kernel.org>
>>> Reported-by: Tony Dinh <mibodhi@gmail.com>
>>> ---
>>> Tony and Marcel, could you test this change if all your kwboot use cases
>>> still work correctly?
>>> ---
>>>   tools/kwboot.c | 18 ++++++++++++++----
>>>   1 file changed, 14 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/tools/kwboot.c b/tools/kwboot.c
>>> index 3b45e19a30aa..9f2dd2de4ef5 100644
>>> --- a/tools/kwboot.c
>>> +++ b/tools/kwboot.c
>>> @@ -2073,7 +2073,8 @@ main(int argc, char **argv)
>>>                          bootmsg = 1;
>>>                          if (prev_optind == optind)
>>>                                  goto usage;
>>> -                       if (optind < argc - 1 && argv[optind] && argv[optind][0] != '-')
>>> +                       /* Option -b could have optional argument which specify image path */
>>> +                       if (optind < argc && argv[optind] && argv[optind][0] != '-')
>>>                                  imgpath = argv[optind++];
>>>                          break;
>>>
>>> @@ -2128,10 +2129,19 @@ main(int argc, char **argv)
>>>          if (!bootmsg && !term && !debugmsg && !imgpath)
>>>                  goto usage;
>>>
>>> -       ttypath = argv[optind++];
>>> -
>>> -       if (optind != argc)
>>> +       /*
>>> +        * If there is no remaining argument but optional imgpath was parsed
>>> +        * then it means that optional imgpath was eaten by getopt parser.
>>> +        * Reassing imgpath to required ttypath argument.
>>> +        */
>>> +       if (optind == argc && imgpath) {
>>> +               ttypath = imgpath;
>>> +               imgpath = NULL;
>>> +       } else if (optind + 1 == argc) {
>>> +               ttypath = argv[optind];
>>> +       } else {
>>>                  goto usage;
>>> +       }
>>>
>>>          /* boot and debug message use baudrate 115200 */
>>>          if (((bootmsg && !imgpath) || debugmsg) && baudrate != 115200) {
>>> --
>>> 2.20.1
>>>

Viele Grüße,
Stefan Roese

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 1/3] tools: kwboot: Allow to specify custom baudrate only in supported operations
  2022-03-07 18:03 [PATCH v2 1/3] tools: kwboot: Allow to specify custom baudrate only in supported operations Pali Rohár
                   ` (2 preceding siblings ...)
  2022-03-08 12:14 ` [PATCH v2 1/3] tools: kwboot: Allow to specify custom baudrate only in supported operations Stefan Roese
@ 2022-03-14 15:19 ` Stefan Roese
  3 siblings, 0 replies; 12+ messages in thread
From: Stefan Roese @ 2022-03-14 15:19 UTC (permalink / raw)
  To: Pali Rohár, Marek Behún, Marcel Ziswiler, Tony Dinh; +Cc: u-boot

On 3/7/22 19:03, Pali Rohár wrote:
> Custom baudrate different than 115200 may be specified only when kwboot is
> not going to send boot/debug message pattern or when it is going to send
> boot message pattern with image file (in which case baudrate change happens
> after sending kwbimage header). BootROM detects boot/debug message pattern
> only at baudrate 115200.
> 
> Signed-off-by: Pali Rohár <pali@kernel.org>

Applied to u-boot-marvell/master

Thanks,
Stefan

> ---
>   tools/kwboot.c | 6 ++++++
>   1 file changed, 6 insertions(+)
> 
> diff --git a/tools/kwboot.c b/tools/kwboot.c
> index 69d1be0f4823..986f27c2012a 100644
> --- a/tools/kwboot.c
> +++ b/tools/kwboot.c
> @@ -2133,6 +2133,12 @@ main(int argc, char **argv)
>   	if (optind != argc)
>   		goto usage;
>   
> +	/* boot and debug message use baudrate 115200 */
> +	if (((bootmsg && !imgpath) || debugmsg) && baudrate != 115200) {
> +		fprintf(stderr, "Baudrate other than 115200 cannot be used for this operation.\n");
> +		goto usage;
> +	}
> +
>   	tty = kwboot_open_tty(ttypath, imgpath ? 115200 : baudrate);
>   	if (tty < 0) {
>   		perror(ttypath);

Viele Grüße,
Stefan Roese

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 3/3] tools: kwboot: Allow to mix positional arguments with option -b
  2022-03-07 18:03 ` [PATCH v2 3/3] tools: kwboot: Allow to mix positional arguments with option -b Pali Rohár
  2022-03-08  5:23   ` Tony Dinh
  2022-03-08 12:15   ` Stefan Roese
@ 2022-03-14 15:23   ` Stefan Roese
  2 siblings, 0 replies; 12+ messages in thread
From: Stefan Roese @ 2022-03-14 15:23 UTC (permalink / raw)
  To: Pali Rohár, Marek Behún, Marcel Ziswiler, Tony Dinh; +Cc: u-boot

On 3/7/22 19:03, Pali Rohár wrote:
> Commit 9e6d71d2b55f ("tools: kwboot: Allow to use -b without image path as
> the last getopt() option") broke usage of kwboot with following arguments:
> 
>    kwboot -t -B 115200 /dev/ttyUSB0 -b u-boot-spl.kwb
> 
> Fix parsing of option -b with optional argument again.
> 
> Fixes: 9e6d71d2b55f ("tools: kwboot: Allow to use -b without image path as the last getopt() option")
> Signed-off-by: Pali Rohár <pali@kernel.org>
> Reported-by: Tony Dinh <mibodhi@gmail.com>

Applied to u-boot-marvell/master

Thanks,
Stefan

> ---
> Tony and Marcel, could you test this change if all your kwboot use cases
> still work correctly?
> ---
>   tools/kwboot.c | 18 ++++++++++++++----
>   1 file changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/kwboot.c b/tools/kwboot.c
> index 3b45e19a30aa..9f2dd2de4ef5 100644
> --- a/tools/kwboot.c
> +++ b/tools/kwboot.c
> @@ -2073,7 +2073,8 @@ main(int argc, char **argv)
>   			bootmsg = 1;
>   			if (prev_optind == optind)
>   				goto usage;
> -			if (optind < argc - 1 && argv[optind] && argv[optind][0] != '-')
> +			/* Option -b could have optional argument which specify image path */
> +			if (optind < argc && argv[optind] && argv[optind][0] != '-')
>   				imgpath = argv[optind++];
>   			break;
>   
> @@ -2128,10 +2129,19 @@ main(int argc, char **argv)
>   	if (!bootmsg && !term && !debugmsg && !imgpath)
>   		goto usage;
>   
> -	ttypath = argv[optind++];
> -
> -	if (optind != argc)
> +	/*
> +	 * If there is no remaining argument but optional imgpath was parsed
> +	 * then it means that optional imgpath was eaten by getopt parser.
> +	 * Reassing imgpath to required ttypath argument.
> +	 */
> +	if (optind == argc && imgpath) {
> +		ttypath = imgpath;
> +		imgpath = NULL;
> +	} else if (optind + 1 == argc) {
> +		ttypath = argv[optind];
> +	} else {
>   		goto usage;
> +	}
>   
>   	/* boot and debug message use baudrate 115200 */
>   	if (((bootmsg && !imgpath) || debugmsg) && baudrate != 115200) {

Viele Grüße,
Stefan Roese

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 2/3] tools: kwboot: Check if baudrate value is supported before sending image
  2022-03-06 23:12 [PATCH] arm: kirkwood: Sheevaplug : Use Marvell uclass mvgbe and PHY driver for Ethernet Tony Dinh
@ 2022-03-14 15:19 ` Stefan Roese
  0 siblings, 0 replies; 12+ messages in thread
From: Stefan Roese @ 2022-03-14 15:19 UTC (permalink / raw)
  To: Tony Dinh, U-Boot Mailing List; +Cc: Tom Rini

On 3/7/22 19:03, Pali Rohár wrote:
> The Globalscale Technologies Sheevaplug board has the network chip
> Marvell 88E1116R. Use uclass mvgbe and the compatible driver M88E1310
> driver to bring up Ethernet.
> 
> - Currently, CONFIG_RESET_PHY_R symbol is used in
> arch/arm/mach-kirkwood/include/mach/config.h for all Kirkwood
> boards with mv8831116 PHY, with each board defines the function
> reset_phy(). Undefine it for this board.
> - Add board_eth_init() to use uclass mvgbe to bring up the network.
> And remove ad-hoc code.
> - Enable CONFIG_PHY_MARVELL to properly configure the network.
> - Miscellaneous changes: Move constants to .c file and remove header file
> board/Marvell/sheevaplug/sheevaplug.h, use BIT macro, and add/cleanup
> comments.
> 
> Signed-off-by: Tony Dinh <mibodhi@gmail.com>

Applied to u-boot-marvell/master

Thanks,
Stefan

> ---
> 
>   board/Marvell/sheevaplug/sheevaplug.c | 83 +++++----------------------
>   board/Marvell/sheevaplug/sheevaplug.h | 24 --------
>   configs/sheevaplug_defconfig          |  1 +
>   include/configs/sheevaplug.h          | 17 +++---
>   4 files changed, 22 insertions(+), 103 deletions(-)
>   delete mode 100644 board/Marvell/sheevaplug/sheevaplug.h
> 
> diff --git a/board/Marvell/sheevaplug/sheevaplug.c b/board/Marvell/sheevaplug/sheev

Viele Grüße,
Stefan Roese

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2022-03-14 15:23 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-07 18:03 [PATCH v2 1/3] tools: kwboot: Allow to specify custom baudrate only in supported operations Pali Rohár
2022-03-07 18:03 ` [PATCH v2 2/3] tools: kwboot: Check if baudrate value is supported before sending image Pali Rohár
2022-03-08 12:15   ` Stefan Roese
2022-03-07 18:03 ` [PATCH v2 3/3] tools: kwboot: Allow to mix positional arguments with option -b Pali Rohár
2022-03-08  5:23   ` Tony Dinh
2022-03-08 19:22     ` Pali Rohár
2022-03-09  5:51       ` Stefan Roese
2022-03-08 12:15   ` Stefan Roese
2022-03-14 15:23   ` Stefan Roese
2022-03-08 12:14 ` [PATCH v2 1/3] tools: kwboot: Allow to specify custom baudrate only in supported operations Stefan Roese
2022-03-14 15:19 ` Stefan Roese
  -- strict thread matches above, loose matches on Subject: below --
2022-03-06 23:12 [PATCH] arm: kirkwood: Sheevaplug : Use Marvell uclass mvgbe and PHY driver for Ethernet Tony Dinh
2022-03-14 15:19 ` [PATCH v2 2/3] tools: kwboot: Check if baudrate value is supported before sending image Stefan Roese

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.