Refactor flash code.

15 files changed, 238 insertions, 236 deletions

diff --git a/projects/board-test/keystore-perf.c b/projects/board-test/keystore-perf.c
index 75b4e3f..09528a2 100644
--- a/projects/board-test/keystore-perf.c
+++ b/projects/board-test/keystore-perf.c
@@ -64,7 +64,7 @@ static void test_erase_sector(void)
     int err;
 
     for (i = 0; i < KEYSTORE_NUM_SECTORS; ++i) {
-        err = keystore_erase_sectors(i, i);
+        err = keystore_erase_sector(i);
         if (err != 1) {
             uart_send_string("ERROR: keystore_erase_sector returned ");
             uart_send_integer(err, 0);
@@ -83,7 +83,7 @@ static void test_erase_subsector(void)
     int err;
 
     for (i = 0; i < KEYSTORE_NUM_SUBSECTORS; ++i) {
-        err = keystore_erase_subsectors(i, i);
+        err = keystore_erase_subsector(i);
         if (err != 1) {
             uart_send_string("ERROR: keystore_erase_subsector returned ");
             uart_send_integer(err, 0);
diff --git a/projects/board-test/spiflash-perf.c b/projects/board-test/spiflash-perf.c
index 1abcd7b..53f29cb 100644
--- a/projects/board-test/spiflash-perf.c
+++ b/projects/board-test/spiflash-perf.c
@@ -23,7 +23,7 @@ extern struct spiflash_ctx keystore_ctx;
 static struct spiflash_ctx *ctx = &keystore_ctx;
 
 /*
- * 1. Read the entire flash by pages, ignoring data.
+ * 1a. Read the entire flash by pages, ignoring data.
  */
 static void test_read_page(void)
 {
@@ -43,6 +43,26 @@ static void test_read_page(void)
 }
 
 /*
+ * 1b. Read the entire flash by subsectors, ignoring data.
+ */
+static void test_read_subsector(void)
+{
+    uint8_t read_buf[N25Q128_SUBSECTOR_SIZE];
+    uint32_t i;
+    int err;
+
+    for (i = 0; i < N25Q128_NUM_SUBSECTORS; ++i) {
+        err = n25q128_read_subsector(ctx, i, read_buf);
+        if (err != 1) {
+            uart_send_string("ERROR: n25q128_read_subsector returned ");
+            uart_send_integer(err, 0);
+            uart_send_string("\r\n");
+            break;
+        }
+    }
+}
+
+/*
  * Read the flash data and verify it against a known pattern.
  * It turns out that verification doesn't slow us down in any measurable
  * way, because memcmp on 256 bytes is pretty inconsequential.
@@ -217,6 +237,7 @@ int main(void)
     uart_send_string("Starting...\r\n");
 
     time_check("read page       ", test_read_page(),       N25Q128_NUM_PAGES);
+    time_check("read subsector  ", test_read_subsector(),  N25Q128_NUM_SUBSECTORS);
     time_check("erase subsector ", test_erase_subsector(), N25Q128_NUM_SUBSECTORS);
     time_check("erase sector    ", test_erase_sector(),    N25Q128_NUM_SECTORS);
     time_check("erase bulk      ", test_erase_bulk(),      1);
diff --git a/projects/cli-test/mgmt-dfu.c b/projects/cli-test/mgmt-dfu.c
index 8f258ea..851c8ea 100644
--- a/projects/cli-test/mgmt-dfu.c
+++ b/projects/cli-test/mgmt-dfu.c
@@ -72,9 +72,7 @@ static int cmd_dfu_erase(struct cli_def *cli, const char *command, char *argv[],
 {
     int status;
 
-    cli_print(cli, "Erasing flash sectors %i to %i (address %p to %p) - expect the CLI to crash now",
-	      stm_flash_sector_num((uint32_t) dfu_firmware),
-	      stm_flash_sector_num((uint32_t) dfu_firmware_end),
+    cli_print(cli, "Erasing flash address %p to %p - expect the CLI to crash now",
 	      dfu_firmware,
 	      dfu_firmware_end);
 
diff --git a/projects/cli-test/mgmt-fpga.c b/projects/cli-test/mgmt-fpga.c
index 31a6e27..b1b0973 100644
--- a/projects/cli-test/mgmt-fpga.c
+++ b/projects/cli-test/mgmt-fpga.c
@@ -47,7 +47,13 @@ static volatile uint32_t dfu_offset = 0;
 
 
 
-static int _flash_write_callback(uint8_t *buf, size_t len) {
+static int _flash_write_callback(uint8_t *buf, size_t len)
+{
+    if ((dfu_offset % FPGACFG_SECTOR_SIZE) == 0)
+	/* first page in sector, need to erase sector */
+	if (fpgacfg_erase_sector(dfu_offset / FPGACFG_SECTOR_SIZE) != 1)
+	    return CLI_ERROR;
+
     int res = fpgacfg_write_data(dfu_offset, buf, BITSTREAM_UPLOAD_CHUNK_SIZE) == 1;
     dfu_offset += BITSTREAM_UPLOAD_CHUNK_SIZE;
     return res;
@@ -91,7 +97,7 @@ static int cmd_fpga_bitstream_erase(struct cli_def *cli, const char *command, ch
      *
      * This command could be made to accept an argument indicating the whole memory should be erased.
      */
-    if (! fpgacfg_erase_sectors(1)) {
+    if (fpgacfg_erase_sector(0) != 0) {
 	cli_print(cli, "Erasing first sector in FPGA config memory failed");
 	return CLI_ERROR;
     }
diff --git a/projects/cli-test/mgmt-keystore.c b/projects/cli-test/mgmt-keystore.c
index 457abc2..09d512e 100644
--- a/projects/cli-test/mgmt-keystore.c
+++ b/projects/cli-test/mgmt-keystore.c
@@ -334,7 +334,7 @@ static int cmd_keystore_erase(struct cli_def *cli, const char *command, char *ar
     }
 
     cli_print(cli, "OK, erasing keystore, this might take a while...");
-    if ((status = keystore_erase_sectors(0, KEYSTORE_NUM_SECTORS - 1)) != 1) {
+    if ((status = keystore_erase_bulk()) != 1) {
         cli_print(cli, "Failed erasing token keystore: %i", status);
 	return CLI_ERROR;
     }
diff --git a/projects/cli-test/mgmt-show.c b/projects/cli-test/mgmt-show.c
index 5cca8b7..f124830 100644
--- a/projects/cli-test/mgmt-show.c
+++ b/projects/cli-test/mgmt-show.c
@@ -132,7 +132,7 @@ static int cmd_show_keystore_data(struct cli_def *cli, const char *command, char
 	}
     } else {
 	cli_print(cli, "Erasing first sector since all the first 8 bytes are tombstones");
-	if ((i = keystore_erase_sectors(1, 1)) != 1) {
+	if ((i = keystore_erase_sector(0)) != 1) {
 	    cli_print(cli, "Failed erasing the first sector: %li", i);
 	    return CLI_ERROR;
 	}
diff --git a/projects/hsm/mgmt-fpga.c b/projects/hsm/mgmt-fpga.c
index d76315e..06f2a26 100644
--- a/projects/hsm/mgmt-fpga.c
+++ b/projects/hsm/mgmt-fpga.c
@@ -3,7 +3,7 @@
  * -----------
  * CLI code to manage the FPGA configuration etc.
  *
- * Copyright (c) 2016, NORDUnet A/S All rights reserved.
+ * Copyright (c) 2016-2017, NORDUnet A/S All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
@@ -55,7 +55,13 @@ extern hal_user_t user;
 static volatile uint32_t dfu_offset = 0;
 
 
-static int _flash_write_callback(uint8_t *buf, size_t len) {
+static int _flash_write_callback(uint8_t *buf, size_t len)
+{
+    if ((dfu_offset % FPGACFG_SECTOR_SIZE) == 0)
+	/* first page in sector, need to erase sector */
+	if (fpgacfg_erase_sector(dfu_offset / FPGACFG_SECTOR_SIZE) != 1)
+	    return CLI_ERROR;
+
     int res = fpgacfg_write_data(dfu_offset, buf, BITSTREAM_UPLOAD_CHUNK_SIZE) == 1;
     dfu_offset += BITSTREAM_UPLOAD_CHUNK_SIZE;
     return res;
@@ -104,7 +110,7 @@ static int cmd_fpga_bitstream_erase(struct cli_def *cli, const char *command, ch
      *
      * This command could be made to accept an argument indicating the whole memory should be erased.
      */
-    if (! fpgacfg_erase_sectors(1)) {
+    if (fpgacfg_erase_sector(0) != 0) {
 	cli_print(cli, "Erasing first sector in FPGA config memory failed");
 	return CLI_ERROR;
     }
diff --git a/spiflash_n25q128.c b/spiflash_n25q128.c
index 133ecb4..5c4a3b2 100644
--- a/spiflash_n25q128.c
+++ b/spiflash_n25q128.c
@@ -37,91 +37,129 @@
 
 #include "spiflash_n25q128.h"
 
-#define _n25q128_select(ctx)	HAL_GPIO_WritePin(ctx->cs_n_port, ctx->cs_n_pin, GPIO_PIN_RESET);
-#define _n25q128_deselect(ctx)	HAL_GPIO_WritePin(ctx->cs_n_port, ctx->cs_n_pin, GPIO_PIN_SET);
-
-
 #define N25Q128_NUM_BYTES	(N25Q128_PAGE_SIZE * N25Q128_NUM_PAGES)
 
 #if N25Q128_SECTOR_SIZE    * N25Q128_NUM_SECTORS    != N25Q128_NUM_BYTES || \
     N25Q128_SUBSECTOR_SIZE * N25Q128_NUM_SUBSECTORS != N25Q128_NUM_BYTES
-#error Inconsistant definitions for pages / sectors / subsectors
+#error Inconsistent definitions for pages / sectors / subsectors
 #endif
 
 
-static int _n25q128_get_wel_flag(struct spiflash_ctx *ctx);
-static int _wait_while_wip(struct spiflash_ctx *ctx, uint32_t timeout);
+static inline void _n25q128_select(struct spiflash_ctx *ctx)
+{
+    HAL_GPIO_WritePin(ctx->cs_n_port, ctx->cs_n_pin, GPIO_PIN_RESET);
+}
 
+static inline void _n25q128_deselect(struct spiflash_ctx *ctx)
+{
+    HAL_GPIO_WritePin(ctx->cs_n_port, ctx->cs_n_pin, GPIO_PIN_SET);
+}
 
-int n25q128_check_id(struct spiflash_ctx *ctx)
+/* Read a bit from the status register. */
+static inline int _n25q128_get_status_bit(struct spiflash_ctx *ctx, unsigned bitnum)
 {
     // tx, rx buffers
-    uint8_t spi_tx[4];
-    uint8_t spi_rx[4];
+    uint8_t spi_tx[2];
+    uint8_t spi_rx[2];
 
     // result
     HAL_StatusTypeDef ok;
 
-    // send READ ID command
-    spi_tx[0] = N25Q128_COMMAND_READ_ID;
+    //assert(bitnum < sizeof(uint8_t));
 
-    // select, send command & read response, deselect
+    // send READ STATUS command
+    spi_tx[0] = N25Q128_COMMAND_READ_STATUS;
+
+    // send command, read response, deselect
     _n25q128_select(ctx);
-    ok = HAL_SPI_TransmitReceive(ctx->hspi, spi_tx, spi_rx, 4, N25Q128_SPI_TIMEOUT);
+    ok = HAL_SPI_TransmitReceive(ctx->hspi, spi_tx, spi_rx, 2, N25Q128_SPI_TIMEOUT);
     _n25q128_deselect(ctx);
 
     // check
-    if (ok != HAL_OK) return 0;
-
-    // parse response (note, that the very first byte was received during the
-    // transfer of the command byte, so it contains garbage and should
-    // be ignored here)
-    if (spi_rx[1] != N25Q128_ID_MANUFACTURER) return 0;
-    if (spi_rx[2] != N25Q128_ID_DEVICE_TYPE) return 0;
-    if (spi_rx[3] != N25Q128_ID_DEVICE_CAPACITY) return 0;
+    if (ok != HAL_OK) return -1;
 
     // done
-    return 1;
+    return ((spi_rx[1] >> bitnum) & 1);
 }
 
+/* Read the Write Enable Latch bit in the status register. */
+static inline int _n25q128_get_wel_flag(struct spiflash_ctx *ctx)
+{
+    return _n25q128_get_status_bit(ctx, 1);
+}
+
+/* Read the Write In Progress bit in the status register. */
+static inline int _n25q128_get_wip_flag(struct spiflash_ctx *ctx)
+{
+    return _n25q128_get_status_bit(ctx, 0);
+}
 
-int n25q128_read_page(struct spiflash_ctx *ctx, uint32_t page_offset, uint8_t *page_buffer)
+/* Wait until the flash memory is done writing */
+static int _n25q128_wait_while_wip(struct spiflash_ctx *ctx, uint32_t timeout)
+{
+    uint32_t tick_end = HAL_GetTick() + timeout;
+    int i;
+
+    do {
+	i = _n25q128_get_wip_flag(ctx);
+	if (i < 0) return 0;
+	if (! i) return 1;
+    } while (HAL_GetTick() < tick_end);
+
+    return 0;
+}
+
+/* Send the Write Enable command */
+static int _n25q128_write_enable(struct spiflash_ctx *ctx)
 {
     // tx buffer
-    uint8_t spi_tx[4];
+    uint8_t spi_tx[1];
 
     // result
     HAL_StatusTypeDef ok;
 
-    // check offset
-    if (page_offset >= N25Q128_NUM_PAGES) return 0;
-
-    // calculate byte address
-    page_offset *= N25Q128_PAGE_SIZE;
-
-    // prepare READ command
-    spi_tx[0] = N25Q128_COMMAND_READ_PAGE;
-    spi_tx[1] = (uint8_t)(page_offset >> 16);
-    spi_tx[2] = (uint8_t)(page_offset >>  8);
-    spi_tx[3] = (uint8_t)(page_offset >>  0);
+    // enable writing
+    spi_tx[0] = N25Q128_COMMAND_WRITE_ENABLE;
 
-    // activate, send command
+    // activate, send command, deselect
     _n25q128_select(ctx);
-    ok = HAL_SPI_Transmit(ctx->hspi, spi_tx, 4, N25Q128_SPI_TIMEOUT);
+    ok = HAL_SPI_Transmit(ctx->hspi, spi_tx, 1, N25Q128_SPI_TIMEOUT);
+    _n25q128_deselect(ctx);
 
     // check
-    if (ok != HAL_OK) {
-	_n25q128_deselect(ctx);
-	return 0;
-    }
+    if (ok != HAL_OK) return -1;
 
-    // read response, deselect
-    ok = HAL_SPI_Receive(ctx->hspi, page_buffer, N25Q128_PAGE_SIZE, N25Q128_SPI_TIMEOUT);
+    // make sure, that write enable did the job
+    return _n25q128_get_wel_flag(ctx);
+}
+
+int n25q128_check_id(struct spiflash_ctx *ctx)
+{
+    // tx, rx buffers
+    uint8_t spi_tx[4];
+    uint8_t spi_rx[4];
+
+    // result
+    HAL_StatusTypeDef ok;
+
+    // send READ ID command
+    spi_tx[0] = N25Q128_COMMAND_READ_ID;
+
+    // select, send command & read response, deselect
+    _n25q128_select(ctx);
+    ok = HAL_SPI_TransmitReceive(ctx->hspi, spi_tx, spi_rx, 4, N25Q128_SPI_TIMEOUT);
     _n25q128_deselect(ctx);
 
     // check
     if (ok != HAL_OK) return 0;
 
+    // parse response (note, that the very first byte was received during the
+    // transfer of the command byte, so it contains garbage and should
+    // be ignored here)
+    if (spi_rx[1] != N25Q128_ID_MANUFACTURER) return 0;
+    if (spi_rx[2] != N25Q128_ID_DEVICE_TYPE) return 0;
+    if (spi_rx[3] != N25Q128_ID_DEVICE_CAPACITY) return 0;
+
     // done
     return 1;
 }
@@ -139,28 +177,16 @@ int n25q128_write_page(struct spiflash_ctx *ctx, uint32_t page_offset, const uin
     if (page_offset >= N25Q128_NUM_PAGES) return 0;
 
     // enable writing
-    spi_tx[0] = N25Q128_COMMAND_WRITE_ENABLE;
-
-    // activate, send command, deselect
-    _n25q128_select(ctx);
-    ok = HAL_SPI_Transmit(ctx->hspi, spi_tx, 1, N25Q128_SPI_TIMEOUT);
-    _n25q128_deselect(ctx);
-
-    // check
-    if (ok != HAL_OK) return 0;
-
-    // make sure, that write enable did the job
-    int wel = _n25q128_get_wel_flag(ctx);
-    if (wel != 1) return 0;
+    if (_n25q128_write_enable(ctx) != 1) return 0;
 
     // calculate byte address
-    page_offset *= N25Q128_PAGE_SIZE;
+    uint32_t byte_offset = page_offset * N25Q128_PAGE_SIZE;
 
     // prepare PROGRAM PAGE command
     spi_tx[0] = N25Q128_COMMAND_PAGE_PROGRAM;
-    spi_tx[1] = (uint8_t)(page_offset >> 16);
-    spi_tx[2] = (uint8_t)(page_offset >>  8);
-    spi_tx[3] = (uint8_t)(page_offset >>  0);
+    spi_tx[1] = (uint8_t)(byte_offset >> 16);
+    spi_tx[2] = (uint8_t)(byte_offset >>  8);
+    spi_tx[3] = (uint8_t)(byte_offset >>  0);
 
     // activate, send command
     _n25q128_select(ctx);
@@ -180,52 +206,13 @@ int n25q128_write_page(struct spiflash_ctx *ctx, uint32_t page_offset, const uin
     if (ok != HAL_OK) return 0;
 
     // wait until write finishes
-    if (! _wait_while_wip(ctx, 1000)) return 0;
+    if (! _n25q128_wait_while_wip(ctx, 1000)) return 0;
 
     // done
     return 1;
 }
 
 
-static int n25q128_get_wip_flag(struct spiflash_ctx *ctx)
-{
-    // tx, rx buffers
-    uint8_t spi_tx[2];
-    uint8_t spi_rx[2];
-
-    // result
-    HAL_StatusTypeDef ok;
-
-    // send READ STATUS command
-    spi_tx[0] = N25Q128_COMMAND_READ_STATUS;
-
-    // send command, read response, deselect
-    _n25q128_select(ctx);
-    ok = HAL_SPI_TransmitReceive(ctx->hspi, spi_tx, spi_rx, 2, N25Q128_SPI_TIMEOUT);
-    _n25q128_deselect(ctx);
-
-    // check
-    if (ok != HAL_OK) return -1;
-
-    // done
-    return (spi_rx[1] & 1);
-}
-
-/* Wait until the flash memory is done writing (wip = Write In Progress) */
-static int _wait_while_wip(struct spiflash_ctx *ctx, uint32_t timeout)
-{
-    uint32_t tick_end = HAL_GetTick() + timeout;
-    int i;
-
-    do {
-	i = n25q128_get_wip_flag(ctx);
-	if (i < 0) return 0;
-	if (! i) return 1;
-    } while (HAL_GetTick() < tick_end);
-
-    return 0;
-}
-
 static int n25q128_erase_something(struct spiflash_ctx *ctx, uint8_t command, uint32_t byte_offset)
 {
     // check offset
@@ -238,19 +225,7 @@ static int n25q128_erase_something(struct spiflash_ctx *ctx, uint8_t command, ui
     HAL_StatusTypeDef ok;
 
     // enable writing
-    spi_tx[0] = N25Q128_COMMAND_WRITE_ENABLE;
-
-    // select, send command, deselect
-    _n25q128_select(ctx);
-    ok = HAL_SPI_Transmit(ctx->hspi, spi_tx, 1, N25Q128_SPI_TIMEOUT);
-    _n25q128_deselect(ctx);
-
-    // check
-    if (ok != HAL_OK) return 0;
-
-    // make sure, that write enable did the job
-    int wel = _n25q128_get_wel_flag(ctx);
-    if (wel != 1) return 0;
+    if (_n25q128_write_enable(ctx) != 1) return 0;
 
     // send command (ERASE SECTOR or ERASE SUBSECTOR)
     spi_tx[0] = command;
@@ -268,7 +243,7 @@ static int n25q128_erase_something(struct spiflash_ctx *ctx, uint8_t command, ui
 
     // wait for erase to finish
 
-    if (! _wait_while_wip(ctx, 1000)) return 0;
+    if (! _n25q128_wait_while_wip(ctx, 1000)) return 0;
 
     // done
     return 1;
@@ -298,19 +273,7 @@ int n25q128_erase_bulk(struct spiflash_ctx *ctx)
     HAL_StatusTypeDef ok;
 
     // enable writing
-    spi_tx[0] = N25Q128_COMMAND_WRITE_ENABLE;
-
-    // select, send command, deselect
-    _n25q128_select(ctx);
-    ok = HAL_SPI_Transmit(ctx->hspi, spi_tx, 1, N25Q128_SPI_TIMEOUT);
-    _n25q128_deselect(ctx);
-
-    // check
-    if (ok != HAL_OK) return 0;
-
-    // make sure, that write enable did the job
-    int wel = _n25q128_get_wel_flag(ctx);
-    if (wel != 1) return 0;
+    if (_n25q128_write_enable(ctx) != 1) return 0;
 
     // send command
     spi_tx[0] = N25Q128_COMMAND_ERASE_BULK;
@@ -325,41 +288,13 @@ int n25q128_erase_bulk(struct spiflash_ctx *ctx)
 
     // wait for erase to finish
 
-    if (! _wait_while_wip(ctx, 60000)) return 0;
+    if (! _n25q128_wait_while_wip(ctx, 60000)) return 0;
 
     // done
     return 1;
 }
 
 
-/*
- * Read the Write Enable Latch bit in the status register.
- */
-static int _n25q128_get_wel_flag(struct spiflash_ctx *ctx)
-{
-    // tx, rx buffers
-    uint8_t spi_tx[2];
-    uint8_t spi_rx[2];
-
-    // result
-    HAL_StatusTypeDef ok;
-
-    // send READ STATUS command
-    spi_tx[0] = N25Q128_COMMAND_READ_STATUS;
-
-    // send command, read response, deselect
-    _n25q128_select(ctx);
-    ok = HAL_SPI_TransmitReceive(ctx->hspi, spi_tx, spi_rx, 2, N25Q128_SPI_TIMEOUT);
-    _n25q128_deselect(ctx);
-
-    // check
-    if (ok != HAL_OK) return -1;
-
-    // done
-    return ((spi_rx[1] >> 1) & 1);
-}
-
-
 /* This function writes of a number of pages to the flash memory.
  * The caller is responsible for ensuring that the pages have been erased.
  */
@@ -367,7 +302,24 @@ int n25q128_write_data(struct spiflash_ctx *ctx, uint32_t offset, const uint8_t
 {
     uint32_t page;
 
-    /* Ensure alignment */
+    /*
+     * The data sheet says:
+     * If the bits of the least significant address, which is the starting
+     * address, are not all zero, all data transmitted beyond the end of the
+     * current page is programmed from the starting address of the same page.
+     * If the number of bytes sent to the device exceed the maximum page size,
+     * previously latched data is discarded and only the last maximum
+     * page-size number of data bytes are guaranteed to be programmed
+     * correctly within the same page. If the number of bytes sent to the
+     * device is less than the maximum page size, they are correctly
+     * programmed at the specified addresses without any effect on the other
+     * bytes of the same page.
+     *
+     * This is sufficiently confusing that it makes sense to constrain the API
+     * to page alignment in address and length, because that's how we're using
+     * it anyway.
+     */
+
     if ((offset % N25Q128_PAGE_SIZE) != 0) return -1;
     if ((len % N25Q128_PAGE_SIZE) != 0) return -2;
 
@@ -377,10 +329,6 @@ int n25q128_write_data(struct spiflash_ctx *ctx, uint32_t offset, const uint8_t
 	}
 	buf += N25Q128_PAGE_SIZE;
 	offset += N25Q128_PAGE_SIZE;
-
-	/* XXX read back data and verify it, or maybe just verify ability to write
-	 * to memory by verifying the contents of one page after erase?
-	 */
     }
 
     return 1;
@@ -389,19 +337,50 @@ int n25q128_write_data(struct spiflash_ctx *ctx, uint32_t offset, const uint8_t
 /* This function reads zero or more pages from the SPI flash. */
 int n25q128_read_data(struct spiflash_ctx *ctx, uint32_t offset, uint8_t *buf, const uint32_t len)
 {
-    uint32_t page;
+    // tx buffer
+    uint8_t spi_tx[4];
 
-    /* Ensure alignment */
-    if ((offset % N25Q128_PAGE_SIZE) != 0) return -1;
-    if ((len % N25Q128_PAGE_SIZE) != 0) return -2;
+    // result
+    HAL_StatusTypeDef ok;
 
-    for (page = 0; page < len / N25Q128_PAGE_SIZE; page++) {
-	if (! n25q128_read_page(ctx, offset / N25Q128_PAGE_SIZE, buf)) {
-	    return -3;
-	}
-	buf += N25Q128_PAGE_SIZE;
-	offset += N25Q128_PAGE_SIZE;
+    /*
+     * The data sheet says:
+     * The addressed byte can be at any location, and the address
+     * automatically increments to the next address after each byte of data is
+     * shifted out; therefore, the entire memory can be read with a single
+     * command. The operation is terminated by driving S# [chip select] HIGH
+     * at any time during data output.
+     *
+     * We're only going to call this with page-aligned address and length, but
+     * we're not going to enforce it here.
+     */
+
+    // avoid overflow
+    if (offset + len > N25Q128_NUM_BYTES) return -3;
+
+    // prepare READ command
+    spi_tx[0] = N25Q128_COMMAND_READ;
+    spi_tx[1] = (uint8_t)(offset >> 16);
+    spi_tx[2] = (uint8_t)(offset >>  8);
+    spi_tx[3] = (uint8_t)(offset >>  0);
+
+    // activate, send command
+    _n25q128_select(ctx);
+    ok = HAL_SPI_Transmit(ctx->hspi, spi_tx, 4, N25Q128_SPI_TIMEOUT);
+
+    // check
+    if (ok != HAL_OK) {
+	_n25q128_deselect(ctx);
+	return 0;
     }
 
+    // read response, deselect
+    ok = HAL_SPI_Receive(ctx->hspi, buf, len, N25Q128_SPI_TIMEOUT);
+    _n25q128_deselect(ctx);
+
+    // check
+    if (ok != HAL_OK) return 0;
+
+    // done
     return 1;
 }
diff --git a/spiflash_n25q128.h b/spiflash_n25q128.h
index ff14c34..50243bd 100644
--- a/spiflash_n25q128.h
+++ b/spiflash_n25q128.h
@@ -6,7 +6,7 @@
  * The Alpha board has two of these SPI flash memorys, the FPGA config memory
  * and the keystore memory.
  *
- * Copyright (c) 2016, NORDUnet A/S All rights reserved.
+ * Copyright (c) 2016-2017, NORDUnet A/S All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
@@ -40,9 +40,9 @@
 
 #include "stm32f4xx_hal.h"
 
-#define N25Q128_COMMAND_READ_ID		0x9E
-#define N25Q128_COMMAND_READ_PAGE	0x03
+#define N25Q128_COMMAND_READ		0x03
 #define N25Q128_COMMAND_READ_STATUS	0x05
+#define N25Q128_COMMAND_READ_ID		0x9E
 #define N25Q128_COMMAND_WRITE_ENABLE	0x06
 #define N25Q128_COMMAND_ERASE_SECTOR	0xD8
 #define N25Q128_COMMAND_ERASE_SUBSECTOR	0x20
@@ -71,12 +71,16 @@ struct spiflash_ctx {
 };
 
 extern int n25q128_check_id(struct spiflash_ctx *ctx);
-extern int n25q128_read_page(struct spiflash_ctx *ctx, uint32_t page_offset, uint8_t *page_buffer);
+extern int n25q128_read_data(struct spiflash_ctx *ctx, uint32_t offset, uint8_t *buf, const uint32_t len);
 extern int n25q128_write_page(struct spiflash_ctx *ctx, uint32_t page_offset, const uint8_t *page_buffer);
-extern int n25q128_erase_sector(struct spiflash_ctx *ctx, uint32_t sector_offset);
+extern int n25q128_write_data(struct spiflash_ctx *ctx, uint32_t offset, const uint8_t *buf, const uint32_t len);
 extern int n25q128_erase_subsector(struct spiflash_ctx *ctx, uint32_t subsector_offset);
+extern int n25q128_erase_sector(struct spiflash_ctx *ctx, uint32_t sector_offset);
 extern int n25q128_erase_bulk(struct spiflash_ctx *ctx);
 
-extern int n25q128_write_data(struct spiflash_ctx *ctx, uint32_t offset, const uint8_t *buf, const uint32_t len);
-extern int n25q128_read_data(struct spiflash_ctx *ctx, uint32_t offset, uint8_t *buf, const uint32_t len);
+#define n25q128_read_page(ctx, page_offset, page_buffer) \
+    n25q128_read_data(ctx, page_offset * N25Q128_PAGE_SIZE, page_buffer, N25Q128_PAGE_SIZE)
+#define n25q128_read_subsector(ctx, subsector_offset, subsector_buffer) \
+    n25q128_read_data(ctx, subsector_offset * N25Q128_SUBSECTOR_SIZE, subsector_buffer, N25Q128_SUBSECTOR_SIZE)
+
 #endif /* __STM32_SPIFLASH_N25Q128_H */
diff --git a/stm-flash.c b/stm-flash.c
index fc79ea0..4456f44 100644
--- a/stm-flash.c
+++ b/stm-flash.c
@@ -70,7 +70,7 @@ uint32_t flash_sector_offsets[FLASH_NUM_SECTORS + 1] = {
     0x08200000  /* first address *after* flash */
 };
 
-int stm_flash_sector_num(const uint32_t offset)
+static int stm_flash_sector_num(const uint32_t offset)
 {
     int i;
 
diff --git a/stm-flash.h b/stm-flash.h
index a9cf7db..aecb87f 100644
--- a/stm-flash.h
+++ b/stm-flash.h
@@ -35,7 +35,6 @@
 #ifndef __STM32_FLASH_H
 #define __STM32_FLASH_H
 
-extern int stm_flash_sector_num(const uint32_t offset);
 extern int stm_flash_erase_sectors(const uint32_t start_offset, const uint32_t end_offset);
 extern int stm_flash_write32(const uint32_t offset, const uint32_t *buf, const uint32_t elements);
 
diff --git a/stm-fpgacfg.c b/stm-fpgacfg.c
index 16c490b..10abc57 100644
--- a/stm-fpgacfg.c
+++ b/stm-fpgacfg.c
@@ -4,7 +4,7 @@
  * Functions for accessing the FPGA config memory and controlling
  * the low-level status of the FPGA (reset registers/reboot etc.).
  *
- * Copyright (c) 2016, NORDUnet A/S All rights reserved.
+ * Copyright (c) 2016-2017, NORDUnet A/S All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
@@ -48,11 +48,6 @@ int fpgacfg_check_id()
 
 int fpgacfg_write_data(uint32_t offset, const uint8_t *buf, const uint32_t len)
 {
-    if ((offset % N25Q128_SECTOR_SIZE) == 0)
-	// first page in sector, need to erase sector
-	if (! n25q128_erase_sector(&fpgacfg_ctx, offset / N25Q128_SECTOR_SIZE))
-	    return -4;
-
     return n25q128_write_data(&fpgacfg_ctx, offset, buf, len);
 }
 
@@ -97,14 +92,7 @@ int fpgacfg_check_done(void)
     return (status == GPIO_PIN_SET);
 }
 
-int fpgacfg_erase_sectors(int num)
+int fpgacfg_erase_sector(uint32_t sector_offset)
 {
-    if (num > N25Q128_NUM_SECTORS || num < 0) num = N25Q128_NUM_SECTORS;
-    while (num) {
-	if (! n25q128_erase_sector(&fpgacfg_ctx, num - 1)) {
-            return -1;
-        }
-	num--;
-    }
-    return 1;
+    return n25q128_erase_sector(&fpgacfg_ctx, sector_offset);
 }
diff --git a/stm-fpgacfg.h b/stm-fpgacfg.h
index 0d5eeb9..9baebc4 100644
--- a/stm-fpgacfg.h
+++ b/stm-fpgacfg.h
@@ -4,7 +4,7 @@
  * Functions and defines for accessing the FPGA config memory and controlling
  * the low-level status of the FPGA (reset registers/reboot etc.).
  *
- * Copyright (c) 2016, NORDUnet A/S All rights reserved.
+ * Copyright (c) 2016-2017, NORDUnet A/S All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
@@ -39,6 +39,8 @@
 #include "stm32f4xx_hal.h"
 #include "spiflash_n25q128.h"
 
+#define FPGACFG_SECTOR_SIZE		N25Q128_SECTOR_SIZE
+
 /* Pins connected to the FPGA config memory (SPI flash) */
 #define PROM_FPGA_DIS_Pin              GPIO_PIN_14
 #define PROM_FPGA_DIS_GPIO_Port        GPIOI
@@ -87,7 +89,7 @@ extern SPI_HandleTypeDef hspi_fpgacfg;
 
 extern int fpgacfg_check_id(void);
 extern int fpgacfg_write_data(uint32_t offset, const uint8_t *buf, const uint32_t len);
-extern int fpgacfg_erase_sectors(int num);
+extern int fpgacfg_erase_sector(uint32_t sector_offset);
 extern void fpgacfg_access_control(enum fpgacfg_access_ctrl access);
 /* Reset the FPGA */
 extern void fpgacfg_reset_fpga(enum fpgacfg_reset reset);
diff --git a/stm-keystore.c b/stm-keystore.c
index 86fad91..b941489 100644
--- a/stm-keystore.c
+++ b/stm-keystore.c
@@ -3,7 +3,7 @@
  * ----------
  * Functions for accessing the keystore memory.
  *
- * Copyright (c) 2016, NORDUnet A/S All rights reserved.
+ * Copyright (c) 2016-2017, NORDUnet A/S All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
@@ -55,35 +55,31 @@ int keystore_write_data(uint32_t offset, const uint8_t *buf, const uint32_t len)
     return n25q128_write_data(&keystore_ctx, offset, buf, len);
 }
 
-static int keystore_erase_something(uint32_t start, uint32_t stop, uint32_t limit,
-				    int (*eraser)(struct spiflash_ctx *, uint32_t))
+int keystore_erase_subsector(uint32_t subsector_offset)
 {
-    uint32_t something;
-
-    if (start > limit) return -2;
-    if (stop > limit || stop < start) return -3;
-
-    for (something = start; something <= stop; something++) {
-	if (! eraser(&keystore_ctx, something)) {
-            return -1;
-        }
-    }
-    return 1;
+    return n25q128_erase_subsector(&keystore_ctx, subsector_offset);
 }
 
-int keystore_erase_sectors(uint32_t start, uint32_t stop)
+int keystore_erase_sector(uint32_t sector_offset)
 {
-    return keystore_erase_something(start, stop, N25Q128_NUM_SECTORS,
-				    n25q128_erase_sector);
+    return n25q128_erase_sector(&keystore_ctx, sector_offset);
 }
 
-int keystore_erase_subsectors(uint32_t start, uint32_t stop)
+int keystore_erase_bulk(void)
 {
-    return keystore_erase_something(start, stop, N25Q128_NUM_SUBSECTORS,
-				    n25q128_erase_subsector);
+    return n25q128_erase_bulk(&keystore_ctx);
 }
 
-int keystore_erase_bulk(void)
+/*
+ * Deprecated, will be removed when we fix libhal/ks_flash.c to use the
+ * new function. I love inter-dependent repos.
+ */
+
+int keystore_erase_subsectors(uint32_t start, uint32_t stop)
 {
-    return n25q128_erase_bulk(&keystore_ctx);
+    for (int i = start; i <= stop; ++i) {
+        if (keystore_erase_subsector(i) != 1)
+            return 0;
+    }
+    return 1;
 }
diff --git a/stm-keystore.h b/stm-keystore.h
index f27687a..419e12e 100644
--- a/stm-keystore.h
+++ b/stm-keystore.h
@@ -3,7 +3,7 @@
  * ---------
  * Functions and defines for accessing the keystore memory.
  *
- * Copyright (c) 2016, NORDUnet A/S All rights reserved.
+ * Copyright (c) 2016-2017, NORDUnet A/S All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
@@ -60,8 +60,11 @@ extern SPI_HandleTypeDef hspi_keystore;
 extern int keystore_check_id(void);
 extern int keystore_read_data(uint32_t offset, uint8_t *buf, const uint32_t len);
 extern int keystore_write_data(uint32_t offset, const uint8_t *buf, const uint32_t len);
-extern int keystore_erase_sectors(uint32_t start, uint32_t stop);
-extern int keystore_erase_subsectors(uint32_t start, uint32_t stop);
+extern int keystore_erase_subsector(uint32_t subsector_offset);
+extern int keystore_erase_sector(uint32_t sector_offset);
 extern int keystore_erase_bulk(void);
 
+/* Deprecated, will be removed. */
+extern int keystore_erase_subsectors(uint32_t start, uint32_t stop);
+
 #endif /* __STM32_KEYSTORE_H */