| Message ID | 20210803142844.19455-7-pali@kernel.org |
|---|---|
| State | Accepted |
| Commit | c97b2557bcd5899cdf7bd57e09379b159f4796c8 |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | xyz-modem: Fix cancelling and closing transfers | expand |
On 03.08.21 16:28, Pali Rohár wrote: > Show "## Start Addr" or "## Binary (...) download aborted" information like > in Kermit "loadb" command. > > Signed-off-by: Pali Rohár <pali@kernel.org> > --- > cmd/load.c | 16 ++++++++++++++++ > 1 file changed, 16 insertions(+) > > diff --git a/cmd/load.c b/cmd/load.c > index 1ed05a9cd21e..573c44b912fd 100644 > --- a/cmd/load.c > +++ b/cmd/load.c > @@ -474,6 +474,14 @@ static int do_load_serial_bin(struct cmd_tbl *cmdtp, int flag, int argc, > > addr = load_serial_ymodem(offset, xyzModem_ymodem); > > + if (addr == ~0) { > + image_load_addr = 0; > + printf("## Binary (ymodem) download aborted\n"); xyzModem_stream_terminate() already prints a message "!!!! TRANSFER ABORT !!!!\n" if the transfer is aborted. > + rcode = 1; > + } else { > + printf("## Start Addr = 0x%08lX\n", addr); In existing code '## ' is used to indicate errors. This message is not for an error. Please, avoid alternative language referring to the load address. Cf. net/nfs.c:918: printf("\nLoad address: 0x%lx\nLoading: *\b", image_load_addr); net/tftp.c:851: printf("Load address: 0x%lx\n", tftp_load_addr); net/tftp.c:905: printf("Load address: 0x%lx\n", tftp_load_addr) Please, use log_err() and log_info() instead of printf(). Best regards Heinrich > + image_load_addr = addr; > + } > } else if (strcmp(argv[0],"loadx")==0) { > printf("## Ready for binary (xmodem) download " > "to 0x%08lX at %d bps...\n", > @@ -482,6 +490,14 @@ static int do_load_serial_bin(struct cmd_tbl *cmdtp, int flag, int argc, > > addr = load_serial_ymodem(offset, xyzModem_xmodem); > > + if (addr == ~0) { > + image_load_addr = 0; > + printf("## Binary (xmodem) download aborted\n"); > + rcode = 1; > + } else { > + printf("## Start Addr = 0x%08lX\n", addr); > + image_load_addr = addr; > + } > } else { > > printf("## Ready for binary (kermit) download " >
On Wednesday 04 August 2021 11:15:53 Heinrich Schuchardt wrote: > On 03.08.21 16:28, Pali Rohár wrote: > > Show "## Start Addr" or "## Binary (...) download aborted" information like > > in Kermit "loadb" command. > > > > Signed-off-by: Pali Rohár <pali@kernel.org> > > --- > > cmd/load.c | 16 ++++++++++++++++ > > 1 file changed, 16 insertions(+) > > > > diff --git a/cmd/load.c b/cmd/load.c > > index 1ed05a9cd21e..573c44b912fd 100644 > > --- a/cmd/load.c > > +++ b/cmd/load.c > > @@ -474,6 +474,14 @@ static int do_load_serial_bin(struct cmd_tbl *cmdtp, int flag, int argc, > > addr = load_serial_ymodem(offset, xyzModem_ymodem); > > + if (addr == ~0) { > > + image_load_addr = 0; > > + printf("## Binary (ymodem) download aborted\n"); > > xyzModem_stream_terminate() already prints a message > "!!!! TRANSFER ABORT !!!!\n" > if the transfer is aborted. But prior returning terminal into the normal state. So in most cases receiver does not see this message as it would be eat by ymodem client application. I have tested it and it is not visible in ckermit tool (which is terminal with support for xmodem, ymodem and kermit protocols). > > + rcode = 1; > > + } else { > > + printf("## Start Addr = 0x%08lX\n", addr); > > In existing code '## ' is used to indicate errors. This message is not for > an error. Existing code also already uses it for information about starting transfer and also for information about size of the transfer. This is for all load* commands. And for loadb and loads it prints also this "Start Addr", so for consistency with older load* transfers commands I added it also into xmodem and ymodem. And I used exactly same style and format, to have uniform output. > Please, avoid alternative language referring to the load address. Cf. > net/nfs.c:918: > printf("\nLoad address: 0x%lx\nLoading: *\b", image_load_addr); > net/tftp.c:851: > printf("Load address: 0x%lx\n", tftp_load_addr); > net/tftp.c:905: > printf("Load address: 0x%lx\n", tftp_load_addr) > > Please, use log_err() and log_info() instead of printf(). > > Best regards > > Heinrich > > > + image_load_addr = addr; > > + } > > } else if (strcmp(argv[0],"loadx")==0) { > > printf("## Ready for binary (xmodem) download " > > "to 0x%08lX at %d bps...\n", > > @@ -482,6 +490,14 @@ static int do_load_serial_bin(struct cmd_tbl *cmdtp, int flag, int argc, > > addr = load_serial_ymodem(offset, xyzModem_xmodem); > > + if (addr == ~0) { > > + image_load_addr = 0; > > + printf("## Binary (xmodem) download aborted\n"); > > + rcode = 1; > > + } else { > > + printf("## Start Addr = 0x%08lX\n", addr); > > + image_load_addr = addr; > > + } > > } else { > > printf("## Ready for binary (kermit) download " > >
diff --git a/cmd/load.c b/cmd/load.c index 1ed05a9cd21e..573c44b912fd 100644 --- a/cmd/load.c +++ b/cmd/load.c @@ -474,6 +474,14 @@ static int do_load_serial_bin(struct cmd_tbl *cmdtp, int flag, int argc, addr = load_serial_ymodem(offset, xyzModem_ymodem); + if (addr == ~0) { + image_load_addr = 0; + printf("## Binary (ymodem) download aborted\n"); + rcode = 1; + } else { + printf("## Start Addr = 0x%08lX\n", addr); + image_load_addr = addr; + } } else if (strcmp(argv[0],"loadx")==0) { printf("## Ready for binary (xmodem) download " "to 0x%08lX at %d bps...\n", @@ -482,6 +490,14 @@ static int do_load_serial_bin(struct cmd_tbl *cmdtp, int flag, int argc, addr = load_serial_ymodem(offset, xyzModem_xmodem); + if (addr == ~0) { + image_load_addr = 0; + printf("## Binary (xmodem) download aborted\n"); + rcode = 1; + } else { + printf("## Start Addr = 0x%08lX\n", addr); + image_load_addr = addr; + } } else { printf("## Ready for binary (kermit) download "
Show "## Start Addr" or "## Binary (...) download aborted" information like in Kermit "loadb" command. Signed-off-by: Pali Rohár <pali@kernel.org> --- cmd/load.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)