[RFC PATCH] spi: ich: Drop while loop in hardware sequencing erase case

Wolfgang Wallner wolfgang.wallner at br-automation.com
Tue Jan 14 14:05:48 CET 2020


When ich_spi_exec_op_hwseq() is called to erase a 4k block
(opcode = SPINOR_OP_BE_4K), it expects to find a length value in
op->data.nbytes, but that value is always 0. As a result, the while loop
is never executed and no erase is carried out.

Fix this by dropping the loop code entirely, only keeping the relevant
parts of the loop body.

Signed-off-by: Wolfgang Wallner <wolfgang.wallner at br-automation.com>

---
I tried using an SPI NOR flash on an Apollo Lake based board, but calling
'sf erase' fails to erase the flash memory, it looks like it does nothing.
Debugging the issue I find that the function ich_spi_exec_op_hwseq() in
drivers/spi/ich.c tries to loop over the provided number of bytes, but that
value is always 0.

The spi operation that is handled by ich_spi_exec_op_hwseq() is initially
created in the function spi_nor_erase_sector(), and handed down via the
following call stack:

spi_nor_erase_sector()  in drivers/mtd/spi/spi-nor-core.c
spi_mem_exec_op()       in drivers/spi/spi-mem.c
    spi_mem_exec_op() calls ops->mem_ops->exec_op() which points to
    ich_spi_exec_op()
ich_spi_exec_op()       in drivers/spi/ich.c
ich_spi_exec_op_hwseq() in drivers/spi/ich.c

The number of bytes (op.data.nbytes) is always set to 0 in
spi_nor_erase_sector(), thus the loop in ich_spi_exec_op_hwseq can never be
executed.

I tried to fix this by dropping the loop code entirely, and with this
change 'sf erase' works for me as expected.

The code in spi_nor_erase_sector() that sets data.nbytes always to 0 was
introduced in commit f909ddb3e177 ("mtd: spi: Replace ad-hoc default
implementation with spi_mem_op"), but that commit is older then the code
in ich_spi_exec_op_hwseq() which was introduced in commit 1facebd18fd7
("spi: ich: Support hardware sequencing").

I would like to ask for feedback on the provided patch.

 drivers/spi/ich.c | 12 ++----------
 1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/drivers/spi/ich.c b/drivers/spi/ich.c
index 133b25b72e..a9d7715a55 100644
--- a/drivers/spi/ich.c
+++ b/drivers/spi/ich.c
@@ -562,16 +562,8 @@ static int ich_spi_exec_op_hwseq(struct spi_slave *slave,
 		return 0;  /* ignore */
 	case SPINOR_OP_BE_4K:
 		cycle = HSFSTS_CYCLE_4K_ERASE;
-		while (len) {
-			uint xfer_len = 0x1000;
-
-			ret = exec_sync_hwseq_xfer(regs, cycle, offset, 0);
-			if (ret)
-				return ret;
-			offset += xfer_len;
-			len -= xfer_len;
-		}
-		return 0;
+		ret = exec_sync_hwseq_xfer(regs, cycle, offset, 0);
+		return ret;
 	default:
 		debug("Unknown cycle %x\n", op->cmd.opcode);
 		return -EINVAL;
-- 
2.24.1




More information about the U-Boot mailing list