* [PATCH 1/2] mtd: nand: Add a few more timings to nand_sdr_timings
2016-10-01 8:24 [PATCH 0/2] mtd: nand: Enforce tCCS wait time after a column change Boris Brezillon
@ 2016-10-01 8:24 ` Boris Brezillon
2016-10-01 8:24 ` [PATCH 2/2] mtd: nand: Wait tCCS after a column change Boris Brezillon
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Boris Brezillon @ 2016-10-01 8:24 UTC (permalink / raw)
To: David Woodhouse, Brian Norris, linux-mtd, Boris Brezillon,
Richard Weinberger
Cc: Marc Gonzalez
Add the tR_max, tBERS_max, tPROG_max and tCCS_min timings to the
nand_sdr_timings struct.
Assign default/safe values for the statically defined timings, and
extract them from the ONFI parameter table if the NAND is ONFI
compliant.
Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
---
drivers/mtd/nand/nand_timings.c | 26 +++++++++++++++++++++++++-
include/linux/mtd/nand.h | 8 ++++++++
2 files changed, 33 insertions(+), 1 deletion(-)
diff --git a/drivers/mtd/nand/nand_timings.c b/drivers/mtd/nand/nand_timings.c
index 13a587407be3..f06312df3669 100644
--- a/drivers/mtd/nand/nand_timings.c
+++ b/drivers/mtd/nand/nand_timings.c
@@ -18,6 +18,8 @@ static const struct nand_data_interface onfi_sdr_timings[] = {
{
.type = NAND_SDR_IFACE,
.timings.sdr = {
+ .tCCS_min = 500000,
+ .tR_max = 200000000,
.tADL_min = 400000,
.tALH_min = 20000,
.tALS_min = 50000,
@@ -58,6 +60,8 @@ static const struct nand_data_interface onfi_sdr_timings[] = {
{
.type = NAND_SDR_IFACE,
.timings.sdr = {
+ .tCCS_min = 500000,
+ .tR_max = 200000000,
.tADL_min = 400000,
.tALH_min = 10000,
.tALS_min = 25000,
@@ -98,6 +102,8 @@ static const struct nand_data_interface onfi_sdr_timings[] = {
{
.type = NAND_SDR_IFACE,
.timings.sdr = {
+ .tCCS_min = 500000,
+ .tR_max = 200000000,
.tADL_min = 400000,
.tALH_min = 10000,
.tALS_min = 15000,
@@ -138,6 +144,8 @@ static const struct nand_data_interface onfi_sdr_timings[] = {
{
.type = NAND_SDR_IFACE,
.timings.sdr = {
+ .tCCS_min = 500000,
+ .tR_max = 200000000,
.tADL_min = 400000,
.tALH_min = 5000,
.tALS_min = 10000,
@@ -178,6 +186,8 @@ static const struct nand_data_interface onfi_sdr_timings[] = {
{
.type = NAND_SDR_IFACE,
.timings.sdr = {
+ .tCCS_min = 500000,
+ .tR_max = 200000000,
.tADL_min = 400000,
.tALH_min = 5000,
.tALS_min = 10000,
@@ -218,6 +228,8 @@ static const struct nand_data_interface onfi_sdr_timings[] = {
{
.type = NAND_SDR_IFACE,
.timings.sdr = {
+ .tCCS_min = 500000,
+ .tR_max = 200000000,
.tADL_min = 400000,
.tALH_min = 5000,
.tALS_min = 10000,
@@ -290,10 +302,22 @@ int onfi_init_data_interface(struct nand_chip *chip,
*iface = onfi_sdr_timings[timing_mode];
/*
- * TODO: initialize timings that cannot be deduced from timing mode:
+ * Initialize timings that cannot be deduced from timing mode:
* tR, tPROG, tCCS, ...
* These information are part of the ONFI parameter page.
*/
+ if (chip->onfi_version) {
+ struct nand_onfi_params *params = &chip->onfi_params;
+ struct nand_sdr_timings *timings = &iface->timings.sdr;
+
+ /* microseconds -> picoseconds */
+ timings->tPROG_max = 1000000UL * le16_to_cpu(params->t_prog);
+ timings->tBERS_max = 1000000UL * le16_to_cpu(params->t_bers);
+ timings->tR_max = 1000000UL * le16_to_cpu(params->t_r);
+
+ /* nanoseconds -> picoseconds */
+ timings->tCCS_min = 1000UL * le16_to_cpu(params->t_ccs);
+ }
return 0;
}
diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
index c5d3d5024fc8..6fe83bce83a6 100644
--- a/include/linux/mtd/nand.h
+++ b/include/linux/mtd/nand.h
@@ -584,6 +584,10 @@ struct nand_buffers {
*
* All these timings are expressed in picoseconds.
*
+ * @tBERS_max: Block erase time
+ * @tCCS_min: Change column setup time
+ * @tPROG_max: Page program time
+ * @tR_max: Page read time
* @tALH_min: ALE hold time
* @tADL_min: ALE to data loading time
* @tALS_min: ALE setup time
@@ -621,6 +625,10 @@ struct nand_buffers {
* @tWW_min: WP# transition to WE# low
*/
struct nand_sdr_timings {
+ u32 tBERS_max;
+ u32 tCCS_min;
+ u32 tPROG_max;
+ u32 tR_max;
u32 tALH_min;
u32 tADL_min;
u32 tALS_min;
--
2.7.4
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH 2/2] mtd: nand: Wait tCCS after a column change
2016-10-01 8:24 [PATCH 0/2] mtd: nand: Enforce tCCS wait time after a column change Boris Brezillon
2016-10-01 8:24 ` [PATCH 1/2] mtd: nand: Add a few more timings to nand_sdr_timings Boris Brezillon
@ 2016-10-01 8:24 ` Boris Brezillon
2016-10-25 12:31 ` [PATCH 0/2] mtd: nand: Enforce tCCS wait time " Marc Gonzalez
2016-10-28 8:54 ` Boris Brezillon
3 siblings, 0 replies; 5+ messages in thread
From: Boris Brezillon @ 2016-10-01 8:24 UTC (permalink / raw)
To: David Woodhouse, Brian Norris, linux-mtd, Boris Brezillon,
Richard Weinberger
Cc: Marc Gonzalez
Drivers implementing ->cmd_ctrl() and relying on the default ->cmdfunc()
implementation usually don't wait tCCS when a column change (RNDIN or
RNDOUT) is requested.
Add an option flag to ask the core to do so (note that we keep this as
an opt-in to avoid breaking existing implementations), and make use of
the ->data_interface information is available (otherwise, wait 500ns).
Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
---
drivers/mtd/nand/nand_base.c | 26 +++++++++++++++++++++++++-
include/linux/mtd/nand.h | 10 ++++++++++
2 files changed, 35 insertions(+), 1 deletion(-)
diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index 7a00245ebada..3b0e6d861e2e 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -709,6 +709,25 @@ static void nand_command(struct mtd_info *mtd, unsigned int command,
nand_wait_ready(mtd);
}
+static void nand_ccs_delay(struct nand_chip *chip)
+{
+ /*
+ * The controller already takes care of waiting for tCCS when the RNDIN
+ * or RNDOUT command is sent, return directly.
+ */
+ if (!(chip->options & NAND_WAIT_TCCS))
+ return;
+
+ /*
+ * Wait tCCS_min if it is correctly defined, otherwise wait 500ns
+ * (which should be safe for all NANDs).
+ */
+ if (chip->data_interface && chip->data_interface->timings.sdr.tCCS_min)
+ ndelay(chip->data_interface->timings.sdr.tCCS_min / 1000);
+ else
+ ndelay(500);
+}
+
/**
* nand_command_lp - [DEFAULT] Send command to NAND large page device
* @mtd: MTD device structure
@@ -773,10 +792,13 @@ static void nand_command_lp(struct mtd_info *mtd, unsigned int command,
case NAND_CMD_ERASE1:
case NAND_CMD_ERASE2:
case NAND_CMD_SEQIN:
- case NAND_CMD_RNDIN:
case NAND_CMD_STATUS:
return;
+ case NAND_CMD_RNDIN:
+ nand_ccs_delay(chip);
+ return;
+
case NAND_CMD_RESET:
if (chip->dev_ready)
break;
@@ -795,6 +817,8 @@ static void nand_command_lp(struct mtd_info *mtd, unsigned int command,
NAND_NCE | NAND_CLE | NAND_CTRL_CHANGE);
chip->cmd_ctrl(mtd, NAND_CMD_NONE,
NAND_NCE | NAND_CTRL_CHANGE);
+
+ nand_ccs_delay(chip);
return;
case NAND_CMD_READ0:
diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
index 6fe83bce83a6..970ceb948835 100644
--- a/include/linux/mtd/nand.h
+++ b/include/linux/mtd/nand.h
@@ -210,6 +210,16 @@ enum nand_ecc_algo {
*/
#define NAND_USE_BOUNCE_BUFFER 0x00100000
+/*
+ * In case your controller is implementing ->cmd_ctrl() and is relying on the
+ * default ->cmdfunc() implementation, you may want to let the core handle the
+ * tCCS delay which is required when a column change (RNDIN or RNDOUT) is
+ * requested.
+ * If your controller already takes care of this delay, you don't need to set
+ * this flag.
+ */
+#define NAND_WAIT_TCCS 0x00200000
+
/* Options set by nand scan */
/* Nand scan has allocated controller struct */
#define NAND_CONTROLLER_ALLOC 0x80000000
--
2.7.4
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH 0/2] mtd: nand: Enforce tCCS wait time after a column change
2016-10-01 8:24 [PATCH 0/2] mtd: nand: Enforce tCCS wait time after a column change Boris Brezillon
` (2 preceding siblings ...)
2016-10-25 12:31 ` [PATCH 0/2] mtd: nand: Enforce tCCS wait time " Marc Gonzalez
@ 2016-10-28 8:54 ` Boris Brezillon
3 siblings, 0 replies; 5+ messages in thread
From: Boris Brezillon @ 2016-10-28 8:54 UTC (permalink / raw)
To: David Woodhouse, Brian Norris, linux-mtd, Boris Brezillon,
Richard Weinberger
Cc: Marc Gonzalez
On Sat, 1 Oct 2016 10:24:01 +0200
Boris Brezillon <boris.brezillon@free-electrons.com> wrote:
> Hi,
>
> Marc recently struggled with the RNDIN/RNDOUT commands when trying to
> add support for the Tango NAND controller.
> After some investigation it appeared that nothing in the
> nand_command_lp() code waits tCCS, which is required to make sure the
> NAND chip is ready to receive/send data on the bus after a column
> change.
>
> This series adds some more timings to the nand_sdr_timings struct
> (including tCCS) and try to extract them from the ONFI parameter table
> (if available).
>
> It then adds a new flags to ask the core to enforce the tCCS
> constraint.
> As noted in the commit message, this is an opt-in flag to avoid perf
> regressions on existing implementations (adding an ndelay() might be
> useless if the controller IP or driver already takes care of that),
> but it might appear that some implementations are actually broken and
> needs this flag as well.
Applied.
>
> Regards,
>
> Boris
>
> Boris Brezillon (2):
> mtd: nand: Add a few more timings to nand_sdr_timings
> mtd: nand: Wait tCCS after a column change
>
> drivers/mtd/nand/nand_base.c | 26 +++++++++++++++++++++++++-
> drivers/mtd/nand/nand_timings.c | 26 +++++++++++++++++++++++++-
> include/linux/mtd/nand.h | 18 ++++++++++++++++++
> 3 files changed, 68 insertions(+), 2 deletions(-)
>
^ permalink raw reply [flat|nested] 5+ messages in thread