* [PATCH] mtd: rawnand: add default values for dynamic timings
@ 2018-05-03 12:16 Miquel Raynal
2018-05-03 13:07 ` Boris Brezillon
0 siblings, 1 reply; 2+ messages in thread
From: Miquel Raynal @ 2018-05-03 12:16 UTC (permalink / raw)
To: Boris Brezillon, Richard Weinberger, David Woodhouse,
Brian Norris, Marek Vasut
Cc: linux-mtd, 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
under certain circumstances: the chip is ONFI compliant and
->setup_data_interface() is populated.
The former makes the use of these timings unreliable if the driver uses
one of these four values with a non-ONFI chip, while the latter is a
problem for drivers not populating ->setup_data_interface() hook but
using these delays nonetheless in the code.
Fix this situation by taking the highest possible value for each
timing (stored as unsigned 16-bit entries in the parameter page).
This makes tBERS, tPROG and tR maximum times being ~65ms while typical
values are at most a few milliseconds. As these are timeouts, it is not
impacting at all on the performances in nominal use.
tCCS minimum time (delay to wait after a change column) becomes ~65us
while typical values are a few hundred nanoseconds. This might have an
impact depending on the driver's implementation.
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
drivers/mtd/nand/raw/nand_timings.c | 26 ++++++++++++++++++++++++++
1 file changed, 26 insertions(+)
diff --git a/drivers/mtd/nand/raw/nand_timings.c b/drivers/mtd/nand/raw/nand_timings.c
index 7c4e4a371bbc..848446f8004c 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 */
{
@@ -24,6 +26,8 @@ static const struct nand_data_interface onfi_sdr_timings[] = {
.tALH_min = 20000,
.tALS_min = 50000,
.tAR_min = 25000,
+ .tBERS_max = 1000000ULL * ONFI_DYN_TIMING_MAX,
+ .tCCS_min = 1000UL * ONFI_DYN_TIMING_MAX,
.tCEA_max = 100000,
.tCEH_min = 20000,
.tCH_min = 20000,
@@ -38,6 +42,8 @@ static const struct nand_data_interface onfi_sdr_timings[] = {
.tFEAT_max = 1000000,
.tIR_min = 10000,
.tITC_max = 1000000,
+ .tPROG_max = 1000000ULL * ONFI_DYN_TIMING_MAX,
+ .tR_max = 1000000ULL * ONFI_DYN_TIMING_MAX,
.tRC_min = 100000,
.tREA_max = 40000,
.tREH_min = 30000,
@@ -66,6 +72,8 @@ static const struct nand_data_interface onfi_sdr_timings[] = {
.tALH_min = 10000,
.tALS_min = 25000,
.tAR_min = 10000,
+ .tBERS_max = 1000000ULL * ONFI_DYN_TIMING_MAX,
+ .tCCS_min = 1000UL * ONFI_DYN_TIMING_MAX,
.tCEA_max = 45000,
.tCEH_min = 20000,
.tCH_min = 10000,
@@ -80,6 +88,8 @@ static const struct nand_data_interface onfi_sdr_timings[] = {
.tFEAT_max = 1000000,
.tIR_min = 0,
.tITC_max = 1000000,
+ .tPROG_max = 1000000ULL * ONFI_DYN_TIMING_MAX,
+ .tR_max = 1000000ULL * ONFI_DYN_TIMING_MAX,
.tRC_min = 50000,
.tREA_max = 30000,
.tREH_min = 15000,
@@ -108,6 +118,8 @@ static const struct nand_data_interface onfi_sdr_timings[] = {
.tALH_min = 10000,
.tALS_min = 15000,
.tAR_min = 10000,
+ .tBERS_max = 1000000ULL * ONFI_DYN_TIMING_MAX,
+ .tCCS_min = 1000UL * ONFI_DYN_TIMING_MAX,
.tCEA_max = 30000,
.tCEH_min = 20000,
.tCH_min = 10000,
@@ -122,6 +134,8 @@ static const struct nand_data_interface onfi_sdr_timings[] = {
.tFEAT_max = 1000000,
.tIR_min = 0,
.tITC_max = 1000000,
+ .tPROG_max = 1000000ULL * ONFI_DYN_TIMING_MAX,
+ .tR_max = 1000000ULL * ONFI_DYN_TIMING_MAX,
.tRC_min = 35000,
.tREA_max = 25000,
.tREH_min = 15000,
@@ -150,6 +164,8 @@ static const struct nand_data_interface onfi_sdr_timings[] = {
.tALH_min = 5000,
.tALS_min = 10000,
.tAR_min = 10000,
+ .tBERS_max = 1000000ULL * ONFI_DYN_TIMING_MAX,
+ .tCCS_min = 1000UL * ONFI_DYN_TIMING_MAX,
.tCEA_max = 25000,
.tCEH_min = 20000,
.tCH_min = 5000,
@@ -164,6 +180,8 @@ static const struct nand_data_interface onfi_sdr_timings[] = {
.tFEAT_max = 1000000,
.tIR_min = 0,
.tITC_max = 1000000,
+ .tPROG_max = 1000000ULL * ONFI_DYN_TIMING_MAX,
+ .tR_max = 1000000ULL * ONFI_DYN_TIMING_MAX,
.tRC_min = 30000,
.tREA_max = 20000,
.tREH_min = 10000,
@@ -192,6 +210,8 @@ static const struct nand_data_interface onfi_sdr_timings[] = {
.tALH_min = 5000,
.tALS_min = 10000,
.tAR_min = 10000,
+ .tBERS_max = 1000000ULL * ONFI_DYN_TIMING_MAX,
+ .tCCS_min = 1000UL * ONFI_DYN_TIMING_MAX,
.tCEA_max = 25000,
.tCEH_min = 20000,
.tCH_min = 5000,
@@ -206,6 +226,8 @@ static const struct nand_data_interface onfi_sdr_timings[] = {
.tFEAT_max = 1000000,
.tIR_min = 0,
.tITC_max = 1000000,
+ .tPROG_max = 1000000ULL * ONFI_DYN_TIMING_MAX,
+ .tR_max = 1000000ULL * ONFI_DYN_TIMING_MAX,
.tRC_min = 25000,
.tREA_max = 20000,
.tREH_min = 10000,
@@ -234,6 +256,8 @@ static const struct nand_data_interface onfi_sdr_timings[] = {
.tALH_min = 5000,
.tALS_min = 10000,
.tAR_min = 10000,
+ .tBERS_max = 1000000ULL * ONFI_DYN_TIMING_MAX,
+ .tCCS_min = 1000UL * ONFI_DYN_TIMING_MAX,
.tCEA_max = 25000,
.tCEH_min = 20000,
.tCH_min = 5000,
@@ -248,6 +272,8 @@ static const struct nand_data_interface onfi_sdr_timings[] = {
.tFEAT_max = 1000000,
.tIR_min = 0,
.tITC_max = 1000000,
+ .tPROG_max = 1000000ULL * ONFI_DYN_TIMING_MAX,
+ .tR_max = 1000000ULL * ONFI_DYN_TIMING_MAX,
.tRC_min = 20000,
.tREA_max = 16000,
.tREH_min = 7000,
--
2.14.1
^ permalink raw reply related [flat|nested] 2+ messages in thread* Re: [PATCH] mtd: rawnand: add default values for dynamic timings
2018-05-03 12:16 [PATCH] mtd: rawnand: add default values for dynamic timings Miquel Raynal
@ 2018-05-03 13:07 ` Boris Brezillon
0 siblings, 0 replies; 2+ messages in thread
From: Boris Brezillon @ 2018-05-03 13:07 UTC (permalink / raw)
To: Miquel Raynal
Cc: Richard Weinberger, David Woodhouse, Brian Norris, Marek Vasut,
linux-mtd
Hi Miquel,
On Thu, 3 May 2018 14:16:20 +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
> under certain circumstances: the chip is ONFI compliant and
> ->setup_data_interface() is populated.
The later is not true, is it? It seems that we use timing mode 0 even
for setup where ->setup_data_interface() is not implemented [1].
>
> The former makes the use of these timings unreliable if the driver uses
> one of these four values with a non-ONFI chip, while the latter is a
> problem for drivers not populating ->setup_data_interface() hook but
> using these delays nonetheless in the code.
>
> Fix this situation by taking the highest possible value for each
> timing (stored as unsigned 16-bit entries in the parameter page).
>
> This makes tBERS, tPROG and tR maximum times being ~65ms while typical
> values are at most a few milliseconds. As these are timeouts, it is not
> impacting at all on the performances in nominal use.
>
> tCCS minimum time (delay to wait after a change column) becomes ~65us
> while typical values are a few hundred nanoseconds. This might have an
> impact depending on the driver's implementation.
I think we should do things differently for tCCS and tR. The ONFI spec
says:
"
For execution of the first Read Parameter Page command, prior to
complete initialization, a tR value of 200 microseconds and tCCS value
of 500 ns shall be used. For page reads, including execution of
additional Read Parameter Page commands after initialization is
complete, the value for tR and tCCS contained in the parameter page
shall be used.
"
So tR = 200us and tCCS = 500ns sound like better default values for non
ONFI chips.
>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
> drivers/mtd/nand/raw/nand_timings.c | 26 ++++++++++++++++++++++++++
> 1 file changed, 26 insertions(+)
>
> diff --git a/drivers/mtd/nand/raw/nand_timings.c b/drivers/mtd/nand/raw/nand_timings.c
> index 7c4e4a371bbc..848446f8004c 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 */
> {
> @@ -24,6 +26,8 @@ static const struct nand_data_interface onfi_sdr_timings[] = {
> .tALH_min = 20000,
> .tALS_min = 50000,
> .tAR_min = 25000,
> + .tBERS_max = 1000000ULL * ONFI_DYN_TIMING_MAX,
> + .tCCS_min = 1000UL * ONFI_DYN_TIMING_MAX,
> .tCEA_max = 100000,
> .tCEH_min = 20000,
> .tCH_min = 20000,
> @@ -38,6 +42,8 @@ static const struct nand_data_interface onfi_sdr_timings[] = {
> .tFEAT_max = 1000000,
> .tIR_min = 10000,
> .tITC_max = 1000000,
> + .tPROG_max = 1000000ULL * ONFI_DYN_TIMING_MAX,
> + .tR_max = 1000000ULL * ONFI_DYN_TIMING_MAX,
> .tRC_min = 100000,
> .tREA_max = 40000,
> .tREH_min = 30000,
> @@ -66,6 +72,8 @@ static const struct nand_data_interface onfi_sdr_timings[] = {
> .tALH_min = 10000,
> .tALS_min = 25000,
> .tAR_min = 10000,
> + .tBERS_max = 1000000ULL * ONFI_DYN_TIMING_MAX,
> + .tCCS_min = 1000UL * ONFI_DYN_TIMING_MAX,
> .tCEA_max = 45000,
> .tCEH_min = 20000,
> .tCH_min = 10000,
> @@ -80,6 +88,8 @@ static const struct nand_data_interface onfi_sdr_timings[] = {
> .tFEAT_max = 1000000,
> .tIR_min = 0,
> .tITC_max = 1000000,
> + .tPROG_max = 1000000ULL * ONFI_DYN_TIMING_MAX,
> + .tR_max = 1000000ULL * ONFI_DYN_TIMING_MAX,
> .tRC_min = 50000,
> .tREA_max = 30000,
> .tREH_min = 15000,
Instead of adding those definitions for each mode, maybe you can add
an 'else' statement in the code to fill those fields with the
default/max values when the NAND is not ONFI.
Regards,
Boris
[1]https://elixir.bootlin.com/linux/v4.17-rc3/source/drivers/mtd/nand/raw/nand_base.c#L5867
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2018-05-03 13:07 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-03 12:16 [PATCH] mtd: rawnand: add default values for dynamic timings Miquel Raynal
2018-05-03 13:07 ` 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.