All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] mtd: nand: Wait for PAGEPROG to finish in drivers setting NAND_ECC_CUSTOM_PAGE_ACCESS
@ 2017-05-16 16:27 Boris Brezillon
  2017-05-17 10:41 ` Marc Gonzalez
  2017-05-29 18:51 ` Boris Brezillon
  0 siblings, 2 replies; 5+ messages in thread
From: Boris Brezillon @ 2017-05-16 16:27 UTC (permalink / raw)
  To: Wenyou Yang, Marc Gonzalez, Boris Brezillon, Richard Weinberger,
	linux-mtd
  Cc: David Woodhouse, Brian Norris, Marek Vasut, Cyrille Pitchen

Drivers setting NAND_ECC_CUSTOM_PAGE_ACCESS are supposed to handle the
full read/write page sequence, and waiting for a page to actually be
programmed is part of this write-page sequence.
This is also what is done in ->write_oob_xxx() hooks, so let's do that in
->write_page_xxx() as well to make it consistent.

Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
---
 drivers/mtd/nand/atmel/nand-controller.c |  6 +++++-
 drivers/mtd/nand/nand_base.c             | 10 ++++++----
 drivers/mtd/nand/nand_micron.c           | 10 ++++++++--
 drivers/mtd/nand/tango_nand.c            | 13 ++++++++++++-
 4 files changed, 31 insertions(+), 8 deletions(-)

diff --git a/drivers/mtd/nand/atmel/nand-controller.c b/drivers/mtd/nand/atmel/nand-controller.c
index 3b2446896147..e5122539d4f1 100644
--- a/drivers/mtd/nand/atmel/nand-controller.c
+++ b/drivers/mtd/nand/atmel/nand-controller.c
@@ -912,7 +912,7 @@ static int atmel_hsmc_nand_pmecc_write_pg(struct nand_chip *chip,
 	struct mtd_info *mtd = nand_to_mtd(chip);
 	struct atmel_nand *nand = to_atmel_nand(chip);
 	struct atmel_hsmc_nand_controller *nc;
-	int ret;
+	int ret, status;
 
 	nc = to_hsmc_nand_controller(chip->controller);
 
@@ -954,6 +954,10 @@ static int atmel_hsmc_nand_pmecc_write_pg(struct nand_chip *chip,
 		dev_err(nc->base.dev, "Failed to program NAND page (err = %d)\n",
 			ret);
 
+	status = chip->waitfunc(mtd, chip);
+	if (status & NAND_STATUS_FAIL)
+		return -EIO;
+
 	return ret;
 }
 
diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index 8dafd2a54e11..08ff98c47e1f 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -2574,11 +2574,13 @@ static int nand_write_page(struct mtd_info *mtd, struct nand_chip *chip,
 	if (status < 0)
 		return status;
 
-	if (nand_standard_page_accessors(&chip->ecc))
+	if (nand_standard_page_accessors(&chip->ecc)) {
 		chip->cmdfunc(mtd, NAND_CMD_PAGEPROG, -1, -1);
-	status = chip->waitfunc(mtd, chip);
-	if (status & NAND_STATUS_FAIL)
-		return -EIO;
+
+		status = chip->waitfunc(mtd, chip);
+		if (status & NAND_STATUS_FAIL)
+			return -EIO;
+	}
 
 	return 0;
 }
diff --git a/drivers/mtd/nand/nand_micron.c b/drivers/mtd/nand/nand_micron.c
index 0987f32dc6f7..f3c35fafd51a 100644
--- a/drivers/mtd/nand/nand_micron.c
+++ b/drivers/mtd/nand/nand_micron.c
@@ -151,15 +151,18 @@ micron_nand_write_page_on_die_ecc(struct mtd_info *mtd, struct nand_chip *chip,
 				  const uint8_t *buf, int oob_required,
 				  int page)
 {
+	int status;
+
 	micron_nand_on_die_ecc_setup(chip, true);
 
 	chip->cmdfunc(mtd, NAND_CMD_SEQIN, 0x00, page);
 	nand_write_page_raw(mtd, chip, buf, oob_required, page);
 	chip->cmdfunc(mtd, NAND_CMD_PAGEPROG, -1, -1);
+	status = chip->waitfunc(mtd, chip);
 
 	micron_nand_on_die_ecc_setup(chip, false);
 
-	return 0;
+	return status & NAND_STATUS_FAIL ? -EIO : 0;
 }
 
 static int
@@ -180,11 +183,14 @@ micron_nand_write_page_raw_on_die_ecc(struct mtd_info *mtd,
 				      const uint8_t *buf, int oob_required,
 				      int page)
 {
+	int status;
+
 	chip->cmdfunc(mtd, NAND_CMD_SEQIN, 0x00, page);
 	nand_write_page_raw(mtd, chip, buf, oob_required, page);
 	chip->cmdfunc(mtd, NAND_CMD_PAGEPROG, -1, -1);
+	status = chip->waitfunc(mtd, chip);
 
-	return 0;
+	return status & NAND_STATUS_FAIL ? -EIO : 0;
 }
 
 enum {
diff --git a/drivers/mtd/nand/tango_nand.c b/drivers/mtd/nand/tango_nand.c
index a2150b15d4c1..8498fa36e533 100644
--- a/drivers/mtd/nand/tango_nand.c
+++ b/drivers/mtd/nand/tango_nand.c
@@ -295,7 +295,7 @@ static int tango_write_page(struct mtd_info *mtd, struct nand_chip *chip,
 			    const u8 *buf, int oob_required, int page)
 {
 	struct tango_nfc *nfc = to_tango_nfc(chip->controller);
-	int err, len = mtd->writesize;
+	int err, status, len = mtd->writesize;
 
 	/* Calling tango_write_oob() would send PAGEPROG twice */
 	if (oob_required)
@@ -306,6 +306,10 @@ static int tango_write_page(struct mtd_info *mtd, struct nand_chip *chip,
 	if (err)
 		return err;
 
+	status = chip->waitfunc(mtd, chip);
+	if (status & NAND_STATUS_FAIL)
+		return -EIO;
+
 	return 0;
 }
 
@@ -423,9 +427,16 @@ static int tango_read_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
 static int tango_write_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
 				const u8 *buf, int oob_required, int page)
 {
+	int status;
+
 	chip->cmdfunc(mtd, NAND_CMD_SEQIN, 0, page);
 	raw_write(chip, buf, chip->oob_poi);
 	chip->cmdfunc(mtd, NAND_CMD_PAGEPROG, -1, -1);
+
+	status = chip->waitfunc(mtd, chip);
+	if (status & NAND_STATUS_FAIL)
+		return -EIO;
+
 	return 0;
 }
 
-- 
2.7.4

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

* Re: [PATCH] mtd: nand: Wait for PAGEPROG to finish in drivers setting NAND_ECC_CUSTOM_PAGE_ACCESS
  2017-05-16 16:27 [PATCH] mtd: nand: Wait for PAGEPROG to finish in drivers setting NAND_ECC_CUSTOM_PAGE_ACCESS Boris Brezillon
@ 2017-05-17 10:41 ` Marc Gonzalez
  2017-05-17 10:51   ` Boris Brezillon
  2017-05-29 18:51 ` Boris Brezillon
  1 sibling, 1 reply; 5+ messages in thread
From: Marc Gonzalez @ 2017-05-17 10:41 UTC (permalink / raw)
  To: Boris Brezillon, Richard Weinberger, linux-mtd
  Cc: Wenyou Yang, David Woodhouse, Brian Norris, Marek Vasut, Cyrille Pitchen

On 16/05/2017 18:27, Boris Brezillon wrote:

> Drivers setting NAND_ECC_CUSTOM_PAGE_ACCESS are supposed to handle the
> full read/write page sequence, and waiting for a page to actually be
> programmed is part of this write-page sequence.
> This is also what is done in ->write_oob_xxx() hooks, so let's do that in
> ->write_page_xxx() as well to make it consistent.
> 
> Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
> ---
>  drivers/mtd/nand/atmel/nand-controller.c |  6 +++++-
>  drivers/mtd/nand/nand_base.c             | 10 ++++++----
>  drivers/mtd/nand/nand_micron.c           | 10 ++++++++--
>  drivers/mtd/nand/tango_nand.c            | 13 ++++++++++++-
>  4 files changed, 31 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
> index 8dafd2a54e11..08ff98c47e1f 100644
> --- a/drivers/mtd/nand/nand_base.c
> +++ b/drivers/mtd/nand/nand_base.c
> @@ -2574,11 +2574,13 @@ static int nand_write_page(struct mtd_info *mtd, struct nand_chip *chip,
>  	if (status < 0)
>  		return status;
>  
> -	if (nand_standard_page_accessors(&chip->ecc))
> +	if (nand_standard_page_accessors(&chip->ecc)) {
>  		chip->cmdfunc(mtd, NAND_CMD_PAGEPROG, -1, -1);
> -	status = chip->waitfunc(mtd, chip);
> -	if (status & NAND_STATUS_FAIL)
> -		return -EIO;
> +
> +		status = chip->waitfunc(mtd, chip);
> +		if (status & NAND_STATUS_FAIL)
> +			return -EIO;
> +	}

AFAIU, the wait operation used to be unconditional;
but it is now skipped for CUSTOM accessors. OK.


> diff --git a/drivers/mtd/nand/tango_nand.c b/drivers/mtd/nand/tango_nand.c
> index a2150b15d4c1..8498fa36e533 100644
> --- a/drivers/mtd/nand/tango_nand.c
> +++ b/drivers/mtd/nand/tango_nand.c
> @@ -295,7 +295,7 @@ static int tango_write_page(struct mtd_info *mtd, struct nand_chip *chip,
>  			    const u8 *buf, int oob_required, int page)
>  {
>  	struct tango_nfc *nfc = to_tango_nfc(chip->controller);
> -	int err, len = mtd->writesize;
> +	int err, status, len = mtd->writesize;
>  
>  	/* Calling tango_write_oob() would send PAGEPROG twice */
>  	if (oob_required)
> @@ -306,6 +306,10 @@ static int tango_write_page(struct mtd_info *mtd, struct nand_chip *chip,
>  	if (err)
>  		return err;
>  
> +	status = chip->waitfunc(mtd, chip);
> +	if (status & NAND_STATUS_FAIL)
> +		return -EIO;
> +
>  	return 0;
>  }

When I introduced the custom_accessors flag, I missed the removal
of this wait operation. The tango NFC is supposed to take care of
everything, from start to finish.

I applied the following patch:

diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -2675,6 +2675,9 @@ static int nand_write_page(struct mtd_info *mtd, struct nand_chip *chip,
        if (status < 0)
                return status;
 
+       if (!nand_standard_page_accessors(&chip->ecc))
+               return 0;
+

And ran mtd_speedtest. No measurable change.
mtd_stresstest did not detect any issues.

> @@ -423,9 +427,16 @@ static int tango_read_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
>  static int tango_write_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
>  				const u8 *buf, int oob_required, int page)
>  {
> +	int status;
> +
>  	chip->cmdfunc(mtd, NAND_CMD_SEQIN, 0, page);
>  	raw_write(chip, buf, chip->oob_poi);
>  	chip->cmdfunc(mtd, NAND_CMD_PAGEPROG, -1, -1);
> +
> +	status = chip->waitfunc(mtd, chip);
> +	if (status & NAND_STATUS_FAIL)
> +		return -EIO;
> +
>  	return 0;
>  }

This one would still be required, since we're going behind the NFC's back.
Should we test NAND_STATUS_FAIL in tango_write_oob() too?
It might not matter, since you plan to change it to nand_prog_page_end_op()
ultimately.

Regards.

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

* Re: [PATCH] mtd: nand: Wait for PAGEPROG to finish in drivers setting NAND_ECC_CUSTOM_PAGE_ACCESS
  2017-05-17 10:41 ` Marc Gonzalez
@ 2017-05-17 10:51   ` Boris Brezillon
  2017-05-19 11:41     ` Marc Gonzalez
  0 siblings, 1 reply; 5+ messages in thread
From: Boris Brezillon @ 2017-05-17 10:51 UTC (permalink / raw)
  To: Marc Gonzalez
  Cc: Richard Weinberger, linux-mtd, Wenyou Yang, David Woodhouse,
	Brian Norris, Marek Vasut, Cyrille Pitchen

On Wed, 17 May 2017 12:41:01 +0200
Marc Gonzalez <marc_gonzalez@sigmadesigns.com> wrote:

> On 16/05/2017 18:27, Boris Brezillon wrote:
> 
> > Drivers setting NAND_ECC_CUSTOM_PAGE_ACCESS are supposed to handle the
> > full read/write page sequence, and waiting for a page to actually be
> > programmed is part of this write-page sequence.
> > This is also what is done in ->write_oob_xxx() hooks, so let's do that in  
> > ->write_page_xxx() as well to make it consistent.  
> > 
> > Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
> > ---
> >  drivers/mtd/nand/atmel/nand-controller.c |  6 +++++-
> >  drivers/mtd/nand/nand_base.c             | 10 ++++++----
> >  drivers/mtd/nand/nand_micron.c           | 10 ++++++++--
> >  drivers/mtd/nand/tango_nand.c            | 13 ++++++++++++-
> >  4 files changed, 31 insertions(+), 8 deletions(-)
> > 
> > diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
> > index 8dafd2a54e11..08ff98c47e1f 100644
> > --- a/drivers/mtd/nand/nand_base.c
> > +++ b/drivers/mtd/nand/nand_base.c
> > @@ -2574,11 +2574,13 @@ static int nand_write_page(struct mtd_info *mtd, struct nand_chip *chip,
> >  	if (status < 0)
> >  		return status;
> >  
> > -	if (nand_standard_page_accessors(&chip->ecc))
> > +	if (nand_standard_page_accessors(&chip->ecc)) {
> >  		chip->cmdfunc(mtd, NAND_CMD_PAGEPROG, -1, -1);
> > -	status = chip->waitfunc(mtd, chip);
> > -	if (status & NAND_STATUS_FAIL)
> > -		return -EIO;
> > +
> > +		status = chip->waitfunc(mtd, chip);
> > +		if (status & NAND_STATUS_FAIL)
> > +			return -EIO;
> > +	}  
> 
> AFAIU, the wait operation used to be unconditional;
> but it is now skipped for CUSTOM accessors. OK.

Yep.

> 
> 
> > diff --git a/drivers/mtd/nand/tango_nand.c b/drivers/mtd/nand/tango_nand.c
> > index a2150b15d4c1..8498fa36e533 100644
> > --- a/drivers/mtd/nand/tango_nand.c
> > +++ b/drivers/mtd/nand/tango_nand.c
> > @@ -295,7 +295,7 @@ static int tango_write_page(struct mtd_info *mtd, struct nand_chip *chip,
> >  			    const u8 *buf, int oob_required, int page)
> >  {
> >  	struct tango_nfc *nfc = to_tango_nfc(chip->controller);
> > -	int err, len = mtd->writesize;
> > +	int err, status, len = mtd->writesize;
> >  
> >  	/* Calling tango_write_oob() would send PAGEPROG twice */
> >  	if (oob_required)
> > @@ -306,6 +306,10 @@ static int tango_write_page(struct mtd_info *mtd, struct nand_chip *chip,
> >  	if (err)
> >  		return err;
> >  
> > +	status = chip->waitfunc(mtd, chip);
> > +	if (status & NAND_STATUS_FAIL)
> > +		return -EIO;
> > +
> >  	return 0;
> >  }  
> 
> When I introduced the custom_accessors flag, I missed the removal
> of this wait operation. The tango NFC is supposed to take care of
> everything, from start to finish.
> 
> I applied the following patch:
> 
> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
> --- a/drivers/mtd/nand/nand_base.c
> +++ b/drivers/mtd/nand/nand_base.c
> @@ -2675,6 +2675,9 @@ static int nand_write_page(struct mtd_info *mtd, struct nand_chip *chip,
>         if (status < 0)
>                 return status;
>  
> +       if (!nand_standard_page_accessors(&chip->ecc))
> +               return 0;
> +
> 
> And ran mtd_speedtest. No measurable change.
> mtd_stresstest did not detect any issues.

Okay. I think I'll keep the patch as is to avoid introducing extra
functional changes.
You can then send a patch removing the extra chip->waitfunc() from
tango_write_page() and explain why this is unneeded in your commit
message. Would that work for you?

> 
> > @@ -423,9 +427,16 @@ static int tango_read_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
> >  static int tango_write_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
> >  				const u8 *buf, int oob_required, int page)
> >  {
> > +	int status;
> > +
> >  	chip->cmdfunc(mtd, NAND_CMD_SEQIN, 0, page);
> >  	raw_write(chip, buf, chip->oob_poi);
> >  	chip->cmdfunc(mtd, NAND_CMD_PAGEPROG, -1, -1);
> > +
> > +	status = chip->waitfunc(mtd, chip);
> > +	if (status & NAND_STATUS_FAIL)
> > +		return -EIO;
> > +
> >  	return 0;
> >  }  
> 
> This one would still be required, since we're going behind the NFC's back.
> Should we test NAND_STATUS_FAIL in tango_write_oob() too?

Yes, you should.

> It might not matter, since you plan to change it to nand_prog_page_end_op()
> ultimately.

Well, I don't know when this will land, so we'd better fix it now and
mark it for stable backport.

Regards,

Boris

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

* Re: [PATCH] mtd: nand: Wait for PAGEPROG to finish in drivers setting NAND_ECC_CUSTOM_PAGE_ACCESS
  2017-05-17 10:51   ` Boris Brezillon
@ 2017-05-19 11:41     ` Marc Gonzalez
  0 siblings, 0 replies; 5+ messages in thread
From: Marc Gonzalez @ 2017-05-19 11:41 UTC (permalink / raw)
  To: Boris Brezillon
  Cc: Richard Weinberger, linux-mtd, Wenyou Yang, David Woodhouse,
	Brian Norris, Marek Vasut, Cyrille Pitchen

On 17/05/2017 12:51, Boris Brezillon wrote:

> On Wed, 17 May 2017 12:41:01 +0200
> Marc Gonzalez <marc_gonzalez@sigmadesigns.com> wrote:
> 
>> On 16/05/2017 18:27, Boris Brezillon wrote:
>>
>>> Drivers setting NAND_ECC_CUSTOM_PAGE_ACCESS are supposed to handle the
>>> full read/write page sequence, and waiting for a page to actually be
>>> programmed is part of this write-page sequence.
>>> This is also what is done in ->write_oob_xxx() hooks, so let's do that in  
>>> ->write_page_xxx() as well to make it consistent.  
>>>
>>> Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
>>> ---
>>>  drivers/mtd/nand/atmel/nand-controller.c |  6 +++++-
>>>  drivers/mtd/nand/nand_base.c             | 10 ++++++----
>>>  drivers/mtd/nand/nand_micron.c           | 10 ++++++++--
>>>  drivers/mtd/nand/tango_nand.c            | 13 ++++++++++++-
>>>  4 files changed, 31 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
>>> index 8dafd2a54e11..08ff98c47e1f 100644
>>> --- a/drivers/mtd/nand/nand_base.c
>>> +++ b/drivers/mtd/nand/nand_base.c
>>> @@ -2574,11 +2574,13 @@ static int nand_write_page(struct mtd_info *mtd, struct nand_chip *chip,
>>>  	if (status < 0)
>>>  		return status;
>>>  
>>> -	if (nand_standard_page_accessors(&chip->ecc))
>>> +	if (nand_standard_page_accessors(&chip->ecc)) {
>>>  		chip->cmdfunc(mtd, NAND_CMD_PAGEPROG, -1, -1);
>>> -	status = chip->waitfunc(mtd, chip);
>>> -	if (status & NAND_STATUS_FAIL)
>>> -		return -EIO;
>>> +
>>> +		status = chip->waitfunc(mtd, chip);
>>> +		if (status & NAND_STATUS_FAIL)
>>> +			return -EIO;
>>> +	}  
>>
>> AFAIU, the wait operation used to be unconditional;
>> but it is now skipped for CUSTOM accessors. OK.
> 
> Yep.
> 
>>
>>
>>> diff --git a/drivers/mtd/nand/tango_nand.c b/drivers/mtd/nand/tango_nand.c
>>> index a2150b15d4c1..8498fa36e533 100644
>>> --- a/drivers/mtd/nand/tango_nand.c
>>> +++ b/drivers/mtd/nand/tango_nand.c
>>> @@ -295,7 +295,7 @@ static int tango_write_page(struct mtd_info *mtd, struct nand_chip *chip,
>>>  			    const u8 *buf, int oob_required, int page)
>>>  {
>>>  	struct tango_nfc *nfc = to_tango_nfc(chip->controller);
>>> -	int err, len = mtd->writesize;
>>> +	int err, status, len = mtd->writesize;
>>>  
>>>  	/* Calling tango_write_oob() would send PAGEPROG twice */
>>>  	if (oob_required)
>>> @@ -306,6 +306,10 @@ static int tango_write_page(struct mtd_info *mtd, struct nand_chip *chip,
>>>  	if (err)
>>>  		return err;
>>>  
>>> +	status = chip->waitfunc(mtd, chip);
>>> +	if (status & NAND_STATUS_FAIL)
>>> +		return -EIO;
>>> +
>>>  	return 0;
>>>  }  
>>
>> When I introduced the custom_accessors flag, I missed the removal
>> of this wait operation. The tango NFC is supposed to take care of
>> everything, from start to finish.
>>
>> I applied the following patch:
>>
>> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
>> --- a/drivers/mtd/nand/nand_base.c
>> +++ b/drivers/mtd/nand/nand_base.c
>> @@ -2675,6 +2675,9 @@ static int nand_write_page(struct mtd_info *mtd, struct nand_chip *chip,
>>         if (status < 0)
>>                 return status;
>>  
>> +       if (!nand_standard_page_accessors(&chip->ecc))
>> +               return 0;
>> +
>>
>> And ran mtd_speedtest. No measurable change.
>> mtd_stresstest did not detect any issues.
> 
> Okay. I think I'll keep the patch as is to avoid introducing extra
> functional changes.
> You can then send a patch removing the extra chip->waitfunc() from
> tango_write_page() and explain why this is unneeded in your commit
> message. Would that work for you?

Could you ping me when the patch hits linux-next?
Then I'll send a fixup.

Regards.

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

* Re: [PATCH] mtd: nand: Wait for PAGEPROG to finish in drivers setting NAND_ECC_CUSTOM_PAGE_ACCESS
  2017-05-16 16:27 [PATCH] mtd: nand: Wait for PAGEPROG to finish in drivers setting NAND_ECC_CUSTOM_PAGE_ACCESS Boris Brezillon
  2017-05-17 10:41 ` Marc Gonzalez
@ 2017-05-29 18:51 ` Boris Brezillon
  1 sibling, 0 replies; 5+ messages in thread
From: Boris Brezillon @ 2017-05-29 18:51 UTC (permalink / raw)
  To: Wenyou Yang, Marc Gonzalez, Boris Brezillon, Richard Weinberger,
	linux-mtd
  Cc: Marek Vasut, Brian Norris, David Woodhouse, Cyrille Pitchen

On Tue, 16 May 2017 18:27:49 +0200
Boris Brezillon <boris.brezillon@free-electrons.com> wrote:

> Drivers setting NAND_ECC_CUSTOM_PAGE_ACCESS are supposed to handle the
> full read/write page sequence, and waiting for a page to actually be
> programmed is part of this write-page sequence.
> This is also what is done in ->write_oob_xxx() hooks, so let's do that in
> ->write_page_xxx() as well to make it consistent.  
> 

Applied to nand/next.

> Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
> ---
>  drivers/mtd/nand/atmel/nand-controller.c |  6 +++++-
>  drivers/mtd/nand/nand_base.c             | 10 ++++++----
>  drivers/mtd/nand/nand_micron.c           | 10 ++++++++--
>  drivers/mtd/nand/tango_nand.c            | 13 ++++++++++++-
>  4 files changed, 31 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/mtd/nand/atmel/nand-controller.c b/drivers/mtd/nand/atmel/nand-controller.c
> index 3b2446896147..e5122539d4f1 100644
> --- a/drivers/mtd/nand/atmel/nand-controller.c
> +++ b/drivers/mtd/nand/atmel/nand-controller.c
> @@ -912,7 +912,7 @@ static int atmel_hsmc_nand_pmecc_write_pg(struct nand_chip *chip,
>  	struct mtd_info *mtd = nand_to_mtd(chip);
>  	struct atmel_nand *nand = to_atmel_nand(chip);
>  	struct atmel_hsmc_nand_controller *nc;
> -	int ret;
> +	int ret, status;
>  
>  	nc = to_hsmc_nand_controller(chip->controller);
>  
> @@ -954,6 +954,10 @@ static int atmel_hsmc_nand_pmecc_write_pg(struct nand_chip *chip,
>  		dev_err(nc->base.dev, "Failed to program NAND page (err = %d)\n",
>  			ret);
>  
> +	status = chip->waitfunc(mtd, chip);
> +	if (status & NAND_STATUS_FAIL)
> +		return -EIO;
> +
>  	return ret;
>  }
>  
> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
> index 8dafd2a54e11..08ff98c47e1f 100644
> --- a/drivers/mtd/nand/nand_base.c
> +++ b/drivers/mtd/nand/nand_base.c
> @@ -2574,11 +2574,13 @@ static int nand_write_page(struct mtd_info *mtd, struct nand_chip *chip,
>  	if (status < 0)
>  		return status;
>  
> -	if (nand_standard_page_accessors(&chip->ecc))
> +	if (nand_standard_page_accessors(&chip->ecc)) {
>  		chip->cmdfunc(mtd, NAND_CMD_PAGEPROG, -1, -1);
> -	status = chip->waitfunc(mtd, chip);
> -	if (status & NAND_STATUS_FAIL)
> -		return -EIO;
> +
> +		status = chip->waitfunc(mtd, chip);
> +		if (status & NAND_STATUS_FAIL)
> +			return -EIO;
> +	}
>  
>  	return 0;
>  }
> diff --git a/drivers/mtd/nand/nand_micron.c b/drivers/mtd/nand/nand_micron.c
> index 0987f32dc6f7..f3c35fafd51a 100644
> --- a/drivers/mtd/nand/nand_micron.c
> +++ b/drivers/mtd/nand/nand_micron.c
> @@ -151,15 +151,18 @@ micron_nand_write_page_on_die_ecc(struct mtd_info *mtd, struct nand_chip *chip,
>  				  const uint8_t *buf, int oob_required,
>  				  int page)
>  {
> +	int status;
> +
>  	micron_nand_on_die_ecc_setup(chip, true);
>  
>  	chip->cmdfunc(mtd, NAND_CMD_SEQIN, 0x00, page);
>  	nand_write_page_raw(mtd, chip, buf, oob_required, page);
>  	chip->cmdfunc(mtd, NAND_CMD_PAGEPROG, -1, -1);
> +	status = chip->waitfunc(mtd, chip);
>  
>  	micron_nand_on_die_ecc_setup(chip, false);
>  
> -	return 0;
> +	return status & NAND_STATUS_FAIL ? -EIO : 0;
>  }
>  
>  static int
> @@ -180,11 +183,14 @@ micron_nand_write_page_raw_on_die_ecc(struct mtd_info *mtd,
>  				      const uint8_t *buf, int oob_required,
>  				      int page)
>  {
> +	int status;
> +
>  	chip->cmdfunc(mtd, NAND_CMD_SEQIN, 0x00, page);
>  	nand_write_page_raw(mtd, chip, buf, oob_required, page);
>  	chip->cmdfunc(mtd, NAND_CMD_PAGEPROG, -1, -1);
> +	status = chip->waitfunc(mtd, chip);
>  
> -	return 0;
> +	return status & NAND_STATUS_FAIL ? -EIO : 0;
>  }
>  
>  enum {
> diff --git a/drivers/mtd/nand/tango_nand.c b/drivers/mtd/nand/tango_nand.c
> index a2150b15d4c1..8498fa36e533 100644
> --- a/drivers/mtd/nand/tango_nand.c
> +++ b/drivers/mtd/nand/tango_nand.c
> @@ -295,7 +295,7 @@ static int tango_write_page(struct mtd_info *mtd, struct nand_chip *chip,
>  			    const u8 *buf, int oob_required, int page)
>  {
>  	struct tango_nfc *nfc = to_tango_nfc(chip->controller);
> -	int err, len = mtd->writesize;
> +	int err, status, len = mtd->writesize;
>  
>  	/* Calling tango_write_oob() would send PAGEPROG twice */
>  	if (oob_required)
> @@ -306,6 +306,10 @@ static int tango_write_page(struct mtd_info *mtd, struct nand_chip *chip,
>  	if (err)
>  		return err;
>  
> +	status = chip->waitfunc(mtd, chip);
> +	if (status & NAND_STATUS_FAIL)
> +		return -EIO;
> +
>  	return 0;
>  }
>  
> @@ -423,9 +427,16 @@ static int tango_read_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
>  static int tango_write_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
>  				const u8 *buf, int oob_required, int page)
>  {
> +	int status;
> +
>  	chip->cmdfunc(mtd, NAND_CMD_SEQIN, 0, page);
>  	raw_write(chip, buf, chip->oob_poi);
>  	chip->cmdfunc(mtd, NAND_CMD_PAGEPROG, -1, -1);
> +
> +	status = chip->waitfunc(mtd, chip);
> +	if (status & NAND_STATUS_FAIL)
> +		return -EIO;
> +
>  	return 0;
>  }
>  

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

end of thread, other threads:[~2017-05-29 18:51 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-05-16 16:27 [PATCH] mtd: nand: Wait for PAGEPROG to finish in drivers setting NAND_ECC_CUSTOM_PAGE_ACCESS Boris Brezillon
2017-05-17 10:41 ` Marc Gonzalez
2017-05-17 10:51   ` Boris Brezillon
2017-05-19 11:41     ` Marc Gonzalez
2017-05-29 18:51 ` Boris Brezillon

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.