mtd: spi-nor: intel-spi: Fix atomic sequence handling

On many older systems using SW sequencer the PREOP_OPTYPE register
contains two preopcodes as following:

  PREOP_OPTYPE=0xf2785006

The last two bytes are the opcodes decoded to:

  0x50 - Write enable for volatile status register
  0x06 - Write enable

The former is used to modify volatile bits in the status register. For
non-volatile bits the latter is needed. Preopcodes are used in SW
sequencer to send one command "atomically" without anything else
interfering the transfer. The sequence that gets executed is:

  - Send preopcode (write enable) from PREOP_OPTYPE register
  - Send the actual SPI command
  - Poll busy bit in the status register (0x05, RDSR)

Commit 8c473dd61bb5 ("spi-nor: intel-spi: Don't assume OPMENU0/1 to be
programmed by BIOS") enabled atomic sequence handling but because both
preopcodes are programmed, the following happens:

  if (preop >> 8)
  	val |= SSFSTS_CTL_SPOP;

Since on these systems preop >> 8 == 0x50 we end up picking volatile
write enable instead. Because of this the actual write command is pretty
much NOP unless there is a WREN latched in the chip already.

Furthermore we should not really just assume that WREN was issued in
previous call to intel_spi_write_reg() because that might not be the
case.

This updates driver to first check that the opcode is actually available
in PREOP_OPTYPE register and if not return error back to the spi-nor
core (if the controller is not locked we program it now). In addition we
save the opcode to ispi->atomic_preopcode field which is checked in next
call to intel_spi_sw_cycle() to actually enable atomic sequence using
the requested preopcode.

Fixes: 8c473dd61bb5 ("spi-nor: intel-spi: Don't assume OPMENU0/1 to be programmed by BIOS")
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Cc: stable@vger.kernel.org
Reviewed-by: Marek Vasut <marek.vasut@gmail.com>
Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>

diff --git a/drivers/mtd/spi-nor/intel-spi.c b/drivers/mtd/spi-nor/intel-spi.c
index 6999515231791c1010d83aa11692f19c5ca22e70..8e98f4ab87c199a180797d806ab6cf6daaa44a49 100644 (file)
--- a/drivers/mtd/spi-nor/intel-spi.c
+++ b/drivers/mtd/spi-nor/intel-spi.c
@@ -136,6 +136,7 @@
  * @swseq_reg: Use SW sequencer in register reads/writes
  * @swseq_erase: Use SW sequencer in erase operation
  * @erase_64k: 64k erase supported
+ * @atomic_preopcode: Holds preopcode when atomic sequence is requested
  * @opcodes: Opcodes which are supported. This are programmed by BIOS
  *           before it locks down the controller.
  */
@@ -153,6 +154,7 @@ struct intel_spi {
        bool swseq_reg;
        bool swseq_erase;
        bool erase_64k;
+       u8 atomic_preopcode;
        u8 opcodes[8];
 };
 
@@ -474,7 +476,7 @@ static int intel_spi_sw_cycle(struct intel_spi *ispi, u8 opcode, int len,
                              int optype)
 {
        u32 val = 0, status;
-       u16 preop;
+       u8 atomic_preopcode;
        int ret;
 
        ret = intel_spi_opcode_index(ispi, opcode, optype);
@@ -484,17 +486,42 @@ static int intel_spi_sw_cycle(struct intel_spi *ispi, u8 opcode, int len,
        if (len > INTEL_SPI_FIFO_SZ)
                return -EINVAL;
 
+       /*
+        * Always clear it after each SW sequencer operation regardless
+        * of whether it is successful or not.
+        */
+       atomic_preopcode = ispi->atomic_preopcode;
+       ispi->atomic_preopcode = 0;
+
        /* Only mark 'Data Cycle' bit when there is data to be transferred */
        if (len > 0)
                val = ((len - 1) << SSFSTS_CTL_DBC_SHIFT) | SSFSTS_CTL_DS;
        val |= ret << SSFSTS_CTL_COP_SHIFT;
        val |= SSFSTS_CTL_FCERR | SSFSTS_CTL_FDONE;
        val |= SSFSTS_CTL_SCGO;
-       preop = readw(ispi->sregs + PREOP_OPTYPE);
-       if (preop) {
-               val |= SSFSTS_CTL_ACS;
-               if (preop >> 8)
-                       val |= SSFSTS_CTL_SPOP;
+       if (atomic_preopcode) {
+               u16 preop;
+
+               switch (optype) {
+               case OPTYPE_WRITE_NO_ADDR:
+               case OPTYPE_WRITE_WITH_ADDR:
+                       /* Pick matching preopcode for the atomic sequence */
+                       preop = readw(ispi->sregs + PREOP_OPTYPE);
+                       if ((preop & 0xff) == atomic_preopcode)
+                               ; /* Do nothing */
+                       else if ((preop >> 8) == atomic_preopcode)
+                               val |= SSFSTS_CTL_SPOP;
+                       else
+                               return -EINVAL;
+
+                       /* Enable atomic sequence */
+                       val |= SSFSTS_CTL_ACS;
+                       break;
+
+               default:
+                       return -EINVAL;
+               }
+
        }
        writel(val, ispi->sregs + SSFSTS_CTL);
 
@@ -538,13 +565,31 @@ static int intel_spi_write_reg(struct spi_nor *nor, u8 opcode, u8 *buf, int len)
 
        /*
         * This is handled with atomic operation and preop code in Intel
-        * controller so skip it here now. If the controller is not locked,
-        * program the opcode to the PREOP register for later use.
+        * controller so we only verify that it is available. If the
+        * controller is not locked, program the opcode to the PREOP
+        * register for later use.
+        *
+        * When hardware sequencer is used there is no need to program
+        * any opcodes (it handles them automatically as part of a command).
         */
        if (opcode == SPINOR_OP_WREN) {
-               if (!ispi->locked)
+               u16 preop;
+
+               if (!ispi->swseq_reg)
+                       return 0;
+
+               preop = readw(ispi->sregs + PREOP_OPTYPE);
+               if ((preop & 0xff) != opcode && (preop >> 8) != opcode) {
+                       if (ispi->locked)
+                               return -EINVAL;
                        writel(opcode, ispi->sregs + PREOP_OPTYPE);
+               }
 
+               /*
+                * This enables atomic sequence on next SW sycle. Will
+                * be cleared after next operation.
+                */
+               ispi->atomic_preopcode = opcode;
                return 0;
        }
 
@@ -569,6 +614,13 @@ static ssize_t intel_spi_read(struct spi_nor *nor, loff_t from, size_t len,
        u32 val, status;
        ssize_t ret;
 
+       /*
+        * Atomic sequence is not expected with HW sequencer reads. Make
+        * sure it is cleared regardless.
+        */
+       if (WARN_ON_ONCE(ispi->atomic_preopcode))
+               ispi->atomic_preopcode = 0;
+
        switch (nor->read_opcode) {
        case SPINOR_OP_READ:
        case SPINOR_OP_READ_FAST:
@@ -627,6 +679,9 @@ static ssize_t intel_spi_write(struct spi_nor *nor, loff_t to, size_t len,
        u32 val, status;
        ssize_t ret;
 
+       /* Not needed with HW sequencer write, make sure it is cleared */
+       ispi->atomic_preopcode = 0;
+
        while (len > 0) {
                block_size = min_t(size_t, len, INTEL_SPI_FIFO_SZ);
 
@@ -707,6 +762,9 @@ static int intel_spi_erase(struct spi_nor *nor, loff_t offs)
                return 0;
        }
 
+       /* Not needed with HW sequencer erase, make sure it is cleared */
+       ispi->atomic_preopcode = 0;
+
        while (len > 0) {
                writel(offs, ispi->base + FADDR);