[PATCH v2 1/3] tools: kwboot: Allow to specify custom baudrate only in supported operations

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);

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;

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

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) {

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

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

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

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

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

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

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
participants (3)
-
Pali Rohár
-
Stefan Roese
-
Tony Dinh