Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752364AbdGEHGV (ORCPT ); Wed, 5 Jul 2017 03:06:21 -0400 Received: from mail.free-electrons.com ([62.4.15.54]:42280 "EHLO mail.free-electrons.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752285AbdGEHEz (ORCPT ); Wed, 5 Jul 2017 03:04:55 -0400 Date: Wed, 5 Jul 2017 09:04:53 +0200 From: Boris Brezillon To: Miquel Raynal Cc: Richard Weinberger , David Woodhouse , Brian Norris , Marek Vasut , Cyrille Pitchen , linux-mtd@lists.infradead.org (open list:NAND FLASH SUBSYSTEM), linux-kernel@vger.kernel.org (open list) Subject: Re: [PATCH] nand: fix wrong default oob layout for small pages using soft ecc Message-ID: <20170705090453.7e412982@bbrezillon> In-Reply-To: <20170705065109.7738-1-miquel.raynal@free-electrons.com> References: <20170705065109.7738-1-miquel.raynal@free-electrons.com> X-Mailer: Claws Mail 3.13.2 (GTK+ 2.24.30; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2303 Lines: 72 Hi Miquel, On Wed, 5 Jul 2017 08:51:09 +0200 Miquel Raynal wrote: > When using soft ecc, if no ooblayout is given, the core automatically > uses one of the nand_ooblayout_{sp,lp}*() functions to determine the > layout inside the out of band data. > > Until kernel version 4.6, struct nand_ecclayout was used for that > purpose. During the migration from 4.6 to 4.7, an error shown up in the > small page layout, in the case oob section is only 8 bytes long. > > The layout was using three bytes (0, 1, 2) for ecc, two bytes (3, 4) > as free bytes, one byte (5) for bad block marker and finally > two bytes (6, 7) as free bytes, as shown there: > > [linux-4.6] drivers/mtd/nand/nand_base.c:52 > static struct nand_ecclayout nand_oob_8 = { > .eccbytes = 3, > .eccpos = {0, 1, 2}, > .oobfree = { > {.offset = 3, > .length = 2}, > {.offset = 6, > .length = 2} } > }; > > This fixes the current implementation which is incoherent. It > references bit 3 at the same time as an ecc byte and a free byte. > > Furthermore, it is clear with the previous implementation that there > is only one ecc section with 8 bytes oob sections. We shall return > -ERANGE in the nand_ooblayout_ecc_sp() function when asked for the > second section. > > Signed-off-by: Miquel Raynal Looks good to me. BTW, it looks like a good candidate for stable. No need to send a v2, I'll add the following when applying: Fixes: 41b207a70d3a ("mtd: nand: implement the default mtd_ooblayout_ops") Cc: Thanks, Boris > --- > drivers/mtd/nand/nand_base.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c > index b1dd12729f19..c5221795a1e8 100644 > --- a/drivers/mtd/nand/nand_base.c > +++ b/drivers/mtd/nand/nand_base.c > @@ -65,8 +65,14 @@ static int nand_ooblayout_ecc_sp(struct mtd_info *mtd, int section, > > if (!section) { > oobregion->offset = 0; > - oobregion->length = 4; > + if (mtd->oobsize == 16) > + oobregion->length = 4; > + else > + oobregion->length = 3; > } else { > + if (mtd->oobsize == 8) > + return -ERANGE; > + > oobregion->offset = 6; > oobregion->length = ecc->total - 4; > }