[PATCH 1/2] Factorize flash_erase function

75 views
Skip to first unread message

Stefano Babic

unread,
May 12, 2017, 6:51:58 AM5/12/17
to swup...@googlegroups.com, Stefano Babic
Move flash_erase outside of flash handler to be called by other
functions.

Signed-off-by: Stefano Babic <sba...@denx.de>
---
handlers/flash_handler.c | 110 -----------------------------------------------
include/flash.h | 4 +-
2 files changed, 3 insertions(+), 111 deletions(-)

diff --git a/handlers/flash_handler.c b/handlers/flash_handler.c
index 3e2ad22..650a4b0 100644
--- a/handlers/flash_handler.c
+++ b/handlers/flash_handler.c
@@ -69,116 +69,6 @@
void flash_handler(void);
void flash_1bit_hamming_handler(void);

-/*
- * Note: the functions here are derived directly
- * with minor changes from mtd-utils.
- */
-#define EMPTY_BYTE 0xFF
-
-static int flash_erase(int mtdnum)
-{
- int fd;
- char mtd_device[LINESIZE];
- struct mtd_dev_info *mtd;
- int noskipbad = 0;
- int unlock = 0;
- int ret = 0;
- unsigned int eb, eb_start, eb_cnt, i;
- uint8_t *buf;
- struct flash_description *flash = get_flash_info();
-
- if (!mtd_dev_present(flash->libmtd, mtdnum)) {
- ERROR("MTD %d does not exist\n", mtdnum);
- return -ENODEV;
- }
- mtd = &flash->mtd_info[mtdnum].mtd;
- snprintf(mtd_device, sizeof(mtd_device), "/dev/mtd%d", mtdnum);
-
- if ((fd = open(mtd_device, O_RDWR)) < 0) {
- ERROR( "%s: %s: %s", __func__, mtd_device, strerror(errno));
- return -ENODEV;
- }
-
- /*
- * prepare to erase all of the MTD partition,
- */
- buf = (uint8_t *)malloc(mtd->eb_size);
- if (!buf) {
- ERROR("No memory for temporary buffer of %d bytes",
- mtd->eb_size);
- close(fd);
- return -ENOMEM;
- }
-
- eb_start = 0;
- eb_cnt = (mtd->size / mtd->eb_size) - eb_start;
- for (eb = 0; eb < eb_start + eb_cnt; eb++) {
-
- /* Always skip bad sectors */
- if (!noskipbad) {
- int isbad = mtd_is_bad(mtd, fd, eb);
- if (isbad > 0) {
- continue;
- } else if (isbad < 0) {
- if (errno == EOPNOTSUPP) {
- noskipbad = 1;
- } else {
- ERROR("%s: MTD get bad block failed", mtd_device);
- ret = -EFAULT;
- goto erase_out;
- }
- }
- }
-
- /*
- * In case of NOR flash, check if the flash
- * is already empty. This can save
- * an amount of time because erasing
- * a NOR flash is very time expensive.
- * NAND flash is always erased.
- */
- if (!isNand(flash, mtdnum)) {
- if (mtd_read(mtd, fd, eb, 0, buf, mtd->eb_size) != 0) {
- ERROR("%s: MTD Read failure", mtd_device);
- ret = -EIO;
- goto erase_out;
- }
-
- /* check if already empty */
- for (i = 0; i < mtd->eb_size; i++) {
- if (buf[i] != EMPTY_BYTE)
- break;
- }
-
- /* skip erase if empty */
- if (i == mtd->eb_size)
- continue;
-
- }
-
- /* The sector contains data and it must be erased */
- if (unlock) {
- if (mtd_unlock(mtd, fd, eb) != 0) {
- TRACE("%s: MTD unlock failure", mtd_device);
- continue;
- }
- }
-
- if (mtd_erase(flash->libmtd, mtd, fd, eb) != 0) {
- ERROR("%s: MTD Erase failure", mtd_device);
- ret = -EIO;
- goto erase_out;
- }
- }
-
-erase_out:
- free(buf);
-
- close(fd);
-
- return ret;
-}
-
#ifdef CONFIG_CFIHAMMING1
static unsigned char calc_bitwise_parity(unsigned char val, unsigned char mask)
{
diff --git a/include/flash.h b/include/flash.h
index 9a76e8d..7f11568 100644
--- a/include/flash.h
+++ b/include/flash.h
@@ -44,7 +44,8 @@ struct mtd_ubi_info {
struct ubilist ubi_partitions;
struct ubi_attach_request req;
struct mtd_dev_info mtd;
- int skipubi;
+ int skipubi; /* set if no UBI scan must run */
+ int has_ubi; /* set if MTD must always have UBI */
int scanned;
};

@@ -66,6 +67,7 @@ int scan_mtd_devices (void);
void mtd_cleanup (void);
int get_mtd_from_device(char *s);
int get_mtd_from_name(const char *s);
+int flash_erase(int mtdnum);

struct flash_description *get_flash_info(void);
#define isNand(flash, index) \
--
2.7.4

Stefano Babic

unread,
May 12, 2017, 6:52:02 AM5/12/17
to swup...@googlegroups.com, Stefano Babic
Some MTD devices are planned to be used with UBI. If an attach fails,
the whole update stops. However, specially in the factory, it is not
guaranteed which is the status of a flash before it is accessed the
first time and it is not always empty. It can happen that a flash has
some data and an attach fails, and SWUpdate stops with error.
After erasing the flash, everything works as expected.

Introduce a CONFIG_UBIWHITELIST to signal MTD devices that have always
UBI. This has a lower priority as CONFIG_UBIBLACKLIST (if a MTD device
is put in both lists, it will be blacklisted). In case UBI attach fails
and the MTD device is whitelisted, erase once the device and tries once
again to call attach.

Signed-off-by: Stefano Babic <sba...@denx.de>
---
corelib/mtd-interface.c | 207 +++++++++++++++++++++++++++++++++++++++++-------
handlers/Config.in | 12 +++
2 files changed, 189 insertions(+), 30 deletions(-)

diff --git a/corelib/mtd-interface.c b/corelib/mtd-interface.c
index af50ec4..1f8d328 100644
--- a/corelib/mtd-interface.c
+++ b/corelib/mtd-interface.c
@@ -20,6 +20,8 @@

#include <stdio.h>
#include <stdlib.h>
+#include <stdbool.h>
+#include <fcntl.h>
#include <string.h>
#include <mtd/mtd-user.h>
#include <sys/types.h>
@@ -33,6 +35,117 @@

static char mtd_ubi_blacklist[100] = { 0 };

+/*
+ * Note: the functions here are derived directly
+ * with minor changes from mtd-utils.
+ */
+#define EMPTY_BYTE 0xFF
+
+int flash_erase(int mtdnum)
+{
+ int fd;
+ char mtd_device[80];
+ struct mtd_dev_info *mtd;
+ int noskipbad = 0;
+ int unlock = 0;
+ int ret = 0;
+ unsigned int eb, eb_start, eb_cnt, i;
+ uint8_t *buf;
+ struct flash_description *flash = get_flash_info();
+
+ if (!mtd_dev_present(flash->libmtd, mtdnum)) {
+ ERROR("MTD %d does not exist\n", mtdnum);
+ return -ENODEV;
+ }
+ mtd = &flash->mtd_info[mtdnum].mtd;
+ snprintf(mtd_device, sizeof(mtd_device), "/dev/mtd%d", mtdnum);
+
+ if ((fd = open(mtd_device, O_RDWR)) < 0) {
+ ERROR( "%s: %s: %s", __func__, mtd_device, strerror(errno));
+ return -ENODEV;
+ }
+
+ /*
+ * prepare to erase all of the MTD partition,
+ */
+ buf = (uint8_t *)malloc(mtd->eb_size);
+ if (!buf) {
+ ERROR("No memory for temporary buffer of %d bytes",
+ mtd->eb_size);
+ close(fd);
+ return -ENOMEM;
+ }
+
+ eb_start = 0;
+ eb_cnt = (mtd->size / mtd->eb_size) - eb_start;
+ for (eb = 0; eb < eb_start + eb_cnt; eb++) {
+
+ /* Always skip bad sectors */
+ if (!noskipbad) {
+ int isbad = mtd_is_bad(mtd, fd, eb);
+ if (isbad > 0) {
+ continue;
+ } else if (isbad < 0) {
+ if (errno == EOPNOTSUPP) {
+ noskipbad = 1;
+ } else {
+ ERROR("%s: MTD get bad block failed", mtd_device);
+ ret = -EFAULT;
+ goto erase_out;
+ }
+ }
+ }
+
+ /*
+ * In case of NOR flash, check if the flash
+ * is already empty. This can save
+ * an amount of time because erasing
+ * a NOR flash is very time expensive.
+ * NAND flash is always erased.
+ */
+ if (!isNand(flash, mtdnum)) {
+ if (mtd_read(mtd, fd, eb, 0, buf, mtd->eb_size) != 0) {
+ ERROR("%s: MTD Read failure", mtd_device);
+ ret = -EIO;
+ goto erase_out;
+ }
+
+ /* check if already empty */
+ for (i = 0; i < mtd->eb_size; i++) {
+ if (buf[i] != EMPTY_BYTE)
+ break;
+ }
+
+ /* skip erase if empty */
+ if (i == mtd->eb_size)
+ continue;
+
+ }
+
+ /* The sector contains data and it must be erased */
+ if (unlock) {
+ if (mtd_unlock(mtd, fd, eb) != 0) {
+ TRACE("%s: MTD unlock failure", mtd_device);
+ continue;
+ }
+ }
+
+ if (mtd_erase(flash->libmtd, mtd, fd, eb) != 0) {
+ ERROR("%s: MTD Erase failure", mtd_device);
+ ret = -EIO;
+ goto erase_out;
+ }
+ }
+
+erase_out:
+ free(buf);
+
+ close(fd);
+
+ return ret;
+}
+
+
void mtd_init(void)
{
struct flash_description *flash = get_flash_info();
@@ -123,12 +236,18 @@ void ubi_init(void)
}
}

-static void ubi_insert_blacklist(int index, struct flash_description *flash)
+static void ubi_insert_list(int index, struct flash_description *flash, bool black)
{
struct mtd_info *mtd = &flash->mtd;

if (index >= mtd->lowest_mtd_num && index <= mtd->highest_mtd_num) {
- flash->mtd_info[index].skipubi = 1;
+ if (black) {
+ flash->mtd_info[index].skipubi = 1;
+ flash->mtd_info[index].has_ubi = 0;
+ } else {
+ flash->mtd_info[index].skipubi = 0;
+ flash->mtd_info[index].has_ubi = 1;
+ }
}
}

@@ -208,7 +327,7 @@ static void scan_ubi_partitions(int mtd)
{
struct flash_description *flash = get_flash_info();
libubi_t libubi = flash->libubi;
- int err;
+ int err, tryattach = 0;
struct mtd_ubi_info *mtd_info;

if (mtd < 0) {
@@ -238,17 +357,29 @@ static void scan_ubi_partitions(int mtd)
* and tries to get information, if not found
* try to attach.
*/
- err = ubi_attach(libubi, DEFAULT_CTRL_DEV, &mtd_info->req);
- if (err) {
- ERROR("cannot attach mtd%d - maybe not a NAND or raw device", mtd);
- return;
- }
+ do {
+ err = ubi_attach(libubi, DEFAULT_CTRL_DEV, &mtd_info->req);
+ if (err) {
+ if (mtd_info->has_ubi && !tryattach) {
+ TRACE("cannot attach mtd%d ..try erasing", mtd);
+ if (flash_erase(mtd)) {
+ ERROR("mtd%d cannot be erased", mtd);
+ return;
+ }
+ } else {
+ ERROR("cannot attach mtd%d - maybe not a NAND or raw device", mtd);
+ return;
+ }
+ tryattach++;
+ }
+ } while (err != 0 && tryattach < 2);

err = ubi_get_dev_info1(libubi, mtd_info->req.dev_num, &mtd_info->dev_info);
if (err) {
ERROR("cannot get information about UBI device %d", mtd_info->req.dev_num);
return;
}
+
scan_ubi_volumes(mtd_info);
}
#endif
@@ -260,18 +391,11 @@ int scan_mtd_devices (void)
struct mtd_info *mtd_info = &flash->mtd;
struct mtd_ubi_info *mtd_ubi_info;
libmtd_t libmtd = flash->libmtd;
- char blacklist[100] = { 0 };
+ char list[100];
char *token;
char *saveptr;
int i, index;
-
-#if defined(CONFIG_UBIBLACKLIST)
- strncpy(blacklist, CONFIG_UBIBLACKLIST, sizeof(blacklist));
-#endif
-
- /* Blacklist passed on the command line has priority */
- if (strlen(mtd_ubi_blacklist))
- strncpy(blacklist, mtd_ubi_blacklist, sizeof(blacklist));
+ bool black;

if (!libmtd) {
ERROR("MTD is not present on the target");
@@ -293,19 +417,42 @@ int scan_mtd_devices (void)
return -ENOMEM;
}

- token = strtok_r(blacklist, " ", &saveptr);
- if (token) {
- errno = 0;
- index = strtoul(token, NULL, 10);
- if (errno == 0) {
- ubi_insert_blacklist(index, flash);
-
- while ((token = strtok_r(NULL, " ", &saveptr))) {
- errno = 0;
- index = strtoul(token, NULL, 10);
- if (errno != 0)
- break;
- ubi_insert_blacklist(index, flash);
+ for (i = 0; i < 2; i++) {
+ memset(list, 0, sizeof(list));
+ switch (i) {
+ case 0:
+ black = true;
+#if defined(CONFIG_UBIBLACKLIST)
+ strncpy(list, CONFIG_UBIBLACKLIST,
+ sizeof(list));
+#endif
+ /* Blacklist passed on the command line has priority */
+ if (strlen(mtd_ubi_blacklist))
+ strncpy(list, mtd_ubi_blacklist, sizeof(list));
+ break;
+ case 1:
+ black = false;
+#if defined(CONFIG_UBIWHITELIST)
+ strncpy(list, CONFIG_UBIWHITELIST,
+ sizeof(list));
+#endif
+ break;
+ }
+
+ token = strtok_r(list, " ", &saveptr);
+ if (token) {
+ errno = 0;
+ index = strtoul(token, NULL, 10);
+ if (errno == 0) {
+ ubi_insert_list(index, flash, black);
+
+ while ((token = strtok_r(NULL, " ", &saveptr))) {
+ errno = 0;
+ index = strtoul(token, NULL, 10);
+ if (errno != 0)
+ break;
+ ubi_insert_list(index, flash, black);
+ }
}
}
}
diff --git a/handlers/Config.in b/handlers/Config.in
index 5b14397..1576255 100644
--- a/handlers/Config.in
+++ b/handlers/Config.in
@@ -29,6 +29,18 @@ config UBIBLACKLIST
Examples: "0 1 2"
This excludes mtd0-mtd1-mtd2 to be searched for UBI volumes

+config UBIWHITELIST
+ string "List of MTD devices that must have UBI"
+ depends on UBIVOL
+ help
+ Define a list of MTD devices that are planned to have
+ always UBI. If first attach fails, the device is erased
+ and tried again.
+ The list can be set as a string with the mtd numbers.
+ Examples: "0 1 2"
+ This sets mtd0-mtd1-mtd2 to be used as UBI volumes.
+ UBIBLACKLIST has priority on UBIWHITELIST.
+
config UBIVIDOFFSET
int "VID Header Offset"
depends on UBIVOL
--
2.7.4

Stefano Babic

unread,
May 12, 2017, 7:20:55 AM5/12/17
to swup...@googlegroups.com, Stefano Babic
Some MTD devices are planned to be used with UBI. If an attach fails,
the whole update stops. However, specially in the factory, it is not
guaranteed which is the status of a flash before it is accessed the
first time and it is not always empty. It can happen that a flash has
some data and an attach fails, and SWUpdate stops with error.
After erasing the flash, everything works as expected.

Introduce a CONFIG_UBIWHITELIST to signal MTD devices that have always
UBI. This has a lower priority as CONFIG_UBIBLACKLIST (if a MTD device
is put in both lists, it will be blacklisted). In case UBI attach fails
and the MTD device is whitelisted, erase once the device and tries once
again to call attach.

Signed-off-by: Stefano Babic <sba...@denx.de>
---
corelib/mtd-interface.c | 94 +++++++++++++++++++++++++++++++++----------------
handlers/Config.in | 12 +++++++
include/flash.h | 3 +-
3 files changed, 78 insertions(+), 31 deletions(-)

diff --git a/corelib/mtd-interface.c b/corelib/mtd-interface.c
index 64c96ac..1f8d328 100644
--- a/corelib/mtd-interface.c
+++ b/corelib/mtd-interface.c
@@ -236,12 +236,18 @@ void ubi_init(void)
}
}

-static void ubi_insert_blacklist(int index, struct flash_description *flash)
+static void ubi_insert_list(int index, struct flash_description *flash, bool black)
{
struct mtd_info *mtd = &flash->mtd;

if (index >= mtd->lowest_mtd_num && index <= mtd->highest_mtd_num) {
- flash->mtd_info[index].skipubi = 1;
+ if (black) {
+ flash->mtd_info[index].skipubi = 1;
+ flash->mtd_info[index].has_ubi = 0;
+ } else {
+ flash->mtd_info[index].skipubi = 0;
+ flash->mtd_info[index].has_ubi = 1;
+ }
}
}

@@ -321,7 +327,7 @@ static void scan_ubi_partitions(int mtd)
{
struct flash_description *flash = get_flash_info();
libubi_t libubi = flash->libubi;
- int err;
+ int err, tryattach = 0;
struct mtd_ubi_info *mtd_info;

if (mtd < 0) {
@@ -351,17 +357,29 @@ static void scan_ubi_partitions(int mtd)
@@ -373,18 +391,11 @@ int scan_mtd_devices (void)
struct mtd_info *mtd_info = &flash->mtd;
struct mtd_ubi_info *mtd_ubi_info;
libmtd_t libmtd = flash->libmtd;
- char blacklist[100] = { 0 };
+ char list[100];
char *token;
char *saveptr;
int i, index;
-
-#if defined(CONFIG_UBIBLACKLIST)
- strncpy(blacklist, CONFIG_UBIBLACKLIST, sizeof(blacklist));
-#endif
-
- /* Blacklist passed on the command line has priority */
- if (strlen(mtd_ubi_blacklist))
- strncpy(blacklist, mtd_ubi_blacklist, sizeof(blacklist));
+ bool black;

if (!libmtd) {
ERROR("MTD is not present on the target");
@@ -406,19 +417,42 @@ int scan_mtd_devices (void)
diff --git a/include/flash.h b/include/flash.h
index ee3942e..7f11568 100644
--- a/include/flash.h
+++ b/include/flash.h
@@ -44,7 +44,8 @@ struct mtd_ubi_info {
struct ubilist ubi_partitions;
struct ubi_attach_request req;
struct mtd_dev_info mtd;
- int skipubi;
+ int skipubi; /* set if no UBI scan must run */
+ int has_ubi; /* set if MTD must always have UBI */
int scanned;
};

--
2.7.4

Stefano Babic

unread,
May 12, 2017, 7:20:55 AM5/12/17
to swup...@googlegroups.com, Stefano Babic
Move flash_erase outside of flash handler to be called by other
functions.

Signed-off-by: Stefano Babic <sba...@denx.de>
---

Changes since V1: split not correct without changes in mtd_interface

corelib/mtd-interface.c | 113 +++++++++++++++++++++++++++++++++++++++++++++++
handlers/flash_handler.c | 110 ---------------------------------------------
include/flash.h | 1 +
3 files changed, 114 insertions(+), 110 deletions(-)

diff --git a/corelib/mtd-interface.c b/corelib/mtd-interface.c
index af50ec4..64c96ac 100644
--- a/corelib/mtd-interface.c
+++ b/corelib/mtd-interface.c
+ break;
+ }
+
struct flash_description *flash = get_flash_info();
- /* The sector contains data and it must be erased */
- if (unlock) {
- if (mtd_unlock(mtd, fd, eb) != 0) {
- TRACE("%s: MTD unlock failure", mtd_device);
- continue;
- }
- }
-
- if (mtd_erase(flash->libmtd, mtd, fd, eb) != 0) {
- ERROR("%s: MTD Erase failure", mtd_device);
- ret = -EIO;
- goto erase_out;
- }
- }
-
-erase_out:
- free(buf);
-
- close(fd);
-
- return ret;
-}
-
#ifdef CONFIG_CFIHAMMING1
static unsigned char calc_bitwise_parity(unsigned char val, unsigned char mask)
{
diff --git a/include/flash.h b/include/flash.h
index 9a76e8d..ee3942e 100644
--- a/include/flash.h
+++ b/include/flash.h
@@ -66,6 +66,7 @@ int scan_mtd_devices (void);
Reply all
Reply to author
Forward
0 new messages