flash/nor: update support for TI MSP432 devices

Added fixes for issues found in additional code reviews.

Fixed host Endianness issues with using buffer reads
and writes instead of the *_u32 variants.

Changed code that tried to ID banks by hardcode
bank_number values to use instead the bank base
address. This fixes problems using configurations
with multiple devices.

Note that this replaces Change 4786 which has
been abandoned because of extensive changes to
the code to stop IDing banks by name.  And I
think I really messed up a rebase/merge on the
document file.

Tested on MSP432P401R, MSP432P4111, and MSP432E401Y
Launchpads.

Change-Id: Id05798b3aa78ae5cbe725ee762a164d673ee5767
Signed-off-by: Edward Fewell <efewell@ti.com>
Reviewed-on: http://openocd.zylin.com/5481
Tested-by: jenkins
Reviewed-by: Tomas Vanek <vanekt@fbl.cz>

3 files changed, 127 insertions, 117 deletions

diff --git a/doc/openocd.texi b/doc/openocd.texi
index 8aab1ad9..454ae067 100644
--- a/doc/openocd.texi
+++ b/doc/openocd.texi
@@ -6299,14 +6299,14 @@ if @{ [info exists IMEMORY] && [string equal $IMEMORY true] @} @{
 All versions of the SimpleLink MSP432 microcontrollers from Texas
 Instruments include internal flash. The msp432 flash driver automatically
 recognizes the specific version's flash parameters and autoconfigures itself.
-Main program flash (starting at address 0) is flash bank 0. Information flash
-region on MSP432P4 versions (starting at address 0x200000) is flash bank 1.
+Main program flash starts at address 0. The information flash region on
+MSP432P4 versions starts at address 0x200000.
 
 @example
 flash bank $_FLASHNAME msp432 0 0 0 0 $_TARGETNAME
 @end example
 
-@deffn Command {msp432 mass_erase} [main|all]
+@deffn Command {msp432 mass_erase} bank_id [main|all]
 Performs a complete erase of flash. By default, @command{mass_erase} will erase
 only the main program flash.
 
@@ -6315,7 +6315,7 @@ main program and information flash regions. To also erase the BSL in information
 flash, the user must first use the @command{bsl} command.
 @end deffn
 
-@deffn Command {msp432 bsl} [unlock|lock]
+@deffn Command {msp432 bsl} bank_id [unlock|lock]
 On MSP432P4 versions, @command{bsl} unlocks and locks the bootstrap loader (BSL)
 region in information flash so that flash commands can erase or write the BSL.
 Leave the BSL locked to prevent accidentally corrupting the bootstrap loader.
diff --git a/src/flash/nor/msp432.c b/src/flash/nor/msp432.c
index e9e4be33..95c99b97 100644
--- a/src/flash/nor/msp432.c
+++ b/src/flash/nor/msp432.c
@@ -49,7 +49,8 @@ struct msp432_bank {
 	int family_type;
 	int device_type;
 	uint32_t sector_length;
-	bool probed[2];
+	bool probed_main;
+	bool probed_info;
 	bool unlock_bsl;
 	struct working_area *working_area;
 	struct armv7m_algorithm armv7m_info;
@@ -194,8 +195,7 @@ static int msp432_exec_cmd(struct target *target, struct msp432_algo_params
 		return retval;
 
 	/* Write out command to target memory */
-	retval = target_write_buffer(target, ALGO_FLASH_COMMAND_ADDR,
-				sizeof(command), (uint8_t *)&command);
+	retval = target_write_u32(target, ALGO_FLASH_COMMAND_ADDR, command);
 
 	return retval;
 }
@@ -210,8 +210,7 @@ static int msp432_wait_return_code(struct target *target)
 
 	start_ms = timeval_ms();
 	while ((0 == return_code) || (FLASH_BUSY == return_code)) {
-		retval = target_read_buffer(target, ALGO_RETURN_CODE_ADDR,
-					sizeof(return_code), (uint8_t *)&return_code);
+		retval = target_read_u32(target, ALGO_RETURN_CODE_ADDR, &return_code);
 		if (ERROR_OK != retval)
 			return retval;
 
@@ -253,8 +252,7 @@ static int msp432_wait_inactive(struct target *target, uint32_t buffer)
 
 	start_ms = timeval_ms();
 	while (BUFFER_INACTIVE != status_code) {
-		retval = target_read_buffer(target, status_addr, sizeof(status_code),
-					(uint8_t *)&status_code);
+		retval = target_read_u32(target, status_addr, &status_code);
 		if (ERROR_OK != retval)
 			return retval;
 
@@ -477,15 +475,23 @@ COMMAND_HANDLER(msp432_mass_erase_command)
 	struct flash_bank *bank;
 	struct msp432_bank *msp432_bank;
 	bool all;
+
 	int retval;
 
-	if (0 == CMD_ARGC) {
+	if (1 > CMD_ARGC)
+		return ERROR_COMMAND_SYNTAX_ERROR;
+
+	retval = CALL_COMMAND_HANDLER(flash_command_get_bank, 0, &bank);
+	if (retval != ERROR_OK)
+		return retval;
+
+	if (1 == CMD_ARGC) {
 		all = false;
-	} else if (1 == CMD_ARGC) {
+	} else if (2 == CMD_ARGC) {
 		/* Check argument for how much to erase */
-		if (0 == strcmp(CMD_ARGV[0], "main"))
+		if (0 == strcmp(CMD_ARGV[1], "main"))
 			all = false;
-		else if (0 == strcmp(CMD_ARGV[0], "all"))
+		else if (0 == strcmp(CMD_ARGV[1], "all"))
 			all = true;
 		else
 			return ERROR_COMMAND_SYNTAX_ERROR;
@@ -493,10 +499,6 @@ COMMAND_HANDLER(msp432_mass_erase_command)
 		return ERROR_COMMAND_SYNTAX_ERROR;
 	}
 
-	retval = get_flash_bank_by_num(0, &bank);
-	if (ERROR_OK != retval)
-		return retval;
-
 	msp432_bank = bank->driver_priv;
 
 	if (MSP432E4 == msp432_bank->family_type) {
@@ -513,7 +515,7 @@ COMMAND_HANDLER(msp432_mass_erase_command)
 		LOG_INFO("msp432: Mass erase of flash is complete");
 	} else {
 		LOG_INFO("msp432: Mass erase of %s is complete",
-			all ? "main + info flash" : "main flash");
+			all ? "main + information flash" : "main flash");
 	}
 
 	return ERROR_OK;
@@ -523,13 +525,14 @@ COMMAND_HANDLER(msp432_bsl_command)
 {
 	struct flash_bank *bank;
 	struct msp432_bank *msp432_bank;
+
 	int retval;
 
-	if (1 < CMD_ARGC)
+	if (1 > CMD_ARGC)
 		return ERROR_COMMAND_SYNTAX_ERROR;
 
-	retval = get_flash_bank_by_num(0, &bank);
-	if (ERROR_OK != retval)
+	retval = CALL_COMMAND_HANDLER(flash_command_get_bank, 0, &bank);
+	if (retval != ERROR_OK)
 		return retval;
 
 	msp432_bank = bank->driver_priv;
@@ -539,13 +542,16 @@ COMMAND_HANDLER(msp432_bsl_command)
 		return ERROR_OK;
 	}
 
-	if (1 == CMD_ARGC) {
-		if (0 == strcmp(CMD_ARGV[0], "lock"))
+	if (2 == CMD_ARGC) {
+		if (0 == strcmp(CMD_ARGV[1], "lock"))
 			msp432_bank->unlock_bsl = false;
-		else if (0 == strcmp(CMD_ARGV[0], "unlock"))
+		else if (0 == strcmp(CMD_ARGV[1], "unlock"))
 			msp432_bank->unlock_bsl = true;
 		else
 			return ERROR_COMMAND_SYNTAX_ERROR;
+	} else if (1 != CMD_ARGC) {
+		/* Extra, unknown argument passed in */
+		return ERROR_COMMAND_SYNTAX_ERROR;
 	}
 
 	LOG_INFO("msp432: BSL flash region is currently %slocked",
@@ -561,6 +567,7 @@ FLASH_BANK_COMMAND_HANDLER(msp432_flash_bank_command)
 	if (CMD_ARGC < 6)
 		return ERROR_COMMAND_SYNTAX_ERROR;
 
+	/* Create shared private struct for flash banks */
 	msp432_bank = malloc(sizeof(struct msp432_bank));
 	if (NULL == msp432_bank)
 		return ERROR_FAIL;
@@ -571,14 +578,14 @@ FLASH_BANK_COMMAND_HANDLER(msp432_flash_bank_command)
 	msp432_bank->family_type = MSP432_NO_FAMILY;
 	msp432_bank->device_type = MSP432_NO_TYPE;
 	msp432_bank->sector_length = 0x1000;
-	msp432_bank->probed[0] = false;
-	msp432_bank->probed[1] = false;
+	msp432_bank->probed_main = false;
+	msp432_bank->probed_info = false;
 	msp432_bank->unlock_bsl = false;
 	msp432_bank->working_area = NULL;
 
-	/* Finish initialization of bank 0 (main flash) */
+	/* Finish up initial settings here */
 	bank->driver_priv = msp432_bank;
-	bank->next = NULL;
+	bank->base = FLASH_BASE;
 
 	return ERROR_OK;
 }
@@ -589,6 +596,9 @@ static int msp432_erase(struct flash_bank *bank, int first, int last)
 	struct msp432_bank *msp432_bank = bank->driver_priv;
 	struct msp432_algo_params algo_params;
 
+	bool is_main = FLASH_BASE == bank->base;
+	bool is_info = P4_FLASH_INFO_BASE == bank->base;
+
 	int retval;
 
 	if (TARGET_HALTED != target->state) {
@@ -597,8 +607,7 @@ static int msp432_erase(struct flash_bank *bank, int first, int last)
 	}
 
 	/* Do a mass erase if user requested all sectors of main flash */
-	if ((0 == bank->bank_number) && (first == 0) &&
-		(last == (bank->num_sectors - 1))) {
+	if (is_main && (first == 0) && (last == (bank->num_sectors - 1))) {
 		/* Request mass erase of main flash */
 		return msp432_mass_erase(bank, false);
 	}
@@ -611,7 +620,7 @@ static int msp432_erase(struct flash_bank *bank, int first, int last)
 	msp432_init_params(&algo_params);
 
 	/* Adjust params if this is the info bank */
-	if (1 == bank->bank_number) {
+	if (is_info) {
 		buf_set_u32(algo_params.erase_param, 0, 32, FLASH_ERASE_INFO);
 		/* And flag if BSL is unlocked */
 		if (msp432_bank->unlock_bsl)
@@ -622,11 +631,11 @@ static int msp432_erase(struct flash_bank *bank, int first, int last)
 	for (int i = first; i <= last; i++) {
 
 		/* Skip TVL (read-only) sector of the info bank */
-		if (1 == bank->bank_number && 1 == i)
+		if (is_info && 1 == i)
 			continue;
 
 		/* Skip BSL sectors of info bank if locked */
-		if (1 == bank->bank_number && (2 == i || 3 == i) &&
+		if (is_info && (2 == i || 3 == i) &&
 			!msp432_bank->unlock_bsl)
 			continue;
 
@@ -666,6 +675,8 @@ static int msp432_write(struct flash_bank *bank, const uint8_t *buffer,
 	long long start_ms;
 	long long elapsed_ms;
 
+	bool is_info = P4_FLASH_INFO_BASE == bank->base;
+
 	int retval;
 
 	if (TARGET_HALTED != target->state) {
@@ -679,7 +690,7 @@ static int msp432_write(struct flash_bank *bank, const uint8_t *buffer,
 	 * The BSL region in sectors 2 and 3 of the info flash may be unlocked
 	 * The helper algorithm will hang on attempts to write to TVL
 	 */
-	if (1 == bank->bank_number) {
+	if (is_info) {
 		/* Set read-only start to TVL sector */
 		uint32_t start = 0x1000;
 		/* Set read-only end after BSL region if locked */
@@ -722,7 +733,7 @@ static int msp432_write(struct flash_bank *bank, const uint8_t *buffer,
 	buf_set_u32(algo_params.length, 0, 32, count);
 
 	/* Check if this is the info bank */
-	if (1 == bank->bank_number) {
+	if (is_info) {
 		/* And flag if BSL is unlocked */
 		if (msp432_bank->unlock_bsl)
 			buf_set_u32(algo_params.unlock_bsl, 0, 32, FLASH_UNLOCK_BSL);
@@ -753,8 +764,8 @@ static int msp432_write(struct flash_bank *bank, const uint8_t *buffer,
 		}
 
 		/* Signal the flash helper algorithm that data is ready to flash */
-		retval = target_write_buffer(target, ALGO_BUFFER1_STATUS_ADDR,
-					sizeof(data_ready), (uint8_t *)&data_ready);
+		retval = target_write_u32(target, ALGO_BUFFER1_STATUS_ADDR,
+					data_ready);
 		if (ERROR_OK != retval) {
 			(void)msp432_quit(bank);
 			return ERROR_FLASH_OPERATION_FAILED;
@@ -793,20 +804,23 @@ static int msp432_probe(struct flash_bank *bank)
 	struct target *target = bank->target;
 	struct msp432_bank *msp432_bank = bank->driver_priv;
 
-	char *name;
-
 	uint32_t device_id;
 	uint32_t hardware_rev;
 
-	uint32_t base;
 	uint32_t sector_length;
 	uint32_t size;
 	int num_sectors;
-	int bank_id;
+
+	bool is_main = FLASH_BASE == bank->base;
+	bool is_info = P4_FLASH_INFO_BASE == bank->base;
 
 	int retval;
 
-	bank_id = bank->bank_number;
+	/* Check if this bank has already been successfully probed */
+	if (is_main && msp432_bank->probed_main)
+		return ERROR_OK;
+	if (is_info && msp432_bank->probed_info)
+		return ERROR_OK;
 
 	/* Read the flash size register to determine this is a P4 or not */
 	/* MSP432P4s will return the size of flash.  MSP432E4s will return zero */
@@ -849,63 +863,16 @@ static int msp432_probe(struct flash_bank *bank)
 	msp432_bank->device_type = msp432_device_type(msp432_bank->family_type,
 		msp432_bank->device_id, msp432_bank->hardware_rev);
 
-	/* If not already allocated, create the info bank for MSP432P4 */
-	/* We could not determine it was needed until device was probed */
-	if (MSP432P4 == msp432_bank->family_type) {
-		/* If we've been given bank 1, then this was already done */
-		if (0 == bank_id) {
-			/* And only allocate it if it doesn't exist yet */
-			if (NULL == bank->next) {
-				struct flash_bank *info_bank;
-				info_bank = malloc(sizeof(struct flash_bank));
-				if (NULL == info_bank)
-					return ERROR_FAIL;
-
-				name = malloc(strlen(bank->name)+1);
-				if (NULL == name) {
-					free(info_bank);
-					return ERROR_FAIL;
-				}
-				strcpy(name, bank->name);
-
-				/* Initialize bank 1 (info region) */
-				info_bank->name = name;
-				info_bank->target = bank->target;
-				info_bank->driver = bank->driver;
-				info_bank->driver_priv = bank->driver_priv;
-				info_bank->bank_number = 1;
-				info_bank->base = 0x00200000;
-				info_bank->size = 0;
-				info_bank->chip_width = 0;
-				info_bank->bus_width = 0;
-				info_bank->erased_value = 0xff;
-				info_bank->default_padded_value = 0xff;
-				info_bank->write_start_alignment = 0;
-				info_bank->write_end_alignment = 0;
-				info_bank->minimal_write_gap = FLASH_WRITE_GAP_SECTOR;
-				info_bank->num_sectors = 0;
-				info_bank->sectors = NULL;
-				info_bank->num_prot_blocks = 0;
-				info_bank->prot_blocks = NULL;
-				info_bank->next = NULL;
-
-				/* Enable the new bank */
-				bank->next = info_bank;
-			}
-		}
-	}
-
 	if (MSP432P4 == msp432_bank->family_type) {
 		/* Set up MSP432P4 specific flash parameters */
-		if (0 == bank_id) {
+		if (is_main) {
 			retval = target_read_u32(target, P4_FLASH_MAIN_SIZE_REG, &size);
 			if (ERROR_OK != retval)
 				return retval;
 
-			base = P4_FLASH_MAIN_BASE;
 			sector_length = P4_SECTOR_LENGTH;
 			num_sectors = size / sector_length;
-		} else if (1 == bank_id) {
+		} else if (is_info) {
 			if (msp432_bank->device_type == MSP432P411X ||
 				msp432_bank->device_type == MSP432P411X_GUESS) {
 				/* MSP432P411x has an info size register, use that for size */
@@ -916,19 +883,22 @@ static int msp432_probe(struct flash_bank *bank)
 				/* All other MSP432P401x devices have fixed info region size */
 				size = 0x4000; /* 16 KB info region */
 			}
-			base = P4_FLASH_INFO_BASE;
 			sector_length = P4_SECTOR_LENGTH;
 			num_sectors = size / sector_length;
 		} else {
-			/* Invalid bank number somehow */
+			/* Invalid bank somehow */
 			return ERROR_FAIL;
 		}
 	} else {
 		/* Set up MSP432E4 specific flash parameters */
-		base = E4_FLASH_BASE;
-		size = E4_FLASH_SIZE;
-		sector_length = E4_SECTOR_LENGTH;
-		num_sectors = size / sector_length;
+		if (is_main) {
+			size = E4_FLASH_SIZE;
+			sector_length = E4_SECTOR_LENGTH;
+			num_sectors = size / sector_length;
+		} else {
+			/* Invalid bank somehow */
+			return ERROR_FAIL;
+		}
 	}
 
 	if (NULL != bank->sectors) {
@@ -936,11 +906,12 @@ static int msp432_probe(struct flash_bank *bank)
 		bank->sectors = NULL;
 	}
 
-	bank->sectors = malloc(sizeof(struct flash_sector) * num_sectors);
-	if (NULL == bank->sectors)
-		return ERROR_FAIL;
+	if (num_sectors > 0) {
+		bank->sectors = malloc(sizeof(struct flash_sector) * num_sectors);
+		if (NULL == bank->sectors)
+			return ERROR_FAIL;
+	}
 
-	bank->base = base;
 	bank->size = size;
 	bank->write_start_alignment = 0;
 	bank->write_end_alignment = 0;
@@ -955,7 +926,31 @@ static int msp432_probe(struct flash_bank *bank)
 	}
 
 	/* We've successfully determined the stats on this flash bank */
-	msp432_bank->probed[bank_id] = true;
+	if (is_main)
+		msp432_bank->probed_main = true;
+	if (is_info)
+		msp432_bank->probed_info = true;
+
+	if (is_main && MSP432P4 == msp432_bank->family_type) {
+		/* Create the info flash bank needed by MSP432P4 variants */
+		struct flash_bank *info = calloc(sizeof(struct flash_bank), 1);
+		if (NULL == info)
+			return ERROR_FAIL;
+
+		/* Create a name for the info bank, append "_1" to main name */
+		char *name = malloc(strlen(bank->name) + 3);
+		strcpy(name, bank->name);
+		strcat(name, "_1");
+
+		/* Initialize info bank */
+		info->name = name;
+		info->target = bank->target;
+		info->driver = bank->driver;
+		info->driver_priv = msp432_bank;
+		info->base = P4_FLASH_INFO_BASE;
+
+		flash_bank_add(info);
+	}
 
 	/* If we fall through to here, then all went well */
 
@@ -966,15 +961,17 @@ static int msp432_auto_probe(struct flash_bank *bank)
 {
 	struct msp432_bank *msp432_bank = bank->driver_priv;
 
-	int retval = ERROR_OK;
+	bool is_main = FLASH_BASE == bank->base;
+	bool is_info = P4_FLASH_INFO_BASE == bank->base;
 
-	if (bank->bank_number < 0 || bank->bank_number > 1) {
-		/* Invalid bank number somehow */
-		return ERROR_FAIL;
-	}
+	int retval = ERROR_OK;
 
-	if (!msp432_bank->probed[bank->bank_number])
-		retval = msp432_probe(bank);
+	if (is_main)
+		if (!msp432_bank->probed_main)
+			retval = msp432_probe(bank);
+	if (is_info)
+		if (!msp432_bank->probed_info)
+			retval = msp432_probe(bank);
 
 	return retval;
 }
@@ -1036,12 +1033,21 @@ static int msp432_info(struct flash_bank *bank, char *buf, int buf_size)
 	return ERROR_OK;
 }
 
+static int msp432_protect_check(struct flash_bank *bank)
+{
+	/* Added to suppress warning, not needed for MSP432 flash */
+	return ERROR_OK;
+}
+
 static void msp432_flash_free_driver_priv(struct flash_bank *bank)
 {
+	bool is_main = FLASH_BASE == bank->base;
+
 	/* A single private struct is shared between main and info banks */
-	/* Only free it on the call for main bank (#0) */
-	if ((0 == bank->bank_number) && (NULL != bank->driver_priv))
+	/* Only free it on the call for main bank */
+	if (is_main && (NULL != bank->driver_priv))
 		free(bank->driver_priv);
+
 	/* Forget about the private struct on both main and info banks */
 	bank->driver_priv = NULL;
 }
@@ -1052,14 +1058,14 @@ static const struct command_registration msp432_exec_command_handlers[] = {
 		.handler = msp432_mass_erase_command,
 		.mode = COMMAND_EXEC,
 		.help = "Erase entire flash memory on device.",
-		.usage = "['main' | 'all']",
+		.usage = "bank_id ['main' | 'all']",
 	},
 	{
 		.name = "bsl",
 		.handler = msp432_bsl_command,
 		.mode = COMMAND_EXEC,
 		.help = "Allow BSL to be erased or written by flash commands.",
-		.usage = "['unlock' | 'lock']",
+		.usage = "bank_id ['unlock' | 'lock']",
 	},
 	COMMAND_REGISTRATION_DONE
 };
@@ -1085,6 +1091,7 @@ const struct flash_driver msp432_flash = {
 	.probe = msp432_probe,
 	.auto_probe = msp432_auto_probe,
 	.erase_check = default_flash_blank_check,
+	.protect_check = msp432_protect_check,
 	.info = msp432_info,
 	.free_driver_priv = msp432_flash_free_driver_priv,
 };
diff --git a/src/flash/nor/msp432.h b/src/flash/nor/msp432.h
index ffefa8f4..663393b7 100644
--- a/src/flash/nor/msp432.h
+++ b/src/flash/nor/msp432.h
@@ -34,14 +34,17 @@
 #define MSP432E411Y       7 /* MSP432E401Y device */
 #define MSP432E4X_GUESS   8 /* Assuming it's an MSP432E4x device */
 
+/* Common MSP432 flash parameters */
+#define FLASH_BASE 0x00000000
+
 /* MSP432P4 flash parameters */
-#define P4_FLASH_MAIN_BASE 0x00000000
+#define P4_FLASH_MAIN_BASE FLASH_BASE
 #define P4_FLASH_INFO_BASE 0x00200000
 #define P4_SECTOR_LENGTH   0x1000
 #define P4_ALGO_ENTRY_ADDR 0x01000110
 
 /* MSP432E4 flash paramters */
-#define E4_FLASH_BASE      0x00000000
+#define E4_FLASH_BASE      FLASH_BASE
 #define E4_FLASH_SIZE      0x100000
 #define E4_SECTOR_LENGTH   0x4000
 #define E4_ALGO_ENTRY_ADDR 0x20000110