[RFC][PATCH] bcm47xxnflash: use small delay between polling

[RFC][PATCH] bcm47xxnflash: use small delay between polling

[Rafał Miłecki]

---
I think it's a good idea to add some delays between polling the
hardware. I'm not sure however what delays should be used.

In the proposed patch I put "ndelay(1)" which is extermely low delay,
but even with that it's common for the loop to make only 0-3 iterations.
For that reason I don't want to put delays like "ndelay(10)" or bigger.
This could mean waiting 10ns while the hardware is ready after 1ns.

What do you think about this?
---
 drivers/mtd/nand/bcm47xxnflash/ops_bcm4706.c |   16 +++++++++++-----
 1 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/mtd/nand/bcm47xxnflash/ops_bcm4706.c b/drivers/mtd/nand/bcm47xxnflash/ops_bcm4706.c
index ece343c..1026100 100644
--- a/drivers/mtd/nand/bcm47xxnflash/ops_bcm4706.c
+++ b/drivers/mtd/nand/bcm47xxnflash/ops_bcm4706.c
@@ -16,9 +16,13 @@
 
 #include "bcm47xxnflash.h"
 
-/* Broadcom uses 1'000'000 but it seems to be too many. Tests on WNDR4500 has
- * shown 164 retries as maxiumum. */
-#define NFLASH_READY_RETRIES		1000
+/* Broadcom uses 1'000'000 for both without any delay.
+ * Tests on WNDR4500 have shown that in can take:
+ * 1) 0-5 retries for ctl_cmd with ndelay(1)
+ * 2) 0-800 retries for poll with ndelay(1)
+ */
+#define NFLASH_CTL_CMD_RETRIES		100
+#define NFLASH_POLL_RETRIES		10000
 
 #define NFLASH_SECTOR_SIZE		512
 
@@ -45,11 +49,12 @@ static int bcm47xxnflash_ops_bcm4706_ctl_cmd(struct bcma_drv_cc *cc, u32 code)
 	int i = 0;
 
 	bcma_cc_write32(cc, BCMA_CC_NFLASH_CTL, NCTL_START | code);
-	for (i = 0; i < NFLASH_READY_RETRIES; i++) {
+	for (i = 0; i < NFLASH_CTL_CMD_RETRIES; i++) {
 		if (!(bcma_cc_read32(cc, BCMA_CC_NFLASH_CTL) & NCTL_START)) {
 			i = 0;
 			break;
 		}
+		ndelay(1);
 	}
 	if (i) {
 		pr_err("NFLASH control command not ready!\n");
@@ -62,7 +67,7 @@ static int bcm47xxnflash_ops_bcm4706_poll(struct bcma_drv_cc *cc)
 {
 	int i;
 
-	for (i = 0; i < NFLASH_READY_RETRIES; i++) {
+	for (i = 0; i < NFLASH_POLL_RETRIES; i++) {
 		if (bcma_cc_read32(cc, BCMA_CC_NFLASH_CTL) & NCTL_READY) {
 			if (bcma_cc_read32(cc, BCMA_CC_NFLASH_CTL) &
 			    BCMA_CC_NFLASH_CTL_ERR) {
@@ -72,6 +77,7 @@ static int bcm47xxnflash_ops_bcm4706_poll(struct bcma_drv_cc *cc)
 				return 0;
 			}
 		}
+		ndelay(1);
 	}
 
 	pr_err("Polling timeout!\n");
-- 
1.7.7

Re: [RFC][PATCH] bcm47xxnflash: use small delay between polling

[Kevin Cernekee]

On Fri, Nov 23, 2012 at 9:43 AM, Rafał Miłecki <zajec5@gmail.com> wrote:
> ---
> I think it's a good idea to add some delays between polling the
> hardware. I'm not sure however what delays should be used.
>
> In the proposed patch I put "ndelay(1)" which is extermely low delay,
> but even with that it's common for the loop to make only 0-3 iterations.
> For that reason I don't want to put delays like "ndelay(10)" or bigger.
> This could mean waiting 10ns while the hardware is ready after 1ns.
>
> What do you think about this?

You might want to run a few tests to see how much time the register
access itself takes.  I don't know about this platform specifically,
but it is not uncommon to see times in the range of 0.5us ~ 1us.  So a
1ns or 10ns delay might not even be noticeable.

1ns is one cycle on a 1 GHz processor; peripherals (and even RAM) are
usually much much slower than that.

A quick and dirty way to measure register access time is to run, say,
10 million reads, and time the loop with a stopwatch.