Add flash programming counter]

Add flash programming counter. It prints how many KB has been programmed in case you suspect it hangs when programming large files.

---
 drivers/mtd/cfi_flash.c |   12 ++++++++++++
 1 files changed, 12 insertions(+), 0 deletions(-)

index 439c950..c55cf1d 100644
@@ -1188,6 +1188,7 @@ void flash_print_info (flash_info_t * info)
 int write_buff (flash_info_t * info, uchar * src, ulong addr, ulong cnt)
 {
 	ulong wp;
+	ulong count, temp;
 	uchar *p;
 	int aln;
 	cfiword_t cword;

@@ -1196,6 +1197,7 @@ int write_buff (flash_info_t * info, uchar * src, ulong addr, ulong cnt)
 #ifdef CFG_FLASH_USE_BUFFER_WRITE
 	int buffered_size;
 #endif
+	count = cnt;
 	/* get lower aligned address */
 	wp = (addr & ~(info->portwidth - 1));
 

@@ -1222,6 +1224,7 @@ int write_buff (flash_info_t * info, uchar * src, ulong addr, ulong cnt)
 	}
 
 	/* handle the aligned part */
+	temp = 0;
 #ifdef CFG_FLASH_USE_BUFFER_WRITE
 	buffered_size = (info->portwidth / info->chipwidth);
 	buffered_size *= info->buffer_size;

@@ -1248,6 +1251,10 @@ int write_buff (flash_info_t * info, uchar * src, ulong addr, ulong cnt)
 		wp += i;
 		src += i;
 		cnt -= i;
+		if (((count-cnt)>>10) > temp) {
+			temp = (count-cnt)>>10;
+			printf("\r\t\t%d KB",temp);
+		}
 	}
 #else
 	while (cnt >= info->portwidth) {

@@ -1259,8 +1266,13 @@ int write_buff (flash_info_t * info, uchar * src, ulong addr, ulong cnt)
 			return rc;
 		wp += info->portwidth;
 		cnt -= info->portwidth;
+		if (((count-cnt)>>10) > temp) {
+			temp = (count-cnt)>>10;
+			printf("\r\t\t%d KB",temp);
+		}
 	}
 #endif /* CFG_FLASH_USE_BUFFER_WRITE */
+	printf("\n");
 	if (cnt == 0) {
 		return (0);
 	}

I do like such a progress output. But printing each KB seems a little too much from my point of view. Could even slow down the programming because of the slow serial output on some systems.

How about this version:

index 439c950..5721c6b 100644
@@ -1188,6 +1188,7 @@ void flash_print_info (flash_info_t * info)
 int write_buff (flash_info_t * info, uchar * src, ulong addr, ulong cnt)
 {
 	ulong wp;
+	ulong count;
 	uchar *p;
 	int aln;
 	cfiword_t cword;

@@ -1196,6 +1197,7 @@ int write_buff (flash_info_t * info, uchar * src, ulong addr, ulong cnt)
 #ifdef CFG_FLASH_USE_BUFFER_WRITE
 	int buffered_size;
 #endif
+	count = cnt;
 	/* get lower aligned address */
 	wp = (addr & ~(info->portwidth - 1));
 

@@ -1248,6 +1250,8 @@ int write_buff (flash_info_t * info, uchar * src, ulong addr, ulong cnt)
 		wp += i;
 		src += i;
 		cnt -= i;
+		if (!((count - cnt) % (1 << 14)))
+			printf("\r\t\t %d KB ", (count - cnt) >> 10);
 	}
 #else
 	while (cnt >= info->portwidth) {

@@ -1259,6 +1263,8 @@ int write_buff (flash_info_t * info, uchar * src, ulong addr, ulong cnt)
 			return rc;
 		wp += info->portwidth;
 		cnt -= info->portwidth;
+		if (!((count - cnt) % (1 << 14)))
+			printf("\r\t\t %d KB ", (count - cnt) >> 10);
 	}
 #endif /* CFG_FLASH_USE_BUFFER_WRITE */
 	if (cnt == 0) {

What's do other think? Should we add such an "enhancement" to the common CFI driver?

Thanks.

Best regards,
Stefan

=====================================================================
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office at denx.de
=====================================================================

u-boot-users-bounces at lists.sourceforge.net wrote on :

[...]

I would appreciate such a progress counter - if it does not slow down flash programming too much.

At a baud rate of 9600 bit/s one "xxxx KB " Message needs about 12,5 ms on the serial line. With a suggested granularity of 2^14 bit (= 16 KiB), programming time for 1 MiB increases about 0,8 s in the worst case. The worst case is, if the UART does not use any FIFO. With a FIFO the added time will only be about 1/10th, thus 0,08 s/MiB. For higher baud rates the additionally needed time will be even lower. So for me it would be OK, to add this featur to the common CFI driver.

Regards,
Martin

I agree that a counter is overkill.

I suggest we continue to do what we've been doing in other flash access routines (like many of the earlier custom drivers), i. e. print out dots, approximately one per second.

This still shows that somethingis happening on the board, but it avoids all the overhead of printing tons of characters and control sequences on the serial port.

Best regards,

Wolfgang Denk

DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Es ist offensichtlich, dass das menschliche Gehirn wie  ein  Computer
funktioniert.  Da  es  keine  dummen Computer gibt, gibt es also auch
keine dummen Menschen. Nur ein paar Leute, die unter DOS laufen.
                                                       -- <unbekannt>

You misunderstood me. I like this kind of output (kB or % programmed). It's just that the York's version printed one line per kByte. This seems too much. My version is simpler and prints only 1 line per 16 kByte. We can change to 32k or even more.

Fine with me too. Even though I would prefer to switch this progress output on all FLASH access routines to something like % too. This way we have an idea how long this operation it is going to last.

Best regards,
Stefan

=====================================================================
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office at denx.de
=====================================================================

Please let's stay terse. Printing a dot is a single character on the console. I dislike funny stuff which requires output of non-printing characters or (weven worse!) terminal specific escape sequences.

Best regards,

Wolfgang Denk

DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"I used to think that the brain was the most wonderful  organ  in  my
body. Then I realized who was telling me this."        - Emo Phillips

Backspace or CR without LF should work on all terminals, no?

No matter how it is implemented, I am strongly in favor of *some* kind of progress output.

If it is possible to estimate how long the operation will take, this would be a big plus IMHO (which precludes the simple dots).

cu
Michael

Hi Michael, Stefan, Wolfgang,

I understand where you are coming from and like countdowns a lot when driving the system from a terminal.

The dark side of countdowns with \r characters is if you capture it in a log file. It isn't impossibly bad, but you end up with a lot of crap in your log file.

The dark side of dots, as you point out, is that you don't know how many dots are suppose to print, at least the first couple of times you do it.

Here is a thought, what about printing a bar and then print the dots. How sophisticated is our printf() formatting capabilities? Hmmm. How about something like this (I think the?

#include <stdio.h>
#include <unistd.h>

int main(int argc, char *argv[])

{
         int k;
         int cnt;

         if(sscanf(argv[1], "%d", &cnt) != 1) {
                 fprintf(stderr, "sscanf() failed\n");
                 return 0;
         } else {

                 printf("%*c\r", (cnt + (1 << 14) - 1) / (1 << 14), '|');
                 fflush(stdout);

                 for(k = 0; k < cnt; k++) {
                         if ((k % (1 << 14)) == 0) {
                                 sleep(1);
                                 putchar('.');
                                 fflush(stdout);
                         }
                 }
                 printf("\n");
         }
         return 0;
}

Example use:

After three dots are printed:
./dots 491520
... |

At the end:

..............................

Note that, if our printf() doesn't understand the '*' modifier, it could still be done with a static string that is at least as long as the expected length. That would be much less desirable, however, because it is hard to guess what the maximum length string should be.

The above also is a little iffy with line wrapping if you have more than 79 dots.

Using '\r' rather than '\n' on the progress bar is on the iffy side. It looks cool on a video terminal, but '\n' is a safer choice. It also has less problems with line wrapping (ugly, but still understandable). Substituting

                 printf("%*c\n", (cnt + (1 << 14) - 1) / (1 << 14), '|');

looks like:

                              |

..............................

Best regards,
gvb

ACK from my side to Jerry's version. Maybe a quite long fixed length (~40 characters) bar would also be reasonable and the dot-time scaled to fit the progress.

A progress bar needs IMO two informations:
- that it's still working... so a quite frequent output of something to keep me calm.
- how long it will take... so I know how much time I will have to get the next cup of coffee to keep me tickin'.

Perfect (= close to overkill, I know) would be IMO an additional output like:

Programming Flash from 0xc0ldbeef to 0xc0ldcafe takes 112s.
................. |

So, I don't need to estimate from the first dots how long it will take to complete.

Regards,

Clemens

I don't think printing the programming time/speed at beginning of the operation is doable. At least not without bigger changes. But an output at end of the operation like:

Wrote 512kB in 5.4 seconds (94,8 kB/s)

would be nice. This way the developer could see, if the interface to the FLASH chips is optimized. But I think this is overkill too. Let's concentrate on a clean progress bar with a fixed length.

Patches welcome. :)

Best regards,
Stefan

=====================================================================
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office at denx.de
=====================================================================

All but the timing part ;-)
<http://article.gmane.org/gmane.comp.boot-loaders.u-boot/37626>

Best regards,
gvb

Yes, thanks. I did already write a reply to this. Crashes on my system while programming 16Mbytes.

Best regards,
Stefan

=====================================================================
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office at denx.de
=====================================================================

Odd, I didn't see your reply, saying "cr at sh" must trigger our mail filter, it's built on Microsoft technology. ;-) I can read your reply in gmane.org, now that I know to look. :-/

The crash is undoubtedly due to a buffer overflow on the format string (16MB => . Adopting Clemen's proposal for a fixed length bar (see previous email for a 50 dot bar example) is trivial.

The saveenv also looks funky. I only mucked with the cmd_mem.c command to make it display better with the progress dots, obviously the saveenv command needs to have the same changes
s/"Writing to Flash... "/"Writing to Flash\n"/
Apparently saveenv does four very short program operations.

If you want to extend my patch, here's your chance to grab all the glory. ;-)

Best regards,
gvb

Understood. :)

Right.

I already have enough "glory" in U-Boot history. ;) I leave this for somebody else for now. Let's see, if somebody steps up...

Best regards,
Stefan

=====================================================================
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office at denx.de
=====================================================================

Indeed: since v1.3.1:

Developers with the most changesets
Stefan Roese 109 (13.2%)

Kumar Gala                  61 (7.4%)
Wolfgang Denk               45 (5.4%)
Matthias Fuchs              35 (4.2%)

Jean-Christophe PLAGNIOL-VILLARD 34 (4.1%)
...

Best regards,

Wolfgang Denk

DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
First study the enemy.  Seek weakness.
	-- Romulan Commander, "Balance of Terror", stardate 1709.2

and I wasn't even trying for 'glory' :)

- k

Yippiieee!

I have to open a bottle of nice red wine this evening to celebrate this "victory". ;)

Best regards,
Stefan

=====================================================================
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office at denx.de
=====================================================================

Please don't.

That shall not become a 200 lines output. Please keep this on a single line.

It makes no sense to have all useful information srolling out of sight all too quickly just for such bells and whistles.

Best regards,

Wolfgang Denk

DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
To understand a program you must become  both  the  machine  and  the
program.

I was too terse. The problem is that I added a progress bar. Previously, the printout sequence was:
Writing to Flash...
followed by an indeterminate wait, followed by the string "done." which formed the composite:
Writing to Flash... done.
I added a progress bar, but the progress bar needs to be on a new line.
I fixed this in cmd_mem.c, but not in the env code, resulting in
Stefan's complaint (b).

With the (next version of the) progress dots patch, the sequence will be:
Writing to Flash
v
..................................................
done.

One thing I am debating is to add a parameter to the flash write command that says whether to do the progress dots or not. Then we could suppress the (silly) dots on short writes.

Best regards,
gvb

That's what it shall remain: a single line of text.

Please don't.

Best regards,

Wolfgang Denk

DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
War isn't a good life, but it's life.
	-- Kirk, "A Private Little War", stardate 4211.8

There is no way to add an end marker without using a new line (or use '\r' which is really unacceptable, and more justifiably so). Since we are settling in on having 50 (?probably want to use a #define) dots unless there are fewer than 50 units to be written, I suppose we can live without a end marker. Sigh, the end marker was the fun part.

I also contend that it was a useful part even if you /know/ that there will be 50 dots because the human eye cannot count dots at a glance.

Illustration:

Tell me at a glance how close this is to completion[1]:
Writing to Flash..............................

Now tell me here:
Writing to Flash
v
..............................

With an added parameter to the flash write routine (which would serve double duty to suppress the progress dots), I can condense the above into two lines. This is still 100% more lines than Wolfgang demands, but I think it has sufficient value to pay its freight.

Writing to Flash v
..............................

Best regards,
gvb

[1] Answer: 60% done, 40% remaining for all the examples.

Yes, I understand this.

But there is also a few other things to consider: keeping the output terse and efficient; keeping the same look and feel acrross different boards and platforms, etc. If you add such code to the CFI driver, users might request that such code also gets added to all those flash drivers where the CFI driver cannot be used, etc.

Remember the KISS principle.

You're lucky. To me it seems always to end up with hitting the 90-90 rule.

Best regards,

Wolfgang Denk

DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Where there's no emotion, there's no motive for violence.
	-- Spock, "Dagger of the Mind", stardate 2715.1

Wolfgang Denk wrote:

[snip]

OK, here is a simulation of what I will create tonight. I think this is both workable, very effective, and meets Wolfgang's constraints.

Notes:
* The mailer linewraps the " Done", in real life it all fits on an 80 column line (76 columns to be exact).

* The "Writing to Flash... " and "Done" strings are not under the CFI flash write routine's domain. With the New Improved[tm] countup, it means that I don't have to modify cmd_mem.c and friends. That is a Good Thing[tm].

* I'm thinking I should simply suppress the countup if there are fewer than say 5 units of programming to do?

* ACK on Jon's suggestion for a control #define, but I counterpropose CONFIG_FLASH_SHOW_PROGRESS ("FLASH" instead of "CFI") so that the custom flash people can use it too if they get ambitious.

Writing to Flash... 0....1....2....3....4....5....6....7....8....9.... Done

Writing to Flash... 0 Done

Writing to Flash... 0....1....2....3.... Done

Writing to Flash... 0....1....2....3....4....5....6....7....8....9.... Done

:-P :-D

Best regards,
gvb

I like it.

This is kind of misleading. I hear customers (or participants of our training classes) asking if "0 done" means that nothing was done.

Can we just omit the 0?

Best regards,

Wolfgang Denk

DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Quantum particles: The dreams that stuff is made of.

To quote a quote I got from you:

Zero is an enigmatic value. It can mean success (fclose) or failure (scanf). It can mean black or white. It can mean no permissions (chmod) or all permissions (umask). It can mean now (setjmp) or later (atexit). It can mean the beginning (lseek) or the end (read). It can mean myself (getpgrp) or child (fork). It can mean all (kill's 1st argument) or nothing (kill's 2nd argument). It can mean 'default' (SIG_IGN) or 'I don't care' (waitpid) or 'try to guess' (strtol). Indeed 0 lets you talk to God (setuid). Verily is 0 all things to all people.

-- an ancient usenet posting

My current thought is
* Start counting at 1
* Suppress the dots if there are fewer than the configured max (rationale: if there are fewer units to program than dots, it will be done very quickly anyway)
* Have a configurable upper limit, default to 45 (1..9). This will allow changing the #define to make the dots friendlier to 40 column displays, for instance.

Writing to Flash... Done

Writing to Flash... Done

Writing to Flash... ....1....2....3....4....5....6....7....8....9 Done

Writing to Flash... ....1....2....3....4....5....6....7....8....9 Done

gvb

I thought it safe to assume a terminal is 80 characters wide..(80 - strlen("Writing to Flash") - strlen("done")) == 60?

Kim

No, this is not a safe assumption. We have a couple of boards supported with QVGA displays. These usually show 12 x 40 characters only.

Just one example...

Best regards,

Wolfgang Denk

DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"I haven't lost my mind - it's backed up on tape somewhere."

U-Boot output shall be terse. This may be acceptable for debug mode, but not in the production version.

Best regards,

Wolfgang Denk

DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
It's certainly  convenient  the  way  the  crime  (or  condition)  of
stupidity   carries   with   it  its  own  punishment,  automatically
admisistered without remorse, pity, or prejudice. :-)
         -- Tom Christiansen in <[email protected]>

Scaling the dots is trivial. The disadvantage of scaling the dots is that they will come out at a different rate depending on the size of what you are programming. This is OK for small things, but devolves back into a "how long to I wait for the next dot" problem for large copies.

Hardcoding a prediction of time to program is difficult and a horrible maintenance burden. We could do one dot's worth of programming measuring elapsed time *before* printing the progress end marker, and then use that to print the estimated time of completion. Cute, but seems like overkill to me.

Here is a revised command line example that autoscales to 50 dots:

#include <stdio.h>
#include <unistd.h>

int main(int argc, char *argv[])

{
         int k;
         int cnt;
         int scale;

         if(sscanf(argv[1], "%d", &cnt) != 1) {
                 fprintf(stderr, "sscanf() failed\n");
                 return 0;
         } else {

                 scale = (cnt >= 50) ? cnt / 50 : 1;

#ifdef ONELINEBAR
                 printf("%*c\r", (cnt + scale - 1) / scale, '|');
#else
                 printf("%*c\n", (cnt + scale - 1) / scale, 'v');
#endif
                 fflush(stdout);

                 for(k = 0; k < cnt; k++) {
                         if ((k % scale) == 0) {
                                 usleep(100000);
                                 putchar('.');
                                 fflush(stdout);
                         }
                 }
                 printf("\n");
         }
         return 0;
}

Best regards,
gvb

[snip]

Oops, the above line should be:

                 scale = (cnt >= 50) ? (cnt +49) / 50 : 1;

Forgot to test the (50*n)-1 corner case for n > 0. :-P

gvb

I don't see here any problems either.

Sure. We all agree on this.

Yes, this would be a great plus of the new idea with printing "kB" or "%". We would have an idea on how long this operation will take to complete. From my point of view this is no "funny stuff" but really helpful information which is currently missing. And the implementation effort as you have seen is quite low, probably even lower than printing dots each second while programming.

Best regards,
Stefan

=====================================================================
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office at denx.de
=====================================================================

It messes up log files and is a PITA to filter out in regression tests.

If you really insist, then pre-calculate the maximum length and scale the output accordingly.

Please do not output control characters or escape sequences.

Best regards,

Wolfgang Denk

DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
When in doubt, mumble;   when in trouble, delegate;  when in  charge,
ponder.                                             -- James H. Boren

I see. I didn't think of this. Even though a nice user interface has higher priority to me. Just my 2 cents.

I insist on nothing. I'm just pointing out, that the current implementation can be enhanced.

Best regards,
Stefan

=====================================================================
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office at denx.de
=====================================================================

How about adding a CONFIG_CFI_SHOW_PROGRESS option that enable/disables this feature as well?

jdl