1.) Why add the ECC information to the nand_chip{} ?
Each nand chip has its requirement for the ECC correctability, such as
"4bit ECC for each 512Byte" or "40bit ECC for each 1024Byte".
This ECC info is very important to the nand controller, such as gpmi.
Take the Micron MT29F64G08CBABA for example, its geometry is
8k page size, 744 bytes oob size and it requires 40bit ECC per 1K bytes.
If we do not provide the ECC info to the gpmi nand driver, it has to
calculate the ECC correctability itself. The gpmi driver will gets the 56bit
ECC for per 1K bytes which is beyond its BCH's 40bit ecc capibility.
The gpmi will quits in this case. But in actually, the gpmi can supports
this nand chip if it can get the right ECC info.
2.) About the patch set:
2.1) patch 1:
The keynote patch.
2.2) patch 2 ~ patch 7:
These patches are for ONFI nand.
Parse out the ecc info from the parameter page if we can, else
parse out the ecc info from the extended parameter page.
2.2) patch 8 ~ patch 10:
Add the ECC info for full-id nand, and parse it out.
2.3) patch 11
The gpmi uses the ecc info to set the BCH module. and with this
patch set, the gpmi can supports the MT29F64G08CBABA now.
v4 --> v5:
[0] rebase the new l2-mtd(dropped the 3 merged patches).
[1] rename ecc_size to ecc_step.
[2] change the comment for the new fields.
v3 --> v4:
[0] remove the printk for "out of memory".
[1] remove the hardcode nand_command_lp(). Update the chip->cmd_func
before we call the nand_flash_detect_ext_param_page().
[2] split the ecc_info to two fields for full id nand.
[3] update the comments for ecc_strength/ecc_size (from Brian).
v2 --> v3:
[0] add a new patch to define the semantics of the two fields.
[1] Use the Change Read Column command to remove the "last" argument.
[2] simplify the onfi_feature().
[3] Use kmalloc() to replace kcalloc().
[4] others.
v1 --> v2:
[0] Since the first 3 patches are accepted, I do not send them in the
version 2.
[1] add NAND_ prefix for macros used by the full-id nands.
[2] add onfi_ prefix for the extend parameter page data structures.
[3] rename the onfi_get_feature() to onfi_feature().
[4] I re-test this patch set again.
Huang Shijie (11):
mtd: add datasheet's ECC information to nand_chip{}
mtd: increase max OOB size to 744
mtd: get the ECC info from the parameter page for ONFI nand
mtd: add data structures for Extended Parameter Page
mtd: add a helper to get the supported features for ONFI nand
mtd: get the ECC info from the Extended Parameter Page
mtd: replace the hardcode with the onfi_feature()
mtd: add ECC info for nand_flash_dev{}
mtd: parse out the ECC info for the full-id nand chips
mtd: add the ecc info for some full-id nand chips
mtd: gpmi: set the BCH's geometry with the ecc info
drivers/mtd/nand/gpmi-nand/gpmi-nand.c | 128 +++++++++++++++++++++++++++++++-
drivers/mtd/nand/nand_base.c | 90 +++++++++++++++++++++-
drivers/mtd/nand/nand_ids.c | 8 +-
include/linux/mtd/nand.h | 74 ++++++++++++++++++-
4 files changed, 290 insertions(+), 10 deletions(-)
1.) Why add the ECC information to the nand_chip{} ?
Each nand chip has its requirement for the ECC correctability, such as
"4bit ECC for each 512Byte" or "40bit ECC for each 1024Byte".
This ECC info is very important to the nand controller, such as gpmi.
Take the Micron MT29F64G08CBABA for example, its geometry is
8k page size, 744 bytes oob size and it requires 40bit ECC per 1K bytes.
If we do not provide the ECC info to the gpmi nand driver, it has to
calculate the ECC correctability itself. The gpmi driver will gets the 56bit
ECC for per 1K bytes which is beyond its BCH's 40bit ecc capibility.
The gpmi will quits in this case. But in actually, the gpmi can supports
this nand chip if it can get the right ECC info.
2.) about the new fields.
The @ecc_strength stands for the ecc bits needed within the @ecc_step.
Both of the new fields should be set which conform to the datasheet.
For example:
"4bit ECC for each 512Byte" could be:
@ecc_strength = 4, @ecc_step = 512.
"40bit ECC for each 1024Byte" could be:
@ecc_strength = 40, @ecc_step = 1024.
3.) Why do not re-use the @strength and @size in the nand_ecc_ctrl{}?
The @strength and @size in nand_ecc_ctrl{} is used by the nand controller
driver, while the @ecc_strength and @ecc_step are get from the datasheet.
Signed-off-by: Huang Shijie <[email protected]>
---
include/linux/mtd/nand.h | 8 ++++++++
1 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
index 50c5ea0..134d470 100644
--- a/include/linux/mtd/nand.h
+++ b/include/linux/mtd/nand.h
@@ -434,6 +434,12 @@ struct nand_buffers {
* bad block marker position; i.e., BBM == 11110111b is
* not bad when badblockbits == 7
* @cellinfo: [INTERN] MLC/multichip data from chip ident
+ * @ecc_strength: [INTERN] ECC correctability from the datasheet.
+ * Minimum amount of bit errors per @ecc_step guaranteed to
+ * be correctable. If unknown, set to zero.
+ * @ecc_step: [INTERN] ECC step required by the @ecc_strength,
+ * also from the datasheet. It is the recommended ECC step
+ * size, if known; if unknown, set to zero.
* @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
@@ -510,6 +516,8 @@ struct nand_chip {
unsigned int pagebuf_bitflips;
int subpagesize;
uint8_t cellinfo;
+ uint16_t ecc_strength;
+ uint16_t ecc_step;
int badblockpos;
int badblockbits;
--
1.7.1
Add an instance of an anonymous struct to store the ECC infor for full id
nand chips.
@ecc.strength: ECC correctability from the datasheet.
@ecc.step: ECC size required by the @ecc.strength,
Also add the necessary macros to make the code simple and clean.
Signed-off-by: Huang Shijie <[email protected]>
---
include/linux/mtd/nand.h | 15 +++++++++++++++
1 files changed, 15 insertions(+), 0 deletions(-)
diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
index c503850..718d2c5 100644
--- a/include/linux/mtd/nand.h
+++ b/include/linux/mtd/nand.h
@@ -625,6 +625,11 @@ struct nand_chip {
{ .name = (nm), {{ .dev_id = (devid) }}, .chipsize = (chipsz), \
.options = (opts) }
+#define NAND_ECC_INFO(_strength, _step) \
+ { .strength = (_strength), .step = (_step) }
+#define NAND_ECC_STRENGTH(type) ((type)->ecc.strength)
+#define NAND_ECC_STEP(type) ((type)->ecc.step)
+
/**
* struct nand_flash_dev - NAND Flash Device ID Structure
* @name: a human-readable name of the NAND chip
@@ -642,6 +647,12 @@ struct nand_chip {
* @options: stores various chip bit options
* @id_len: The valid length of the @id.
* @oobsize: OOB size
+ * @ecc.strength: The ECC correctability from the datasheet, same as the
+ * @ecc_strength in nand_chip{}.
+ * @ecc.step: The ECC step required by the @ecc.strength, same as the
+ * @ecc_size in nand_chip{}, also from the datasheet.
+ * For example, the "4bit ECC for each 512Byte" can be set with
+ * NAND_ECC_INFO(4, 512).
*/
struct nand_flash_dev {
char *name;
@@ -658,6 +669,10 @@ struct nand_flash_dev {
unsigned int options;
uint16_t id_len;
uint16_t oobsize;
+ struct {
+ uint16_t strength;
+ uint16_t step;
+ } ecc;
};
/**
--
1.7.1
Since the ONFI 2.1, the onfi spec adds the Extended Parameter Page
to store the ECC info.
The onfi spec tells us that if the nand chip's recommended ECC codeword
size is not 512 bytes, then the @ecc_bits is 0xff. The host _SHOULD_ then
read the Extended ECC information that is part of the extended parameter
page to retrieve the ECC requirements for this device.
This patch implement the reading of the Extended Parameter Page, and parses
the sections for ECC type, and get the ECC info from the ECC section.
Tested this patch with Micron MT29F64G08CBABAWP.
Acked-by: Pekon Gupta <[email protected]>
Signed-off-by: Huang Shijie <[email protected]>
---
drivers/mtd/nand/nand_base.c | 77 ++++++++++++++++++++++++++++++++++++++++++
1 files changed, 77 insertions(+), 0 deletions(-)
diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index 15630ef..e98ea69 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -2835,6 +2835,68 @@ static u16 onfi_crc16(u16 crc, u8 const *p, size_t len)
return crc;
}
+/* Parse the Extended Parameter Page. */
+static int nand_flash_detect_ext_param_page(struct mtd_info *mtd,
+ struct nand_chip *chip, struct nand_onfi_params *p)
+{
+ struct onfi_ext_param_page *ep;
+ struct onfi_ext_section *s;
+ struct onfi_ext_ecc_info *ecc;
+ uint8_t *cursor;
+ int len;
+ int ret;
+ int i;
+
+ len = le16_to_cpu(p->ext_param_page_length) * 16;
+ ep = kmalloc(len, GFP_KERNEL);
+ if (!ep) {
+ ret = -ENOMEM;
+ goto ext_out;
+ }
+
+ /* Send our own NAND_CMD_PARAM. */
+ chip->cmdfunc(mtd, NAND_CMD_PARAM, 0, -1);
+
+ /* Use the Change Read Column command to skip the ONFI param pages. */
+ chip->cmdfunc(mtd, NAND_CMD_RNDOUT,
+ sizeof(*p) * p->num_of_param_pages , -1);
+
+ /* Read out the Extended Parameter Page. */
+ chip->read_buf(mtd, (uint8_t *)ep, len);
+ if ((onfi_crc16(ONFI_CRC_BASE, ((uint8_t *)ep) + 2, len - 2)
+ != le16_to_cpu(ep->crc)) || strncmp(ep->sig, "EPPS", 4)) {
+ pr_debug("fail in the CRC.\n");
+ ret = -EINVAL;
+ goto ext_out;
+ }
+
+ /* find the ECC section. */
+ cursor = (uint8_t *)(ep + 1);
+ for (i = 0; i < ONFI_EXT_SECTION_MAX; i++) {
+ s = ep->sections + i;
+ if (s->type == ONFI_SECTION_TYPE_2)
+ break;
+ cursor += s->length * 16;
+ }
+ if (i == ONFI_EXT_SECTION_MAX) {
+ pr_debug("We can not find the ECC section.\n");
+ ret = -EINVAL;
+ goto ext_out;
+ }
+
+ /* get the info we want. */
+ ecc = (struct onfi_ext_ecc_info *)cursor;
+ chip->ecc_strength = ecc->ecc_bits;
+ chip->ecc_step = 1 << ecc->codeword_size;
+
+ pr_info("ONFI extended param page detected.\n");
+ return 0;
+
+ext_out:
+ kfree(ep);
+ return ret;
+}
+
/*
* Check if the NAND chip is ONFI compliant, returns 1 if it is, 0 otherwise.
*/
@@ -2903,6 +2965,21 @@ static int nand_flash_detect_onfi(struct mtd_info *mtd, struct nand_chip *chip,
if (p->ecc_bits != 0xff) {
chip->ecc_strength = p->ecc_bits;
chip->ecc_step = 512;
+ } else if (chip->onfi_version >= 21 &&
+ (onfi_feature(chip) & ONFI_FEATURE_EXT_PARAM_PAGE)) {
+
+ /*
+ * The nand_flash_detect_ext_param_page() uses the
+ * Change Read Column command which maybe not supported
+ * by the chip->cmdfunc. So try to update the chip->cmdfunc
+ * now. We do not replace user supplied command function.
+ */
+ if (mtd->writesize > 512 && chip->cmdfunc == nand_command)
+ chip->cmdfunc = nand_command_lp;
+
+ /* The Extended Parameter Page is supported since ONFI 2.1. */
+ if (nand_flash_detect_ext_param_page(mtd, chip, p))
+ pr_info("Failed to detect the extended param page.\n");
}
pr_info("ONFI flash detected\n");
--
1.7.1
Parse out the ECC information for the full-id nand chips.
Signed-off-by: Huang Shijie <[email protected]>
---
drivers/mtd/nand/nand_base.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index 41e5c07..359e105 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -3243,6 +3243,8 @@ static bool find_full_id_nand(struct mtd_info *mtd, struct nand_chip *chip,
chip->cellinfo = id_data[2];
chip->chipsize = (uint64_t)type->chipsize << 20;
chip->options |= type->options;
+ chip->ecc_strength = NAND_ECC_STRENGTH(type);
+ chip->ecc_step = NAND_ECC_STEP(type);
*busw = type->options & NAND_BUSWIDTH_16;
--
1.7.1
The current code uses the hardcode to detect the 16-bit bus width.
Use the onfi_feature() to replace it.
Signed-off-by: Huang Shijie <[email protected]>
---
drivers/mtd/nand/nand_base.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index e98ea69..41e5c07 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -2958,9 +2958,9 @@ static int nand_flash_detect_onfi(struct mtd_info *mtd, struct nand_chip *chip,
mtd->oobsize = le16_to_cpu(p->spare_bytes_per_page);
chip->chipsize = le32_to_cpu(p->blocks_per_lun);
chip->chipsize *= (uint64_t)mtd->erasesize * p->lun_count;
- *busw = 0;
- if (le16_to_cpu(p->features) & 1)
- *busw = NAND_BUSWIDTH_16;
+
+ *busw = (onfi_feature(chip) & ONFI_FEATURE_16_BIT_BUS) ?
+ NAND_BUSWIDTH_16 : 0;
if (p->ecc_bits != 0xff) {
chip->ecc_strength = p->ecc_bits;
--
1.7.1
The oob size of Micron's MT29F64G08CBABAWP is 744 bytes.
So increase the NAND_MAX_OOBSIZE to 744.
Signed-off-by: Huang Shijie <[email protected]>
---
include/linux/mtd/nand.h | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
index 134d470..6f7788a 100644
--- a/include/linux/mtd/nand.h
+++ b/include/linux/mtd/nand.h
@@ -56,7 +56,7 @@ extern int nand_unlock(struct mtd_info *mtd, loff_t ofs, uint64_t len);
* is supported now. If you add a chip with bigger oobsize/page
* adjust this accordingly.
*/
-#define NAND_MAX_OOBSIZE 640
+#define NAND_MAX_OOBSIZE 744
#define NAND_MAX_PAGESIZE 8192
/*
--
1.7.1
Since the ONFI 2.1, the onfi spec adds the Extended Parameter Page
to store the ECC info.
The onfi spec tells us that if the nand chip's recommended ECC codeword
size is not 512 bytes, then the @ecc_bits is 0xff. The host _SHOULD_ then
read the Extended ECC information that is part of the extended parameter
page to retrieve the ECC requirements for this device.
This patch adds
[1] the neccessary fields for nand_onfi_params{},
[2] and adds the onfi_ext_ecc_info{} for Extended ECC information,
[3] adds onfi_ext_section{} for extended sections,
[4] and adds onfi_ext_param_page{} for the Extended Parameter Page.
Acked-by: Pekon Gupta <[email protected]>
Signed-off-by: Huang Shijie <[email protected]>
---
include/linux/mtd/nand.h | 39 ++++++++++++++++++++++++++++++++++++++-
1 files changed, 38 insertions(+), 1 deletions(-)
diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
index 6f7788a..8b8fd93 100644
--- a/include/linux/mtd/nand.h
+++ b/include/linux/mtd/nand.h
@@ -224,7 +224,10 @@ struct nand_onfi_params {
__le16 revision;
__le16 features;
__le16 opt_cmd;
- u8 reserved[22];
+ u8 reserved0[2];
+ __le16 ext_param_page_length; /* since ONFI 2.1 */
+ u8 num_of_param_pages; /* since ONFI 2.1 */
+ u8 reserved1[17];
/* manufacturer information block */
char manufacturer[12];
@@ -281,6 +284,40 @@ struct nand_onfi_params {
#define ONFI_CRC_BASE 0x4F4E
+/* Extended ECC information Block Definition (since ONFI 2.1) */
+struct onfi_ext_ecc_info {
+ u8 ecc_bits;
+ u8 codeword_size;
+ __le16 bb_per_lun;
+ __le16 block_endurance;
+ u8 reserved[2];
+} __attribute__((packed));
+
+#define ONFI_SECTION_TYPE_0 0 /* Unused section. */
+#define ONFI_SECTION_TYPE_1 1 /* for additional sections. */
+#define ONFI_SECTION_TYPE_2 2 /* for ECC information. */
+struct onfi_ext_section {
+ u8 type;
+ u8 length;
+} __attribute__((packed));
+
+#define ONFI_EXT_SECTION_MAX 8
+
+/* Extended Parameter Page Definition (since ONFI 2.1) */
+struct onfi_ext_param_page {
+ __le16 crc;
+ u8 sig[4]; /* 'E' 'P' 'P' 'S' */
+ u8 reserved0[10];
+ struct onfi_ext_section sections[ONFI_EXT_SECTION_MAX];
+
+ /*
+ * The actual size of the Extended Parameter Page is in
+ * @ext_param_page_length of nand_onfi_params{}.
+ * The following are the variable length sections.
+ * So we do not add any fields below. Please see the ONFI spec.
+ */
+} __attribute__((packed));
+
/**
* struct nand_hw_control - Control structure for hardware controller (e.g ECC generator) shared among independent devices
* @lock: protection lock
--
1.7.1
From: Huang Shijie <[email protected]>
add a helper to get the supported features for ONFI nand.
Also add the neccessary macros.
Signed-off-by: Huang Shijie <[email protected]>
---
include/linux/mtd/nand.h | 10 ++++++++++
1 files changed, 10 insertions(+), 0 deletions(-)
diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
index 8b8fd93..c503850 100644
--- a/include/linux/mtd/nand.h
+++ b/include/linux/mtd/nand.h
@@ -202,6 +202,10 @@ typedef enum {
/* Keep gcc happy */
struct nand_chip;
+/* ONFI features */
+#define ONFI_FEATURE_16_BIT_BUS (1 << 0)
+#define ONFI_FEATURE_EXT_PARAM_PAGE (1 << 7)
+
/* ONFI timing mode, used in both asynchronous and synchronous mode */
#define ONFI_TIMING_MODE_0 (1 << 0)
#define ONFI_TIMING_MODE_1 (1 << 1)
@@ -753,6 +757,12 @@ struct platform_nand_chip *get_platform_nandchip(struct mtd_info *mtd)
return chip->priv;
}
+/* return the supported features. */
+static inline int onfi_feature(struct nand_chip *chip)
+{
+ return chip->onfi_version ? le16_to_cpu(chip->onfi_params.features) : 0;
+}
+
/* return the supported asynchronous timing mode. */
static inline int onfi_get_async_timing_mode(struct nand_chip *chip)
{
--
1.7.1
>From the ONFI spec, we can just get the ECC info from the @ecc_bits field of
the parameter page.
Signed-off-by: Huang Shijie <[email protected]>
---
drivers/mtd/nand/nand_base.c | 5 +++++
1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index edc7663..15630ef 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -2900,6 +2900,11 @@ static int nand_flash_detect_onfi(struct mtd_info *mtd, struct nand_chip *chip,
if (le16_to_cpu(p->features) & 1)
*busw = NAND_BUSWIDTH_16;
+ if (p->ecc_bits != 0xff) {
+ chip->ecc_strength = p->ecc_bits;
+ chip->ecc_step = 512;
+ }
+
pr_info("ONFI flash detected\n");
return 1;
}
--
1.7.1
Add the ecc info for TC58NVG2S0F, TC58NVG3S0F, TC58NVG5D2 and TC58NVG6D2.
>From these chips' datasheets, we know that:
The TC58NVG2S0F and TC58NVG3S0F require 4bit ECC for per 512byte.
The TC58NVG5D2 and TC58NVG6D2 require 40bits ECC for per 1024byte.
Signed-off-by: Huang Shijie <[email protected]>
---
drivers/mtd/nand/nand_ids.c | 8 ++++----
1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/mtd/nand/nand_ids.c b/drivers/mtd/nand/nand_ids.c
index 683813a..a87b0a3 100644
--- a/drivers/mtd/nand/nand_ids.c
+++ b/drivers/mtd/nand/nand_ids.c
@@ -33,16 +33,16 @@ struct nand_flash_dev nand_flash_ids[] = {
*/
{"TC58NVG2S0F 4G 3.3V 8-bit",
{ .id = {0x98, 0xdc, 0x90, 0x26, 0x76, 0x15, 0x01, 0x08} },
- SZ_4K, SZ_512, SZ_256K, 0, 8, 224},
+ SZ_4K, SZ_512, SZ_256K, 0, 8, 224, NAND_ECC_INFO(4, SZ_512) },
{"TC58NVG3S0F 8G 3.3V 8-bit",
{ .id = {0x98, 0xd3, 0x90, 0x26, 0x76, 0x15, 0x02, 0x08} },
- SZ_4K, SZ_1K, SZ_256K, 0, 8, 232},
+ SZ_4K, SZ_1K, SZ_256K, 0, 8, 232, NAND_ECC_INFO(4, SZ_512) },
{"TC58NVG5D2 32G 3.3V 8-bit",
{ .id = {0x98, 0xd7, 0x94, 0x32, 0x76, 0x56, 0x09, 0x00} },
- SZ_8K, SZ_4K, SZ_1M, 0, 8, 640},
+ SZ_8K, SZ_4K, SZ_1M, 0, 8, 640, NAND_ECC_INFO(40, SZ_1K) },
{"TC58NVG6D2 64G 3.3V 8-bit",
{ .id = {0x98, 0xde, 0x94, 0x82, 0x76, 0x56, 0x04, 0x20} },
- SZ_8K, SZ_8K, SZ_2M, 0, 8, 640},
+ SZ_8K, SZ_8K, SZ_2M, 0, 8, 640, NAND_ECC_INFO(40, SZ_1K) },
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),
--
1.7.1
If the nand chip provides us the ECC info, we can use it firstly.
The set_geometry_by_ecc_info() will use the ECC info, and
calculate the parameters we need.
Rename the old code to lagacy_set_geometry() which will takes effect
when there is no ECC info from the nand chip or we fails in the ECC info case.
Signed-off-by: Huang Shijie <[email protected]>
---
drivers/mtd/nand/gpmi-nand/gpmi-nand.c | 128 +++++++++++++++++++++++++++++++-
1 files changed, 127 insertions(+), 1 deletions(-)
diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
index 25ecfa1..53180da 100644
--- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
+++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
@@ -112,7 +112,128 @@ static inline bool gpmi_check_ecc(struct gpmi_nand_data *this)
return true;
}
-int common_nfc_set_geometry(struct gpmi_nand_data *this)
+/*
+ * If we can get the ECC information from the nand chip, we do not
+ * need to calculate them ourselves.
+ *
+ * We may have available oob space in this case.
+ */
+static bool set_geometry_by_ecc_info(struct gpmi_nand_data *this)
+{
+ struct bch_geometry *geo = &this->bch_geometry;
+ struct mtd_info *mtd = &this->mtd;
+ struct nand_chip *chip = mtd->priv;
+ struct nand_oobfree *of = gpmi_hw_ecclayout.oobfree;
+ unsigned int block_mark_bit_offset;
+
+ if (!(chip->ecc_strength > 0 && chip->ecc_step > 0))
+ return false;
+
+ switch (chip->ecc_step) {
+ case SZ_512:
+ geo->gf_len = 13;
+ break;
+ case SZ_1K:
+ geo->gf_len = 14;
+ break;
+ default:
+ dev_err(this->dev,
+ "unsupported nand chip. ecc bits : %d, ecc size : %d\n",
+ chip->ecc_strength, chip->ecc_step);
+ return false;
+ }
+ geo->ecc_chunk_size = chip->ecc_step;
+ geo->ecc_strength = round_up(chip->ecc_strength, 2);
+ if (!gpmi_check_ecc(this))
+ return false;
+
+ /* Keep the C >= O */
+ if (geo->ecc_chunk_size < mtd->oobsize) {
+ dev_err(this->dev,
+ "unsupported nand chip. ecc size: %d, oob size : %d\n",
+ chip->ecc_step, mtd->oobsize);
+ return false;
+ }
+
+ /* The default value, see comment in the lagacy_set_geometry(). */
+ geo->metadata_size = 10;
+
+ geo->ecc_chunk_count = mtd->writesize / geo->ecc_chunk_size;
+
+ /*
+ * Now, the NAND chip with 2K page(data chunk is 512byte) shows below:
+ *
+ * | P |
+ * |<----------------------------------------------------->|
+ * | |
+ * | (Block Mark) |
+ * | P' | | | |
+ * |<-------------------------------------------->| D | | O' |
+ * | |<---->| |<--->|
+ * V V V V V
+ * +---+----------+-+----------+-+----------+-+----------+-+-----+
+ * | M | data |E| data |E| data |E| data |E| |
+ * +---+----------+-+----------+-+----------+-+----------+-+-----+
+ *
+ * P : the page size for BCH module.
+ * E : The ECC strength.
+ * G : the length of Galois Field.
+ * N : The chunk count of per page.
+ * M : the metasize of per page.
+ * C : the ecc chunk size, aka the "data" above.
+ * P': the nand chip's page size.
+ * O : the nand chip's oob size.
+ * O': the free oob.
+ *
+ * The formula for P is :
+ *
+ * E * G * N
+ * P = ------------ + P' + M
+ * 8
+ *
+ * The position of block mark moves forward in the ECC-based view
+ * of page, and the delta is:
+ *
+ * E * G * (N - 1)
+ * D = (---------------- + M)
+ * 8
+ *
+ * Please see the comment in lagacy_set_geometry().
+ * With the condition C >= O , we still can get same result.
+ * So the bit position of the physical block mark within the ECC-based
+ * view of the page is :
+ * (P' - D) * 8
+ */
+ geo->page_size = mtd->writesize + geo->metadata_size +
+ (geo->gf_len * geo->ecc_strength * geo->ecc_chunk_count) / 8;
+
+ /* The available oob size we have. */
+ if (geo->page_size < mtd->writesize + mtd->oobsize) {
+ of->offset = geo->page_size - mtd->writesize;
+ of->length = mtd->oobsize - of->offset;
+ mtd->oobavail = gpmi_hw_ecclayout.oobavail = of->length;
+ }
+
+ geo->payload_size = mtd->writesize;
+
+ geo->auxiliary_status_offset = ALIGN(geo->metadata_size, 4);
+ geo->auxiliary_size = ALIGN(geo->metadata_size, 4)
+ + ALIGN(geo->ecc_chunk_count, 4);
+
+ if (!this->swap_block_mark)
+ return true;
+
+ /* For bit swap. */
+ block_mark_bit_offset = mtd->writesize * 8 -
+ (geo->ecc_strength * geo->gf_len * (geo->ecc_chunk_count - 1)
+ + geo->metadata_size * 8);
+
+ geo->block_mark_byte_offset = block_mark_bit_offset / 8;
+ geo->block_mark_bit_offset = block_mark_bit_offset % 8;
+ return true;
+}
+
+static int lagacy_set_geometry(struct gpmi_nand_data *this)
{
struct bch_geometry *geo = &this->bch_geometry;
struct mtd_info *mtd = &this->mtd;
@@ -224,6 +345,11 @@ int common_nfc_set_geometry(struct gpmi_nand_data *this)
return 0;
}
+int common_nfc_set_geometry(struct gpmi_nand_data *this)
+{
+ return set_geometry_by_ecc_info(this) ? 0 : lagacy_set_geometry(this);
+}
+
struct dma_chan *get_dma_chan(struct gpmi_nand_data *this)
{
int chipnr = this->current_chip;
--
1.7.1
On Wed, 2013-05-15 at 16:40 +0800, Huang Shijie wrote:
> 1.) Why add the ECC information to the nand_chip{} ?
> Each nand chip has its requirement for the ECC correctability, such as
> "4bit ECC for each 512Byte" or "40bit ECC for each 1024Byte".
> This ECC info is very important to the nand controller, such as gpmi.
>
> Take the Micron MT29F64G08CBABA for example, its geometry is
> 8k page size, 744 bytes oob size and it requires 40bit ECC per 1K bytes.
> If we do not provide the ECC info to the gpmi nand driver, it has to
> calculate the ECC correctability itself. The gpmi driver will gets the 56bit
> ECC for per 1K bytes which is beyond its BCH's 40bit ecc capibility.
> The gpmi will quits in this case. But in actually, the gpmi can supports
> this nand chip if it can get the right ECC info.
Thanks. Brian has disappeared, may be busy, but I would still like to
wait for his response. At the very least, I would like to make sure he
does not object.
--
Best Regards,
Artem Bityutskiy
On Wed, 2013-05-15 at 16:40 +0800, Huang Shijie wrote:
> + * @ecc_strength: [INTERN] ECC correctability from the datasheet.
> + * Minimum amount of bit errors per @ecc_step guaranteed to
> + * be correctable. If unknown, set to zero.
> + * @ecc_step: [INTERN] ECC step required by the @ecc_strength,
> + * also from the datasheet. It is the recommended ECC step
> + * size, if known; if unknown, set to zero.
Here and in other places you talk about "datasheet". Do you assume that
the real ECC strength/step used by NAND chips may be different? Or you
assume it must be the same?
--
Best Regards,
Artem Bityutskiy
On Wed, 2013-05-15 at 16:40 +0800, Huang Shijie wrote:
> The oob size of Micron's MT29F64G08CBABAWP is 744 bytes.
> So increase the NAND_MAX_OOBSIZE to 744.
>
> Signed-off-by: Huang Shijie <[email protected]>
OK, this patch is really independent. I do not know why you put it to
this series. Let me apply it separately.
--
Best Regards,
Artem Bityutskiy
于 2013年05月15日 20:11, Artem Bityutskiy 写道:
> On Wed, 2013-05-15 at 16:40 +0800, Huang Shijie wrote:
>> + * @ecc_strength: [INTERN] ECC correctability from the datasheet.
>> + * Minimum amount of bit errors per @ecc_step guaranteed to
>> + * be correctable. If unknown, set to zero.
>> + * @ecc_step: [INTERN] ECC step required by the @ecc_strength,
>> + * also from the datasheet. It is the recommended ECC step
>> + * size, if known; if unknown, set to zero.
> Here and in other places you talk about "datasheet". Do you assume that
> the real ECC strength/step used by NAND chips may be different? Or you
> assume it must be the same?
>
The two fields are used to store the ecc info from the datasheet.
The two fields are just for a reference.
[1] The nand controller may do not use these two fields, it's ok;
For example, the datasheet requires "4bits per 512 bytes".
The nand controller can uses 8bits per 512 bytes.
[2] but sometimes the nand controller must use these two fields.
For example, the datasheet requires "40bits per 1024 bytes".
For the hardware limit, the nand controller(BCH) may supports the
40bits ecc in the maximum.
So the nand controller must use these two fields now.
thanks
Huang Shijie
On Thu, 2013-05-16 at 10:16 +0800, Huang Shijie wrote:
> 于 2013年05月15日 20:11, Artem Bityutskiy 写道:
> > On Wed, 2013-05-15 at 16:40 +0800, Huang Shijie wrote:
> >> + * @ecc_strength: [INTERN] ECC correctability from the datasheet.
> >> + * Minimum amount of bit errors per @ecc_step guaranteed to
> >> + * be correctable. If unknown, set to zero.
> >> + * @ecc_step: [INTERN] ECC step required by the @ecc_strength,
> >> + * also from the datasheet. It is the recommended ECC step
> >> + * size, if known; if unknown, set to zero.
> > Here and in other places you talk about "datasheet". Do you assume that
> > the real ECC strength/step used by NAND chips may be different? Or you
> > assume it must be the same?
> >
> The two fields are used to store the ecc info from the datasheet.
> The two fields are just for a reference.
>
> [1] The nand controller may do not use these two fields, it's ok;
> For example, the datasheet requires "4bits per 512 bytes".
> The nand controller can uses 8bits per 512 bytes.
>
>
> [2] but sometimes the nand controller must use these two fields.
> For example, the datasheet requires "40bits per 1024 bytes".
> For the hardware limit, the nand controller(BCH) may supports the
> 40bits ecc in the maximum.
> So the nand controller must use these two fields now.
I wonder if it makes sense to name things so that it is clear form the
names whether that is the "theoretical" datasheet values or the real
ones. I would prefer to clearly distinguish between them, in names and
comments. Thoughts?
--
Best Regards,
Artem Bityutskiy
于 2013年05月16日 15:14, Artem Bityutskiy 写道:
> On Thu, 2013-05-16 at 10:16 +0800, Huang Shijie wrote:
>> 于 2013年05月15日 20:11, Artem Bityutskiy 写道:
>>> On Wed, 2013-05-15 at 16:40 +0800, Huang Shijie wrote:
>>>> + * @ecc_strength: [INTERN] ECC correctability from the datasheet.
>>>> + * Minimum amount of bit errors per @ecc_step guaranteed to
>>>> + * be correctable. If unknown, set to zero.
>>>> + * @ecc_step: [INTERN] ECC step required by the @ecc_strength,
>>>> + * also from the datasheet. It is the recommended ECC step
>>>> + * size, if known; if unknown, set to zero.
>>> Here and in other places you talk about "datasheet". Do you assume that
>>> the real ECC strength/step used by NAND chips may be different? Or you
>>> assume it must be the same?
>>>
>> The two fields are used to store the ecc info from the datasheet.
>> The two fields are just for a reference.
>>
>> [1] The nand controller may do not use these two fields, it's ok;
>> For example, the datasheet requires "4bits per 512 bytes".
>> The nand controller can uses 8bits per 512 bytes.
>>
>>
>> [2] but sometimes the nand controller must use these two fields.
>> For example, the datasheet requires "40bits per 1024 bytes".
>> For the hardware limit, the nand controller(BCH) may supports the
>> 40bits ecc in the maximum.
>> So the nand controller must use these two fields now.
> I wonder if it makes sense to name things so that it is clear form the
> names whether that is the "theoretical" datasheet values or the real
> ones. I would prefer to clearly distinguish between them, in names and
> comments. Thoughts?
>
what's about add the "_datasheet" for these two fields?
such as
ecc_strength__datasheet;ecc_step__datasheet
Huang Shijie
On Thu, 2013-05-16 at 16:06 +0800, Huang Shijie wrote:
> what's about add the "_datasheet" for these two fields?
> such as
>
> ecc_strength__datasheet;ecc_step__datasheet
May be, if these are not too long names. Or ecc_ds_strignth /
ecc_ds_step, for example.
--
Best Regards,
Artem Bityutskiy
于 2013年05月16日 17:36, Artem Bityutskiy 写道:
> May be, if these are not too long names. Or ecc_ds_strignth /
> ecc_ds_step, for example.
I prefer to the ecc_strength_ds/ecc_step_ds. :)
Huang Shijie