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

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

[Bean Huo]

From: bean huo <beanhuo@outlook.com>

For some Norflashes,the size of the buffer program has been
increased from 256 bytes to 512 bytes,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.If haven't probed this information,or smaller than 2000us,then
specify a defualt value 2000us.

Tested with Micron JS28F512M29EWx and Micron MT28EW512ABA flash devices.

Signed-off-by: bean huo <beanhuo@outlook.com>
---
	v1->v2:Deleted unused parameters in this patch(word_write_time_max and erase_time_max).
	Using usecs_to_jiffies instead of msecs_to_jiffies for convert timeout value into jiffies.
	v2->v3:Removed unnecessary messages form comments and deleted trailing whitespace.
	V3->v4:If haven't probed timeout information,or smaller than 2000us,specify default 2000us.
	v4->v5:Add space before '>='
	v5->v6:Add the same indentation as 'if' at the front of 'else'

 drivers/mtd/chips/cfi_cmdset_0002.c |   29 +++++++++++++++++++++++++++--
 1 file changed, 27 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c b/drivers/mtd/chips/cfi_cmdset_0002.c
index e21fde9..aed6f9e 100644
--- a/drivers/mtd/chips/cfi_cmdset_0002.c
+++ b/drivers/mtd/chips/cfi_cmdset_0002.c
@@ -628,6 +628,25 @@ 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;
+		/*
+		 * 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,or smaller than
+		 * 2000us,we will specify defualt value 2000us,and the time
+		 * unit is us.
+		 */
+		if (cfi->cfiq->BufWriteTimeoutTyp &&
+		    cfi->cfiq->BufWriteTimeoutMax)
+			cfi->chips[i].buffer_write_time_max =
+				1<<(cfi->cfiq->BufWriteTimeoutTyp +
+				    cfi->cfiq->BufWriteTimeoutMax);
+		else
+			cfi->chips[i].buffer_write_time_max = 0;
+
+		cfi->chips[i].buffer_write_time_max =
+			((cfi->chips[i].buffer_write_time_max >= 2000)
+			 ? cfi->chips[i].buffer_write_time_max : 2000);
+
 		cfi->chips[i].ref_point_counter = 0;
 		init_waitqueue_head(&(cfi->chips[i].wq));
 	}
@@ -1462,8 +1481,14 @@ 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;
+	/*
+	 * Use the result that calculated according to timeout field
+	 * of struct cfi_ident that probed from norflash's CFI aera.
+	 * See more comments in cfi_cmdset_0002().uWriteTimeout is
+	 * used for timeout step,it must be concerted into jiffies.
+	 */
+	unsigned long uWriteTimeout =
+				usecs_to_jiffies(chip->buffer_write_time_max);
 	int ret = -EIO;
 	unsigned long cmd_adr;
 	int z, words;
-- 
1.7.9.5

Re: [PATCH v6] mtd:nor:timeout:fix do_write_buffer() timeout error

[Brian Norris]

Hi Bean,

On Wed, Jul 16, 2014 at 09:05:26AM -0700, Bean Huo wrote:
> From: bean huo <beanhuo@outlook.com>
> 
> For some Norflashes,the size of the buffer program has been
> increased from 256 bytes to 512 bytes,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.If haven't probed this information,or smaller than 2000us,then
> specify a defualt value 2000us.
> 
> Tested with Micron JS28F512M29EWx and Micron MT28EW512ABA flash devices.
> 
> Signed-off-by: bean huo <beanhuo@outlook.com>
> ---
> 	v1->v2:Deleted unused parameters in this patch(word_write_time_max and erase_time_max).
> 	Using usecs_to_jiffies instead of msecs_to_jiffies for convert timeout value into jiffies.
> 	v2->v3:Removed unnecessary messages form comments and deleted trailing whitespace.
> 	V3->v4:If haven't probed timeout information,or smaller than 2000us,specify default 2000us.
> 	v4->v5:Add space before '>='
> 	v5->v6:Add the same indentation as 'if' at the front of 'else'
> 
>  drivers/mtd/chips/cfi_cmdset_0002.c |   29 +++++++++++++++++++++++++++--
>  1 file changed, 27 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c b/drivers/mtd/chips/cfi_cmdset_0002.c
> index e21fde9..aed6f9e 100644
> --- a/drivers/mtd/chips/cfi_cmdset_0002.c
> +++ b/drivers/mtd/chips/cfi_cmdset_0002.c
> @@ -628,6 +628,25 @@ struct mtd_info *cfi_cmdset_0002(struct map_info *map, int primary)
[...]
> +		cfi->chips[i].buffer_write_time_max =
> +			((cfi->chips[i].buffer_write_time_max >= 2000)
> +			 ? cfi->chips[i].buffer_write_time_max : 2000);

This should just use the 'max()' macro, from <linux/kernel.h>.

> +
>  		cfi->chips[i].ref_point_counter = 0;
>  		init_waitqueue_head(&(cfi->chips[i].wq));
>  	}

I have just fixed up your patch myself, to use max() and to improve some
of the language. Let me know if the below works for you.

>From 6534e6809e6c9d48114b537afe03bee3fd33bf01 Mon Sep 17 00:00:00 2001
From: Bean Huo <beanhuo@outlook.com>
Date: Wed, 16 Jul 2014 09:05:26 -0700
Subject: [PATCH] mtd: cfi_cmdset_0002: fix do_write_buffer() timeout error

For some NOR flashes, the size of the buffer program has been increased
from 256 bytes to 512 bytes, and so 2ms maximum timeout can may not be
sufficient for all different vendor's NOR flash. There is maximum
timeout information in the CFI area, so we instead of picking a fixed
value, we can calculate this according to the standard CFI parameters
parsed at probe time. If we haven't probed this information, or it is
smaller than 2000us, then specify a minimum value 2000us.

Tested with Micron JS28F512M29EWx and Micron MT28EW512ABA flash devices.

Signed-off-by: Bean Huo <beanhuo@outlook.com>
[Brian: fix up comments, use 'max()']
Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
 drivers/mtd/chips/cfi_cmdset_0002.c | 25 +++++++++++++++++++++++--
 1 file changed, 23 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c b/drivers/mtd/chips/cfi_cmdset_0002.c
index bf313be6ee26..5a4bfe33112a 100644
--- a/drivers/mtd/chips/cfi_cmdset_0002.c
+++ b/drivers/mtd/chips/cfi_cmdset_0002.c
@@ -645,6 +645,23 @@ 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;
+		/*
+		 * First calculate the timeout max according to timeout field
+		 * of struct cfi_ident that probed from chip's CFI aera, if
+		 * available. Specify a minimum of 2000us, in case the CFI data
+		 * is wrong.
+		 */
+		if (cfi->cfiq->BufWriteTimeoutTyp &&
+		    cfi->cfiq->BufWriteTimeoutMax)
+			cfi->chips[i].buffer_write_time_max =
+				1 << (cfi->cfiq->BufWriteTimeoutTyp +
+				      cfi->cfiq->BufWriteTimeoutMax);
+		else
+			cfi->chips[i].buffer_write_time_max = 0;
+
+		cfi->chips[i].buffer_write_time_max =
+			max(cfi->chips[i].buffer_write_time_max, 2000);
+
 		cfi->chips[i].ref_point_counter = 0;
 		init_waitqueue_head(&(cfi->chips[i].wq));
 	}
@@ -1774,8 +1791,12 @@ 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;
+	/*
+	 * Timeout is calculated according to CFI data, if available.
+	 * See more comments in cfi_cmdset_0002().
+	 */
+	unsigned long uWriteTimeout =
+				usecs_to_jiffies(chip->buffer_write_time_max);
 	int ret = -EIO;
 	unsigned long cmd_adr;
 	int z, words;
-- 
1.9.1

RE: [PATCH v6] mtd:nor:timeout:fix do_write_buffer() timeout error

[Bean Huo]

> Hi Bean,

>> + cfi->chips[i].buffer_write_time_max =
>> + ((cfi->chips[i].buffer_write_time_max>= 2000)
>> + ? cfi->chips[i].buffer_write_time_max : 2000);
>
> This should just use the 'max()' macro, from <linux/kernel.h>.

> I have just fixed up your patch myself, to use max() and to improve some
> of the language. Let me know if the below works for you.

hi,Brian
 Thanks once again.I test and it works Ok.It can be accepted now? 		 	   		  

Re: [PATCH v6] mtd:nor:timeout:fix do_write_buffer() timeout error

[Brian Norris]

On Thu, Jul 17, 2014 at 12:54:17AM +0000, Bean Huo wrote:
> > I have just fixed up your patch myself, to use max() and to improve some
> > of the language. Let me know if the below works for you.
> 
> hi,Brian
>  Thanks once again.I test and it works Ok.It can be accepted now? 		 	   		  

Yes. Pushed to l2-mtd.git.

Please be more diligent next time to follow CodingStyle and email
submission practices next time. Even if your company email gives
trouble, I'm sure you can find some way to send patches properly. For
instance, a quick search shows me that your outlook.com email supports
sending email via SMTP:

  http://windows.microsoft.com/en-us/windows/outlook/send-receive-from-app

That means you can, for instance, use git-send-email to send patches.

Brian

RE: [PATCH v6] mtd:nor:timeout:fix do_write_buffer() timeout error

[Gupta, Pekon]

>From: Brian Norris
>>On Thu, Jul 17, 2014 at 12:54:17AM +0000, Bean Huo wrote:
>> > I have just fixed up your patch myself, to use max() and to improve some
>> > of the language. Let me know if the below works for you.
>>
>> hi,Brian
>>  Thanks once again.I test and it works Ok.It can be accepted now?
>
>Yes. Pushed to l2-mtd.git.
>
This works well with Spansion NOR device (S29GL512S10) present on
J6-EVM, using http://patchwork.ozlabs.org/patch/372866/.

Though it's already accepted in l2-mtd.git, but just for record.
Tested-by: Pekon Gupta <pekon@ti.com>


with regards, pekon