linux-mtd.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: Miquel Raynal <miquel.raynal@bootlin.com>
To: Arseniy Krasnov <avkrasnov@sberdevices.ru>
Cc: Liang Yang <liang.yang@amlogic.com>,
	Richard Weinberger <richard@nod.at>,
	Vignesh Raghavendra <vigneshr@ti.com>,
	Neil Armstrong <neil.armstrong@linaro.org>,
	Kevin Hilman <khilman@baylibre.com>,
	Jerome Brunet <jbrunet@baylibre.com>,
	Martin Blumenstingl <martin.blumenstingl@googlemail.com>,
	Jianxin Pan <jianxin.pan@amlogic.com>,
	Yixun Lan <yixun.lan@amlogic.com>, <oxffffaa@gmail.com>,
	<kernel@sberdevices.ru>, <linux-mtd@lists.infradead.org>,
	<linux-arm-kernel@lists.infradead.org>,
	<linux-amlogic@lists.infradead.org>,
	<linux-kernel@vger.kernel.org>,
	"yonghui.yu" <yonghui.yu@amlogic.com>
Subject: Re: [PATCH v1 4/5] mtd: rawnand: meson: clear OOB buffer before read
Date: Tue, 18 Apr 2023 15:25:05 +0200	[thread overview]
Message-ID: <20230418152505.72fc16da@xps-13> (raw)
In-Reply-To: <e7c49f2d-b3c1-8d9b-76fe-c8759b37c7c7@sberdevices.ru>

Hi Arseniy,

> >> Hello again @Liang @Miquel!
> >>
> >> One more question about OOB access, as I can see current driver uses the following
> >> callbacks:
> >>
> >>     nand->ecc.write_oob_raw = nand_write_oob_std;
> >>     nand->ecc.write_oob = nand_write_oob_std;
> >>
> >>
> >> Function 'nand_write_oob_std()' writes data to the end of the page. But as I
> >> can see by dumping 'data_buf' during read, physical layout of each page is the
> >> following (1KB ECC):
> >>
> >> 0x000: [         1 KB of data        ]
> >> 0x400: [ 2B user data] [ 14B ECC code]
> >> 0x410: [         1 KB of data        ]    (A)
> >> 0x810: [ 2B user data] [ 14B ECC code]
> >> 0x820: [        32B unused           ]
> >>
> >>
> >>
> >> So, after 'nand_write_oob_std()' (let data be sequence from [0x0 ... 0x3f]),
> >> page will look like this:
> >>
> >> 0x000: [             0xFF            ]
> >> 0x400: [           ........          ]
> >> 0x7f0: [             0xFF            ]
> >> 0x800: [ 00 .......................  ]
> >> 0x830: [ ........................ 3f ]
> >>
> >> Here we have two problems:
> >> 1) Attempt to display raw data by 'nanddump' utility produces a little bit
> >>     invalid output, as driver relies on layout (A) from above. E.g. OOB data
> >>     is at 0x400 and 0x810. Here is an example (attempt to write 0x11 0x22 0x33 0x44):
> >>
> >> 0x000007f0: 11 22 ff ff ff ff ff ff ff ff ff ff ff ff ff ff  |."..............|
> >>    OOB Data: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff  |................|
> >>    OOB Data: 33 44 ff ff ff ff ff ff ff ff ff ff ff ff ff ff  |3D..............|
> >>    OOB Data: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff  |................|
> >>    OOB Data: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff  |................|
> >>  
> > Hi Arseniy,
> > 
> > I realized the write_oob_raw() and write_oob() are wrong in meson_nand.c. I suggest both of them should be reworked and follow the format of meson nand controller. i.e. firstly format the data in Layout (A) and then write. reading is firstly reading the data of layout (A) and then compost the layout (B).  
> 
> IIUC after such writing only OOB (e.g. user bytes) according layout (A), hw will also write ECC codes, so
> it will be impossible to write data to this page later, because we cannot update ECC codes properly for the newly
> written data (we can't update bits from 0 to 1).
> 
> > 
> >   
> >>
> >> 2) Attempt to read data in ECC mode will fail, because IIUC page is in dirty
> >>     state (I mean was written at least once) and NAND controller tries to use
> >>     ECC codes at 0x400 and 0x810, which are obviously broken in this case. Thus  
> > 
> > As i said above, write_oob_raw() and write_oob() should be reworked.
> > i don't know what do you mean page was written at least once. anyway the page should be written once, even just write_oob_raw().  
> 
> Sorry, You mean that after OOB write, we cannot write to the data area (e.g. 0x0 .. 0x810) until page will be erased? For example
> JFFS2 writes to OOB own markers, then it tries to write to the data area of such page.

A page is written after two steps:
- loading the data into the NAND chip cache (that's when you use the
  bus)
- programming the NAND array with the data loaded in cache (that's when
  you wait)

In theory it does not matter where you write in the cache, it's regular
DRAM, you can make random writes there with the appropriate NAND
commands. Of course when using embedded hardware ECC engines, the
controllers usually expect to be fed in a certain way in order to
produce the ECC bytes and put them at the right location in cache.

And then, when you actually send the "program" command, the NAND cells
actually get programmed based on what has been loaded in cache.

Thanks,
Miquèl

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

  reply	other threads:[~2023-04-18 13:25 UTC|newest]

Thread overview: 83+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-04-12  6:16 [PATCH v1 0/5] refactoring and fix for Meson NAND Arseniy Krasnov
2023-04-12  6:16 ` [PATCH v1 1/5] mtd: rawnand: meson: fix NAND access for read/write Arseniy Krasnov
2023-04-12  9:37   ` Liang Yang
2023-04-12 10:24     ` Arseniy Krasnov
2023-04-12 12:03       ` Arseniy Krasnov
2023-04-12 13:30         ` Liang Yang
2023-04-13  5:10           ` Arseniy Krasnov
2023-04-13  5:57             ` Liang Yang
2023-04-17  6:47               ` Arseniy Krasnov
2023-04-17 13:54                 ` Liang Yang
2023-04-17 14:10                   ` Arseniy Krasnov
2023-04-19 19:43                     ` Arseniy Krasnov
2023-04-20 14:22                       ` Liang Yang
2023-04-21  5:57                         ` Arseniy Krasnov
2023-04-26  7:53                           ` Arseniy Krasnov
2023-04-26 12:17                       ` Liang Yang
2023-04-26 14:47                         ` Arseniy Krasnov
2023-05-04  6:16                           ` Arseniy Krasnov
2023-05-10 11:34                             ` Arseniy Krasnov
2023-05-11 10:43                               ` Arseniy Krasnov
2023-04-12  6:16 ` [PATCH v1 2/5] mtd: rawnand: meson: replace GENMASK() macro with define Arseniy Krasnov
2023-04-12  7:37   ` Neil Armstrong
2023-04-12 10:06   ` David Laight
2023-04-12 10:11     ` Arseniy Krasnov
2023-04-12  6:16 ` [PATCH v1 3/5] mtd: rawnand: meson: check buffer length Arseniy Krasnov
2023-04-12  7:39   ` Miquel Raynal
2023-04-12  6:16 ` [PATCH v1 4/5] mtd: rawnand: meson: clear OOB buffer before read Arseniy Krasnov
2023-04-12  7:44   ` Miquel Raynal
2023-04-12  7:47     ` Miquel Raynal
2023-04-12  9:20     ` Arseniy Krasnov
2023-04-12  9:36       ` Miquel Raynal
2023-04-12 10:14         ` Arseniy Krasnov
2023-04-12 10:51           ` Liang Yang
2023-04-12 11:36             ` Liang Yang
2023-04-12 11:43               ` Dmitry Rokosov
2023-04-12 11:47                 ` Arseniy Krasnov
2023-04-12 12:28                 ` Liang Yang
2023-04-12 12:18           ` Miquel Raynal
2023-04-12 12:22             ` Arseniy Krasnov
2023-04-12 12:57               ` Miquel Raynal
2023-04-12 14:04                 ` Liang Yang
2023-04-12 14:32                   ` Miquel Raynal
2023-04-13  5:32                     ` Liang Yang
2023-04-13  6:11                       ` Liang Yang
2023-04-13  7:00                         ` Arseniy Krasnov
2023-04-13  8:22                           ` Miquel Raynal
2023-04-13  9:36                             ` Arseniy Krasnov
2023-04-13 10:22                               ` Miquel Raynal
2023-04-13 10:35                                 ` Arseniy Krasnov
2023-04-18  5:12                                   ` Arseniy Krasnov
2023-04-18 12:24                                     ` Liang Yang
2023-04-18 12:44                                       ` Arseniy Krasnov
2023-04-18 13:25                                         ` Miquel Raynal [this message]
2023-04-18 14:57                                           ` Arseniy Krasnov
2023-04-18 15:07                                             ` Arseniy Krasnov
2023-04-19  3:05                                             ` Liang Yang
2023-04-19  6:41                                               ` Arseniy Krasnov
2023-04-20  9:37                                                 ` Arseniy Krasnov
2023-04-26 13:51                                                   ` Liang Yang
2023-04-26 14:46                                                     ` Arseniy Krasnov
2023-05-02  9:59                                                       ` Miquel Raynal
2023-05-02 10:11                                                         ` Arseniy Krasnov
2023-05-02 11:27                                                           ` Miquel Raynal
2023-05-02 11:32                                                             ` Arseniy Krasnov
2023-05-02 12:17                                                               ` Miquel Raynal
2023-05-02 12:24                                                                 ` Arseniy Krasnov
2023-05-02 13:05                                                                   ` Miquel Raynal
2023-05-02 16:13                                                                     ` Arseniy Krasnov
2023-05-03  8:03                                                                       ` Miquel Raynal
2023-05-03 10:23                                                                         ` Arseniy Krasnov
2023-05-04 11:37                                                                           ` Arseniy Krasnov
2023-05-04 12:17                                                                             ` Miquel Raynal
2023-05-04 12:31                                                                               ` Arseniy Krasnov
2023-05-03 19:48                                                                         ` Richard Weinberger
2023-05-04 11:40                                                                           ` Arseniy Krasnov
2023-04-13  8:22                       ` Miquel Raynal
2023-04-12 19:15                   ` Dmitry Rokosov
2023-04-12 20:56                     ` Miquel Raynal
2023-04-13  9:27                       ` Dmitry Rokosov
2023-04-13 10:29                         ` Miquel Raynal
2023-04-13 14:03                           ` Dmitry Rokosov
2023-04-12  6:16 ` [PATCH v1 5/5] mtd: rawnand: meson: remove unneeded bitwise OR with zeroes Arseniy Krasnov
2023-04-12  7:45 ` [PATCH v1 0/5] refactoring and fix for Meson NAND Miquel Raynal

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230418152505.72fc16da@xps-13 \
    --to=miquel.raynal@bootlin.com \
    --cc=avkrasnov@sberdevices.ru \
    --cc=jbrunet@baylibre.com \
    --cc=jianxin.pan@amlogic.com \
    --cc=kernel@sberdevices.ru \
    --cc=khilman@baylibre.com \
    --cc=liang.yang@amlogic.com \
    --cc=linux-amlogic@lists.infradead.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mtd@lists.infradead.org \
    --cc=martin.blumenstingl@googlemail.com \
    --cc=neil.armstrong@linaro.org \
    --cc=oxffffaa@gmail.com \
    --cc=richard@nod.at \
    --cc=vigneshr@ti.com \
    --cc=yixun.lan@amlogic.com \
    --cc=yonghui.yu@amlogic.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).