Add additional fixes to EEPROM driver selection (#7274) (#266)

* Add additional fixes to EEPROM driver selection (#7274)

- uprintf -> dprintf
- Fix atsam "vendor" eeprom.
- Bump Kinetis K20x to 64 bytes, too.
- Rollback Kinetis to 32 bytes as partitioning can only be done once. Add warning about changing the value.
- Change RAM-backed "fake" EEPROM implementations to match eeconfig's current usage.
- Add 24LC128 by request.

* format code according to conventions [skip ci]

Co-authored-by: Nick Brassel <nick@tzarc.org>
Co-authored-by: QMK Bot <hello@qmk.fm>
Co-authored-by: Florian Didron <fdidron@users.noreply.github.com>

common_features.mk

@@ -102,6 +102,57 @@ ifeq ($(strip $(UNICODE_COMMON)), yes)
     SRC += $(QUANTUM_DIR)/process_keycode/process_unicode_common.c
 endif
 
+VALID_EEPROM_DRIVER_TYPES := vendor custom transient i2c
+EEPROM_DRIVER ?= vendor
+ifeq ($(filter $(EEPROM_DRIVER),$(VALID_EEPROM_DRIVER_TYPES)),)
+  $(error EEPROM_DRIVER="$(EEPROM_DRIVER)" is not a valid EEPROM driver)
+else
+  OPT_DEFS += -DEEPROM_ENABLE
+  ifeq ($(strip $(EEPROM_DRIVER)), custom)
+    OPT_DEFS += -DEEPROM_DRIVER -DEEPROM_CUSTOM
+    COMMON_VPATH += $(DRIVER_PATH)/eeprom
+    SRC += eeprom_driver.c
+  else ifeq ($(strip $(EEPROM_DRIVER)), i2c)
+    OPT_DEFS += -DEEPROM_DRIVER -DEEPROM_I2C
+    COMMON_VPATH += $(DRIVER_PATH)/eeprom
+    QUANTUM_LIB_SRC += i2c_master.c
+    SRC += eeprom_driver.c eeprom_i2c.c
+  else ifeq ($(strip $(EEPROM_DRIVER)), transient)
+    OPT_DEFS += -DEEPROM_DRIVER -DEEPROM_TRANSIENT
+    COMMON_VPATH += $(DRIVER_PATH)/eeprom
+    SRC += eeprom_driver.c eeprom_transient.c
+  else ifeq ($(strip $(EEPROM_DRIVER)), vendor)
+    OPT_DEFS += -DEEPROM_VENDOR
+    ifeq ($(PLATFORM),AVR)
+      # Automatically provided by avr-libc, nothing required
+    else ifeq ($(PLATFORM),CHIBIOS)
+      ifeq ($(MCU_SERIES), STM32F3xx)
+        SRC += $(PLATFORM_COMMON_DIR)/eeprom_stm32.c
+        SRC += $(PLATFORM_COMMON_DIR)/flash_stm32.c
+        OPT_DEFS += -DEEPROM_EMU_STM32F303xC
+        OPT_DEFS += -DSTM32_EEPROM_ENABLE
+      else ifeq ($(MCU_SERIES), STM32F1xx)
+        SRC += $(PLATFORM_COMMON_DIR)/eeprom_stm32.c
+        SRC += $(PLATFORM_COMMON_DIR)/flash_stm32.c
+        OPT_DEFS += -DEEPROM_EMU_STM32F103xB
+        OPT_DEFS += -DSTM32_EEPROM_ENABLE
+      else ifeq ($(MCU_SERIES)_$(MCU_LDSCRIPT), STM32F0xx_STM32F072xB)
+        SRC += $(PLATFORM_COMMON_DIR)/eeprom_stm32.c
+        SRC += $(PLATFORM_COMMON_DIR)/flash_stm32.c
+        OPT_DEFS += -DEEPROM_EMU_STM32F072xB
+        OPT_DEFS += -DSTM32_EEPROM_ENABLE
+      else
+        # This will effectively work the same as "transient" if not supported by the chip
+        SRC += $(PLATFORM_COMMON_DIR)/eeprom_teensy.c
+      endif
+    else ifeq ($(PLATFORM),ARM_ATSAM)
+      SRC += $(PLATFORM_COMMON_DIR)/eeprom.c
+    else ifeq ($(PLATFORM),TEST)
+      SRC += $(PLATFORM_COMMON_DIR)/eeprom.c
+    endif
+  endif
+endif
+
 ifeq ($(strip $(RGBLIGHT_ENABLE)), yes)
     POST_CONFIG_H += $(QUANTUM_DIR)/rgblight_post_config.h
     OPT_DEFS += -DRGBLIGHT_ENABLE

drivers/eeprom/eeprom_custom.c-template

@@ -0,0 +1,46 @@
+/* Copyright 2019 Nick Brassel (tzarc)
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <stdint.h>
+#include <string.h>
+
+#include "eeprom_driver.h"
+
+void eeprom_driver_init(void) {
+    /* Any initialisation code */
+ }
+
+void eeprom_driver_erase(void) {
+    /* Wipe out the EEPROM, setting values to zero */
+}
+
+void eeprom_read_block(void *buf, const void *addr, size_t len) {
+    /*
+        Read a block of data:
+            buf: target buffer
+            addr: 0-based offset within the EEPROM
+            len: length to read
+     */
+}
+
+void eeprom_write_block(const void *buf, void *addr, size_t len) {
+    /*
+        Write a block of data:
+            buf: target buffer
+            addr: 0-based offset within the EEPROM
+            len: length to write
+     */
+}

drivers/eeprom/eeprom_i2c.c

@@ -76,11 +76,11 @@ void eeprom_read_block(void *buf, const void *addr, size_t len) {
     i2c_receive(EXTERNAL_EEPROM_I2C_ADDRESS((intptr_t)addr), buf, len, 100);
 
 #ifdef DEBUG_EEPROM_OUTPUT
-    uprintf("[EEPROM R] 0x%04X: ", ((int)addr));
+    dprintf("[EEPROM R] 0x%04X: ", ((int)addr));
     for (size_t i = 0; i < len; ++i) {
-        uprintf(" %02X", (int)(((uint8_t *)buf)[i]));
+        dprintf(" %02X", (int)(((uint8_t *)buf)[i]));
     }
-    uprintf("\n");
+    dprintf("\n");
 #endif  // DEBUG_EEPROM_OUTPUT
 }
 
@@ -103,11 +103,11 @@ void eeprom_write_block(const void *buf, void *addr, size_t len) {
         }
 
 #ifdef DEBUG_EEPROM_OUTPUT
-        uprintf("[EEPROM W] 0x%04X: ", ((int)target_addr));
+        dprintf("[EEPROM W] 0x%04X: ", ((int)target_addr));
         for (uint8_t i = 0; i < write_length; i++) {
-            uprintf(" %02X", (int)(read_buf[i]));
+            dprintf(" %02X", (int)(read_buf[i]));
         }
-        uprintf("\n");
+        dprintf("\n");
 #endif  // DEBUG_EEPROM_OUTPUT
 
         i2c_transmit(EXTERNAL_EEPROM_I2C_ADDRESS((intptr_t)addr), complete_packet, EXTERNAL_EEPROM_ADDRESS_SIZE + write_length, 100);

drivers/eeprom/eeprom_transient.c

@@ -20,7 +20,7 @@
 #include "eeprom_driver.h"
 #include "eeprom_transient.h"
 
-static uint8_t transientBuffer[TRANSIENT_EEPROM_SIZE] = {0};
+__attribute__((aligned(4))) static uint8_t transientBuffer[TRANSIENT_EEPROM_SIZE] = {0};
 
 size_t clamp_length(intptr_t offset, size_t len) {
     if (offset + len > TRANSIENT_EEPROM_SIZE) {

drivers/eeprom/eeprom_transient.h

@@ -20,5 +20,6 @@
     The size of the transient EEPROM buffer size.
 */
 #ifndef TRANSIENT_EEPROM_SIZE
-#    define TRANSIENT_EEPROM_SIZE 64
+#    include "eeconfig.h"
+#    define TRANSIENT_EEPROM_SIZE (((EECONFIG_SIZE + 3) / 4) * 4)  // based off eeconfig's current usage, aligned to 4-byte sizes, to deal with LTO
 #endif

tmk_core/common/arm_atsam/eeprom.c

@@ -16,9 +16,12 @@
 
 #include "eeprom.h"
 
-#define EEPROM_SIZE 32
+#ifndef EEPROM_SIZE
+#    include "eeconfig.h"
+#    define EEPROM_SIZE (((EECONFIG_SIZE + 3) / 4) * 4)  // based off eeconfig's current usage, aligned to 4-byte sizes, to deal with LTO
+#endif
 
-static uint8_t buffer[EEPROM_SIZE];
+__attribute__((aligned(4))) static uint8_t buffer[EEPROM_SIZE];
 
 uint8_t eeprom_read_byte(const uint8_t *addr) {
     uintptr_t offset = (uintptr_t)addr;

tmk_core/common/chibios/eeprom_teensy.c

@@ -50,7 +50,16 @@
 // (aligned to 2 or 4 byte boundaries) has twice the endurance
 // compared to writing 8 bit bytes.
 //
+#    ifndef EEPROM_SIZE
 #        define EEPROM_SIZE 32
+#    endif
+
+/*
+    ^^^ Here be dragons:
+        NXP AppNote AN4282 section 3.1 states that partitioning must only be done once.
+        Once EEPROM partitioning is done, the size is locked to this initial configuration.
+        Attempts to modify the EEPROM_SIZE setting may brick your board.
+*/
 
 // Writing unaligned 16 or 32 bit data is handled automatically when
 // this is defined, but at a cost of extra code size.  Without this,
@@ -517,8 +526,11 @@ void eeprom_write_block(const void *buf, void *addr, uint32_t len) {
 #else
 // No EEPROM supported, so emulate it
 
-#    define EEPROM_SIZE 64
+#    ifndef EEPROM_SIZE
-static uint8_t buffer[EEPROM_SIZE];
+#        include "eeconfig.h"
+#        define EEPROM_SIZE (((EECONFIG_SIZE + 3) / 4) * 4)  // based off eeconfig's current usage, aligned to 4-byte sizes, to deal with LTO
+#    endif
+__attribute__((aligned(4))) static uint8_t buffer[EEPROM_SIZE];
 
 uint8_t eeprom_read_byte(const uint8_t *addr) {
     uint32_t offset = (uint32_t)addr;