mtd: spi-nor: Fix clearing of QE bit on lock()/unlock()

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>
Reviewed-by: Vignesh Raghavendra <vigneshr@ti.com>

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index 9960e09..d696334 100644 (file)
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -959,12 +959,19 @@ static int spi_nor_write_sr(struct spi_nor *nor, const u8 *sr, size_t len)
        return spi_nor_wait_till_ready(nor);
 }
 
-/* Write status register and ensure bits in mask match written values */
-static int spi_nor_write_sr_and_check(struct spi_nor *nor, u8 status_new)
+/**
+ * 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] = status_new;
+       nor->bouncebuf[0] = sr1;
 
        ret = spi_nor_write_sr(nor, nor->bouncebuf, 1);
        if (ret)
@@ -974,8 +981,73 @@ static int spi_nor_write_sr_and_check(struct spi_nor *nor, u8 status_new)
        if (ret)
                return ret;
 
-       if (nor->bouncebuf[0] != status_new) {
-               dev_dbg(nor->dev, "SR: read back test failed\n");
+       if (nor->bouncebuf[0] != sr1) {
+               dev_dbg(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->params.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->params.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;
+
+       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_dbg(nor->dev, "CR: read back test failed\n");
                return -EIO;
        }
 
@@ -983,6 +1055,23 @@ static int spi_nor_write_sr_and_check(struct spi_nor *nor, u8 status_new)
 }
 
 /**
+ * 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.
+ *
+ * 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);
+}
+
+/**
  * spi_nor_write_sr2() - Write the Status Register 2 using the
  * SPINOR_OP_WRSR2 (3eh) command.
  * @nor:       pointer to 'struct spi_nor'.
@@ -3634,19 +3723,38 @@ 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.
+                */
        case BFPT_DWORD15_QER_SR2_BIT1_NO_RD:
+               /*
+                * Read Configuration Register (35h) instruction is not
+                * supported.
+                */
+               nor->flags |= SNOR_F_HAS_16BIT_SR | SNOR_F_NO_READ_CR;
                params->quad_enable = spansion_no_read_cr_quad_enable;
                break;
 
        case BFPT_DWORD15_QER_SR1_BIT6:
+               nor->flags &= ~SNOR_F_HAS_16BIT_SR;
                params->quad_enable = macronix_quad_enable;
                break;
 
        case BFPT_DWORD15_QER_SR2_BIT7:
+               nor->flags &= ~SNOR_F_HAS_16BIT_SR;
                params->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;
+
                params->quad_enable = spansion_read_cr_quad_enable;
                break;
 
@@ -4613,6 +4721,8 @@ static void spi_nor_info_init_params(struct spi_nor *nor)
        params->quad_enable = spansion_read_cr_quad_enable;
        params->set_4byte = spansion_set_4byte;
        params->setup = spi_nor_default_setup;
+       /* Default to 16-bit Write Status (01h) Command */
+       nor->flags |= SNOR_F_HAS_16BIT_SR;
 
        /* Set SPI NOR sizes. */
        params->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 d1d736d..d6ec55c 100644 (file)
--- a/include/linux/mtd/spi-nor.h
+++ b/include/linux/mtd/spi-nor.h
@@ -243,6 +243,9 @@ 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),
+
 };
 
 /**