[PATCH 1/1] mtd:nor:timeout:fix do_write_buffer() timeout error

Bean Huo 霍斌斌 (beanhuo) beanhuo at micron.com
Mon May 19 23:08:20 PDT 2014


The size of the buffer program has been increased from 256 to 512 ,
2ms maximum timeout can not adapt to all the different vendor's norflash,
There maximum timeout information in the CFI area,so the best way is to
choose the result calculated according to timeout field of struct cfi_ident
that probed from norflash's CFI aera.This is also a standard defined by CFI.

Without this change, if the size of buffer program is 512 or bigger than 256,
due to timeout is the shorter than that the chip required,do_write_buffer
sometimes fails.

Tested with Micron JS28F512M29EWx and Micron MT28EW512ABA flash devices.

Signed-off-by: bean huo <beanhuo at micron.com>
---
 drivers/mtd/chips/cfi_cmdset_0002.c |   49 ++++++++++++++++++++++++++++++++---
 1 file changed, 45 insertions(+), 4 deletions(-)

diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c b/drivers/mtd/chips/cfi_cmdset_0002.c
index e21fde9..87d446a 100644
--- a/drivers/mtd/chips/cfi_cmdset_0002.c
+++ b/drivers/mtd/chips/cfi_cmdset_0002.c
@@ -628,10 +628,42 @@ struct mtd_info *cfi_cmdset_0002(struct map_info *map, int primary)
 		cfi->chips[i].word_write_time = 1<<cfi->cfiq->WordWriteTimeoutTyp;
 		cfi->chips[i].buffer_write_time = 1<<cfi->cfiq->BufWriteTimeoutTyp;
 		cfi->chips[i].erase_time = 1<<cfi->cfiq->BlockEraseTimeoutTyp;
+		/*
+		 * We first calculate the timeout max according to timeout
+		 * field of struct cfi_ident that probed from chip's CFI
+		 * aera,If haven't probed this information,we will specify
+		 * a default value,and the time unit is us.
+		 */
+		if (cfi->cfiq->WordWriteTimeoutTyp &&
+			cfi->cfiq->WordWriteTimeoutMax){
+			cfi->chips[i].word_write_time_max =
+				1<<(cfi->cfiq->WordWriteTimeoutTyp +
+					cfi->cfiq->WordWriteTimeoutMax);
+			} else {
+		/* specify maximum timeout for byte/word program 2000us */
+				cfi->chips[i].word_write_time_max = 2000;
+				}
+		if (cfi->cfiq->BufWriteTimeoutTyp &&
+			cfi->cfiq->BufWriteTimeoutMax){
+			cfi->chips[i].buffer_write_time_max =
+				1<<(cfi->cfiq->BufWriteTimeoutTyp +
+					cfi->cfiq->BufWriteTimeoutMax);
+			} else {
+		/* specify maximum timeout for buffer program 2000us */
+				cfi->chips[i].buffer_write_time_max = 2000;
+				}
+		if (cfi->cfiq->BlockEraseTimeoutTyp &&
+			cfi->cfiq->BlockEraseTimeoutMax){
+			cfi->chips[i].erase_time_max =
+				1<<(cfi->cfiq->BlockEraseTimeoutTyp +
+					cfi->cfiq->BlockEraseTimeoutMax);
+			} else {
+	/* specify maximum timeout per individual block erase 3000000us */
+				cfi->chips[i].erase_time_max = 3000000;
+				}
 		cfi->chips[i].ref_point_counter = 0;
 		init_waitqueue_head(&(cfi->chips[i].wq));
 	}
-
 	map->fldrv = &cfi_amdstd_chipdrv;
 
 	return cfi_amdstd_setup(mtd);
@@ -1462,8 +1494,16 @@ static int __xipram do_write_buffer(struct map_info *map, struct flchip *chip,
 {
 	struct cfi_private *cfi = map->fldrv_priv;
 	unsigned long timeo = jiffies + HZ;
-	/* see comments in do_write_oneword() regarding uWriteTimeo. */
-	unsigned long uWriteTimeout = ( HZ / 1000 ) + 1;
+	/* The size of the buffer program has been increased from 256 to 512,
+	 * 2ms maximum timeout can not adapt to all the different vendor's
+	 * norflash.There maximum timeout information in the CFI area,so the
+	 * best way is to choose the result calculated according to timeout
+	 * field of struct cfi_ident that probed from norflash's CFI aera,see
+	 * comments in cfi_cmdset_0002().The time unit of uWriteTimeout is ms.
+	 */
+	unsigned long uWriteTimeout = (chip->buffer_write_time_max % 1000) ?
+			(chip->buffer_write_time_max / 1000 + 1) :
+			(chip->buffer_write_time_max / 1000);
 	int ret = -EIO;
 	unsigned long cmd_adr;
 	int z, words;
@@ -1520,7 +1560,8 @@ static int __xipram do_write_buffer(struct map_info *map, struct flchip *chip,
 				adr, map_bankwidth(map),
 				chip->word_write_time);
 
-	timeo = jiffies + uWriteTimeout;
+	/*Maximum timeout must be converted into jiffies */
+	timeo = jiffies + msecs_to_jiffies(uWriteTimeout);
 
 	for (;;) {
 		if (chip->state != FL_WRITING) {
-- 
1.7.9.5

Thanks your suggest.Here is new patch for it. Can accept this patch?for other timeout issue in cfi_cmdset_0002.C, I will submit them one by one laterly.

-----Original Message-----
From: Alexander Shiyan [mailto:shc_work at mail.ru] 
Sent: Monday, May 19, 2014 12:45 PM
To: Bean Huo 霍斌斌 (beanhuo)
Cc: Kees Cook; Artem Bityutskiy; Jingoo Han; Paul Gortmaker; linux-mtd at lists.infradead.org; Christian Riesch; David Woodhouse; Brian Norris
Subject: Re: [PATCH 1/1] mtd:nor:timeout:fix do_write_buffer() timeout error

Mon, 19 May 2014 02:41:29 +0000 от Bean Huo 霍斌斌 (beanhuo) <beanhuo at micron.com>:
> The size of the buffer program has been increased from 256 to 512 , 
> 2ms maximum timeout can not adapt to all the different vendor's 
> norflash, There maximum timeout information in the CFI area,so the 
> best way is to choose the result calculated according to timeout field 
> of struct cfi_ident that probed from norflash's CFI aera.This is also a standard defined by CFI.
> 
> Without this change, if the size of buffer program is 512 or bigger 
> than 256, due to timeout is the shorter than that the chip 
> required,do_write_buffer sometimes fails.
> 
> Tested with Micron JS28F512M29EWx and Micron MT28EW512ABA flash devices.
> 
> Signed-off-by: bean huo <beanhuo at micron.com>
> ---
>  drivers/mtd/chips/cfi_cmdset_0002.c |   46 ++++++++++++++++++++++++++++++++---
>  1 file changed, 43 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c 
> b/drivers/mtd/chips/cfi_cmdset_0002.c
> index e21fde9..2a08f9f 100644
> --- a/drivers/mtd/chips/cfi_cmdset_0002.c
> +++ b/drivers/mtd/chips/cfi_cmdset_0002.c
> @@ -628,10 +628,42 @@ struct mtd_info *cfi_cmdset_0002(struct map_info *map, int primary)
>  		cfi->chips[i].word_write_time = 1<<cfi->cfiq->WordWriteTimeoutTyp;
>  		cfi->chips[i].buffer_write_time = 1<<cfi->cfiq->BufWriteTimeoutTyp;
>  		cfi->chips[i].erase_time = 1<<cfi->cfiq->BlockEraseTimeoutTyp;
> +		/*
> +		 * We first calculate the timeout max according to timeout
> +		 * field of struct cfi_ident that probed from chip's CFI
> +		 * aera,If haven't probed this information,we will specify
> +		 * a default value,and the time unit is us.
> +		 */
> +		if (cfi->cfiq->WordWriteTimeoutTyp &&
> +			cfi->cfiq->WordWriteTimeoutMax){
> +			cfi->chips[i].word_write_time_max =
> +				1<<(cfi->cfiq->WordWriteTimeoutTyp +
> +					cfi->cfiq->WordWriteTimeoutMax);
> +			} else {
> +		/* specify maximum timeout for byte/word program 2000us */
> +				cfi->chips[i].word_write_time_max = 2000;
> +				}
> +		if (cfi->cfiq->BufWriteTimeoutTyp &&
> +			cfi->cfiq->BufWriteTimeoutMax){
> +			cfi->chips[i].buffer_write_time_max =
> +				1<<(cfi->cfiq->BufWriteTimeoutTyp +
> +					cfi->cfiq->BufWriteTimeoutMax);
> +			} else {
> +		/* specify maximum timeout for buffer program 2000us */
> +				cfi->chips[i].buffer_write_time_max = 2000;
> +				}
> +		if (cfi->cfiq->BlockEraseTimeoutTyp &&
> +			cfi->cfiq->BlockEraseTimeoutMax){
> +			cfi->chips[i].erase_time_max =
> +				1<<(cfi->cfiq->BlockEraseTimeoutTyp +
> +					cfi->cfiq->BlockEraseTimeoutMax);
> +			} else {
> +	/* specify maximum timeout per individual block erase 3000000us */
> +				cfi->chips[i].erase_time_max = 3000000;
> +				}
>  		cfi->chips[i].ref_point_counter = 0;
>  		init_waitqueue_head(&(cfi->chips[i].wq));
>  	}
> -
>  	map->fldrv = &cfi_amdstd_chipdrv;
>  
>  	return cfi_amdstd_setup(mtd);
> @@ -1462,8 +1494,16 @@ static int __xipram do_write_buffer(struct 
> map_info *map, struct flchip *chip,  {
>  	struct cfi_private *cfi = map->fldrv_priv;
>  	unsigned long timeo = jiffies + HZ;

I want to suggest one stuff not related to this patch.
On my opinion, all occurrences of HZ must be converted into msecs_to_jiffies().

---



More information about the linux-mtd mailing list