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

stm32/flashbdev: linker script flash storage layout should override the defaults in flashbdev.c #8390

Closed
iabdalkader opened this issue Mar 7, 2022 · 10 comments

Comments

@iabdalkader
Copy link
Contributor

In 8496919 flash storage layout configuration via linker script was introduced, but not all MCUs were converted to use it in 35e70c1. The MCUs that still have defaults in flashbdev.c, for example STM32F427xx, can't use the linker script to configure the flash storage layout. Can all MCUs be switched to use the linker script approach ? Or alternatively, can a board-level config option be added, that when enabled gives precedence to the linker script layout ?

@dpgeorge
Copy link
Member

dpgeorge commented Mar 8, 2022

Can all MCUs be switched to use the linker script approach ?

Yes. This was the intention, I just didn't get around to it.

@iabdalkader
Copy link
Contributor Author

Yes. This was the intention, I just didn't get around to it.

I can send a PR later to convert the remaining MCUs, following the others.

@iabdalkader
Copy link
Contributor Author

iabdalkader commented Mar 13, 2022

@dpgeorge Is this right ? It used to be the max flash sector, now it's the entire flash cache size:

#define FLASH_SECTOR_SIZE_MAX \
    (&_micropy_hw_internal_flash_storage_ram_cache_end[0] - &_micropy_hw_internal_flash_storage_ram_cache_start[0])

The max sector size and flash cache size are mostly equal, except for a few MCUs, for example STM32F746:

#define CACHE_MEM_START_ADDR (0x20000000) // DTCM data RAM, 64k
#define FLASH_SECTOR_SIZE_MAX (0x08000) // 32k max

Also this is wrong:

#if defined(STM32F401xE) || defined(STM32F411xE) || defined(STM32F412Zx) || defined(STM32F446xx)
....
#define FLASH_SECTOR_SIZE_MAX (0x4000) // 16k max due to size of cache buffer
#define FLASH_MEM_SEG1_NUM_BLOCKS (128) // sectors 1,2,3,4: 16k+16k+16k+16k(of 64k)=64k

Sector 4 for these MCUs is 64K not 16K, which means either the cache mem size should be increased to 64K, or the number of blocks should be reduced to 96 blocks.

@dpgeorge
Copy link
Member

Is this right ? It used to be the max flash sector, now it's the entire flash cache size:

Yes that looks correct, FLASH_SECTOR_SIZE_MAX should be the maximum amount of RAM available in the cache.

For the F746, it won't matter if this increases to 64k, it'll only ever use 32k because that's the maximum flash sector in the range used by the storage.

Also this is wrong:

No, it's correct. The comment says 16k(of 64k) which is correct (only 16k of the 64k sector 4 is used). 128 = 16k*4 / 512.

iabdalkader added a commit to iabdalkader/micropython that referenced this issue Mar 16, 2022
iabdalkader added a commit to iabdalkader/micropython that referenced this issue Mar 16, 2022
@iabdalkader
Copy link
Contributor Author

iabdalkader commented Mar 16, 2022

only 16k of the 64k sector 4 is used

But wouldn't this report the wrong FS size ?

EDIT: The num of blocks will be more than what's actually usable:

#define FLASH_MEM_SEG1_NUM_BLOCKS \
    ((&_micropy_hw_internal_flash_storage_end - &_micropy_hw_internal_flash_storage_start) / 512)

@dpgeorge
Copy link
Member

The 128 is correct in the existing code.

But I see your point, that the new scheme that computes the number of blocks from the linker symbols won't work... I think what needs to be done is the FLASH_FS constants need to be decreased to reflect only the usable part of flash for the filesystem.

@iabdalkader
Copy link
Contributor Author

the new scheme that computes the number of blocks from the linker symbols won't work.

Exactly.

@iabdalkader
Copy link
Contributor Author

what needs to be done is the FLASH_FS constants need to be decreased to reflect only the usable part of flash for the filesystem.

Okay but the remainder of the sector can not be used for code, right ? The whole sector must be erased when updating the firmware.

@dpgeorge
Copy link
Member

Okay but the remainder of the sector can not be used for code, right

Correct.

The whole sector must be erased when updating the firmware.

Note that the filesystem is not touched when updating the firmware. So it should be enough to just shrink FLASH_FS to the usable size.

@iabdalkader
Copy link
Contributor Author

Note that the filesystem is not touched when updating the firmware

I know, I was just saying that the remainder of the sector will be unusable.

iabdalkader added a commit to iabdalkader/micropython that referenced this issue Mar 16, 2022
iabdalkader added a commit to iabdalkader/micropython that referenced this issue Mar 16, 2022
dpgeorge pushed a commit that referenced this issue Mar 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants