Rahix / avr-hal

embedded-hal abstractions for AVR microcontrollers
Apache License 2.0
1.32k stars 223 forks source link

Is there anyway to access EEPROM ? #265

Closed krishnaTORQUE closed 1 year ago

krishnaTORQUE commented 2 years ago

Is there anyway to access EEPROM ?

Rahix commented 2 years ago

Not yet, this still needs to be implemented.

krishnaTORQUE commented 2 years ago

Thanks for the quick response. Looking forward to it.

fvilante commented 2 years ago

I'm sharing a workaround I coded while EEPROM is not implemented in avr-hal. It works for avr328p.

Low level function to read/write EEPROM on avr328p 8-bits microcontroler

Rahix commented 2 years ago

@fvilante, you might want to use the API from avr-device instead of those raw register accesses.

flyingyizi commented 2 years ago

there are lots of avr eeprom c code. e.g. eeprom.c in grbl. below shows a rust rewritten sample.


mod eeprom {
    use core::arch::asm;

    /// Read byte from EEPROM.it reads one byte from a given EEPROM address.
    ///
    /// The CPU is halted for 4 clock cycles during EEPROM read.
    pub fn eeprom_get_char(address: u16) -> u8 {
        let ptr = arduino_hal::hal::pac::EEPROM::ptr();

        unsafe {
            // wait until EEPE become to zero by hardware. on other word,
            //Wait for completion of previous write.
            while (*ptr).eecr.read().eepe().bit_is_set() {}
            // set EEPROM address register
            (*ptr).eear.write(|w| w.bits(address));
            //Start EEPROM read operation
            (*ptr).eecr.write(|w| w.eere().set_bit());
        }
        // Return the byte read from EEPROM
        unsafe { (*ptr).eedr.read().bits() }
    }

    /// Write byte to EEPROM.it writes one byte to a given EEPROM address.
    ///  The differences between the existing byte and the new value is used
    ///  to select the most efficient EEPROM programming mode.
    ///
    ///  \note  The CPU is halted for 2 clock cycles during EEPROM programming.
    ///
    ///  \note  When this function returns, the new EEPROM value is not available
    ///         until the EEPROM programming time has passed. The EEPE bit in EECR
    ///         should be polled to check whether the programming is finished.
    ///
    ///  \note  The eeprom_get_char function checks the EEPE bit automatically.
    pub fn eeprom_put_char(address: u16, data: u8) -> () {
        let ptr = arduino_hal::hal::pac::EEPROM::ptr();

        unsafe {
            // do something without interrupts
            asm!("CLI");
            // wait until EEPE become to zero by hardware
            while (*ptr).eecr.read().eepe().bit_is_set() {}

            // set EEPROM address
            (*ptr).eear.write(|w| w.bits(address));

            //Start EEPROM read operation
            (*ptr).eecr.write(|w| w.eere().set_bit());
            let old_value = (*ptr).eedr.read().bits();
            let diff_mask = old_value ^ data;

            // Check if any bits are changed to '1' in the new value.
            if (diff_mask & data) != 0 {
                // Now we know that _some_ bits need to be erased to '1'.

                // // Check if any bits in the new value are '0'.
                if data != 0xff {
                    // Now we know that some bits need to be programmed to '0' also.
                    (*ptr).eedr.write(|w| w.bits(data)); // Set EEPROM data register.
                    (*ptr).eecr.write(|w| {
                        w.eempe().set_bit(); // Set Master Write Enable bit
                        w.eepm().val_0x00() // ...and Erase+Write mode.
                    });
                    (*ptr).eecr.write(|w| w.eepe().set_bit()); // Start Erase+Write operation.
                } else {
                    // Now we know that all bits should be erased.
                    (*ptr).eecr.write(|w| {
                        w.eempe().set_bit(); // Set Master Write Enable bit
                        w.eepm().val_0x01() // ...and Erase-only mode..
                    });
                    (*ptr).eecr.write(|w| w.eepe().set_bit()); // Start Erase-only operation.
                }
            }
            //
            else {
                // Now we know that _no_ bits need to be erased to '1'.

                // Check if any bits are changed from '1' in the old value.
                if diff_mask != 0 {
                    // Now we know that _some_ bits need to the programmed to '0'.
                    (*ptr).eedr.write(|w| w.bits(data)); // Set EEPROM data register.
                    (*ptr).eecr.write(|w| {
                        w.eempe().set_bit(); // Set Master Write Enable bit
                        w.eepm().val_0x02() // ...and Write-only mode..
                    });
                    (*ptr).eecr.write(|w| w.eepe().set_bit()); // Start Write-only operation.
                }
            }

            asm!("SEI");
        }
    }
}
DoubleHyphen commented 2 years ago

@fvilante I was looking at the code example you pasted.

I'm a tad… apprehensive… about the way you set and clear single bits. At least one of us has completely misunderstood what's supposed to be going on, and I don't know who it is.

You define EEPE thusly: const EEPE: u8 = 0b0000010;. Thus, quite obviously, it is supposed to be a bit-mask; |=ing it with the EEPROM control register ought to set that bit, and ^=ing it with same ought to flip it. It's because it's so obviously a bit-mask that it's defined as a binary number with only one 1 in it… right?

17 lines later…

while (read_register(EECR) & (1<<EEPE)) == 1 { };

you bit-shift 1 by EEPE positions before you binary-and it with EECR. And I'm like… are you very sure that's what you need to do? This examines bit no. 2, which is EEMPE, not EEPE.

Further…

// normalize is to clamp the 16 bits address to a 9 bits address (1+8)

An ATMega328p has 1kiB of SRAM, not 512 bytes. This corresponds to 10 bits' worth of address space, in the range 0..=9. Other microcontrollers have less, per the data-sheet… but it's kind of queer that you're taking none of that into account.

Speaking of the data-sheet: Its code examples do contain this 1<<EEPE part, but… EEPE is never defined anywhere. It is therefore entirely possible that it's defined with a value of 1, so that 1<<EEPE == 0b0000_0010.

Rahix commented 2 years ago

@DoubleHyphen, the register accesses should really use the avr-device API as I wrote in the comment above. That API prevents such odd mistakes by design.

Rahix commented 2 years ago

@flyingyizi, thanks for providing the code snippets. Just FYI, instead of the raw cli and sei instructions, you should probably use the avr_device::interrupt::free helper. It makes sure interrupts are only re-enabled if they were actually enabled previously. This is very important for making the code generally reusable.

DoubleHyphen commented 2 years ago

Speaking of which… is there any way to get the total size of the EEPROM from somewhere? The EEPROM struct only appears to contain a pointer.

Rahix commented 1 year ago

Hm, I don't think there is... We should probably add a constant to the HAL crates alongside a proper EEPROM API...

flyingyizi commented 1 year ago

@Rahix :PR #371

for example, atmega328p finally eeprom codes is:

~/avr-hal/mcu/atmega-hal$ cargo +nightly install cargo-expand
~/avr-hal/mcu/atmega-hal$ cargo expand --features atmega328p  ep

will shows below snippet:

#[cfg(feature = "device-selected")]
pub mod ep {
    /// Flash erase/program error
    pub enum Error {
        Bounds,
        Other,
    }
    #[automatically_derived]     
    #[allow(unused_qualifications)]
    impl ::core::fmt::Debug for Error {
        fn fmt(&self, f: &mut ::core::fmt::Formatter) -> ::core::fmt::Result {
            match &*self {
                &Error::Bounds => ::core::fmt::Formatter::write_str(f, "Bounds"),
                &Error::Other => ::core::fmt::Formatter::write_str(f, "Other"),  
            }
        }
    }
    #[automatically_derived]
    #[allow(unused_qualifications)]
    impl ::core::clone::Clone for Error {
        #[inline]
        fn clone(&self) -> Error {
            *self
        }
    }
    #[automatically_derived]
    #[allow(unused_qualifications)]
    impl ::core::marker::Copy for Error {}
    pub struct Eeprom {}
    impl Eeprom {
        #[inline]
        unsafe fn wait_ready(&self) {
            while (*<crate::pac::EEPROM>::ptr()).eecr.read().eepe().bit_is_set() {}
        }
        #[inline]
        unsafe fn eeprom_set_address(&self, address: u16) {
            self.wait_ready();
            let peripheral = &(*<crate::pac::EEPROM>::ptr());
            let address = address;
            {
                peripheral.eear.write(|w| w.bits(address));
            }
        }
        #[inline]
        unsafe fn eeprom_set_erasewrite_mode(&self) {
            let peripheral = &(*<crate::pac::EEPROM>::ptr());
            {
                peripheral
                    .eecr
                    .write(|w| {
                        w.eempe().set_bit();
                        w.eepm().val_0x00()
                    });
            }
        }
        #[inline]
        unsafe fn eeprom_set_erase_mode(&self) {
            let peripheral = &(*<crate::pac::EEPROM>::ptr());
            {
                peripheral
                    .eecr
                    .write(|w| {
                        w.eempe().set_bit();
                        w.eepm().val_0x01()
                    });
            }
        }
        #[inline]
        unsafe fn eeprom_set_write_mode(&self) {
            let peripheral = &(*<crate::pac::EEPROM>::ptr());
            {
                peripheral
                    .eecr
                    .write(|w| {
                        w.eempe().set_bit();
                        w.eepm().val_0x02()
                    });
            }
        }
        unsafe fn eeprom_get_char(&mut self, address: u16) -> u8 {
            self.eeprom_set_address(address);
            (*<crate::pac::EEPROM>::ptr()).eecr.write(|w| w.eere().set_bit());
            (*<crate::pac::EEPROM>::ptr()).eedr.read().bits()
        }
        /// attention: if call it, should better call between disab/enable interrupt
        unsafe fn eeprom_put_char(&mut self, address: u16, data: u8) -> () {
            self.eeprom_set_address(address);
            let periph = &(*<crate::pac::EEPROM>::ptr());
            periph.eecr.write(|w| w.eere().set_bit());
            let old_value = periph.eedr.read().bits();
            let diff_mask = old_value ^ data;
            if (diff_mask & data) != 0 {
                if data != 0xff {
                    periph.eedr.write(|w| w.bits(data));
                    self.eeprom_set_erasewrite_mode();
                    periph.eecr.write(|w| w.eepe().set_bit());
                } else {
                    self.eeprom_set_erase_mode();
                    periph.eecr.write(|w| w.eepe().set_bit());
                }
            } else {
                if diff_mask != 0 {
                    periph.eedr.write(|w| w.bits(data));
                    self.eeprom_set_write_mode();
                    periph.eecr.write(|w| w.eepe().set_bit());
                }
            }
        }
    }
    impl ::avr_hal_generic::embedded_storage::nor_flash::ReadNorFlash for Eeprom {
        type Error = Error;
        const READ_SIZE: usize = 1;
        fn read(&mut self, offset: u32, bytes: &mut [u8]) -> Result<(), Self::Error> {
            if bytes.len() + offset as usize > 1024 {
                return Err(Self::Error::Bounds);
            }
            let len = bytes.len();
            let mut offset = offset as u16;
            for i in 0..len {
                bytes[i] = unsafe { self.eeprom_get_char(offset) };
                offset += 1;
            }
            Ok(())
        }
        fn capacity(&self) -> usize {
            1024
        }
    }
    impl ::avr_hal_generic::embedded_storage::nor_flash::NorFlash for Eeprom {
        const WRITE_SIZE: usize = 1;
        const ERASE_SIZE: usize = 1;
        fn erase(&mut self, from: u32, to: u32) -> Result<(), Self::Error> {
            if to > 1024 {
                return Err(Self::Error::Bounds);
            }
            ::avr_hal_generic::avr_device::interrupt::free(|_cs| {
                unsafe {
                    for i in from..to {
                        self.eeprom_set_address(i as u16);
                        self.eeprom_set_erase_mode();
                        (*<crate::pac::EEPROM>::ptr())
                            .eecr
                            .write(|w| w.eepe().set_bit());
                    }
                }
            });
            Ok(())
        }
        fn write(&mut self, offset: u32, bytes: &[u8]) -> Result<(), Self::Error> {
            if bytes.len() + offset as usize > 1024 {
                return Err(Self::Error::Bounds);
            }
            let mut offset = offset as u16;
            for i in bytes {
                ::avr_hal_generic::avr_device::interrupt::free(|_cs| {
                    unsafe { self.eeprom_put_char(offset as u16, *i) };
                    offset += 1;
                });
            }
            Ok(())
        }
    }
    impl ::avr_hal_generic::embedded_storage::nor_flash::MultiwriteNorFlash for Eeprom {}
}
Rahix commented 1 year ago

A proper EEPROM driver exists in avr-hal now. Thanks to @flyingyizi!