[PATCH 4.17 015/220] mtd: spi-nor: intel-spi: Fix atomic sequence handling

From: Greg Kroah-Hartman
Date: Sun Jul 01 2018 - 13:28:22 EST


4.17-stable review patch. If anyone has any objections, please let me know.

------------------

From: Mika Westerberg <mika.westerberg@xxxxxxxxxxxxxxx>

commit c7d6a82d90e193b1e4daba957e3908f26306d491 upstream.

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@xxxxxxxxxxxxxxx>
Cc: stable@xxxxxxxxxxxxxxx
Reviewed-by: Marek Vasut <marek.vasut@xxxxxxxxx>
Signed-off-by: Boris Brezillon <boris.brezillon@xxxxxxxxxxx>
Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>

---
drivers/mtd/spi-nor/intel-spi.c | 76 +++++++++++++++++++++++++++++++++++-----
1 file changed, 67 insertions(+), 9 deletions(-)

--- 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 int
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 int
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 sp

/*
* 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
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 sp
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_no
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);