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] [day] [month] [year] [list]

Message-ID: <87sekry2xa.fsf@bootlin.com>
Date: Mon, 26 May 2025 14:25:21 +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 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