[Skiboot] [RFC PATCH] libflash: Introduce a `safemode`

Cyril Bur cyrilbur at au1.ibm.com
Fri Jan 6 18:04:46 AEDT 2017


Some chips don't handle even the most conservative timings used by the
flash setup functions.

Testing has revealed that these conditions are usually very unusual. For
example, safemode is required when a chip has power cut during an erase
command. For this reason it may be best to have a safemode rather than
lower the conservative timings as this may unnecessarily decrease base
performance. Furthermore it can be difficult to detect these 'bad states'
at controller init as all may appear ok, as such the safemode function can
be used by higher levels of the stack that detect unusual errors.

Signed-off-by: Cyril Bur <cyrilbur at au1.ibm.com>
---
I'm in the process of testing this patch. I'm ensuring there are no
regressions but also need to get my hands on some of the 'bad' flash
chips. These are usually quite common as colleagues enjoy erasing
chips with watchdogs still active, it seems now that I can 'easily'
fix these chips people remember to disable watchdogs - maybe we don't
need this upstream then ;).


 hw/ast-bmc/ast-sf-ctrl.c | 18 ++++++++++++++++++
 libflash/libflash-priv.h | 15 ++++++++++++++-
 libflash/libflash.c      | 21 ++++++++++++++++-----
 3 files changed, 48 insertions(+), 6 deletions(-)

diff --git a/hw/ast-bmc/ast-sf-ctrl.c b/hw/ast-bmc/ast-sf-ctrl.c
index bc877d7c..7603007b 100644
--- a/hw/ast-bmc/ast-sf-ctrl.c
+++ b/hw/ast-bmc/ast-sf-ctrl.c
@@ -38,6 +38,9 @@ struct ast_sf_ctrl {
 	/* Address and previous value of the ctrl register */
 	uint32_t		ctl_reg;
 
+	/* Control register safemode value */
+	uint32_t		ctl_safemode_val;
+
 	/* Control register value for normal commands */
 	uint32_t		ctl_val;
 
@@ -678,6 +681,16 @@ static int ast_sf_setup_micron(struct ast_sf_ctrl *ct, struct flash_info *info)
 	return 0;
 }
 
+static int ast_sf_safemode(struct spi_flash_ctrl *ctrl)
+{
+	ct->fread_timing_val = 0;
+	ct->ctl_val = ct->ctl_safemode_val;
+	ct->ctl_read_val = ct->ctl_safemode_val;
+	ast_asb_writel(ct->ctl_safemode_val, ct->ctl_reg);
+	ast_ahb_writel(ct->fread_timing_val, ct->fread_timing_reg);
+	return 0;
+}
+
 static int ast_sf_setup(struct spi_flash_ctrl *ctrl, uint32_t *tsize)
 {
 	struct ast_sf_ctrl *ct = container_of(ctrl, struct ast_sf_ctrl, ops);
@@ -874,6 +887,7 @@ int ast_sf_open(uint8_t type, struct spi_flash_ctrl **ctrl)
 	if (type == AST_SF_TYPE_MEM) {
 		ct->ops.cmd_wr = NULL;
 		ct->ops.cmd_rd = NULL;
+		ct->ops.safemode = NULL;
 		ct->ops.read = ast_sf_read;
 		ct->ops.set_4b = ast_mem_set4b;
 		ct->ops.write = ast_mem_write;
@@ -887,6 +901,7 @@ int ast_sf_open(uint8_t type, struct spi_flash_ctrl **ctrl)
 		ct->ops.set_4b = ast_sf_set_4b;
 		ct->ops.read = ast_sf_read;
 		ct->ops.setup = ast_sf_setup;
+		ct->ops.safemode = ast_sf_safemode;
 	}
 
 	ast_get_ahb_freq();
@@ -899,6 +914,9 @@ int ast_sf_open(uint8_t type, struct spi_flash_ctrl **ctrl)
 			goto fail;
 	}
 
+	/* These are the dumbest safest settings. */
+	ct->ctl_savemode_val = ct->ctl_val;
+
 	*ctrl = &ct->ops;
 
 	return 0;
diff --git a/libflash/libflash-priv.h b/libflash/libflash-priv.h
index e59d829a..a6306724 100644
--- a/libflash/libflash-priv.h
+++ b/libflash/libflash-priv.h
@@ -98,6 +98,19 @@ struct spi_flash_ctrl {
 	int (*setup)(struct spi_flash_ctrl *ctrl, uint32_t *tsize);
 
 	/*
+	 * - safemode(ctrl)
+	 *
+	 * Inform the controller that the higher level believes something
+	 * is wrong and that it should use the most conservative settings it
+	 * can.
+	 *
+	 * This can help in the case where a chip is in a state where the
+	 * high levels cannot enable the WREN bit. This can be caused by
+	 * power being interrupted during an erase.
+	 */
+	int (*safemode)(struct spi_flash_ctrl *ctrl);
+
+	/*
 	 * - set_4b(ctrl, enable)
 	 *
 	 *  enable    : Switch to 4bytes (true) or 3bytes (false) address mode
@@ -227,7 +240,7 @@ struct spi_flash_ctrl {
 	void *priv;
 };
 
-extern int fl_wren(struct spi_flash_ctrl *ct);
+extern int fl_wren(struct flash_chip *c);
 extern int fl_read_stat(struct spi_flash_ctrl *ct, uint8_t *stat);
 extern int fl_sync_wait_idle(struct spi_flash_ctrl *ct);
 
diff --git a/libflash/libflash.c b/libflash/libflash.c
index 9bdc7d6f..4dbdee26 100644
--- a/libflash/libflash.c
+++ b/libflash/libflash.c
@@ -58,6 +58,7 @@ struct flash_chip {
 	bool			mode_4b;	/* Flash currently in 4b mode */
 	struct flash_req	*cur_req;	/* Current request */
 	void			*smart_buf;	/* Buffer for smart writes */
+	bool 			safemode;	/* Have called it? */
 	struct blocklevel_device bl;
 };
 
@@ -102,10 +103,11 @@ int fl_sync_wait_idle(struct spi_flash_ctrl *ct)
 }
 
 /* Exported for internal use */
-int fl_wren(struct spi_flash_ctrl *ct)
+int fl_wren(struct flash_chip *c)
 {
 	int i, rc;
 	uint8_t stat;
+	struct spi_flash_ctrl *ct = c->ctrl;
 
 	/* Some flashes need it to be hammered */
 	for (i = 0; i < 1000; i++) {
@@ -123,6 +125,15 @@ int fl_wren(struct spi_flash_ctrl *ct)
 		if (stat & STAT_WEN)
 			return 0;
 	}
+
+	if (!c->safemode && ct->safemode) {
+		FL_DBG("LIBFLASH: Couldn't set WREN. Enabling safemode\n");
+		rc = ct->safemode(ct);
+		if (!rc)
+			c->safemode = true;
+		return fl_wren(c);
+	}
+
 	return FLASH_ERR_WREN_TIMEOUT;
 }
 
@@ -259,7 +270,7 @@ static int flash_erase(struct blocklevel_device *bl, uint64_t dst, uint64_t size
 		fl_get_best_erase(c, dst, size, &chunk, &cmd);
 
 		/* Poke write enable */
-		rc = fl_wren(ct);
+		rc = fl_wren(c);
 		if (rc)
 			return rc;
 
@@ -294,7 +305,7 @@ int flash_erase_chip(struct flash_chip *c)
 	if (ct->erase)
 		return ct->erase(ct, 0, 0xffffffff);
 
-	rc = fl_wren(ct);
+	rc = fl_wren(c);
 	if (rc) return rc;
 
 	if (c->info.flags & FL_ERASE_CHIP)
@@ -317,7 +328,7 @@ static int fl_wpage(struct flash_chip *c, uint32_t dst, const void *src,
 	if (size < 1 || size > 0x100)
 		return FLASH_ERR_BAD_PAGE_SIZE;
 
-	rc = fl_wren(ct);
+	rc = fl_wren(c);
 	if (rc) return rc;
 
 	rc = ct->cmd_wr(ct, CMD_PP, true, dst, src, size);
@@ -693,7 +704,7 @@ static int flash_set_4b(struct flash_chip *c, bool enable)
 		return 0;
 
 	/* Some flash chips want this */
-	rc = fl_wren(ct);
+	rc = fl_wren(c);
 	if (rc) {
 		FL_ERR("LIBFLASH: Error %d enabling write for set_4b\n", rc);
 		/* Ignore the error & move on (could be wrprotect chip) */
-- 
2.11.0



More information about the Skiboot mailing list