1/5: cmd_boot
PATCH 1/1 1/5: cmd_boot
Hi,
here are some last minute patches, I hope Wolfgang has time to check them in before the release.
-----
cmd_boot:
- fix whitespace for bdinfo command
- fix garbage characters output
- add ctrl-c support for kermit download
Robert
Signature · 74 lines
Dipl.-Ing. Robert Schwebel | http://www.pengutronix.de
Pengutronix - Linux Solutions for Science and Industry
Braunschweiger Str. 79, 31134 Hildesheim, Germany
Handelsregister: Amtsgericht Hildesheim, HRA 2686
Phone: +49-5121-28619-0 | Fax: +49-5121-28619-4
-------------- next part --------------
diff -x CVS -x ptx-patches -urN u-boot/common/cmd_boot.c u-boot-ptx/common/cmd_boot.c
--- u-boot/common/cmd_boot.c 2003-03-31 08:34:19.000000000 +0200
+++ u-boot-ptx/common/cmd_boot.c 2003-03-31 08:39:33.000000000 +0200
@@ -163,10 +163,10 @@
printf ("%c%02X", i ? ':' : ' ', bd->bi_enetaddr[i]);
}
printf ("\n"
- "ip_addr = ");
+ "ip_addr = ");
print_IPaddr (bd->bi_ip_addr);
printf ("\n"
- "baudrate = %d bps\n", bd->bi_baudrate);
+ "baudrate = %d bps\n", bd->bi_baudrate);
return 0;
}
@@ -575,6 +575,7 @@
#define XON_CHAR 17
#define XOFF_CHAR 19
#define START_CHAR 0x01
+#define ETX_CHAR 0x03
#define END_CHAR 0x0D
#define SPACE 0x20
#define K_ESCAPE 0x23
@@ -687,11 +688,24 @@
static ulong load_serial_bin (ulong offset)
{
- int size;
+ int size, i;
char buf[32];
set_kerm_bin_mode ((ulong *) offset);
size = k_recv ();
+
+ /*
+ * Gather any trailing characters (for instance, the ^D which
+ * is sent by 'cu' after sending a file), and give the
+ * box some time (100 * 1 ms)
+ */
+ for (i=0; i<100; ++i) {
+ if (serial_tstc()) {
+ (void) serial_getc();
+ }
+ udelay(1000);
+ }
+
flush_cache (offset, size);
printf("## Total Size = 0x%08x = %d Bytes\n", size, size);
@@ -996,10 +1011,16 @@
/* get a packet */
/* wait for the starting character */
- while (serial_getc () != START_CHAR);
+ do {
+ new_char = serial_getc();
+ if (new_char == ETX_CHAR) return 0;
+
+ } while (new_char != START_CHAR);
+
/* get length of packet */
sum = 0;
new_char = serial_getc ();
+
if ((new_char & 0xE0) == 0)
goto packet_error;
sum += new_char & 0xff;PATCH 1/1 1/5: cmd_boot
Quoted 11 lines · In message <[email protected]> you wrote:
here are some last minute patches, I hope Wolfgang has time to check them in before the release.
-----
cmd_boot:- fix whitespace for bdinfo command
- fix garbage characters output
- add ctrl-c support for kermit download
See below.
common/cmd_boot.c line 169
Quoted 12 lines
common/cmd_boot.c +2
-2@@ -163,10 +163,10 @@printf ("%c%02X" , i ?':' :' ' , bd->bi_enetaddr[i]); }printf ("\n" -"ip_addr = " ); +"ip_addr = " );print_IPaddr (bd->bi_ip_addr);printf ("\n" -"baudrate = %d bps\n" , bd->bi_baudrate); +"baudrate = %d bps\n" , bd->bi_baudrate);
Which "whitespace" problem are you trying to fix? To me the current output looks good:
=> bdi memstart = 0x00000000 memsize = 0x01000000 flashstart = 0x40000000 flashsize = 0x00800000 flashoffset = 0x00040000 sramstart = 0x00000000 sramsize = 0x00000000 immr_base = 0xFFF00000 bootflags = 0x00000001 intfreq = 50 MHz busfreq = 50 MHz ethaddr = 00:D0:93:00:7B:8B IP addr = 10.0.0.99 baudrate = 115200 bps
Quoted 20 lines
common/cmd_boot.c +4
-1@@ -575,6 +575,7 @@#define XON_CHAR 17#define XOFF_CHAR 19#define START_CHAR 0x01 +#define ETX_CHAR 0x03#define END_CHAR 0x0D#define SPACE 0x20#define K_ESCAPE 0x23@@ -687,11 +688,24 @@static ulong load_serial_bin (ulong offset) { -int size; +int size, i;char buf[32 ];set_kerm_bin_mode ((ulong *) offset); size =k_recv (); + +/*
common/cmd_boot.c line 707
Quoted 10 lines
common/cmd_boot.c +10
-0+* Gather any trailing characters (for instance, the ^D which +* is sent by 'cu' after sending a file), and give the +* box some time (100 * 1 ms) +*/ +for (i=0 ; i<100 ; ++i) { +if (serial_tstc ()) { + (void )serial_getc (); + } +udelay (1000 ); + }
This makes no sense to me. load_serial_bin() is used for kermit serial protocol download, but "cu" is used for S-Record (ASCII file) download. So how would "cu" disturb load_serial_bin() ?
Can you please explain which problem you are trying to fix?
common/cmd_boot.c
Quoted 8 lines
common/cmd_boot.c +6
-1/* wait for the starting character */ -while (serial_getc () !=START_CHAR ); +do { + new_char =serial_getc (); +if (new_char ==ETX_CHAR )return 0 ; + + }while (new_char !=START_CHAR ); +
Will add this.
Best regards,
Wolfgang Denk
Signature · 4 lines
Software Engineering: Embedded and Realtime Systems, Embedded Linux Phone: (+49)-8142-4596-87 Fax: (+49)-8142-4596-88 Email: wd at denx.de If there are self-made purgatories, then we all have to live in them. -- Spock, "This Side of Paradise", stardate 3417.7
PATCH 1/1 1/5: cmd_boot
Quoted 18 lines · Wolfgang Denk
Which "whitespace" problem are you trying to fix? To me the current output looks good:
=> bdi memstart = 0x00000000 memsize = 0x01000000 flashstart = 0x40000000 flashsize = 0x00800000 flashoffset = 0x00040000 sramstart = 0x00000000 sramsize = 0x00000000 immr_base = 0xFFF00000 bootflags = 0x00000001 intfreq = 50 MHz busfreq = 50 MHz ethaddr = 00:D0:93:00:7B:8B IP addr = 10.0.0.99 baudrate = 115200 bps
... but you have tried it on ppc, not on ARM :-) Your = is in column 13, and so it is in my patch.
Quoted 7 lines
Quoted 7 lines
(quoted diff) +7
-0+ /* + * Gather any trailing characters (for instance, the ^D which + * is sent by 'cu' after sending a file), and give the + * box some time (100 * 1 ms) + */ + for (i=0; i<100; ++i) { + if (serial_tstc()) {
Quoted 8 lines
Review
Quoted 4 lines
(quoted diff) +4
-0+ (void) serial_getc(); + } + udelay(1000); + }This makes no sense to me.
load_serial_bin()is used for kermit serial protocol download, but "cu" is used for S-Record (ASCII file) download. So how would "cu" disturbload_serial_bin()?
Oops, documentation-cut-and-paste error. It's the same with kermit. Without the code (which was there before including the comment, I just moved it to the proper position, which I forgot in the last patch you've already applied).
Robert
Signature · 5 lines
Dipl.-Ing. Robert Schwebel | http://www.pengutronix.de
Pengutronix - Linux Solutions for Science and Industry
Braunschweiger Str. 79, 31134 Hildesheim, Germany
Handelsregister: Amtsgericht Hildesheim, HRA 2686
Phone: +49-5121-28619-0 | Fax: +49-5121-28619-4PATCH 1/1 1/5: cmd_boot
Quoted 3 lines · In message <[email protected]> you wrote:
... but you have tried it on ppc, not on ARM :-) Your = is in column 13, and so it is in my patch.
I see. Thanks for the explanation -- added.
Quoted 4 lines
Oops, documentation-cut-and-paste error. It's the same with kermit. Without the code (which was there before including the comment, I just moved it to the proper position, which I forgot in the last patch you've already applied).
I never experienced any problems with the kermit code, and I used it a lot. Can you please describe in detail how you use it, and which sort of problems you have?
You are _not_ using minicom by any chance?
Best regards,
Wolfgang Denk
Signature · 4 lines
Software Engineering: Embedded and Realtime Systems, Embedded Linux Phone: (+49)-8142-4596-87 Fax: (+49)-8142-4596-88 Email: wd at denx.de A meeting is an event at which the minutes are kept and the hours are lost.
PATCH 1/1 1/5: cmd_boot
Quoted 10 lines · Wolfgang Denk
Quoted 4 lines
Oops, documentation-cut-and-paste error. It's the same with kermit. Without the code (which was there before including the comment, I just moved it to the proper position, which I forgot in the last patch you've already applied).
I never experienced any problems with the kermit code, and I used it a lot. Can you please describe in detail how you use it, and which sort of problems you have?
You are _not_ using minicom by any chance?
I do, and nobody was able to explain me why it should be bad. It simply works with all other firmwares and it works with U-Boot with the patch.
Robert
Signature · 5 lines
Dipl.-Ing. Robert Schwebel | http://www.pengutronix.de
Pengutronix - Linux Solutions for Science and Industry
Braunschweiger Str. 79, 31134 Hildesheim, Germany
Handelsregister: Amtsgericht Hildesheim, HRA 2686
Phone: +49-5121-28619-0 | Fax: +49-5121-28619-4PATCH 1/1 1/5: cmd_boot
Quoted 5 lines · In message <[email protected]> you wrote:
Quoted 2 lines
You are _not_ using minicom by any chance?
I do, and nobody was able to explain me why it should be bad. It simply works with all other firmwares and it works with U-Boot with the patch.
I recommend to kermit directly, and there will be no problem. The patch adds code size in a critical area (there are some boards with memory limitations) and does not fix any real problem (instead, it implements a workaround for other broken software).
Does anybody else really _need_ this patch?
Best regards,
Wolfgang Denk
Signature · 3 lines
Software Engineering: Embedded and Realtime Systems, Embedded Linux Phone: (+49)-8142-4596-87 Fax: (+49)-8142-4596-88 Email: wd at denx.de It is much easier to suggest solutions when you know nothing
PATCH 1/1 1/5: cmd_boot
Quoted 4 lines · Wolfgang Denk
I recommend to kermit directly, and there will be no problem. The patch adds code size in a critical area (there are some boards with memory limitations) and does not fix any real problem (instead, it implements a workaround for other broken software).
Come on, minicom is by large the most used terminal program on Linux, and I suppose u-boot should work with it. Don't tell me that I'm the only one using it - nearly EVERYONE who works with serial hardware (terminal servers, serve consoles, embedded boards) does it with minicom today, and u-boot is the only firmware I've seen problems with so far.
Robert
Signature · 5 lines
Dipl.-Ing. Robert Schwebel | http://www.pengutronix.de
Pengutronix - Linux Solutions for Science and Industry
Braunschweiger Str. 79, 31134 Hildesheim, Germany
Handelsregister: Amtsgericht Hildesheim, HRA 2686
Phone: +49-5121-28619-0 | Fax: +49-5121-28619-4PATCH 1/1 1/5: cmd_boot
Quoted 7 lines · In message <[email protected]> you wrote:
Quoted 4 lines · Wolfgang Denk
I recommend to kermit directly, and there will be no problem. The patch adds code size in a critical area (there are some boards with memory limitations) and does not fix any real problem (instead, it implements a workaround for other broken software).
Come on, minicom is by large the most used terminal program on Linux,
That does not mean that it doesn't show some weird behaviour.
Quoted 1 line
and I suppose u-boot should work with it. Don't tell me that I'm the
Ummm... if there are better. working alternatives I feel tempted to ignore broken software.
Quoted 2 lines
only one using it - nearly EVERYONE who works with serial hardware (terminal servers, serve consoles, embedded boards) does it with minicom
Really? I don't. And I don't recommend it. Instead, I recommend to ignore it. I see little advanteages. Use Ckermit - it is much more flexible, it works, and it it available for a plethora of systems, not only Linux.
Best regards,
Wolfgang Denk
Signature · 3 lines
Software Engineering: Embedded and Realtime Systems, Embedded Linux Phone: (+49)-8142-4596-87 Fax: (+49)-8142-4596-88 Email: wd at denx.de What was sliced bread the greatest thing since?
PATCH 1/1 1/5: cmd_boot
Quoted 1 line · Wolfgang Denk
That does not mean that it doesn't show some weird behaviour.
Are you sure the weird behaviour comes from minicom? Or is it the combination of minicom and some of it's backend programs?
Quoted 2 lines
Ummm... if there are better. working alternatives I feel tempted to ignore broken software.
I generally have no problem with using stone aged software, but for a terminal program (which is also used by non-hackers) I prefer using something with at least a _little_ bit of usability comfort (menues and stuff). Unfortunately, there is no better thing than minicom around for Linux...
Quoted 4 lines
Really? I don't. And I don't recommend it. Instead, I recommend to ignore it. I see little advanteages. Use Ckermit - it is much more flexible, it works, and it it available for a plethora of systems, not only Linux.
People use it, so it should be supported. Other question: does my patch do any harm to your ckermit? If not, what's the problem?
Robert
Signature · 5 lines
Dipl.-Ing. Robert Schwebel | http://www.pengutronix.de
Pengutronix - Linux Solutions for Science and Industry
Braunschweiger Str. 79, 31134 Hildesheim, Germany
Handelsregister: Amtsgericht Hildesheim, HRA 2686
Phone: +49-5121-28619-0 | Fax: +49-5121-28619-4PATCH 1/1 1/5: cmd_boot
Quoted 5 lines · In message <[email protected]> you wrote:
Quoted 1 line · Wolfgang Denk
That does not mean that it doesn't show some weird behaviour.
Are you sure the weird behaviour comes from minicom? Or is it the combination of minicom and some of it's backend programs?
I don't know.
Quoted 2 lines
stuff). Unfortunately, there is no better thing than minicom around for Linux...
Ummm... YMMV, but IMHO a working tool is _always_ better than a broken one, no matter which user interface it has.
Quoted 2 lines
People use it, so it should be supported. Other question: does my patch do any harm to your ckermit? If not, what's the problem?
Code size. Yes, even these few bytes.
Best regards,
Wolfgang Denk
Signature · 4 lines
Software Engineering: Embedded and Realtime Systems, Embedded Linux Phone: (+49)-8142-4596-87 Fax: (+49)-8142-4596-88 Email: wd at denx.de Life would be so much easier if we could just look at the source code. -- Dave Olson
PATCH 1/1 1/5: cmd_boot
Quoted 2 lines · Wolfgang Denk
Ummm... YMMV, but IMHO a working tool is _always_ better than a broken one, no matter which user interface it has.
Above you stated that you don't know if minicom is broken or if it has another reason.
Quoted 4 lines
Quoted 2 lines
People use it, so it should be supported. Other question: does my patch do any harm to your ckermit? If not, what's the problem?
Code size. Yes, even these few bytes.
Wolfgang, the code has been in that file for ages. The only thing I did is to move it to the right location, which is _before_ the printf() in load_serial_bin(), not _after_ the function returned. Please explain why moving code changes the binary size.
Robert
Signature · 5 lines
Dipl.-Ing. Robert Schwebel | http://www.pengutronix.de
Pengutronix - Linux Solutions for Science and Industry
Braunschweiger Str. 79, 31134 Hildesheim, Germany
Handelsregister: Amtsgericht Hildesheim, HRA 2686
Phone: +49-5121-28619-0 | Fax: +49-5121-28619-4PATCH 1/1 1/5: cmd_boot
Quoted 5 lines · In message <[email protected]> you wrote:
Wolfgang, the code has been in that file for ages. The only thing I did is to move it to the right location, which is _before_ the
printf()inload_serial_bin(), not _after_ the function returned. Please explain why moving code changes the binary size.
You patch does not delete code, but it adds code, increasing the size:
common/cmd_boot.c +17 -3
@@ -163,10 +163,10 @@printf ("%c%02X" , i ?':' :' ' , bd->bi_enetaddr[i]); }printf ("\n" -"ip_addr = " ); +"ip_addr = " );print_IPaddr (bd->bi_ip_addr);printf ("\n" -"baudrate = %d bps\n" , bd->bi_baudrate); +"baudrate = %d bps\n" , bd->bi_baudrate);return 0 ; }
@@ -575,6 +575,7 @@#define XON_CHAR 17#define XOFF_CHAR 19#define START_CHAR 0x01 +#define ETX_CHAR 0x03#define END_CHAR 0x0D#define SPACE 0x20#define K_ESCAPE 0x23
@@ -687,11 +688,24 @@static ulong load_serial_bin (ulong offset) { -int size; +int size, i;char buf[32 ];set_kerm_bin_mode ((ulong *) offset); size =k_recv (); + +/* +* Gather any trailing characters (for instance, the ^D which +* is sent by 'cu' after sending a file), and give the +* box some time (100 * 1 ms) +*/ +for (i=0 ; i<100 ; ++i) { +if (serial_tstc ()) { + (void )serial_getc (); + } +udelay (1000 ); + } +flush_cache (offset, size);printf ("## Total Size = 0x%08x = %d Bytes\n" , size, size);
...
Let's wait a day if anybody speaks up who has the same problem like you. if not, I will ignore this.
Best regards,
Wolfgang Denk
Signature · 5 lines
Software Engineering: Embedded and Realtime Systems, Embedded Linux Phone: (+49)-8142-4596-87 Fax: (+49)-8142-4596-88 Email: [email protected] If it happens once, it's a bug. If it happens twice, it's a feature. If it happens more than twice, it's a design philosophy.
PATCH 1/1 1/5: cmd_boot
Quoted 2 lines · Wolfgang Denk
You patch does not delete code, but it adds code, increasing the size:
We are talking about this piece of code, right?
common/cmd_boot.c line 708
Quoted 13 lines
common/cmd_boot.c +13
-0+ +/* +* Gather any trailing characters (for instance, the ^D which +* is sent by 'cu' after sending a file), and give the +* box some time (100 * 1 ms) +*/ +for (i=0 ; i<100 ; ++i) { +if (serial_tstc ()) { + (void )serial_getc (); + } +udelay (1000 ); + } +
Exactly this code sniplet was located directly after the call to load_serial_bin() in cmd_boot.c:do_load_serial_bin() before I started looking for the problems. When I sent my first patch which _removed_ this code I somehow forgot to send you the hunk which adds it at the right place. So this patch does not add new code, it only re-adds the hunk which was removed instead of re-located to the right place.
Before:
load_serial_bin();
download via kermit
say "I'm finished"
throw away garbage characters
go on
Now:
load_serial_bin();
download via kermit
throw away garbage characters
say "I'm finished"
go on.
I hope you understand what I mean...
Robert
Signature · 5 lines
Dipl.-Ing. Robert Schwebel | http://www.pengutronix.de
Pengutronix - Linux Solutions for Science and Industry
Braunschweiger Str. 79, 31134 Hildesheim, Germany
Handelsregister: Amtsgericht Hildesheim, HRA 2686
Phone: +49-5121-28619-0 | Fax: +49-5121-28619-4PATCH 1/1 1/5: cmd_boot
Quoted 7 lines · In message <[email protected]> you wrote:
Exactly this code sniplet was located directly after the call to
load_serial_bin()in cmd_boot.c:do_load_serial_bin() before I started looking for the problems. When I sent my first patch which _removed_ this code I somehow forgot to send you the hunk which adds it at the right place. So this patch does not add new code, it only re-adds the hunk which was removed instead of re-located to the right place.
Aaaaarggghhh.... why didn't you clearly say so?
Added.
And it would be really nice if you didn't split two parts of the same patch over a distance of 12+ days :-(
Wolfgang Denk
Signature · 4 lines
Software Engineering: Embedded and Realtime Systems, Embedded Linux Phone: (+49)-8142-4596-87 Fax: (+49)-8142-4596-88 Email: wd at denx.de Actual war is a very messy business. Very, very messy business. -- Kirk, "A Taste of Armageddon", stardate 3193.0
PATCH 1/1 1/5: cmd_boot
Quoted 1 line · Wolfgang Denk
Aaaaarggghhh.... why didn't you clearly say so?
I tried my best...
Quoted 2 lines
And it would be really nice if you didn't split two parts of the same patch over a distance of 12+ days :-(
Sorry, it was just a mistake...
Robert
Signature · 5 lines
Dipl.-Ing. Robert Schwebel | http://www.pengutronix.de
Pengutronix - Linux Solutions for Science and Industry
Braunschweiger Str. 79, 31134 Hildesheim, Germany
Handelsregister: Amtsgericht Hildesheim, HRA 2686
Phone: +49-5121-28619-0 | Fax: +49-5121-28619-4