All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kamal Dasu <kdasu.kdev@gmail.com>
To: linux-mtd@lists.infradead.org
Cc: f.fainelli@gmail.com, bcm-kernel-feedback-list@broadcom.com,
	dwmw2@infradead.org, computersforpeace@gmail.com,
	boris.brezillon@free-electrons.com, marek.vasut@gmail.com,
	richard@nod.at, cyrille.pitchen@atmel.com,
	Kamal Dasu <kdasu.kdev@gmail.com>
Subject: [PATCH V4, 2/2] mtd: nand: brcmnand: Check flash #WP pin status before nand erase/program
Date: Tue, 28 Feb 2017 11:55:26 -0500	[thread overview]
Message-ID: <1488300926-3517-2-git-send-email-kdasu.kdev@gmail.com> (raw)
In-Reply-To: <1488300926-3517-1-git-send-email-kdasu.kdev@gmail.com>

brcmnand controller v6.x and v7.x lets driver to enable disable #WP pin
via NAND_WP bit in CS_SELECT register. Driver implementation assumes that
setting/resetting the bit would assert/de-assert  #WP pin instantaneously
from the flash part's perspective, and was proceeding to erase/program
without verifying flash status byte for protection bit. In rigorous
testing this was causing rare data corruptions with erase and/or
subsequent programming. To fix this check NAND status in addition to
controller #WP pin status to make sure both sides are ready to accept
new commands.

Signed-off-by: Kamal Dasu <kdasu.kdev@gmail.com>
---
 drivers/mtd/nand/brcmnand/brcmnand.c | 70 +++++++++++++++++++++++++++++++++---
 1 file changed, 65 insertions(+), 5 deletions(-)

diff --git a/drivers/mtd/nand/brcmnand/brcmnand.c b/drivers/mtd/nand/brcmnand/brcmnand.c
index c7c4efe..af32685 100644
--- a/drivers/mtd/nand/brcmnand/brcmnand.c
+++ b/drivers/mtd/nand/brcmnand/brcmnand.c
@@ -101,6 +101,9 @@ struct brcm_nand_dma_desc {
 #define BRCMNAND_MIN_BLOCKSIZE	(8 * 1024)
 #define BRCMNAND_MIN_DEVSIZE	(4ULL * 1024 * 1024)
 
+#define NAND_CTRL_RDY			(INTFC_CTLR_READY | INTFC_FLASH_READY)
+#define NAND_POLL_STATUS_TIMEOUT_MS	100
+
 /* Controller feature flags */
 enum {
 	BRCMNAND_HAS_1K_SECTORS			= BIT(0),
@@ -765,12 +768,69 @@ enum {
 	CS_SELECT_AUTO_DEVICE_ID_CFG		= BIT(30),
 };
 
-static int brcmnand_set_wp(struct brcmnand_host *host, int en)
+static int bcmnand_ctrl_poll_status(struct brcmnand_controller *ctrl,
+				    u32 mask, u32 expected_val,
+				    unsigned long timeout_ms)
+{
+	unsigned long limit;
+	u32 val;
+
+	if (!timeout_ms)
+		timeout_ms = NAND_POLL_STATUS_TIMEOUT_MS;
+
+	limit = jiffies + msecs_to_jiffies(timeout_ms);
+	do {
+		val = brcmnand_read_reg(ctrl, BRCMNAND_INTFC_STATUS);
+		if ((val & mask) == expected_val)
+			return 0;
+
+		cpu_relax();
+	} while (time_after(limit, jiffies));
+
+	dev_warn(ctrl->dev, "timeout on status poll (expected %x got %x)\n",
+		 expected_val, val & mask);
+
+	return -ETIMEDOUT;
+}
+
+static inline void brcmnand_set_wp_reg(struct brcmnand_controller *ctrl, int en)
 {
-	struct brcmnand_controller *ctrl = host->ctrl;
 	u32 val = en ? CS_SELECT_NAND_WP : 0;
 
 	brcmnand_rmw_reg(ctrl, BRCMNAND_CS_SELECT, CS_SELECT_NAND_WP, 0, val);
+}
+
+static int brcmnand_set_wp(struct brcmnand_host *host, int en)
+{
+	struct brcmnand_controller *ctrl = host->ctrl;
+	struct mtd_info *mtd = nand_to_mtd(&host->chip);
+	struct nand_chip *chip = mtd_to_nand(mtd);
+	int ret;
+
+	/*
+	 * make sure ctrl/flash ready before and after
+	 * changing state of WP PIN
+	 */
+	ret = bcmnand_ctrl_poll_status(ctrl, NAND_CTRL_RDY | NAND_STATUS_READY,
+				       NAND_CTRL_RDY | NAND_STATUS_READY, 0);
+	if (ret)
+		return ret;
+
+	brcmnand_set_wp_reg(ctrl, en);
+	chip->cmdfunc(mtd, NAND_CMD_STATUS, -1, -1);
+	/* NAND_STATUS_WP 0x00 = protected, 0x80 = not protected */
+	ret = bcmnand_ctrl_poll_status(ctrl,
+				       NAND_CTRL_RDY | NAND_STATUS_READY |
+				       NAND_STATUS_WP,
+				       NAND_CTRL_RDY | NAND_STATUS_READY |
+				       (en ? 0 : NAND_STATUS_WP), 0);
+
+	if (ret) {
+		dev_err_ratelimited(&host->pdev->dev,
+				    "nand #WP expected %s\n",
+				    en ? "On" : "Off");
+		return ret;
+	}
 
 	return 0;
 }
@@ -1160,15 +1220,15 @@ static irqreturn_t brcmnand_dma_irq(int irq, void *data)
 static void brcmnand_send_cmd(struct brcmnand_host *host, int cmd)
 {
 	struct brcmnand_controller *ctrl = host->ctrl;
-	u32 intfc;
+	int ret;
 
 	dev_dbg(ctrl->dev, "send native cmd %d addr_lo 0x%x\n", cmd,
 		brcmnand_read_reg(ctrl, BRCMNAND_CMD_ADDRESS));
 	BUG_ON(ctrl->cmd_pending != 0);
 	ctrl->cmd_pending = cmd;
 
-	intfc = brcmnand_read_reg(ctrl, BRCMNAND_INTFC_STATUS);
-	WARN_ON(!(intfc & INTFC_CTLR_READY));
+	ret = bcmnand_ctrl_poll_status(ctrl, NAND_CTRL_RDY, NAND_CTRL_RDY, 0);
+	WARN_ON(ret);
 
 	mb(); /* flush previous writes */
 	brcmnand_write_reg(ctrl, BRCMNAND_CMD_START,
-- 
1.9.1

  reply	other threads:[~2017-02-28 16:57 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-02-28 16:55 [PATCH V4, 1/2] mtd: nand: brcmnand: Change brcmnand_set_wp() prototype Kamal Dasu
2017-02-28 16:55 ` Kamal Dasu [this message]
2017-02-28 18:25 ` Marek Vasut
2017-02-28 19:28   ` Kamal Dasu
2017-02-28 22:05 ` Boris Brezillon
2017-02-28 22:20   ` Kamal Dasu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1488300926-3517-2-git-send-email-kdasu.kdev@gmail.com \
    --to=kdasu.kdev@gmail.com \
    --cc=bcm-kernel-feedback-list@broadcom.com \
    --cc=boris.brezillon@free-electrons.com \
    --cc=computersforpeace@gmail.com \
    --cc=cyrille.pitchen@atmel.com \
    --cc=dwmw2@infradead.org \
    --cc=f.fainelli@gmail.com \
    --cc=linux-mtd@lists.infradead.org \
    --cc=marek.vasut@gmail.com \
    --cc=richard@nod.at \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.