From: <Tudor.Ambarus@microchip.com>
To: <vigneshr@ti.com>, <boris.brezillon@collabora.com>,
<marek.vasut@gmail.com>, <linux-mtd@lists.infradead.org>,
<geert+renesas@glider.be>, <jonas@norrbonn.se>
Cc: <dwmw2@infradead.org>, <computersforpeace@gmail.com>,
<miquel.raynal@bootlin.com>, <richard@nod.at>, <joel@jms.id.au>,
<andrew@aj.id.au>, <matthias.bgg@gmail.com>, <vz@mleia.com>,
<linux-arm-kernel@lists.infradead.org>,
<linux-aspeed@lists.ozlabs.org>, <linux-kernel@vger.kernel.org>,
<linux-mediatek@lists.infradead.org>,
<Tudor.Ambarus@microchip.com>
Subject: [PATCH v2 18/22] mtd: spi-nor: Fix clearing of QE bit on lock()/unlock()
Date: Tue, 24 Sep 2019 07:46:50 +0000 [thread overview]
Message-ID: <20190924074533.6618-19-tudor.ambarus@microchip.com> (raw)
In-Reply-To: <20190924074533.6618-1-tudor.ambarus@microchip.com>
From: Tudor Ambarus <tudor.ambarus@microchip.com>
Make sure that when doing a lock() or an unlock() operation we don't clear
the QE bit from Status Register 2.
JESD216 revB or later offers information about the *default* Status
Register commands to use (see BFPT DWORDS[15], bits 22:20). In this
standard, Status Register 1 refers to the first data byte transferred on a
Read Status (05h) or Write Status (01h) command. Status register 2 refers
to the byte read using instruction 35h. Status register 2 is the second
byte transferred in a Write Status (01h) command.
Industry naming and definitions of these Status Registers may differ.
The definitions are described in JESD216B, BFPT DWORDS[15], bits 22:20.
There are cases in which writing only one byte to the Status Register 1
has the side-effect of clearing Status Register 2 and implicitly the Quad
Enable bit. This side-effect is hit just by the
BFPT_DWORD15_QER_SR2_BIT1_BUGGY and BFPT_DWORD15_QER_SR2_BIT1 cases.
Suggested-by: Boris Brezillon <boris.brezillon@collabora.com>
Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
---
drivers/mtd/spi-nor/spi-nor.c | 165 +++++++++++++++++++++++++++++++++++++-----
include/linux/mtd/spi-nor.h | 2 +
2 files changed, 147 insertions(+), 20 deletions(-)
diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index 48bcb2ee1be5..8ada2003f1c9 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -836,6 +836,127 @@ static int spi_nor_read_sr2(struct spi_nor *nor, u8 *sr2)
return ret;
}
+/**
+ * spi_nor_write_sr1_and_check() - Write one byte to the Status Register 1 and
+ * ensure that the byte written match the received value.
+ * @nor: pointer to a 'struct spi_nor'.
+ * @sr1: byte value to be written to the Status Register.
+ *
+ * Return: 0 on success, -errno otherwise.
+ */
+static int spi_nor_write_sr1_and_check(struct spi_nor *nor, u8 sr1)
+{
+ int ret;
+
+ nor->bouncebuf[0] = sr1;
+
+ ret = spi_nor_write_sr(nor, &nor->bouncebuf[0], 1);
+ if (ret)
+ return ret;
+
+ ret = spi_nor_read_sr(nor, &nor->bouncebuf[0]);
+ if (ret)
+ return ret;
+
+ if (nor->bouncebuf[0] != sr1) {
+ dev_err(nor->dev, "SR1: read back test failed\n");
+ return -EIO;
+ }
+
+ return 0;
+}
+
+/**
+ * spi_nor_write_16bit_sr_and_check() - Write the Status Register 1 and the
+ * Status Register 2 in one shot. Ensure that the byte written in the Status
+ * Register 1 match the received value, and that the 16-bit Write did not
+ * affect what was already in the Status Register 2.
+ * @nor: pointer to a 'struct spi_nor'.
+ * @sr1: byte value to be written to the Status Register 1.
+ *
+ * Return: 0 on success, -errno otherwise.
+ */
+static int spi_nor_write_16bit_sr_and_check(struct spi_nor *nor, u8 sr1)
+{
+ int ret;
+ u8 *sr_cr = nor->bouncebuf;
+ u8 cr_written;
+
+ /* Make sure we don't overwrite the contents of Status Register 2. */
+ if (!(nor->flags & SNOR_F_NO_READ_CR)) {
+ ret = spi_nor_read_cr(nor, &sr_cr[1]);
+ if (ret)
+ return ret;
+ } else if (nor->flash.quad_enable) {
+ /*
+ * If the Status Register 2 Read command (35h) is not
+ * supported, we should at least be sure we don't
+ * change the value of the SR2 Quad Enable bit.
+ *
+ * We can safely assume that when the Quad Enable method is
+ * set, the value of the QE bit is one, as a consequence of the
+ * nor->flash.quad_enable() call.
+ *
+ * We can safely assume that the Quad Enable bit is present in
+ * the Status Register 2 at BIT(1). According to the JESD216
+ * revB standard, BFPT DWORDS[15], bits 22:20, the 16-bit
+ * Write Status (01h) command is available just for the cases
+ * in which the QE bit is described in SR2 at BIT(1).
+ */
+ sr_cr[1] = CR_QUAD_EN_SPAN;
+ } else {
+ sr_cr[1] = 0;
+ }
+
+ sr_cr[0] = sr1;
+
+ ret = spi_nor_write_sr(nor, sr_cr, 2);
+ if (ret)
+ return ret;
+
+ ret = spi_nor_read_sr(nor, &sr_cr[0]);
+ if (ret)
+ return ret;
+
+ if (sr_cr[0] != sr1) {
+ dev_err(nor->dev, "SR1: read back test failed\n");
+ return -EIO;
+ }
+
+ if (nor->flags & SNOR_F_NO_READ_CR)
+ return 0;
+
+ cr_written = sr_cr[1];
+
+ ret = spi_nor_read_cr(nor, &sr_cr[1]);
+ if (ret)
+ return ret;
+
+ if (cr_written != sr_cr[1]) {
+ dev_err(nor->dev, "CR: read back test failed\n");
+ return -EIO;
+ }
+
+ return 0;
+}
+
+/**
+ * spi_nor_write_sr_and_check() - Write the Status Register 1 and ensure that
+ * the byte written match the received value without affecting other bits in the
+ * Status Register 1 and 2.
+ * @nor: pointer to a 'struct spi_nor'.
+ * @sr1: byte value to be written to the Status Register 1.
+ *
+ * Return: 0 on success, -errno otherwise.
+ */
+static int spi_nor_write_sr_and_check(struct spi_nor *nor, u8 sr1)
+{
+ if (nor->flags & SNOR_F_HAS_16BIT_SR)
+ return spi_nor_write_16bit_sr_and_check(nor, sr1);
+
+ return spi_nor_write_sr1_and_check(nor, sr1);
+}
+
static struct spi_nor *mtd_to_spi_nor(struct mtd_info *mtd)
{
return mtd->priv;
@@ -1492,24 +1613,6 @@ static int spi_nor_erase(struct mtd_info *mtd, struct erase_info *instr)
return ret;
}
-/* Write status register and ensure bits in mask match written values */
-static int write_sr_and_check(struct spi_nor *nor, u8 status_new)
-{
- int ret;
-
- nor->bouncebuf[0] = status_new;
-
- ret = spi_nor_write_sr(nor, &nor->bouncebuf[0], 1);
- if (ret)
- return ret;
-
- ret = spi_nor_read_sr(nor, &nor->bouncebuf[0]);
- if (ret)
- return ret;
-
- return (nor->bouncebuf[0] != status_new) ? -EIO : 0;
-}
-
static void stm_get_locked_range(struct spi_nor *nor, u8 sr, loff_t *ofs,
uint64_t *len)
{
@@ -1673,7 +1776,7 @@ static int stm_lock(struct spi_nor *nor, loff_t ofs, uint64_t len)
if ((status_new & mask) < (status_old & mask))
return -EINVAL;
- return write_sr_and_check(nor, status_new);
+ return spi_nor_write_sr_and_check(nor, status_new);
}
/*
@@ -1758,7 +1861,7 @@ static int stm_unlock(struct spi_nor *nor, loff_t ofs, uint64_t len)
if ((status_new & mask) > (status_old & mask))
return -EINVAL;
- return write_sr_and_check(nor, status_new);
+ return spi_nor_write_sr_and_check(nor, status_new);
}
/*
@@ -3536,19 +3639,39 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor,
break;
case BFPT_DWORD15_QER_SR2_BIT1_BUGGY:
+ /*
+ * Writing only one byte to the Status Register has the
+ * side-effect of clearing Status Register 2.
+ */
+ /* fall through */
case BFPT_DWORD15_QER_SR2_BIT1_NO_RD:
+ nor->flags |= SNOR_F_HAS_16BIT_SR;
+ /*
+ * Read Configuration Register (35h) instruction is not
+ * supported.
+ */
+ nor->flags |= SNOR_F_NO_READ_CR;
flash->quad_enable = spansion_no_read_cr_quad_enable;
break;
case BFPT_DWORD15_QER_SR1_BIT6:
+ nor->flags &= ~SNOR_F_HAS_16BIT_SR;
flash->quad_enable = macronix_quad_enable;
break;
case BFPT_DWORD15_QER_SR2_BIT7:
+ nor->flags &= ~SNOR_F_HAS_16BIT_SR;
flash->quad_enable = sr2_bit7_quad_enable;
break;
case BFPT_DWORD15_QER_SR2_BIT1:
+ /*
+ * JESD216 rev B or later does not specify if writing only one
+ * byte to the Status Register clears or not the Status
+ * Register 2, so let's be cautious and keep the default
+ * assumption of a 16-bit Write Status (01h) command.
+ */
+ nor->flags |= SNOR_F_HAS_16BIT_SR;
flash->quad_enable = spansion_read_cr_quad_enable;
break;
@@ -4515,6 +4638,8 @@ static void spi_nor_info_init_flash_params(struct spi_nor *nor)
flash->quad_enable = spansion_read_cr_quad_enable;
flash->set_4byte = spansion_set_4byte;
flash->setup = spi_nor_default_setup;
+ /* Default to 16-bit Write Status (01h) Command */
+ nor->flags |= SNOR_F_HAS_16BIT_SR;
/* Set SPI NOR sizes. */
flash->size = (u64)info->sector_size * info->n_sectors;
diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
index 12961b157743..fc3a8f5209f0 100644
--- a/include/linux/mtd/spi-nor.h
+++ b/include/linux/mtd/spi-nor.h
@@ -243,6 +243,8 @@ enum spi_nor_option_flags {
SNOR_F_4B_OPCODES = BIT(6),
SNOR_F_HAS_4BAIT = BIT(7),
SNOR_F_HAS_LOCK = BIT(8),
+ SNOR_F_HAS_16BIT_SR = BIT(9),
+ SNOR_F_NO_READ_CR = BIT(10),
};
/**
--
2.9.5
next prev parent reply other threads:[~2019-09-24 7:47 UTC|newest]
Thread overview: 52+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-09-24 7:45 [PATCH v2 00/22] mtd: spi-nor: Quad Enable and (un)lock methods Tudor.Ambarus
2019-09-24 7:45 ` [PATCH v2 01/22] mtd: spi-nor: hisi-sfc: Drop nor->erase NULL assignment Tudor.Ambarus
2019-10-10 6:49 ` Boris Brezillon
2019-10-23 21:24 ` Tudor.Ambarus
2019-09-24 7:45 ` [PATCH v2 02/22] mtd: spi-nor: Introduce 'struct spi_nor_controller_ops' Tudor.Ambarus
2019-10-10 6:51 ` Boris Brezillon
2019-10-23 21:24 ` Tudor.Ambarus
2019-09-24 7:45 ` [PATCH v2 03/22] mtd: spi-nor: cadence-quadspi: Fix cqspi_command_read() definition Tudor.Ambarus
2019-10-10 6:54 ` Boris Brezillon
2019-10-23 21:25 ` Tudor.Ambarus
2019-09-24 7:46 ` [PATCH v2 04/22] mtd: spi-nor: Rename nor->params to nor->flash Tudor.Ambarus
2019-10-10 7:05 ` Boris Brezillon
2019-10-23 21:34 ` Tudor.Ambarus
2019-09-24 7:46 ` [PATCH v2 05/22] mtd: spi-nor: Rework read_sr() Tudor.Ambarus
2019-10-10 7:13 ` Boris Brezillon
2019-09-24 7:46 ` [PATCH v2 06/22] mtd: spi-nor: Rework read_fsr() Tudor.Ambarus
2019-10-10 7:14 ` Boris Brezillon
2019-09-24 7:46 ` [PATCH v2 07/22] mtd: spi-nor: Rework read_cr() Tudor.Ambarus
2019-10-10 7:16 ` Boris Brezillon
2019-10-10 7:16 ` Boris Brezillon
2019-09-24 7:46 ` [PATCH v2 08/22] mtd: spi-nor: Rework write_enable/disable() Tudor.Ambarus
2019-10-10 7:21 ` Boris Brezillon
2019-10-23 23:39 ` Tudor.Ambarus
2019-10-24 6:04 ` Boris Brezillon
2019-10-24 11:01 ` Tudor.Ambarus
2019-09-24 7:46 ` [PATCH v2 09/22] mtd: spi-nor: Fix retlen handling in sst_write() Tudor.Ambarus
2019-10-10 7:33 ` Boris Brezillon
2019-10-25 7:34 ` Tudor.Ambarus
2019-09-24 7:46 ` [PATCH v2 10/22] mtd: spi-nor: Rework write_sr() Tudor.Ambarus
2019-10-04 9:39 ` John Garry
2019-10-04 10:03 ` Tudor.Ambarus
2019-10-04 10:26 ` John Garry
2019-10-04 10:47 ` [PATCH] mtd: spi-nor: Fix direction of the write_sr() transfer Tudor.Ambarus
2019-10-04 10:48 ` Tudor.Ambarus
2019-10-04 11:31 ` John Garry
2019-10-04 15:17 ` John Garry
2019-10-04 15:50 ` Tudor.Ambarus
2019-10-04 16:06 ` Vignesh Raghavendra
2019-10-04 16:09 ` Miquel Raynal
2019-09-24 7:46 ` [PATCH v2 11/22] mtd: spi-nor: Rework spi_nor_read/write_sr2() Tudor.Ambarus
2019-09-24 7:46 ` [PATCH v2 12/22] mtd: spi-nor: Report error in spi_nor_xread_sr() Tudor.Ambarus
2019-09-24 7:46 ` [PATCH v2 13/22] mtd: spi-nor: Void return type for spi_nor_clear_sr/fsr() Tudor.Ambarus
2019-09-24 7:46 ` [PATCH v2 14/22] mtd: spi-nor: Drop duplicated new line Tudor.Ambarus
2019-09-24 7:46 ` [PATCH v2 15/22] mtd: spi-nor: Drop spansion_quad_enable() Tudor.Ambarus
2019-09-24 7:46 ` [PATCH v2 16/22] mtd: spi-nor: Fix errno on quad_enable methods Tudor.Ambarus
2019-09-24 7:46 ` [PATCH v2 17/22] mtd: spi-nor: Check all the bits written, not just the BP ones Tudor.Ambarus
2019-09-24 7:46 ` Tudor.Ambarus [this message]
2019-09-24 7:46 ` [PATCH v2 19/22] mtd: spi-nor: Rework macronix_quad_enable() Tudor.Ambarus
2019-09-24 7:46 ` [PATCH v2 20/22] mtd: spi-nor: Rework spansion(_no)_read_cr_quad_enable() Tudor.Ambarus
2019-09-24 7:47 ` [PATCH v2 21/22] mtd: spi-nor: Update sr2_bit7_quad_enable() Tudor.Ambarus
2019-09-24 7:47 ` [PATCH v2 22/22] mtd: spi-nor: Rework the disabling of block write protection Tudor.Ambarus
2019-09-25 10:11 ` [PATCH v2 00/22] mtd: spi-nor: Quad Enable and (un)lock methods Tudor.Ambarus
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=20190924074533.6618-19-tudor.ambarus@microchip.com \
--to=tudor.ambarus@microchip.com \
--cc=andrew@aj.id.au \
--cc=boris.brezillon@collabora.com \
--cc=computersforpeace@gmail.com \
--cc=dwmw2@infradead.org \
--cc=geert+renesas@glider.be \
--cc=joel@jms.id.au \
--cc=jonas@norrbonn.se \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-aspeed@lists.ozlabs.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mediatek@lists.infradead.org \
--cc=linux-mtd@lists.infradead.org \
--cc=marek.vasut@gmail.com \
--cc=matthias.bgg@gmail.com \
--cc=miquel.raynal@bootlin.com \
--cc=richard@nod.at \
--cc=vigneshr@ti.com \
--cc=vz@mleia.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).