Hi,
the code looks very good! Nit-picking below.
Please address the comments below and add a commit comment.
1. Make changes.
2. Create commit & patch
git add .
git commit
# write comment
git format-patch HEAD^
=> email patch w/commit comments.
Read openocd/HACKING for more info.
Thanks!
1. Switch to LOG_ERROR
+ struct stm32lx_flash_bank *stm32lx_info;
+ if (CMD_ARGC < 6)
+ {
+ LOG_WARNING("incomplete flash_bank stm32lx configuration");
+ return ERROR_FLASH_BANK_INVALID;
+ }
2. Delete unimplemented code, leave only a comment. Mass erase is
notoriously hard
to test based upon feedback on the list so we want this seriously
tested before we want
to introduce extra code-paths.
+ // Mass erase does not work yet
+ if (0)
+ {
+ if ((first == 0) && (last == bank->num_sectors - 1))
+ {
+ // Erase all if required
+ return stm32lx_mass_erase(bank);
+ }
+ }
+
3. Return error message or at least add comment that this is a
successful no-op always.
+static int stm32lx_protect(struct flash_bank *bank, int set, int first,
+ int last)
+{
+ return ERROR_OK;
+}
+
4. Remove "err:"
+ LOG_ERROR("err: failed to unlock program memory");
+ return retval;
+ }
5. add timeout
stm32lx_wait_until_bsy_clear
6. add memory allocation check.
+ // allocate a buffer to read memory
+ uint8_t *buffer = malloc(buffer_size);
7. reformat to use multiple lines as is done elsewhere in the code.
+static const struct command_registration stm32lx_command_handlers[] =
+{
+{ .name = "stm32lx", .mode = COMMAND_ANY, .help =
+"stm32lx flash command group", .chain =
+stm32lx_exec_command_handlers, },
+COMMAND_REGISTRATION_DONE };
8. Remove these comments, they add no information:
+ // auto probe command
+ .auto_probe = stm32lx_auto_probe,
9. Remove or elaborate on these comments. Perhaps better write a short
paragraph instead of these inline comments. The comments say nothing
that the code doesn't already.
+// prototypes
+static int stm32lx_unlock_pecr(struct flash_bank *bank)
+{
+ // Write key2
+ retval = target_write_u32(target, FLASH_PEKEYR, PEKEY2);
+ // Then unlock the program memory, write key1
+ retval = target_write_u32(target, FLASH_PRGKEYR, PRGKEY1);
9. Just for information. You are here adding two errors, first
target_write_memory()
reports an error, then you amend that information with a bit of
context, which is
OK if that's what you intended.
+ retval = target_read_u32(target, FLASH_OBR, ®32);
+ if (retval != ERROR_OK)
+ {
+ LOG_ERROR("stm32lx_read_rdp: failed to read FLASH_OBR");
10. Remove this error message? Change to LOG_DEBUG()? Certainly remove
"stm32lx_write_rdp:" because LOG_ERROR() adds that info already.
+ retval = stm32lx_unlock_option_byte(bank);
+ if (retval != ERROR_OK)
+ {
+ LOG_ERROR("stm32lx_write_rdp: failed to unlock option byte");
+ return retval;
--
Øyvind Harboe - Can Zylin Consulting help on your project?
US toll free 1-866-980-3434
http://www.zylin.com/
_______________________________________________
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development
