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

[Pratyush Yadav <pratyush@kernel.org>]

On Fri, Mar 13 2026, Hendrik Donner wrote:

> Hello,
>
> On 3/13/26 12:46, Pratyush Yadav wrote:
>> On Fri, Mar 06 2026, Hendrik Donner wrote:
>> 
>>> 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) {
>> 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.

[...]

-- 
Regards,
Pratyush Yadav