[U-Boot] [PATCH 1/2] mxc nand: Add support for i.MX5

[Benoît Thébaudeau]

Dear Marek Vasut,

On Tuesday, January 8, 2013 11:38:39 AM, Marek Vasut wrote:
> Dear Benoît Thébaudeau,
> 
> > Dear Marek Vasut,
> > 
> > On Tuesday, January 8, 2013 10:33:29 AM, Marek Vasut wrote:
> > > Subject: [PATCH 1/2] mxc nand: Add support for i.MX5
> > 
> > Where is the 2/2?
> 
> Nowhere, it's a typo.

OK.

> > > From: Benoît Thébaudeau <benoit.thebaudeau at advansee.com>
> > > 
> > > Signed-off-by: Benoît Thébaudeau <benoit.thebaudeau at advansee.com>
> > > Cc: Scott Wood <scottwood at freescale.com>
> > > Cc: Stefano Babic <sbabic at denx.de>
> > > ---
> > > 
> > >  arch/arm/include/asm/arch-mx5/imx-regs.h |    9 ++
> > >  drivers/mtd/nand/mxc_nand.c              |  219
> > >  ++++++++++++++++++++++--------
> > >  include/fsl_nfc.h                        |  149
> > >  +++++++++++++-------
> > >  nand_spl/nand_boot_fsl_nfc.c             |  114 ++++++++++++----
> > >  4 files changed, 364 insertions(+), 127 deletions(-)
> > > 
> > > NOTE: I'm seeing issues when this is compiled into U-Boot. U-Boot
> > > won't boot on
> > > my MX53 board and will hang right after printing "DRAM:" and
> > > before
> > > printing the
> > > CPU info. I suspect it's some kind of unaligned access.
> > 
> > OK. I'm waiting for your test results. It works for me on i.MX51
> > with
> > 2012.07.
> 
> It works with 2013.01-rc1, but I have issues. When I boot the board
> from SD, it
> hangs. When I load via JTAG, it doesn't hang. I suspect it's
> something related
> to the MXC NAND driver, since if the driver is not compiled in, it
> works like
> charm both ways.
> 
> Otherwise, when booted, the NAND works fine.
> 
> I wonder if there's some unaligned access or something happening in
> the NAND
> driver. Or maybe some other issue where the code is miscompiled.

Or there is something wrong with the Flash header or SPL?

> > > NOTE2: I fixed a few complaints from GCC
> > 
> > Which ones? I don't see any such change in this new version.
> 
> This one I think:
> 
> 354 @@ -1167,8 +1226,8 @@ static struct nand_bbt_descr
> bbt_mirror_descr = {
> 355  int board_nand_init(struct nand_chip *this)
> 356  {
> 357         struct mtd_info *mtd;
> 358 -#ifdef MXC_NFC_V2_1
> 359 -       uint16_t tmp;
> 360 +#if defined(MXC_NFC_V2_1) || defined(MXC_NFC_V3_2)
> 361 +       uint32_t tmp;
> 362  #endif
> 363
> 364  #ifdef CONFIG_SYS_NAND_USE_FLASH_BBT

This is just the rebase. Or perhaps you started from
http://patchwork.ozlabs.org/patch/177098/ instead of
http://patchwork.ozlabs.org/patch/179176/.

> > > and rebased on top of
> > > master.
> > 
> > OK.
> > 
> > [--snip--]
> > 
> > > @@ -698,7 +757,7 @@ static int mxc_nand_correct_data(struct
> > > mtd_info
> > > *mtd, u_char *dat,
> > > 
> > >  	 * additional correction.  2-Bit errors cannot be corrected by
> > >  	 * HW ECC, so we need to return failure
> > >  	 */
> > > 
> > > -	uint16_t ecc_status = readw(&host->regs->ecc_status_result);
> > > +	uint32_t ecc_status = readnfc(&host->regs->ecc_status_result);
> > 
> >         ^
> > Here it was uint16_t in my last version, and this was correct since
> > this is
> > code for MXC_NFC_V1. This change should not hurt, but it was not
> > necessary.
> 
> But in case readnfc() results in readl(), u16 is too small.

readnfc() always results in readw() for MXC_NFC_V1, so u16 is just fine here.

> > Is this change related to the GCC complaints that you mentioned?
> 
> No, there's one more, see above.
> 
> > >  	if (((ecc_status & 0x3) == 2) || ((ecc_status >> 2) == 2)) {
> > >  	
> > >  		MTDDEBUG(MTD_DEBUG_LEVEL0,
> > 
> > [--snip--]
> > 
> > The other changes (i.e. the rebase) are fine with me.
> 
> I hope I didn't screw up the attribution. I used "From:" which I hope
> is
> correct.

It's correctly filled, but I don't know if it's used by U-Boot's Git process. I
have already seen this "From:" used before.

Best regards,
Benoît