EEPROM write problem on avr-libc-1.7.0

Go To Last Post
13 posts / 0 new
Author
Message
#1
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Hello,

I was using GCC-4.3.3 with avr-libc-1.6.7 and I upgraded to GCC-4.3.4 with avr-libc-1.7.0. Now my WriteSingleByteEEPROM function doesn't work anymore. I use avr-gdb to look at the EEPROM before and after the EEPROM writing routine. It does work however using the avr/eeprom.h routines. Details below.

I had my own C function to write a byte in EEPROM, as specified in the datasheet of my uC (at90usb1287):

void WriteSingleByteEEPROM(uint16_t addr, uint8_t data)
{
        // wait completion of previous write
        while(EECR & _BV(EEPE));

        // set up address and data
        EEAR = addr;
        EEDR = data;

        // make the write
        EECR |= _BV(EEMPE);
        EECR |= _BV(EEPE);
}

Compiling this code I can see this on the list:

void WriteSingleByteEEPROM(uint16_t addr, uint8_t data)
{
    5ac4:       9c 01           movw    r18, r24
        // wait completion of previous write
        while(EECR & _BV(EEPE));
    5ac6:       ef e3           ldi     r30, 0x3F       ; 63
    5ac8:       f0 e0           ldi     r31, 0x00       ; 0
    5aca:       80 81           ld      r24, Z
    5acc:       81 fd           sbrc    r24, 1
    5ace:       fd cf           rjmp    .-6             ; 0x5aca 

        // set up address and data
        EEAR = addr;
    5ad0:       32 bd           out     0x22, r19       ; 34
    5ad2:       21 bd           out     0x21, r18       ; 33
        EEDR = data;
    5ad4:       60 bd           out     0x20, r22       ; 32

        // make the write
        EECR |= _BV(EEMPE);
    5ad6:       ef e3           ldi     r30, 0x3F       ; 63
    5ad8:       f0 e0           ldi     r31, 0x00       ; 0
    5ada:       80 81           ld      r24, Z
    5adc:       84 60           ori     r24, 0x04       ; 4
    5ade:       80 83           st      Z, r24
        EECR |= _BV(EEPE);
    5ae0:       80 81           ld      r24, Z
    5ae2:       82 60           ori     r24, 0x02       ; 2
    5ae4:       80 83           st      Z, r24
}
    5ae6:       08 95           ret

which seems correct, with the mention that EECR is used at address 0x3F (i.e. in data space) because of the use of st and ld instructions; if the in/out instructions are used (see below) then EECR is at address 0x1F.

Looking at the list for the program using the avr/eeprom.h routines I can see this:

00009186 <__eewr_r18_usb1287>:
    9186:       f9 99           sbic    0x1f, 1 ; 31
    9188:       fe cf           rjmp    .-4             ; 0x9186 <__eewr_r18_usb1287>
    918a:       1f ba           out     0x1f, r1        ; 31
    918c:       92 bd           out     0x22, r25       ; 34
    918e:       81 bd           out     0x21, r24       ; 33
    9190:       20 bd           out     0x20, r18       ; 32
    9192:       0f b6           in      r0, 0x3f        ; 63
    9194:       f8 94           cli
    9196:       fa 9a           sbi     0x1f, 2 ; 31
    9198:       f9 9a           sbi     0x1f, 1 ; 31
    919a:       0f be           out     0x3f, r0        ; 63
    919c:       01 96           adiw    r24, 0x01       ; 1
    919e:       08 95           ret

The code seems correct with the exception that
this function is clearing EECR at the beginning (this was decribed also in https://www.avrfreaks.net/index.php?name=PNphpBB2&file=viewtopic&t=98095 but still not sure of the explanation)

So my problem is that I don't understand why the code in avr/eeprom.h is working and mine not. Even more, why my code was working before using avr-libc-1.7.0 and now it is not anymore.

Any help is appreciated. The quick solution of course might be to "just use the avr/eeprom routines". Well yes, but I want to understand the issue and be able to use my own stuff.

Thanks.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Silly question, but if the code in avr/eeprom.h works why bother reinventing the wheel?

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:

in the datasheet of my uC (at90usb1287):

Quote:
The following procedure should be followed when writing
the EEPROM (the order of steps 3 and 4 is not essential):
1. Wait until EEPE becomes zero.
2. Wait until SELFPRGEN in SPMCSR becomes zero.
3. Write new EEPROM address to EEAR (optional).
4. Write new EEPROM data to EEDR (optional).
5. Write a logical one to the EEMPE bit while writing a zero to EEPE in EECR.
6. Within four clock cycles after setting EEMPE, write a logical one to EEPE.

See item 6, and then look at your code. Within 4 cycles?

I'd look at your optimization level. And, what Cliff said--there is a reason why experienced people have developed the helper snippets, especially for the "within n cycles" situations.

You can put lipstick on a pig, but it is still a pig.

I've never met a pig I didn't like, as long as you have some salt and pepper.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

If I take your code and build it in WinAVR20100110 with -Os (or -O3 or -O2) I get:

void WriteSingleByteEEPROM(uint16_t addr, uint8_t data) 
{ 
        // wait completion of previous write 
        while(EECR & _BV(EEPE)); 
  e8:	f9 99       	sbic	0x1f, 1	; 31
  ea:	fe cf       	rjmp	.-4      	; 0xe8 

        // set up address and data 
        EEAR = addr; 
  ec:	92 bd       	out	0x22, r25	; 34
  ee:	81 bd       	out	0x21, r24	; 33
        EEDR = data; 
  f0:	60 bd       	out	0x20, r22	; 32

        // make the write 
        EECR |= _BV(EEMPE); 
  f2:	fa 9a       	sbi	0x1f, 2	; 31
        EECR |= _BV(EEPE); 
  f4:	f9 9a       	sbi	0x1f, 1	; 31
}
  f6:	08 95       	ret

Built with -O1 I get:

void WriteSingleByteEEPROM(uint16_t addr, uint8_t data) 
{ 
  e8:	9c 01       	movw	r18, r24
        // wait completion of previous write 
        while(EECR & _BV(EEPE)); 
  ea:	ef e3       	ldi	r30, 0x3F	; 63
  ec:	f0 e0       	ldi	r31, 0x00	; 0
  ee:	80 81       	ld	r24, Z
  f0:	81 fd       	sbrc	r24, 1
  f2:	fd cf       	rjmp	.-6      	; 0xee 

        // set up address and data 
        EEAR = addr; 
  f4:	32 bd       	out	0x22, r19	; 34
  f6:	21 bd       	out	0x21, r18	; 33
        EEDR = data; 
  f8:	60 bd       	out	0x20, r22	; 32

        // make the write 
        EECR |= _BV(EEMPE); 
  fa:	ef e3       	ldi	r30, 0x3F	; 63
  fc:	f0 e0       	ldi	r31, 0x00	; 0
  fe:	80 81       	ld	r24, Z
 100:	84 60       	ori	r24, 0x04	; 4
 102:	80 83       	st	Z, r24
        EECR |= _BV(EEPE); 
 104:	80 81       	ld	r24, Z
 106:	82 60       	ori	r24, 0x02	; 2
 108:	80 83       	st	Z, r24
}
 10a:	08 95       	ret

As -O0 is far, far worse my guess is that you are using -O1 rather than -Os/2/3. With time critical code like this you cannot mess with half-arsed optimization levels.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I was not quite reinventing the wheel. I already had my wheel which was better than the existing one. Then the new wheel came with improvements. Anyhow, I want to understand the issue not just going with the wheel that might at some point break.

Looking at the code, it seems that my code takes 3 cycles (2 cycles for a ld instruction plus 1 cycle for the ori instruction) between etting the EEMPE and EEPE so I don't see how I break the "less than 4 cycles" rule.

Am I missing something?

Thanks again.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Clawson,

Indeed I am using O1 instead of Os or O2. I might change this and maybe fix the issue. However am I correct in that the code only takes 3 cycles (between the two st instructions) and thus should not be an issue?

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:

Am I missing something?

Did you miss the post I just made? I believe you are using -O1 and it's simply not good enough for this time critical code.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:

I already had my wheel which was better than the existing one.

OK, I'll bite: List the ways that it is better.

We have already seen at least two ways it is worse: The 4-cycle limit isn't honored. And even if it were, it is not protected from interrupts so that could cause erratic operation.

Quote:

However am I correct in that the code only takes 3 cycles (between the two st instructions) and thus should not be an issue?

I saw that as well. I think you have to count at least one of the ST instructions. Or half of both. That brings it to 5 and is likely causing the symptoms.

For completeness, it might be interesting to see the instruction sequence when it "used to work", and know that optimization level. In any case, your routine is not rock-solid.

Lee

You can put lipstick on a pig, but it is still a pig.

I've never met a pig I didn't like, as long as you have some salt and pepper.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Lee,

Before giving more details, I should mention that indeed I changed the optimization level between the builds and this is probably the cause of the problem rather than the new library. I also thought of the possibility that the second st instruction might not actually take place until its end and thus have 5 cycles there but wasn't sure.

What I was referring to when I said my wheel was better (compared to version 1.6.7):
- I was using a code as in the datasheet and known
- I was controlling interrupts (also I do stop interrupts when I call that method, regarding your comment on this ... I also have a function for multiple bytes)
- I am not sure if there were update methods in the old version
- i needed control when erasing the full EEPROM and do some more custom writing so I wanted to have my own routines. For example the methods available in the previous versions didn't allow me to erase the EEPROM in a good way (i.e. Withouth wearing the whole memory). Probably now is better as I've seen in the change log.

Thanks for the help.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:

indeed I changed the optimization level between the builds and this is probably the cause of the problem rather than the new library.

Kind of obvious.
Quote:

- I was using a code as in the datasheet and known

Which is an example code at best, and not necessarily "best practice". In fact, some examples are downright wrong and/or n/a to that model.

Quote:

- I was controlling interrupts (also I do stop interrupts when I call that method, regarding your comment on this ... I also have a function for multiple bytes)

So now you take your "proven" driver routine and drop it into a generic full app. And you get erratic EEPROM write behaviour, because every once in a while an interrupt hits during the timed sequence. What do you do then?

Quote:

- I am not sure if there were update methods in the old version

How is that related to anything in this thread? I see no "update" in the posted code that gave you problems, and that I am commenting on.

Quote:

For example the methods available in the previous versions didn't allow me to erase the EEPROM in a good way (i.e. Withouth wearing the whole memory).

Again, how is this related to the code snippet you presented with problems, and the GCC one? How does wear-leveling enter into this? How does GCC's bullet-proof write routine "wear the whole memory"?

True enough, you could make an update method that checks for the same value and skips the erase/write if it finds it. My toolchain of choice has done that from Day 1. You could also have a pre-erased block, and then do write-only.

But these are extensions--when the write is done, it still must be done in a bullet-proof manner.

You can put lipstick on a pig, but it is still a pig.

I've never met a pig I didn't like, as long as you have some salt and pepper.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Got the point, thanks.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

One more thing theusch,

Can you tell me why is the EECR being cleared completely at the beginning of the write_byte routine? (see my first post). I didn't see any reply to this. Hence, this might be a reason why your own code might be useful, you know what you've written.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Source code for eewr_r18 is here:

http://svn.savannah.nongnu.org/v...

(the #else that isn't for Xmega)