linux-mtd.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: Brian Norris <computersforpeace@gmail.com>
To: <linux-mtd@lists.infradead.org>
Cc: linux-kernel@vger.kernel.org,
	"Brian Norris" <computersforpeace@gmail.com>,
	"Marek Vasut" <marex@denx.de>, "Rafał Miłecki" <zajec5@gmail.com>,
	"Ezequiel Garcia" <ezequiel@vanguardiasur.com.ar>,
	"Julius Werner" <jwerner@chromium.org>
Subject: [PATCH] mtd: spi-nor: perform read-back check after SR lock/unlock
Date: Fri,  1 Jul 2016 15:23:16 -0700	[thread overview]
Message-ID: <1467411796-110267-1-git-send-email-computersforpeace@gmail.com> (raw)

When programming the Status Register for performing write protect, it's
important to know the result of the operation (i.e., success or
failure). Particularly, it's possible to fail when the hardware write
protect pin (WP#) is asserted, potentially disallowing writes to the
status register. Previously, even if we weren't able to update the
status register, ioctl(MEMLOCK) and ioctl(MEMUNLOCK) would return
success.

Let's add a read-back check, to make sure that the status register was
updated appropriately. If the relevant bits weren't updated, then return
-EIO.

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
 drivers/mtd/spi-nor/spi-nor.c | 52 ++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 46 insertions(+), 6 deletions(-)

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index 14cf6ac8c0a5..9551e41699c8 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -519,12 +519,18 @@ static int stm_lock(struct spi_nor *nor, loff_t ofs, uint64_t len)
 	struct mtd_info *mtd = &nor->mtd;
 	int status_old, status_new;
 	u8 mask = SR_BP2 | SR_BP1 | SR_BP0;
+	u8 wr_mask = mask;
 	u8 shift = ffs(mask) - 1, pow, val;
 	loff_t lock_len;
-	bool can_be_top = true, can_be_bottom = nor->flags & SNOR_F_HAS_SR_TB;
+	bool can_be_top = true, can_be_bottom = false;
 	bool use_top;
 	int ret;
 
+	if (nor->flags & SNOR_F_HAS_SR_TB) {
+		wr_mask |= SR_TB;
+		can_be_bottom = true;
+	}
+
 	status_old = read_sr(nor);
 	if (status_old < 0)
 		return status_old;
@@ -571,7 +577,7 @@ static int stm_lock(struct spi_nor *nor, loff_t ofs, uint64_t len)
 	if (!(val & mask))
 		return -EINVAL;
 
-	status_new = (status_old & ~mask & ~SR_TB) | val;
+	status_new = (status_old & ~wr_mask) | val;
 
 	/* Disallow further writes if WP pin is asserted */
 	status_new |= SR_SRWD;
@@ -591,7 +597,21 @@ static int stm_lock(struct spi_nor *nor, loff_t ofs, uint64_t len)
 	ret = write_sr(nor, status_new);
 	if (ret)
 		return ret;
-	return spi_nor_wait_till_ready(nor);
+	ret = spi_nor_wait_till_ready(nor);
+	if (ret)
+		return ret;
+
+	ret = read_sr(nor);
+	if (ret < 0)
+		return ret;
+
+	if ((ret ^ status_new) & (wr_mask | SR_SRWD)) {
+		dev_dbg(nor->dev, "read-back failure: %#x, %#x\n",
+			status_new, ret);
+		return -EIO;
+	}
+
+	return 0;
 }
 
 /*
@@ -604,12 +624,18 @@ static int stm_unlock(struct spi_nor *nor, loff_t ofs, uint64_t len)
 	struct mtd_info *mtd = &nor->mtd;
 	int status_old, status_new;
 	u8 mask = SR_BP2 | SR_BP1 | SR_BP0;
+	u8 wr_mask = mask;
 	u8 shift = ffs(mask) - 1, pow, val;
 	loff_t lock_len;
-	bool can_be_top = true, can_be_bottom = nor->flags & SNOR_F_HAS_SR_TB;
+	bool can_be_top = true, can_be_bottom = false;
 	bool use_top;
 	int ret;
 
+	if (nor->flags & SNOR_F_HAS_SR_TB) {
+		wr_mask |= SR_TB;
+		can_be_bottom = true;
+	}
+
 	status_old = read_sr(nor);
 	if (status_old < 0)
 		return status_old;
@@ -658,7 +684,7 @@ static int stm_unlock(struct spi_nor *nor, loff_t ofs, uint64_t len)
 			return -EINVAL;
 	}
 
-	status_new = (status_old & ~mask & ~SR_TB) | val;
+	status_new = (status_old & ~wr_mask) | val;
 
 	/* Don't protect status register if we're fully unlocked */
 	if (lock_len == 0)
@@ -679,7 +705,21 @@ static int stm_unlock(struct spi_nor *nor, loff_t ofs, uint64_t len)
 	ret = write_sr(nor, status_new);
 	if (ret)
 		return ret;
-	return spi_nor_wait_till_ready(nor);
+	ret = spi_nor_wait_till_ready(nor);
+	if (ret)
+		return ret;
+
+	ret = read_sr(nor);
+	if (ret < 0)
+		return ret;
+
+	if ((ret ^ status_new) & (wr_mask | SR_SRWD)) {
+		dev_dbg(nor->dev, "read-back failure: %#x, %#x\n",
+			status_new, ret);
+		return -EIO;
+	}
+
+	return 0;
 }
 
 /*
-- 
2.8.0.rc3.226.g39d4020

                 reply	other threads:[~2016-07-01 22:24 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

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=1467411796-110267-1-git-send-email-computersforpeace@gmail.com \
    --to=computersforpeace@gmail.com \
    --cc=ezequiel@vanguardiasur.com.ar \
    --cc=jwerner@chromium.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mtd@lists.infradead.org \
    --cc=marex@denx.de \
    --cc=zajec5@gmail.com \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).