All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/1] mtd:nor:timeout:fix do_write_buffer() timeout error
@ 2014-05-19  2:41 ` Bean Huo 霍斌斌 (beanhuo)
  0 siblings, 0 replies; 10+ messages in thread
From: Bean Huo 霍斌斌 (beanhuo) @ 2014-05-19  2:41 UTC (permalink / raw)
  To: David Woodhouse, Brian Norris
  Cc: Artem Bityutskiy, Jingoo Han, Paul Gortmaker, Christian Riesch,
	Kees Cook, linux-mtd, linux-kernel

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@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;
-	/* 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;
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 1/1] mtd:nor:timeout:fix do_write_buffer() timeout error
@ 2014-05-19  2:41 ` Bean Huo 霍斌斌 (beanhuo)
  0 siblings, 0 replies; 10+ messages in thread
From: Bean Huo 霍斌斌 (beanhuo) @ 2014-05-19  2:41 UTC (permalink / raw)
  To: David Woodhouse, Brian Norris
  Cc: Kees Cook, Artem Bityutskiy, Jingoo Han, linux-kernel,
	Paul Gortmaker, linux-mtd, Christian Riesch

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@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;
-	/* 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;
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/1] mtd:nor:timeout:fix do_write_buffer() timeout error
  2014-05-19  2:41 ` Bean Huo 霍斌斌 (beanhuo)
  (?)
@ 2014-05-19  4:45 ` Alexander Shiyan
  2014-05-20  6:08   ` Bean Huo 霍斌斌 (beanhuo)
                     ` (2 more replies)
  -1 siblings, 3 replies; 10+ messages in thread
From: Alexander Shiyan @ 2014-05-19  4:45 UTC (permalink / raw)
  To: Bean Huo 霍斌斌 (beanhuo)
  Cc: Kees Cook, Artem Bityutskiy, Jingoo Han, Paul Gortmaker,
	linux-mtd, Brian Norris, Christian Riesch, David Woodhouse

Mon, 19 May 2014 02:41:29 +0000 от Bean Huo 霍斌斌 (beanhuo) <beanhuo@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@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().

---


^ permalink raw reply	[flat|nested] 10+ messages in thread

* RE: [PATCH 1/1] mtd:nor:timeout:fix do_write_buffer() timeout error
  2014-05-19  4:45 ` Alexander Shiyan
@ 2014-05-20  6:08   ` Bean Huo 霍斌斌 (beanhuo)
  2014-05-20  6:17   ` Bean Huo 霍斌斌 (beanhuo)
  2014-05-26  1:01   ` Bean Huo 霍斌斌 (beanhuo)
  2 siblings, 0 replies; 10+ messages in thread
From: Bean Huo 霍斌斌 (beanhuo) @ 2014-05-20  6:08 UTC (permalink / raw)
  To: Alexander Shiyan
  Cc: Kees Cook, Artem Bityutskiy, Jingoo Han, Paul Gortmaker,
	linux-mtd, Brian Norris, Christian Riesch, David Woodhouse

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@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@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@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@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@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().

---


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* RE: [PATCH 1/1] mtd:nor:timeout:fix do_write_buffer() timeout error
  2014-05-19  4:45 ` Alexander Shiyan
  2014-05-20  6:08   ` Bean Huo 霍斌斌 (beanhuo)
@ 2014-05-20  6:17   ` Bean Huo 霍斌斌 (beanhuo)
  2014-05-26  1:01   ` Bean Huo 霍斌斌 (beanhuo)
  2 siblings, 0 replies; 10+ messages in thread
From: Bean Huo 霍斌斌 (beanhuo) @ 2014-05-20  6:17 UTC (permalink / raw)
  To: Alexander Shiyan
  Cc: Kees Cook, Artem Bityutskiy, Jingoo Han, Paul Gortmaker,
	linux-mtd, Brian Norris, Christian Riesch, David Woodhouse,
	Bean Huo 霍斌斌 (beanhuo)

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@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@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@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@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@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().

---


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* RE: [PATCH 1/1] mtd:nor:timeout:fix do_write_buffer() timeout error
  2014-05-19  4:45 ` Alexander Shiyan
  2014-05-20  6:08   ` Bean Huo 霍斌斌 (beanhuo)
  2014-05-20  6:17   ` Bean Huo 霍斌斌 (beanhuo)
@ 2014-05-26  1:01   ` Bean Huo 霍斌斌 (beanhuo)
  2014-06-02 10:25     ` Christian Riesch
  2 siblings, 1 reply; 10+ messages in thread
From: Bean Huo 霍斌斌 (beanhuo) @ 2014-05-26  1:01 UTC (permalink / raw)
  To: David Woodhouse, Brian Norris, Alexander Shiyan
  Cc: Christian Riesch, Paul Gortmaker, Jingoo Han, linux-mtd, Kees Cook

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@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



-----Original Message-----
From: Bean Huo 霍斌斌 (beanhuo) 
Sent: Tuesday, May 20, 2014 2:17 PM
To: 'Alexander Shiyan'
Cc: 'Kees Cook'; 'Artem Bityutskiy'; 'Jingoo Han'; 'Paul Gortmaker'; 'linux-mtd@lists.infradead.org'; 'Christian Riesch'; 'David Woodhouse'; 'Brian Norris'; Bean Huo 霍斌斌 (beanhuo)
Subject: RE: [PATCH 1/1] mtd:nor:timeout:fix do_write_buffer() timeout error

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@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@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@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@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@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().

---


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/1] mtd:nor:timeout:fix do_write_buffer() timeout error
  2014-05-26  1:01   ` Bean Huo 霍斌斌 (beanhuo)
@ 2014-06-02 10:25     ` Christian Riesch
  2014-06-03  6:26       ` Bean Huo 霍斌斌 (beanhuo)
  0 siblings, 1 reply; 10+ messages in thread
From: Christian Riesch @ 2014-06-02 10:25 UTC (permalink / raw)
  To: Bean Huo 霍斌斌 (beanhuo)
  Cc: Kees Cook, Alexander Shiyan, Jingoo Han, Paul Gortmaker,
	linux-mtd, Brian Norris, David Woodhouse

Hi,
When you send an updated version of a patch, please put version
numbers into the subject, e.g. [PATCH v2], [PATCH v3]..., and include
a changelog. And I think you shouldn't send the new versions as a
reply to an earlier mail, send them as a new post instead.

On Mon, May 26, 2014 at 3:01 AM, Bean Huo 霍斌斌 (beanhuo)
<beanhuo@micron.com> wrote:
> 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.

What kind of device is the MT28EW512ABA? Aren't MT28 devices
StrataFlashes that use the cfi_cmdset_0001.c driver?

>
> Signed-off-by: bean huo <beanhuo@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;
> +                               }

You are setting several parameters here, but you only use
buffer_write_time_max. Why?

>                 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);

How about using usecs_to_jiffies below? Then you wouldn't need this
conversion, right?

>         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);
>

A few lines below in the loop timeo is modified again, timeo = jiffies
+ (HZ / 2); This line has a big /* FIXME */ comment. Any ideas what
should be fixed here?

Best regards,
Christian

>         for (;;) {
>                 if (chip->state != FL_WRITING) {
> --
> 1.7.9.5
>
>
>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* RE: [PATCH 1/1] mtd:nor:timeout:fix do_write_buffer() timeout error
  2014-06-02 10:25     ` Christian Riesch
@ 2014-06-03  6:26       ` Bean Huo 霍斌斌 (beanhuo)
  2014-06-03  7:50         ` Christian Riesch
  0 siblings, 1 reply; 10+ messages in thread
From: Bean Huo 霍斌斌 (beanhuo) @ 2014-06-03  6:26 UTC (permalink / raw)
  To: Christian Riesch
  Cc: Kees Cook, Alexander Shiyan, Jingoo Han, Paul Gortmaker,
	linux-mtd, Brian Norris, David Woodhouse

Hi,Christian 

1、MT28EW512ABA is Micron's parallel Norflash that use the cfi_cmdset_0002.c driver.

2、yes,I am setting several parameters here(word_write_time_max、buffer_write_time_max、erase_time_max).For this patch,I only used buffer_write_time_max parameter.Because this patch is mainly focus on fix do_write_buffer() timeout error. If this patch is accepted, I will submit another patch that for fix single word program timeout error and erase timeout error.thanks.

3、The time unit of uWriteTimeout is ms. In order to achieve large rather than small timeout ,so using following conversion,

> +       unsigned long uWriteTimeout = (chip->buffer_write_time_max % 1000) ?
> +                       (chip->buffer_write_time_max / 1000 + 1) :
> +                       (chip->buffer_write_time_max / 1000);

4、For the following code,I now have some ideas,but having not test and verify now.If it test Ok later,I will submit it.
  timeo = jiffies+ (HZ / 2); This line has a big /* FIXME */

-----Original Message-----
From: christian.riesch@gmail.com [mailto:christian.riesch@gmail.com] On Behalf Of Christian Riesch
Sent: Monday, June 02, 2014 6:26 PM
To: Bean Huo 霍斌斌 (beanhuo)
Cc: David Woodhouse; Brian Norris; Alexander Shiyan; Paul Gortmaker; Jingoo Han; linux-mtd@lists.infradead.org; Kees Cook
Subject: Re: [PATCH 1/1] mtd:nor:timeout:fix do_write_buffer() timeout error

Hi,
When you send an updated version of a patch, please put version numbers into the subject, e.g. [PATCH v2], [PATCH v3]..., and include a changelog. And I think you shouldn't send the new versions as a reply to an earlier mail, send them as a new post instead.

On Mon, May 26, 2014 at 3:01 AM, Bean Huo 霍斌斌 (beanhuo) <beanhuo@micron.com> wrote:
> 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.

What kind of device is the MT28EW512ABA? Aren't MT28 devices StrataFlashes that use the cfi_cmdset_0001.c driver?

>
> Signed-off-by: bean huo <beanhuo@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.
> 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;
> +                               }

You are setting several parameters here, but you only use buffer_write_time_max. Why?

>                 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);

How about using usecs_to_jiffies below? Then you wouldn't need this conversion, right?

>         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);
>

A few lines below in the loop timeo is modified again, timeo = jiffies
+ (HZ / 2); This line has a big /* FIXME */ comment. Any ideas what
should be fixed here?

Best regards,
Christian

>         for (;;) {
>                 if (chip->state != FL_WRITING) {
> --
> 1.7.9.5
>
>
>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/1] mtd:nor:timeout:fix do_write_buffer() timeout error
  2014-06-03  6:26       ` Bean Huo 霍斌斌 (beanhuo)
@ 2014-06-03  7:50         ` Christian Riesch
  2014-06-03  9:30           ` Bean Huo 霍斌斌 (beanhuo)
  0 siblings, 1 reply; 10+ messages in thread
From: Christian Riesch @ 2014-06-03  7:50 UTC (permalink / raw)
  To: Bean Huo 霍斌斌 (beanhuo)
  Cc: Kees Cook, Alexander Shiyan, Jingoo Han, Paul Gortmaker,
	linux-mtd, Brian Norris, David Woodhouse

Hi,
Please do not top post.

On Tue, Jun 3, 2014 at 8:26 AM, Bean Huo 霍斌斌 (beanhuo)
<beanhuo@micron.com> wrote:
> Hi,Christian
>
> 1、MT28EW512ABA is Micron's parallel Norflash that use the cfi_cmdset_0002.c driver.

Is it a new device? I can't find it on the Micron web site. If it is
already there, could you please provide a link to the data sheet?

>
> 2、yes,I am setting several parameters here(word_write_time_max、buffer_write_time_max、erase_time_max).For this patch,I only used buffer_write_time_max parameter.Because this patch is mainly focus on fix do_write_buffer() timeout error. If this patch is accepted, I will submit another patch that for fix single word program timeout error and erase timeout error.thanks.
>

Ok. IMHO you should only set the parameters that you are actually
using (and add word_write_time_max, erase_time_max etc later with the
patches that actually use them). Otherwise we could end with lots of
unused code if your later patches are not accepted. But I guess that's
up to the maintainer to decide.

> 3、The time unit of uWriteTimeout is ms. In order to achieve large rather than small timeout ,so using following conversion,
>
>> +       unsigned long uWriteTimeout = (chip->buffer_write_time_max % 1000) ?
>> +                       (chip->buffer_write_time_max / 1000 + 1) :
>> +                       (chip->buffer_write_time_max / 1000);

But I think the result of usecs_to_jiffies would be the same, or am I wrong?

If I understood the code of usecs_to_jiffies correctly, it returns at
least one jiffy if the specified time is less than a jiffy.

>
> 4、For the following code,I now have some ideas,but having not test and verify now.If it test Ok later,I will submit it.
>   timeo = jiffies+ (HZ / 2); This line has a big /* FIXME */

Ok, thanks.
Christian

>
> -----Original Message-----
> From: christian.riesch@gmail.com [mailto:christian.riesch@gmail.com] On Behalf Of Christian Riesch
> Sent: Monday, June 02, 2014 6:26 PM
> To: Bean Huo 霍斌斌 (beanhuo)
> Cc: David Woodhouse; Brian Norris; Alexander Shiyan; Paul Gortmaker; Jingoo Han; linux-mtd@lists.infradead.org; Kees Cook
> Subject: Re: [PATCH 1/1] mtd:nor:timeout:fix do_write_buffer() timeout error
>
> Hi,
> When you send an updated version of a patch, please put version numbers into the subject, e.g. [PATCH v2], [PATCH v3]..., and include a changelog. And I think you shouldn't send the new versions as a reply to an earlier mail, send them as a new post instead.
>
> On Mon, May 26, 2014 at 3:01 AM, Bean Huo 霍斌斌 (beanhuo) <beanhuo@micron.com> wrote:
>> 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.
>
> What kind of device is the MT28EW512ABA? Aren't MT28 devices StrataFlashes that use the cfi_cmdset_0001.c driver?
>
>>
>> Signed-off-by: bean huo <beanhuo@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.
>> 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;
>> +                               }
>
> You are setting several parameters here, but you only use buffer_write_time_max. Why?
>
>>                 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);
>
> How about using usecs_to_jiffies below? Then you wouldn't need this conversion, right?
>
>>         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);
>>
>
> A few lines below in the loop timeo is modified again, timeo = jiffies
> + (HZ / 2); This line has a big /* FIXME */ comment. Any ideas what
> should be fixed here?
>
> Best regards,
> Christian
>
>>         for (;;) {
>>                 if (chip->state != FL_WRITING) {
>> --
>> 1.7.9.5
>>
>>
>>
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply	[flat|nested] 10+ messages in thread

* RE: [PATCH 1/1] mtd:nor:timeout:fix do_write_buffer() timeout error
  2014-06-03  7:50         ` Christian Riesch
@ 2014-06-03  9:30           ` Bean Huo 霍斌斌 (beanhuo)
  0 siblings, 0 replies; 10+ messages in thread
From: Bean Huo 霍斌斌 (beanhuo) @ 2014-06-03  9:30 UTC (permalink / raw)
  To: Christian Riesch
  Cc: Kees Cook, Alexander Shiyan, Jingoo Han, Paul Gortmaker,
	linux-mtd, Brian Norris, David Woodhouse


Hi,Christian
>> 
>>
>> 1、MT28EW512ABA is Micron's parallel Norflash that use the cfi_cmdset_0002.c driver.

>Is it a new device? I can't find it on the Micron web site. If it is already there, could you please provide a link to the data sheet?

Yes,it is a new device that release this year.You must register an account at Micron web,at the same time, this account must be approved by micron, then you can find it.
>>
>> 2、yes,I am setting several parameters here(word_write_time_max、buffer_write_time_max、erase_time_max).For this patch,I only used buffer_write_time_max parameter.Because this patch is mainly focus on fix do_write_buffer() timeout error. If this patch is accepted, I will submit another patch that for fix single word program timeout error and erase timeout error.thanks.
>

>Ok. IMHO you should only set the parameters that you are actually using (and add word_write_time_max, erase_time_max etc later with the patches that actually use them). Otherwise we could end with lots of unused code if your later patches are not accepted. But I guess that's up to the maintainer to decide.

Ok,thanks for your advice,I will modify it and then submit patch V2

>> 3、The time unit of uWriteTimeout is ms. In order to achieve large 
>> rather than small timeout ,so using following conversion,
>>
>>> +       unsigned long uWriteTimeout = (chip->buffer_write_time_max % 1000) ?
>>> +                       (chip->buffer_write_time_max / 1000 + 1) :
>>> +                       (chip->buffer_write_time_max / 1000);

>But I think the result of usecs_to_jiffies would be the same, or am I wrong?

>If I understood the code of usecs_to_jiffies correctly, it returns at least one jiffy if the specified time is less than a jiffy.

Yes ,you are right.This will be modified in patch V2.

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2014-06-03  9:30 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-05-19  2:41 [PATCH 1/1] mtd:nor:timeout:fix do_write_buffer() timeout error Bean Huo 霍斌斌 (beanhuo)
2014-05-19  2:41 ` Bean Huo 霍斌斌 (beanhuo)
2014-05-19  4:45 ` Alexander Shiyan
2014-05-20  6:08   ` Bean Huo 霍斌斌 (beanhuo)
2014-05-20  6:17   ` Bean Huo 霍斌斌 (beanhuo)
2014-05-26  1:01   ` Bean Huo 霍斌斌 (beanhuo)
2014-06-02 10:25     ` Christian Riesch
2014-06-03  6:26       ` Bean Huo 霍斌斌 (beanhuo)
2014-06-03  7:50         ` Christian Riesch
2014-06-03  9:30           ` Bean Huo 霍斌斌 (beanhuo)

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.