Re: [PATCH nand/next] mtd: nand: spi: Use write_cache first and then update_cache in write operation

[Chuanhong Guo <gch981213@gmail.com>]

Hello!
在 2024/12/5 23:32, Miquel Raynal 写道:
> Hello,
> 
> On 19/11/2024 at 17:39:49 +08, Sky Huang <SkyLake.Huang@mediatek.com> wrote:
> 
>> From: Sky Huang <skylake.huang@mediatek.com>
>>
>> According to discussion with Miquel Raynal <miquel.raynal@bootlin.com>
>> and Chuanhong Guo <gch981213@gmail.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...
This patch does exactly that, making the first instruction issued 0x32.
It should be applied to fix the issue above.

---
Regards,
Chuanhong Guo