Skip to content

Commit

Permalink
libflash/blocklevel: Return region start from ecc_protected()
Browse files Browse the repository at this point in the history
Currently all ecc_protected() does is say if a region is ECC protected
or not. Knowing a region is ECC protected is one thing but there isn't
much that can be done afterwards if this is the only known fact. A lot
more can be done if the caller is told where the ECC region begins.

Knowing where the ECC region start it allows to caller to align its
read/and writes. This allows for more flexibility calling read and write
without knowing exactly how the backing store is organised.

Signed-off-by: Cyril Bur <cyril.bur@au1.ibm.com>
Signed-off-by: Stewart Smith <stewart@linux.ibm.com>
  • Loading branch information
cyrilbur-ibm authored and stewartsmith committed Apr 9, 2018
1 parent f771351 commit 3df9b3c
Show file tree
Hide file tree
Showing 2 changed files with 63 additions and 22 deletions.
63 changes: 52 additions & 11 deletions libflash/blocklevel.c
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
#include <string.h>
#include <inttypes.h>

#include <libflash/libflash.h>
#include <libflash/errors.h>

#include "blocklevel.h"
Expand All @@ -34,7 +35,7 @@
* 0 - The region is not ECC protected
* -1 - Partially protected
*/
static int ecc_protected(struct blocklevel_device *bl, uint64_t pos, uint64_t len)
static int ecc_protected(struct blocklevel_device *bl, uint64_t pos, uint64_t len, uint64_t *start)
{
int i;

Expand All @@ -44,17 +45,25 @@ static int ecc_protected(struct blocklevel_device *bl, uint64_t pos, uint64_t le

for (i = 0; i < bl->ecc_prot.n_prot; i++) {
/* Fits entirely within the range */
if (bl->ecc_prot.prot[i].start <= pos && bl->ecc_prot.prot[i].start + bl->ecc_prot.prot[i].len >= pos + len)
if (bl->ecc_prot.prot[i].start <= pos &&
bl->ecc_prot.prot[i].start + bl->ecc_prot.prot[i].len >= pos + len) {
if (start)
*start = bl->ecc_prot.prot[i].start;
return 1;
}

/*
* Since we merge regions on inserting we can be sure that a
* partial fit means that the non fitting region won't fit in another ecc
* region
*/
if ((bl->ecc_prot.prot[i].start >= pos && bl->ecc_prot.prot[i].start < pos + len) ||
(bl->ecc_prot.prot[i].start <= pos && bl->ecc_prot.prot[i].start + bl->ecc_prot.prot[i].len > pos))
(bl->ecc_prot.prot[i].start <= pos &&
bl->ecc_prot.prot[i].start + bl->ecc_prot.prot[i].len > pos)) {
if (start)
*start = bl->ecc_prot.prot[i].start;
return -1;
}
}
return 0;
}
Expand Down Expand Up @@ -101,20 +110,35 @@ int blocklevel_raw_read(struct blocklevel_device *bl, uint64_t pos, void *buf, u

int blocklevel_read(struct blocklevel_device *bl, uint64_t pos, void *buf, uint64_t len)
{
int rc;
int rc, ecc_protection;
struct ecc64 *buffer;
uint64_t ecc_len = ecc_buffer_size(len);
uint64_t ecc_start, ecc_len = ecc_buffer_size(len);

FL_DBG("%s: 0x%" PRIx64 "\t%p\t0x%" PRIx64 "\n", __func__, pos, buf, len);
if (!bl || !buf) {
errno = EINVAL;
return FLASH_ERR_PARM_ERROR;
}

if (!ecc_protected(bl, pos, len))
ecc_protection = ecc_protected(bl, pos, len, &ecc_start);

FL_DBG("%s: 0x%" PRIx64 " for 0x%" PRIx64 " ecc=%s",
__func__, pos, len, ecc_protection ?
(ecc_protection == -1 ? "partial" : "yes") : "no");

if (!ecc_protection)
return blocklevel_raw_read(bl, pos, buf, len);

FL_DBG("%s: region has ECC\n", __func__);
/*
* The region we're reading to has both ecc protection and not.
* Perhaps one day in the future blocklevel can cope with this.
*/
if (ecc_protection == -1) {
FL_ERR("%s: Can't cope with partial ecc\n", __func__);
errno = EINVAL;
return FLASH_ERR_PARM_ERROR;
}

buffer = malloc(ecc_len);
if (!buffer) {
errno = ENOMEM;
Expand Down Expand Up @@ -161,20 +185,36 @@ int blocklevel_raw_write(struct blocklevel_device *bl, uint64_t pos,
int blocklevel_write(struct blocklevel_device *bl, uint64_t pos, const void *buf,
uint64_t len)
{
int rc;
int rc, ecc_protection;
struct ecc64 *buffer;
uint64_t ecc_len = ecc_buffer_size(len);
uint64_t ecc_start;

FL_DBG("%s: 0x%" PRIx64 "\t%p\t0x%" PRIx64 "\n", __func__, pos, buf, len);
if (!bl || !buf) {
errno = EINVAL;
return FLASH_ERR_PARM_ERROR;
}

if (!ecc_protected(bl, pos, len))
ecc_protection = ecc_protected(bl, pos, len, &ecc_start);

FL_DBG("%s: 0x%" PRIx64 " for 0x%" PRIx64 " ecc=%s",
__func__, pos, len, ecc_protection ?
(ecc_protection == -1 ? "partial" : "yes") : "no");

if (!ecc_protection)
return blocklevel_raw_write(bl, pos, buf, len);

FL_DBG("%s: region has ECC\n", __func__);
/*
* The region we're writing to has both ecc protection and not.
* Perhaps one day in the future blocklevel can cope with this.
*/
if (ecc_protection == -1) {
FL_ERR("%s: Can't cope with partial ecc\n", __func__);
errno = EINVAL;
return FLASH_ERR_PARM_ERROR;
}

buffer = malloc(ecc_len);
if (!buffer) {
errno = ENOMEM;
Expand Down Expand Up @@ -420,6 +460,7 @@ int blocklevel_smart_write(struct blocklevel_device *bl, uint64_t pos, const voi
uint32_t erase_size;
const void *write_buf = buf;
void *write_buf_start = NULL;
uint64_t ecc_start;
void *erase_buf;
int rc = 0;

Expand All @@ -439,7 +480,7 @@ int blocklevel_smart_write(struct blocklevel_device *bl, uint64_t pos, const voi
if (rc)
return rc;

if (ecc_protected(bl, pos, len)) {
if (ecc_protected(bl, pos, len, &ecc_start)) {
FL_DBG("%s: region has ECC\n", __func__);

len = ecc_buffer_size(len);
Expand Down
22 changes: 11 additions & 11 deletions libflash/test/test-blocklevel.c
Original file line number Diff line number Diff line change
Expand Up @@ -162,17 +162,17 @@ int main(void)
return 1;
}

if (ecc_protected(bl, 0, 1) != 1) {
if (ecc_protected(bl, 0, 1, NULL) != 1) {
ERR("Invaid result for ecc_protected(0, 1)\n");
return 1;
}

if (ecc_protected(bl, 0, 0x1000) != 1) {
if (ecc_protected(bl, 0, 0x1000, NULL) != 1) {
ERR("Invalid result for ecc_protected(0, 0x1000)\n");
return 1;
}

if (ecc_protected(bl, 0x100, 0x100) != 1) {
if (ecc_protected(bl, 0x100, 0x100, NULL) != 1) {
ERR("Invalid result for ecc_protected(0x0100, 0x100)\n");
return 1;
}
Expand All @@ -190,33 +190,33 @@ int main(void)
return 1;
}

if (ecc_protected(bl, 0x1000, 0) != 1) {
if (ecc_protected(bl, 0x1000, 0, NULL) != 1) {
ERR("Invalid result for ecc_protected(0x1000, 0)\n");
return 1;
}

if (ecc_protected(bl, 0x1000, 0x1000) != -1) {
if (ecc_protected(bl, 0x1000, 0x1000, NULL) != -1) {
ERR("Invalid result for ecc_protected(0x1000, 0x1000)\n");
return 1;
}

if (ecc_protected(bl, 0x1000, 0x100) != 1) {
if (ecc_protected(bl, 0x1000, 0x100, NULL) != 1) {
ERR("Invalid result for ecc_protected(0x1000, 0x100)\n");
return 1;
}

if (ecc_protected(bl, 0x2000, 0) != 0) {
if (ecc_protected(bl, 0x2000, 0, NULL) != 0) {
ERR("Invalid result for ecc_protected(0x2000, 0)\n");
return 1;
}

if (ecc_protected(bl, 0x4000, 1) != 0) {
if (ecc_protected(bl, 0x4000, 1, NULL) != 0) {
ERR("Invalid result for ecc_protected(0x4000, 1)\n");
return 1;
}

/* Check for asking for a region with mixed protection */
if (ecc_protected(bl, 0x100, 0x2000) != -1) {
if (ecc_protected(bl, 0x100, 0x2000, NULL) != -1) {
ERR("Invalid result for ecc_protected(0x100, 0x2000)\n");
return 1;
}
Expand All @@ -237,7 +237,7 @@ int main(void)
return 1;
}

if (ecc_protected(bl, 0x5120, 0x10) != 1) {
if (ecc_protected(bl, 0x5120, 0x10, NULL) != 1) {
ERR("Invalid result for ecc_protected(0x5120, 0x10)\n");
return 1;
}
Expand All @@ -252,7 +252,7 @@ int main(void)
return 1;
}

if (ecc_protected(bl, 0x4920, 0x10) != 1) {
if (ecc_protected(bl, 0x4920, 0x10, NULL) != 1) {
ERR("Invalid result for ecc_protected(0x4920, 0x10)\n");
return 1;
}
Expand Down

0 comments on commit 3df9b3c

Please sign in to comment.