Merge pull request #867 from UweBonnes/clean-static-from_probe

Clean static from probe

Some target probe functions used static RAM variables. This will fail if several of the same targets are in the JTAG/SWD chain and unnecessary remove RAM form the RAM pool if the target is not used. If the target needs more than one word, it allocated priv_storage is used. For a single needed word, one word has been added to the target structure as a union to allow decent naming.

src/target/efm32.c

@@ -584,7 +584,10 @@ static efm32_device_t const * efm32_get_device(size_t index)
 /**
  * Probe
  */
-static char efm32_variant_string[60];
+struct efm32_priv_s {
+	char efm32_variant_string[60];
+};
+
 bool efm32_probe(target *t)
 {
 	uint8_t di_version = 1;
@@ -636,13 +639,17 @@ bool efm32_probe(target *t)
 	uint32_t ram_size   = ram_kib   * 0x400;
 	uint32_t flash_page_size = device->flash_page_size;
 
-	snprintf(efm32_variant_string, sizeof(efm32_variant_string), "%c\b%c\b%s %d F%d %s",
+	struct efm32_priv_s *priv_storage = calloc(1, sizeof(*priv_storage));
+	t->target_storage = (void*)priv_storage;
+
+	snprintf(priv_storage->efm32_variant_string,
+			 sizeof(priv_storage->efm32_variant_string), "%c\b%c\b%s %d F%d %s",
 			di_version + 48, (uint8_t)device_index + 32,
 			device->name, part_number, flash_kib, device->description);
 
 	/* Setup Target */
 	t->target_options |= CORTEXM_TOPT_INHIBIT_SRST;
-	t->driver = efm32_variant_string;
+	t->driver = priv_storage->efm32_variant_string;
 	tc_printf(t, "flash size %d page size %d\n", flash_size, flash_page_size);
 	target_add_ram (t, SRAM_BASE, ram_size);
 	efm32_add_flash(t, 0x00000000, flash_size, flash_page_size);
@@ -980,7 +987,10 @@ static bool nop_function(void)
 /**
  * AAP Probe
  */
-char aap_driver_string[42];
+struct efm32_aap_priv_s {
+       char aap_driver_string[42];
+};
+
 void efm32_aap_probe(ADIv5_AP_t *ap)
 {
 	if ((ap->idr & EFM32_APP_IDR_MASK) == EFM32_AAP_IDR) {
@@ -1004,10 +1014,11 @@ void efm32_aap_probe(ADIv5_AP_t *ap)
 	/* Read status */
 	DEBUG_INFO("EFM32: AAP STATUS=%08"PRIx32"\n", adiv5_ap_read(ap, AAP_STATUS));
 
-	sprintf(aap_driver_string,
+	struct efm32_aap_priv_s *priv_storage = calloc(1, sizeof(*priv_storage));
+	sprintf(priv_storage->aap_driver_string,
 			"EFM32 Authentication Access Port rev.%d",
 			aap_revision);
-	t->driver = aap_driver_string;
+	t->driver = priv_storage->aap_driver_string;
 	t->attach = (void*)nop_function;
 	t->detach = (void*)nop_function;
 	t->check_error = (void*)nop_function;

src/target/kinetis.c

@@ -74,7 +74,6 @@
 #define K64_WRITE_LEN 8
 
 static bool kinetis_cmd_unsafe(target *t, int argc, char *argv[]);
-static bool unsafe_enabled;
 
 const struct command_s kinetis_cmd_list[] = {
 	{"unsafe", (cmd_handler)kinetis_cmd_unsafe, "Allow programming security byte (enable|disable)"},
@@ -85,9 +84,9 @@ static bool kinetis_cmd_unsafe(target *t, int argc, char *argv[])
 {
 	if (argc == 1) {
 		tc_printf(t, "Allow programming security byte: %s\n",
-			  unsafe_enabled ? "enabled" : "disabled");
+			  t->unsafe_enabled ? "enabled" : "disabled");
 	} else {
-		parse_enable_or_disable(argv[1], &unsafe_enabled);
+		parse_enable_or_disable(argv[1], &t->unsafe_enabled);
 	}
 	return true;
 }
@@ -336,7 +335,7 @@ bool kinetis_probe(target *t)
 	default:
 		return false;
 	}
-	unsafe_enabled = false;
+	t->unsafe_enabled = false;
 	target_add_commands(t, kinetis_cmd_list, t->driver);
 	return true;
 }
@@ -403,7 +402,7 @@ static int kl_gen_flash_write(struct target_flash *f,
 	struct kinetis_flash *kf = (struct kinetis_flash *)f;
 
 	/* Ensure we don't write something horrible over the security byte */
-	if (!unsafe_enabled &&
+	if (!f->t->unsafe_enabled &&
 	    (dest <= FLASH_SECURITY_BYTE_ADDRESS) &&
 	    ((dest + len) > FLASH_SECURITY_BYTE_ADDRESS)) {
 		((uint8_t*)src)[FLASH_SECURITY_BYTE_ADDRESS - dest] =
@@ -437,7 +436,7 @@ static int kl_gen_flash_done(struct target_flash *f)
 {
 	struct kinetis_flash *kf = (struct kinetis_flash *)f;
 
-	if (unsafe_enabled)
+	if (f->t->unsafe_enabled)
 		return 0;
 
 	if (target_mem_read8(f->t, FLASH_SECURITY_BYTE_ADDRESS) ==
@@ -527,13 +526,12 @@ void kinetis_mdm_probe(ADIv5_AP_t *ap)
 
 /* This is needed as a separate command, as there's no way to  *
  * tell a KE04 from other kinetis in kinetis_mdm_probe()       */
-static bool ke04_mode = false;
 static bool kinetis_mdm_cmd_ke04_mode(target *t, int argc, const char **argv)
 {
 	(void)argc;
 	(void)argv;
 	/* Set a flag to ignore part of the status and assert reset */
-	ke04_mode = true;
+	t->ke04_mode = true;
 	tc_printf(t, "Mass erase for KE04 now allowed\n");
 	return true;
 }
@@ -544,7 +542,7 @@ static bool kinetis_mdm_cmd_erase_mass(target *t, int argc, const char **argv)
 	ADIv5_AP_t *ap = t->priv;
 
 	/* Keep the MCU in reset as stated in KL25PxxM48SF0RM */
-	if(ke04_mode)
+	if(t->ke04_mode)
 		adiv5_ap_write(ap, MDM_CONTROL, MDM_CONTROL_SYS_RESET);
 
 	uint32_t status, control;
@@ -553,13 +551,13 @@ static bool kinetis_mdm_cmd_erase_mass(target *t, int argc, const char **argv)
 	tc_printf(t, "Requesting mass erase (status = 0x%"PRIx32")\n", status);
 
 	/* This flag does not exist on KE04 */
-	if (!(status & MDM_STATUS_MASS_ERASE_ENABLED) && !ke04_mode) {
+	if (!(status & MDM_STATUS_MASS_ERASE_ENABLED) && !t->ke04_mode) {
 		tc_printf(t, "ERROR: Mass erase disabled!\n");
 		return false;
 	}
 
 	/* Flag is not persistent */
-	ke04_mode = false;
+	t->ke04_mode = false;
 
 	if (!(status & MDM_STATUS_FLASH_READY)) {
 		tc_printf(t, "ERROR: Flash not ready!\n");

src/target/nxpke04.c

@@ -126,7 +126,6 @@ static int ke04_flash_done(struct target_flash *f);
 static bool kinetis_cmd_unsafe(target *t, int argc, char *argv[]);
 static bool ke04_cmd_sector_erase(target *t, int argc, char *argv[]);
 static bool ke04_cmd_mass_erase(target *t, int argc, char *argv[]);
-static bool unsafe_enabled;
 
 const struct command_s ke_cmd_list[] = {
 	{"unsafe", (cmd_handler)kinetis_cmd_unsafe, "Allow programming security byte (enable|disable)"},
@@ -173,9 +172,9 @@ static bool kinetis_cmd_unsafe(target *t, int argc, char *argv[])
 {
 	if (argc == 1) {
 		tc_printf(t, "Allow programming security byte: %s\n",
-			  unsafe_enabled ? "enabled" : "disabled");
+			  t->unsafe_enabled ? "enabled" : "disabled");
 	} else {
-		parse_enable_or_disable(argv[1], &unsafe_enabled);
+		parse_enable_or_disable(argv[1], &t->unsafe_enabled);
 	}
 	return true;
 }
@@ -253,7 +252,6 @@ bool ke04_probe(target *t)
 	target_add_flash(t, f);
 
 	/* Add target specific commands */
-	unsafe_enabled = false;
 	target_add_commands(t, ke_cmd_list, t->driver);
 
 	return true;
@@ -343,7 +341,8 @@ static int ke04_flash_write(struct target_flash *f,
                               target_addr dest, const void *src, size_t len)
 {
 	/* Ensure we don't write something horrible over the security byte */
-	if (!unsafe_enabled &&
+	target *t = f->t;
+	if (!t->unsafe_enabled &&
 	    (dest <= FLASH_SECURITY_BYTE_ADDRESS) &&
 	    ((dest + len) > FLASH_SECURITY_BYTE_ADDRESS)) {
 		((uint8_t*)src)[FLASH_SECURITY_BYTE_ADDRESS - dest] =
@@ -364,7 +363,8 @@ static int ke04_flash_write(struct target_flash *f,
 
 static int ke04_flash_done(struct target_flash *f)
 {
-	if (unsafe_enabled)
+	target *t = f->t;
+	if (t->unsafe_enabled)
 		return 0;
 
 	if (target_mem_read8(f->t, FLASH_SECURITY_BYTE_ADDRESS) ==

src/target/samd.c

@@ -441,7 +441,10 @@ static void samd_add_flash(target *t, uint32_t addr, size_t length)
 	target_add_flash(t, f);
 }
 
-static char samd_variant_string[60];
+struct samd_priv_s {
+	char samd_variant_string[60];
+};
+
 bool samd_probe(target *t)
 {
 	ADIv5_AP_t *ap = cortexm_ap(t);
@@ -460,6 +463,9 @@ bool samd_probe(target *t)
 	if ((did & SAMD_DID_MASK) != SAMD_DID_CONST_VALUE)
 		return false;
 
+	struct samd_priv_s *priv_storage = calloc(1, sizeof(*priv_storage));
+	t->target_storage = (void*)priv_storage;
+
 	uint32_t ctrlstat = target_mem_read32(t, SAMD_DSU_CTRLSTAT);
 	struct samd_descr samd = samd_parse_device_id(did);
 
@@ -468,14 +474,14 @@ bool samd_probe(target *t)
 
 	/* Part String */
 	if (protected) {
-		sprintf(samd_variant_string,
+		sprintf(priv_storage->samd_variant_string,
 		        "Atmel SAM%c%d%c%d%c%s (rev %c) (PROT=1)",
 		        samd.family,
 		        samd.series, samd.pin, samd.mem,
 		        samd.variant,
 		        samd.package, samd.revision);
 	} else {
-		sprintf(samd_variant_string,
+		sprintf(priv_storage->samd_variant_string,
 		        "Atmel SAM%c%d%c%d%c%s (rev %c)",
 		        samd.family,
 		        samd.series, samd.pin, samd.mem,
@@ -484,7 +490,7 @@ bool samd_probe(target *t)
 	}
 
 	/* Setup Target */
-	t->driver = samd_variant_string;
+	t->driver = priv_storage->samd_variant_string;
 	t->reset = samd_reset;
 
 	if (samd.series == 20 && samd.revision == 'B') {

src/target/samx5x.c

@@ -344,7 +344,10 @@ static void samx5x_add_flash(target *t, uint32_t addr, size_t length,
 	target_add_flash(t, f);
 }
 
-static char samx5x_variant_string[60];
+struct samx5x_priv_s {
+	char samx5x_variant_string[60];
+};
+
 bool samx5x_probe(target *t)
 {
 	ADIv5_AP_t *ap = cortexm_ap(t);
@@ -370,20 +373,25 @@ bool samx5x_probe(target *t)
 	bool protected = (ctrlstat & SAMX5X_STATUSB_PROT);
 
 	/* Part String */
+	struct samx5x_priv_s *priv_storage = calloc(1, sizeof(*priv_storage));
+	t->target_storage = (void*)priv_storage;
+
 	if (protected) {
-		snprintf(samx5x_variant_string, sizeof(samx5x_variant_string),
+		snprintf(priv_storage->samx5x_variant_string,
+				 sizeof(priv_storage->samx5x_variant_string),
 			 "Microchip SAM%c%d%c%dA (rev %c) (PROT=1)",
 			 samx5x.series_letter, samx5x.series_number,
 			 samx5x.pin, samx5x.mem, samx5x.revision);
 	} else {
-		snprintf(samx5x_variant_string, sizeof(samx5x_variant_string),
+		snprintf(priv_storage->samx5x_variant_string,
+				 sizeof(priv_storage->samx5x_variant_string),
 			 "Microchip SAM%c%d%c%dA (rev %c)",
 			 samx5x.series_letter, samx5x.series_number,
 			 samx5x.pin, samx5x.mem, samx5x.revision);
 	}
 
 	/* Setup Target */
-	t->driver = samx5x_variant_string;
+	t->driver = priv_storage->samx5x_variant_string;
 	t->reset = samx5x_reset;
 
 	if (protected) {
@@ -519,7 +527,7 @@ static int samx5x_flash_erase(struct target_flash *f, target_addr addr,
 	target *t = f->t;
 	uint16_t errs = samx5x_read_nvm_error(t);
 	if (errs) {
-		DEBUG_INFO(NVM_ERROR_BITS_MSG, "erase", addr, len);
+		DEBUG_WARN(NVM_ERROR_BITS_MSG, "erase", addr, len);
 		samx5x_print_nvm_error(errs);
 		samx5x_clear_nvm_error(t);
 	}
@@ -533,11 +541,15 @@ static int samx5x_flash_erase(struct target_flash *f, target_addr addr,
 		SAMX5X_PAGE_SIZE;
 	lock_region_size = flash_size >> 5;
 
-	if (addr < (15 - bootprot) * 8192)
+	if (addr < (15 - bootprot) * 8192) {
-		return -1;
+            DEBUG_WARN("Bootprot\n");
+            return -1;
+        }
 
-	if (~runlock & (1 << addr / lock_region_size))
+	if (~runlock & (1 << addr / lock_region_size)) {
-		return -1;
+            DEBUG_WARN("runlock\n");
+            return -1;
+        }
 
 	while (len) {
 		target_mem_write32(t, SAMX5X_NVMC_ADDRESS, addr);
@@ -553,11 +565,15 @@ static int samx5x_flash_erase(struct target_flash *f, target_addr addr,
 		/* Poll for NVM Ready */
 		while ((target_mem_read32(t, SAMX5X_NVMC_STATUS) &
 			SAMX5X_STATUS_READY) == 0)
-			if (target_check_error(t) || samx5x_check_nvm_error(t))
+                    if (target_check_error(t) || samx5x_check_nvm_error(t)) {
-				return -1;
+                        DEBUG_WARN("NVM Ready\n");
+                        return -1;
+                    }
 
-		if (target_check_error(t) || samx5x_check_nvm_error(t))
+		if (target_check_error(t) || samx5x_check_nvm_error(t)) {
-			return -1;
+                    DEBUG_WARN("Error\n");
+                    return -1;
+                }
 
 		/* Lock */
 		samx5x_lock_current_address(t);

src/target/target_internal.h

@@ -114,6 +114,10 @@ struct target_s {
 	uint16_t t_designer;
 	uint16_t idcode;
 	void *target_storage;
+	union {
+		bool unsafe_enabled;
+		bool ke04_mode;
+	};
 
 	struct target_ram *ram;
 	struct target_flash *flash;