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

[Michael Walle]

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.

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

Please see above.

-michael

Attachment: signature.asc
Description: PGP signature