diff --git a/include/fs/fcb.h b/include/fs/fcb.h index cfcde07d49b..4b006b72169 100644 --- a/include/fs/fcb.h +++ b/include/fs/fcb.h @@ -1,5 +1,5 @@ /* - * Copyright (c) 2017 Nordic Semiconductor ASA + * Copyright (c) 2017-2020 Nordic Semiconductor ASA * Copyright (c) 2015 Runtime Inc * * SPDX-License-Identifier: Apache-2.0 @@ -86,9 +86,12 @@ struct fcb_entry_ctx { struct fcb { /* Caller of fcb_init fills this in */ uint32_t f_magic; - /**< Magic value. It is placed in the beginning of FCB flash sector. - * FCB uses this when determining whether sector contains valid data - * or not. + /**< Magic value, should not be 0xFFFFFFFF. + * It is xored with inversion of f_erase_value and placed in + * the beginning of FCB flash sector. FCB uses this when determining + * whether sector contains valid data or not. + * Giving it value of 0xFFFFFFFF means leaving bytes of the filed + * in "erased" state. */ uint8_t f_version; /**< Current version number of the data */ @@ -121,6 +124,11 @@ struct fcb { /**< Flash area used by the fcb instance, , internal state. * This can be transfer to FCB user */ + + uint8_t f_erase_value; + /**< The value flash takes when it is erased. This is read from + * flash parameters and initialized upon call to fcb_init. + */ }; /** diff --git a/subsys/fs/fcb/fcb.c b/subsys/fs/fcb/fcb.c index aac60d8cc73..782ff1be8b0 100644 --- a/subsys/fs/fcb/fcb.c +++ b/subsys/fs/fcb/fcb.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 2017 Nordic Semiconductor ASA + * Copyright (c) 2017-2020 Nordic Semiconductor ASA * Copyright (c) 2015 Runtime Inc * * SPDX-License-Identifier: Apache-2.0 @@ -12,6 +12,8 @@ #include "fcb_priv.h" #include "string.h" #include +#include +#include uint8_t fcb_get_align(const struct fcb *fcb) @@ -99,6 +101,8 @@ fcb_init(int f_area_id, struct fcb *fcb) int oldest = -1, newest = -1; struct flash_sector *oldest_sector = NULL, *newest_sector = NULL; struct fcb_disk_area fda; + const struct device *dev = NULL; + const struct flash_parameters *fparam; if (!fcb->f_sectors || fcb->f_sector_cnt - fcb->f_scratch_cnt < 1) { return -EINVAL; @@ -109,6 +113,10 @@ fcb_init(int f_area_id, struct fcb *fcb) return -EINVAL; } + dev = device_get_binding(fcb->fap->fa_dev_name); + fparam = flash_get_parameters(dev); + fcb->f_erase_value = fparam->erase_value; + align = fcb_get_align(fcb); if (align == 0U) { return -EINVAL; @@ -194,16 +202,33 @@ fcb_is_empty(struct fcb *fcb) /** * Length of an element is encoded in 1 or 2 bytes. * 1 byte for lengths < 128 bytes, and 2 bytes for < 16384. + * + * The storage of length has been originally designed to work with 0xff erasable + * flash devices and gives length 0xffff special meaning: that there is no value + * written; this is smart way to utilize value in non-written flash to figure + * out where data ends. Additionally it sets highest bit of first byte of + * the length to 1, to mark that there is second byte to be read. + * Above poses some problems when non-0xff erasable flash is used. To solve + * the problem all length values are xored with not of erase value for given + * flash: + * len' = len ^ ~erase_value; + * To obtain original value, the logic is reversed: + * len = len' ^ ~erase_value; + * + * In case of 0xff erased flash this does not modify data that is written to + * flash; in case of other flash devices, e.g. that erase to 0x00, it allows + * to correctly use the first bit of byte to figure out how many bytes are there + * and if there is any data at all or both bytes are equal to erase value. */ int -fcb_put_len(uint8_t *buf, uint16_t len) +fcb_put_len(const struct fcb *fcb, uint8_t *buf, uint16_t len) { if (len < 0x80) { - buf[0] = len; + buf[0] = len ^ ~fcb->f_erase_value; return 1; } else if (len < FCB_MAX_LEN) { - buf[0] = (len & 0x7f) | 0x80; - buf[1] = len >> 7; + buf[0] = (len | 0x80) ^ ~fcb->f_erase_value; + buf[1] = (len >> 7) ^ ~fcb->f_erase_value; return 2; } else { return -EINVAL; @@ -211,18 +236,20 @@ fcb_put_len(uint8_t *buf, uint16_t len) } int -fcb_get_len(uint8_t *buf, uint16_t *len) +fcb_get_len(const struct fcb *fcb, uint8_t *buf, uint16_t *len) { int rc; - if (buf[0] & 0x80) { - if (buf[0] == 0xff && buf[1] == 0xff) { + if ((buf[0] ^ ~fcb->f_erase_value) & 0x80) { + if ((buf[0] == fcb->f_erase_value) && + (buf[1] == fcb->f_erase_value)) { return -ENOTSUP; } - *len = (buf[0] & 0x7f) | (buf[1] << 7); + *len = ((buf[0] ^ ~fcb->f_erase_value) & 0x7f) | + ((uint8_t)(buf[1] ^ ~fcb->f_erase_value) << 7); rc = 2; } else { - *len = buf[0]; + *len = (uint8_t)(buf[0] ^ ~fcb->f_erase_value); rc = 1; } return rc; @@ -237,9 +264,9 @@ fcb_sector_hdr_init(struct fcb *fcb, struct flash_sector *sector, uint16_t id) struct fcb_disk_area fda; int rc; - fda.fd_magic = fcb->f_magic; + fda.fd_magic = fcb_flash_magic(fcb); fda.fd_ver = fcb->f_version; - fda._pad = 0xff; + fda._pad = fcb->f_erase_value; fda.fd_id = id; rc = fcb_flash_write(fcb, sector, 0, &fda, sizeof(fda)); @@ -268,10 +295,10 @@ int fcb_sector_hdr_read(struct fcb *fcb, struct flash_sector *sector, if (rc) { return -EIO; } - if (fdap->fd_magic == 0xffffffff) { + if (fdap->fd_magic == MK32(fcb->f_erase_value)) { return 0; } - if (fdap->fd_magic != fcb->f_magic) { + if (fdap->fd_magic != fcb_flash_magic(fcb)) { return -ENOMSG; } return 1; diff --git a/subsys/fs/fcb/fcb_append.c b/subsys/fs/fcb/fcb_append.c index d7a90241a50..92e0bd209c4 100644 --- a/subsys/fs/fcb/fcb_append.c +++ b/subsys/fs/fcb/fcb_append.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 2017 Nordic Semiconductor ASA + * Copyright (c) 2017-2020 Nordic Semiconductor ASA * Copyright (c) 2015 Runtime Inc * * SPDX-License-Identifier: Apache-2.0 @@ -65,7 +65,7 @@ fcb_append(struct fcb *fcb, uint16_t len, struct fcb_entry *append_loc) int rc; uint8_t tmp_str[8]; - cnt = fcb_put_len(tmp_str, len); + cnt = fcb_put_len(fcb, tmp_str, len); if (cnt < 0) { return cnt; } diff --git a/subsys/fs/fcb/fcb_elem_info.c b/subsys/fs/fcb/fcb_elem_info.c index 9aff6fe7b20..946b7dbf2cc 100644 --- a/subsys/fs/fcb/fcb_elem_info.c +++ b/subsys/fs/fcb/fcb_elem_info.c @@ -34,7 +34,7 @@ fcb_elem_crc8(struct fcb *fcb, struct fcb_entry *loc, uint8_t *c8p) return -EIO; } - cnt = fcb_get_len(tmp_str, &len); + cnt = fcb_get_len(fcb, tmp_str, &len); if (cnt < 0) { return cnt; } diff --git a/subsys/fs/fcb/fcb_priv.h b/subsys/fs/fcb/fcb_priv.h index 59b03211728..ea2643569a0 100644 --- a/subsys/fs/fcb/fcb_priv.h +++ b/subsys/fs/fcb/fcb_priv.h @@ -1,5 +1,5 @@ /* - * Copyright (c) 2017 Nordic Semiconductor ASA + * Copyright (c) 2017-2020 Nordic Semiconductor ASA * Copyright (c) 2015 Runtime Inc * * SPDX-License-Identifier: Apache-2.0 @@ -17,6 +17,31 @@ extern "C" { #define FCB_ID_GT(a, b) (((int16_t)(a) - (int16_t)(b)) > 0) +#define MK32(val) ((((uint32_t)(val)) << 24) | \ + (((uint32_t)((val) & 0xff)) << 16) | \ + (((uint32_t)((val) & 0xff)) << 8) | \ + (((uint32_t)((val) & 0xff)))) + + +/* @brief Gets magic value in flash formatted version + * + * Magic, the fcb->f_magic, prior to being written to flash, is xored with + * binary inversion of fcb->f_erase_value to avoid it matching a 4 consecutive + * bytes of flash erase value, which is used to recognize end of records, + * by accident. Only the value of 0xFFFFFFFF will be always written as + * 4 bytes of erase value. + * + * @param fcb pointer to initialized fcb structure + * + * @return uin32_t formatted magic value + */ +static inline uint32_t fcb_flash_magic(const struct fcb *fcb) +{ + const uint8_t ev = fcb->f_erase_value; + + return (fcb->f_magic ^ ~MK32(ev)); +} + struct fcb_disk_area { uint32_t fd_magic; uint8_t fd_ver; @@ -24,8 +49,8 @@ struct fcb_disk_area { uint16_t fd_id; }; -int fcb_put_len(uint8_t *buf, uint16_t len); -int fcb_get_len(uint8_t *buf, uint16_t *len); +int fcb_put_len(const struct fcb *fcb, uint8_t *buf, uint16_t len); +int fcb_get_len(const struct fcb *fcb, uint8_t *buf, uint16_t *len); static inline int fcb_len_in_flash(struct fcb *fcb, uint16_t len) {