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 at wrong location for devices with flash size other than 128K? #379

Open
arpruss opened this issue Nov 15, 2017 · 9 comments
Open

Comments

@arpruss
Copy link
Contributor

arpruss commented Nov 15, 2017

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
Copy link
Owner

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
Copy link
Contributor Author

arpruss commented Nov 16, 2017

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
Copy link
Contributor Author

arpruss commented Nov 16, 2017

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
Copy link
Owner

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
Copy link
Contributor Author

arpruss commented Nov 16, 2017

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
Copy link
Contributor Author

arpruss commented Nov 23, 2017

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

@stevstrong
Copy link
Collaborator

Is this still actual?

@me21
Copy link
Contributor

me21 commented Feb 6, 2020

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
Copy link
Contributor

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();
}

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

5 participants