[PATCH] spi: spi-qpic-snand: write the feature value before executing SET_FEATURE

[PATCH] spi: spi-qpic-snand: write the feature value before executing SET_FEATURE

[Stanislaw Pal]

qcom_spi_send_cmdaddr() programs NAND_FLASH_CMD/NAND_EXEC_CMD and submits
the descriptors, which makes the controller execute the command
immediately. For SPINAND_SET_FEATURE the value to be written is only
placed into NAND_FLASH_FEATURES afterwards, by qcom_spi_io_op(), in a
second submission - so the chip is programmed with whatever that register
happened to hold from a previous operation, and the intended value is only
applied by the *next* SET_FEATURE.

Measured on a TP-Link Archer AX55 v1 (IPQ5018, ESMT F50L1G41LB): writing
0x40 to the configuration register (0xb0) leaves the chip at 0x00, and the
subsequent write of 0x00 leaves it at 0x40 - every write lands one
operation late.

This stayed unnoticed until v6.18 added SPI-NAND OTP support together
with OTP entries for ESMT chips. spinand_otp_rw() enables OTP mode,
reads, and disables it again, and mtd_otp_nvmem_add() does this during
MTD registration. With the off-by-one, the "disable" write actually
applies the previously requested value, so CFG_OTP_ENABLE ends up set:
the chip stays in OTP mode, every subsequent array read returns the OTP
area instead of the array (UBI reports an empty device) and all writes
fail with -EIO because the OTP area is write protected. On this board
that makes the whole flash unusable and the device unbootable.

Write the feature value into NAND_FLASH_FEATURES as part of the same
transaction, before NAND_EXEC_CMD. While at it, copy only the bytes the
operation actually carries - the previous code dereferenced a 4-byte
pointer on a one-byte buffer (spinand->scratchbuf).

With this patch the flash contents read back bit-identical to a
known-good dump of the same board taken under the vendor firmware
(md5-verified across partitions), and writes work.

Fixes: 7304d1909080 ("spi: spi-qpic: add driver for QCOM SPI NAND flash Interface")
Cc: stable@vger.kernel.org
Signed-off-by: Stanislaw Pal <kuncy7@gmail.com>
---
--- a/drivers/spi/spi-qpic-snand.c
+++ b/drivers/spi/spi-qpic-snand.c
@@ -1358,6 +1358,22 @@
 	snandc->regs->addr0 = cpu_to_le32(op->addr.val);
 	snandc->regs->addr1 = cpu_to_le32(0);
 
+	/*
+	 * The feature value has to reach NAND_FLASH_FEATURES before the
+	 * command is executed, otherwise the controller programs the chip
+	 * with whatever the register happened to hold from a previous
+	 * operation.
+	 */
+	if (opcode == SPINAND_SET_FEATURE) {
+		u32 ftr = 0;
+
+		memcpy(&ftr, op->data.buf.out,
+		       min_t(size_t, op->data.nbytes, sizeof(ftr)));
+		snandc->regs->flash_feature = cpu_to_le32(ftr);
+		qcom_write_reg_dma(snandc, &snandc->regs->flash_feature,
+				   NAND_FLASH_FEATURES, 1, NAND_BAM_NEXT_SGL);
+	}
+
 	qcom_write_reg_dma(snandc, &snandc->regs->cmd, NAND_FLASH_CMD, 3, NAND_BAM_NEXT_SGL);
 	qcom_write_reg_dma(snandc, &snandc->regs->exec, NAND_EXEC_CMD, 1, NAND_BAM_NEXT_SGL);
 
@@ -1395,10 +1411,8 @@
 		copy_ftr = true;
 		break;
 	case SPINAND_SET_FEATURE:
-		snandc->regs->flash_feature = cpu_to_le32(*(u32 *)op->data.buf.out);
-		qcom_write_reg_dma(snandc, &snandc->regs->flash_feature,
-				   NAND_FLASH_FEATURES, 1, NAND_BAM_NEXT_SGL);
-		break;
+		/* fully handled by qcom_spi_send_cmdaddr() */
+		return 0;
 	case SPINAND_PROGRAM_EXECUTE:
 	case SPINAND_WRITE_EN:
 	case SPINAND_RESET:

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH] spi: spi-qpic-snand: write the feature value before executing SET_FEATURE

[Md Sadre Alam]

On 7/27/2026 10:02 PM, Stanislaw Pal wrote:
> qcom_spi_send_cmdaddr() programs NAND_FLASH_CMD/NAND_EXEC_CMD and submits
> the descriptors, which makes the controller execute the command
> immediately. For SPINAND_SET_FEATURE the value to be written is only
> placed into NAND_FLASH_FEATURES afterwards, by qcom_spi_io_op(), in a
> second submission - so the chip is programmed with whatever that register
> happened to hold from a previous operation, and the intended value is only
> applied by the *next* SET_FEATURE.
> 
> Measured on a TP-Link Archer AX55 v1 (IPQ5018, ESMT F50L1G41LB): writing
> 0x40 to the configuration register (0xb0) leaves the chip at 0x00, and the
> subsequent write of 0x00 leaves it at 0x40 - every write lands one
> operation late.
> 
> This stayed unnoticed until v6.18 added SPI-NAND OTP support together
> with OTP entries for ESMT chips. spinand_otp_rw() enables OTP mode,
> reads, and disables it again, and mtd_otp_nvmem_add() does this during
> MTD registration. With the off-by-one, the "disable" write actually
> applies the previously requested value, so CFG_OTP_ENABLE ends up set:
> the chip stays in OTP mode, every subsequent array read returns the OTP
> area instead of the array (UBI reports an empty device) and all writes
> fail with -EIO because the OTP area is write protected. On this board
> that makes the whole flash unusable and the device unbootable.
> 
> Write the feature value into NAND_FLASH_FEATURES as part of the same
> transaction, before NAND_EXEC_CMD. While at it, copy only the bytes the
> operation actually carries - the previous code dereferenced a 4-byte
> pointer on a one-byte buffer (spinand->scratchbuf).
> 
> With this patch the flash contents read back bit-identical to a
> known-good dump of the same board taken under the vendor firmware
> (md5-verified across partitions), and writes work.
> 
> Fixes: 7304d1909080 ("spi: spi-qpic: add driver for QCOM SPI NAND flash Interface")
> Cc: stable@vger.kernel.org
> Signed-off-by: Stanislaw Pal <kuncy7@gmail.com>
> ---

Reviewed-by: Md Sadre Alam <md.alam@oss.qualcomm.com>


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH] spi: spi-qpic-snand: write the feature value before executing SET_FEATURE

[Mark Brown]

On Mon, 27 Jul 2026 18:32:16 +0200, Stanislaw Pal wrote:
> spi: spi-qpic-snand: write the feature value before executing SET_FEATURE

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-7.2

Thanks!

[1/1] spi: spi-qpic-snand: write the feature value before executing SET_FEATURE
      https://git.kernel.org/broonie/spi/c/8fd62901d6bf

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/