Skip to content

Commit

Permalink
Make eeprom driver behaviour more uniform. Adding eeprom_driver_forma…
Browse files Browse the repository at this point in the history
…t abstraction.

Before this change, we used to call epprom_driver_erase(), if we detected that the eeprom
content did not have the necessary magic number, but only of it was implemented as an
"eeprom_driver". This is sometimes necessary, and sometimes not. We used to make the decision
based on whether the eeprom_driver_erase() function has been implemented or not, but that is
not really the best way to decide.

Erase in necessary for example when eeprom emulation/wear leveling is in use. This is because
the actual backing store also contains metadata that needs to start out in a clean state.
It is not necessary to erase I2C and SPI EEPROMs before use, however we used to do it.

The new eeprom_driver_format(erase=false) call will allow each eeprom driver to decide what is the
minimum amount of initialization/erasure that must be done to make the eeprom usable.
This also leaves open the door for future optimizations for eeprom emulation/wear leveling
drivers to only erase a subset of the backing store.

eeprom_driver_format(erase=true) will also make sure that the content of the eeprom is erased.

This change replaces calls from eeconfig to eeprom_driver_erase() to eeprom_driver_format().
As a result the following behavioral changes will happen:

|Platform                                            | Behavioral Change (if no magic number is found in EEPROM)                                      |
|----------------------------------------------------|------------------------------------------------------------------------------------------------|
|EEPROM_DRIVER=custom                                | No Change (Always Erase, in case eeprom_driver_format is not implemented by the custom driver) |
|EEPROM_DRIVER=wear_leveling                         | No Change (Always Erase)                                                                       |
|EEPROM_DRIVER=i2c                                   | Erase -> Don't Erase                                                                           |
|EEPROM_DRIVER=spi                                   | Erase -> Don't Erase                                                                           |
|EEPROM_DRIVER=legacy_stm32_flash                    | Doesn't work, references inexistent file. If corrected, then No Change -> (Always Erase)       |
|EEPROM_DRIVER=transient                             | Erase -> Don't Erase (Not necessary to erase because the backing store is in the .bss section) |
|EEPROM_DRIVER=vendor (AVR)                          | No Change (Don't erase)                                                                        |
|EEPROM_DRIVER=vendor (STM32 072xB 042x6 - legacy)   | No Change (Always Erase)                                                                       |
|EEPROM_DRIVER=vendor (STM32 using wear_leveling)    | No Change (Always Erase) (same as EEPROM_DRIVER=wear_leveling)                                 |
|EEPROM_DRIVER=vendor (STM32_L0_L1 true eeprom)      | Erase -> Don't Erase                                                                           |
|EEPROM_DRIVER=vendor (RP2040)                       | No Change (Always Erase) (same as EEPROM_DRIVER=wear_leveling)                                 |
|EEPROM_DRIVER=vendor (Kinetis Flexram (KL2x, K20x)) | No Change (EEPROM_DRIVER macro is not defined)                                                 |
|EEPROM_DRIVER=vendor (other chibios=transient)      | Erase -> Don't Erase (Not necessary to erase because the backing store is in the .bss section) |
|EEPROM_DRIVER=vendor (ARM_ATSAM)                    | No Change (EEPROM_DRIVER macro is not defined)                                                 |

In particular this results in speed improvements when using STM32L0, i2c, and spi EEPROMs
  • Loading branch information
purdeaandrei committed Apr 15, 2023
1 parent 0ddb7d7 commit a51882c
Show file tree
Hide file tree
Showing 10 changed files with 63 additions and 4 deletions.
11 changes: 11 additions & 0 deletions drivers/eeprom/eeprom_custom.c-template
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,17 @@ void eeprom_driver_init(void) {
/* Any initialisation code */
}

void eeprom_driver_format(bool erase) {
/* If erase=false, then only do the absolute minimum initialisation necessary
to make sure that the eeprom driver is usable. It doesn't need to guarantee
that the content of the eeprom is reset to any particular value. For many
eeprom drivers this may be a no-op.

If erase=true, then in addition to making sure the eeprom driver is in a
usable state, also make sure that it is erased.
*/
}

void eeprom_driver_erase(void) {
/* Wipe out the EEPROM, setting values to zero */
}
Expand Down
6 changes: 6 additions & 0 deletions drivers/eeprom/eeprom_driver.c
Original file line number Diff line number Diff line change
Expand Up @@ -77,3 +77,9 @@ void eeprom_update_dword(uint32_t *addr, uint32_t value) {
eeprom_write_dword(addr, value);
}
}

void eeprom_driver_format(bool erase) __attribute__((weak));
void eeprom_driver_format(bool erase) {
(void)erase; /* The default implementation assumes that the eeprom must be erased in order to be usable. */
eeprom_driver_erase();
}
2 changes: 2 additions & 0 deletions drivers/eeprom/eeprom_driver.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@

#pragma once

#include <stdbool.h>
#include "eeprom.h"

void eeprom_driver_init(void);
void eeprom_driver_format(bool erase);
void eeprom_driver_erase(void);
8 changes: 8 additions & 0 deletions drivers/eeprom/eeprom_i2c.c
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
#include "wait.h"
#include "i2c_master.h"
#include "eeprom.h"
#include "eeprom_driver.h"
#include "eeprom_i2c.h"

// #define DEBUG_EEPROM_OUTPUT
Expand All @@ -62,6 +63,13 @@ void eeprom_driver_init(void) {
#endif
}

void eeprom_driver_format(bool erase) {
/* i2c eeproms do not need to be formatted before use */
if (erase) {
eeprom_driver_erase();
}
}

void eeprom_driver_erase(void) {
#if defined(CONSOLE_ENABLE) && defined(DEBUG_EEPROM_OUTPUT)
uint32_t start = timer_read32();
Expand Down
8 changes: 8 additions & 0 deletions drivers/eeprom/eeprom_spi.c
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
#include "timer.h"
#include "spi_master.h"
#include "eeprom.h"
#include "eeprom_driver.h"
#include "eeprom_spi.h"

#define CMD_WREN 6
Expand Down Expand Up @@ -92,6 +93,13 @@ void eeprom_driver_init(void) {
spi_init();
}

void eeprom_driver_format(bool erase) {
/* spi eeproms do not need to be formatted before use */
if (erase) {
eeprom_driver_erase();
}
}

void eeprom_driver_erase(void) {
#if defined(CONSOLE_ENABLE) && defined(DEBUG_EEPROM_OUTPUT)
uint32_t start = timer_read32();
Expand Down
9 changes: 7 additions & 2 deletions drivers/eeprom/eeprom_transient.c
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,13 @@ size_t clamp_length(intptr_t offset, size_t len) {
return len;
}

void eeprom_driver_init(void) {
eeprom_driver_erase();
void eeprom_driver_init(void) {}

void eeprom_driver_format(bool erase) {
/* The transient eeprom driver doesn't necessarily need to be formatted before use, and it always starts up filled with zeros, due to placement in the .bss section */
if (erase) {
eeprom_driver_erase();
}
}

void eeprom_driver_erase(void) {
Expand Down
6 changes: 6 additions & 0 deletions drivers/eeprom/eeprom_wear_leveling.c
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,12 @@ void eeprom_driver_init(void) {
wear_leveling_init();
}

void eeprom_driver_format(bool erase) {
/* wear leveling requires the write log data structures to be erased before use. */
(void)erase;
eeprom_driver_erase();
}

void eeprom_driver_erase(void) {
wear_leveling_erase();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
#include "debug.h"
#include "eeprom_legacy_emulated_flash.h"
#include "legacy_flash_ops.h"
#include "eeprom_driver.h"

/*
* We emulate eeprom by writing a snapshot compacted view of eeprom contents,
Expand Down Expand Up @@ -564,6 +565,12 @@ void eeprom_driver_init(void) {
EEPROM_Init();
}

void eeprom_driver_format(bool erase) {
/* emulated eepron requires the write log data structures to be erased before use. */
(void)erase;
eeprom_driver_erase();
}

void eeprom_driver_erase(void) {
EEPROM_Erase();
}
Expand Down
6 changes: 6 additions & 0 deletions platforms/chibios/drivers/eeprom/eeprom_stm32_L0_L1.c
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,12 @@ static inline void STM32_L0_L1_EEPROM_Lock(void) {

void eeprom_driver_init(void) {}

void eeprom_driver_format(bool erase) {
if (erase) {
eeprom_driver_erase();
}
}

void eeprom_driver_erase(void) {
STM32_L0_L1_EEPROM_Unlock();

Expand Down
4 changes: 2 additions & 2 deletions quantum/eeconfig.c
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ __attribute__((weak)) void eeconfig_init_kb(void) {
*/
void eeconfig_init_quantum(void) {
#if defined(EEPROM_DRIVER)
eeprom_driver_erase();
eeprom_driver_format(false);
#endif
eeprom_update_word(EECONFIG_MAGIC, EECONFIG_MAGIC_NUMBER);
eeprom_update_byte(EECONFIG_DEBUG, 0);
Expand Down Expand Up @@ -111,7 +111,7 @@ void eeconfig_enable(void) {
*/
void eeconfig_disable(void) {
#if defined(EEPROM_DRIVER)
eeprom_driver_erase();
eeprom_driver_format(false);
#endif
eeprom_update_word(EECONFIG_MAGIC, EECONFIG_MAGIC_NUMBER_OFF);
}
Expand Down

0 comments on commit a51882c

Please sign in to comment.