Skip to content

FlashIAP: Fix problem of programming source buffer not aligned to 4 #6864

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
May 14, 2018
Merged

FlashIAP: Fix problem of programming source buffer not aligned to 4 #6864

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 23 additions & 12 deletions TESTS/mbed_drivers/flashiap/main.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
#include "utest/utest.h"
#include "unity/unity.h"
#include "greentea-client/test_env.h"
#include <algorithm>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What exactly is this importing? This is the first time I've ever seen a system file simply called "algorithm".

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is actually used quite a lot. It imports the std::min and std::max functions.


#include "mbed.h"

Expand All @@ -48,10 +49,10 @@ void flashiap_program_test()
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[page_size];
for (uint32_t i = 0; i < page_size; i++) {
data[i] = test_value;
uint32_t prog_size = std::max(page_size, (uint32_t)8);
uint8_t *data = new uint8_t[prog_size + 2];
for (uint32_t i = 0; i < prog_size + 2; i++) {
data[i] = i;
}

// the one before the last sector in the system
Expand All @@ -61,19 +62,29 @@ void flashiap_program_test()
TEST_ASSERT_EQUAL_INT32(0, ret);


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

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

// check programming of unaligned buffer and size
ret = flash_device.erase(address, sector_size);
TEST_ASSERT_EQUAL_INT32(0, ret);
ret = flash_device.program(data + 2, address, prog_size);
TEST_ASSERT_EQUAL_INT32(0, ret);
ret = flash_device.read(data_flashed, address, prog_size - 1);
TEST_ASSERT_EQUAL_INT32(0, ret);
TEST_ASSERT_EQUAL_UINT8_ARRAY(data + 2, data_flashed, prog_size - 1);

delete[] data;
delete[] data_flashed;

Expand Down
12 changes: 10 additions & 2 deletions drivers/FlashIAP.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,10 +107,18 @@ int FlashIAP::program(const void *buffer, uint32_t addr, uint32_t size)
_mutex->lock();
while (size) {
uint32_t current_sector_size = flash_get_sector_size(&_flash, addr);
bool unaligned_src = (((size_t) buf / sizeof(uint32_t) * sizeof(uint32_t)) != (size_t) buf);
chunk = std::min(current_sector_size - (addr % current_sector_size), size);
if (chunk < page_size) {
// Need to use the internal page buffer in any of these two cases:
// 1. Size is not page aligned
// 2. Source buffer is not aligned to uint32_t. This is not supported by many targets (although
// the pointer they accept is of uint8_t).
if (unaligned_src || (chunk < page_size)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thoughts on just copying to the internal buffer for all program calls?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not very efficient. I suspect that the vast majority of calls will have both buffer and size aligned.

chunk = std::min(chunk, page_size);
memcpy(_page_buf, buf, chunk);
memset(_page_buf + chunk, 0xFF, page_size - chunk);
if (chunk < page_size) {
memset(_page_buf + chunk, 0xFF, page_size - chunk);
}
prog_buf = _page_buf;
prog_size = page_size;
} else {
Expand Down