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.44k stars 6.4k forks source link

flash_write() in STM32L0 MCU throws hard fault #33729

Closed azeemshatp closed 3 years ago

azeemshatp commented 3 years ago

Describe the bug There seems to a bug in the flash driver for STM32L0 MCU, possibly in one of these functions: flash_stm32_write_range() or write_value().

There should not be any alignment restrictions on the source address for the function flash_write(). However, this does not work as expected for the STM32L0 MCU.

To Reproduce If you do a flash write using the below code, it throws a hard fault

char data[30];
for(uint8_t i = 0; i < sizeof(data); i++) {data[i]= i;} 
flash_write(flash_dev, address, (uint8_t *)&data[3], FLASH_PAGE_SIZE);

On the otherhand if you write using the source index as multiples of 4 as below, it works fine.

flash_write(flash_dev, address, (uint8_t *)&data[4], FLASH_PAGE_SIZE);

However, this issue does not exist in STM32G4 MCU, so it's only specific to L0, I think.

Expected behavior The flash write needs to happen without any restrictions on source alignment. https://docs.zephyrproject.org/apidoc/latest/group__flash__interface_ga76d7880cc5e18ca40237736d3bd94324.html

Logs and console output With the below logs enabled: CONFIG_FLASH_LOG_LEVEL_ERR=y CONFIG_FLASH_LOG_LEVEL_DBG=y

D: Flash initialized. BS: 4 D: Block 0: bs: 128 count: 1536 D: Disabling write protection D: Disable write protection Booting Zephyr OS build zephyr-v2.5.0-15-g0a630c8332a6 D: Disable write protection D: Erase offset: 98304, len: 128 D: Disable write protection D: Write offset: 98304, len: 128 E: HARD FAULT E: r0/a1: 0x00000000 r1/a2: 0x00018000 r2/a3: 0x00000002 E: r3/a4: 0x1ffec86b r12/ip: 0x00000001 r14/lr: 0x0800abf7 E: xpsr: 0x21000000 E: Faulting instruction address (r15/pc): 0x0800ae26 E: >>> ZEPHYR FATAL ERROR 0: CPU exception on CPU 0 E: Current thread: 0x20001578 (unknown) E: Halting system

Environment (please complete the following information):

Additional context Add any other context about the problem here.

erwango commented 3 years ago

@azeemshatp, flash API actually mentions alignment restrictions that should be respected:

"All flash drivers support a source buffer located either in RAM or SoC flash, without alignment restrictions on the source address. Write size and offset must be multiples of the minimum write block size supported by the driver."

In stm32l0 case: write-block-size = <4>;

From that point of view, I think driver behavior is correct. Can you have a new check ?

azeemshatp commented 3 years ago

@erwango : May be I don't understand it completely. I used the offset address = 0x18000 and the size = FLASH_PAGE_SIZE. So write size and offset are multiples 4 already. But the question was about the source address. As mentioned in the above link "without alignment restrictions on the source address", in my example code, I tried using the source addresses &data[3] and &data[4], which gave me the issue. Also, please note that the same code works fine in the G4 MCU.

erwango commented 3 years ago

@azeemshatp I think the difference of behavior comes from the cores that are different on L0 (M0) and G4(M4). On M0 cores, unaligned accesses are not supported: (cf https://developer.arm.com/documentation/dui0497/a/the-cortex-m0-instruction-set/about-the-instruction-descriptions/address-alignment) "There is no support for unaligned accesses on the Cortex-M0 processor. Any attempt to perform an unaligned memory access operation results in a HardFault exception."

azeemshatp commented 3 years ago

That's interesting. Thanks for the information

erwango commented 3 years ago

@azeemshatp Don't hesitate to close the issue if possible

azeemshatp commented 3 years ago

Here is a solution that works for me in L0. It does a memcpy() to a local buffer before writing to flash. Probably useful to consider in a future fix.

void write_flash_page(char *source_buf, uint32_t dest_address)
{
    // Unlock the FLASH_PECR register access.
    // Then unlock the program memory access.
    flash_unlock();

    /*
    * Erase a page in the Flash at BOOTLOADER_STATUS_STRUCT_ADDR - For STM32L073RZ
    * (1) Set the ERASE and PROG bits in the FLASH_PECR register to enable page erasing
    * (2) Write a 32-bit word value in an address of the selected page to start the erase sequence
    * (3) Wait until the BSY bit is reset in the FLASH_SR register
    * (4) Check the EOP flag in the FLASH_SR register
    * (5) Clear EOP flag by software by writing EOP at 1
    * (6) Reset the ERASE and PROG bits in the FLASH_PECR register to disable the page erase
    * Program and erase control register (FLASH_PECR) - This register can only be written after a
    * good write sequence done in FLASH_PEKEYR, resetting the PELOCK bit.
    */
    FLASH->PECR |= FLASH_PECR_ERASE | FLASH_PECR_PROG;

    *(__IO uint32_t *)dest_address = (uint32_t)0;

    while ((FLASH->SR & FLASH_SR_BSY) != 0) {
        /* For robust implementation, add here time-out management */
    }

    if ((FLASH->SR & FLASH_SR_EOP) != 0) {
        FLASH->SR = FLASH_SR_EOP;
    }
    else {
        /* Manage the error cases */
    }
    FLASH->PECR &= ~(FLASH_PECR_ERASE | FLASH_PECR_PROG);

    /*
    * Write status struct to the flash
    * (1) Set the PROG and FPRG bits in the FLASH_PECR register to enable a half page programming
    * (2) Perform the data write (half-word) at the desired address
    * (3) Wait until the BSY bit is reset in the FLASH_SR register
    * (4) Check the EOP flag in the FLASH_SR register
    * (5) clear it by software by writing it at 1
    * (6) Reset the PROG and FPRG bits to disable programming
    */

    uint32_t data[FLASH_PAGE_SIZE];

    // Copy 1 page of flash
    memcpy(data, (uint32_t *)source_buf, FLASH_PAGE_SIZE);

    for (unsigned int i = 0; i < FLASH_PAGE_SIZE / sizeof(uint32_t); i++) {
        flash_program(dest_address, data[i]);
        dest_address += 4;
    }

    // Locking FLASH_PECR register again
    // Set the PRGLOCK Bit to lock the FLASH Registers access
    SET_BIT(FLASH->PECR, FLASH_PECR_PRGLOCK);
}

bool flash_program(uint32_t address, uint32_t data)
{
    bool status;

    /* Wait for last operation to be completed */
    status = flash_wait_for_last_operation(50);
    if (status == true) {

        /* Set PG bit */
        SET_BIT(FLASH->CR, FLASH_CR_PG);
        /* Program the word */
        *(uint32_t *)address = (uint32_t)data;

        /* Wait for last operation to be completed */
        status = flash_wait_for_last_operation(50);

        /* If the program operation is completed, disable the PG or FSTPG Bit */
        CLEAR_BIT(FLASH->CR, FLASH_CR_PG);
    }

    return status;
}