rogerclarkmelbourne / Arduino_STM32

Arduino STM32. Hardware files to support STM32 boards, on Arduino IDE 1.8.x including LeafLabs Maple and other generic STM32F103 boards
Other
2.53k stars 1.26k forks source link

EEPROM at wrong location for devices with flash size other than 128K? #379

Open arpruss opened 6 years ago

arpruss commented 6 years ago

If I am reading the code wrong, just close this.

The only place I can find EEPROM_START_ADDRESS to be defined is EEPROM.h. But as I read EEPROM.h, because of the hack of defining MCU_STM32F103RB on all MCUs, EEPROM_START_ADDRESS is set to 0x8000000 plus 124K always. This means that on devices with 64K flash (e.g., those STM32F103C8 units that have only 64K), the EEPROM is outside the available memory, and on devices with more than 128K flash, the EEPROM storage may overwrite program code.

Maybe EEPROM_START_ADDRESS is defined somewhere else in the codebase and I just can't find it?

rogerclarkmelbourne commented 6 years ago

The only way to do this correctly is to add another define into boards.txt which holds the Flash size.

We can't do this at the variant level, as F103C8 and F103CB use the same variant folder

In reality this is hardly ever and issue as virtually all F103C8's have 128k

However looking in the class there is a init() function which you can pass the page addresses

https://github.com/rogerclarkmelbourne/Arduino_STM32/blob/master/STM32F1/libraries/EEPROM/EEPROM.cpp#L290-L296

Which looks like it solves the problem in the sort term

arpruss commented 6 years ago

This could lead to a really hard to find bug on a device with 256k flash and a large sketch. Maybe one could define something based on the upload.maximum_size field in boards.txt. As far as I know, there are only a few flash sizes in common use on the f1, like 64, 128, 192, 256 and 512. It would not be hard to guess the right one based on the maximum_size field.

arpruss commented 6 years ago

Here's what I am thinking:

In platform.txt, for both c and c++ building, add:

-DARDUINO_UPLOAD_MAXIMUM_SIZE={upload.maximum_size}

Then in EEPROM.h we can put:

#define FLASH_SIZE \
  ( ARDUINO_UPLOAD_MAXIMUM_SIZE > 768*1024 ? 1024*1024 : \
    ARDUINO_UPLOAD_MAXIMUM_SIZE > 512*1024 ? 768*1024 : \
    ARDUINO_UPLOAD_MAXIMUM_SIZE > 384*1024 ? 512*1024 : \
    ARDUINO_UPLOAD_MAXIMUM_SIZE > 256*1024 ? 384*1024 : \
    ARDUINO_UPLOAD_MAXIMUM_SIZE > 128*1024 ? 256*1024 : \
    ARDUINO_UPLOAD_MAXIMUM_SIZE > 64*1024 ? 128*1024 : \
    ARDUINO_UPLOAD_MAXIMUM_SIZE > 32*1024 ? 64*1024 : \
    ARDUINO_UPLOAD_MAXIMUM_SIZE > 16*1024 ? 32*1024 : \
    16*1024 ) // these are all the sizes of flashes for stm32f1

#define EEPROM_START_ADDRESS    ((uint32)(0x8000000 + FLASH_SIZE - 2 * EEPROM_PAGE_SIZE))
rogerclarkmelbourne commented 6 years ago

Yes. That would work

But we may be able to do the calculation in C rather than in a complex macro, as I think ultimately the EEPROM_START_ADDRESS gets put into a variable

arpruss commented 6 years ago

I'm still not used to modern compilers and being able to count on them to optimize all complex constant expressions away.

Yes, one could put it in a const variable.

An additional good thing one could do is to use the flash size and the table here to figure out if it's a low, medium or high density device, and then set the page size to 1k for low/medium and 2k for high density.

(There are also connectivity-line devices, stm32f105/7, where the page size would be incorrectly determined by this method, but the core doesn't support them.)

arpruss commented 6 years ago

I put in a PR that handles this issue (from a different github username, for my convenience).

stevstrong commented 5 years ago

Is this still actual?

me21 commented 4 years ago

It is. EEPROM library still needs manual setting of EEPROM location for devices with flash memory size !=128K. It can be done with preprocessor macros passed to compiler when building, but enabling the auto-adjustment would save some time debugging why my EEPROM doesn't work out of the box on STM32F103C8, for example, where only 64K is available.

tomaskovacik commented 4 years ago

hello I just fight with EEPROM library and I can say that this not works on my bluepills (64 and 128kB versions):

setup(){
  EEPROM.PageBase0 = 0x801F000;
  EEPROM.PageBase1 = 0x801F800;
  EEPROM.PageSize  = 0x400;
  EEPROM.init();

so I swithced to using init(uint32,uint32,uint32) and calculation in my code, which works:

#define EEPROM_PAGE_SIZE        (uint16)0x400  /* Page size = 1KByte */
//set this absed on your chip flash, but mostly 64 will do, this FW is 40k big for now ... 
#define FLASH_SIZE 64
#define EEPROM_START_ADDRESS    ((uint32)(0x8000000 + FLASH_SIZE * 1024 - 2 * EEPROM_PAGE_SIZE))
#define EEPROM_PAGE0_BASE               ((uint32)(EEPROM_START_ADDRESS + 0x000))
#define EEPROM_PAGE1_BASE               ((uint32)(EEPROM_START_ADDRESS + EEPROM_PAGE_SIZE))

setup(){
void setup ()
{
   EEPROM.init(EEPROM_PAGE0_BASE,EEPROM_PAGE1_BASE,EEPROM_PAGE_SIZE);

as the only parameter which library requires to calculate offsets correctly is flash size, I add this init version, and it works for 64 and also 128k, but for now, I stick with init function which takes base0, base1 and pagesize as parameters, for compatibility after upgrade of stm32core

uint16 EEPROMClass::init(uint16 flashSizeInKB)
{
        if (flashSizeInKB>128)
                PageSize = (uint16)0x800;  //2k for flash size over 128?
        else
                PageSize = (uint16)0x400;  //1k

        PageBase0 = (uint32)(0x8000000 + flashSizeInKB * 1024 - 2 * PageSize);
        PageBase1 = PageBase0 + PageSize;
        return init();
}