Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

EEPROM library not working #26

Closed
obdevel opened this issue Oct 9, 2020 · 18 comments
Closed

EEPROM library not working #26

obdevel opened this issue Oct 9, 2020 · 18 comments

Comments

@obdevel
Copy link

obdevel commented Oct 9, 2020

Arduino 1.8.13, macOS 10.13.6

AVR128DA28 DIP package w/Optiboot

EEPROM writes not sticking, e.g.

EEPROM[34] = 10;
Serial.println(EEPROM[34]); // prints 255

I haven't got jtag2updi working so I set the fuses and programmed the bootloader using pyupdi. I based the fuse settings on the avrdude command line displayed by Tools->Burn Bootloader

Everything else seems to be working so far.

Edited to add -------

avr-libc routines don't work either, e.g.

#include <avr/eeprom.h>

  if (eeprom_is_ready()) {
    eeprom_write_byte((uint8_t *)1, 22);
    eeprom_busy_wait();
    Serial.println(eeprom_read_byte((uint8_t *)1));
  }
@SpenceKonde
Copy link
Owner

SpenceKonde commented Oct 9, 2020 via email

@obdevel
Copy link
Author

obdevel commented Oct 10, 2020

Any pointers to how I might solve this myself ? I don't see any relevant appnotes and the one AS Start example isn't helpful either. I've read the NVMCTRL section of the datasheet but I don't know how to convert this to code.

@SpenceKonde
Copy link
Owner

Hold your horses, it's a 5-minute fix from my end...

@SpenceKonde
Copy link
Owner

Wait wait wait - the avr-libc routines don't work? Ugh... Okay, that will be a bit longer :-(

@SpenceKonde
Copy link
Owner

Need to implement those macros, in that case...

@SpenceKonde
Copy link
Owner

What's in there now is definitely totally hosed though

@SpenceKonde
Copy link
Owner

I just pushed a version of EEPROM that I would expect to work (in github repo - obv. not in a board manager release).

Does that not work?

@SpenceKonde
Copy link
Owner

If it doesn't work, please describe how it doesn't work...

@obdevel
Copy link
Author

obdevel commented Oct 10, 2020

Thanks. I copied that straight over the file in /Users/xxx/Documents/Arduino/hardware/DxCore/megaavr/libraries/EEPROM/src.

Compilation output: [eeprom_test_compile_v1.txt]. I restarted the IDE just to make sure there wasn't anything cached. (https://github.com/SpenceKonde/DxCore/files/5359825/eeprom_test_compile_v1.txt)

Same result - whatever value or address I write reads back as 255, e.g.

EEPROM[0] = 22;
Serial.println(EEPROM[0]);
EEPROM.write(1, 22);
Serial.println(EEPROM.read(1));

But, if it's calling eeprom_read_byte() surely that's resolved from avr-libc ?

@SpenceKonde
Copy link
Owner

SpenceKonde commented Oct 10, 2020

Check NVMCTRL.STATUS after attempting a write, see if there are any error flags.

I suspect read is working correctly, and write is not.

It's easy enough to verify read by just directly reading from the address of the mapped EEPROM

@obdevel
Copy link
Author

obdevel commented Oct 10, 2020

After a write, NVMCTRL.STATUS is 16. Per the datasheet (sec 10.5.3), that's the LSB of the error code, which translates to 0x1 INVALIDCMD (The selected command is not supported).

@SpenceKonde
Copy link
Owner

Yup, I had a feeling that was what you'd see....

Yeah, eeprom_write_byte needs to be reimplemented....

@obdevel
Copy link
Author

obdevel commented Oct 11, 2020

I installed MPLABX and used the mcc to generate some code, but I don't know how to implement this correctly. Maybe this will be a helpful pointer ?

nvmctrl.c.txt

/**
 * \brief Read a byte from eeprom
 *
 * \param[in] eeprom_adr The byte-address in eeprom to read from
 *
 * \return The read byte
 */
uint8_t DA_ReadEepromByte(eeprom_adr_t eeprom_adr)
{
		// Read operation will be stalled by hardware if any write is in progress		
		return *(uint8_t *)(EEPROM_START + eeprom_adr);
	
}

/**
 * \brief Write a byte to eeprom
 *
 * \param[in] eeprom_adr The byte-address in eeprom to write to
 * \param[in] data The byte to write
 *
 * \return Status of write operation
 */
nvmctrl_status_t DA_WriteEepromByte(eeprom_adr_t eeprom_adr, uint8_t data)
{
		/* Wait for completion of previous write */
		while (NVMCTRL.STATUS & (NVMCTRL_EEBUSY_bm|NVMCTRL_FBUSY_bm));

		/* Program the EEPROM with desired value(s) */
		ccp_write_spm((void *)&NVMCTRL.CTRLA, NVMCTRL_CMD_EEERWR_gc);

		/* Write byte to EEPROM */
		*(uint8_t *)(EEPROM_START + eeprom_adr) = data;
		
		/* Clear the current command */
		ccp_write_spm((void *)&NVMCTRL.CTRLA, NVMCTRL_CMD_NONE_gc); 

		return NVM_OK;		
}

static inline void ccp_write_spm(void *addr, uint8_t value)
{
	protected_write_io(addr, CCP_SPM_gc, value);
}

There is no implementation provided for protected_write_io() so I assume it's buried somewhere in some library ?

@obdevel
Copy link
Author

obdevel commented Oct 11, 2020

Ok, this seems to work. Again, cobbled together from disparate MCP sources.

...
#include <avr/xmega.h>
...

/** Datatype for return status of NVMCTRL operations */
typedef enum {
    NVM_OK    = 0, ///< NVMCTRL free, no write ongoing.
    NVM_ERROR = 1, ///< NVMCTRL operation retsulted in error
    NVM_BUSY  = 2, ///< NVMCTRL busy, write ongoing.
} nvmctrl_status_t;

uint8_t da_eeprom_read_byte(uint8_t * index) {
  return *(uint8_t *)(EEPROM_START + index);
}

nvmctrl_status_t da_eeprom_write_byte(uint8_t *index, uint8_t in) {

  /* Wait for completion of previous write */
  while (NVMCTRL.STATUS & (NVMCTRL_EEBUSY_bm|NVMCTRL_FBUSY_bm));

  /* Program the EEPROM with desired value(s) */
  _PROTECTED_WRITE_SPM(NVMCTRL.CTRLA, NVMCTRL_CMD_EEERWR_gc);
  
  /* Write byte to EEPROM */
  *(uint8_t *)(EEPROM_START + index) = in;
    
  /* Clear the current command */
  _PROTECTED_WRITE_SPM(NVMCTRL.CTRLA, NVMCTRL_CMD_NONE_gc);

  return NVM_OK;    
}

@SpenceKonde
Copy link
Owner

5403512 .... but yeah your approach is better if that works, somehow I thought (looks like incorrectly) that you weren't supposed to change the command while it was busy....

@obdevel
Copy link
Author

obdevel commented Oct 11, 2020

I was hoping you'd know better than me ! As I said, it was hacked together using MCC, a couple of app notes and some googling.

Works fine using your changes. Thanks.

@SpenceKonde
Copy link
Owner

SpenceKonde commented Oct 12, 2020

I had a similar problem when I tried to do it in the past defining a macro like that. I was as baffled as you were. That's why I didn't do it like that this time - I'd tripped over it before. No clue why the include guards don't do their job here...

@obdevel
Copy link
Author

obdevel commented Oct 13, 2020

https://github.com/SpenceKonde/DxCore/blob/master/megaavr/libraries/EEPROM/src/EEPROM.h#L38

Oops :) index needs to be wider than 8 bits to address 512 bytes. Wondered why I was getting strange overwrites.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants