From 4732471b2e122357403fbcaa53fb712db0d8235e Mon Sep 17 00:00:00 2001 From: Tomas Vanek Date: Mon, 18 Jul 2016 22:59:17 +0200 Subject: [PATCH] flash/nor: at91samd protection bits write fix Flash protection set on a device with MANW=1 was lost after reset. Since #2903 the driver honored MANW bit and issued Write Page command just for main flash write. This change adds similar technique to samd_modify_user_row(). Minor code improvements: samd_check_error() returns error code corresponding to error type instead of bool. samd_check_error() does not clear STATUS register if no error bit is set. Eliminated double error check in call sequence samd_issue_nvmctrl_command() folowed by samd_check_error(). Missing error code ERROR_FLASH_PROTECTED added to src/flash/common.h. Change-Id: Icf59ab8803305d0cb3170c8a5089b8f9828b99f8 Signed-off-by: Tomas Vanek Reviewed-on: http://openocd.zylin.com/3550 Tested-by: jenkins Reviewed-by: Andreas Fritiofson --- src/flash/common.h | 1 + src/flash/nor/at91samd.c | 83 +++++++++++++++++++++------------------- 2 files changed, 44 insertions(+), 40 deletions(-) diff --git a/src/flash/common.h b/src/flash/common.h index ce26fccd75..4244f13606 100644 --- a/src/flash/common.h +++ b/src/flash/common.h @@ -44,5 +44,6 @@ bool flash_driver_name_matches(const char *name, const char *expected); #define ERROR_FLASH_SECTOR_NOT_ERASED (-906) #define ERROR_FLASH_BANK_NOT_PROBED (-907) #define ERROR_FLASH_OPER_UNSUPPORTED (-908) +#define ERROR_FLASH_PROTECTED (-909) #endif /* OPENOCD_FLASH_COMMON_H */ diff --git a/src/flash/nor/at91samd.c b/src/flash/nor/at91samd.c index cdc43b8b3e..2673b0eeca 100644 --- a/src/flash/nor/at91samd.c +++ b/src/flash/nor/at91samd.c @@ -423,39 +423,43 @@ static int samd_probe(struct flash_bank *bank) return ERROR_OK; } -static bool samd_check_error(struct target *target) +static int samd_check_error(struct target *target) { - int ret; - bool error; + int ret, ret2; uint16_t status; ret = target_read_u16(target, SAMD_NVMCTRL + SAMD_NVMCTRL_STATUS, &status); if (ret != ERROR_OK) { LOG_ERROR("Can't read NVM status"); - return true; + return ret; } - if (status & 0x001C) { - if (status & (1 << 4)) /* NVME */ - LOG_ERROR("SAMD: NVM Error"); - if (status & (1 << 3)) /* LOCKE */ - LOG_ERROR("SAMD: NVM lock error"); - if (status & (1 << 2)) /* PROGE */ - LOG_ERROR("SAMD: NVM programming error"); + if ((status & 0x001C) == 0) + return ERROR_OK; - error = true; - } else { - error = false; + if (status & (1 << 4)) { /* NVME */ + LOG_ERROR("SAMD: NVM Error"); + ret = ERROR_FLASH_OPERATION_FAILED; + } + + if (status & (1 << 3)) { /* LOCKE */ + LOG_ERROR("SAMD: NVM lock error"); + ret = ERROR_FLASH_PROTECTED; + } + + if (status & (1 << 2)) { /* PROGE */ + LOG_ERROR("SAMD: NVM programming error"); + ret = ERROR_FLASH_OPER_UNSUPPORTED; } /* Clear the error conditions by writing a one to them */ - ret = target_write_u16(target, + ret2 = target_write_u16(target, SAMD_NVMCTRL + SAMD_NVMCTRL_STATUS, status); - if (ret != ERROR_OK) + if (ret2 != ERROR_OK) LOG_ERROR("Can't clear NVM error conditions"); - return error; + return ret; } static int samd_issue_nvmctrl_command(struct target *target, uint16_t cmd) @@ -474,10 +478,7 @@ static int samd_issue_nvmctrl_command(struct target *target, uint16_t cmd) return res; /* Check to see if the NVM command resulted in an error condition. */ - if (samd_check_error(target)) - return ERROR_FAIL; - - return ERROR_OK; + return samd_check_error(target); } static int samd_erase_row(struct target *target, uint32_t address) @@ -531,12 +532,19 @@ static int samd_modify_user_row(struct target *target, uint32_t value, uint8_t startb, uint8_t endb) { int res; + uint32_t nvm_ctrlb; + bool manual_wp = true; if (is_user_row_reserved_bit(startb) || is_user_row_reserved_bit(endb)) { LOG_ERROR("Can't modify bits in the requested range"); return ERROR_FAIL; } + /* Check if we need to do manual page write commands */ + res = target_read_u32(target, SAMD_NVMCTRL + SAMD_NVMCTRL_CTRLB, &nvm_ctrlb); + if (res == ERROR_OK) + manual_wp = (nvm_ctrlb & SAMD_NVM_CTRLB_MANW) != 0; + /* Retrieve the MCU's page size, in bytes. This is also the size of the * entire User Row. */ uint32_t page_size; @@ -559,8 +567,8 @@ static int samd_modify_user_row(struct target *target, uint32_t value, if (!buf) return ERROR_FAIL; - /* Read the user row (comprising one page) by half-words. */ - res = target_read_memory(target, SAMD_USER_ROW, 2, page_size / 2, buf); + /* Read the user row (comprising one page) by words. */ + res = target_read_memory(target, SAMD_USER_ROW, 4, page_size / 4, buf); if (res != ERROR_OK) goto out_user_row; @@ -579,20 +587,18 @@ static int samd_modify_user_row(struct target *target, uint32_t value, /* Modify */ buf_set_u32(buf, startb, endb - startb + 1, value); - /* Write the page buffer back out to the target. A Flash write will be - * triggered automatically. */ + /* Write the page buffer back out to the target. */ res = target_write_memory(target, SAMD_USER_ROW, 4, page_size / 4, buf); if (res != ERROR_OK) goto out_user_row; - if (samd_check_error(target)) { - res = ERROR_FAIL; - goto out_user_row; + if (manual_wp) { + /* Trigger flash write */ + res = samd_issue_nvmctrl_command(target, SAMD_NVM_CMD_WAP); + } else { + res = samd_check_error(target); } - /* Success */ - res = ERROR_OK; - out_user_row: free(buf); @@ -784,18 +790,15 @@ static int samd_write(struct flash_bank *bank, const uint8_t *buffer, * then issue CMD_WP always */ if (manual_wp || pg_offset + 4 * nw < chip->page_size) { res = samd_issue_nvmctrl_command(bank->target, SAMD_NVM_CMD_WP); - if (res != ERROR_OK) { - LOG_ERROR("%s: %d", __func__, __LINE__); - goto free_pb; - } - } + } else { + /* Access through AHB is stalled while flash is being programmed */ + usleep(200); - /* Access through AHB is stalled while flash is being programmed */ - usleep(200); + res = samd_check_error(bank->target); + } - if (samd_check_error(bank->target)) { + if (res != ERROR_OK) { LOG_ERROR("%s: write failed at address 0x%08" PRIx32, __func__, address); - res = ERROR_FAIL; goto free_pb; } -- 2.30.2