[bug #41857] avr109 atmega2564rfr2 erase timeout to short

[avrs-admin]

oni
Thu 13 Mar 2014 03:46:13 PM UTC

An atmega2564rfr2 is not able to erase its program flash in under 5s when running at 8MHz.
That leads into a serial read timeout when using an avr109 like bootloader.

I have two ideas to fix this:

1. set timeout to 10s, will increase the time to wait for all AVRs if a command really fails.
   2.introducing a "wait longer" character like '!' to tell avrdude to wait a bit longer. This adds some kind of "non-standard" thinks to the avr109 protocol of avrdude.
   Which of these ideas would you prefer or are there other/better ideas?

file #30888: timeout.patch
file #31392: serial_timeout.patch

This issue was migrated from https://savannah.nongnu.org/bugs/?41857

[avrs-admin]

Joerg Wunsch <joerg_wunsch>
Thu 13 Mar 2014 03:55:38 PM UTC

3rd idea: add an adjustable timeout option to the config syntax,
so this can be configured on a per-programmer basis.

[avrs-admin]

Georg von Zengen
Thu 13 Mar 2014 09:10:31 PM UTC

setting the timeout for the programmer will also increase the time for fault detection for all AVRs.
The patch I attached implements a fourth idea.
For the butterfly chip erase function it takes the number of flash pages and multiplies it with 10. If this is higher the the default timeout it is applied.
For a chip with 1024 pages like the 2564rfr2 that means ~10s timeout.
I think a multiplier of 10 is a good value because it corresponds with my observations and the 7.3ms - 8.9ms per page erase mentioned in the datasheet.

(file #30888)

[avrs-admin]

Georg von Zengen
Tue 01 Apr 2014 07:47:42 AM UTC

Hi,

any comments about my patch?

[avrs-admin]

Klaus Leidinger
Tue 01 Apr 2014 09:49:30 PM UTC

Hi,
avrdude already accepts programmer specific parameters with the -x 'extendet parameter'
http://www.nongnu.org/avrdude/user-manual/avrdude_4.html#Option-Descriptions
and
http://www.nongnu.org/avrdude/user-manual/avrdude_5.html#Programmers-accepting-extended-parameters

So this should be the place for a "extend_delay' or any similar command.
Just my 2 cents

How do other  programmers handle the longer delay time of this chip?

[avrs-admin]

Joerg Wunsch <joerg_wunsch>
Wed 02 Apr 2014 05:42:24 AM UTC

Other programmers (like JTAGICE, AVRISPmkII) simply have much
longer default timeouts, so the timeout is never hit even by
the somewhat longer erase time of this device.

The serial communications timeout is meant just as a fallback
if nothing responds at all at the other end, so extending it
to some seconds should normally not hurt.

Given that, why not take the simple solution (as already
suggested in the report itself), and bump it to 10 s?

[avrs-admin]

Georg von Zengen
Wed 02 Apr 2014 07:00:23 AM UTC

One of the problems with avr109 is that it has only a chip erase command which only sends a reply when the memory is erased completely. That is why the process bar of the erase command simply jumps to 100% immediately. I think other programmers are responding in between.

Using some kind of extendet parameter to increase the timeout is hacky in my opinion. Avrdude should support all AVRs with all Atmel Programmers (avr109 is a app-note) with the "default" parameters.

My argument against setting the timeout to 10s is that in cases of errors the time out for all commands is twice as long as it is now. Additionally The same problem will appear again for AVRs with 2048 pages. By now there are no such AVRs but using a solution where the next problem is foreseeable is not an option. Especially if there is another solution without this problem which does not have any side effects.

[avrs-admin]

Klaus Leidinger
Wed 02 Apr 2014 07:00:57 AM UTC

My vote for the simple solution to match the timeout to other serial programmers. Thanks Jörg for pointing this out.

[avrs-admin]

Georg von Zengen
Wed 02 Apr 2014 07:47:34 AM UTC

What is your reason against the "complicated" (8 lines) solution which solves the problem and not only delays its appearance.

[avrs-admin]

Yannic Schröder <yan_nic>
Wed 02 Apr 2014 09:00:11 AM UTC

I dislike the option to generally raise the serial timeout to 10 seconds. When I have a new untested chip/board it happens a lot, that my programmer times out. When this timeout increases to 10 seconds the whole avrdude will feel way more unresponsive.

Also as Georg pointed out, when new chips with even more flash arrive then this timeout has to be increased again (20 seconds for 2048 pages).

The proposed solution with a dynamic timeout calculated with the number of flash pages seems best to me as it does not hurt the responsiveness of avrdude on small chips and makes it possible to program devices with arbitrary large flash sizes on avr109 programmers.

In any case this is a workaround for a design flaw in the avr109 specification.

[avrs-admin]

Dennis Reimers
Wed 02 Apr 2014 11:51:58 AM UTC

I vote for the dynamic solution because its more future-proof. In general giving the user feedback in case of unexpected/faulty behavior as soon as possible is the better solution than let the program hang forever. So we should look if other programmers might have similar problems.

[avrs-admin]

Klaus Leidinger
Wed 02 Apr 2014 12:48:42 PM UTC

Georg,
for me the reason was keep it simple. A longer timeout is only valid for not connected/responding programmers and can be stopped with ctrlC

And this is may be a "general" fix for programmers which use serial protocol instead of fixing one by one. I expect more programmers running in this problem (at least avr910 is one)

After having a look to the behavier of stk500 shows, that it does a retry for several times before it quits with an error. (or is stopped with ctrlC ) The interval itself seems to be 5 seconds.
So does stk500 delete the atmega2564rfr2 without timeout? Do you have a stk500 for testing this?

The Method of 10 times wait per # of pages depends on clock rate of the Controller as far as I understand your post. What about 4 or 1 MHz?

So if choosing this fix, it should be deployed to at least avr910 also.

If this problem really hits only avr109 and avr910 why not pimping the erase command with a kind of status bar (running dots) and setting the timeout for this erase command to a higher time for next Chips and slower clockrate? Whould be a nice improvement for the old protocol.

[avrs-admin]

Georg von Zengen
Wed 02 Apr 2014 03:49:02 PM UTC

First of all, 8 lines are simple.
Second, in my opinion sending SIG-TERM to a process should not be the normal way to end it in error cases.
No, I do not have a stk500 but jtagICE II does not time out.

NO, the erase cycle duration does not depend on the clock rate.
As I wrote before it is "7.3ms - 8.9ms per page" (values copied from the datasheet).
More information about flash erase can be found here:
http://en.wikipedia.org/wiki/Flash_memory
The only clock rate dependent operation is to increase the page counter.

"So if choosing this fix, it should be deployed to at least avr910 also." I do not have the any avr910 based system, so I can not test it. If you have such devices: feel free to do so.

"Whould be a nice improvement for the old protocol."
NO, it will cause in compatibility with other software like avr-studio.

[avrs-admin]

Georg von Zengen
Fri 16 May 2014 07:00:36 PM UTC

Hi,

I appended a new version of my patch which applies to revision 1304.

regards
Georg von Zengen

(file #31392)

From 4930a8b4bd92a19851b00dd3106fc94604803293 Mon Sep 17 00:00:00 2001
From: Georg von Zengen 
Date: Fri, 16 May 2014 20:40:11 +0200
Subject: butterfly: implemented a variabel timeout for the erase call in the
 avr109 protocol. Needed to support AVRs with more than 128kB programm memory.

---
 butterfly.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/butterfly.c b/butterfly.c
index 3d4d6d3..68175fa 100644
--- a/butterfly.c
+++ b/butterfly.c
@@ -155,11 +155,19 @@ static int butterfly_vfy_led(PROGRAMMER * pgm, int value)
  */
 static int butterfly_chip_erase(PROGRAMMER * pgm, AVRPART * p)
 {
+  AVRMEM * mem;
+  char mem_type[]="flash";
+  mem = avr_locate_mem(p, (char *)(&mem_type));
+  long old_serial_recv_timeout= serial_recv_timeout;
+  if (serial_recv_timeout < mem->num_pages*10){
+    serial_recv_timeout=mem->num_pages*10;
+  }
   butterfly_send(pgm, "e", 1);
+  int ret=0;
   if (butterfly_vfy_cmd_sent(pgm, "chip erase") < 0)
-      return -1;
-
-  return 0;
+	  ret=-1;
+  serial_recv_timeout=old_serial_recv_timeout;
+  return ret;
 }
 
 
-- 
2.0.0.rc0

[mcuee]

Joerg Wunsch <joerg_wunsch> Thu 13 Mar 2014 03:55:38 PM UTC

3rd idea: add an adjustable timeout option to the config syntax, so this can be configured on a per-programmer basis.

This seems to be in line with the following feature request.

• [bug #41637] avrdude timeout should be specifiable #332

[mcuee]

Help wanted from the community to check whether the issue still exists. PR (which can be new or based on the above patch) is welcome.

[stefanrueger]

An atmega2564rfr2 is not able to erase its program flash in under 5s when running at 8MHz.
That leads into a serial read timeout when using an avr109 like boot

This has been solved for -c usbtiny by the following idea: when the programmer knows it talks to a bootloader then estimate the time it will take the bootloader to erase the chip by the number of pages times chip_erase_delay; this means no change in serial_recv_timeout is needed:

avrdude/src/usbtiny.c

Lines 606 to 611 in f0da9a6

	if(pgm->prog_modes & PM_SPM) { // Talking to bootloader directly
	AVRMEM *fl = avr_locate_mem(p, "flash");
	// Estimated time it takes to erase all pages in bootloader
	usleep(p->chip_erase_delay * (fl? fl->num_pages: 999));
	} else
	usleep(p->chip_erase_delay);

This technique could be promoted from usbtiny.c's chip_erase() to the general avr_chip_erase(), so it is deployed for any and all programmers iff they serve a bootloader.

Thoughts?

[mcuee]

This technique could be promoted from usbtiny.c's chip_erase() to the general avr_chip_erase(), so it is deployed for any and all programmers iff they serve a bootloader.

Thoughts?

I think this is a good idea.

[mcuee]

For now, other than those programmers specific to bootloaders, as far as I know, only the following prorgammer serves both bootloader and real programmer.

1. -c arduino, -c stk500v1 and -c urclcok
2. -c stk500v2 and -c wiring
3. -c usbtiny
4. -c usbasp (bootloader is not popular)

[stefanrueger]

progammer serves both bootloader and real programmer

PM_SPM should always be the single programming mode mentioned for a programmer that serves a bootloader. The same programmer should not be used for both bootloaders and real programmers. For example, we parented off arduino_gemma from usbtiny to create a different programmer for bootloader use. We did this, because when you do a chip erase with a real programmer it takes ca 10 ms, when a bootloader erases page after page it will be n times 10 ms, where n is the number of pages.

[stefanrueger]

Turns out the increased delay needs to be done in the corresponding programmer types as was done for usbtiny derived programmers. So @mcuee's list above is very useful.

[stefanrueger]

I have had a look and followed the ideas of the original patch to address this issue, as timeout handling is different for different programmers.