From mboxrd@z Thu Jan  1 00:00:00 1970
From: =?ISO-8859-1?Q?J=F6rg?= Krause <joerg.krause@embedded.rocks>
Date: Mon, 13 Apr 2015 13:38:56 +0200
Subject: [U-Boot] [PATCH] mxs_nand: Fix ECC strength for NAND flash with
 OOB size of 256
In-Reply-To: <552B85DA.2090503@denx.de>
References: <1428826652-314-1-git-send-email-hs@denx.de>
 <1428914386.26500.5.camel@embedded.rocks>
 <201504131042.02552.marex@denx.de> <552B85DA.2090503@denx.de>
Message-ID: <1428925136.8695.17.camel@embedded.rocks>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

Hi Marek, Heiko,

On Mo, 2015-04-13 at 11:01 +0200, Heiko Schocher wrote:
> Hello Marek, Joerg,
> 
> Am 13.04.2015 10:42, schrieb Marek Vasut:
> > On Monday, April 13, 2015 at 10:39:46 AM, J?rg Krause wrote:
> > > Hi Heiko,
> > > 
> > > On So, 2015-04-12 at 10:17 +0200, Heiko Schocher wrote:
> > > > On the i.mx6 based aristainetos2 board a Toshiba 
> > > > TH58NYG3S0HBAI4
> > > > is used, which has 4096 pagesize and 256b oob. The ECC strength
> > > > was not correct detected by U-Boot
> > > > 
> > > > Signed-off-by: Heiko Schocher <hs@denx.de>
> > > > ---
> > > > 
> > > >   drivers/mtd/nand/mxs_nand.c | 3 +++
> > > >   1 file changed, 3 insertions(+)
> > > > 
> > > > diff --git a/drivers/mtd/nand/mxs_nand.c
> > > > b/drivers/mtd/nand/mxs_nand.c
> > > > index 2d2b938..00bf036 100644
> > > > --- a/drivers/mtd/nand/mxs_nand.c
> > > > +++ b/drivers/mtd/nand/mxs_nand.c
> > > > @@ -163,6 +163,9 @@ static inline uint32_t
> > > > mxs_nand_get_ecc_strength(uint32_t page_data_size,
> > > > 
> > > >                if (page_oob_size == 224)
> > > > 
> > > >                        return 16;
> > > > 
> > > > +
> > > > +             if (page_oob_size == 256)
> > > > +                     return 18;
> > > > 
> > > >        }
> > > > 
> > > >        return 0;
> > > 
> > > How about calculation the ECC strength dynamically? Peng Fan from
> > > Freescale send a patch doing this in December 2014 which was 
> > > already
> > > reviewed by Marek:
> > > https://patchwork.ozlabs.org/patch/422756/
> > > 
> > > It is also necessary to change the calculation in mxsboot...
> > 
> > Would be nice if the patch got applied, but I think there were some
> > comments and the patch was never fixed/reposted. If Heiko wants to
> > do it, that'd be nice.
> 
> Hmm.. I feel, I have not much time left for fixing such things...

I can re-submit the patch from Peng Fan together with my fixes for 
mxsboot.

> Joerg: You wrote on Jan. 27, 2015, 11:14 p.m.:
> "I was able to fix mxsboot, but I had difficulties with round_down, 
> which
> is a macro definition in linux/kernel.h. I've copied the macro
> definition to mxsboot. I will submit the patch in a seperate mail."
> 
> Did you post such a patch? Was this the onyl problem of the patch
> from Peng Fan?

No, I didn't. I waited for some comment and then I just forgot about 
it. The main problem with the patch from Peng Fan was that it was not 
consistent with mxsboot, which still has the hardcoded oobsizes. I 
copied the calculation to mxsboot.c, but failed to include 
linux/kernel.h, because mxsboot is compiled with the host compiler and 
u-boot with the cross-compiler. So I just copied the macro definition 
for round_down from kernel.h to mxsboot.c.

> "I would like to see a comment or a macro for the magic number 13, 
> which
> is the value for the Galois Field, just for clarification"
> 
> I have no idea what 13 means ...

This is cited from the i.MX28 Reference Manual:
    BCH-codes are a type of block-code, which implies that all error-
    correction is performed over a block of N-symbols. The BCH 
    operation will be performed over GF (2^13 = 8192), which is the 
    Galois Field consisting of 8191 one-bit symbols.

> 
> > Nice domain name btw ;-)
> 
> Indeed.

Thanks :-)

> 
> bye,
> Heiko