From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail.bootlin.com ([62.4.15.54]) by bombadil.infradead.org with esmtp (Exim 4.90_1 #2 (Red Hat Linux)) id 1ffAXK-0005dq-Lo for linux-mtd@lists.infradead.org; Mon, 16 Jul 2018 20:56:04 +0000 Date: Mon, 16 Jul 2018 22:55:38 +0200 From: Boris Brezillon To: Chris Packham Cc: Richard Weinberger , Miquel Raynal , "linux-mtd@lists.infradead.org" , Marek Vasut , Brian Norris , David Woodhouse , "Bean Huo" Subject: Re: [2/3] mtd: rawnand: micron: Disable ECC earlier in the read path Message-ID: <20180716225538.542cb096@bbrezillon> In-Reply-To: <20180716181057.65d78303@bbrezillon> References: <20180703122009.29914-3-boris.brezillon@bootlin.com> <20180716181057.65d78303@bbrezillon> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit List-Id: Linux MTD discussion mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , On Mon, 16 Jul 2018 18:10:57 +0200 Boris Brezillon wrote: > On Mon, 16 Jul 2018 09:00:59 +0000 > Chris Packham wrote: > > > Hi Boris, > > > > On 04/07/18 00:20, Boris Brezillon wrote: > > > We are about to support extracting the real number of bitflips for > > > 4-bits ECC when WRITE_RECOMMEND is returned. This requires re-reading > > > the page in raw mode to compare it to the corrected version, and this > > > logic will be placed in micron_nand_on_die_ecc_status_4(). > > > > > > Moving the micron_nand_on_die_ecc_setup() will allow us to disable > > > ECC only once. > > > > > > As a result, we have to rework the exit path and add an error path > > > where the ECC is disabled. > > > > > > Signed-off-by: Boris Brezillon > > > > As I said on the other thread this appears to cause a problem for me on > > the MT29F1G08ABAFAWP-ITE setup I have. I notice we're not able to find > > the BBT, not sure if that is symptom or cause. It's most likely the symptom, not the cause. > > > > > > diff --git a/drivers/mtd/nand/raw/nand_micron.c b/drivers/mtd/nand/raw/nand_micron.c > > > index 63ac98a36ed7..b9cbaf125a98 100644 > > > --- a/drivers/mtd/nand/raw/nand_micron.c > > > +++ b/drivers/mtd/nand/raw/nand_micron.c > > > @@ -197,30 +197,37 @@ micron_nand_read_page_on_die_ecc(struct mtd_info *mtd, struct nand_chip *chip, > > > > > > ret = nand_read_page_op(chip, page, 0, NULL, 0); > > > if (ret) > > > - goto out; > > > + goto err_disable_ecc; > > > > > > ret = nand_status_op(chip, &status); > > > if (ret) > > > - goto out; > > > + goto err_disable_ecc; > > > > > > ret = nand_exit_status_op(chip); > > > if (ret) > > > - goto out; > > > + goto err_disable_ecc; > > > > > > - if (chip->ecc.strength == 4) > > > - max_bitflips = micron_nand_on_die_ecc_status_4(chip, status); > > > - else > > > - max_bitflips = micron_nand_on_die_ecc_status_8(chip, status); > > > + micron_nand_on_die_ecc_setup(chip, false); > > Hm, can you try to move the micron_nand_on_die_ecc_setup(chip, false) > call just before nand_exit_status_op()? > Just pushed a branch fixing that [1]. Can you test it? If it works, I'll ask Miquel to drop the initial set of patches and instead pick the fixed ones so that we don't break bisectibility. [1]https://github.com/bbrezillon/linux-0day/commits/nand/next