[RFC] nand/davinci: Fix comment to match the code

[RFC] nand/davinci: Fix comment to match the code

[Wolfram Sang]

Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
Cc: Sudhakar Rajashekhara <sudhakar.raj@ti.com>
Cc: Sneha Narnakaje <nsnehaprabha@ti.com>
Cc: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>
---

Found while debugging a NAND issue with a dm365.

 drivers/mtd/nand/davinci_nand.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/nand/davinci_nand.c b/drivers/mtd/nand/davinci_nand.c
index 2ac7367..53f864a 100644
--- a/drivers/mtd/nand/davinci_nand.c
+++ b/drivers/mtd/nand/davinci_nand.c
@@ -369,8 +369,9 @@ compare:
 	 * after setting the 4BITECC_ADD_CALC_START bit. So if you immediately
 	 * begin trying to poll for the state, you may fall right out of your
 	 * loop without any of the correction calculations having taken place.
-	 * The recommendation from the hardware team is to wait till ECC_STATE
-	 * reads less than 4, which means ECC HW has entered correction state.
+	 * The recommendation from the hardware team is to initially delay as
+	 * long as ECC_STATE reads less than 4. After that, ECC HW has entered
+	 * correction state.
 	 */
 	do {
 		ecc_state = (davinci_nand_readl(info,
-- 
1.7.1

RE: [RFC] nand/davinci: Fix comment to match the code

[Sudhakar Rajashekhara]

Hi,

On Wed, Aug 25, 2010 at 17:48:20, Wolfram Sang wrote:
> Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
> Cc: Sudhakar Rajashekhara <sudhakar.raj@ti.com>
> Cc: Sneha Narnakaje <nsnehaprabha@ti.com>
> Cc: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>
> ---
> 
> Found while debugging a NAND issue with a dm365.
> 
>  drivers/mtd/nand/davinci_nand.c |    5 +++--
>  1 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mtd/nand/davinci_nand.c b/drivers/mtd/nand/davinci_nand.c
> index 2ac7367..53f864a 100644
> --- a/drivers/mtd/nand/davinci_nand.c
> +++ b/drivers/mtd/nand/davinci_nand.c
> @@ -369,8 +369,9 @@ compare:
>  	 * after setting the 4BITECC_ADD_CALC_START bit. So if you immediately
>  	 * begin trying to poll for the state, you may fall right out of your
>  	 * loop without any of the correction calculations having taken place.
> -	 * The recommendation from the hardware team is to wait till ECC_STATE
> -	 * reads less than 4, which means ECC HW has entered correction state.
> +	 * The recommendation from the hardware team is to initially delay as
> +	 * long as ECC_STATE reads less than 4. After that, ECC HW has entered
> +	 * correction state.
>  	 */
>  	do {
>  		ecc_state = (davinci_nand_readl(info,
> -- 

Thanks for pointing this out, but I have already corrected this and submitted
the patch. Artem Bityutskity has pushed this patch to his l2-mtd-2.6 / master
tree.

Regards,
Sudhakar

RE: [RFC] nand/davinci: Fix comment to match the code

[Artem Bityutskiy]

On Wed, 2010-08-25 at 17:57 +0530, Sudhakar Rajashekhara wrote:
> > --- a/drivers/mtd/nand/davinci_nand.c
> > +++ b/drivers/mtd/nand/davinci_nand.c
> > @@ -369,8 +369,9 @@ compare:
> >  	 * after setting the 4BITECC_ADD_CALC_START bit. So if you immediately
> >  	 * begin trying to poll for the state, you may fall right out of your
> >  	 * loop without any of the correction calculations having taken place.
> > -	 * The recommendation from the hardware team is to wait till ECC_STATE
> > -	 * reads less than 4, which means ECC HW has entered correction state.
> > +	 * The recommendation from the hardware team is to initially delay as
> > +	 * long as ECC_STATE reads less than 4. After that, ECC HW has entered
> > +	 * correction state.
> >  	 */
> >  	do {
> >  		ecc_state = (davinci_nand_readl(info,
> > -- 
> 
> Thanks for pointing this out, but I have already corrected this and submitted
> the patch. Artem Bityutskity has pushed this patch to his l2-mtd-2.6 / master
> tree.

If you mean

commit 1c3275b656045aff9a75bb2c9f3251af1043ebb3
Author: Sudhakar Rajashekhara <sudhakar.raj@ti.com>
Date:   Tue Jul 20 15:24:01 2010 -0700

    mtd: nand: davinci: correct 4-bit error correction
    
    On TI's DA830/OMAP-L137, DA850/OMAP-L138 and DM365, after setting the
    4BITECC_ADD_CALC_START bit in the NAND Flash control register to 1 and
    before waiting for the NAND Flash status register to be equal to 1, 2 or
    3, we have to wait till the ECC HW goes to correction state.  Without this
    wait, ECC correction calculations will not be proper.
    
    This has been tested on DA830/OMAP-L137, DA850/OMAP-L138, DM355 and DM365
    EVMs.
    
    Signed-off-by: Sudhakar Rajashekhara <sudhakar.raj@ti.com>
    Acked-by: Sneha Narnakaje <nsnehaprabha@ti.com>
    Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
    Signed-off-by: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>
    Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>

then it is already in upstream in Linus' tree. I do not have anything else from you.

-- 
Best Regards,
Artem Bityutskiy (Артём Битюцкий)

Re: [RFC] nand/davinci: Fix comment to match the code

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 412 bytes --]

On Wed, Aug 25, 2010 at 03:55:06PM +0300, Artem Bityutskiy wrote:

> then it is already in upstream in Linus' tree. I do not have anything else from you.

I checked linux-next before creating this patch; the comment was the
same as in linus-tree.

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

RE: [RFC] nand/davinci: Fix comment to match the code

[Sudhakar Rajashekhara]

On Wed, Aug 25, 2010 at 18:25:06, Artem Bityutskiy wrote:
> On Wed, 2010-08-25 at 17:57 +0530, Sudhakar Rajashekhara wrote:
> > > --- a/drivers/mtd/nand/davinci_nand.c
> > > +++ b/drivers/mtd/nand/davinci_nand.c
> > > @@ -369,8 +369,9 @@ compare:
> > >  	 * after setting the 4BITECC_ADD_CALC_START bit. So if you immediately
> > >  	 * begin trying to poll for the state, you may fall right out of your
> > >  	 * loop without any of the correction calculations having taken place.
> > > -	 * The recommendation from the hardware team is to wait till ECC_STATE
> > > -	 * reads less than 4, which means ECC HW has entered correction state.
> > > +	 * The recommendation from the hardware team is to initially delay as
> > > +	 * long as ECC_STATE reads less than 4. After that, ECC HW has entered
> > > +	 * correction state.
> > >  	 */
> > >  	do {
> > >  		ecc_state = (davinci_nand_readl(info,
> > > -- 
> > 
> > Thanks for pointing this out, but I have already corrected this and submitted
> > the patch. Artem Bityutskity has pushed this patch to his l2-mtd-2.6 / master
> > tree.
> 
> If you mean
> 
> commit 1c3275b656045aff9a75bb2c9f3251af1043ebb3
> Author: Sudhakar Rajashekhara <sudhakar.raj@ti.com>
> Date:   Tue Jul 20 15:24:01 2010 -0700
> 
>     mtd: nand: davinci: correct 4-bit error correction
>     
>     On TI's DA830/OMAP-L137, DA850/OMAP-L138 and DM365, after setting the
>     4BITECC_ADD_CALC_START bit in the NAND Flash control register to 1 and
>     before waiting for the NAND Flash status register to be equal to 1, 2 or
>     3, we have to wait till the ECC HW goes to correction state.  Without this
>     wait, ECC correction calculations will not be proper.
>     
>     This has been tested on DA830/OMAP-L137, DA850/OMAP-L138, DM355 and DM365
>     EVMs.
>     
>     Signed-off-by: Sudhakar Rajashekhara <sudhakar.raj@ti.com>
>     Acked-by: Sneha Narnakaje <nsnehaprabha@ti.com>
>     Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
>     Signed-off-by: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>
>     Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
> 
> then it is already in upstream in Linus' tree. I do not have anything else from you.
> 

Yes, this was the patch I was talking about. But this patch seems little
different than the one I submitted to linux-mtd list at
http://patchwork.ozlabs.org/patch/59180/. But I can submit another patch
which corrects it. 

Wolfram Sang,
Do you agree that in the above link which shows the patch I have submitted,
the comment matches the code?

Regards,
Sudhakar

Re: [RFC] nand/davinci: Fix comment to match the code

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 1970 bytes --]

Sudhakar,

> > commit 1c3275b656045aff9a75bb2c9f3251af1043ebb3
> > Author: Sudhakar Rajashekhara <sudhakar.raj@ti.com>
> > Date:   Tue Jul 20 15:24:01 2010 -0700
> > 
> >     mtd: nand: davinci: correct 4-bit error correction
> >     
> >     On TI's DA830/OMAP-L137, DA850/OMAP-L138 and DM365, after setting the
> >     4BITECC_ADD_CALC_START bit in the NAND Flash control register to 1 and
> >     before waiting for the NAND Flash status register to be equal to 1, 2 or
> >     3, we have to wait till the ECC HW goes to correction state.  Without this
> >     wait, ECC correction calculations will not be proper.
> >     
> >     This has been tested on DA830/OMAP-L137, DA850/OMAP-L138, DM355 and DM365
> >     EVMs.
> >     
> >     Signed-off-by: Sudhakar Rajashekhara <sudhakar.raj@ti.com>
> >     Acked-by: Sneha Narnakaje <nsnehaprabha@ti.com>
> >     Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> >     Signed-off-by: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>
> >     Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
> > 
> > then it is already in upstream in Linus' tree. I do not have anything else from you.
> > 
> 
> Yes, this was the patch I was talking about. But this patch seems little
> different than the one I submitted to linux-mtd list at
> http://patchwork.ozlabs.org/patch/59180/. But I can submit another patch
> which corrects it. 
> 
> Wolfram Sang,
> Do you agree that in the above link which shows the patch I have submitted,
> the comment matches the code?

Yes. Also, where and how 'timeo' is handled is much better in this patch. If
you create an update patch to make the code look like in the patchwork-link,
you can instantly add my:

Acked-by: Wolfram Sang <w.sang@pengutronix.de>

Thanks,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [RFC] nand/davinci: Fix comment to match the code

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 428 bytes --]


> Yes, this was the patch I was talking about. But this patch seems little
> different than the one I submitted to linux-mtd list at
> http://patchwork.ozlabs.org/patch/59180/. But I can submit another patch
> which corrects it. 

Ping. Any timeframe for this?

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [RFC] nand/davinci: Fix comment to match the code

[Artem Bityutskiy]

On Wed, 2010-08-25 at 14:18 +0200, Wolfram Sang wrote:
> Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
> Cc: Sudhakar Rajashekhara <sudhakar.raj@ti.com>
> Cc: Sneha Narnakaje <nsnehaprabha@ti.com>
> Cc: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>
> ---
> 
> Found while debugging a NAND issue with a dm365.

Pushed to my l2-mtd-2.6.git / master, thanks.

-- 
Best Regards,
Artem Bityutskiy (Битюцкий Артём)

[PATCH] nand/davinci: relax a timeout for ECC-initialization

[Wolfram Sang]

Sudhakar found out that 100us are enough. Sadly, his updated patch was
overlooked and an older version still using 100ms was merged. Fix this.

Reference: http://patchwork.ozlabs.org/patch/59180/

Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
Cc: Sudhakar Rajashekhara <sudhakar.raj@ti.com>
Cc: Sneha Narnakaje <nsnehaprabha@ti.com>
Cc: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>
---

Artem, if you pick up this one, too, then we have what Sudhakar sent as
v3 of his original patch. Except for some minor rewording in the comment.

Sudhakar, I hope it is okay if I send this patch now. I just want to have
the confusion about the different versions gone.

 drivers/mtd/nand/davinci_nand.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/mtd/nand/davinci_nand.c b/drivers/mtd/nand/davinci_nand.c
index 53f864a..1ad0eed 100644
--- a/drivers/mtd/nand/davinci_nand.c
+++ b/drivers/mtd/nand/davinci_nand.c
@@ -313,7 +313,7 @@ static int nand_davinci_correct_4bit(struct mtd_info *mtd,
 	u32 syndrome[4];
 	u32 ecc_state;
 	unsigned num_errors, corrected;
-	unsigned long timeo = jiffies + msecs_to_jiffies(100);
+	unsigned long timeo;
 
 	/* All bytes 0xff?  It's an erased page; ignore its ECC. */
 	for (i = 0; i < 10; i++) {
@@ -373,6 +373,7 @@ compare:
 	 * long as ECC_STATE reads less than 4. After that, ECC HW has entered
 	 * correction state.
 	 */
+	timeo = jiffies + usecs_to_jiffies(100);
 	do {
 		ecc_state = (davinci_nand_readl(info,
 				NANDFSR_OFFSET) >> 8) & 0x0f;
-- 
1.7.1

RE: [PATCH] nand/davinci: relax a timeout for ECC-initialization

[Sudhakar Rajashekhara]

Hi Wolfram Sang,

On Fri, Sep 03, 2010 at 16:05:37, Wolfram Sang wrote:
> Sudhakar found out that 100us are enough. Sadly, his updated patch was
> overlooked and an older version still using 100ms was merged. Fix this.
> 
> Reference: http://patchwork.ozlabs.org/patch/59180/
> 
> Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
> Cc: Sudhakar Rajashekhara <sudhakar.raj@ti.com>
> Cc: Sneha Narnakaje <nsnehaprabha@ti.com>
> Cc: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>
> ---
> 
> Artem, if you pick up this one, too, then we have what Sudhakar sent as
> v3 of his original patch. Except for some minor rewording in the comment.
> 
> Sudhakar, I hope it is okay if I send this patch now. I just want to have
> the confusion about the different versions gone.

I was tied up for the past few days, so couldn't work on this patch. Thanks
for taking this up. Can you modify the comment also to match my patch and
re-submit this again?

Thanks,
Sudhakar

Re: [PATCH] nand/davinci: relax a timeout for ECC-initialization

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 593 bytes --]

Hi Sudhakar,

> > Sudhakar, I hope it is okay if I send this patch now. I just want to have
> > the confusion about the different versions gone.
> 
> I was tied up for the past few days, so couldn't work on this patch. Thanks
> for taking this up. Can you modify the comment also to match my patch and
> re-submit this again?

Honestly, I think the comments are similar enough, why another change?

Regards,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH] nand/davinci: relax a timeout for ECC-initialization

[Artem Bityutskiy]

On Fri, 2010-09-03 at 12:35 +0200, Wolfram Sang wrote:
> Sudhakar found out that 100us are enough. Sadly, his updated patch was
> overlooked and an older version still using 100ms was merged. Fix this.
> 
> Reference: http://patchwork.ozlabs.org/patch/59180/
> 
> Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
> Cc: Sudhakar Rajashekhara <sudhakar.raj@ti.com>
> Cc: Sneha Narnakaje <nsnehaprabha@ti.com>
> Cc: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>

Pushed to l2-mtd-2.6.git, thanks.

FYI, we prefix all MTD patches with "mtd:".

-- 
Best Regards,
Artem Bityutskiy (Артём Битюцкий)