EEPROM emulation example - bugs

[khevessy]

Describe the set-up
The example was ported to our custom board with STM32H735V and there I discovered the bugs.
IDE used is STM32CubeIDE version 1.19.0.

Describe the bug

1. As mentioned here, there are missing calls to SCB_EnableICache() before exit points in EE_VerifyPageFullWriteVariable().
   

   STM32CubeH7/Projects/NUCLEO-H743ZI/Applications/EEPROM/EEPROM_Emulation/Src/eeprom.c

   Lines 617 to 645 in 8bc1198

   	SCB_DisableICache();
   	/* Check each active page address starting from beginning */
   	while (Address < PageEndAddress)
   	{
   	/* Verify if Address and Address+2 contents are 0xFFFFFFFF */
   	if ((*(__IO uint32_t*)Address) == 0xFFFFFFFF)
   	{
   	/* Set variable data */
   	FlashStatus = HAL_FLASH_Program(FLASH_TYPEPROGRAM_FLASHWORD, Address, ((uint32_t)data32));
   	/* If program operation was failed, a Flash error code is returned */
   	if (FlashStatus != HAL_OK)
   	{
   	return FlashStatus;
   	}
   	/* Set variable virtual address */
   	FlashStatus = HAL_FLASH_Program(FLASH_TYPEPROGRAM_FLASHWORD, Address + 32, ((uint32_t)VirtAddress1));
   	/* Return program operation status */
   	return FlashStatus;
   	}
   	else
   	{
   	/* Next address location */
   	Address = Address + 64;
   	}
   	}
   	SCB_EnableICache();

   
   Instruction cache then may end up disabled forever.

2. EE_VerifyPageFullyErased() - if really the whole sector is to be checked, either increments have to be 2-byte or dereferenced value be uint32_t (four bytes). I prefer the former since the value is checked against ERASED which is uint16_t.
   

   STM32CubeH7/Projects/NUCLEO-H743ZI/Applications/EEPROM/EEPROM_Emulation/Src/eeprom.c

   Lines 341 to 360 in 8bc1198

   	EndAddress = (uint32_t)(Address + (PAGE_SIZE - 4U));
   	/* Check each active page address starting from end */
   	while (Address <= EndAddress)
   	{
   	/* Get the current location content to be compared with virtual address */
   	AddressValue = (*(__IO uint16_t*)Address);
   	/* Compare the read address with the virtual address */
   	if (AddressValue != ERASED)
   	{
   	/* In case variable value is read, reset ReadStatus flag */
   	ReadStatus = 0;
   	break;
   	}
   	/* Next address location */
   	Address = Address + 4;
   	}

   
   My solution is that EndAddress = (uint32_t)(Address + (PAGE_SIZE - 2U)); and increment Address = Address + 2;.

3. EE_VerifyPageFullWriteVariable() does not actually check if all the bytes are set to 0xFFFFFFFF. Since there are two 32-byte writes, actually 16 32-bit words should be checked for 0xFFFFFFFF.
   

   STM32CubeH7/Projects/NUCLEO-H743ZI/Applications/EEPROM/EEPROM_Emulation/Src/eeprom.c

   Lines 622 to 634 in 8bc1198

   	/* Verify if Address and Address+2 contents are 0xFFFFFFFF */
   	if ((*(__IO uint32_t*)Address) == 0xFFFFFFFF)
   	{
   	/* Set variable data */
   	FlashStatus = HAL_FLASH_Program(FLASH_TYPEPROGRAM_FLASHWORD, Address, ((uint32_t)data32));
   	/* If program operation was failed, a Flash error code is returned */
   	if (FlashStatus != HAL_OK)
   	{
   	return FlashStatus;
   	}
   	/* Set variable virtual address */
   	FlashStatus = HAL_FLASH_Program(FLASH_TYPEPROGRAM_FLASHWORD, Address + 32, ((uint32_t)VirtAddress1));

   
   The code actually checks only the first word. My fix was along the lines of
   if (is_flashword_erased(Address) && is_flashword_erased(Address + 32))
   with

static inline int is_flashword_erased(uint32_t addr) {
    for (int i = 0; i < 8; ++i) {
        if (*((__IO uint32_t*)(addr + i*4)) != 0xFFFFFFFFU) return 0;
    }
    return 1;
}

4. This is the gravest one that I think actually caused our problems. It consists of three parts:
   a. Wrong offset in EE_Init() initial check for detecting the variable.
   

   STM32CubeH7/Projects/NUCLEO-H743ZI/Applications/EEPROM/EEPROM_Emulation/Src/eeprom.c

   Line 137 in 8bc1198

   if (( *(__IO uint16_t*)(PAGE0_BASE_ADDRESS + 6)) == VirtAddVarTab[VarIdx])

   
   Where does the value 6 come from? I think that the correct should be 128 - 64 for the header and 64 for the address offset. So actually
   if (( *(__IO uint16_t*)(PAGE0_BASE_ADDRESS + PAGE_HEADER_SIZE + EE_ADDRESS_OFFSET)) == VirtAddVarTab[VarIdx]) where PAGE_HEADER_SIZE is 64 and EE_ADDRESS_OFFSET is 64.
   b. Wrong stopping address in EE_ReadVariable() - the last check goes to the page header.
   

   STM32CubeH7/Projects/NUCLEO-H743ZI/Applications/EEPROM/EEPROM_Emulation/Src/eeprom.c

   Line 398 in 8bc1198

   while (Address > (PageStartAddress + 32))

   . I think that the correct code is
   while (Address >= (PageStartAddress + PAGE_HEADER_SIZE))
   or maybe just > since the first four-word actually contains the variable value.
   c. EE_VerifyPageFullWriteVariable() start address - this is the main problem
   

   STM32CubeH7/Projects/NUCLEO-H743ZI/Applications/EEPROM/EEPROM_Emulation/Src/eeprom.c

   Line 612 in 8bc1198

   Address = (uint32_t)(EEPROM_START_ADDRESS + (uint32_t)(ValidPage * PAGE_SIZE));

   
   The check actually starts at the sector beginning which should contain the page header. Even though it should not be erased if it is a valid page, this ends up colliding with the header after some time. I think that the correct start address should be Address = (uint32_t)(EEPROM_START_ADDRESS + (uint32_t)(ValidPage * PAGE_SIZE) + PAGE_HEADER_SIZE);

How To Reproduce
With intensive writing, I always end up with both pages containing data with the page header inconsistent which in turn results in deleting of one of the pages and missing data. I think that it was caused by the point 4. which caused header invalidation and subsequent data inconsistency. With the fixes the code now seems to be running reliably.

Aditional Context
Again, also relevant to STM32H743I-EVAL. Not sure for NUCLEO-H7A3ZI-Q, maybe all the offsets I introduce should be halved since the writes are half the size.

[TOUNSTM]

ST Internal Reference: 219313

[TOUNSTM]

Hello @khevessy,

Thank you for your report. Regarding point 4 that you mentioned, I am unable to identify any issues with the current implementation. Could you please provide more details or clarify the specific problem you are experiencing?

Best regards,

[khevessy]

Page header has 2 bytes at the beginning (receive data / valid page), then there are 30 zero bytes from the 32 byte write. Since EE_VerifyPageFullWriteVariable() increases the address by 64 when searching the erased words, there are also 32 empty bytes, that's why I say that the header is 64 bytes in total.

4a. IIUC, code tries directly checking the first variable saved in the memory. However, dereferencing PAGE0_BASE_ADDRESS + 6 goes into page header.
4b. Search for the variable to read goes into the page header in the last iteration.
4c. Search for erased address for write of the variable starts at the page beginning which is occupied by the page header. The variable can never be written there.

With only points from 4., I am now unable to reproduce the issue with invalid header, it was probably caused by some combination of the mentioned issues since I was fixing them step by step.