[PATCH v2] mtd: rawnand: add default values for dynamic timings

[PATCH v2] mtd: rawnand: add default values for dynamic timings

[Miquel Raynal]

Some timings like tBERS (block erase time), tCCs (change column setup
time), tPROG (page program time) and tR (page read time) are derived
from the ONFI parameter page. They are set in the SDR interface only
if the chip is ONFI compliant.

It makes these timings unreliable and prevent the driver to use one of
these four values with a non-ONFI chip.

Fix this situation by taking the highest possible value (or a default
one) value for each missing timing (stored as unsigned 16-bit entries in
the parameter page).

This makes tBERS and tPROG being ~65ms while typical values are at most
a few milliseconds. As these are timeouts, it is not impacting at all
the performances in nominal use.

tR maximum time and tCCS minimum time (delay to wait after a change
column) are set, according to the ONFI specification, to default 'slow'
values; respectively 200us and 500ns.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---

Changes since v1
================
* Remove in the commit message a wrong assertion on which the problem
  depicted above happened when no ->setup_data_inferface() hook was set.
* The missing timings were set, for each timing mode, in the
  onfi_sdr_timings structure, not it is done in an else block in
  onfi_fill_data_interface() once.
* Changed tR and tCCS to be 'timing mode 0' values by default (should
  limit the impact of such a change in throughputs).


 drivers/mtd/nand/raw/nand_timings.c | 20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/nand/raw/nand_timings.c b/drivers/mtd/nand/raw/nand_timings.c
index 7c4e4a371bbc..9bb599106a31 100644
--- a/drivers/mtd/nand/raw/nand_timings.c
+++ b/drivers/mtd/nand/raw/nand_timings.c
@@ -13,6 +13,8 @@
 #include <linux/export.h>
 #include <linux/mtd/rawnand.h>
 
+#define ONFI_DYN_TIMING_MAX U16_MAX
+
 static const struct nand_data_interface onfi_sdr_timings[] = {
 	/* Mode 0 */
 	{
@@ -303,7 +305,7 @@ int onfi_fill_data_interface(struct nand_chip *chip,
 
 	/*
 	 * Initialize timings that cannot be deduced from timing mode:
-	 * tR, tPROG, tCCS, ...
+	 * tPROG, tBERS, tR and tCCS.
 	 * These information are part of the ONFI parameter page.
 	 */
 	if (chip->parameters.onfi.version) {
@@ -317,6 +319,22 @@ int onfi_fill_data_interface(struct nand_chip *chip,
 
 		/* nanoseconds -> picoseconds */
 		timings->tCCS_min = 1000UL * params->onfi.tCCS;
+	} else {
+		struct nand_sdr_timings *timings = &iface->timings.sdr;
+		/*
+		 * For non-ONFI chips we use the highest possible value for
+		 * tPROG and tBERS. tR and tCCS will take the default values
+		 * precised in the ONFI specification for timing mode 0,
+		 * respectively 200us and 500ns.
+		 */
+
+		/* microseconds -> picoseconds */
+		timings->tPROG_max = 1000000ULL * ONFI_DYN_TIMING_MAX;
+		timings->tBERS_max = 1000000ULL * ONFI_DYN_TIMING_MAX;
+		timings->tR_max = 1000000ULL * 200000000ULL;
+
+		/* nanoseconds -> picoseconds */
+		timings->tCCS_min = 1000UL * 500000;
 	}
 
 	return 0;
-- 
2.14.1

Re: [PATCH v2] mtd: rawnand: add default values for dynamic timings

[Boris Brezillon]

On Sat, 14 Jul 2018 12:23:54 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> Some timings like tBERS (block erase time), tCCs (change column setup
> time), tPROG (page program time) and tR (page read time) are derived
> from the ONFI parameter page. They are set in the SDR interface only
> if the chip is ONFI compliant.
> 
> It makes these timings unreliable and prevent the driver to use one of
> these four values with a non-ONFI chip.
> 
> Fix this situation by taking the highest possible value (or a default
> one) value for each missing timing (stored as unsigned 16-bit entries in
> the parameter page).
> 
> This makes tBERS and tPROG being ~65ms while typical values are at most
> a few milliseconds. As these are timeouts, it is not impacting at all
> the performances in nominal use.
> 
> tR maximum time and tCCS minimum time (delay to wait after a change
> column) are set, according to the ONFI specification, to default 'slow'
> values; respectively 200us and 500ns.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>

Reviewed-by: Boris Brezillon <boris.brezillon@bootlin.com>

> ---
> 
> Changes since v1
> ================
> * Remove in the commit message a wrong assertion on which the problem
>   depicted above happened when no ->setup_data_inferface() hook was set.
> * The missing timings were set, for each timing mode, in the
>   onfi_sdr_timings structure, not it is done in an else block in
>   onfi_fill_data_interface() once.
> * Changed tR and tCCS to be 'timing mode 0' values by default (should
>   limit the impact of such a change in throughputs).
> 
> 
>  drivers/mtd/nand/raw/nand_timings.c | 20 +++++++++++++++++++-
>  1 file changed, 19 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/nand/raw/nand_timings.c b/drivers/mtd/nand/raw/nand_timings.c
> index 7c4e4a371bbc..9bb599106a31 100644
> --- a/drivers/mtd/nand/raw/nand_timings.c
> +++ b/drivers/mtd/nand/raw/nand_timings.c
> @@ -13,6 +13,8 @@
>  #include <linux/export.h>
>  #include <linux/mtd/rawnand.h>
>  
> +#define ONFI_DYN_TIMING_MAX U16_MAX
> +
>  static const struct nand_data_interface onfi_sdr_timings[] = {
>  	/* Mode 0 */
>  	{
> @@ -303,7 +305,7 @@ int onfi_fill_data_interface(struct nand_chip *chip,
>  
>  	/*
>  	 * Initialize timings that cannot be deduced from timing mode:
> -	 * tR, tPROG, tCCS, ...
> +	 * tPROG, tBERS, tR and tCCS.
>  	 * These information are part of the ONFI parameter page.
>  	 */
>  	if (chip->parameters.onfi.version) {
> @@ -317,6 +319,22 @@ int onfi_fill_data_interface(struct nand_chip *chip,
>  
>  		/* nanoseconds -> picoseconds */
>  		timings->tCCS_min = 1000UL * params->onfi.tCCS;
> +	} else {
> +		struct nand_sdr_timings *timings = &iface->timings.sdr;
> +		/*
> +		 * For non-ONFI chips we use the highest possible value for
> +		 * tPROG and tBERS. tR and tCCS will take the default values
> +		 * precised in the ONFI specification for timing mode 0,
> +		 * respectively 200us and 500ns.
> +		 */
> +
> +		/* microseconds -> picoseconds */
> +		timings->tPROG_max = 1000000ULL * ONFI_DYN_TIMING_MAX;
> +		timings->tBERS_max = 1000000ULL * ONFI_DYN_TIMING_MAX;
> +		timings->tR_max = 1000000ULL * 200000000ULL;
> +
> +		/* nanoseconds -> picoseconds */
> +		timings->tCCS_min = 1000UL * 500000;
>  	}
>  
>  	return 0;

Re: [PATCH v2] mtd: rawnand: add default values for dynamic timings

[Miquel Raynal]

> On Sat, 14 Jul 2018 12:23:54 +0200
> Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> 
> > Some timings like tBERS (block erase time), tCCs (change column setup
> > time), tPROG (page program time) and tR (page read time) are derived
> > from the ONFI parameter page. They are set in the SDR interface only
> > if the chip is ONFI compliant.
> > 
> > It makes these timings unreliable and prevent the driver to use one of
> > these four values with a non-ONFI chip.
> > 
> > Fix this situation by taking the highest possible value (or a default
> > one) value for each missing timing (stored as unsigned 16-bit entries in
> > the parameter page).
> > 
> > This makes tBERS and tPROG being ~65ms while typical values are at most
> > a few milliseconds. As these are timeouts, it is not impacting at all
> > the performances in nominal use.
> > 
> > tR maximum time and tCCS minimum time (delay to wait after a change
> > column) are set, according to the ONFI specification, to default 'slow'
> > values; respectively 200us and 500ns.
> > 
> > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>  
> 
> Reviewed-by: Boris Brezillon <boris.brezillon@bootlin.com>
> 

Applied to nand/next.

Miquèl