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

2011-12-08 Thread Scott Wood
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-
 m...@lists.infradead.org; dw...@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

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


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

2011-12-07 Thread Scott Wood
On 12/07/2011 12:30 AM, Liu Shengzhou-B36685 wrote:
 
 -Original Message-
 From: Wood Scott-B07421
 Sent: Wednesday, December 07, 2011 1:16 AM
 To: Liu Shengzhou-B36685
 Cc: linuxppc-dev@lists.ozlabs.org; linux-...@lists.infradead.org;
 dw...@infradead.org; Gala Kumar-B11780
 Subject: Re: [PATCH 1/2 v2] mtd/nand: fixup for fmr initialization of
 Freescale NAND controller

 On 12/06/2011 02:54 AM, Shengzhou Liu wrote:
 There was a bug for fmr initialization, which lead to  fmr was always
 0x100 in fsl_elbc_chip_init() and caused FCM command timeout before
 calling fsl_elbc_chip_init_tail(), now we initialize CWTO to maximum
 timeout value and not relying on the setting of bootloader.

 Signed-off-by: Shengzhou Liu shengzhou@freescale.com
 ---
 v2: make fmr not relying on the setting of bootloader.

  drivers/mtd/nand/fsl_elbc_nand.c |   10 +-
  1 files changed, 5 insertions(+), 5 deletions(-)

 diff --git a/drivers/mtd/nand/fsl_elbc_nand.c
 b/drivers/mtd/nand/fsl_elbc_nand.c
 index eedd8ee..4f405a0 100644
 --- a/drivers/mtd/nand/fsl_elbc_nand.c
 +++ b/drivers/mtd/nand/fsl_elbc_nand.c
 @@ -659,9 +659,7 @@ static int fsl_elbc_chip_init_tail(struct mtd_info
 *mtd)
 if (chip-pagemask  0xff00)
 al++;

 -   /* add to ECCM mode set in fsl_elbc_init */
 -   priv-fmr |= (12  FMR_CWTO_SHIFT) |  /* Timeout  12 ms */
 -(al  FMR_AL_SHIFT);
 +   priv-fmr |= al  FMR_AL_SHIFT;

 dev_dbg(priv-dev, fsl_elbc_init: nand-numchips = %d\n,
 chip-numchips);
 @@ -764,8 +762,10 @@ static int fsl_elbc_chip_init(struct fsl_elbc_mtd
 *priv)
 priv-mtd.priv = chip;
 priv-mtd.owner = THIS_MODULE;

 -   /* Set the ECCM according to the settings in bootloader.*/
 -   priv-fmr = in_be32(lbc-fmr)  FMR_ECCM;
 +   /* set timeout to maximum */
 +   priv-fmr = 15  FMR_CWTO_SHIFT;
 +   if (in_be32(lbc-bank[priv-bank].or)  OR_FCM_PGS)
 +   priv-fmr |= FMR_ECCM;

 Please do not change the way ECCM is handled.  We probably should have
 done it this way from the start, but at this point it breaks
 compatibility if you have a large page flash and the firmware didn't
 touch NAND.

 -Scott
 [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

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


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

2011-12-07 Thread Liu Shengzhou-B36685


 -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-
 m...@lists.infradead.org; dw...@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:
 
  -Original Message-
  From: Wood Scott-B07421
  Sent: Wednesday, December 07, 2011 1:16 AM
  To: Liu Shengzhou-B36685
  Cc: linuxppc-dev@lists.ozlabs.org; linux-...@lists.infradead.org;
  dw...@infradead.org; Gala Kumar-B11780
  Subject: Re: [PATCH 1/2 v2] mtd/nand: fixup for fmr initialization of
  Freescale NAND controller
 
  On 12/06/2011 02:54 AM, Shengzhou Liu wrote:
  There was a bug for fmr initialization, which lead to  fmr was
  always 0x100 in fsl_elbc_chip_init() and caused FCM command timeout
  before calling fsl_elbc_chip_init_tail(), now we initialize CWTO to
  maximum timeout value and not relying on the setting of bootloader.
 
  Signed-off-by: Shengzhou Liu shengzhou@freescale.com
  ---
  v2: make fmr not relying on the setting of bootloader.
 
   drivers/mtd/nand/fsl_elbc_nand.c |   10 +-
   1 files changed, 5 insertions(+), 5 deletions(-)
 
  diff --git a/drivers/mtd/nand/fsl_elbc_nand.c
  b/drivers/mtd/nand/fsl_elbc_nand.c
  index eedd8ee..4f405a0 100644
  --- a/drivers/mtd/nand/fsl_elbc_nand.c
  +++ b/drivers/mtd/nand/fsl_elbc_nand.c
  @@ -659,9 +659,7 @@ static int fsl_elbc_chip_init_tail(struct
  mtd_info
  *mtd)
if (chip-pagemask  0xff00)
al++;
 
  - /* add to ECCM mode set in fsl_elbc_init */
  - priv-fmr |= (12  FMR_CWTO_SHIFT) |  /* Timeout  12 ms */
  -  (al  FMR_AL_SHIFT);
  + priv-fmr |= al  FMR_AL_SHIFT;
 
dev_dbg(priv-dev, fsl_elbc_init: nand-numchips = %d\n,
chip-numchips);
  @@ -764,8 +762,10 @@ static int fsl_elbc_chip_init(struct
  fsl_elbc_mtd
  *priv)
priv-mtd.priv = chip;
priv-mtd.owner = THIS_MODULE;
 
  - /* Set the ECCM according to the settings in bootloader.*/
  - priv-fmr = in_be32(lbc-fmr)  FMR_ECCM;
  + /* set timeout to maximum */
  + priv-fmr = 15  FMR_CWTO_SHIFT;
  + if (in_be32(lbc-bank[priv-bank].or)  OR_FCM_PGS)
  + priv-fmr |= FMR_ECCM;
 
  Please do not change the way ECCM is handled.  We probably should
  have done it this way from the start, but at this point it breaks
  compatibility if you have a large page flash and the firmware didn't
  touch NAND.
 
  -Scott
  [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. Do we still need CONFIG_FSL_ELBC_FMR in kernel? 
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


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

2011-12-06 Thread Scott Wood
On 12/06/2011 02:54 AM, Shengzhou Liu wrote:
 There was a bug for fmr initialization, which lead to  fmr was always 0x100
 in fsl_elbc_chip_init() and caused FCM command timeout before calling
 fsl_elbc_chip_init_tail(), now we initialize CWTO to maximum timeout value
 and not relying on the setting of bootloader.
 
 Signed-off-by: Shengzhou Liu shengzhou@freescale.com
 ---
 v2: make fmr not relying on the setting of bootloader.
 
  drivers/mtd/nand/fsl_elbc_nand.c |   10 +-
  1 files changed, 5 insertions(+), 5 deletions(-)
 
 diff --git a/drivers/mtd/nand/fsl_elbc_nand.c 
 b/drivers/mtd/nand/fsl_elbc_nand.c
 index eedd8ee..4f405a0 100644
 --- a/drivers/mtd/nand/fsl_elbc_nand.c
 +++ b/drivers/mtd/nand/fsl_elbc_nand.c
 @@ -659,9 +659,7 @@ static int fsl_elbc_chip_init_tail(struct mtd_info *mtd)
   if (chip-pagemask  0xff00)
   al++;
  
 - /* add to ECCM mode set in fsl_elbc_init */
 - priv-fmr |= (12  FMR_CWTO_SHIFT) |  /* Timeout  12 ms */
 -  (al  FMR_AL_SHIFT);
 + priv-fmr |= al  FMR_AL_SHIFT;
  
   dev_dbg(priv-dev, fsl_elbc_init: nand-numchips = %d\n,
   chip-numchips);
 @@ -764,8 +762,10 @@ static int fsl_elbc_chip_init(struct fsl_elbc_mtd *priv)
   priv-mtd.priv = chip;
   priv-mtd.owner = THIS_MODULE;
  
 - /* Set the ECCM according to the settings in bootloader.*/
 - priv-fmr = in_be32(lbc-fmr)  FMR_ECCM;
 + /* set timeout to maximum */
 + priv-fmr = 15  FMR_CWTO_SHIFT;
 + if (in_be32(lbc-bank[priv-bank].or)  OR_FCM_PGS)
 + priv-fmr |= FMR_ECCM;

Please do not change the way ECCM is handled.  We probably should have
done it this way from the start, but at this point it breaks
compatibility if you have a large page flash and the firmware didn't
touch NAND.

-Scott

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


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

2011-12-06 Thread Liu Shengzhou-B36685

 -Original Message-
 From: Wood Scott-B07421
 Sent: Wednesday, December 07, 2011 1:16 AM
 To: Liu Shengzhou-B36685
 Cc: linuxppc-dev@lists.ozlabs.org; linux-...@lists.infradead.org;
 dw...@infradead.org; Gala Kumar-B11780
 Subject: Re: [PATCH 1/2 v2] mtd/nand: fixup for fmr initialization of
 Freescale NAND controller
 
 On 12/06/2011 02:54 AM, Shengzhou Liu wrote:
  There was a bug for fmr initialization, which lead to  fmr was always
  0x100 in fsl_elbc_chip_init() and caused FCM command timeout before
  calling fsl_elbc_chip_init_tail(), now we initialize CWTO to maximum
  timeout value and not relying on the setting of bootloader.
 
  Signed-off-by: Shengzhou Liu shengzhou@freescale.com
  ---
  v2: make fmr not relying on the setting of bootloader.
 
   drivers/mtd/nand/fsl_elbc_nand.c |   10 +-
   1 files changed, 5 insertions(+), 5 deletions(-)
 
  diff --git a/drivers/mtd/nand/fsl_elbc_nand.c
  b/drivers/mtd/nand/fsl_elbc_nand.c
  index eedd8ee..4f405a0 100644
  --- a/drivers/mtd/nand/fsl_elbc_nand.c
  +++ b/drivers/mtd/nand/fsl_elbc_nand.c
  @@ -659,9 +659,7 @@ static int fsl_elbc_chip_init_tail(struct mtd_info
 *mtd)
  if (chip-pagemask  0xff00)
  al++;
 
  -   /* add to ECCM mode set in fsl_elbc_init */
  -   priv-fmr |= (12  FMR_CWTO_SHIFT) |  /* Timeout  12 ms */
  -(al  FMR_AL_SHIFT);
  +   priv-fmr |= al  FMR_AL_SHIFT;
 
  dev_dbg(priv-dev, fsl_elbc_init: nand-numchips = %d\n,
  chip-numchips);
  @@ -764,8 +762,10 @@ static int fsl_elbc_chip_init(struct fsl_elbc_mtd
 *priv)
  priv-mtd.priv = chip;
  priv-mtd.owner = THIS_MODULE;
 
  -   /* Set the ECCM according to the settings in bootloader.*/
  -   priv-fmr = in_be32(lbc-fmr)  FMR_ECCM;
  +   /* set timeout to maximum */
  +   priv-fmr = 15  FMR_CWTO_SHIFT;
  +   if (in_be32(lbc-bank[priv-bank].or)  OR_FCM_PGS)
  +   priv-fmr |= FMR_ECCM;
 
 Please do not change the way ECCM is handled.  We probably should have
 done it this way from the start, but at this point it breaks
 compatibility if you have a large page flash and the firmware didn't
 touch NAND.
 
 -Scott
[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.  
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev