Re: [RFC PATCH v5 0/6] refactoring, fixes and updates for Meson NAND

Arseniy Krasnov
2023-06-01T07:59:37Z

On 01.06.2023 10:50, Miquel Raynal wrote:
> Hi Arseniy,
>
> AVKrasnov@sberdevices.ru wrote on Thu, 1 Jun 2023 09:18:43 +0300:
>
>> Hello,
>>
>> this patchset does several things:
>
> All the fixes should contain:
>
> Fixes: <hash> ("log")
> Cc: stable@vger.kernel.org

Hello Miquel,

Sorry. my fault, I'll add this in the next version for 0001 (anyway i think current is not final).

Thanks, Arseniy

>
>>
>> 1) Fixes value of ready/busy command. This new value was suggested by
>> Liang Yang <liang.yang@amlogic.com>.
>>
>> 2) Adds waiting for command completion by calling 'nand_soft_waitrdy()'
>> instead of using ready/busy pin and command from 1). This is really
>> needed because I don't have device with such pin implemented.
>>
>> 3) It moves OOB free bytes to non-protected by ECC area. Here are some
>> details:
>>
>> Current OOB free bytes are 4 bytes (2 x 2 user bytes) under ECC engine.
>> Here is how it looks like in the current implementation:
>>
>> [ 2B user bytes ][ 14B ECC codes ]
>> [ 2B user bytes ][ 14B ECC codes ]
>> [ 16B unused area, not protected by ECC ]
>> [ 16B unused area, not protected by ECC ]
>>
>> All 4 user bytes are protected by ECC. This patch changes OOB free
>> bytes in this way:
>>
>> [ 2B unused area ][ 14B ECC codes ]
>> [ 2B unused area ][ 14B ECC codes ]
>> [ 16B user bytes, not protected by ECC ]
>> [ 16B user bytes, not protected by ECC ]
>>
>> Now OOB user bytes are 32 bytes instead of 4 bytes and not protected
>> by ECC.
>>
>> Motivation of this layout comes from problem with JFFS2. It uses OOB
>> free bytes for cleanmarkers. Each cleanmarker is 4 bytes and written
>> by JFFS2 driver (small remark - cleanmarkers are always written in
>> case of NAND storage for JFFS2).
>> We have two ways to write this data to OOB (e.g. user bytes):
>>
>> 1) ECC mode. In this case it will be ECC covered user bytes, e.g.
>> writing this bytes will update ECC codes. Problem fires, when
>> JFFS2 tries to write this page later - this write makes controller
>> to update ECC codes again, but it is impossible to do it correctly,
>> because we can't update bits from 0 to 1 (only from 1 to 0).
>>
>> 2) Raw mode. In this case ECC codes won't be updated. But later, it
>> will be impossible to read this page in ECC mode, because we have
>> some user bytes, but ECC codes are missed.
>>
>> So let's move OOB free bytes out of ECC area. In this case we can
>> read/write OOB separately in raw mode and at the same time work with
>> data in ECC mode. JFFS2 is happy now. User bytes are untouched - all
>> of them are ignored during non-OOB access.
>>
>> I've tested this with mount/unmount/read/write cases for JFFS2 and
>> nanddump/nandwrite utlities on AXG family (A113X SoC).
>>
>> Here is link to discussion:
>> https://lore.kernel.org/linux-mtd/a9f8307a-77d7-a69f-ce11-2629909172d2@sberdevices.ru/T/#m3087bd06386a7f430cd5e343e22b25d724d3e2d7
>>
>> 4) Replaces calculation of OOB related thing with macros. This is just
>> cosmetic change.
>>
>> 5) Checks buffer length on accesses to NAND controller.
>>
>> 6) Removes useless bitwise OR with zeroes.
>>
>> Link to v1:
>> https://lore.kernel.org/linux-mtd/20230412061700.1492474-1-AVKrasnov@sberdevices.ru/
>> Link to v2:
>> https://lore.kernel.org/linux-mtd/20230426073632.3905682-1-AVKrasnov@sberdevices.ru/
>> Link to v3:
>> https://lore.kernel.org/linux-mtd/20230510110835.26115-1-AVKrasnov@sberdevices.ru/
>> Link to v4:
>> https://lore.kernel.org/linux-mtd/20230515094440.3552094-1-AVKrasnov@sberdevices.ru/
>>
>> Changelog:
>>
>> v1 -> v2:
>> * Add patch which renames dts value for chip select.
>> * Add patch which moves OOB to non-protected ECC area.
>> v2 -> v3:
>> * Change patch which fixes read/write access according discussion link
>> in 1) above.
>> v3 -> v4:
>> * Remove patch which renames dts value for chip select.
>> Here is link to discussion:
>> https://lore.kernel.org/linux-mtd/20230510110835.26115-7-AVKrasnov@sberdevices.ru/
>> * Pass 1 to 'meson_nfc_queue_rb()' in case of NAND_OP_WAITRDY_INSTR.
>> This fixes ONFI page processing during NAND driver initialization.
>> v4 -> v5:
>> * Move update of 'NFC_CMD_RB_INT' to the separate patch.
>> * Replace code which uses extra status and READ0 commands for waiting
>> command with 'nand_soft_waitrdy()'. In fact this patch adds second
>> mode for command waiting by using 'nand_soft_waitrdy()'.
>> * For OOB layout patch see changelog in a patch file.
>> * For check length patch see changelog in a patch file.
>>
>> Arseniy Krasnov (6):
>> mtd: rawnand: meson: fix ready/busy command
>> mtd: rawnand: meson: wait for command in polling mode
>> mtd: rawnand: meson: only expose unprotected user OOB bytes
>> mtd: rawnand: meson: use macro for OOB area
>> mtd: rawnand: meson: check buffer length
>> mtd: rawnand: meson: remove unneeded bitwise OR with zeroes
>>
>> drivers/mtd/nand/raw/meson_nand.c | 234 +++++++++++++++++++++++-------
>> 1 file changed, 182 insertions(+), 52 deletions(-)
>>
>
>
> Thanks,
> Miquèl