This is an automated email from Gerrit.
"Tomas Vanek <van...@fbl.cz>" just uploaded a new patch set to Gerrit, which
you can find at https://review.openocd.org/c/openocd/+/8457
-- gerrit
commit a7dd7d24af5952ee6db774d8ea390be167bbb3cf
Author: Tomas Vanek <van...@fbl.cz>
Date: Wed Aug 14 17:43:13 2024 +0200
flash/nor/rp2xxx: minor code improvements
Add error messages and proper error propagation.
Type cleaning.
Use saved chip id.
Cosmetics: separating lines added.
Signed-off-by: Tomas Vanek <van...@fbl.cz>
Change-Id: I151e684e1fbfc9476ec429036caf85f4c9329547
diff --git a/src/flash/nor/rp2xxx.c b/src/flash/nor/rp2xxx.c
index e9a72f9766..1c77f4288e 100644
--- a/src/flash/nor/rp2xxx.c
+++ b/src/flash/nor/rp2xxx.c
@@ -226,11 +226,13 @@ static int rp2040_lookup_rom_symbol(struct target
*target, uint16_t tag, uint16_
err = target_read_u16(target, ptr_to_entry, &entry_tag);
if (err != ERROR_OK)
return err;
+
if (entry_tag == tag) {
/* 16 bit symbol is next */
err = target_read_u16(target, ptr_to_entry + 2,
symbol_out);
if (err != ERROR_OK)
return err;
+
LOG_ROM_SYMBOL_DEBUG(" -> found: 0x%04x", *symbol_out);
return ERROR_OK;
}
@@ -270,10 +272,12 @@ static int rp2350_a0_lookup_symbol(struct target *target,
uint16_t tag, uint16_t
err = target_read_u16(target, ptr_to_entry, &entry_tag);
if (err != ERROR_OK)
return err;
+
if (entry_tag == tag) {
err = target_read_u16(target, ptr_to_entry + 2,
symbol_out);
if (err != ERROR_OK)
return err;
+
LOG_ROM_SYMBOL_DEBUG(" -> found: 0x%04x", *symbol_out);
return ERROR_OK;
}
@@ -296,9 +300,10 @@ static int rp2350_lookup_rom_symbol(struct target *target,
uint32_t ptr_to_entry
while (true) {
uint16_t entry_tag, entry_flags;
- uint32_t err = target_read_u16(target, ptr_to_entry,
&entry_tag);
+ int err = target_read_u16(target, ptr_to_entry, &entry_tag);
if (err != ERROR_OK)
return err;
+
if (entry_tag == 0) {
*symbol_out = 0;
return ERROR_FAIL;
@@ -308,6 +313,7 @@ static int rp2350_lookup_rom_symbol(struct target *target,
uint32_t ptr_to_entry
err = target_read_u16(target, ptr_to_entry, &entry_flags);
if (err != ERROR_OK)
return err;
+
ptr_to_entry += 2;
uint16_t matching_flags = flags & entry_flags;
@@ -331,6 +337,7 @@ static int rp2350_lookup_rom_symbol(struct target *target,
uint32_t ptr_to_entry
err = target_read_u16(target, ptr_to_entry, symbol_out);
if (err != ERROR_OK)
return err;
+
LOG_ROM_SYMBOL_DEBUG(" -> found: 0x%04x", *symbol_out);
return ERROR_OK;
}
@@ -419,18 +426,23 @@ static int rp2xxx_populate_rom_pointer_cache(struct
target *target, struct rp2xx
// From this point are optional functions which do not exist on e.g.
RP2040
// or pre-production RP2350 ROM versions:
+ if (IS_RP2040(priv->id)) {
+ priv->jump_bootrom_reset_state = 0;
+ priv->jump_flash_reset_address_trans = 0;
+ return ERROR_OK;
+ }
err = rp2xxx_lookup_rom_symbol(target, FUNC_BOOTROM_STATE_RESET,
symtype_func, &priv->jump_bootrom_reset_state);
if (err != ERROR_OK) {
priv->jump_bootrom_reset_state = 0;
- LOG_WARNING("Function FUNC_BOOTROM_STATE_RESET not found in
RP2xxx ROM. (probably an RP2040 or an RP2350 A0)");
+ LOG_WARNING("Function FUNC_BOOTROM_STATE_RESET not found in
RP2xxx ROM. (probably an RP2350 A0)");
}
err = rp2xxx_lookup_rom_symbol(target, FUNC_FLASH_RESET_ADDRESS_TRANS,
symtype_func, &priv->jump_flash_reset_address_trans);
if (err != ERROR_OK) {
priv->jump_flash_reset_address_trans = 0;
- LOG_WARNING("Function FUNC_FLASH_RESET_ADDRESS_TRANS not found
in RP2xxx ROM. (probably an RP2040 or an RP2350 A0)");
+ LOG_WARNING("Function FUNC_FLASH_RESET_ADDRESS_TRANS not found
in RP2xxx ROM. (probably an RP2350 A0)");
}
return ERROR_OK;
}
@@ -441,7 +453,7 @@ static int rp2xxx_call_rom_func_batch(struct target
*target, struct rp2xxx_flash
rp2xxx_rom_call_batch_record_t *calls, unsigned int n_calls)
{
// Note +1 is for the null terminator
- uint batch_words = 1 + (ROM_CALL_BATCH_ALGO_SIZE_BYTES +
+ unsigned int batch_words = 1 + (ROM_CALL_BATCH_ALGO_SIZE_BYTES +
n_calls *
sizeof(rp2xxx_rom_call_batch_record_t)
) / sizeof(uint32_t);
@@ -458,7 +470,7 @@ static int rp2xxx_call_rom_func_batch(struct target
*target, struct rp2xxx_flash
for (unsigned int i = 0; i < n_calls; ++i) {
LOG_DEBUG(" func @ %" PRIx32, calls[i].pc);
LOG_DEBUG(" sp = %" PRIx32, calls[i].sp);
- for (int j = 0; j < 4; ++j)
+ for (unsigned int j = 0; j < 4; ++j)
LOG_DEBUG(" a%d = %" PRIx32, j, calls[i].args[j]);
}
LOG_DEBUG("Calling on core \"%s\"", target->cmd_name);
@@ -600,14 +612,22 @@ static int rp2350_init_arm_core0(struct target *target,
struct rp2xxx_flash_bank
// Flash algorithms (and the RCP init stub called by this function) must
// run in the Secure state, so flip the state now before attempting to
// execute any code on the core.
+ int retval;
uint32_t dscsr;
- (void)target_read_u32(target, DCB_DSCSR, &dscsr);
+ retval = target_read_u32(target, DCB_DSCSR, &dscsr);
+ if (retval != ERROR_OK) {
+ LOG_ERROR("RP2350 init ARM core: DSCSR read failed");
+ return retval;
+ }
+
LOG_DEBUG("DSCSR: %08x\n", dscsr);
if (!(dscsr & DSCSR_CDS)) {
LOG_DEBUG("Setting Current Domain Secure in DSCSR\n");
- (void)target_write_u32(target, DCB_DSCSR, (dscsr &
~DSCSR_CDSKEY) | DSCSR_CDS);
- (void)target_read_u32(target, DCB_DSCSR, &dscsr);
- LOG_DEBUG("DSCSR*: %08x\n", dscsr);
+ retval = target_write_u32(target, DCB_DSCSR, (dscsr &
~DSCSR_CDSKEY) | DSCSR_CDS);
+ if (retval != ERROR_OK) {
+ LOG_ERROR("RP2350 init ARM core: DSCSR read failed");
+ return retval;
+ }
}
if (!priv->stack) {
@@ -675,8 +695,7 @@ static int setup_for_raw_flash_cmd(struct target *target,
struct rp2xxx_flash_ba
}
}
- // hacky RP2350 check -- either RISC-V or v8-M
- if (is_arm(target_to_arm(target)) ? target_to_arm(target)->arch ==
ARM_ARCH_V8M : true) {
+ if (IS_RP2350(priv->id)) {
err = rp2350_init_accessctrl(target);
if (err != ERROR_OK) {
LOG_ERROR("Failed to init ACCESSCTRL before ROM call");
@@ -854,16 +873,16 @@ static int rp2xxx_flash_erase(struct flash_bank *bank,
unsigned int first, unsig
return ERROR_TARGET_NOT_HALTED;
}
- uint32_t start_addr = bank->sectors[first].offset;
- uint32_t length = bank->sectors[last].offset + bank->sectors[last].size
- start_addr;
- LOG_DEBUG("erase %d bytes starting at 0x%" PRIx32, length, start_addr);
+ uint32_t offset_start = bank->sectors[first].offset;
+ uint32_t offset_last = bank->sectors[last].offset +
bank->sectors[last].size;
+ uint32_t length = offset_last - offset_start;
+ LOG_DEBUG("erase %d bytes starting at 0x%" PRIx32, length,
offset_start);
int err = setup_for_raw_flash_cmd(target, priv);
if (err != ERROR_OK)
goto cleanup_and_return;
- uint32_t offset_next = bank->sectors[first].offset;
- uint32_t offset_last = bank->sectors[last].offset +
bank->sectors[last].size;
+ uint32_t offset_next = offset_start;
/* Break erase into multiple calls to avoid timeout on large erase.
Choose 128k chunk which has
fairly low ROM call overhead and empirically seems to avoid the
default keep_alive() limit
--
