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

[Miquel Raynal <miquel.raynal@bootlin.com>]

Hello Chuanhong,

>> >>> 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.
>
> I just checked a few datasheets of SPI-NANDs from
> Toshiba/Winbond/Etron/ESMT/GigaDevice/Macronix.
> All of them specify the programming sequence to be:
> write_enable->write_cache->update_cache if needed->
> program_execute->poll status.
> Some of them mentions that the only difference between write_cache
> and update_cache is whether the page cache is cleared before
> writing (Winbond), while others don't specifically say that.
>
> The original sequence doesn't seem to be following any manufacturers'
> instructions. We just haven't run into any problems until this FORESEE
> one.

That is what I am saying. It currently works, and we may encounter
silent failures all around because of this change, even if it might be
theoretically correct. Anyhow, we have these templates (both write and
update) so let's try to re-use them.

However I dislike the current implementation which changes the global
wdesc template and puts it back. Can you please propose something cleaner?

Thanks,
Miquèl