[U-Boot] [PATCH 0/6] mtd fixes for u-boot-2016.07-rc1

[U-Boot] [PATCH 0/6] mtd fixes for u-boot-2016.07-rc1

[Ladislav Michl]

This patch serie contains various bugfixes related to mtd found during
converting igep00x0 to use UBI. All patches was sent previously as a part
of RFCs, but as noone commented so far, I'm resending them separately.
Former versions already sent was marked as "superseded" in patchwork.

Best regards,
	ladis

[U-Boot] [PATCH 1/6] armv7: add reset timeout to identify_nand_chip

[Ladislav Michl]

identify_nand_chip hangs forever in loop when NAND is not present.
As IGEPv2 comes either with NAND or OneNAND flash, add reset timeout
to let function fail gracefully allowing caller to know NAND is
not present. On NAND equipped board, reset succeeds on first read,
so 1000 loops seems to be a safe timeout.

Signed-off-by: Ladislav Michl <ladis@linux-mips.org>

diff --git a/arch/arm/cpu/armv7/omap3/spl_id_nand.c b/arch/arm/cpu/armv7/omap3/spl_id_nand.c
index db6de09..26d3aa4 100644
--- a/arch/arm/cpu/armv7/omap3/spl_id_nand.c
+++ b/arch/arm/cpu/armv7/omap3/spl_id_nand.c
@@ -20,29 +20,16 @@
 
 static struct gpmc *gpmc_config = (struct gpmc *)GPMC_BASE;
 
-/* nand_command: Send a flash command to the flash chip */
-static void nand_command(u8 command)
-{
-	writeb(command, &gpmc_config->cs[0].nand_cmd);
-
-	if (command == NAND_CMD_RESET) {
-		unsigned char ret_val;
-		writeb(NAND_CMD_STATUS, &gpmc_config->cs[0].nand_cmd);
-		do {
-			/* Wait until ready */
-			ret_val = readl(&gpmc_config->cs[0].nand_dat);
-		} while ((ret_val & NAND_STATUS_READY) != NAND_STATUS_READY);
-	}
-}
-
 /*
  * Many boards will want to know the results of the NAND_CMD_READID command
  * in order to decide what to do about DDR initialization.  This function
  * allows us to do that very early and to pass those results back to the
  * board so it can make whatever decisions need to be made.
  */
-void identify_nand_chip(int *mfr, int *id)
+int identify_nand_chip(int *mfr, int *id)
 {
+	int loops = 1000;
+
 	/* Make sure that we have setup GPMC for NAND correctly. */
 	writel(M_NAND_GPMC_CONFIG1, &gpmc_config->cs[0].config1);
 	writel(M_NAND_GPMC_CONFIG2, &gpmc_config->cs[0].config2);
@@ -62,8 +49,15 @@ void identify_nand_chip(int *mfr, int *id)
 	sdelay(2000);
 
 	/* Issue a RESET and then READID */
-	nand_command(NAND_CMD_RESET);
-	nand_command(NAND_CMD_READID);
+	writeb(NAND_CMD_RESET, &gpmc_config->cs[0].nand_cmd);
+	writeb(NAND_CMD_STATUS, &gpmc_config->cs[0].nand_cmd);
+	while ((readl(&gpmc_config->cs[0].nand_dat) & NAND_STATUS_READY)
+	                                           != NAND_STATUS_READY) {
+		sdelay(100);
+		if (--loops == 0)
+			return 1;
+	}
+	writeb(NAND_CMD_READID, &gpmc_config->cs[0].nand_cmd);
 
 	/* Set the address to read to 0x0 */
 	writeb(0x0, &gpmc_config->cs[0].nand_adr);
@@ -71,4 +65,6 @@ void identify_nand_chip(int *mfr, int *id)
 	/* Read off the manufacturer and device id. */
 	*mfr = readb(&gpmc_config->cs[0].nand_dat);
 	*id = readb(&gpmc_config->cs[0].nand_dat);
+
+	return 0;
 }
diff --git a/arch/arm/include/asm/arch-omap3/sys_proto.h b/arch/arm/include/asm/arch-omap3/sys_proto.h
index 24563c0..1be2b15 100644
--- a/arch/arm/include/asm/arch-omap3/sys_proto.h
+++ b/arch/arm/include/asm/arch-omap3/sys_proto.h
@@ -40,7 +40,7 @@ void sdrc_init(void);
 void do_sdrc_init(u32, u32);
 
 void get_board_mem_timings(struct board_sdrc_timings *timings);
-void identify_nand_chip(int *mfr, int *id);
+int identify_nand_chip(int *mfr, int *id);
 void emif4_init(void);
 void gpmc_init(void);
 void enable_gpmc_cs_config(const u32 *gpmc_config, struct gpmc_cs *cs, u32 base,
-- 
2.1.4

[U-Boot] [PATCH 2/6] mtd: OneNAND: add timeout to wait ready loops

[Ladislav Michl]

Add timeout to onenand_wait ready loop as it hangs here indefinitely
when chip not present. Once there, do the same for onenand_bbt_wait
as well (note: recent Linux driver code does the same)

Signed-off-by: Ladislav Michl <ladis@linux-mips.org>

diff --git a/drivers/mtd/onenand/onenand_base.c b/drivers/mtd/onenand/onenand_base.c
index 03deabc..d194d97 100644
--- a/drivers/mtd/onenand/onenand_base.c
+++ b/drivers/mtd/onenand/onenand_base.c
@@ -20,6 +20,7 @@
  */
 
 #include <common.h>
+#include <watchdog.h>
 #include <linux/compat.h>
 #include <linux/mtd/mtd.h>
 #include "linux/mtd/flashchip.h"
@@ -467,15 +468,18 @@ static int onenand_read_ecc(struct onenand_chip *this)
 static int onenand_wait(struct mtd_info *mtd, int state)
 {
 	struct onenand_chip *this = mtd->priv;
-	unsigned int flags = ONENAND_INT_MASTER;
 	unsigned int interrupt = 0;
 	unsigned int ctrl;
 
-	while (1) {
+	/* Wait at most 20ms ... */
+	u32 timeo = (CONFIG_SYS_HZ * 20) / 1000;
+	u32 time_start = get_timer(0);
+	do {
+		WATCHDOG_RESET();
+		if (get_timer(time_start) > timeo)
+			return -EIO;
 		interrupt = this->read_word(this->base + ONENAND_REG_INTERRUPT);
-		if (interrupt & flags)
-			break;
-	}
+	} while ((interrupt & ONENAND_INT_MASTER) == 0);
 
 	ctrl = this->read_word(this->base + ONENAND_REG_CTRL_STATUS);
 
@@ -1154,15 +1158,18 @@ int onenand_read_oob(struct mtd_info *mtd, loff_t from,
 static int onenand_bbt_wait(struct mtd_info *mtd, int state)
 {
 	struct onenand_chip *this = mtd->priv;
-	unsigned int flags = ONENAND_INT_MASTER;
 	unsigned int interrupt;
 	unsigned int ctrl;
 
-	while (1) {
+	/* Wait at most 20ms ... */
+	u32 timeo = (CONFIG_SYS_HZ * 20) / 1000;
+	u32 time_start = get_timer(0);
+	do {
+		WATCHDOG_RESET();
+		if (get_timer(time_start) > timeo)
+			return ONENAND_BBT_READ_FATAL_ERROR;
 		interrupt = this->read_word(this->base + ONENAND_REG_INTERRUPT);
-		if (interrupt & flags)
-			break;
-	}
+	} while ((interrupt & ONENAND_INT_MASTER) == 0);
 
 	/* To get correct interrupt status in timeout case */
 	interrupt = this->read_word(this->base + ONENAND_REG_INTERRUPT);
@@ -2536,7 +2543,8 @@ static int onenand_chip_probe(struct mtd_info *mtd)
 	this->write_word(ONENAND_CMD_RESET, this->base + ONENAND_BOOTRAM);
 
 	/* Wait reset */
-	this->wait(mtd, FL_RESETING);
+	if (this->wait(mtd, FL_RESETING))
+		return -ENXIO;
 
 	/* Restore system configuration 1 */
 	this->write_word(syscfg, this->base + ONENAND_REG_SYS_CFG1);
-- 
2.1.4

[U-Boot] [PATCH 3/6] mtd: OneNAND: initialize mtd->writebufsize to let UBI work

[Ladislav Michl]

io_init checks this value and fails with "bad write buffer size 0 for
2048 min. I/O unit"

Signed-off-by: Ladislav Michl <ladis@linux-mips.org>

diff --git a/drivers/mtd/onenand/onenand_base.c b/drivers/mtd/onenand/onenand_base.c
index d194d97..0e35dc5 100644
--- a/drivers/mtd/onenand/onenand_base.c
+++ b/drivers/mtd/onenand/onenand_base.c
@@ -2657,6 +2657,7 @@ int onenand_probe(struct mtd_info *mtd)
 	mtd->_sync = onenand_sync;
 	mtd->_block_isbad = onenand_block_isbad;
 	mtd->_block_markbad = onenand_block_markbad;
+	mtd->writebufsize = mtd->writesize;
 
 	return 0;
 }
-- 
2.1.4

[U-Boot] [PATCH 4/6] cmd: mtdparts: fix mtdparts variable presence confusion in mtdparts_init

[Ladislav Michl]

A private buffer is used to read mtdparts variable from non-relocated
environment. A pointer to that buffer is returned unconditionally,
confusing later test for variable presence in the environment.
Fix it by returning NULL when getenv_f fails.

Signed-off-by: Ladislav Michl <ladis@linux-mips.org>
--
diff --git a/cmd/mtdparts.c b/cmd/mtdparts.c
index 44b2c3a..3a88a10 100644
--- a/cmd/mtdparts.c
+++ b/cmd/mtdparts.c
@@ -1720,11 +1720,13 @@ int mtdparts_init(void)
 	 * before the env is relocated, then we need to use our own stack
 	 * buffer.  gd->env_buf will be too small.
 	 */
-	if (gd->flags & GD_FLG_ENV_READY) {
+	if (gd->flags & GD_FLG_ENV_READY)
 		parts = getenv("mtdparts");
-	} else {
-		parts = tmp_parts;
-		getenv_f("mtdparts", tmp_parts, MTDPARTS_MAXLEN);
+	else {
+		if (getenv_f("mtdparts", tmp_parts, MTDPARTS_MAXLEN) != -1)
+			parts = tmp_parts;
+		else
+			parts = NULL;
 	}
 	current_partition = getenv("partition");
 
-- 
2.1.4

[U-Boot] [PATCH 5/6] cmd: mtdparts: fix null pointer dereference in parse_mtdparts

[Ladislav Michl]

In case there is no mtdparts variable in relocated environment,
NULL is assigned to p, which is later fed to strncpy.
Also function parameter mtdparts is completely ignored, so use it
in case mtdparts variable is not found in environment. This
parameter is checked not to be NULL in caller.

Signed-off-by: Ladislav Michl <ladis@linux-mips.org>
--
diff --git a/cmd/mtdparts.c b/cmd/mtdparts.c
index 3a88a10..995cb87 100644
--- a/cmd/mtdparts.c
+++ b/cmd/mtdparts.c
@@ -1524,7 +1524,7 @@ static int spread_partitions(void)
  */
 static int parse_mtdparts(const char *const mtdparts)
 {
-	const char *p = mtdparts;
+	const char *p;
 	struct mtd_device *dev;
 	int err = 1;
 	char tmp_parts[MTDPARTS_MAXLEN];
@@ -1538,20 +1538,25 @@ static int parse_mtdparts(const char *const mtdparts)
 	}
 
 	/* re-read 'mtdparts' variable, mtd_devices_init may be updating env */
-	if (gd->flags & GD_FLG_ENV_READY) {
+	if (gd->flags & GD_FLG_ENV_READY)
 		p = getenv("mtdparts");
-	} else {
-		p = tmp_parts;
-		getenv_f("mtdparts", tmp_parts, MTDPARTS_MAXLEN);
+	else {
+		if (getenv_f("mtdparts", tmp_parts, MTDPARTS_MAXLEN) != -1)
+			p = tmp_parts;
+		else
+			p = NULL;
 	}
 
+	if (!p)
+		p = mtdparts;
+
 	if (strncmp(p, "mtdparts=", 9) != 0) {
 		printf("mtdparts variable doesn't start with 'mtdparts='\n");
 		return err;
 	}
 	p += 9;
 
-	while (p && (*p != '\0')) {
+	while (*p != '\0') {
 		err = 1;
 		if ((device_parse(p, &p, &dev) != 0) || (!dev))
 			break;
@@ -1569,12 +1574,10 @@ static int parse_mtdparts(const char *const mtdparts)
 		list_add_tail(&dev->link, &devices);
 		err = 0;
 	}
-	if (err == 1) {
+	if (err == 1)
 		device_delall(&devices);
-		return 1;
-	}
 
-	return 0;
+	return err;
 }
 
 /**
-- 
2.1.4

[U-Boot] [PATCH 6/6] cmd: mtdparts: consolidate mtdparts reading from env

[Ladislav Michl]

Signed-off-by: Ladislav Michl <ladis@linux-mips.org>
--
diff --git a/cmd/mtdparts.c b/cmd/mtdparts.c
index 995cb87..7860ed9 100644
--- a/cmd/mtdparts.c
+++ b/cmd/mtdparts.c
@@ -1516,6 +1516,23 @@ static int spread_partitions(void)
 #endif /* CONFIG_CMD_MTDPARTS_SPREAD */
 
 /**
+ * The mtdparts variable tends to be long. If we need to access it
+ * before the env is relocated, then we need to use our own stack
+ * buffer.  gd->env_buf will be too small.
+ *
+ * @param buf temporary buffer pointer MTDPARTS_MAXLEN long
+ * @return mtdparts variable string, NULL if not found
+ */
+static const char *getenv_mtdparts(char *buf)
+{
+	if (gd->flags & GD_FLG_ENV_READY)
+		return getenv("mtdparts");
+	if (getenv_f("mtdparts", buf, MTDPARTS_MAXLEN) != -1)
+		return buf;
+	return NULL;
+}
+
+/**
  * Accept character string describing mtd partitions and call device_parse()
  * for each entry. Add created devices to the global devices list.
  *
@@ -1538,15 +1555,7 @@ static int parse_mtdparts(const char *const mtdparts)
 	}
 
 	/* re-read 'mtdparts' variable, mtd_devices_init may be updating env */
-	if (gd->flags & GD_FLG_ENV_READY)
-		p = getenv("mtdparts");
-	else {
-		if (getenv_f("mtdparts", tmp_parts, MTDPARTS_MAXLEN) != -1)
-			p = tmp_parts;
-		else
-			p = NULL;
-	}
-
+	p = getenv_mtdparts(tmp_parts);
 	if (!p)
 		p = mtdparts;
 
@@ -1691,6 +1700,7 @@ static int parse_mtdids(const char *const ids)
 	return 0;
 }
 
+
 /**
  * Parse and initialize global mtdids mapping and create global
  * device/partition list.
@@ -1718,19 +1728,7 @@ int mtdparts_init(void)
 
 	/* get variables */
 	ids = getenv("mtdids");
-	/*
-	 * The mtdparts variable tends to be long. If we need to access it
-	 * before the env is relocated, then we need to use our own stack
-	 * buffer.  gd->env_buf will be too small.
-	 */
-	if (gd->flags & GD_FLG_ENV_READY)
-		parts = getenv("mtdparts");
-	else {
-		if (getenv_f("mtdparts", tmp_parts, MTDPARTS_MAXLEN) != -1)
-			parts = tmp_parts;
-		else
-			parts = NULL;
-	}
+	parts = getenv_mtdparts(tmp_parts);
 	current_partition = getenv("partition");
 
 	/* save it for later parsing, cannot rely on current partition pointer
-- 
2.1.4