Skip to content

Commit

Permalink
drivers/flash/mcux: fix flash_read() operation on LPC55S36
Browse files Browse the repository at this point in the history
As other targets in the LPC55xxx series, the LPC55S36 has a Flash
controller that raises ECC errors when reading erased pages directly.
To avoid this, there is special code for this platform that calls the
HAL FLASH_IsFlashAreaReadable() function. However, this in turn calls a
function at an hardcoded address in ROM that _always_ causes an
instruction fault, making the situation worse.

This patch reworks the read operation to use the FLASH_Read() HAL
function for this target to gracefully handle error conditions and
properly emulate accesses to erased pages. Also, the #ifdef conditions
have been converted to 'if (IS_ENABLED(...))' to improve readability.

Signed-off-by: Luca Burelli <l.burelli@arduino.cc>
  • Loading branch information
pillo79 committed Nov 3, 2024
1 parent 185432c commit ec14af1
Showing 1 changed file with 46 additions and 26 deletions.
72 changes: 46 additions & 26 deletions drivers/flash/soc_flash_mcux.c
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ LOG_MODULE_REGISTER(flash_mcux);

#define SOC_NV_FLASH_NODE DT_INST(0, soc_nv_flash)

#if defined(CONFIG_CHECK_BEFORE_READING) && !defined(CONFIG_SOC_LPC55S36)
#if defined(CONFIG_CHECK_BEFORE_READING) && !defined(CONFIG_SOC_LPC55S36)
#define FMC_STATUS_FAIL FLASH_INT_CLR_ENABLE_FAIL_MASK
#define FMC_STATUS_ERR FLASH_INT_CLR_ENABLE_ERR_MASK
#define FMC_STATUS_DONE FLASH_INT_CLR_ENABLE_DONE_MASK
Expand Down Expand Up @@ -123,6 +123,9 @@ static status_t is_area_readable(uint32_t addr, size_t len)

return 0;
}
#else
/* Empty forward definition */
status_t is_area_readable(uint32_t addr, size_t len);
#endif /* CONFIG_CHECK_BEFORE_READING && ! CONFIG_SOC_LPC55S36 */

struct flash_priv {
Expand Down Expand Up @@ -204,36 +207,53 @@ static int flash_mcux_read(const struct device *dev, off_t offset,
*/
addr = offset + priv->pflash_block_base;

#ifdef CONFIG_CHECK_BEFORE_READING
#ifdef CONFIG_SOC_LPC55S36
/* Validates the given address range is loaded in the flash hiding region. */
rc = FLASH_IsFlashAreaReadable(&priv->config, addr, len);
if (rc != kStatus_FLASH_Success) {
rc = -EIO;
} else {
/* Check whether the flash is erased ("len" and "addr" must be word-aligned). */
rc = FLASH_VerifyErase(&priv->config, ((addr + 0x3) & ~0x3), ((len + 0x3) & ~0x3));
if (rc == kStatus_FLASH_Success) {
rc = -ENODATA;
if (IS_ENABLED(CONFIG_CHECK_BEFORE_READING)) {
/*
* Ensure the area is readable, since a direct access may cause faults on erased
* or otherwise unreadable pages. Emulate erased pages, return other errors.
*/
if (IS_ENABLED(CONFIG_SOC_LPC55S36)) {
/* On LPC55S36, use a HAL function to safely copy from Flash. */
rc = FLASH_Read(&priv->config, addr, data, len);
switch (rc) {
case kStatus_FLASH_Success:
rc = 0;
break;
case kStatus_FLASH_EccError:
/* Check id the ECC issue is due to the Flash being erased
* ("addr" and "len" must be word-aligned for this call).
*/
rc = FLASH_VerifyErase(&priv->config, ((addr + 0x3) & ~0x3),
((len + 0x3) & ~0x3));
if (rc == kStatus_FLASH_Success) {

Check notice on line 228 in drivers/flash/soc_flash_mcux.c

View workflow job for this annotation

GitHub Actions / Run compliance checks on patch series (PR)

You may want to run clang-format on this change

drivers/flash/soc_flash_mcux.c:228 - ((len + 0x3) & ~0x3)); + ((len + 0x3) & ~0x3));
rc = -ENODATA;
} else {
rc = -EIO;
}
break;
default:
rc = -EIO;
break;
}
} else {
rc = 0;
/* On all other targets, check if the Flash area is readable.
* If so, copy data from it directly. */

Check warning on line 240 in drivers/flash/soc_flash_mcux.c

View workflow job for this annotation

GitHub Actions / Run compliance checks on patch series (PR)

BLOCK_COMMENT_STYLE

drivers/flash/soc_flash_mcux.c:240 Block comments use a trailing */ on a separate line
rc = is_area_readable(addr, len);
if (!rc) {
memcpy(data, (void *) addr, len);
}

Check notice on line 244 in drivers/flash/soc_flash_mcux.c

View workflow job for this annotation

GitHub Actions / Run compliance checks on patch series (PR)

You may want to run clang-format on this change

drivers/flash/soc_flash_mcux.c:244 - memcpy(data, (void *) addr, len); + memcpy(data, (void *)addr, len);
}
}
#else
rc = is_area_readable(addr, len);
#endif /* CONFIG_SOC_LPC55S36 */
#endif /* CONFIG_CHECK_BEFORE_READING */

if (!rc) {
if (rc == -ENODATA) {
/* Erased area, return dummy data as an erased page. */
memset(data, 0xFF, len);
rc = 0;
}
} else {
/* No safety checks, directly copy the memory mapped data. */
memcpy(data, (void *) addr, len);
}
#ifdef CONFIG_CHECK_BEFORE_READING
else if (rc == -ENODATA) {
/* Erased area, return dummy data as an erased page. */
memset(data, 0xFF, len);
rc = 0;
}
#endif

return rc;
}

Expand Down

0 comments on commit ec14af1

Please sign in to comment.