All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3] mtd:nor:timeout:fix do_write_buffer() timeout error
@ 2014-07-03  0:40 ` Bean Huo 霍斌斌 (beanhuo)
  0 siblings, 0 replies; 24+ messages in thread
From: Bean Huo 霍斌斌 (beanhuo) @ 2014-07-03  0:40 UTC (permalink / raw)
  To: computersforpeace; +Cc: linux-mtd, linux-kernel, Jingoo Han, Artem Bityutskiy

The size of the buffer program has been increased from 256 to 512 , 2ms maximum timeout for do_write_buffer 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>
---
changes 
	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.

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

diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c b/drivers/mtd/chips/cfi_cmdset_0002.c
index e21fde9..f7098d6 100644
--- a/drivers/mtd/chips/cfi_cmdset_0002.c
+++ b/drivers/mtd/chips/cfi_cmdset_0002.c
@@ -628,10 +628,24 @@ 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->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;
+				}
 		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 +1476,15 @@ 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;
+	/* 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().
+	 * 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

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

* [PATCH v3] mtd:nor:timeout:fix do_write_buffer() timeout error
@ 2014-07-03  0:40 ` Bean Huo 霍斌斌 (beanhuo)
  0 siblings, 0 replies; 24+ messages in thread
From: Bean Huo 霍斌斌 (beanhuo) @ 2014-07-03  0:40 UTC (permalink / raw)
  To: computersforpeace; +Cc: Artem Bityutskiy, Jingoo Han, linux-mtd, linux-kernel

The size of the buffer program has been increased from 256 to 512 , 2ms maximum timeout for do_write_buffer 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>
---
changes 
	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.

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

diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c b/drivers/mtd/chips/cfi_cmdset_0002.c
index e21fde9..f7098d6 100644
--- a/drivers/mtd/chips/cfi_cmdset_0002.c
+++ b/drivers/mtd/chips/cfi_cmdset_0002.c
@@ -628,10 +628,24 @@ 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->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;
+				}
 		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 +1476,15 @@ 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;
+	/* 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().
+	 * 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

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

* RE: [PATCH v3] mtd:nor:timeout:fix do_write_buffer() timeout error
  2014-07-15  7:28                 ` Brian Norris
@ 2014-07-16  6:22                   ` Bean Huo
  0 siblings, 0 replies; 24+ messages in thread
From: Bean Huo @ 2014-07-16  6:22 UTC (permalink / raw)
  To: Brian Norris
  Cc: christian.riesch, paul.gortmaker, dwmw2, linux-mtd, Stijn Devriendt



>> For this patch,i will do some changes,please every maintainer and commit-signer check.
>
> Yes, please send a new version (version 4 now?). But please double check
> your patch submission, so that it applies correctly this time.
>
> Thanks,
> Brian
Thanks once again for your advice.Yes,I will send version 4 about this patch.and I have tested it. 		 	   		  

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

* Re: [PATCH v3] mtd:nor:timeout:fix do_write_buffer() timeout error
  2014-07-15  4:23               ` Bean Huo
@ 2014-07-15  7:28                 ` Brian Norris
  2014-07-16  6:22                   ` Bean Huo
  0 siblings, 1 reply; 24+ messages in thread
From: Brian Norris @ 2014-07-15  7:28 UTC (permalink / raw)
  To: Bean Huo
  Cc: christian.riesch, paul.gortmaker, dwmw2, linux-mtd, Stijn Devriendt

Hi Bean,

On Tue, Jul 15, 2014 at 04:23:44AM +0000, Bean Huo wrote:
> Thanks for your warmly response about my patch.I have been waiting for your message too long.
> It's hard for me to give you a good reason that some flash's CFI parameter is non-zero,and 
> incorrect.I just found that if flash'buffer is 512 bytes,not 256 bytes,timeout error will happen.
> If using CFI's timemout value,that is OK.But for cfi_cmdset_0001.c,it also use timeout value of CFI.
> The difference with mine is that if timeout value is not defined in the CFI,the default maxmum timeout
> value is 500000us.

I understand the points here. But this is just a timeout value, so it
isn't a big issue if it's a little too large; but it's a major problem if
it's too short. Since this is an aged driver, which has to accomodate
all sorts of potentially broken chips, I think a "minimum timeout" is a
good idea. And using the current timeout of 2ms (or larger) is fine. So
in pseudocode:

	if (CFI_time_typ != 0 && CFI_time_max != 0)
		timeout_max = CFI_time_typ + CFI_time_max;
	else
		timeout_max = 0;
	timeout_max = max(timeout_max, 2000);

> Because my company's mailbox title exsits chinese words,and i sent this patch for 
> many times,you don't have one response,so i thought that you maybe couldn't receive my email.later,i
> chose my personal email.

It is reasonable to worry about bounced mail occasionally. If the
mailing list rejected your mail, you should get an automated message.
You can also check the archives soon after sending, to make sure your
mail shows up:

  http://lists.infradead.org/pipermail/linux-mtd/

And for LKML email, you can check one of several archives mirrors, like
lkml.org.

> For this patch,i will do some changes,please every maintainer and commit-signer check. 		 	   		  

Yes, please send a new version (version 4 now?). But please double check
your patch submission, so that it applies correctly this time.

Thanks,
Brian

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

* RE: [PATCH v3] mtd:nor:timeout:fix do_write_buffer() timeout error
  2014-07-12  6:46             ` Brian Norris
@ 2014-07-15  4:23               ` Bean Huo
  2014-07-15  7:28                 ` Brian Norris
  0 siblings, 1 reply; 24+ messages in thread
From: Bean Huo @ 2014-07-15  4:23 UTC (permalink / raw)
  To: Brian Norris
  Cc: christian.riesch, paul.gortmaker, dwmw2, linux-mtd, Stijn Devriendt

> + Linux-MTD
>
> Please do not remove the MTD mailing list from your email. I am much
> more likely to ignore it that way.
>
> Also, please don't top post. I'm top-posting right now, because I can't
> understand your thread very easily, and it's hard to resurrect for
> sending to the mailing list anyway.
>
> Regarding Stijn's comments: if you have good reason to believe that some
> flash's CFI parameter will be non-zero, but incorrect, then perhaps you
> could enforce a minimum value.
>
> Brian
>

hi,Brian
Thanks for your warmly response about my patch.I have been waiting for your message too long.
It's hard for me to give you a good reason that some flash's CFI parameter is non-zero,and 
incorrect.I just found that if flash'buffer is 512 bytes,not 256 bytes,timeout error will happen.
If using CFI's timemout value,that is OK.But for cfi_cmdset_0001.c,it also use timeout value of CFI.
The difference with mine is that if timeout value is not defined in the CFI,the default maxmum timeout
value is 500000us.
Because my company's mailbox title exsits chinese words,and i sent this patch for 
many times,you don't have one response,so i thought that you maybe couldn't receive my email.later,i
chose my personal email.

For this patch,i will do some changes,please every maintainer and commit-signer check. 		 	   		  

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

* Re: [PATCH v3] mtd:nor:timeout:fix do_write_buffer() timeout error
       [not found]           ` <BLU185-W73A028C46FBD1D13672BC0A6090@phx.gbl>
@ 2014-07-12  6:46             ` Brian Norris
  2014-07-15  4:23               ` Bean Huo
  0 siblings, 1 reply; 24+ messages in thread
From: Brian Norris @ 2014-07-12  6:46 UTC (permalink / raw)
  To: Bean Huo; +Cc: paul.gortmaker, dwmw2, linux-mtd, Stijn Devriendt

+ Linux-MTD

Please do not remove the MTD mailing list from your email. I am much
more likely to ignore it that way.

Also, please don't top post. I'm top-posting right now, because I can't
understand your thread very easily, and it's hard to resurrect for
sending to the mailing list anyway.

Regarding Stijn's comments: if you have good reason to believe that some
flash's CFI parameter will be non-zero, but incorrect, then perhaps you
could enforce a minimum value.

Brian

On Fri, Jul 11, 2014 at 12:18:40AM +0000, Bean Huo wrote:
> hi,Brian Norris
>     are you a maintainer of MTD? please help me and please give me a result,thanks!
> 
> ________________________________
> > Date: Thu, 10 Jul 2014 11:53:50 +0200 
> > Subject: Re: [PATCH v3] mtd:nor:timeout:fix do_write_buffer() timeout error 
> > From: highguy@gmail.com 
> > To: beanhuo@outlook.com 
> > CC: dwmw2@infradead.org; computersforpeace@gmail.com; 
> > paul.gortmaker@windriver.com 
> > 
> > On 10/07/2014 10:53, Bean Huo wrote: 
> > 
> > hi, 
> > thanks for your response! 
> > please look at the file cfi_cmdset_0001.c,It also use timeout value of CFI. 
> > But the difference with my patch is that if timeout value is not defined in the CFI,the default timeout value is 500000us. 
> > Do you mean that the default timeout value extend moure than 2MS? 
> > 
> > if (cfi->cfiq->BufWriteTimeoutTyp && 
> > cfi->cfiq->BufWriteTimeoutMax) 
> > cfi->chips[i].buffer_write_time_max = 
> > 1<<(cfi->cfiq->BufWriteTimeoutTyp + 
> > cfi->cfiq->BufWriteTimeoutMax); 
> > 
> > 
> > Good question. I don't think I can answer that one. Perhaps one of the MTD 
> > maintainers can. 
> > 
> > Regards, 
> > Stijn 
> > 
> > ________________________________ 
> > 
> > 
> > Date: Thu, 10 Jul 2014 08:33:16 +0200 
> > Subject: Re: [PATCH v3] mtd:nor:timeout:fix do_write_buffer() timeout error 
> > From: highguy@gmail.com<mailto:highguy@gmail.com> 
> > To: beanhuo@outlook.com<mailto:beanhuo@outlook.com> 
> > CC: dwmw2@infradead.org<mailto:dwmw2@infradead.org>; computersforpeace@gmail.com<mailto:computersforpeace@gmail.com>; 
> > paul.gortmaker@windriver.com<mailto:paul.gortmaker@windriver.com> 
> > 
> > 
> > On Thu, Jul 10, 2014 at 4:22 AM, Bean Huo 
> > <beanhuo@outlook.com<mailto:beanhuo@outlook.com><mailto:beanhuo@outlook.com><mailto:beanhuo@outlook.com>> wrote: 
> > 
> > hi,Stijn 
> > please see below about my answer: 
> > 
> > 
> > ________________________________ 
> > 
> > 
> > Date: Wed, 9 Jul 2014 10:03:02 +0200 
> > Subject: Re: [PATCH v3] mtd:nor:timeout:fix do_write_buffer() timeout error 
> > From: highguy@gmail.com<mailto:highguy@gmail.com><mailto:highguy@gmail.com><mailto:highguy@gmail.com> 
> > To: beanhuo@outlook.com<mailto:beanhuo@outlook.com><mailto:beanhuo@outlook.com><mailto:beanhuo@outlook.com> 
> > CC: dwmw2@infradead.org<mailto:dwmw2@infradead.org><mailto:dwmw2@infradead.org><mailto:dwmw2@infradead.org>; 
> > 
> > 
> > computersforpeace@gmail.com<mailto:computersforpeace@gmail.com><mailto:computersforpeace@gmail.com><mailto:computersforpeace@gmail.com>; 
> > 
> > 
> > paul.gortmaker@windriver.com<mailto:paul.gortmaker@windriver.com><mailto:paul.gortmaker@windriver.com><mailto:paul.gortmaker@windriver.com> 
> > 
> > Hi Bean, 
> > 
> > If I'm not mistaken the 2ms was also there because manufacturers 
> > 
> > 
> > don't always 
> > 
> > 
> > fill out the CFI data properly. I've seen incorrect values for other 
> > fields, so I'd be 
> > hesitant to trust the data. 
> > 
> > My proposition would be 3-fold: 
> > - first read the data from CFI 
> > 
> > 
> > this has been done in my patch.timeout value will be firstly pre-read 
> > from CFI. 
> > 
> > 
> > 
> > - use the value max(value, 2ms) 
> > 
> > 
> > in my patch,if CFI don't exsit this area value,and do_write_buffer max 
> > timeout will still be 2Ms 
> > 
> > 
> > But now you risk breaking flashes with incorrect timeout value <2ms. 
> > Those flashes would have 
> > worked fine, but by using the incorrect CFI value now, they'll silently 
> > break. 
> > 
> > 
> > 
> > - perhaps an override list (aka shame-the-mfr-list) for those flashes 
> > with known incorrect values 
> > 
> > 
> > I think,your worry is completely unnecessary,if flash with incorrect 
> > values ih this area, 
> > this is related to the quality of the flash,not the linux kernel.But,on 
> > the contrary,if forsome flashes with correct timeout 
> > value(because the size of the buffer program has been increased from 
> > 256 Bytes to 512 Bytes,timeout value is greater than 2MS) 
> > in this area,and we now still use default max timeout 
> > 2Ms,sometimes,this will lead to buffer-write operation failed.I have 
> > found this case 
> > in some flashes,also for this reason,I now submit this patch. 
> > 
> > 
> > If flashes have incorrect values, then this IS a worry for the linux kernel. 
> > People don't stop using these flashes because the CFI values are faulty, they 
> > expect S/W to workaround it. That is what the kernel has done until now. 
> > Have a look at the file pci-quirks.c. A whole file dedicated to fixing up 
> > crappy hardware. It's the kernel's job to make things work, no 
> > questions asked. 
> > (although we get to shame the vendors in public). 
> > 
> > Regards, 
> > Stijn 
> > 
> > 
> > 
> > You could probably implement 1&2 with almost no hassle, 3 would probably be 
> > for the first flash that has incorrect values (and needs over 2ms timeout). 
> > 
> > Regards, 
> > Stijn 
> > 
> > 
> > 
> >  		 	   		  

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

* Re: [PATCH v3] mtd:nor:timeout:fix do_write_buffer() timeout error
  2014-07-12  6:30   ` Brian Norris
@ 2014-07-12  6:37     ` Brian Norris
  -1 siblings, 0 replies; 24+ messages in thread
From: Brian Norris @ 2014-07-12  6:37 UTC (permalink / raw)
  To: Bean Huo 霍斌斌 (beanhuo)
  Cc: David Woodhouse, Paul Gortmaker, linux-mtd, linux-kernel,
	Jingoo Han, Artem Bityutskiy, b32955, Christian Riesch

On Fri, Jul 11, 2014 at 11:30:41PM -0700, Brian Norris wrote:
> I see that you have sent this several times in a relatively short
> period, and I see that there are several comments you haven't addressed
> [1] [2].

Wow, and I just noticed there are 3 more recent copies of this patch,
with as few as 2 days' wait in between! Please back off from that. You
can remind by replying to the original email. If you really think
everyone has forgetten your patch (after much more than just a week), a
resend may be worth it.

Brian

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

* Re: [PATCH v3] mtd:nor:timeout:fix do_write_buffer() timeout error
@ 2014-07-12  6:37     ` Brian Norris
  0 siblings, 0 replies; 24+ messages in thread
From: Brian Norris @ 2014-07-12  6:37 UTC (permalink / raw)
  To: Bean Huo 霍斌斌 (beanhuo)
  Cc: b32955, Artem Bityutskiy, Jingoo Han, linux-kernel,
	Christian Riesch, Paul Gortmaker, linux-mtd, David Woodhouse

On Fri, Jul 11, 2014 at 11:30:41PM -0700, Brian Norris wrote:
> I see that you have sent this several times in a relatively short
> period, and I see that there are several comments you haven't addressed
> [1] [2].

Wow, and I just noticed there are 3 more recent copies of this patch,
with as few as 2 days' wait in between! Please back off from that. You
can remind by replying to the original email. If you really think
everyone has forgetten your patch (after much more than just a week), a
resend may be worth it.

Brian

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

* Re: [PATCH v3] mtd:nor:timeout:fix do_write_buffer() timeout error
  2014-06-30  7:56 ` Bean Huo 霍斌斌 (beanhuo)
@ 2014-07-12  6:30   ` Brian Norris
  -1 siblings, 0 replies; 24+ messages in thread
From: Brian Norris @ 2014-07-12  6:30 UTC (permalink / raw)
  To: Bean Huo 霍斌斌 (beanhuo)
  Cc: David Woodhouse, Paul Gortmaker, linux-mtd, linux-kernel,
	Jingoo Han, Artem Bityutskiy, b32955, Christian Riesch

Hi Bean,

I see that you have sent this several times in a relatively short
period, and I see that there are several comments you haven't addressed
[1] [2].

It is very difficult for me to apply your patch. It seems to be an
invalid patch (I think the line counts are wrong), so it doesn't apply
with git-am. Christian already had this problem in [1]. Can you try
sending mail with git-send-email? It will probably do better than your
current mailer. If you're having any doubt, try sending the patch to
yourself and then applying it with git-am.
Documentation/email-clients.txt also has some tips for email.

Your patch also has several coding style issues, one of which Christian
also noticed, in [2]. (Thanks for reviewing, BTW, Chrisitan!)

Can you fix the subject to follow other patches' style? Like:

  mtd: cfi_cmdset_0002: ...

(I can fix one or two patches for these kind of things, but you said you
plan to send several more patches.)

On Mon, Jun 30, 2014 at 07:56:37AM +0000, Bean Huo 霍斌斌 (beanhuo) wrote:
> The size of the buffer program has been increased from 256 to 512 , 2ms maximum timeout for do_write_buffer 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.

Please line-wrap your commit descriptions. This line is >300 characters
long.

> 
> 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>
> ---
> changes 
> 	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.
> 
>  drivers/mtd/chips/cfi_cmdset_0002.c |   27 ++++++++++++++++++++++++---
>  1 file changed, 24 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c b/drivers/mtd/chips/cfi_cmdset_0002.c
> index e21fde9..f7098d6 100644
> --- a/drivers/mtd/chips/cfi_cmdset_0002.c
> +++ b/drivers/mtd/chips/cfi_cmdset_0002.c
> @@ -628,10 +628,24 @@ 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->BufWriteTimeoutTyp &&
> +			cfi->cfiq->BufWriteTimeoutMax){

Can you put a space before the brace?

> +			cfi->chips[i].buffer_write_time_max =
> +				1<<(cfi->cfiq->BufWriteTimeoutTyp +

Please put spaces around the shift operator.

> +					cfi->cfiq->BufWriteTimeoutMax);
> +			} else {
> +			/* specify maximum timeout for buffer program 2000us */
> +				cfi->chips[i].buffer_write_time_max = 2000;
> +				}

The indentation of this entire 'else' block is wrong.

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

Please don't remove this line.

>  	map->fldrv = &cfi_amdstd_chipdrv;
>  
>  	return cfi_amdstd_setup(mtd);
> @@ -1462,8 +1476,15 @@ 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;
> +	/* 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().
> +	 * uWriteTimeout is used for timeout step,it must be concerted
> +	 * into jiffies.

Some of the language here needs improvement, but I can spruce it up once
your patch is ready. Just fix the style up a bit, and make sure the
patch can apply, then I'll take care of the rest. Thanks!

> +	 */
> +	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

Brian

[1] http://lists.infradead.org/pipermail/linux-mtd/2014-June/054362.html
[2] http://lists.infradead.org/pipermail/linux-mtd/2014-June/054367.html

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

* Re: [PATCH v3] mtd:nor:timeout:fix do_write_buffer() timeout error
@ 2014-07-12  6:30   ` Brian Norris
  0 siblings, 0 replies; 24+ messages in thread
From: Brian Norris @ 2014-07-12  6:30 UTC (permalink / raw)
  To: Bean Huo 霍斌斌 (beanhuo)
  Cc: b32955, Artem Bityutskiy, Jingoo Han, linux-kernel,
	Christian Riesch, Paul Gortmaker, linux-mtd, David Woodhouse

Hi Bean,

I see that you have sent this several times in a relatively short
period, and I see that there are several comments you haven't addressed
[1] [2].

It is very difficult for me to apply your patch. It seems to be an
invalid patch (I think the line counts are wrong), so it doesn't apply
with git-am. Christian already had this problem in [1]. Can you try
sending mail with git-send-email? It will probably do better than your
current mailer. If you're having any doubt, try sending the patch to
yourself and then applying it with git-am.
Documentation/email-clients.txt also has some tips for email.

Your patch also has several coding style issues, one of which Christian
also noticed, in [2]. (Thanks for reviewing, BTW, Chrisitan!)

Can you fix the subject to follow other patches' style? Like:

  mtd: cfi_cmdset_0002: ...

(I can fix one or two patches for these kind of things, but you said you
plan to send several more patches.)

On Mon, Jun 30, 2014 at 07:56:37AM +0000, Bean Huo 霍斌斌 (beanhuo) wrote:
> The size of the buffer program has been increased from 256 to 512 , 2ms maximum timeout for do_write_buffer 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.

Please line-wrap your commit descriptions. This line is >300 characters
long.

> 
> 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>
> ---
> changes 
> 	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.
> 
>  drivers/mtd/chips/cfi_cmdset_0002.c |   27 ++++++++++++++++++++++++---
>  1 file changed, 24 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c b/drivers/mtd/chips/cfi_cmdset_0002.c
> index e21fde9..f7098d6 100644
> --- a/drivers/mtd/chips/cfi_cmdset_0002.c
> +++ b/drivers/mtd/chips/cfi_cmdset_0002.c
> @@ -628,10 +628,24 @@ 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->BufWriteTimeoutTyp &&
> +			cfi->cfiq->BufWriteTimeoutMax){

Can you put a space before the brace?

> +			cfi->chips[i].buffer_write_time_max =
> +				1<<(cfi->cfiq->BufWriteTimeoutTyp +

Please put spaces around the shift operator.

> +					cfi->cfiq->BufWriteTimeoutMax);
> +			} else {
> +			/* specify maximum timeout for buffer program 2000us */
> +				cfi->chips[i].buffer_write_time_max = 2000;
> +				}

The indentation of this entire 'else' block is wrong.

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

Please don't remove this line.

>  	map->fldrv = &cfi_amdstd_chipdrv;
>  
>  	return cfi_amdstd_setup(mtd);
> @@ -1462,8 +1476,15 @@ 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;
> +	/* 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().
> +	 * uWriteTimeout is used for timeout step,it must be concerted
> +	 * into jiffies.

Some of the language here needs improvement, but I can spruce it up once
your patch is ready. Just fix the style up a bit, and make sure the
patch can apply, then I'll take care of the rest. Thanks!

> +	 */
> +	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

Brian

[1] http://lists.infradead.org/pipermail/linux-mtd/2014-June/054362.html
[2] http://lists.infradead.org/pipermail/linux-mtd/2014-June/054367.html

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

* [PATCH v3] mtd:nor:timeout:fix do_write_buffer() timeout error
@ 2014-07-09  1:00 bean huo
       [not found] ` <CAOY=C6HaXPrEGPSzEUb5XM79SdqTSGwMPmQXq3U7abNvW0SuQA@mail.gmail.com>
  0 siblings, 1 reply; 24+ messages in thread
From: bean huo @ 2014-07-09  1:00 UTC (permalink / raw)
  To: dwmw2, computersforpeace, paul.gortmaker

The size of the buffer program has been increased from 256 to 512 , 2ms maximum timeout for do_write_buffer 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@outlook.com>
---
changes
     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.
 
 drivers/mtd/chips/cfi_cmdset_0002.c |   27 ++++++++++++++++++++++++---
 1 file changed, 24 insertions(+), 3 deletions(-)
 
diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c b/drivers/mtd/chips/cfi_cmdset_0002.c
index e21fde9..f7098d6 100644
--- a/drivers/mtd/chips/cfi_cmdset_0002.c
+++ b/drivers/mtd/chips/cfi_cmdset_0002.c
@@ -628,10 +628,24 @@ 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->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;
+                       }
            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 +1476,15 @@ 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;
+     /* 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().
+     * 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 		 	   		  

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

* [PATCH v3] mtd:nor:timeout:fix do_write_buffer() timeout error
@ 2014-07-09  0:43 ` Bean Huo
  0 siblings, 0 replies; 24+ messages in thread
From: Bean Huo @ 2014-07-09  0:43 UTC (permalink / raw)
  To: dwmw2, computersforpeace, paul.gortmaker
  Cc: linux-mtd, linux-kernel, jg1.han, artem.bityutskiy

The size of the buffer program has been increased from 256 to 512 ,
2ms maximum timeout for do_write_buffer 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 <jackyard88@gmail.com>
---
changes
     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.

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

diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c
b/drivers/mtd/chips/cfi_cmdset_0002.c
index e21fde9..f7098d6 100644
--- a/drivers/mtd/chips/cfi_cmdset_0002.c
+++ b/drivers/mtd/chips/cfi_cmdset_0002.c
@@ -628,10 +628,24 @@ 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->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;
+                       }
            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 +1476,15 @@ 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;
+     /* 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().
+     * 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

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

* [PATCH v3] mtd:nor:timeout:fix do_write_buffer() timeout error
@ 2014-07-09  0:43 ` Bean Huo
  0 siblings, 0 replies; 24+ messages in thread
From: Bean Huo @ 2014-07-09  0:43 UTC (permalink / raw)
  To: dwmw2, computersforpeace, paul.gortmaker
  Cc: artem.bityutskiy, jg1.han, linux-mtd, linux-kernel

The size of the buffer program has been increased from 256 to 512 ,
2ms maximum timeout for do_write_buffer 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 <jackyard88@gmail.com>
---
changes
     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.

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

diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c
b/drivers/mtd/chips/cfi_cmdset_0002.c
index e21fde9..f7098d6 100644
--- a/drivers/mtd/chips/cfi_cmdset_0002.c
+++ b/drivers/mtd/chips/cfi_cmdset_0002.c
@@ -628,10 +628,24 @@ 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->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;
+                       }
            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 +1476,15 @@ 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;
+     /* 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().
+     * 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

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

* [PATCH v3] mtd:nor:timeout:fix do_write_buffer() timeout error
@ 2014-07-07  6:05 ` Bean Huo 霍斌斌 (beanhuo)
  0 siblings, 0 replies; 24+ messages in thread
From: Bean Huo 霍斌斌 (beanhuo) @ 2014-07-07  6:05 UTC (permalink / raw)
  To: David Woodhouse, Brian Norris, Paul Gortmaker
  Cc: linux-mtd, linux-kernel, Jingoo Han, Artem Bityutskiy, b32955

The size of the buffer program has been increased from 256 to 512 , 2ms maximum timeout for do_write_buffer 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>
---
changes 
	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.

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

diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c b/drivers/mtd/chips/cfi_cmdset_0002.c
index e21fde9..f7098d6 100644
--- a/drivers/mtd/chips/cfi_cmdset_0002.c
+++ b/drivers/mtd/chips/cfi_cmdset_0002.c
@@ -628,10 +628,24 @@ 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->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;
+				}
 		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 +1476,15 @@ 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;
+	/* 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().
+	 * 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

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

* [PATCH v3] mtd:nor:timeout:fix do_write_buffer() timeout error
@ 2014-07-07  6:05 ` Bean Huo 霍斌斌 (beanhuo)
  0 siblings, 0 replies; 24+ messages in thread
From: Bean Huo 霍斌斌 (beanhuo) @ 2014-07-07  6:05 UTC (permalink / raw)
  To: David Woodhouse, Brian Norris, Paul Gortmaker
  Cc: Artem Bityutskiy, Jingoo Han, linux-mtd, linux-kernel, b32955

The size of the buffer program has been increased from 256 to 512 , 2ms maximum timeout for do_write_buffer 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>
---
changes 
	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.

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

diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c b/drivers/mtd/chips/cfi_cmdset_0002.c
index e21fde9..f7098d6 100644
--- a/drivers/mtd/chips/cfi_cmdset_0002.c
+++ b/drivers/mtd/chips/cfi_cmdset_0002.c
@@ -628,10 +628,24 @@ 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->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;
+				}
 		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 +1476,15 @@ 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;
+	/* 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().
+	 * 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

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

* [PATCH v3] mtd:nor:timeout:fix do_write_buffer() timeout error
@ 2014-06-30  7:56 ` Bean Huo 霍斌斌 (beanhuo)
  0 siblings, 0 replies; 24+ messages in thread
From: Bean Huo 霍斌斌 (beanhuo) @ 2014-06-30  7:56 UTC (permalink / raw)
  To: David Woodhouse, Brian Norris, Paul Gortmaker
  Cc: linux-mtd, linux-kernel, Jingoo Han, Artem Bityutskiy, b32955

The size of the buffer program has been increased from 256 to 512 , 2ms maximum timeout for do_write_buffer 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>
---
changes 
	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.

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

diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c b/drivers/mtd/chips/cfi_cmdset_0002.c
index e21fde9..f7098d6 100644
--- a/drivers/mtd/chips/cfi_cmdset_0002.c
+++ b/drivers/mtd/chips/cfi_cmdset_0002.c
@@ -628,10 +628,24 @@ 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->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;
+				}
 		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 +1476,15 @@ 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;
+	/* 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().
+	 * 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

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

* [PATCH v3] mtd:nor:timeout:fix do_write_buffer() timeout error
@ 2014-06-30  7:56 ` Bean Huo 霍斌斌 (beanhuo)
  0 siblings, 0 replies; 24+ messages in thread
From: Bean Huo 霍斌斌 (beanhuo) @ 2014-06-30  7:56 UTC (permalink / raw)
  To: David Woodhouse, Brian Norris, Paul Gortmaker
  Cc: Artem Bityutskiy, Jingoo Han, linux-mtd, linux-kernel, b32955

The size of the buffer program has been increased from 256 to 512 , 2ms maximum timeout for do_write_buffer 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>
---
changes 
	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.

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

diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c b/drivers/mtd/chips/cfi_cmdset_0002.c
index e21fde9..f7098d6 100644
--- a/drivers/mtd/chips/cfi_cmdset_0002.c
+++ b/drivers/mtd/chips/cfi_cmdset_0002.c
@@ -628,10 +628,24 @@ 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->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;
+				}
 		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 +1476,15 @@ 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;
+	/* 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().
+	 * 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

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

* [PATCH v3] mtd:nor:timeout:fix do_write_buffer() timeout error
@ 2014-06-24 17:03 ` Bean Huo 霍斌斌 (beanhuo)
  0 siblings, 0 replies; 24+ messages in thread
From: Bean Huo 霍斌斌 (beanhuo) @ 2014-06-24 17:03 UTC (permalink / raw)
  To: David Woodhouse, Brian Norris, Paul Gortmaker
  Cc: linux-mtd, linux-kernel, Jingoo Han, Artem Bityutskiy, b32955

The size of the buffer program has been increased from 256 to 512 ,
2ms maximum timeout for do_write_buffer 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>
---
changes 
	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.

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

diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c b/drivers/mtd/chips/cfi_cmdset_0002.c
index e21fde9..f7098d6 100644
--- a/drivers/mtd/chips/cfi_cmdset_0002.c
+++ b/drivers/mtd/chips/cfi_cmdset_0002.c
@@ -628,10 +628,24 @@ 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->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;
+				}
 		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 +1476,15 @@ 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;
+	/* 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().
+	 * 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

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

* [PATCH v3] mtd:nor:timeout:fix do_write_buffer() timeout error
@ 2014-06-24 17:03 ` Bean Huo 霍斌斌 (beanhuo)
  0 siblings, 0 replies; 24+ messages in thread
From: Bean Huo 霍斌斌 (beanhuo) @ 2014-06-24 17:03 UTC (permalink / raw)
  To: David Woodhouse, Brian Norris, Paul Gortmaker
  Cc: Artem Bityutskiy, Jingoo Han, linux-mtd, linux-kernel, b32955

The size of the buffer program has been increased from 256 to 512 ,
2ms maximum timeout for do_write_buffer 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>
---
changes 
	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.

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

diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c b/drivers/mtd/chips/cfi_cmdset_0002.c
index e21fde9..f7098d6 100644
--- a/drivers/mtd/chips/cfi_cmdset_0002.c
+++ b/drivers/mtd/chips/cfi_cmdset_0002.c
@@ -628,10 +628,24 @@ 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->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;
+				}
 		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 +1476,15 @@ 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;
+	/* 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().
+	 * 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

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

* Re: [PATCH v3] mtd:nor:timeout:fix do_write_buffer() timeout error
  2014-06-24  6:21   ` Bean Huo 霍斌斌 (beanhuo)
@ 2014-06-24  6:59     ` Christian Riesch
  0 siblings, 0 replies; 24+ messages in thread
From: Christian Riesch @ 2014-06-24  6:59 UTC (permalink / raw)
  To: Bean Huo 霍斌斌 (beanhuo)
  Cc: Kees Cook, Alexander Shiyan, Jingoo Han, Paul Gortmaker,
	linux-mtd, Brian Norris, David Woodhouse

On Tue, Jun 24, 2014 at 8:21 AM, Bean Huo 霍斌斌 (beanhuo)
<beanhuo@micron.com> wrote:
>> v3 applies without any problems! Thanks!
>>
> This patch can now be accepted? I want to submit other patches one by one.thanks!

I don't know, I am not the maintainer. But given that one of my
patchsets has been waiting for more than a year to get accepted on
this list, I wouldn't be too optimistic ;-)

Just send your patches as they become ready.
Christian

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

* Re: [PATCH v3] mtd:nor:timeout:fix do_write_buffer() timeout error
  2014-06-24  2:02 Bean Huo 霍斌斌 (beanhuo)
  2014-06-24  6:17 ` Christian Riesch
@ 2014-06-24  6:55 ` Christian Riesch
  1 sibling, 0 replies; 24+ messages in thread
From: Christian Riesch @ 2014-06-24  6:55 UTC (permalink / raw)
  To: Bean Huo 霍斌斌 (beanhuo)
  Cc: Kees Cook, Alexander Shiyan, Jingoo Han, Paul Gortmaker,
	linux-mtd, Brian Norris, David Woodhouse

Hi,
sorry, I found something else.

On Tue, Jun 24, 2014 at 4:02 AM, Bean Huo 霍斌斌 (beanhuo)
<beanhuo@micron.com> wrote:
> The size of the buffer program has been increased from 256 to 512 ,
> 2ms maximum timeout for do_write_buffer 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>
> ---
> changes
>         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.
>
>  drivers/mtd/chips/cfi_cmdset_0002.c |   25 ++++++++++++++++++++++---
>  1 file changed, 22 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c b/drivers/mtd/chips/cfi_cmdset_0002.c
> index e21fde9..3ee43cf 100644
> --- a/drivers/mtd/chips/cfi_cmdset_0002.c
> +++ b/drivers/mtd/chips/cfi_cmdset_0002.c
> @@ -628,10 +628,24 @@ 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->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;
> +                               }

Wrong indentation above. Please fix that.

Christian

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

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

Hi,

> v3 applies without any problems! Thanks!
>
This patch can now be accepted? I want to submit other patches one by one.thanks!

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

* Re: [PATCH v3] mtd:nor:timeout:fix do_write_buffer() timeout error
  2014-06-24  2:02 Bean Huo 霍斌斌 (beanhuo)
@ 2014-06-24  6:17 ` Christian Riesch
  2014-06-24  6:21   ` Bean Huo 霍斌斌 (beanhuo)
  2014-06-24  6:55 ` Christian Riesch
  1 sibling, 1 reply; 24+ messages in thread
From: Christian Riesch @ 2014-06-24  6:17 UTC (permalink / raw)
  To: Bean Huo 霍斌斌 (beanhuo)
  Cc: Kees Cook, Alexander Shiyan, Jingoo Han, Paul Gortmaker,
	linux-mtd, Brian Norris, David Woodhouse

Hi,

On Tue, Jun 24, 2014 at 4:02 AM, Bean Huo 霍斌斌 (beanhuo)
<beanhuo@micron.com> wrote:
> The size of the buffer program has been increased from 256 to 512 ,
> 2ms maximum timeout for do_write_buffer 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.
>

v3 applies without any problems! Thanks!
Christian

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

* [PATCH v3] mtd:nor:timeout:fix do_write_buffer() timeout error
@ 2014-06-24  2:02 Bean Huo 霍斌斌 (beanhuo)
  2014-06-24  6:17 ` Christian Riesch
  2014-06-24  6:55 ` Christian Riesch
  0 siblings, 2 replies; 24+ messages in thread
From: Bean Huo 霍斌斌 (beanhuo) @ 2014-06-24  2:02 UTC (permalink / raw)
  To: David Woodhouse, Brian Norris, Alexander Shiyan, Christian Riesch
  Cc: Paul Gortmaker, Jingoo Han, linux-mtd, Kees Cook

The size of the buffer program has been increased from 256 to 512 ,
2ms maximum timeout for do_write_buffer 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>
---
changes 
	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.

 drivers/mtd/chips/cfi_cmdset_0002.c |   25 ++++++++++++++++++++++---
 1 file changed, 22 insertions(+), 3 deletions(-)

diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c b/drivers/mtd/chips/cfi_cmdset_0002.c
index e21fde9..3ee43cf 100644
--- a/drivers/mtd/chips/cfi_cmdset_0002.c
+++ b/drivers/mtd/chips/cfi_cmdset_0002.c
@@ -628,10 +628,24 @@ 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->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;
+				}
 		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 +1476,13 @@ 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;
+	/* 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().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

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

end of thread, other threads:[~2014-07-16  6:23 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-07-03  0:40 [PATCH v3] mtd:nor:timeout:fix do_write_buffer() timeout error Bean Huo 霍斌斌 (beanhuo)
2014-07-03  0:40 ` Bean Huo 霍斌斌 (beanhuo)
  -- strict thread matches above, loose matches on Subject: below --
2014-07-09  1:00 bean huo
     [not found] ` <CAOY=C6HaXPrEGPSzEUb5XM79SdqTSGwMPmQXq3U7abNvW0SuQA@mail.gmail.com>
     [not found]   ` <BLU185-W959F702F780985563AD7C0A60E0@phx.gbl>
     [not found]     ` <CAOY=C6GcNzvBieUVm1pk4XQipxj3U9N1Dw09JEM6ZdW7jJLjcA@mail.gmail.com>
     [not found]       ` <BLU185-W92FA6A378C0039524F045FA60E0@phx.gbl>
     [not found]         ` <CAOY=C6FG8fgZGKhXvXG03hQQNSOYz+=uqrfYmNXfHKkz4XRSog@mail.gmail.com>
     [not found]           ` <BLU185-W73A028C46FBD1D13672BC0A6090@phx.gbl>
2014-07-12  6:46             ` Brian Norris
2014-07-15  4:23               ` Bean Huo
2014-07-15  7:28                 ` Brian Norris
2014-07-16  6:22                   ` Bean Huo
2014-07-09  0:43 Bean Huo
2014-07-09  0:43 ` Bean Huo
2014-07-07  6:05 Bean Huo 霍斌斌 (beanhuo)
2014-07-07  6:05 ` Bean Huo 霍斌斌 (beanhuo)
2014-06-30  7:56 Bean Huo 霍斌斌 (beanhuo)
2014-06-30  7:56 ` Bean Huo 霍斌斌 (beanhuo)
2014-07-12  6:30 ` Brian Norris
2014-07-12  6:30   ` Brian Norris
2014-07-12  6:37   ` Brian Norris
2014-07-12  6:37     ` Brian Norris
2014-06-24 17:03 Bean Huo 霍斌斌 (beanhuo)
2014-06-24 17:03 ` Bean Huo 霍斌斌 (beanhuo)
2014-06-24  2:02 Bean Huo 霍斌斌 (beanhuo)
2014-06-24  6:17 ` Christian Riesch
2014-06-24  6:21   ` Bean Huo 霍斌斌 (beanhuo)
2014-06-24  6:59     ` Christian Riesch
2014-06-24  6:55 ` Christian Riesch

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.