carduinoavrarduino-unoavr-gcc

ATmega328P EEPROM not writing


I'm writing to the ATmega328P EEPROM as follows:

eeprom_write_byte(address, (U8_T) 0);
void eeprom_write_byte(void* address, U8_T data)
{
    cli();
    /* Wait for previous write to complete */
    while (TRUE == EECR.bits.EEPE) {
    }

    /* Set erase-and-write mode */
    EECR.bits.EEPM0 = FALSE;
    EECR.bits.EEPM1 = FALSE;

    /* Set up write */
    EEAR.byte = (U8_T) address;
    EEDR.byte = data;

    /* Enable master write */
    EECR.bits.EEMPE = TRUE;
    /* Enable write */
    EECR.bits.EEPE = TRUE;
    sei();
}

Where EECR, EEAR, and EEDR are linked as follows:

SECTIONS {
  EEAR          = 0x41;
  EEDR          = 0x40;
  EECR          = 0x3F;
  ... 

EEMPE and EEPE must certainly be set within 4 clock cycles of each other, right?

I then read from the EEPROM as follows:

U8_T eeprom_read_byte(void* address)
{
    cli();
    /* Wait for previous write to complete */
    while (TRUE == EECR.bits.EEPE) {
    }

    /* Set up read */
    EEAR.byte = (U8_T) address;
    /* Enable read */
    EECR.bits.EERE = TRUE;

    sei();

    return EEDR.byte;
}

But I get back only 0xFF! The erased value! The datasheet says that EEPM0 and EEPM1 set to 0 should erase-and-write. But no writing appears to take place. Any ideas?

I've tried compiling with -O2 with no result. Another post told me to use -O3, but -O3 breaks my program (linker fails). Anyway I work in aerospace and optimizations are forbidden, so it must work without them.

Registers are defined as follows:

/** EEPROM Control Register */
typedef union {
    struct {
        /** EEPROM Read Enable */
        VBOOL_T EERE        : 1;
        /** EEPROM Write Enable */
        VBOOL_T EEPE        : 1;
        /** EEPROM Master Write Enable */
        VBOOL_T EEMPE       : 1;
        /** EEPROM Ready Interrupt Enable */
        VBOOL_T EERIE       : 1;
        /** EEPROM Mode Bits */
        VBOOL_T EEPM0       : 1;
        VBOOL_T EEPM1       : 1;
        VBOOL_T Reserved6   : 1;
        VBOOL_T Reserved7   : 1;
    } bits;
    VU8_T byte;
} EECR_T;
/** EEPROM Control Register */
extern volatile EECR_T EECR;

Solution

  • I haven't used this part, but the manual 13.6.4 describes the procedure:

    1. Wait until EEPE becomes zero.
    2. Wait until SPMEN in SPMCSR becomes zero.
    3. Write new EEPROM address to EEAR (optional).
    4. Write new EEPROM data to EEDR (optional).
    5. Write a '1' to the EEMPE bit while writing a zero to EEPE in EECR.
    6. Within four clock cycles after setting EEMPE, write a '1' to EEPE.
    1. OK.

    2. You don't do this.

    3. You do this but only to the ls byte EEARL. Meaning if the address is larger than the eeprom offset 256, your code will not work since you don't write to EEARH. Or if there's stray values left behind in EEARH, your program will go bananas.

    4. OK.

    5. Hmm. You are using the not recommended bit-field feature of C. I'm not sure if EECR.bits.EERE = TRUE; will result in a byte instruction or a bit instruction on the assembler level. In case of the latter, EEPE will not get written to zero as the manual says it should be.

      Recommended practice is to never use bit-fields, instead write to the whole register at once so you are sure that EEPE will get set.

    6. Yes it seems reasonable that there is no 4 clock cycle delay before EECR.bits.EEPE = TRUE; is executed. But again this depends on what all the bit-field goo results in. Maybe in the worst case something gets translated into read into register, modify register, write register sequence. In which case far more than 4 clock cycles are executed.

    The key to understanding what's going on is to view the disassembly of the program, to see what's actually generated. AVR asm is easy enough to read even for one like me who have never used it.


    Unrelated to that, you have another severe bug in the driver. A driver should never touch the global interrupt mask cli/sei. Particularly not by destroying its previous value as this code does. If you ever touch these registers you need to stack the CCR and restore it afterwards.

    An EEPROM driver may be allowed to disable all interrupts, but only if it ends the whole write session by resetting the MCU.

    It is common practice to enter EEPROM programming through a safe mode, where all interrupts are disabled before programming is commenced. And once it is done, the MCU is rebooted with waiting for the wdog or by writing an incorrect sequence to its register.

    Re-starting execution where it was previously after an EEPROM update is dangerous practice, since all parts of the program aren't necessarily fetching values from the EEPROM continuously.