Re: [PATCH 1/2 v2] mtd/nand: fixup for fmr initialization of Freescale NAND controller

[Scott Wood <scottwood@freescale.com>]

On 12/07/2011 09:36 PM, Liu Shengzhou-B36685 wrote:
> 
> 
>> -----Original Message-----
>> From: Wood Scott-B07421
>> Sent: Thursday, December 08, 2011 1:17 AM
>> To: Liu Shengzhou-B36685
>> Cc: Wood Scott-B07421; linuxppc-dev@lists.ozlabs.org; linux-
>> mtd@lists.infradead.org; dwmw2@infradead.org; Gala Kumar-B11780
>> Subject: Re: [PATCH 1/2 v2] mtd/nand: fixup for fmr initialization of
>> Freescale NAND controller
>>
>> On 12/07/2011 12:30 AM, Liu Shengzhou-B36685 wrote:
>>> [Shengzhou] This patch doesn't change the way ECCM is handled, it's
>> still same as before, just make sure CWTO timeout is set to maximum.
>>
>> It does change it.  It used to use the existing value in FMR, and now it
>> sets it based on ORn[PGS].
>>
>> -Scott
> 
> [Shengzhou]
>   In u-boot:
> 	#ifdef CONFIG_FSL_ELBC_FMR
>            priv->fmr = CONFIG_FSL_ELBC_FMR;
> 	#else
> 	     priv->fmr = (15 << FMR_CWTO_SHIFT) | (2 << FMR_AL_SHIFT);
> 	     or = in_be32(&elbc_ctrl->regs->bank[priv->bank].or);
> 	     if (or & OR_FCM_PGS)
>     		       priv->fmr |= FMR_ECCM;
> 	#endif
> 
> In kernel: It used to be " priv->fmr = in_be32(&lbc->fmr) & FMR_ECCM
> ", so fmr was always 0x100(or 0,depend on ORn[PGS]), CWTO was
> 0(timeout was minimum).   In this patch, for not relying on
> bootloader, fmr is initialized as what u-boot does, except
> FMR_AL_SHIFT is handled in fsl_elbc_chip_init_tail and without
> definition of CONFIG_FSL_ELBC_FMR.
> 
>   So, it doesn't change it.

You're assuming that the above U-Boot code is always run.  This depends
on whether the NAND driver is enabled in U-Boot.

In the future, though, it might also depend on whether a NAND command is
actually run in U-Boot -- this makes the setting of FMR
non-deterministic between boots, which is worse than a one-time breakage
of an unusual setup (driver not enabled in U-Boot at all).

So it is a change, but I now think it's a change we should make.  The
changelog should mention that this is happening, though.

> Do we still need CONFIG_FSL_ELBC_FMR in kernel? 

We do not want such a compile-time constant in the kernel.  Use ORn[PGS]
as the patch currently does.

-Scott