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

login
register
mail settings
Submitter Wolfram Sang
Date Aug. 25, 2010, 12:18 p.m.
Message ID <1282738700-30966-1-git-send-email-w.sang@pengutronix.de>
Download mbox | patch
Permalink /patch/62675/
State New
Headers show

Comments

Wolfram Sang - Aug. 25, 2010, 12:18 p.m.
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(-)
Rajashekhara, Sudhakar - Aug. 25, 2010, 12:27 p.m.
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
Artem Bityutskiy - Aug. 25, 2010, 12:55 p.m.
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.
Wolfram Sang - Aug. 25, 2010, 1:30 p.m.
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.
Rajashekhara, Sudhakar - Aug. 26, 2010, 3:59 a.m.
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
Wolfram Sang - Aug. 26, 2010, 6:23 a.m.
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
Wolfram Sang - Sept. 1, 2010, 8:05 a.m.
> 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?
Artem Bityutskiy - Sept. 1, 2010, 11:14 p.m.
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.

Patch

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,