zephyrproject-rtos / zephyr

Primary Git Repository for the Zephyr Project. Zephyr is a new generation, scalable, optimized, secure RTOS for multiple hardware architectures.
https://docs.zephyrproject.org
Apache License 2.0
10.59k stars 6.48k forks source link

Incorrect flash layout for STM32F427 #73373

Closed rickp-virscient closed 3 weeks ago

rickp-virscient commented 4 months ago

I'm using an stm32f427vgt part, which has a 1MB flash, with a sector layout of 4 16KB, 1 64KB, 3 128KB, 4 16KB, 1 64KB, 3 128KB (ie. 16 sectors).

However, end up with a flash layout with 24 sectors which has 7 * 128KB pages in each bank.

RM0090 specifies both, but Zephyr has implemented only the larger size in drivers/flash/

#elif FLASH_SECTOR_TOTAL == 24                                                                                                                                                                                    
static const struct flash_pages_layout stm32f4_flash_layout[] = {                                                                                                                                                 
        /*                                                                                                                                                                                                        
         * RM0090, table 6: STM32F427xx, STM32F437xx, STM32F429xx, STM32F439xx                                                                                                                                    
         * RM0386, table 4: STM32F469xx, STM32F479xx                                                                                                                                                              
         */                                                                                                                                                                                                       
        {.pages_count = 4, .pages_size = KB(16)},                                                                                                                                                                 
        {.pages_count = 1, .pages_size = KB(64)},                                                                                                                                                                 
        {.pages_count = 7, .pages_size = KB(128)},                                                                                                                                                                
        {.pages_count = 4, .pages_size = KB(16)},                                                                                                                                                                 
        {.pages_count = 1, .pages_size = KB(64)},                                                                                                                                                                 
        {.pages_count = 7, .pages_size = KB(128)},                                                                                                                                                                
};                                                                                                                                                                                                                
#else 

The FLASH_SECTOR_TOTAL comes from modules/hal/stm32/stm32cube/stm32f4xx/drivers/include/stm32f4xx_hal_flash_ex.h and is set to 24 for all 427xx parts:

/*--------------------------------- STM32F42xxx/STM32F43xxx/STM32F469xx/STM32F479xx---------------------*/
#if defined(STM32F427xx) || defined(STM32F437xx) || defined(STM32F429xx)|| defined(STM32F439xx) || defined(STM32F469xx) || defined(STM32F479xx)
#define FLASH_SECTOR_TOTAL  24U
#endif /* STM32F427xx || STM32F437xx || STM32F429xx || STM32F439xx || STM32F469xx || STM32F479xx */

Given this comes from the STM32Cube (I think), then I am unsure of the best approach to fix this.

Would it be acceptable to make flash_stm32_page_layout() a weak function such that we could provide our own layout?

void flash_stm32_page_layout(const struct device *dev,                                                                                                                                                            
                             const struct flash_pages_layout **layout,                                                                                                                                            
                             size_t *layout_size)                                                                                                                                                                 
{                                                                                                                                                                                                                 
        ARG_UNUSED(dev);                                                                                                                                                                                          

        *layout = stm32f4_flash_layout;                                                                                                                                                                           
        *layout_size = ARRAY_SIZE(stm32f4_flash_layout);                                                                                                                                                          
} 

Or is there a way to define the flash sector layout in the device tree?

github-actions[bot] commented 4 months ago

Hi @rickp-virscient! We appreciate you submitting your first issue for our open-source project. 🌟

Even though I'm a bot, I can assure you that the whole community is genuinely grateful for your time and effort. 🤖💙

erwango commented 3 months ago

This is indeed an issue coming from the HAL. Usually we request such issues to be fixed in the HAL, then we report the fix in Zephyr HAL. However in this case, I'm pretty sure they won't fix it as is would require too much changes.

So as mention, it has to be fixed directly in Zephyr and stm32f4_flash_layout definition should also take into account actual available flash size and bank organisation. Device tree is an appealing option, but it would also need to take bank organisation into account, which then makes it a bit more hard.

Regarding "weak", I don't see this as a way to go as the issue is valid and should be fixed for all users.

rickp-virscient commented 3 months ago

Thanks @erwango, I agree that a device tree solution would be best. Dual/Single bank does come into play too as you say.

We've gone with using a 'weak' interface for now so we can make progress whilst a proper solution is thrashed out :)

github-actions[bot] commented 1 month ago

This issue has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this issue will automatically be closed in 14 days. Note, that you can always re-open a closed issue at any time.