diff mbox

[RFC] Special handling for NAND_CMD_PAGEPROG and NAND_CMD_READ0

Message ID 58236370.1050105@sigmadesigns.com
State Superseded
Headers show

Commit Message

Marc Gonzalez Nov. 9, 2016, 5:57 p.m. UTC 
Sample code to generate some discussion around having the framework
send I/O commands (for read_page and write_page) when it is dealing
with "high-level" NFCs that send the commands themselves.
---
 drivers/mtd/nand/nand_base.c  | 6 ++++--
 drivers/mtd/nand/tango_nand.c | 7 ++++++-
 include/linux/mtd/nand.h      | 6 ++++++
 3 files changed, 16 insertions(+), 3 deletions(-)

Comments

Marc Gonzalez Nov. 9, 2016, 6:02 p.m. UTC | #1 
On 09/11/2016 18:57, Marc Gonzalez wrote:

> Sample code to generate some discussion around having the framework
> send I/O commands (for read_page and write_page) when it is dealing
> with "high-level" NFCs that send the commands themselves.

According to mtd_speedtest, read speed goes from 12.0 to 13.8 MB/s
(15% improvement). Write speed is unchanged. (Is that expected?)

BEFORE:

[  399.571081] mtd_speedtest: MTD device: 1
[  399.575073] mtd_speedtest: MTD device size 536870912, eraseblock size 131072,
	page size 2048, count of eraseblocks 4096, pages per eraseblock 64, OOB size 64
[  399.590138] mtd_test: scanning for bad eraseblocks
[  399.598289] mtd_test: scanned 4096 eraseblocks, 0 are bad
[  402.804550] mtd_speedtest: testing eraseblock write speed
[  463.958290] mtd_speedtest: eraseblock write speed is 8574 KiB/s
[  463.964259] mtd_speedtest: testing eraseblock read speed
[  507.512545] mtd_speedtest: eraseblock read speed is 12040 KiB/s
[  510.746349] mtd_speedtest: testing page write speed
[  572.262041] mtd_speedtest: page write speed is 8523 KiB/s
[  572.267485] mtd_speedtest: testing page read speed
[  616.225641] mtd_speedtest: page read speed is 11928 KiB/s
[  619.457760] mtd_speedtest: testing 2 page write speed
[  680.780774] mtd_speedtest: 2 page write speed is 8550 KiB/s
[  680.786394] mtd_speedtest: testing 2 page read speed
[  724.532680] mtd_speedtest: 2 page read speed is 11986 KiB/s
[  724.538303] mtd_speedtest: Testing erase speed
[  727.768600] mtd_speedtest: erase speed is 162569 KiB/s
[  727.773777] mtd_speedtest: Testing 2x multi-block erase speed
[  729.428229] mtd_speedtest: 2x multi-block erase speed is 318135 KiB/s
[  729.434714] mtd_speedtest: Testing 4x multi-block erase speed
[  731.087529] mtd_speedtest: 4x multi-block erase speed is 318329 KiB/s
[  731.094015] mtd_speedtest: Testing 8x multi-block erase speed
[  732.746219] mtd_speedtest: 8x multi-block erase speed is 318522 KiB/s
[  732.752704] mtd_speedtest: Testing 16x multi-block erase speed
[  734.404625] mtd_speedtest: 16x multi-block erase speed is 318522 KiB/s
[  734.411197] mtd_speedtest: Testing 32x multi-block erase speed
[  736.062671] mtd_speedtest: 32x multi-block erase speed is 318716 KiB/s
[  736.069262] mtd_speedtest: Testing 64x multi-block erase speed
[  737.721396] mtd_speedtest: 64x multi-block erase speed is 318522 KiB/s
[  737.727966] mtd_speedtest: finished


AFTER:

[   43.104453] mtd_speedtest: MTD device: 1
[   43.108432] mtd_speedtest: MTD device size 536870912, eraseblock size 131072,
	page size 2048, count of eraseblocks 4096, pages per eraseblock 64, OOB size 64
[   43.123486] mtd_test: scanning for bad eraseblocks
[   43.131522] mtd_test: scanned 4096 eraseblocks, 0 are bad
[   44.786687] mtd_speedtest: testing eraseblock write speed
[  105.775235] mtd_speedtest: eraseblock write speed is 8597 KiB/s
[  105.781206] mtd_speedtest: testing eraseblock read speed
[  143.754987] mtd_speedtest: eraseblock read speed is 13808 KiB/s
[  146.985881] mtd_speedtest: testing page write speed
[  208.400273] mtd_speedtest: page write speed is 8537 KiB/s
[  208.405718] mtd_speedtest: testing page read speed
[  246.811007] mtd_speedtest: page read speed is 13653 KiB/s
[  250.041459] mtd_speedtest: testing 2 page write speed
[  311.306291] mtd_speedtest: 2 page write speed is 8558 KiB/s
[  311.311911] mtd_speedtest: testing 2 page read speed
[  349.519121] mtd_speedtest: 2 page read speed is 13724 KiB/s
[  349.524745] mtd_speedtest: Testing erase speed
[  352.754362] mtd_speedtest: erase speed is 162569 KiB/s
[  352.759539] mtd_speedtest: Testing 2x multi-block erase speed
[  354.413433] mtd_speedtest: 2x multi-block erase speed is 318135 KiB/s
[  354.419920] mtd_speedtest: Testing 4x multi-block erase speed
[  356.072314] mtd_speedtest: 4x multi-block erase speed is 318522 KiB/s
[  356.078808] mtd_speedtest: Testing 8x multi-block erase speed
[  357.730646] mtd_speedtest: 8x multi-block erase speed is 318522 KiB/s
[  357.737131] mtd_speedtest: Testing 16x multi-block erase speed
[  359.388793] mtd_speedtest: 16x multi-block erase speed is 318716 KiB/s
[  359.395365] mtd_speedtest: Testing 32x multi-block erase speed
[  361.046569] mtd_speedtest: 32x multi-block erase speed is 318716 KiB/s
[  361.053140] mtd_speedtest: Testing 64x multi-block erase speed
[  362.704484] mtd_speedtest: 64x multi-block erase speed is 318716 KiB/s
[  362.711055] mtd_speedtest: finished
Boris Brezillon Nov. 9, 2016, 6:49 p.m. UTC | #2 
On Wed, 9 Nov 2016 18:57:04 +0100
Marc Gonzalez <marc_gonzalez@sigmadesigns.com> wrote:

> Sample code to generate some discussion around having the framework
> send I/O commands (for read_page and write_page) when it is dealing
> with "high-level" NFCs that send the commands themselves.
> ---
>  drivers/mtd/nand/nand_base.c  | 6 ++++--
>  drivers/mtd/nand/tango_nand.c | 7 ++++++-
>  include/linux/mtd/nand.h      | 6 ++++++
>  3 files changed, 16 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
> index 50cdf37cb8e4..b4149101342c 100644
> --- a/drivers/mtd/nand/nand_base.c
> +++ b/drivers/mtd/nand/nand_base.c
> @@ -1970,7 +1970,8 @@ static int nand_do_read_ops(struct mtd_info *mtd, loff_t from,
>  						 __func__, buf);
>  
>  read_retry:
> -			chip->cmdfunc(mtd, NAND_CMD_READ0, 0x00, page);
> +			if (!(chip->options & NAND_FOO))
> +				chip->cmdfunc(mtd, NAND_CMD_READ0, 0x00, page);
>  

You'll have to patch the standard implementations provided by the core
(nand_read/write_page_xx()) to send these READ0/SEQIN/PAGEPROG commands
when the NAND_FOO flag is set.

>  			/*
>  			 * Now read the page into the buffer.  Absent an error,
> @@ -2681,7 +2682,8 @@ static int nand_write_page(struct mtd_info *mtd, struct nand_chip *chip,
>  
>  	if (!cached || !NAND_HAS_CACHEPROG(chip)) {
>  
> -		chip->cmdfunc(mtd, NAND_CMD_PAGEPROG, -1, -1);
> +		if (!(chip->options & NAND_FOO))
> +			chip->cmdfunc(mtd, NAND_CMD_PAGEPROG, -1, -1);

If you ask the core to not send NAND_CMD_PAGEPROG, it should also not
send the SEQIN command. 

>  		status = chip->waitfunc(mtd, chip);
>  		/*
>  		 * See if operation failed and additional status checks are
> diff --git a/drivers/mtd/nand/tango_nand.c b/drivers/mtd/nand/tango_nand.c
> index 74e39a92771c..d3679fdf2020 100644
> --- a/drivers/mtd/nand/tango_nand.c
> +++ b/drivers/mtd/nand/tango_nand.c
> @@ -401,13 +401,17 @@ static int raw_write(struct nand_chip *chip, const u8 *buf, const u8 *oob)
>  static int tango_read_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
>  		uint8_t *buf, int oob_required, int page)
>  {
> +	chip->cmdfunc(mtd, NAND_CMD_READ0, 0, page);
>  	return raw_read(chip, buf, chip->oob_poi);
>  }
>  
>  static int tango_write_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
>  		const uint8_t *buf, int oob_required, int page)
>  {
> -	return raw_write(chip, buf, chip->oob_poi);
> +	/* what about NAND_CMD_SEQIN ? */

You should send SEQIN as well, and patch the core to not send it when
NAND_FOO is set.

> +	raw_write(chip, buf, chip->oob_poi);
> +	chip->cmdfunc(mtd, NAND_CMD_PAGEPROG, -1, -1);
> +	return 0;
>  }
>  
>  static int tango_read_oob(struct mtd_info *mtd, struct nand_chip *chip, int page)
> @@ -527,6 +531,7 @@ static int chip_init(struct device *dev, struct device_node *np)
>  	chip->setup_data_interface = tango_set_timings;
>  	chip->options = NAND_USE_BOUNCE_BUFFER
>  		| NAND_NO_SUBPAGE_WRITE
> +		| NAND_FOO
>  		| NAND_WAIT_TCCS;
>  	chip->controller = &nfc->hw;
>  	tchip->base = nfc->pbus_base + (cs * 256);
> diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
> index 06d0c9d740f7..fd8968f3ccca 100644
> --- a/include/linux/mtd/nand.h
> +++ b/include/linux/mtd/nand.h
> @@ -220,6 +220,12 @@ enum nand_ecc_algo {
>   */
>  #define NAND_WAIT_TCCS		0x00200000
>  
> +/*
> + * Controller sends NAND_CMD_PAGEPROG (write_page) and NAND_CMD_READ0 (read_page)
> + * therefore the framework should not send these commands.
> + */
> +#define NAND_FOO		0x00400000
> +

Nice name :-).

>  /* Options set by nand scan */
>  /* Nand scan has allocated controller struct */
>  #define NAND_CONTROLLER_ALLOC	0x80000000
Mason Nov. 9, 2016, 8:18 p.m. UTC | #3 
On 09/11/2016 19:49, Boris Brezillon wrote:

> Marc Gonzalez <marc_gonzalez@sigmadesigns.com> wrote:
> 
>> Sample code to generate some discussion around having the framework
>> send I/O commands (for read_page and write_page) when it is dealing
>> with "high-level" NFCs that send the commands themselves.
>> ---
>>  drivers/mtd/nand/nand_base.c  | 6 ++++--
>>  drivers/mtd/nand/tango_nand.c | 7 ++++++-
>>  include/linux/mtd/nand.h      | 6 ++++++
>>  3 files changed, 16 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
>> index 50cdf37cb8e4..b4149101342c 100644
>> --- a/drivers/mtd/nand/nand_base.c
>> +++ b/drivers/mtd/nand/nand_base.c
>> @@ -1970,7 +1970,8 @@ static int nand_do_read_ops(struct mtd_info *mtd, loff_t from,
>>  						 __func__, buf);
>>  
>>  read_retry:
>> -			chip->cmdfunc(mtd, NAND_CMD_READ0, 0x00, page);
>> +			if (!(chip->options & NAND_FOO))
>> +				chip->cmdfunc(mtd, NAND_CMD_READ0, 0x00, page);
>>  
> 
> You'll have to patch the standard implementations provided by the core
> (nand_read/write_page_xx()) to send these READ0/SEQIN/PAGEPROG commands
> when the NAND_FOO flag is set.

Thanks, I had completely overlooked that part.


>> @@ -2681,7 +2682,8 @@ static int nand_write_page(struct mtd_info *mtd, struct nand_chip *chip,
>>  
>>  	if (!cached || !NAND_HAS_CACHEPROG(chip)) {
>>  
>> -		chip->cmdfunc(mtd, NAND_CMD_PAGEPROG, -1, -1);
>> +		if (!(chip->options & NAND_FOO))
>> +			chip->cmdfunc(mtd, NAND_CMD_PAGEPROG, -1, -1);
> 
> If you ask the core to not send NAND_CMD_PAGEPROG, it should also not
> send the SEQIN command.

OK.


>>  static int tango_write_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
>>  		const uint8_t *buf, int oob_required, int page)
>>  {
>> -	return raw_write(chip, buf, chip->oob_poi);
>> +	/* what about NAND_CMD_SEQIN ? */
> 
> You should send SEQIN as well, and patch the core to not send it when
> NAND_FOO is set.

OK.


>> +/*
>> + * Controller sends NAND_CMD_PAGEPROG (write_page) and NAND_CMD_READ0 (read_page)
>> + * therefore the framework should not send these commands.
>> + */
>> +#define NAND_FOO		0x00400000
>> +
> 
> Nice name :-).

I was worried there might be some bike-shedding :-)
Is this the right place to put it? And the right bit to use?
Would anyone care to suggest a good name?

I've thought of
NAND_DONT_SEND_RW_CMD
NAND_ZEALOUS_NFC
NAND_HIGH_LEVEL_NFC
NAND_HIGH_LEVEL_RW
NAND_COMPLEX_RW

Something else?

Regards.
Boris Brezillon Nov. 10, 2016, 3:29 p.m. UTC | #4 
On Thu, 10 Nov 2016 15:47:34 +0100
Marc Gonzalez <marc_gonzalez@sigmadesigns.com> wrote:

> Sample code to generate some discussion around having the framework
> *not* send I/O commands for read_page and write_page, when it is
> dealing with "high-level" NFCs that already send the commands
> themselves.
> ---
> diff from v1 to v2:
> - handle NAND_CMD_SEQIN as well
> - implement check_page_access_callbacks
> 
> If this RFC is an acceptable state, we can pick an appropriate
> name for the option, and I'll make a formal submission.
> ---
>  drivers/mtd/nand/nand_base.c  | 27 ++++++++++++++++++++++++---
>  drivers/mtd/nand/tango_nand.c |  7 ++++++-
>  include/linux/mtd/nand.h      |  8 ++++++++
>  3 files changed, 38 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
> index 50cdf37cb8e4..f42f79d8d0c4 100644
> --- a/drivers/mtd/nand/nand_base.c
> +++ b/drivers/mtd/nand/nand_base.c
> @@ -1970,7 +1970,8 @@ static int nand_do_read_ops(struct mtd_info *mtd, loff_t from,
>  						 __func__, buf);
>  
>  read_retry:
> -			chip->cmdfunc(mtd, NAND_CMD_READ0, 0x00, page);
> +			if (!(chip->options & NAND_FOO))
> +				chip->cmdfunc(mtd, NAND_CMD_READ0, 0x00, page);
>  
>  			/*
>  			 * Now read the page into the buffer.  Absent an error,
> @@ -2658,7 +2659,8 @@ static int nand_write_page(struct mtd_info *mtd, struct nand_chip *chip,
>  	else
>  		subpage = 0;
>  
> -	chip->cmdfunc(mtd, NAND_CMD_SEQIN, 0x00, page);
> +	if (!(chip->options & NAND_FOO))
> +		chip->cmdfunc(mtd, NAND_CMD_SEQIN, 0x00, page);
>  
>  	if (unlikely(raw))
>  		status = chip->ecc.write_page_raw(mtd, chip, buf,
> @@ -2681,7 +2683,8 @@ static int nand_write_page(struct mtd_info *mtd, struct nand_chip *chip,
>  
>  	if (!cached || !NAND_HAS_CACHEPROG(chip)) {
>  
> -		chip->cmdfunc(mtd, NAND_CMD_PAGEPROG, -1, -1);
> +		if (!(chip->options & NAND_FOO))
> +			chip->cmdfunc(mtd, NAND_CMD_PAGEPROG, -1, -1);
>  		status = chip->waitfunc(mtd, chip);
>  		/*
>  		 * See if operation failed and additional status checks are
> @@ -4539,6 +4542,21 @@ static bool nand_ecc_strength_good(struct mtd_info *mtd)
>  	return corr >= ds_corr && ecc->strength >= chip->ecc_strength_ds;
>  }
>  
> +static int check_page_access_callbacks(struct nand_chip *chip)

check_*ecc*_page_accessors() ?

> +{
> +	int err = 0;
> +	struct nand_ecc_ctrl *ecc = &chip->ecc;
> +
> +	if (chip->options & NAND_FOO) {
> +		err |= !ecc->read_page || !ecc->write_page;
> +		err |= !ecc->read_page_raw || !ecc->write_page_raw;
> +		err |= !ecc->read_subpage && NAND_HAS_SUBPAGE_READ(chip);
> +		err |= !ecc->write_subpage && NAND_HAS_SUBPAGE_WRITE(chip);
> +	}

Please return a real error code:

	if (err)
		return -EINVAL;

	return 0;

I think I'd prefer something more straightforward (but slightly longer):

	if (!(chip->options & NAND_FOO))
		return 0;

	/*
	 * NAND_FOO flag is set, make sure the NAND controller
	 * driver implements all the page accessors because
	 * default helpers are not suitable when the core does
	 * not send the READ0/PAGEPROG commands.
	 */
	if (!ecc->read_page || !ecc->write_page ||
	    !ecc->read_page_raw || !ecc->write_page_raw ||
	    (NAND_HAS_SUBPAGE_READ(chip) && !ecc->read_subpage) ||
	    (NAND_HAS_SUBPAGE_WRITE(chip) && !ecc->write_subpage))
		return -EINVAL;

	return 0;

> +	return err;
> +}
> +
>  /**
>   * nand_scan_tail - [NAND Interface] Scan for the NAND device
>   * @mtd: MTD device structure
> @@ -4559,6 +4577,9 @@ int nand_scan_tail(struct mtd_info *mtd)
>  		   !(chip->bbt_options & NAND_BBT_USE_FLASH)))
>  		return -EINVAL;
>  
> +	if (WARN_ON(check_page_access_callbacks(chip)))
> +		return -EINVAL;
> +
>  	if (!(chip->options & NAND_OWN_BUFFERS)) {
>  		nbuf = kzalloc(sizeof(*nbuf) + mtd->writesize
>  				+ mtd->oobsize * 3, GFP_KERNEL);
> diff --git a/drivers/mtd/nand/tango_nand.c b/drivers/mtd/nand/tango_nand.c
> index 74e39a92771c..2c9c0cac8f49 100644
> --- a/drivers/mtd/nand/tango_nand.c
> +++ b/drivers/mtd/nand/tango_nand.c
> @@ -401,13 +401,17 @@ static int raw_write(struct nand_chip *chip, const u8 *buf, const u8 *oob)
>  static int tango_read_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
>  		uint8_t *buf, int oob_required, int page)
>  {
> +	chip->cmdfunc(mtd, NAND_CMD_READ0, 0, page);
>  	return raw_read(chip, buf, chip->oob_poi);
>  }
>  
>  static int tango_write_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
>  		const uint8_t *buf, int oob_required, int page)
>  {
> -	return raw_write(chip, buf, chip->oob_poi);
> +	chip->cmdfunc(mtd, NAND_CMD_SEQIN, 0, page);
> +	raw_write(chip, buf, chip->oob_poi);
> +	chip->cmdfunc(mtd, NAND_CMD_PAGEPROG, -1, -1);
> +	return 0;
>  }
>  
>  static int tango_read_oob(struct mtd_info *mtd, struct nand_chip *chip, int page)
> @@ -527,6 +531,7 @@ static int chip_init(struct device *dev, struct device_node *np)
>  	chip->setup_data_interface = tango_set_timings;
>  	chip->options = NAND_USE_BOUNCE_BUFFER
>  		| NAND_NO_SUBPAGE_WRITE
> +		| NAND_FOO

This is an ECC engine related flag, as such it should be set in
ecc->options and defined next to NAND_ECC_GENERIC_ERASED_CHECK and
NAND_ECC_MAXIMIZE.

>  		| NAND_WAIT_TCCS;
>  	chip->controller = &nfc->hw;
>  	tchip->base = nfc->pbus_base + (cs * 256);
> diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
> index 06d0c9d740f7..6999196d04f3 100644
> --- a/include/linux/mtd/nand.h
> +++ b/include/linux/mtd/nand.h
> @@ -186,6 +186,7 @@ enum nand_ecc_algo {
>  /* Macros to identify the above */
>  #define NAND_HAS_CACHEPROG(chip) ((chip->options & NAND_CACHEPRG))
>  #define NAND_HAS_SUBPAGE_READ(chip) ((chip->options & NAND_SUBPAGE_READ))
> +#define NAND_HAS_SUBPAGE_WRITE(chip) (~chip->options & NAND_NO_SUBPAGE_WRITE)
>  
>  /* Non chip related options */
>  /* This option skips the bbt scan during initialization. */
> @@ -220,6 +221,13 @@ enum nand_ecc_algo {
>   */
>  #define NAND_WAIT_TCCS		0x00200000
>  
> +/*
> + * Controller sends NAND_CMD_PAGEPROG (write_page) and NAND_CMD_READ0 (read_page)
> + * therefore the framework should not send these commands.
> + * TODO: find a real name. Write a better description.
> + */
> +#define NAND_FOO		0x00400000
> +

Okay, we need to find a real name for this flag, otherwise, it looks
good to me.

Regarding the name, I have the following suggestions:

NAND_ECC_SKIP_READ_PROG_CMDS
NAND_ECC_DELEGATE_READ_PROG_CMDS
NAND_ECC_DELEGATE_PAGE_ACCESS
NAND_ECC_CUSTOM_PAGE_ACCESS


>  /* Options set by nand scan */
>  /* Nand scan has allocated controller struct */
>  #define NAND_CONTROLLER_ALLOC	0x80000000
diff mbox

Patch

diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index 50cdf37cb8e4..b4149101342c 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -1970,7 +1970,8 @@  static int nand_do_read_ops(struct mtd_info *mtd, loff_t from,
 						 __func__, buf);
 
 read_retry:
-			chip->cmdfunc(mtd, NAND_CMD_READ0, 0x00, page);
+			if (!(chip->options & NAND_FOO))
+				chip->cmdfunc(mtd, NAND_CMD_READ0, 0x00, page);
 
 			/*
 			 * Now read the page into the buffer.  Absent an error,
@@ -2681,7 +2682,8 @@  static int nand_write_page(struct mtd_info *mtd, struct nand_chip *chip,
 
 	if (!cached || !NAND_HAS_CACHEPROG(chip)) {
 
-		chip->cmdfunc(mtd, NAND_CMD_PAGEPROG, -1, -1);
+		if (!(chip->options & NAND_FOO))
+			chip->cmdfunc(mtd, NAND_CMD_PAGEPROG, -1, -1);
 		status = chip->waitfunc(mtd, chip);
 		/*
 		 * See if operation failed and additional status checks are
diff --git a/drivers/mtd/nand/tango_nand.c b/drivers/mtd/nand/tango_nand.c
index 74e39a92771c..d3679fdf2020 100644
--- a/drivers/mtd/nand/tango_nand.c
+++ b/drivers/mtd/nand/tango_nand.c
@@ -401,13 +401,17 @@  static int raw_write(struct nand_chip *chip, const u8 *buf, const u8 *oob)
 static int tango_read_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
 		uint8_t *buf, int oob_required, int page)
 {
+	chip->cmdfunc(mtd, NAND_CMD_READ0, 0, page);
 	return raw_read(chip, buf, chip->oob_poi);
 }
 
 static int tango_write_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
 		const uint8_t *buf, int oob_required, int page)
 {
-	return raw_write(chip, buf, chip->oob_poi);
+	/* what about NAND_CMD_SEQIN ? */
+	raw_write(chip, buf, chip->oob_poi);
+	chip->cmdfunc(mtd, NAND_CMD_PAGEPROG, -1, -1);
+	return 0;
 }
 
 static int tango_read_oob(struct mtd_info *mtd, struct nand_chip *chip, int page)
@@ -527,6 +531,7 @@  static int chip_init(struct device *dev, struct device_node *np)
 	chip->setup_data_interface = tango_set_timings;
 	chip->options = NAND_USE_BOUNCE_BUFFER
 		| NAND_NO_SUBPAGE_WRITE
+		| NAND_FOO
 		| NAND_WAIT_TCCS;
 	chip->controller = &nfc->hw;
 	tchip->base = nfc->pbus_base + (cs * 256);
diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
index 06d0c9d740f7..fd8968f3ccca 100644
--- a/include/linux/mtd/nand.h
+++ b/include/linux/mtd/nand.h
@@ -220,6 +220,12 @@  enum nand_ecc_algo {
  */
 #define NAND_WAIT_TCCS		0x00200000
 
+/*
+ * Controller sends NAND_CMD_PAGEPROG (write_page) and NAND_CMD_READ0 (read_page)
+ * therefore the framework should not send these commands.
+ */
+#define NAND_FOO		0x00400000
+
 /* Options set by nand scan */
 /* Nand scan has allocated controller struct */
 #define NAND_CONTROLLER_ALLOC	0x80000000