All of lore.kernel.org
 help / color / mirror / Atom feed
From: Boris Brezillon <boris.brezillon@free-electrons.com>
To: Sascha Hauer <s.hauer@pengutronix.de>
Cc: linux-mtd@lists.infradead.org,
	Richard Weinberger <richard@nod.at>,
	kernel@pengutronix.de, Han Xu <han.xu@nxp.com>
Subject: Re: [PATCH 02/10] mtd: nand: gpmi: Utilize hardware to detect bitflips in erased blocks
Date: Wed, 6 Dec 2017 16:34:31 +0100	[thread overview]
Message-ID: <20171206163431.5fddd6c6@bbrezillon> (raw)
In-Reply-To: <20171206152804.u62dopc6mwwdz457@pengutronix.de>

On Wed, 6 Dec 2017 16:28:04 +0100
Sascha Hauer <s.hauer@pengutronix.de> wrote:

> On Wed, Dec 06, 2017 at 10:27:13AM +0100, Boris Brezillon wrote:
> > Hi Sascha,
> > 
> > On Wed,  6 Dec 2017 10:19:17 +0100
> > Sascha Hauer <s.hauer@pengutronix.de> wrote:
> >   
> > > The GPMI nand has a hardware feature to ignore bitflips in erased pages.
> > > Use this feature rather than the longish code we have now.
> > > Unfortunately the bitflips in erased pages are not corrected, so we have
> > > to memset the read data before passing it to the upper layers.  
> > 
> > There's a good reason we didn't use the HW bitflip detection in the
> > first place: we currently have no way to report the number of corrected
> > bitflips in an erased page, and that's a big problem, because then UBI
> > does not know when it should re-erase the block.  
> 
> Ah, ok.
> 
> > 
> > Maybe we missed something when the initial proposal was done and
> > there's actually a way to retrieve this information, but if that's not
> > the case, I'd prefer to keep the existing implementation even if it's
> > slower and more verbose.  
> 
> I'm not so much concerned about the verbosity of the code but rather
> about the magic it has inside. I have stared at this code for some time
> now and still only vaguely know what it does.
> 
> We could do a bit better: We can still detect the number of bitflips
> using nand_check_erased_ecc_chunk() without reading the oob data.
> That would not be 100% accurate since we do not take the oob data into
> account which might have bitflips aswell, but still should be good
> enough, no?

Nope, we really have to take OOB bytes into account when counting
bitflips. Say that you have almost all in-band data set to 0xff, but
all OOB bytes reserved for ECC are set to 0 because someone decided to
write this portion of the page in raw mode. In this case we can't
consider the page as empty, otherwise we'll fail to correctly write ECC
bytes next time we program the page.

> 
> Sascha
> 

  reply	other threads:[~2017-12-06 15:35 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-12-06  9:19 GPMI nand driver cleanup Sascha Hauer
2017-12-06  9:19 ` [PATCH 01/10] mtd: nand: gpmi: Fix failure when a erased page has a bitflip at BBM Sascha Hauer
2017-12-06  9:19 ` [PATCH 02/10] mtd: nand: gpmi: Utilize hardware to detect bitflips in erased blocks Sascha Hauer
2017-12-06  9:27   ` Boris Brezillon
2017-12-06 15:28     ` Sascha Hauer
2017-12-06 15:34       ` Boris Brezillon [this message]
2017-12-08 10:21         ` Sascha Hauer
2017-12-08 10:35           ` Boris Brezillon
2017-12-08 10:57             ` Sascha Hauer
2017-12-06  9:19 ` [PATCH 03/10] mtd: nand: gpmi: drop dma_ops_type Sascha Hauer
2017-12-06  9:19 ` [PATCH 04/10] mtd: nand: gpmi: pass buffer and len around Sascha Hauer
2017-12-06  9:19 ` [PATCH 05/10] mtd: nand: gpmi: put only once used functions inline Sascha Hauer
2017-12-06  9:19 ` [PATCH 06/10] mtd: nand: gpmi: remove direct_dma_map_ok from driver data struct Sascha Hauer
2017-12-06  9:19 ` [PATCH 07/10] mtd: nand: gpmi: return valid value from bch_set_geometry() Sascha Hauer
2017-12-06  9:19 ` [PATCH 08/10] mtd: nand: gpmi: remove unnecessary variables Sascha Hauer
2017-12-06  9:19 ` [PATCH 09/10] mtd: nand: gpmi: rework gpmi_ecc_write_page Sascha Hauer
2017-12-06 14:57   ` Sascha Hauer
2017-12-06  9:19 ` [PATCH 10/10] mtd: nand: gpmi: return error code from gpmi_ecc_write_page Sascha Hauer

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=20171206163431.5fddd6c6@bbrezillon \
    --to=boris.brezillon@free-electrons.com \
    --cc=han.xu@nxp.com \
    --cc=kernel@pengutronix.de \
    --cc=linux-mtd@lists.infradead.org \
    --cc=richard@nod.at \
    --cc=s.hauer@pengutronix.de \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.