Merge pull request #13947 from LDong-Arm/erase_algorithm_fix

Fix erase type determination for [Q/O/]BlockDevice::erase()
pull/13973/head
Martin Kojtal 2020-11-26 15:53:03 +00:00 committed by GitHub
commit 61e4b55a22
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 195 additions and 69 deletions

View File

@ -141,11 +141,11 @@ int sfdp_find_addr_region(bd_addr_t offset, const sfdp_hdr_info &sfdp_info);
* @param region Region number
* @param smptbl Information about different erase types
*
* @return Largest erase type
* @return Largest erase type, or -1 if none matches the given address and size
*/
int sfdp_iterate_next_largest_erase_type(uint8_t &bitfield,
int size,
int offset,
int sfdp_iterate_next_largest_erase_type(uint8_t bitfield,
bd_size_t size,
bd_addr_t offset,
int region,
const sfdp_smptbl_info &smptbl);

View File

@ -393,34 +393,31 @@ int sfdp_find_addr_region(bd_addr_t offset, const sfdp_hdr_info &sfdp_info)
}
int sfdp_iterate_next_largest_erase_type(uint8_t &bitfield,
int size,
int offset,
int sfdp_iterate_next_largest_erase_type(uint8_t bitfield,
bd_size_t size,
bd_addr_t offset,
int region,
const sfdp_smptbl_info &smptbl)
{
uint8_t type_mask = SFDP_ERASE_BITMASK_TYPE4;
int largest_erase_type = 0;
int idx;
for (idx = 3; idx >= 0; idx--) {
unsigned int erase_size;
for (int idx = 3; idx >= 0; idx--) {
if (bitfield & type_mask) {
largest_erase_type = idx;
if ((size > (int)(smptbl.erase_type_size_arr[largest_erase_type])) &&
((smptbl.region_high_boundary[region] - offset)
> (uint64_t)(smptbl.erase_type_size_arr[largest_erase_type]))) {
break;
} else {
bitfield &= ~type_mask;
erase_size = smptbl.erase_type_size_arr[idx];
// Criteria:
// * offset is aligned to the type's erase size
// * erase size is no larger than the requested size,
// * erase range does not exceed the region boundary
if ((offset % erase_size == 0) && (size >= erase_size) &&
(offset + erase_size - 1 <= smptbl.region_high_boundary[region])) {
return idx;
}
}
type_mask = type_mask >> 1;
}
if (idx == -1) {
tr_error("No erase type was found for current region addr");
}
return largest_erase_type;
tr_error("No erase type was found for current region addr");
return -1;
}
int sfdp_detect_device_density(uint8_t *bptbl_ptr, sfdp_bptbl_info &bptbl_info)

View File

@ -0,0 +1,101 @@
/* Copyright (c) 2020 ARM Limited
* SPDX-License-Identifier: Apache-2.0
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
#include "gtest/gtest.h"
#include "gmock/gmock.h"
#include "drivers/internal/SFDP.h"
class TestSFDP : public testing::Test {
protected:
struct mbed::sfdp_smptbl_info smptbl;
/**
* Construct Mbed OS SFDP info.
* Normally this is parsed from the flash-chips's
* raw SFDP table bytes, but for unit test we construct
* SFDP info manually
*/
virtual void SetUp()
{
// The mock flash supports 4KB, 32KB and 64KB erase types
smptbl.erase_type_size_arr[0] = 4 * 1024;
smptbl.erase_type_size_arr[1] = 32 * 1024;
smptbl.erase_type_size_arr[2] = 64 * 1024;
// The mock flash has three regions, with address ranges:
// * 0 to 64KB - 1B
// * 64KB to 256KB - 1B
// * 256KB to 1024KB - 1B
smptbl.region_high_boundary[0] = 64 * 1024 - 1;
smptbl.region_high_boundary[1] = 256 * 1024 - 1;
smptbl.region_high_boundary[2] = 1024 * 1024 - 1;
// Bitfields indicating which regions support which erase types
smptbl.region_erase_types_bitfld[0] = 0b0001; // 4KB only
smptbl.region_erase_types_bitfld[1] = 0b0111; // 64KB, 32KB, 4KB
smptbl.region_erase_types_bitfld[2] = 0b0110; // 64KB, 32KB
}
};
/**
* Test if sfdp_iterate_next_largest_erase_type() returns the most
* optimal erase type, whose erase size is as large as possible
* while being compatible with both alignment and size requirements.
*/
TEST_F(TestSFDP, TestEraseTypeAlgorithm)
{
// Erase 104KB starting at 92KB
int address = 92 * 1024;
int size = 104 * 1024;
int region = 1;
int type;
// Expected outcome:
// * The starting position 92KB is 4KB-aligned
// * The next position 96KB (92KB + 4KB) is 32KB-aligned
// * The next position 128KB (96KB + 32KB) is 64KB-aligned
// * At the final position 192KB (128KB + 64KB), we only
// have 4KB (104KB - 4KB - 32KB - 64KB) remaining to erase
int expected_erase_KBs[] = {4, 32, 64, 4};
for (int i = 0; i < sizeof(expected_erase_KBs) / sizeof(int); i++) {
type = sfdp_iterate_next_largest_erase_type(
smptbl.region_erase_types_bitfld[region],
size,
address,
region,
smptbl);
int erase_size = smptbl.erase_type_size_arr[type];
EXPECT_EQ(erase_size, expected_erase_KBs[i] * 1024);
address += erase_size;
size -= erase_size;
}
EXPECT_EQ(size, 0); // All erased
// Test unaligned erase
// Region 2 only allows at least 32KB-aligned erases
address = (512 + 16) * 1024;
size = 64 * 1024;
region = 2;
type = sfdp_iterate_next_largest_erase_type(
smptbl.region_erase_types_bitfld[region],
size,
address,
region,
smptbl);
EXPECT_EQ(type, -1); // Invalid erase
}

View File

@ -0,0 +1,20 @@
####################
# UNIT TESTS
####################
set(TEST_SUITE_NAME "SFDP")
# Source files
set(unittest-sources
../drivers/source/SFDP.cpp
)
# Test files
set(unittest-test-sources
../drivers/tests/UNITTESTS/SFDP/test_sfdp.cpp
stubs/mbed_assert_stub.cpp
)
set(unittest-test-flags
-DDEVICE_SPI
)

View File

@ -31,6 +31,7 @@
#include "mbed_trace.h"
#define TRACE_GROUP "OSPIF"
using namespace std::chrono;
using namespace mbed;
/* Default OSPIF Parameters */
@ -409,7 +410,7 @@ int OSPIFBlockDevice::program(const void *buffer, bd_addr_t addr, bd_size_t size
uint32_t chunk = 0;
bd_size_t written_bytes = 0;
tr_debug("Program - Buff: 0x%lxh, addr: %llu, size: %llu", (uint32_t)buffer, addr, size);
tr_debug("Program - Buff: %p, addr: %llu, size: %llu", buffer, addr, size);
while (size > 0) {
// Write on _page_size_bytes boundaries (Default 256 bytes a page)
@ -457,13 +458,10 @@ exit_point:
return status;
}
int OSPIFBlockDevice::erase(bd_addr_t addr, bd_size_t in_size)
int OSPIFBlockDevice::erase(bd_addr_t addr, bd_size_t size)
{
int type = 0;
uint32_t offset = 0;
uint32_t chunk = 4096;
ospi_inst_t cur_erase_inst = OSPI_NO_INST;
int size = (int)in_size;
bool erase_failed = false;
int status = OSPIF_BD_ERROR_OK;
// Find region of erased address
@ -471,14 +469,14 @@ int OSPIFBlockDevice::erase(bd_addr_t addr, bd_size_t in_size)
// Erase Types of selected region
uint8_t bitfield = _sfdp_info.smptbl.region_erase_types_bitfld[region];
tr_debug("Erase - addr: %llu, in_size: %llu", addr, in_size);
tr_debug("Erase - addr: %llu, size: %llu", addr, size);
if ((addr + in_size) > _sfdp_info.bptbl.device_size_bytes) {
if ((addr + size) > _sfdp_info.bptbl.device_size_bytes) {
tr_error("Erase exceeds flash device size");
return OSPIF_BD_ERROR_INVALID_ERASE_PARAMS;
}
if (((addr % get_erase_size(addr)) != 0) || (((addr + in_size) % get_erase_size(addr + in_size - 1)) != 0)) {
if (((addr % get_erase_size(addr)) != 0) || (((addr + size) % get_erase_size(addr + size - 1)) != 0)) {
tr_error("Invalid erase - unaligned address and size");
return OSPIF_BD_ERROR_INVALID_ERASE_PARAMS;
}
@ -500,11 +498,16 @@ int OSPIFBlockDevice::erase(bd_addr_t addr, bd_size_t in_size)
cur_erase_inst = _sfdp_info.bptbl.legacy_erase_instruction;
eu_size = OSPIF_DEFAULT_SE_SIZE;
}
offset = addr % eu_size;
chunk = ((offset + size) < eu_size) ? size : (eu_size - offset);
tr_debug("Erase - addr: %llu, size:%d, Inst: 0x%xh, chunk: %lu ",
addr, size, cur_erase_inst, chunk);
if (addr % eu_size != 0 || addr + size < eu_size) {
// Should not happen if the erase table parsing
// and alignment checks were performed correctly
tr_error("internal error: address %llu not aligned to erase size %u (type %d)",
addr, eu_size, type);
}
tr_debug("Erase - addr: %llu, size:%llu, Inst: 0x%xh, erase size: %u ",
addr, size, cur_erase_inst, eu_size);
tr_debug("Erase - Region: %d, Type:%d ",
region, type);
@ -524,8 +527,8 @@ int OSPIFBlockDevice::erase(bd_addr_t addr, bd_size_t in_size)
goto exit_point;
}
addr += chunk;
size -= chunk;
addr += eu_size;
size -= eu_size;
if ((size > 0) && (addr > _sfdp_info.smptbl.region_high_boundary[region])) {
// erase crossed to next region
@ -1517,7 +1520,7 @@ bool OSPIFBlockDevice::_is_mem_ready()
bool mem_ready = true;
do {
rtos::ThisThread::sleep_for(1);
rtos::ThisThread::sleep_for(1ms);
retries++;
//Read Status Register 1 from device, the number of read byte need to be even in octa flash DOPI mode
if (OSPI_STATUS_OK != _ospi_send_general_command(OSPIF_INST_RSR1, OSPI_NO_ADDRESS_COMMAND,
@ -1664,7 +1667,7 @@ ospi_status_t OSPIFBlockDevice::_ospi_send_general_command(ospi_inst_t instructi
(instruction == OSPIF_INST_RDCR2) || (instruction == OSPIF_INST_RDCR)) {
_ospi.configure_format(_inst_width, _inst_size, _address_width, _address_size, OSPI_CFG_BUS_SINGLE, 0, _data_width, _dummy_cycles);
addr = 0;
} else if ((instruction == OSPIF_INST_WSR1)) {
} else if (instruction == OSPIF_INST_WSR1) {
addr = 0;
}
}

View File

@ -338,7 +338,7 @@ int QSPIFBlockDevice::program(const void *buffer, bd_addr_t addr, bd_size_t size
uint32_t chunk = 0;
bd_size_t written_bytes = 0;
tr_debug("Program - Buff: 0x%lxh, addr: %llu, size: %llu", (uint32_t)buffer, addr, size);
tr_debug("Program - Buff: %p, addr: %llu, size: %llu", buffer, addr, size);
while (size > 0) {
// Write on _page_size_bytes boundaries (Default 256 bytes a page)
@ -385,13 +385,10 @@ exit_point:
return status;
}
int QSPIFBlockDevice::erase(bd_addr_t addr, bd_size_t in_size)
int QSPIFBlockDevice::erase(bd_addr_t addr, bd_size_t size)
{
int type = 0;
uint32_t offset = 0;
uint32_t chunk = 4096;
qspi_inst_t cur_erase_inst = QSPI_NO_INST;
int size = (int)in_size;
bool erase_failed = false;
int status = QSPIF_BD_ERROR_OK;
// Find region of erased address
@ -399,14 +396,14 @@ int QSPIFBlockDevice::erase(bd_addr_t addr, bd_size_t in_size)
// Erase Types of selected region
uint8_t bitfield = _sfdp_info.smptbl.region_erase_types_bitfld[region];
tr_debug("Erase - addr: %llu, in_size: %llu", addr, in_size);
tr_debug("Erase - addr: %llu, size: %llu", addr, size);
if ((addr + in_size) > _sfdp_info.bptbl.device_size_bytes) {
if ((addr + size) > _sfdp_info.bptbl.device_size_bytes) {
tr_error("Erase exceeds flash device size");
return QSPIF_BD_ERROR_INVALID_ERASE_PARAMS;
}
if (((addr % get_erase_size(addr)) != 0) || (((addr + in_size) % get_erase_size(addr + in_size - 1)) != 0)) {
if (((addr % get_erase_size(addr)) != 0) || (((addr + size) % get_erase_size(addr + size - 1)) != 0)) {
tr_error("Invalid erase - unaligned address and size");
return QSPIF_BD_ERROR_INVALID_ERASE_PARAMS;
}
@ -417,7 +414,7 @@ int QSPIFBlockDevice::erase(bd_addr_t addr, bd_size_t in_size)
if (_sfdp_info.bptbl.legacy_erase_instruction == QSPI_NO_INST) {
// Iterate to find next largest erase type that is a) supported by region, and b) smaller than size.
// Find the matching instruction and erase size chunk for that type.
type = sfdp_iterate_next_largest_erase_type(bitfield, size, (int)addr,
type = sfdp_iterate_next_largest_erase_type(bitfield, size, addr,
region,
_sfdp_info.smptbl);
cur_erase_inst = _sfdp_info.smptbl.erase_type_inst_arr[type];
@ -427,11 +424,16 @@ int QSPIFBlockDevice::erase(bd_addr_t addr, bd_size_t in_size)
cur_erase_inst = _sfdp_info.bptbl.legacy_erase_instruction;
eu_size = QSPIF_DEFAULT_SE_SIZE;
}
offset = addr % eu_size;
chunk = ((offset + size) < eu_size) ? size : (eu_size - offset);
tr_debug("Erase - addr: %llu, size:%d, Inst: 0x%xh, chunk: %lu ",
addr, size, cur_erase_inst, chunk);
if (addr % eu_size != 0 || addr + size < eu_size) {
// Should not happen if the erase table parsing
// and alignment checks were performed correctly
tr_error("internal error: address %llu not aligned to erase size %u (type %d)",
addr, eu_size, type);
}
tr_debug("Erase - addr: %llu, size:%llu, Inst: 0x%xh, erase size: %u",
addr, size, cur_erase_inst, eu_size);
tr_debug("Erase - Region: %d, Type:%d ",
region, type);
@ -451,8 +453,8 @@ int QSPIFBlockDevice::erase(bd_addr_t addr, bd_size_t in_size)
goto exit_point;
}
addr += chunk;
size -= chunk;
addr += eu_size;
size -= eu_size;
if ((size > 0) && (addr > _sfdp_info.smptbl.region_high_boundary[region])) {
// erase crossed to next region

View File

@ -25,6 +25,7 @@
#include "mbed_trace.h"
#define TRACE_GROUP "SPIF"
using namespace std::chrono;
using namespace mbed;
/* Default SPIF Parameters */
@ -298,17 +299,15 @@ exit_point:
return status;
}
int SPIFBlockDevice::erase(bd_addr_t addr, bd_size_t in_size)
int SPIFBlockDevice::erase(bd_addr_t addr, bd_size_t size)
{
if (!_is_initialized) {
return BD_ERROR_DEVICE_ERROR;
}
int type = 0;
uint32_t offset = 0;
uint32_t chunk = 4096;
int cur_erase_inst = _erase_instruction;
int size = (int)in_size;
unsigned int curr_erase_size = 0;
bool erase_failed = false;
int status = SPIF_BD_ERROR_OK;
// Find region of erased address
@ -320,14 +319,14 @@ int SPIFBlockDevice::erase(bd_addr_t addr, bd_size_t in_size)
// Erase Types of selected region
uint8_t bitfield = _sfdp_info.smptbl.region_erase_types_bitfld[region];
tr_debug("erase - addr: %llu, in_size: %llu", addr, in_size);
tr_debug("erase - addr: %llu, size: %llu", addr, size);
if ((addr + in_size) > _sfdp_info.bptbl.device_size_bytes) {
if ((addr + size) > _sfdp_info.bptbl.device_size_bytes) {
tr_error("erase exceeds flash device size");
return SPIF_BD_ERROR_INVALID_ERASE_PARAMS;
}
if (((addr % get_erase_size(addr)) != 0) || (((addr + in_size) % get_erase_size(addr + in_size - 1)) != 0)) {
if (((addr % get_erase_size(addr)) != 0) || (((addr + size) % get_erase_size(addr + size - 1)) != 0)) {
tr_error("invalid erase - unaligned address and size");
return SPIF_BD_ERROR_INVALID_ERASE_PARAMS;
}
@ -337,14 +336,18 @@ int SPIFBlockDevice::erase(bd_addr_t addr, bd_size_t in_size)
// iterate to find next Largest erase type ( a. supported by region, b. smaller than size)
// find the matching instruction and erase size chunk for that type.
type = sfdp_iterate_next_largest_erase_type(bitfield, size, (unsigned int)addr, region, _sfdp_info.smptbl);
type = sfdp_iterate_next_largest_erase_type(bitfield, size, addr, region, _sfdp_info.smptbl);
cur_erase_inst = _sfdp_info.smptbl.erase_type_inst_arr[type];
offset = addr % _sfdp_info.smptbl.erase_type_size_arr[type];
chunk = ((offset + size) < _sfdp_info.smptbl.erase_type_size_arr[type]) ?
size : (_sfdp_info.smptbl.erase_type_size_arr[type] - offset);
curr_erase_size = _sfdp_info.smptbl.erase_type_size_arr[type];
if (addr % curr_erase_size != 0 || addr + size < curr_erase_size) {
// Should not happen if the erase table parsing
// and alignment checks were performed correctly
tr_error("internal error: address %llu not aligned to erase size %u (type %d)",
addr, curr_erase_size, type);
}
tr_debug("erase - addr: %llu, size:%d, Inst: 0x%xh, chunk: %" PRIu32 " , ",
addr, size, cur_erase_inst, chunk);
tr_debug("erase - addr: %llu, size:%llu, Inst: 0x%xh, erase size: %u",
addr, size, cur_erase_inst, curr_erase_size);
tr_debug("erase - Region: %d, Type:%d",
region, type);
@ -359,8 +362,8 @@ int SPIFBlockDevice::erase(bd_addr_t addr, bd_size_t in_size)
_spi_send_erase_command(cur_erase_inst, addr, size);
addr += chunk;
size -= chunk;
addr += curr_erase_size;
size -= curr_erase_size;
if ((size > 0) && (addr > _sfdp_info.smptbl.region_high_boundary[region])) {
// erase crossed to next region
@ -693,7 +696,7 @@ bool SPIFBlockDevice::_is_mem_ready()
bool mem_ready = true;
do {
rtos::ThisThread::sleep_for(1);
rtos::ThisThread::sleep_for(1ms);
retries++;
//Read the Status Register from device
if (SPIF_BD_ERROR_OK != _spi_send_general_command(SPIF_RDSR, SPI_NO_ADDRESS_COMMAND, NULL, 0, status_value,