linux-kernel - Re: [PATCH nand/next] mtd: nand: spi: Use write_cache first and then update

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <87bjsfv0x1.fsf@bootlin.com>
Date: Tue, 29 Apr 2025 10:15:22 +0200
From: Miquel Raynal <miquel.raynal@...tlin.com>
To: Chuanhong Guo <gch981213@...il.com>
Cc: Sky Huang <SkyLake.Huang@...iatek.com>,  Matthias Brugger
 <matthias.bgg@...il.com>,  AngeloGioacchino Del Regno
 <angelogioacchino.delregno@...labora.com>,  Richard Weinberger
 <richard@....at>,  Vignesh Raghavendra <vigneshr@...com>,  Daniel Golle
 <daniel@...rotopia.org>,  Chia-Lin Kao <acelan.kao@...onical.com>,  Cheng
 Ming Lin <chengminglin@...c.com.tw>,  Christophe JAILLET
 <christophe.jaillet@...adoo.fr>,  Pratyush Yadav <pratyush@...nel.org>,
  linux-kernel@...r.kernel.org,  linux-mtd@...ts.infradead.org,
  linux-arm-kernel@...ts.infradead.org,
  linux-mediatek@...ts.infradead.org,  Steven Liu <Steven.Liu@...iatek.com>
Subject: Re: [PATCH nand/next] mtd: nand: spi: Use write_cache first and
 then update_cache in write operation

Hello,

On 22/04/2025 at 09:38:26 +08, Chuanhong Guo <gch981213@...il.com> wrote:

> Hello!
> 在 2024/12/5 23:32, Miquel Raynal 写道:
>> Hello,
>> On 19/11/2024 at 17:39:49 +08, Sky Huang <SkyLake.Huang@...iatek.com>
>> wrote:
>> 
>>> From: Sky Huang <skylake.huang@...iatek.com>
>>>
>>> According to discussion with Miquel Raynal <miquel.raynal@...tlin.com>
>>> and Chuanhong Guo <gch981213@...il.com> on FORESEE F35SQA002G patch,
>>> Chuanhong recommmends that we can use the following sequence in
>>> spinand_write_to_cache_op():
>>>
>>> x1 mode:
>>> 02H(program data load) -> 84H(random program data load) -> 84H ...
>>>
>>> x4 mode:
>>> 32H(program data load x4) -> 34H(random program data load x4) -> 34H ...
>>>
>>> 02H or 32H commands will clear cache buffer on SPI-NAND and load
>>> data to it. For those SPI controllers which can't finish transmission
>>> in single step, 84H or 34H will be triggered for the rest data.
>>>
>>> We observe that some current SPI-NANDs, including FORESEE F35SQA001G and
>>> F35SQA002G, must use 02H or 32H to reset cache buffer in flash before
>>> using 84H or 34H. Or users may encounter program failure issue. This issue
>>> is not always reproducible, but it may occur and cause system instability.
>>>
>>> This sequence should work on all SPI-NANDs nowadays. I also check with
>>> Foresee that the sequence can solve the above program failure issue.
>>>
>>> On my test platform (MT7988), SPI driver is drivers/spi/spi-mt65xx.c.
>>> And I limit MTK_SPI_IPM_PACKET_SIZE to SZ_1K to simulate lightweight SPI
>>> controller which can only transmit 1024 bytes.
>>>
>>> The test step is the following:
>>> - mtd erase /dev/mtd2
>>> - dd if=/dev/zero bs=2048 count=1 | tr '\0' '\xA5' > output.bin
>>> - mtd write output.bin /dev/mtd2
>>>
>>> Before applying this patch, write operation uses only 34H(update_cache):
>>> [78.937720] OP code: 0x34, addr val: 0x0, data nbytes: 1020, data 1st byte: 0xa5
>>> [78.945297] OP code: 0x34, addr val: 0x3fc, data nbytes: 1020, data 1st byte: 0xa5
>>> [78.954251] OP code: 0x34, addr val: 0x7f8, data nbytes: 72, data 1st byte: 0xa5
>>> [78.962966] OP code: 0x10, addr val: 0x300
>>> [78.968816] OP code: 0x34, addr val: 0x0, data nbytes: 1020, data 1st byte: 0xff
>>> [78.977233] OP code: 0x34, addr val: 0x3fc, data nbytes: 1020, data 1st byte: 0xff
>>> [78.985124] OP code: 0x34, addr val: 0x7f8, data nbytes: 72, data 1st byte: 0xff
>>> [78.992527] OP code: 0x10, addr val: 0x301
>>> [78.996981] OP code: 0x34, addr val: 0x0, data nbytes: 1020, data 1st byte: 0xff
>>> [79.004416] OP code: 0x34, addr val: 0x3fc, data nbytes: 1020, data 1st byte: 0xff
>>> [79.012031] OP code: 0x34, addr val: 0x7f8, data nbytes: 72, data 1st byte: 0xff
>>> [79.019435] OP code: 0x10, addr val: 0x302
>> I am sorry but above you said that we should not perform:
>>      0x32, 0x32, 0x32...
>> because the second time it would clear the cache again. And here
>> you tell us that actually the core already handles that by performing
>> instead:
>>      0x34, 0x34, 0x34...
>> So what is the problem?
>> Or maybe I misunderstood the issue, but I think Chuanhong raised an
>> issue that is already solved? Isn't it?
>> 
>
> The issue is that the FORESEE NANDs require the first cache writing
> instruction to be WRITE_CACHE instead of UPDATE_CACHE. i.e. it needs a
> command sequence of:
>     0x32, 0x34, 0x34, 0x34...

So Foresee NANDs do not support update_cache, why are they advertised in
the first place? Could you we have a less impacting solution for the
other NANDs?

> This patch does exactly that, making the first instruction issued 0x32.
> It should be applied to fix the issue above.

My understanding is that this is very specific to FORESEE NANDs and you
are changing this for all NANDs. I have fears that it will break
everywhere else.

Overall I understand the problem, but I disagree with the fix. Could you
propose something less impacting as hinted above?

Thanks and sorry for the delay.
Miquèl