Flash programming in ARCH_PRO board doesn't work with size > page_size

[davidsaada]

Description

• Type: Bug
• Priority: Major

---

Bug

Target
ARCH_PRO

Toolchain:
Any

Expected behavior
Program flash with data, whose size is greater than one flash page, should work, in a sense that read data after programming should be identical to the programmed data.

Actual behavior
When programming the data, read data from the second page shows blanks (0xFF) instead of the actual programmed data.

Steps to reproduce
Following is a modified version of the FlashIAP programming test. Changed the program chunks from one page to two pages. Then the test asserts when comparing the read data and programmed data in position 1024 (start of the second page in ARCH_PRO).

void flashiap_program_test()
{
    FlashIAP flash_device;
    uint32_t ret = flash_device.init();
    TEST_ASSERT_EQUAL_INT32(0, ret);

    // get the last sector size (flash size - 1)
    uint32_t sector_size = flash_device.get_sector_size(flash_device.get_flash_start() + flash_device.get_flash_size() - 1UL);
    uint32_t page_size = flash_device.get_page_size();
    uint32_t program_size = 2 * page_size;
    TEST_ASSERT_NOT_EQUAL(0, sector_size);
    TEST_ASSERT_NOT_EQUAL(0, page_size);
    TEST_ASSERT_TRUE(sector_size % page_size == 0);
    const uint8_t test_value = 0xCE;
    uint8_t *data = new uint8_t[program_size];
    for (uint32_t i = 0; i < program_size; i++) {
        data[i] = test_value;
    }

    // the one before the last sector in the system
    uint32_t address = (flash_device.get_flash_start() + flash_device.get_flash_size()) - (sector_size);
    TEST_ASSERT_TRUE(address != 0UL);
    ret = flash_device.erase(address, sector_size);
    TEST_ASSERT_EQUAL_INT32(0, ret);

    for (uint32_t i = 0; i < sector_size / program_size; i++) {
        uint32_t page_addr = address + i * program_size;
        ret = flash_device.program(data, page_addr, program_size);
        TEST_ASSERT_EQUAL_INT32(0, ret);
    }

    uint8_t *data_flashed = new uint8_t[program_size];
    for (uint32_t i = 0; i < sector_size / program_size; i++) {
        uint32_t page_addr = address + i * program_size;
        ret = flash_device.read(data_flashed, page_addr, program_size);
        TEST_ASSERT_EQUAL_INT32(0, ret);
        TEST_ASSERT_EQUAL_UINT8_ARRAY(data, data_flashed, program_size);
    }
    delete[] data;
    delete[] data_flashed;

    ret = flash_device.deinit();
    TEST_ASSERT_EQUAL_INT32(0, ret);
}

Replace the current test with this code and run the following (from the mbed-os directory):

mbed test -t ARM -m ARCH_PRO -v  -n tests-mbed_drivers-flashiap

Additional info

Note that the problem doesn't seem to be in the FlashIAP driver, but in the lower layers (HAL?) relevant for this specific board. This test works fine on other boards.

[0xc0170]

@toyowata any hints for this target?

[toyowata]

@0xc0170 @davidsaada
This is because the flash_program_page() in target code expects to program just a page size (=1024 bytes):

https://github.com/ARMmbed/mbed-os/blob/master/targets/TARGET_NXP/TARGET_LPC176X/device/flash_api.c#L139

In this test case, only 1st 1024 bytes are written and other data is ignored.

Please note that the program size should be 256/512/1024/4096 by device spec below.

[davidsaada]

@toyowata This is problematic. FlashIAP driver allows programming of data if it is aligned to the device page size, meaning that it allows programming of multiple page sizes. The only board that failed the shown test was the ARCH_PRO. Now, if I follow the logic implied by your answer, it means that FlashIAP should only write in flash page units. This is a change to the FlashIAP driver imposed by the behavior of this single board. Just to add that some boards have page size of 1, meaning that the driver will need to write byte by byte, following this logic.

[0xc0170]

It should be possible to write more.

@toyowata Shouldn't we rewrite the HAL driver? It currently writes 1024 as I read your code reference. If we do it in a loop, we should be able to write multiple pages, like other targets do?
We allocate on the heap the size - seems to be also wrong - if we request to write a lot of data we might run out of memory? I would expect to alloc the page size buffer : copy data, flash one page, copy, flash another in the loop until all done?

illustrate the code I was thinking about:

program page()
{
    allocate_(buffer, 1024);
    while (size) {
        copy_data_to_heap();
        IAP_Call (&IAP.cmd, &IAP.stat);// Call IAP Command
    }
    free(buffer);
}

[toyowata]

@davidsaada @0xc0170

I agreed with you both. I think the original code was written for flash driver of the MDK-ARM and we simply re-used it. We should fix the target code.

[toyowata]

@davidsaada @0xc0170

I raised the PR here. Please review.
#6413