Skip to contentSkip to conversation

mtd: spi-nor: Fix SST AAI write mode

PATCH Superseded Sanjaikumar V S 4 participants 13 messages Feb 20 – Mar 13

Subsystems (3)
Stable branch Memory technology devices (MTD) SPI NOR

v2 of 5 Superseded Latest is v5 →

lore
Share & tools
lore

Sanjaikumar V S

author
#0 lore raw

v2 cover mtd: spi-nor: Fix SST AAI write mode

From: Sanjaikumar V S <[email protected]>

This series fixes SST flash AAI (Auto Address Increment) write mode, which was broken due to two issues:

1. When writing starts at an odd address, a single byte is programmed first using the byte program (BP) command. The flash hardware clears the Write Enable Latch (WEL) after this operation, but the driver did not re-enable writes before starting the AAI sequence.

2. When the SPI controller does not support direct mapping (nodirmap=true), the write path falls back to using an operation template created at probe time. This template has the standard page program opcode, not the AAI opcode, causing AAI writes to fail.

Tested on SST25VF016B with i.MX8X running linux-imx 5.15.71. The conditional write enable (only when AAI follows) is based on code analysis and not runtime tested.

v2: Resend with corrected maintainer email addresses

Sanjaikumar V S (2):
mtd: spi-nor: sst: Fix write enable before AAI sequence
mtd: spi-nor: core: Fix AAI mode when dirmap is not available

 drivers/mtd/spi-nor/core.c | 2 +-
 drivers/mtd/spi-nor/sst.c  | 7 +++++++
 2 files changed, 8 insertions(+), 1 deletion(-)
Signature · 1 line
2.43.0
author
#1 lore raw

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

From: Sanjaikumar V S <[email protected]>

When writing to SST flash starting at an odd address, a single byte is first programmed using the byte program (BP) command. After this operation completes, the flash hardware automatically clears the Write Enable Latch (WEL) bit.

If an AAI (Auto Address Increment) word program sequence follows, it requires WEL to be set. Without re-enabling writes, the AAI sequence fails.

Add spi_nor_write_enable() after the odd-address byte program, but only when an AAI sequence will follow (len > 2 bytes remaining).

Cc [email protected]
Signed-off-by Sanjaikumar V S <[email protected]>
---
 drivers/mtd/spi-nor/sst.c | 7 +++++++
 1 file changed, 7 insertions(+)
drivers/mtd/spi-nor/sst.c +7 -0
index 175211fe6a5e..fe714e6d0914 100644
@@ -210,6 +210,13 @@ static int sst_nor_write(struct mtd_info *mtd, loff_t to, size_t len,
 
 		to++;
 		actual++;
+
+		/* BP clears WEL, re-enable if AAI sequence follows */
+		if (actual < len - 1) {
+			ret = spi_nor_write_enable(nor);
+			if (ret)
+				goto out;
+		}
 	}
 
 	/* Write out most of the data here. */
Signature · 1 line
2.43.0
maintainer
#2 lore raw

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

Hi,

Quoted 11 lines · Sanjaikumar V S

From: Sanjaikumar V S <[email protected]>

When writing to SST flash starting at an odd address, a single byte is first programmed using the byte program (BP) command. After this operation completes, the flash hardware automatically clears the Write Enable Latch (WEL) bit.

If an AAI (Auto Address Increment) word program sequence follows, it requires WEL to be set. Without re-enabling writes, the AAI sequence fails.

Quoted 5 lines

Add spi_nor_write_enable() after the odd-address byte program, but only when an AAI sequence will follow (len > 2 bytes remaining).

Cc [email protected]
Signed-off-by Sanjaikumar V S <[email protected]>

Very strange. If that is correct, it actually never worked correctly.

Fixes b199489d37b2 ("mtd: spi-nor: add the framework for SPI NOR")

You've said, you didn't test this. Shouldn't it be pretty easy to write to an odd offset? Also, from reading the code, it looks like if write ends on an odd offset, it doesn't work either?

Quoted 9 lines
---
 drivers/mtd/spi-nor/sst.c | 7 +++++++
 1 file changed, 7 insertions(+)
drivers/mtd/spi-nor/sst.c +0 -0
index 175211fe6a5e..fe714e6d0914 100644

drivers/mtd/spi-nor/sst.c static int sst_nor_write(struct mtd_info *mtd, loff_t to, size_t len, · line 215

Quoted 6 lines
drivers/mtd/spi-nor/sst.c +3 -0
@@ -210,6 +210,13 @@ static int sst_nor_write(struct mtd_info *mtd, loff_t to, size_t len,
 
 		to++;
 		actual++;
+
+		/* BP clears WEL, re-enable if AAI sequence follows */
+		if (actual < len - 1) {

For brevity, I'd drop the repeated condition and just do the write_enable unconditionally.

-michael

Quoted 7 lines
drivers/mtd/spi-nor/sst.c +4 -0
+			ret = spi_nor_write_enable(nor);
+			if (ret)
+				goto out;
+		}
 	}
 
 	/* Write out most of the data here. */
author
#3 lore raw

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

Hi Michael,

Thank you for the review.

Quoted 1 line

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);
}

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.

Quoted 1 line

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.

Quoted 1 line

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.

Please let me know if you'd like me to send a v3 with the simplified unconditional write_enable.

Thanks,
Sanjaikumar

maintainer
#4 lore raw

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

Hi,

Quoted 6 lines · Sanjaikumar V S
Quoted 2 lines

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:

Quoted 6 lines

/* 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.

Quoted 11 lines

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.

Quoted 2 lines

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.

Quoted 5 lines
Quoted 1 line

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.

Quoted 2 lines

Please let me know if you'd like me to send a v3 with the simplified unconditional write_enable.

Please see above.

-michael

#5 lore raw

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

Hello,

Quoted 37 lines · Michael Walle

Hi,

Quoted 6 lines · Sanjaikumar V S
Quoted 2 lines

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:

Quoted 6 lines

/* 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.

Quoted 11 lines

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.

Quoted 2 lines

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.

Quoted 2 lines
Quoted 1 line

Notes the patch lacks runtime testing

Quoted 9 lines
Quoted 3 lines

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:

drivers/mtd/spi-nor/sst.c +7 -0
index 197d2c1101ed5..eaa50561ede2c 100644
@@ -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++;
        }
drivers/mtd/spi-nor/core.c +1 -1
index 1b0c6770c14e4..646bfb2e91a65 100644
@@ -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

Quoted 11 lines
Quoted 2 lines

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/

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

maintainer
#6 lore raw

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

Quoted 75 lines · Hendrik Donner

Hello,

Quoted 34 lines · Michael Walle

Hi,

Quoted 6 lines · Sanjaikumar V S
Quoted 2 lines

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:

Quoted 6 lines

/* 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.

Quoted 11 lines

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.

Quoted 2 lines

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.

Quoted 2 lines
Quoted 1 line

Notes the patch lacks runtime testing

Quoted 8 lines
Quoted 3 lines

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:

drivers/mtd/spi-nor/sst.c +7 -0
index 197d2c1101ed5..eaa50561ede2c 100644
@@ -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++;
        }
drivers/mtd/spi-nor/core.c +0 -0
index 1b0c6770c14e4..646bfb2e91a65 100644

drivers/mtd/spi-nor/core.c static ssize_t spi_nor_spimem_write_data(struct spi_nor *nor, loff_t to, · line 279

Quoted 5 lines
drivers/mtd/spi-nor/core.c +1 -1
@@ -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) {

Why is this better? This removes the use of dirmap for all flashes other than SST.

Quoted 6 lines
drivers/mtd/spi-nor/core.c +0 -0
                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.

There is a v4 here:
https://lore.kernel.org/linux-mtd/[email protected]/T/#u

Can you please review and test it so we can apply it?

Quoted 12 lines

Regards,
Hendrik

Quoted 7 lines
Quoted 2 lines

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/

Signature · 2 lines
Regards,
Pratyush Yadav
#7 lore raw

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

Hello,

Quoted 80 lines · Pratyush Yadav
Quoted 78 lines · Hendrik Donner

Hello,

Quoted 34 lines · Michael Walle

Hi,

Quoted 6 lines · Sanjaikumar V S
Quoted 2 lines

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:

Quoted 6 lines

/* 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.

Quoted 11 lines

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.

Quoted 2 lines

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.

Quoted 2 lines
Quoted 1 line

Notes the patch lacks runtime testing

Quoted 8 lines
Quoted 3 lines

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:

drivers/mtd/spi-nor/sst.c +7 -0
index 197d2c1101ed5..eaa50561ede2c 100644
@@ -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++;
         }
drivers/mtd/spi-nor/core.c +0 -0
index 1b0c6770c14e4..646bfb2e91a65 100644
@@ -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);
Quoted 5 lines

drivers/mtd/spi-nor/core.c static ssize_t spi_nor_spimem_write_data(struct spi_nor *nor, loff_t to, · line 279

Quoted 2 lines
drivers/mtd/spi-nor/core.c +1 -1
-       if (nor->dirmap.wdesc) {
+       if (nor->dirmap.wdesc && nor->program_opcode != SPINOR_OP_AAI_WP) {

Why is this better? This removes the use of dirmap for all flashes other than SST.

i claim the opposite down below? That patch 2 of the posted patch series looks better to me. Sorry if that was unclear.

Regards,
Hendrik

Quoted 26 lines
Quoted 7 lines
drivers/mtd/spi-nor/core.c +0 -0
                 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.

There is a v4 here:
https://lore.kernel.org/linux-mtd/[email protected]/T/#u

Can you please review and test it so we can apply it?

Quoted 12 lines

Regards,
Hendrik

Quoted 7 lines
Quoted 2 lines

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/

maintainer
#8 lore raw

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

Quoted 83 lines · Hendrik Donner

Hello,

Quoted 80 lines · Pratyush Yadav
Quoted 78 lines · Hendrik Donner

Hello,

Quoted 34 lines · Michael Walle

Hi,

Quoted 6 lines · Sanjaikumar V S
Quoted 2 lines

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:

Quoted 6 lines

/* 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.

Quoted 11 lines

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.

Quoted 2 lines

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.

Quoted 2 lines
Quoted 1 line

Notes the patch lacks runtime testing

Quoted 8 lines
Quoted 3 lines

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:

drivers/mtd/spi-nor/sst.c +7 -0
index 197d2c1101ed5..eaa50561ede2c 100644
@@ -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++;
         }
drivers/mtd/spi-nor/core.c +0 -0
index 1b0c6770c14e4..646bfb2e91a65 100644
@@ -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);
Quoted 7 lines
Quoted 4 lines

drivers/mtd/spi-nor/core.c static ssize_t spi_nor_spimem_write_data(struct spi_nor *nor, loff_t to, · line 279

Quoted 2 lines
drivers/mtd/spi-nor/core.c +1 -1
-       if (nor->dirmap.wdesc) {
+       if (nor->dirmap.wdesc && nor->program_opcode != SPINOR_OP_AAI_WP) {

Why is this better? This removes the use of dirmap for all flashes other than SST.

i claim the opposite down below? That patch 2 of the posted patch series looks better to me. Sorry if that was unclear.

Oh, then I misunderstood.

Please review and test the v4, it will help land those fixes.

[...]

Signature · 2 lines
Regards,
Pratyush Yadav
author
#9 lore raw

PATCH v2 2/2 mtd: spi-nor: core: Fix AAI mode when dirmap is not available

In reply to Sanjaikumar V S

From: Sanjaikumar V S <[email protected]>

When the SPI controller does not support direct mapping (nodirmap=true), spi_nor_spimem_write_data() calls spi_mem_dirmap_write() which falls back to spi_mem_no_dirmap_write(). This fallback uses the operation template created at probe time with the standard page program opcode.

For SST flashes using AAI mode, this fails because the template cannot handle the dynamic opcode and address byte changes required by AAI.

Fix by checking nodirmap and using spi_nor_spimem_exec_op() directly, which uses the runtime-built operation with correct AAI configuration.

Cc [email protected]
Signed-off-by Sanjaikumar V S <[email protected]>
---
 drivers/mtd/spi-nor/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/mtd/spi-nor/core.c +1 -1
index d3f8a78efd3b..7caeb508d628 100644
@@ -281,7 +281,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->dirmap.wdesc->nodirmap) {
 		nbytes = spi_mem_dirmap_write(nor->dirmap.wdesc, op.addr.val,
 					      op.data.nbytes, op.data.buf.out);
 	} else {
Signature · 1 line
2.43.0
author
#10 lore raw

v3 cover mtd: spi-nor: Fix SST AAI write mode

In reply to Sanjaikumar V S

From: Sanjaikumar V S <[email protected]>

This patch series addresses two distinct problems affecting SST flash Auto Address Increment write functionality:

1. When writes begin at odd addresses, a single byte is programmed first using byte program command, which clears the Write Enable Latch. The driver fails to re-enable writes before the AAI sequence.

2. When the SPI controller lacks direct mapping support, the fallback path uses a probe-time operation template with standard page program opcodes instead of AAI opcodes.

Changes in v3:
- Patch 1/2: Use local boolean 'needs_write_enable' for clarity as suggested by Michael Walle
- Patch 1/2: Improved comment explaining the fix
- Patch 1/2: Added Fixes tag

Changes in v2:
- Split fixes into separate patches
- Added detailed commit messages

Sanjaikumar V S (2):
mtd: spi-nor: sst: Fix write enable before AAI sequence
mtd: spi-nor: core: Fix AAI mode when dirmap is not available

 drivers/mtd/spi-nor/core.c |  2 +-
 drivers/mtd/spi-nor/sst.c  | 13 +++++++++++++
 2 files changed, 14 insertions(+), 1 deletion(-)

--
2.43.0

author
#11 lore raw

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

From: Sanjaikumar V S <[email protected]>

When writing to SST flash starting at an odd address, a single byte is first programmed using the byte program (BP) command. After this operation completes, the flash hardware automatically clears the Write Enable Latch (WEL) bit.

If an AAI (Auto Address Increment) word program sequence follows, it requires WEL to be set. Without re-enabling writes, the AAI sequence fails.

Add spi_nor_write_enable() after the odd-address byte program when more data needs to be written. Use a local boolean for clarity.

Fixes b199489d37b2 ("mtd: spi-nor: add the framework for SPI NOR")
Cc [email protected]
Signed-off-by Sanjaikumar V S <[email protected]>
---
 drivers/mtd/spi-nor/sst.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)
drivers/mtd/spi-nor/sst.c +13 -0
index 175211fe6a5e..db02c14ba16f 100644
@@ -203,6 +203,8 @@ static int sst_nor_write(struct mtd_info *mtd, loff_t to, size_t len,
 
 	/* Start write from odd address. */
 	if (to % 2) {
+		bool needs_write_enable = (len > 1);
+
 		/* write one byte. */
 		ret = sst_nor_write_data(nor, to, 1, buf);
 		if (ret < 0)

@@ -210,6 +212,17 @@ static int sst_nor_write(struct mtd_info *mtd, loff_t to, size_t len,
 
 		to++;
 		actual++;
+
+		/*
+		 * Byte program clears the write enable latch. If more
+		 * data needs to be written using the AAI sequence,
+		 * re-enable writes.
+		 */
+		if (needs_write_enable) {
+			ret = spi_nor_write_enable(nor);
+			if (ret)
+				goto out;
+		}
 	}
 
 	/* Write out most of the data here. */
Signature · 1 line
2.43.0
author
#12 lore raw

PATCH v3 2/2 mtd: spi-nor: core: Fix AAI mode when dirmap is not available

In reply to Sanjaikumar V S

From: Sanjaikumar V S <[email protected]>

When the SPI controller does not support direct mapping (nodirmap=true), spi_nor_spimem_write_data() calls spi_mem_dirmap_write() which falls back to spi_mem_no_dirmap_write(). This fallback uses the operation template created at probe time with the standard page program opcode.

For SST flashes using AAI mode, this fails because the template cannot handle the dynamic opcode and address byte changes required by AAI.

Fix by checking nodirmap and using spi_nor_spimem_exec_op() directly, which uses the runtime-built operation with correct AAI configuration.

Cc [email protected]
Signed-off-by Sanjaikumar V S <[email protected]>
---
 drivers/mtd/spi-nor/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/mtd/spi-nor/core.c +1 -1
index d3f8a78efd3b..7caeb508d628 100644
@@ -281,7 +281,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->dirmap.wdesc->nodirmap) {
 		nbytes = spi_mem_dirmap_write(nor->dirmap.wdesc, op.addr.val,
 					      op.data.nbytes, op.data.buf.out);
 	} else {
Signature · 1 line
2.43.0