[RFC] Support custom timings for NAND chips

[RFC] Support custom timings for NAND chips

[Roy Spliet]

Hello,

Following is a fairly simple set of three patches that both implement and
demonstrate the use of custom timings for chip. This follows from the
requirement to support the Hynix H27UBG8T2BTR-BC, as shipped by Olimex on
their Allwinner-based ARM-boards.

I motivate my chosen approach as follows:
1) Requirement. The NAND chip is being used in the wild, and using the only
   supported ONFI timing (0) is significantly degrading performance.
2) Simplicity. The three patches are fairly straightforward, and do what they
   should do without compromise. Alternative approaches like sticking timings
   in the DT makes it more difficult for distribution to ship universal
   kernels and DTBs, because it's a NAND chip property and not a board
   property.

I send this out now (asap), because the only legacy that requires a change is
sunxi-nand. The longer we wait, the harder it gets. Don't let that stop you
from doing a proper review job! :-)
Comments? Ideas?
Yours,

Roy



-- 


IMAGINE IT >> MAKE IT

Meet us online at Twitter <http://twitter.com/ultimaker>, Facebook 
<http://facebook.com/ultimaker>, Google+ <http://google.com/+Ultimaker>

www.ultimaker.com

[RFC Patch 1/3] mtd: nand: Store actual timing in nand_chip

[Roy Spliet]

This will pave the way for storing non-ONFI timing modes.
---
 drivers/mtd/nand/nand_base.c  |   7 ++-
 drivers/mtd/nand/sunxi_nand.c |   5 +-
 include/linux/mtd/nand.h      | 106 ++++++++++++++++++++----------------------
 3 files changed, 59 insertions(+), 59 deletions(-)

diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index c2e1232..f561c68 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -3611,6 +3611,8 @@ static inline bool is_full_id_nand(struct nand_flash_dev *type)
 static bool find_full_id_nand(struct mtd_info *mtd, struct nand_chip *chip,
 		   struct nand_flash_dev *type, u8 *id_data, int *busw)
 {
+	int mode;
+
 	if (!strncmp(type->id, id_data, type->id_len)) {
 		mtd->writesize = type->pagesize;
 		mtd->erasesize = type->erasesize;
@@ -3621,8 +3623,9 @@ static bool find_full_id_nand(struct mtd_info *mtd, struct nand_chip *chip,
 		chip->options |= type->options;
 		chip->ecc_strength_ds = NAND_ECC_STRENGTH(type);
 		chip->ecc_step_ds = NAND_ECC_STEP(type);
-		chip->onfi_timing_mode_default =
-					type->onfi_timing_mode_default;
+
+		mode = type->onfi_timing_mode_default;
+		chip->sdr_timings = onfi_async_timing_mode_to_sdr_timings(mode);
 
 		*busw = type->options & NAND_BUSWIDTH_16;
 
diff --git a/drivers/mtd/nand/sunxi_nand.c b/drivers/mtd/nand/sunxi_nand.c
index 5095a32..72e4135 100644
--- a/drivers/mtd/nand/sunxi_nand.c
+++ b/drivers/mtd/nand/sunxi_nand.c
@@ -1083,7 +1083,7 @@ static int sunxi_nand_chip_init_timings(struct sunxi_nand_chip *chip,
 
 	mode = onfi_get_async_timing_mode(&chip->nand);
 	if (mode == ONFI_TIMING_MODE_UNKNOWN) {
-		mode = chip->nand.onfi_timing_mode_default;
+		timings = chip->nand.sdr_timings;
 	} else {
 		uint8_t feature[ONFI_SUBFEATURE_PARAM_LEN] = {};
 
@@ -1097,9 +1097,10 @@ static int sunxi_nand_chip_init_timings(struct sunxi_nand_chip *chip,
 						feature);
 		if (ret)
 			return ret;
+
+		timings = onfi_async_timing_mode_to_sdr_timings(mode);
 	}
 
-	timings = onfi_async_timing_mode_to_sdr_timings(mode);
 	if (IS_ERR(timings))
 		return PTR_ERR(timings);
 
diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
index 3d4ea7e..2eb92a3 100644
--- a/include/linux/mtd/nand.h
+++ b/include/linux/mtd/nand.h
@@ -536,6 +536,55 @@ struct nand_buffers {
 	uint8_t *databuf;
 };
 
+/*
+ * struct nand_sdr_timings - SDR NAND chip timings
+ *
+ * This struct defines the timing requirements of a SDR NAND chip.
+ * These informations can be found in every NAND datasheets and the timings
+ * meaning are described in the ONFI specifications:
+ * www.onfi.org/~/media/ONFI/specs/onfi_3_1_spec.pdf (chapter 4.15 Timing
+ * Parameters)
+ *
+ * All these timings are expressed in picoseconds.
+ */
+
+struct nand_sdr_timings {
+	u32 tALH_min;
+	u32 tADL_min;
+	u32 tALS_min;
+	u32 tAR_min;
+	u32 tCEA_max;
+	u32 tCEH_min;
+	u32 tCH_min;
+	u32 tCHZ_max;
+	u32 tCLH_min;
+	u32 tCLR_min;
+	u32 tCLS_min;
+	u32 tCOH_min;
+	u32 tCS_min;
+	u32 tDH_min;
+	u32 tDS_min;
+	u32 tFEAT_max;
+	u32 tIR_min;
+	u32 tITC_max;
+	u32 tRC_min;
+	u32 tREA_max;
+	u32 tREH_min;
+	u32 tRHOH_min;
+	u32 tRHW_min;
+	u32 tRHZ_max;
+	u32 tRLOH_min;
+	u32 tRP_min;
+	u32 tRR_min;
+	u64 tRST_max;
+	u32 tWB_max;
+	u32 tWC_min;
+	u32 tWH_min;
+	u32 tWHR_min;
+	u32 tWP_min;
+	u32 tWW_min;
+};
+
 /**
  * struct nand_chip - NAND Private Flash Chip Data
  * @IO_ADDR_R:		[BOARDSPECIFIC] address to read the 8 I/O lines of the
@@ -600,11 +649,7 @@ struct nand_buffers {
  * @ecc_step_ds:	[INTERN] ECC step required by the @ecc_strength_ds,
  *                      also from the datasheet. It is the recommended ECC step
  *			size, if known; if unknown, set to zero.
- * @onfi_timing_mode_default: [INTERN] default ONFI timing mode. This field is
- *			      either deduced from the datasheet if the NAND
- *			      chip is not ONFI compliant or set to 0 if it is
- *			      (an ONFI chip is always configured in mode 0
- *			      after a NAND reset)
+ * @sdr_timings		[INTERN] Pointer to default timings for SDR NAND.
  * @numchips:		[INTERN] number of physical chips
  * @chipsize:		[INTERN] the size of one chip for multichip arrays
  * @pagemask:		[INTERN] page number mask = number of (pages / chip) - 1
@@ -689,7 +734,7 @@ struct nand_chip {
 	uint8_t bits_per_cell;
 	uint16_t ecc_strength_ds;
 	uint16_t ecc_step_ds;
-	int onfi_timing_mode_default;
+	const struct nand_sdr_timings *sdr_timings;
 	int badblockpos;
 	int badblockbits;
 
@@ -975,55 +1020,6 @@ static inline int jedec_feature(struct nand_chip *chip)
 		: 0;
 }
 
-/*
- * struct nand_sdr_timings - SDR NAND chip timings
- *
- * This struct defines the timing requirements of a SDR NAND chip.
- * These informations can be found in every NAND datasheets and the timings
- * meaning are described in the ONFI specifications:
- * www.onfi.org/~/media/ONFI/specs/onfi_3_1_spec.pdf (chapter 4.15 Timing
- * Parameters)
- *
- * All these timings are expressed in picoseconds.
- */
-
-struct nand_sdr_timings {
-	u32 tALH_min;
-	u32 tADL_min;
-	u32 tALS_min;
-	u32 tAR_min;
-	u32 tCEA_max;
-	u32 tCEH_min;
-	u32 tCH_min;
-	u32 tCHZ_max;
-	u32 tCLH_min;
-	u32 tCLR_min;
-	u32 tCLS_min;
-	u32 tCOH_min;
-	u32 tCS_min;
-	u32 tDH_min;
-	u32 tDS_min;
-	u32 tFEAT_max;
-	u32 tIR_min;
-	u32 tITC_max;
-	u32 tRC_min;
-	u32 tREA_max;
-	u32 tREH_min;
-	u32 tRHOH_min;
-	u32 tRHW_min;
-	u32 tRHZ_max;
-	u32 tRLOH_min;
-	u32 tRP_min;
-	u32 tRR_min;
-	u64 tRST_max;
-	u32 tWB_max;
-	u32 tWC_min;
-	u32 tWH_min;
-	u32 tWHR_min;
-	u32 tWP_min;
-	u32 tWW_min;
-};
-
 /* get timing characteristics from ONFI timing mode. */
 const struct nand_sdr_timings *onfi_async_timing_mode_to_sdr_timings(int mode);
 #endif /* __LINUX_MTD_NAND_H */
-- 
2.4.3


-- 


IMAGINE IT >> MAKE IT

Meet us online at Twitter <http://twitter.com/ultimaker>, Facebook 
<http://facebook.com/ultimaker>, Google+ <http://google.com/+Ultimaker>

www.ultimaker.com

[RFC Patch 2/3] mtd: nand: Support non-ONFI timing modes

[Roy Spliet]

For chips whose timings are exceptionally aberrant to the point that it
impacts performance, we are going to need some mechanism to steer away
from ONFI modes. This lays the foundation for defining custom modes
---
 drivers/mtd/nand/nand_base.c | 9 +++++++--
 include/linux/mtd/nand.h     | 1 +
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index f561c68..8e636df 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -3624,8 +3624,13 @@ static bool find_full_id_nand(struct mtd_info *mtd, struct nand_chip *chip,
 		chip->ecc_strength_ds = NAND_ECC_STRENGTH(type);
 		chip->ecc_step_ds = NAND_ECC_STEP(type);
 
-		mode = type->onfi_timing_mode_default;
-		chip->sdr_timings = onfi_async_timing_mode_to_sdr_timings(mode);
+		if (type->custom_sdr_timing) {
+			chip->sdr_timings = type->custom_sdr_timing;
+		} else {
+			mode = type->onfi_timing_mode_default;
+			chip->sdr_timings =
+				onfi_async_timing_mode_to_sdr_timings(mode);
+		}
 
 		*busw = type->options & NAND_BUSWIDTH_16;
 
diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
index 2eb92a3..7d9e599 100644
--- a/include/linux/mtd/nand.h
+++ b/include/linux/mtd/nand.h
@@ -863,6 +863,7 @@ struct nand_flash_dev {
 		uint16_t step_ds;
 	} ecc;
 	int onfi_timing_mode_default;
+	const struct nand_sdr_timings *custom_sdr_timing;
 };
 
 /**
-- 
2.4.3


-- 


IMAGINE IT >> MAKE IT

Meet us online at Twitter <http://twitter.com/ultimaker>, Facebook 
<http://facebook.com/ultimaker>, Google+ <http://google.com/+Ultimaker>

www.ultimaker.com

[RFC Patch 3/3] mtd: nand: Add custom timings for Hynix H27UBG8T2BTR-BC

[Roy Spliet]

This particular chip has a tADL so high that only ONFI mode 0 matches all
timings. Using this custom timing definition, we can ramp the clock up to
50MHz, having a significant positive effect on performance (e.g. boot time
reduction of 30%).
---
 drivers/mtd/nand/nand_ids.c | 47 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 47 insertions(+)

diff --git a/drivers/mtd/nand/nand_ids.c b/drivers/mtd/nand/nand_ids.c
index dd620c1..15b4a03 100644
--- a/drivers/mtd/nand/nand_ids.c
+++ b/drivers/mtd/nand/nand_ids.c
@@ -19,6 +19,49 @@
 #define SP_OPTIONS16 (SP_OPTIONS | NAND_BUSWIDTH_16)
 
 /*
+ * Hynix H27UBG8T2BTR timings
+ * This chip has an exceptionally large tADL, which results in only supporting
+ * ONFI timing mode 0. Using these timings, the clock can be raised from
+ * 12.5MHz to 50MHz.
+ */
+const struct nand_sdr_timings hynix_h27ubg8t2btr_sdr_timing = {
+	.tADL_min = 200000,
+	.tALH_min = 5000,
+	.tALS_min = 10000,
+	.tAR_min = 10000,
+	.tCEA_max = 100000,
+	.tCEH_min = 20000,
+	.tCH_min = 5000,
+	.tCHZ_max = 50000,
+	.tCLH_min = 5000,
+	.tCLR_min = 10000,
+	.tCLS_min = 10000,
+	.tCOH_min = 15000,
+	.tCS_min = 20000,
+	.tDH_min = 5000,
+	.tDS_min = 10000,
+	.tFEAT_max = 1000000,
+	.tIR_min = 0,
+	.tITC_max = 1000000,
+	.tRC_min = 20000,
+	.tREA_max = 16000,
+	.tREH_min = 8000,
+	.tRHOH_min = 15000,
+	.tRHW_min = 100000,
+	.tRHZ_max = 100000,
+	.tRLOH_min = 5000,
+	.tRP_min = 10000,
+	.tRST_max = 500000000,
+	.tWB_max = 100000,
+	.tRR_min = 20000,
+	.tWC_min = 20000,
+	.tWH_min = 10000,
+	.tWHR_min = 80000,
+	.tWP_min = 8000,
+	.tWW_min = 100000,
+};
+
+/*
  * The chip ID list:
  *    name, device ID, page size, chip size in MiB, eraseblock size, options
  *
@@ -50,6 +93,10 @@ struct nand_flash_dev nand_flash_ids[] = {
 		{ .id = {0xad, 0xde, 0x94, 0xda, 0x74, 0xc4} },
 		  SZ_8K, SZ_8K, SZ_2M, 0, 6, 640, NAND_ECC_INFO(40, SZ_1K),
 		  4 },
+	{"H27UBG8T2BTR-BC 64G 3.3V 8-bit",
+		{ .id = {0xad, 0xd7, 0x94, 0xda, 0x74, 0xc3} },
+		  SZ_8K, SZ_4K, SZ_2M, 0, 6, 640, NAND_ECC_INFO(40, SZ_1K),
+		  0, &hynix_h27ubg8t2btr_sdr_timing },
 
 	LEGACY_ID_NAND("NAND 4MiB 5V 8-bit",   0x6B, 4, SZ_8K, SP_OPTIONS),
 	LEGACY_ID_NAND("NAND 4MiB 3,3V 8-bit", 0xE3, 4, SZ_8K, SP_OPTIONS),
-- 
2.4.3


-- 


IMAGINE IT >> MAKE IT

Meet us online at Twitter <http://twitter.com/ultimaker>, Facebook 
<http://facebook.com/ultimaker>, Google+ <http://google.com/+Ultimaker>

www.ultimaker.com

Re: [RFC] Support custom timings for NAND chips

[Boris Brezillon]

Hi Roy,

Sorry for the latency.

On Tue, 16 Jun 2015 11:45:01 +0200
Roy Spliet <r.spliet@ultimaker.com> wrote:

> Hello,
> 
> Following is a fairly simple set of three patches that both implement and
> demonstrate the use of custom timings for chip. This follows from the
> requirement to support the Hynix H27UBG8T2BTR-BC, as shipped by Olimex on
> their Allwinner-based ARM-boards.
> 
> I motivate my chosen approach as follows:
> 1) Requirement. The NAND chip is being used in the wild, and using the only
>    supported ONFI timing (0) is significantly degrading performance.
> 2) Simplicity. The three patches are fairly straightforward, and do what they
>    should do without compromise. Alternative approaches like sticking timings
>    in the DT makes it more difficult for distribution to ship universal
>    kernels and DTBs, because it's a NAND chip property and not a board
>    property.

AFAICT, it's even worst than that. I tried to boot this Olimex board
you are talking about, and it seems using ONFI timing mode 0 makes the
NAND completely unreliable. I'm not sure exactly why this happens but
here is what I suspect: the NAND is supposed to work in EDO mode, but
EDO is only active when tRC is lower than 30ns, which is not the case in
mode 0. ITOH, we cannot switch to mode 4 (which, according to the NAND
timings described in the datasheet, is the most appropriate) because
tADL is not met in this mode (tADL = 200ns, while mode 4 expect 70ns).

> 
> I send this out now (asap), because the only legacy that requires a change is
> sunxi-nand. The longer we wait, the harder it gets. Don't let that stop you
> from doing a proper review job! :-)
> Comments? Ideas?

Hm, maybe we should store nand timings directly in the nand_chip
struct, and let the core code fill it with ONFI values (when ONFI is
available) so that controller drivers do not have to do the ONFI mode
to timing struct dance.

For NAND specific timings like the ones you need for the
H27UBG8T2BTR-BC chip, maybe we could keep them in a vendor specific file
instead of defining them in the nand_ids.c file.
Also, I'm not sure whether we should define them with a new struct
nand_sdr_timings instance, or just provide a function adjusting some
fields based on the provided default ONFI timing mode.

Brian, any suggestion?

Best Regards,

Boris

-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com