flash/nor: handle flash write alignment/padding in the infrastructure

Most of flash drivers have to ensure proper flash write block alignment
and padding. As there was no support for it in the flash infrastructure,
each driver does it its own way. Sometimes this part of code is not properly
tested and contains bugs.

flash_write(_unlock) joins all image sections targeted to one flash bank
using padded areas as a glue. This solves alignment problems on section
boundaries but imposes other problems.

Introduce new flash bank parameters write_start_alignment,
write_end_alignment and minimal_write_gap.
New flash drivers can just properly set these values instead of handling
alignment by its own.

Adapt infrastructure (namely flash_write_unlock(), handle_flash_fill_command()
and handle_flash_write_bank_command()) to prepare write data padded
to an alignment required by the flash bank.

Rework flash_write_unlock() to discontinue write block when the gap
between sections is bigger than minimum specified in minimal_write_gap.
minimal_write_gap is set to one sector by default.

Change-Id: I4368dd402dfaf51c193bcbf1332cffff092b239b
Signed-off-by: Tomas Vanek <vanekt@fbl.cz>
Reviewed-on: http://openocd.zylin.com/4399
Tested-by: jenkins
Reviewed-by: Andreas Bolsch <hyphen0break@gmail.com>

3 files changed, 312 insertions, 121 deletions

diff --git a/src/flash/nor/core.c b/src/flash/nor/core.c
index 707dcff..f05c68b 100644
--- a/src/flash/nor/core.c
+++ b/src/flash/nor/core.c
@@ -4,6 +4,7 @@
  *   Copyright (C) 2008 by Spencer Oliver <spen@spen-soft.co.uk>           *
  *   Copyright (C) 2009 Zachary T Welch <zw@superlucidity.net>             *
  *   Copyright (C) 2010 by Antonio Borneo <borneo.antonio@gmail.com>       *
+ *   Copyright (C) 2017-2018 Tomas Vanek <vanekt@fbl.cz>                   *
  *                                                                         *
  *   This program is free software; you can redistribute it and/or modify  *
  *   it under the terms of the GNU General Public License as published by  *
@@ -600,6 +601,87 @@ static int compare_section(const void *a, const void *b)
 		return -1;
 }
 
+/**
+ * Get aligned start address of a flash write region
+ */
+target_addr_t flash_write_align_start(struct flash_bank *bank, target_addr_t addr)
+{
+	if (addr < bank->base || addr >= bank->base + bank->size
+			|| bank->write_start_alignment <= 1)
+		return addr;
+
+	if (bank->write_start_alignment == FLASH_WRITE_ALIGN_SECTOR) {
+		uint32_t offset = addr - bank->base;
+		uint32_t aligned = 0;
+		int sect;
+		for (sect = 0; sect < bank->num_sectors; sect++) {
+			if (bank->sectors[sect].offset > offset)
+				break;
+
+			aligned = bank->sectors[sect].offset;
+		}
+		return bank->base + aligned;
+	}
+
+	return addr & ~(bank->write_start_alignment - 1);
+}
+
+/**
+ * Get aligned end address of a flash write region
+ */
+target_addr_t flash_write_align_end(struct flash_bank *bank, target_addr_t addr)
+{
+	if (addr < bank->base || addr >= bank->base + bank->size
+			|| bank->write_end_alignment <= 1)
+		return addr;
+
+	if (bank->write_end_alignment == FLASH_WRITE_ALIGN_SECTOR) {
+		uint32_t offset = addr - bank->base;
+		uint32_t aligned = 0;
+		int sect;
+		for (sect = 0; sect < bank->num_sectors; sect++) {
+			aligned = bank->sectors[sect].offset + bank->sectors[sect].size - 1;
+			if (aligned >= offset)
+				break;
+		}
+		return bank->base + aligned;
+	}
+
+	return addr | (bank->write_end_alignment - 1);
+}
+
+/**
+ * Check if gap between sections is bigger than minimum required to discontinue flash write
+ */
+static bool flash_write_check_gap(struct flash_bank *bank,
+				target_addr_t addr1, target_addr_t addr2)
+{
+	if (bank->minimal_write_gap == FLASH_WRITE_CONTINUOUS
+			|| addr1 < bank->base || addr1 >= bank->base + bank->size
+			|| addr2 < bank->base || addr2 >= bank->base + bank->size)
+		return false;
+
+	if (bank->minimal_write_gap == FLASH_WRITE_GAP_SECTOR) {
+		int sect;
+		uint32_t offset1 = addr1 - bank->base;
+		/* find the sector following the one containing addr1 */
+		for (sect = 0; sect < bank->num_sectors; sect++) {
+			if (bank->sectors[sect].offset > offset1)
+				break;
+		}
+		if (sect >= bank->num_sectors)
+			return false;
+
+		uint32_t offset2 = addr2 - bank->base;
+		return bank->sectors[sect].offset + bank->sectors[sect].size <= offset2;
+	}
+
+	target_addr_t aligned1 = flash_write_align_end(bank, addr1);
+	target_addr_t aligned2 = flash_write_align_start(bank, addr2);
+	return aligned1 + bank->minimal_write_gap < aligned2;
+}
+
+
 int flash_write_unlock(struct target *target, struct image *image,
 	uint32_t *written, int erase, bool unlock)
 {
@@ -639,7 +721,7 @@ int flash_write_unlock(struct target *target, struct image *image,
 
 	/* loop until we reach end of the image */
 	while (section < image->num_sections) {
-		uint32_t buffer_size;
+		uint32_t buffer_idx;
 		uint8_t *buffer;
 		int section_last;
 		target_addr_t run_address = sections[section]->base_address + section_offset;
@@ -676,43 +758,37 @@ int flash_write_unlock(struct target *target, struct image *image,
 				break;
 			}
 
-			/* FIXME This needlessly touches sectors BETWEEN the
-			 * sections it's writing.  Without auto erase, it just
-			 * writes ones.  That WILL INVALIDATE data in cases
-			 * like Stellaris Tempest chips, corrupting internal
-			 * ECC codes; and at least FreeScale suggests issues
-			 * with that approach (in HC11 documentation).
-			 *
-			 * With auto erase enabled, data in those sectors will
-			 * be needlessly destroyed; and some of the limited
-			 * number of flash erase cycles will be wasted...
-			 *
-			 * In both cases, the extra writes slow things down.
-			 */
-
 			/* if we have multiple sections within our image,
 			 * flash programming could fail due to alignment issues
 			 * attempt to rebuild a consecutive buffer for the flash loader */
 			target_addr_t run_next_addr = run_address + run_size;
-			if (sections[section_last + 1]->base_address < run_next_addr) {
+			target_addr_t next_section_base = sections[section_last + 1]->base_address;
+			if (next_section_base < run_next_addr) {
 				LOG_ERROR("Section at " TARGET_ADDR_FMT
 					" overlaps section ending at " TARGET_ADDR_FMT,
-					sections[section_last + 1]->base_address,
-					run_next_addr);
+					next_section_base, run_next_addr);
 				LOG_ERROR("Flash write aborted.");
 				retval = ERROR_FAIL;
 				goto done;
 			}
 
-			pad_bytes = sections[section_last + 1]->base_address - run_next_addr;
+			pad_bytes = next_section_base - run_next_addr;
+			if (pad_bytes) {
+				if (flash_write_check_gap(c, run_next_addr - 1, next_section_base)) {
+					LOG_INFO("Flash write discontinued at " TARGET_ADDR_FMT
+						", next section at " TARGET_ADDR_FMT,
+						run_next_addr, next_section_base);
+					break;
+				}
+			}
+			if (pad_bytes > 0)
+				LOG_INFO("Padding image section %d at " TARGET_ADDR_FMT
+					" with %d bytes",
+					section_last, run_next_addr, pad_bytes);
+
 			padding[section_last] = pad_bytes;
-			run_size += sections[++section_last]->size;
 			run_size += pad_bytes;
-
-			if (pad_bytes > 0)
-				LOG_INFO("Padding image section %d with %d bytes",
-					section_last-1,
-					pad_bytes);
+			run_size += sections[++section_last]->size;
 		}
 
 		if (run_address + run_size - 1 > c->base + c->size - 1) {
@@ -725,10 +801,38 @@ int flash_write_unlock(struct target *target, struct image *image,
 			assert(run_size > 0);
 		}
 
-		/* If we're applying any sector automagic, then pad this
-		 * (maybe-combined) segment to the end of its last sector.
-		 */
-		if (unlock || erase) {
+		uint32_t padding_at_start = 0;
+		if (c->write_start_alignment || c->write_end_alignment) {
+			/* align write region according to bank requirements */
+			target_addr_t aligned_start = flash_write_align_start(c, run_address);
+			padding_at_start = run_address - aligned_start;
+			if (padding_at_start > 0) {
+				LOG_WARNING("Section start address " TARGET_ADDR_FMT
+					" breaks the required alignment of flash bank %s",
+					run_address, c->name);
+				LOG_WARNING("Padding %d bytes from " TARGET_ADDR_FMT,
+					padding_at_start, aligned_start);
+
+				run_address -= padding_at_start;
+				run_size += padding_at_start;
+			}
+
+			target_addr_t run_end = run_address + run_size - 1;
+			target_addr_t aligned_end = flash_write_align_end(c, run_end);
+			pad_bytes = aligned_end - run_end;
+			if (pad_bytes > 0) {
+				LOG_INFO("Padding image section %d at " TARGET_ADDR_FMT
+					" with %d bytes (bank write end alignment)",
+					section_last, run_end + 1, pad_bytes);
+
+				padding[section_last] += pad_bytes;
+				run_size += pad_bytes;
+			}
+
+		} else if (unlock || erase) {
+			/* If we're applying any sector automagic, then pad this
+			 * (maybe-combined) segment to the end of its last sector.
+			 */
 			int sector;
 			uint32_t offset_start = run_address - c->base;
 			uint32_t offset_end = offset_start + run_size;
@@ -753,13 +857,17 @@ int flash_write_unlock(struct target *target, struct image *image,
 			retval = ERROR_FAIL;
 			goto done;
 		}
-		buffer_size = 0;
+
+		if (padding_at_start)
+			memset(buffer, c->default_padded_value, padding_at_start);
+
+		buffer_idx = padding_at_start;
 
 		/* read sections to the buffer */
-		while (buffer_size < run_size) {
+		while (buffer_idx < run_size) {
 			size_t size_read;
 
-			size_read = run_size - buffer_size;
+			size_read = run_size - buffer_idx;
 			if (size_read > sections[section]->size - section_offset)
 				size_read = sections[section]->size - section_offset;
 
@@ -772,23 +880,25 @@ int flash_write_unlock(struct target *target, struct image *image,
 			int t_section_num = diff / sizeof(struct imagesection);
 
 			LOG_DEBUG("image_read_section: section = %d, t_section_num = %d, "
-					"section_offset = %d, buffer_size = %d, size_read = %d",
-				(int)section, (int)t_section_num, (int)section_offset,
-				(int)buffer_size, (int)size_read);
+					"section_offset = %"PRIu32", buffer_idx = %"PRIu32", size_read = %zu",
+				section, t_section_num, section_offset,
+				buffer_idx, size_read);
 			retval = image_read_section(image, t_section_num, section_offset,
-					size_read, buffer + buffer_size, &size_read);
+					size_read, buffer + buffer_idx, &size_read);
 			if (retval != ERROR_OK || size_read == 0) {
 				free(buffer);
 				goto done;
 			}
 
-			/* see if we need to pad the section */
-			while (padding[section]--)
-				(buffer + buffer_size)[size_read++] = c->default_padded_value;
-
-			buffer_size += size_read;
+			buffer_idx += size_read;
 			section_offset += size_read;
 
+			/* see if we need to pad the section */
+			if (padding[section]) {
+				memset(buffer + buffer_idx, c->default_padded_value, padding[section]);
+				buffer_idx += padding[section];
+			}
+
 			if (section_offset >= sections[section]->size) {
 				section++;
 				section_offset = 0;
diff --git a/src/flash/nor/core.h b/src/flash/nor/core.h
index 1bfe1ab..67de94e 100644
--- a/src/flash/nor/core.h
+++ b/src/flash/nor/core.h
@@ -65,6 +65,13 @@ struct flash_sector {
 	int is_protected;
 };
 
+/** Special value for write_start_alignment and write_end_alignment field */
+#define FLASH_WRITE_ALIGN_SECTOR	UINT32_MAX
+
+/** Special values for minimal_write_gap field */
+#define FLASH_WRITE_CONTINUOUS		0
+#define FLASH_WRITE_GAP_SECTOR		UINT32_MAX
+
 /**
  * Provides details of a flash bank, available either on-chip or through
  * a major interface.
@@ -97,6 +104,18 @@ struct flash_bank {
 	 * erased value. Defaults to 0xFF. */
 	uint8_t default_padded_value;
 
+	/** Required alignment of flash write start address.
+	 * Default 0, no alignment. Can be any power of two or FLASH_WRITE_ALIGN_SECTOR */
+	uint32_t write_start_alignment;
+	/** Required alignment of flash write end address.
+	 * Default 0, no alignment. Can be any power of two or FLASH_WRITE_ALIGN_SECTOR */
+	uint32_t write_end_alignment;
+	/** Minimal gap between sections to discontinue flash write
+	 * Default FLASH_WRITE_GAP_SECTOR splits the write if one or more untouched
+	 * sectors in between.
+     * Can be size in bytes or FLASH_WRITE_CONTINUOUS */
+	uint32_t minimal_write_gap;
+
 	/**
 	 * The number of sectors on this chip.  This value will
 	 * be set intially to 0, and the flash driver must set this to
@@ -136,6 +155,22 @@ int flash_unlock_address_range(struct target *target, uint32_t addr,
 		uint32_t length);
 
 /**
+ * Align start address of a flash write region according to bank requirements.
+ * @param bank Pointer to bank descriptor structure
+ * @param addr Address to align
+ * @returns Aligned address
+*/
+target_addr_t flash_write_align_start(struct flash_bank *bank, target_addr_t addr);
+/**
+ * Align end address of a flash write region according to bank requirements.
+ * Note: Use address of the last byte to write, not the next after the region.
+ * @param bank Pointer to bank descriptor structure
+ * @param addr Address to align (address of the last byte to write)
+ * @returns Aligned address (address of the last byte of padded region)
+*/
+target_addr_t flash_write_align_end(struct flash_bank *bank, target_addr_t addr);
+
+/**
  * Writes @a image into the @a target flash.  The @a written parameter
  * will contain the
  * @param target The target with the flash to be programmed.
diff --git a/src/flash/nor/tcl.c b/src/flash/nor/tcl.c
index e5e2801..34681db 100644
--- a/src/flash/nor/tcl.c
+++ b/src/flash/nor/tcl.c
@@ -3,6 +3,7 @@
  *   Copyright (C) 2007,2008 Øyvind Harboe <oyvind.harboe@zylin.com>       *
  *   Copyright (C) 2008 by Spencer Oliver <spen@spen-soft.co.uk>           *
  *   Copyright (C) 2009 Zachary T Welch <zw@superlucidity.net>             *
+ *   Copyright (C) 2017-2018 Tomas Vanek <vanekt@fbl.cz>                   *
  *                                                                         *
  *   This program is free software; you can redistribute it and/or modify  *
  *   it under the terms of the GNU General Public License as published by  *
@@ -460,42 +461,29 @@ COMMAND_HANDLER(handle_flash_write_image_command)
 
 COMMAND_HANDLER(handle_flash_fill_command)
 {
-	int err = ERROR_OK;
-	uint32_t address;
+	target_addr_t address;
 	uint32_t pattern;
 	uint32_t count;
-	uint32_t wrote = 0;
-	uint32_t cur_size = 0;
-	uint32_t chunk_count;
 	struct target *target = get_current_target(CMD_CTX);
 	unsigned i;
 	uint32_t wordsize;
-	int retval = ERROR_OK;
-
-	static size_t const chunksize = 1024;
-	uint8_t *chunk = NULL, *readback = NULL;
+	int retval;
 
-	if (CMD_ARGC != 3) {
-		retval = ERROR_COMMAND_SYNTAX_ERROR;
-		goto done;
-	}
+	if (CMD_ARGC != 3)
+		return ERROR_COMMAND_SYNTAX_ERROR;
 
+#if BUILD_TARGET64
+	COMMAND_PARSE_NUMBER(u64, CMD_ARGV[0], address);
+#else
 	COMMAND_PARSE_NUMBER(u32, CMD_ARGV[0], address);
+#endif
 	COMMAND_PARSE_NUMBER(u32, CMD_ARGV[1], pattern);
 	COMMAND_PARSE_NUMBER(u32, CMD_ARGV[2], count);
 
-	chunk = malloc(chunksize);
-	if (chunk == NULL)
-		return ERROR_FAIL;
-
-	readback = malloc(chunksize);
-	if (readback == NULL) {
-		free(chunk);
-		return ERROR_FAIL;
-	}
-
-	if (count == 0)
-		goto done;
+	struct flash_bank *bank;
+	retval = get_flash_bank_by_addr(target, address, true, &bank);
+	if (retval != ERROR_OK)
+		return retval;
 
 	switch (CMD_NAME[4]) {
 		case 'w':
@@ -508,73 +496,109 @@ COMMAND_HANDLER(handle_flash_fill_command)
 			wordsize = 1;
 			break;
 		default:
-			retval = ERROR_COMMAND_SYNTAX_ERROR;
-			goto done;
+			return ERROR_COMMAND_SYNTAX_ERROR;
+	}
+
+	if (count == 0)
+		return ERROR_OK;
+
+	if (address + count >= bank->base + bank->size) {
+		LOG_ERROR("Cannot cross flash bank borders");
+		return ERROR_FAIL;
+	}
+
+	uint32_t size_bytes = count * wordsize;
+	target_addr_t aligned_start = flash_write_align_start(bank, address);
+	target_addr_t end_addr = address + size_bytes - 1;
+	target_addr_t aligned_end = flash_write_align_end(bank, end_addr);
+	uint32_t aligned_size = aligned_end + 1 - aligned_start;
+	uint32_t padding_at_start = address - aligned_start;
+	uint32_t padding_at_end = aligned_end - end_addr;
+
+	uint8_t *buffer = malloc(aligned_size);
+	if (buffer == NULL)
+		return ERROR_FAIL;
+
+	if (padding_at_start) {
+		memset(buffer, bank->default_padded_value, padding_at_start);
+		LOG_WARNING("Start address " TARGET_ADDR_FMT
+			" breaks the required alignment of flash bank %s",
+			address, bank->name);
+		LOG_WARNING("Padding %" PRId32 " bytes from " TARGET_ADDR_FMT,
+		    padding_at_start, aligned_start);
 	}
 
-	chunk_count = MIN(count, (chunksize / wordsize));
+	uint8_t *ptr = buffer + padding_at_start;
+
 	switch (wordsize) {
 		case 4:
-			for (i = 0; i < chunk_count; i++)
-				target_buffer_set_u32(target, chunk + i * wordsize, pattern);
+			for (i = 0; i < count; i++, ptr += wordsize)
+				target_buffer_set_u32(target, ptr, pattern);
 			break;
 		case 2:
-			for (i = 0; i < chunk_count; i++)
-				target_buffer_set_u16(target, chunk + i * wordsize, pattern);
+			for (i = 0; i < count; i++, ptr += wordsize)
+				target_buffer_set_u16(target, ptr, pattern);
 			break;
 		case 1:
-			memset(chunk, pattern, chunk_count);
+			memset(ptr, pattern, count);
+			ptr += count;
 			break;
 		default:
 			LOG_ERROR("BUG: can't happen");
 			exit(-1);
 	}
 
+	if (padding_at_end) {
+		memset(ptr, bank->default_padded_value, padding_at_end);
+		LOG_INFO("Padding at " TARGET_ADDR_FMT " with %" PRId32
+			" bytes (bank write end alignment)",
+			end_addr + 1, padding_at_end);
+	}
+
 	struct duration bench;
 	duration_start(&bench);
 
-	for (wrote = 0; wrote < (count*wordsize); wrote += cur_size) {
-		struct flash_bank *bank;
+	retval = flash_driver_write(bank, buffer, aligned_start - bank->base, aligned_size);
+	if (retval != ERROR_OK)
+		goto done;
 
-		retval = get_flash_bank_by_addr(target, address, true, &bank);
-		if (retval != ERROR_OK)
-			goto done;
+	retval = flash_driver_read(bank, buffer, address - bank->base, size_bytes);
+	if (retval != ERROR_OK)
+		goto done;
 
-		cur_size = MIN((count * wordsize - wrote), chunksize);
-		err = flash_driver_write(bank, chunk, address - bank->base + wrote, cur_size);
-		if (err != ERROR_OK) {
-			retval = err;
-			goto done;
-		}
+	for (i = 0, ptr = buffer; i < count; i++) {
+		uint32_t readback = 0;
 
-		err = flash_driver_read(bank, readback, address - bank->base + wrote, cur_size);
-		if (err != ERROR_OK) {
-			retval = err;
-			goto done;
+		switch (wordsize) {
+			case 4:
+				readback = target_buffer_get_u32(target, ptr);
+				break;
+			case 2:
+				readback = target_buffer_get_u16(target, ptr);
+				break;
+			case 1:
+				readback = *ptr;
+				break;
 		}
-
-		for (i = 0; i < cur_size; i++) {
-			if (readback[i] != chunk[i]) {
-				LOG_ERROR(
-					"Verification error address 0x%08" PRIx32 ", read back 0x%02x, expected 0x%02x",
-					address + wrote + i,
-					readback[i],
-					chunk[i]);
-				retval = ERROR_FAIL;
-				goto done;
-			}
+		if (readback != pattern) {
+			LOG_ERROR(
+				"Verification error address " TARGET_ADDR_FMT
+				", read back 0x%02" PRIx32 ", expected 0x%02" PRIx32,
+				address + i * wordsize, readback, pattern);
+			retval = ERROR_FAIL;
+			goto done;
 		}
+		ptr += wordsize;
 	}
 
 	if ((retval == ERROR_OK) && (duration_measure(&bench) == ERROR_OK)) {
-		command_print(CMD_CTX, "wrote %" PRIu32 " bytes to 0x%8.8" PRIx32
-			" in %fs (%0.3f KiB/s)", wrote, address,
-			duration_elapsed(&bench), duration_kbps(&bench, wrote));
+		command_print(CMD_CTX, "wrote %" PRIu32 " bytes to " TARGET_ADDR_FMT
+			" in %fs (%0.3f KiB/s)", size_bytes, address,
+			duration_elapsed(&bench), duration_kbps(&bench, size_bytes));
 	}
 
 done:
-	free(readback);
-	free(chunk);
+	free(buffer);
 
 	return retval;
 }
@@ -592,8 +616,8 @@ COMMAND_HANDLER(handle_flash_write_bank_command)
 	struct duration bench;
 	duration_start(&bench);
 
-	struct flash_bank *p;
-	int retval = CALL_COMMAND_HANDLER(flash_command_get_bank, 0, &p);
+	struct flash_bank *bank;
+	int retval = CALL_COMMAND_HANDLER(flash_command_get_bank, 0, &bank);
 	if (ERROR_OK != retval)
 		return retval;
 
@@ -602,7 +626,7 @@ COMMAND_HANDLER(handle_flash_write_bank_command)
 	if (CMD_ARGC > 2)
 		COMMAND_PARSE_NUMBER(u32, CMD_ARGV[2], offset);
 
-	if (offset > p->size) {
+	if (offset > bank->size) {
 		LOG_ERROR("Offset 0x%8.8" PRIx32 " is out of range of the flash bank",
 			offset);
 		return ERROR_COMMAND_ARGUMENT_INVALID;
@@ -618,7 +642,7 @@ COMMAND_HANDLER(handle_flash_write_bank_command)
 		return retval;
 	}
 
-	length = MIN(filesize, p->size - offset);
+	length = MIN(filesize, bank->size - offset);
 
 	if (!length) {
 		LOG_INFO("Nothing to write to flash bank");
@@ -630,14 +654,33 @@ COMMAND_HANDLER(handle_flash_write_bank_command)
 		LOG_INFO("File content exceeds flash bank size. Only writing the "
 			"first %zu bytes of the file", length);
 
-	buffer = malloc(length);
+	target_addr_t start_addr = bank->base + offset;
+	target_addr_t aligned_start = flash_write_align_start(bank, start_addr);
+	target_addr_t end_addr = start_addr + length - 1;
+	target_addr_t aligned_end = flash_write_align_end(bank, end_addr);
+	uint32_t aligned_size = aligned_end + 1 - aligned_start;
+	uint32_t padding_at_start = start_addr - aligned_start;
+	uint32_t padding_at_end = aligned_end - end_addr;
+
+	buffer = malloc(aligned_size);
 	if (buffer == NULL) {
 		fileio_close(fileio);
 		LOG_ERROR("Out of memory");
 		return ERROR_FAIL;
 	}
+
+	if (padding_at_start) {
+		memset(buffer, bank->default_padded_value, padding_at_start);
+		LOG_WARNING("Start offset 0x%08" PRIx32
+			" breaks the required alignment of flash bank %s",
+			offset, bank->name);
+		LOG_WARNING("Padding %" PRId32 " bytes from " TARGET_ADDR_FMT,
+		    padding_at_start, aligned_start);
+	}
+
+	uint8_t *ptr = buffer + padding_at_start;
 	size_t buf_cnt;
-	if (fileio_read(fileio, length, buffer, &buf_cnt) != ERROR_OK) {
+	if (fileio_read(fileio, length, ptr, &buf_cnt) != ERROR_OK) {
 		free(buffer);
 		fileio_close(fileio);
 		return ERROR_FAIL;
@@ -649,15 +692,23 @@ COMMAND_HANDLER(handle_flash_write_bank_command)
 		return ERROR_FAIL;
 	}
 
-	retval = flash_driver_write(p, buffer, offset, length);
+	ptr += length;
+
+	if (padding_at_end) {
+		memset(ptr, bank->default_padded_value, padding_at_end);
+		LOG_INFO("Padding at " TARGET_ADDR_FMT " with %" PRId32
+			" bytes (bank write end alignment)",
+			end_addr + 1, padding_at_end);
+	}
+
+	retval = flash_driver_write(bank, buffer, aligned_start - bank->base, aligned_size);
 
 	free(buffer);
-	buffer = NULL;
 
 	if ((ERROR_OK == retval) && (duration_measure(&bench) == ERROR_OK)) {
 		command_print(CMD_CTX, "wrote %zu bytes from file %s to flash bank %u"
 			" at offset 0x%8.8" PRIx32 " in %fs (%0.3f KiB/s)",
-			length, CMD_ARGV[1], p->bank_number, offset,
+			length, CMD_ARGV[1], bank->bank_number, offset,
 			duration_elapsed(&bench), duration_kbps(&bench, length));
 	}
 
@@ -1071,21 +1122,16 @@ COMMAND_HANDLER(handle_flash_bank_command)
 		}
 	}
 
-	struct flash_bank *c = malloc(sizeof(*c));
+	struct flash_bank *c = calloc(1, sizeof(*c));
 	c->name = strdup(bank_name);
 	c->target = target;
 	c->driver = driver;
-	c->driver_priv = NULL;
 	COMMAND_PARSE_NUMBER(u32, CMD_ARGV[1], c->base);
 	COMMAND_PARSE_NUMBER(u32, CMD_ARGV[2], c->size);
 	COMMAND_PARSE_NUMBER(int, CMD_ARGV[3], c->chip_width);
 	COMMAND_PARSE_NUMBER(int, CMD_ARGV[4], c->bus_width);
 	c->default_padded_value = c->erased_value = 0xff;
-	c->num_sectors = 0;
-	c->sectors = NULL;
-	c->num_prot_blocks = 0;
-	c->prot_blocks = NULL;
-	c->next = NULL;
+	c->minimal_write_gap = FLASH_WRITE_GAP_SECTOR;
 
 	int retval;
 	retval = CALL_COMMAND_HANDLER(driver->flash_bank_command, c);