[PATCH v2 1/2] mtd: spi-nor: sst: Fix write enable before AAI sequence

[Hendrik Donner]

Hello,

On 2/23/26 10:29, Michael Walle wrote:
> Hi,
> 
> On Mon Feb 23, 2026 at 10:17 AM CET, Sanjaikumar V S wrote:
>>> Raises concern about writes ending at odd offsets potentially
>>> having the same issue
>>
>> The odd end address case (trailing byte) is already handled in the
>> existing code at lines 243-255:
>>
>> /* Write out trailing byte if it exists. */
>> if (actual != len) {
>>      ret = spi_nor_write_enable(nor);
>>      ...
>>      ret = sst_nor_write_data(nor, to, 1, buf + actual);
>> }
> 
> Ah, I must be blind. I stopped reading at the write_disable.
> 
>> So write_enable is already called before writing the trailing
>> byte. My patch only addresses the odd start case where BP clears
>> WEL before the AAI sequence begins.
>>
>>> Suggests simplifying the conditional logic by removing the length
>>> check
>>
>> The condition `if (actual < len - 1)` avoids an unnecessary
>> write_enable when len == 1 (single byte write at odd address, no
>> AAI follows). But if you prefer unconditional write_enable for
>> simplicity, I can change it in v3.
> 
> I know, but I actually don't like repeating the condition in the for
> loop. So I'd prefer to have a local "needs_write_enable" boolean
> which will be set to true. But then, I wouldn't care too much if
> there is a write enable followed by a write disable for a rare case.
> 
>>> Notes the patch lacks runtime testing
>>
>> I don't have the hardware setup to test odd-address writes at the
>> moment. The fix is based on code analysis. I have tested patch 2/2
>> (dirmap fallback) on hardware.
> 
> I'm hesitant - because like I said, if there is really a bug - it
> would have never worked correctly, since day 1. But yeah, I've also
> read the datasheet and it clearly states that the byte write will
> clear the write enable latch.
> 

i can confirm both patches fix real issues, i have similiar fixes
on a kernel tree i always wanted to clean up and upstream. Diffs
based on 6.6.127:

diff --git a/drivers/mtd/spi-nor/sst.c b/drivers/mtd/spi-nor/sst.c
index 197d2c1101ed5..eaa50561ede2c 100644
--- a/drivers/mtd/spi-nor/sst.c
+++ b/drivers/mtd/spi-nor/sst.c
@@ -155,6 +155,13 @@ static int sst_nor_write(struct mtd_info *mtd, loff_t to, size_t len,
                 if (ret)
                         goto out;

+               ret = spi_nor_write_enable(nor);
+               if (ret)
+                       goto out;
+               ret = spi_nor_wait_till_ready(nor);
+               if (ret)
+                       goto out;
+
                 to++;
                 actual++;
         }


diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index 1b0c6770c14e4..646bfb2e91a65 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -276,7 +276,7 @@ static ssize_t spi_nor_spimem_write_data(struct spi_nor *nor, loff_t to,
         if (spi_nor_spimem_bounce(nor, &op))
                 memcpy(nor->bouncebuf, buf, op.data.nbytes);

-       if (nor->dirmap.wdesc) {
+       if (nor->dirmap.wdesc && nor->program_opcode != SPINOR_OP_AAI_WP) {
                 nbytes = spi_mem_dirmap_write(nor->dirmap.wdesc, op.addr.val,
                                               op.data.nbytes, op.data.buf.out);
         } else {


I think patch 2 of this series is the better approach though.

Regards,
Hendrik

>> Please let me know if you'd like me to send a v3 with the
>> simplified unconditional write_enable.
> 
> Please see above.
> 
> -michael
> 
> 
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/