All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] Enable the read ECC before program the page
@ 2017-09-13 19:15 Arun Nagendran
  2017-09-13 21:31 ` Greg KH
  0 siblings, 1 reply; 6+ messages in thread
From: Arun Nagendran @ 2017-09-13 19:15 UTC (permalink / raw)
  To: gregkh; +Cc: manonuevo, linux-mtd

Current program_page function did following operation:

1. read page (with ECC OFF)
2. modify the page
3. write the page (with ECC ON)

For some case(buggy flash Chip), while read the page without ECC ON,
we may read the page with bit flip error and modify that bad page without
knowing the bit flip error on that page.
also we re-calculate the hash for bad page and write it.
This could bring potential in-consistency problem with Flash data.

Verify this logic with GIGA DEVICE Part(GD5F2GQ4RCFIG):
we see this in-conststency problem wit Giga Device and fix on
this patch resovle that issue.

Signed-off-by: Arun Nagendran <arunrasppi@gmail.com>
---
 drivers/staging/mt29f_spinand/mt29f_spinand.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/mt29f_spinand/mt29f_spinand.c b/drivers/staging/mt29f_spinand/mt29f_spinand.c
index 13eaf16..87595c5 100644
--- a/drivers/staging/mt29f_spinand/mt29f_spinand.c
+++ b/drivers/staging/mt29f_spinand/mt29f_spinand.c
@@ -496,8 +496,12 @@ static int spinand_program_page(struct spi_device *spi_nand,
 	if (!wbuf)
 		return -ENOMEM;
 
-	enable_read_hw_ecc = 0;
-	spinand_read_page(spi_nand, page_id, 0, CACHE_BUF, wbuf);
+	enable_read_hw_ecc = 1;
+	retval = spinand_read_page(spi_nand, page_id, 0, CACHE_BUF, wbuf);
+	if (retval < 0) {
+		dev_err(&spi_nand->dev, "ecc error on read page!!!\n");
+		return retval;
+	}
 
 	for (i = offset, j = 0; i < len; i++, j++)
 		wbuf[i] &= buf[j];
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] Enable the read ECC before program the page
  2017-09-13 19:15 [PATCH] Enable the read ECC before program the page Arun Nagendran
@ 2017-09-13 21:31 ` Greg KH
  2017-09-14 15:56   ` Arun Nagendran
  0 siblings, 1 reply; 6+ messages in thread
From: Greg KH @ 2017-09-13 21:31 UTC (permalink / raw)
  To: Arun Nagendran; +Cc: manonuevo, linux-mtd

On Wed, Sep 13, 2017 at 03:15:26PM -0400, Arun Nagendran wrote:
> Current program_page function did following operation:
> 
> 1. read page (with ECC OFF)
> 2. modify the page
> 3. write the page (with ECC ON)
> 
> For some case(buggy flash Chip), while read the page without ECC ON,
> we may read the page with bit flip error and modify that bad page without
> knowing the bit flip error on that page.
> also we re-calculate the hash for bad page and write it.
> This could bring potential in-consistency problem with Flash data.
> 
> Verify this logic with GIGA DEVICE Part(GD5F2GQ4RCFIG):
> we see this in-conststency problem wit Giga Device and fix on
> this patch resovle that issue.
> 
> Signed-off-by: Arun Nagendran <arunrasppi@gmail.com>

Please read Documentation/SubmittingPatches for how to properly version
your patches so we know what is going on and which one to accept/review.

Also your subject line needs some work, look at the other subjects for
patches that have been accepted for this driver/subsystem for examples
of what needs to be done.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Enable the read ECC before program the page
  2017-09-13 21:31 ` Greg KH
@ 2017-09-14 15:56   ` Arun Nagendran
  0 siblings, 0 replies; 6+ messages in thread
From: Arun Nagendran @ 2017-09-14 15:56 UTC (permalink / raw)
  To: Greg KH; +Cc: manonuevo, linux-mtd

On Wed, Sep 13, 2017 at 02:31:54PM -0700, Greg KH wrote:
> On Wed, Sep 13, 2017 at 03:15:26PM -0400, Arun Nagendran wrote:
> > Current program_page function did following operation:
> > 
> > 1. read page (with ECC OFF)
> > 2. modify the page
> > 3. write the page (with ECC ON)
> > 
> > For some case(buggy flash Chip), while read the page without ECC ON,
> > we may read the page with bit flip error and modify that bad page without
> > knowing the bit flip error on that page.
> > also we re-calculate the hash for bad page and write it.
> > This could bring potential in-consistency problem with Flash data.
> > 
> > Verify this logic with GIGA DEVICE Part(GD5F2GQ4RCFIG):
> > we see this in-conststency problem wit Giga Device and fix on
> > this patch resovle that issue.
> > 
> > Signed-off-by: Arun Nagendran <arunrasppi@gmail.com>
> 
> Please read Documentation/SubmittingPatches for how to properly version
> your patches so we know what is going on and which one to accept/review.
> 
> Also your subject line needs some work, look at the other subjects for
> patches that have been accepted for this driver/subsystem for examples
> of what needs to be done.
> 
> thanks,
> 
> greg k-h

Thank you Greg on your review comments, I am working on 
your suggestion.

Thanks,
-Arun

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Enable the read ECC before program the page
  2017-09-13 18:34 Arun Nagendran
@ 2017-09-13 19:04 ` Greg KH
  0 siblings, 0 replies; 6+ messages in thread
From: Greg KH @ 2017-09-13 19:04 UTC (permalink / raw)
  To: Arun Nagendran; +Cc: manonuevo, linux-mtd

On Wed, Sep 13, 2017 at 02:34:53PM -0400, Arun Nagendran wrote:
> Current program_page function did following operation:
> 
> 1. read page (with ECC OFF)
> 2. modify the page
> 3. write the page (with ECC ON)
> 
> For some case(buggy flash Chip), while read the page without ECC ON,
> we may read the page with bit flip error and modify that bad page without
> knowing the bit flip error on that page.
> also we re-calculate the hash for bad page and write it.
> This could bring potential in-consistency problem with Flash data.
> 
> Verify this logic with GIGA DEVICE Part(GD5F2GQ4RCFIG):
> we see this in-conststency problem wit Giga Device and fix on
> this patch resovle that issue.
> ---
>  drivers/staging/mt29f_spinand/mt29f_spinand.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/mt29f_spinand/mt29f_spinand.c b/drivers/staging/mt29f_spinand/mt29f_spinand.c
> index a4e3ae8..202d222 100644
> --- a/drivers/staging/mt29f_spinand/mt29f_spinand.c
> +++ b/drivers/staging/mt29f_spinand/mt29f_spinand.c
> @@ -496,8 +496,13 @@ static int spinand_program_page(struct spi_device *spi_nand,
>  	if (!wbuf)
>  		return -ENOMEM;
>  
> -	enable_read_hw_ecc = 0;
> -	spinand_read_page(spi_nand, page_id, 0, CACHE_BUF, wbuf);
> +	enable_read_hw_ecc = 1;
> +	retval = spinand_read_page(spi_nand, page_id, 0, CACHE_BUF, wbuf);
> +	if( retval < 0 )
> +	{
> +		dev_err(&spi_nand->dev, "ecc error on read page!!!\n");
> +		return retval;
> +	}

Always use scripts/checkpatch.pl so you don't get grumpy maintainers
telling you to use scripts/checkpatch.pl :(

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH] Enable the read ECC before program the page
@ 2017-09-13 18:34 Arun Nagendran
  2017-09-13 19:04 ` Greg KH
  0 siblings, 1 reply; 6+ messages in thread
From: Arun Nagendran @ 2017-09-13 18:34 UTC (permalink / raw)
  To: gregkh; +Cc: manonuevo, linux-mtd

Current program_page function did following operation:

1. read page (with ECC OFF)
2. modify the page
3. write the page (with ECC ON)

For some case(buggy flash Chip), while read the page without ECC ON,
we may read the page with bit flip error and modify that bad page without
knowing the bit flip error on that page.
also we re-calculate the hash for bad page and write it.
This could bring potential in-consistency problem with Flash data.

Verify this logic with GIGA DEVICE Part(GD5F2GQ4RCFIG):
we see this in-conststency problem wit Giga Device and fix on
this patch resovle that issue.
---
 drivers/staging/mt29f_spinand/mt29f_spinand.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/mt29f_spinand/mt29f_spinand.c b/drivers/staging/mt29f_spinand/mt29f_spinand.c
index a4e3ae8..202d222 100644
--- a/drivers/staging/mt29f_spinand/mt29f_spinand.c
+++ b/drivers/staging/mt29f_spinand/mt29f_spinand.c
@@ -496,8 +496,13 @@ static int spinand_program_page(struct spi_device *spi_nand,
 	if (!wbuf)
 		return -ENOMEM;
 
-	enable_read_hw_ecc = 0;
-	spinand_read_page(spi_nand, page_id, 0, CACHE_BUF, wbuf);
+	enable_read_hw_ecc = 1;
+	retval = spinand_read_page(spi_nand, page_id, 0, CACHE_BUF, wbuf);
+	if( retval < 0 )
+	{
+		dev_err(&spi_nand->dev, "ecc error on read page!!!\n");
+		return retval;
+	}
 
 	for (i = offset, j = 0; i < len; i++, j++)
 		wbuf[i] &= buf[j];
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH] Enable the read ECC before program the page.
@ 2017-09-13 18:20 Arun Nagendran
  0 siblings, 0 replies; 6+ messages in thread
From: Arun Nagendran @ 2017-09-13 18:20 UTC (permalink / raw)
  To: kamlakant.patel; +Cc: gregkh, manonuevo, linux-mtd

Current program_page function did following operation:

1. read page (with ECC OFF)
2. modify the page
3. write the page (with ECC ON)

For some case(buggy flash Chip), while read the page without ECC ON,
we may read the page with bit flip error and modify that bad page without
knowing the bit flip error on that page.
also we re-calculate the hash for bad page and write it.
This could bring potential in-consistency problem with Flash data.

Verify this logic with GIGA DEVICE Part(GD5F2GQ4RCFIG):
we see this in-conststency problem wit Giga Device and fix on
this patch resovle that issue.
---
 drivers/staging/mt29f_spinand/mt29f_spinand.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/mt29f_spinand/mt29f_spinand.c b/drivers/staging/mt29f_spinand/mt29f_spinand.c
index a4e3ae8..2b03cc9 100644
--- a/drivers/staging/mt29f_spinand/mt29f_spinand.c
+++ b/drivers/staging/mt29f_spinand/mt29f_spinand.c
@@ -496,8 +496,12 @@ static int spinand_program_page(struct spi_device *spi_nand,
 	if (!wbuf)
 		return -ENOMEM;
 
-	enable_read_hw_ecc = 0;
-	spinand_read_page(spi_nand, page_id, 0, CACHE_BUF, wbuf);
+	enable_read_hw_ecc = 1;
+	retval = spinand_read_page(spi_nand, page_id, 0, CACHE_BUF, wbuf);
+	{
+		dev_err(&spi_nand->dev, "ecc error on read page!!!\n");
+		return retval;
+	}
 
 	for (i = offset, j = 0; i < len; i++, j++)
 		wbuf[i] &= buf[j];
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2017-09-14 15:57 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-09-13 19:15 [PATCH] Enable the read ECC before program the page Arun Nagendran
2017-09-13 21:31 ` Greg KH
2017-09-14 15:56   ` Arun Nagendran
  -- strict thread matches above, loose matches on Subject: below --
2017-09-13 18:34 Arun Nagendran
2017-09-13 19:04 ` Greg KH
2017-09-13 18:20 Arun Nagendran

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.