Re: [PATCH V3 4/9] mtd: get the ECC info from the Extended Parameter Page

2013-04-25 Thread Huang Shijie

于 2013年04月25日 14:20, Brian Norris 写道:

No, you cannot do this. Some drivers will provide their own cmdfunc,
so nand_command_lp() is unexpected for those drivers.

ok. got it.

Your problem seems, instead, that you are executing this function too
early, before nand_flash_get_type() is able to assign

yes. This is just the case i meet.

nand_command_lp() to be your cmdfunc. You might just want to call this
function after the following lines in nand_flash_get_type():

 /* Do not replace user supplied command function! */
 if (mtd->writesize>  512&&  chip->cmdfunc == nand_command)
 chip->cmdfunc = nand_command_lp;


I will send a fix patch about this patch.

thanks
Huang Shijie


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH V3 4/9] mtd: get the ECC info from the Extended Parameter Page

2013-04-25 Thread Brian Norris
On Tue, Apr 23, 2013 at 1:54 AM, Huang Shijie  wrote:
> 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.
>
> Signed-off-by: Huang Shijie 
> ---
>  drivers/mtd/nand/nand_base.c |   72 
> ++
>  1 files changed, 72 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
> index beff911..584cb0f 100644
> --- a/drivers/mtd/nand/nand_base.c
> +++ b/drivers/mtd/nand/nand_base.c
> @@ -2846,6 +2846,73 @@ 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) {
> +   pr_debug("Out of memory.\n");

I believe people are trying to kill such messages from the kernel. If
kmalloc() fails to allocate memory, the mm system should give plenty
of scary and helpful messages anyway. This print message just becomes
bloat, then.

> +   ret = -ENOMEM;
> +   goto ext_out;
> +   }
> +
> +   /* Send our own NAND_CMD_PARAM. */
> +   chip->cmdfunc(mtd, NAND_CMD_PARAM, 0, -1);
> +
> +   /*
> +* Use the Change Read Columm command to skip the ONFI Parameter 
> Pages.
> +* The nand_command() does not support the Change Read Columm command,
> +* use the nand_command_lp() instead.
> +*/
> +   nand_command_lp(mtd, NAND_CMD_RNDOUT,
> +   sizeof(*p) * p->num_of_param_pages , -1);

No, you cannot do this. Some drivers will provide their own cmdfunc,
so nand_command_lp() is unexpected for those drivers.

Your problem seems, instead, that you are executing this function too
early, before nand_flash_get_type() is able to assign
nand_command_lp() to be your cmdfunc. You might just want to call this
function after the following lines in nand_flash_get_type():

/* Do not replace user supplied command function! */
if (mtd->writesize > 512 && chip->cmdfunc == nand_command)
chip->cmdfunc = nand_command_lp;

> +   /* 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_size = 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.
>   */
> @@ -2914,6 +2981,11 @@ 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_size = 512;
> +   } else if (chip->onfi_version >= 21 &&
> +   (onfi_feature(chip) & ONFI_FEATURE_EXT_PARAM_PAGE)) {
> +   /* 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");

Brian
--
To unsubscribe 

Re: [PATCH V3 4/9] mtd: get the ECC info from the Extended Parameter Page

2013-04-25 Thread Brian Norris
On Tue, Apr 23, 2013 at 1:54 AM, Huang Shijie b32...@freescale.com wrote:
 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.

 Signed-off-by: Huang Shijie b32...@freescale.com
 ---
  drivers/mtd/nand/nand_base.c |   72 
 ++
  1 files changed, 72 insertions(+), 0 deletions(-)

 diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
 index beff911..584cb0f 100644
 --- a/drivers/mtd/nand/nand_base.c
 +++ b/drivers/mtd/nand/nand_base.c
 @@ -2846,6 +2846,73 @@ 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) {
 +   pr_debug(Out of memory.\n);

I believe people are trying to kill such messages from the kernel. If
kmalloc() fails to allocate memory, the mm system should give plenty
of scary and helpful messages anyway. This print message just becomes
bloat, then.

 +   ret = -ENOMEM;
 +   goto ext_out;
 +   }
 +
 +   /* Send our own NAND_CMD_PARAM. */
 +   chip-cmdfunc(mtd, NAND_CMD_PARAM, 0, -1);
 +
 +   /*
 +* Use the Change Read Columm command to skip the ONFI Parameter 
 Pages.
 +* The nand_command() does not support the Change Read Columm command,
 +* use the nand_command_lp() instead.
 +*/
 +   nand_command_lp(mtd, NAND_CMD_RNDOUT,
 +   sizeof(*p) * p-num_of_param_pages , -1);

No, you cannot do this. Some drivers will provide their own cmdfunc,
so nand_command_lp() is unexpected for those drivers.

Your problem seems, instead, that you are executing this function too
early, before nand_flash_get_type() is able to assign
nand_command_lp() to be your cmdfunc. You might just want to call this
function after the following lines in nand_flash_get_type():

/* Do not replace user supplied command function! */
if (mtd-writesize  512  chip-cmdfunc == nand_command)
chip-cmdfunc = nand_command_lp;

 +   /* 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_size = 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.
   */
 @@ -2914,6 +2981,11 @@ 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_size = 512;
 +   } else if (chip-onfi_version = 21 
 +   (onfi_feature(chip)  ONFI_FEATURE_EXT_PARAM_PAGE)) {
 +   /* 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);

Brian
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More 

Re: [PATCH V3 4/9] mtd: get the ECC info from the Extended Parameter Page

2013-04-25 Thread Huang Shijie

于 2013年04月25日 14:20, Brian Norris 写道:

No, you cannot do this. Some drivers will provide their own cmdfunc,
so nand_command_lp() is unexpected for those drivers.

ok. got it.

Your problem seems, instead, that you are executing this function too
early, before nand_flash_get_type() is able to assign

yes. This is just the case i meet.

nand_command_lp() to be your cmdfunc. You might just want to call this
function after the following lines in nand_flash_get_type():

 /* Do not replace user supplied command function! */
 if (mtd-writesize  512  chip-cmdfunc == nand_command)
 chip-cmdfunc = nand_command_lp;


I will send a fix patch about this patch.

thanks
Huang Shijie


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH V3 4/9] mtd: get the ECC info from the Extended Parameter Page

2013-04-23 Thread Huang Shijie
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.

Signed-off-by: Huang Shijie 
---
 drivers/mtd/nand/nand_base.c |   72 ++
 1 files changed, 72 insertions(+), 0 deletions(-)

diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index beff911..584cb0f 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -2846,6 +2846,73 @@ 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) {
+   pr_debug("Out of memory.\n");
+   ret = -ENOMEM;
+   goto ext_out;
+   }
+
+   /* Send our own NAND_CMD_PARAM. */
+   chip->cmdfunc(mtd, NAND_CMD_PARAM, 0, -1);
+
+   /*
+* Use the Change Read Columm command to skip the ONFI Parameter Pages.
+* The nand_command() does not support the Change Read Columm command,
+* use the nand_command_lp() instead.
+*/
+   nand_command_lp(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_size = 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.
  */
@@ -2914,6 +2981,11 @@ 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_size = 512;
+   } else if (chip->onfi_version >= 21 &&
+   (onfi_feature(chip) & ONFI_FEATURE_EXT_PARAM_PAGE)) {
+   /* 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


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH V3 4/9] mtd: get the ECC info from the Extended Parameter Page

2013-04-23 Thread Huang Shijie
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.

Signed-off-by: Huang Shijie b32...@freescale.com
---
 drivers/mtd/nand/nand_base.c |   72 ++
 1 files changed, 72 insertions(+), 0 deletions(-)

diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index beff911..584cb0f 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -2846,6 +2846,73 @@ 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) {
+   pr_debug(Out of memory.\n);
+   ret = -ENOMEM;
+   goto ext_out;
+   }
+
+   /* Send our own NAND_CMD_PARAM. */
+   chip-cmdfunc(mtd, NAND_CMD_PARAM, 0, -1);
+
+   /*
+* Use the Change Read Columm command to skip the ONFI Parameter Pages.
+* The nand_command() does not support the Change Read Columm command,
+* use the nand_command_lp() instead.
+*/
+   nand_command_lp(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_size = 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.
  */
@@ -2914,6 +2981,11 @@ 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_size = 512;
+   } else if (chip-onfi_version = 21 
+   (onfi_feature(chip)  ONFI_FEATURE_EXT_PARAM_PAGE)) {
+   /* 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


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/