Skip to contentSkip to conversation

mtd: nand: denali: Nuke unbounded loop

PATCH New Marek Vasut 2 participants 7 messages Dec 20, 2015 – Dec 21, 2015

Subsystems (2)
Memory technology devices (MTD) NAND flash
lore
Share & tools
lore

Marek Vasut

author
#0 lore raw

PATCH 1/1 mtd: nand: denali: Nuke unbounded loop

This particular unbounded loop in the Denali NAND reset routine can lead to a system hang in case neither of the Timeout and Completed bits get set.

Refactor the code a bit so it's readable and implement timer so the loop is bounded instead. This way the complete hang can be prevented even if the NAND fails to reset.

Signed-off-by Marek Vasut <[email protected]>
Cc Masahiro Yamada <[email protected]>
Cc Scott Wood <[email protected]>
---
 drivers/mtd/nand/denali.c | 26 ++++++++++++++++++++------
 1 file changed, 20 insertions(+), 6 deletions(-)
drivers/mtd/nand/denali.c +20 -6
index 192be7d..8a8cca9 100644
@@ -199,6 +199,7 @@ static void reset_bank(struct denali_nand_info *denali)
 static uint32_t denali_nand_reset(struct denali_nand_info *denali)
 {
 	int i;
+	u32 start, reg;
 
 	for (i = 0; i < denali->max_banks; i++)
 		writel(INTR_STATUS__RST_COMP | INTR_STATUS__TIME_OUT,

@@ -206,12 +207,25 @@ static uint32_t denali_nand_reset(struct denali_nand_info *denali)
 
 	for (i = 0; i < denali->max_banks; i++) {
 		writel(1 << i, denali->flash_reg + DEVICE_RESET);
-		while (!(readl(denali->flash_reg + INTR_STATUS(i)) &
-			(INTR_STATUS__RST_COMP | INTR_STATUS__TIME_OUT)))
-			if (readl(denali->flash_reg + INTR_STATUS(i)) &
-				INTR_STATUS__TIME_OUT)
-				debug("NAND Reset operation timed out on bank"
-				      " %d\n", i);
+
+		start = get_timer(0);
+		while (1) {
+			reg = readl(denali->flash_reg + INTR_STATUS(i));
+			if (reg & INTR_STATUS__TIME_OUT) {
+				debug("NAND Reset operation timed out on bank %d\n",
+				      i);
+				break;
+			}
+
+			/* Reset completed and did not time out, all good. */
+			if (reg & INTR_STATUS__RST_COMP)
+				break;
+
+			if (get_timer(start) > (CONFIG_SYS_HZ / 1000)) {
+				debug("%s: Reset timed out!\n", __func__);
+				break;
+			}
+		}
 	}
 
 	for (i = 0; i < denali->max_banks; i++)
Signature · 1 line
2.1.4
#1 lore raw

PATCH 1/1 mtd: nand: denali: Nuke unbounded loop

Hi Marek,

2015-12-20 11:59 GMT+09:00 Marek Vasut <[email protected]>:

Quoted 46 lines

This particular unbounded loop in the Denali NAND reset routine can lead to a system hang in case neither of the Timeout and Completed bits get set.

Refactor the code a bit so it's readable and implement timer so the loop is bounded instead. This way the complete hang can be prevented even if the NAND fails to reset.

Signed-off-by Marek Vasut <[email protected]>
Cc Masahiro Yamada <[email protected]>
Cc Scott Wood <[email protected]>
---
 drivers/mtd/nand/denali.c | 26 ++++++++++++++++++++------
 1 file changed, 20 insertions(+), 6 deletions(-)
drivers/mtd/nand/denali.c +10 -6
index 192be7d..8a8cca9 100644
@@ -199,6 +199,7 @@ static void reset_bank(struct denali_nand_info *denali)
 static uint32_t denali_nand_reset(struct denali_nand_info *denali)
 {
        int i;
+       u32 start, reg;
 
        for (i = 0; i < denali->max_banks; i++)
                writel(INTR_STATUS__RST_COMP | INTR_STATUS__TIME_OUT,

@@ -206,12 +207,25 @@ static uint32_t denali_nand_reset(struct denali_nand_info *denali)
 
        for (i = 0; i < denali->max_banks; i++) {
                writel(1 << i, denali->flash_reg + DEVICE_RESET);
-               while (!(readl(denali->flash_reg + INTR_STATUS(i)) &
-                       (INTR_STATUS__RST_COMP | INTR_STATUS__TIME_OUT)))
-                       if (readl(denali->flash_reg + INTR_STATUS(i)) &
-                               INTR_STATUS__TIME_OUT)
-                               debug("NAND Reset operation timed out on bank"
-                                     " %d\n", i);
+
+               start = get_timer(0);
+               while (1) {
+                       reg = readl(denali->flash_reg + INTR_STATUS(i));
+                       if (reg & INTR_STATUS__TIME_OUT) {
+                               debug("NAND Reset operation timed out on bank %d\n",
+                                     i);
+                               break;
+                       }

drivers/mtd/nand/denali.c static uint32_t denali_nand_reset(struct denali_nand_info *denali) · line 228

Quoted 10 lines
drivers/mtd/nand/denali.c +10 -0
+
+                       /* Reset completed and did not time out, all good. */
+                       if (reg & INTR_STATUS__RST_COMP)
+                               break;
+
+                       if (get_timer(start) > (CONFIG_SYS_HZ / 1000)) {
+                               debug("%s: Reset timed out!\n", __func__);
+                               break;
+                       }
+               }

I feel it is too much here
about using get_timer() & CONFIG_SYS_HZ.

How about iterating udelay(1) up to reasonable times?

See the wait_for_irq() function in this file.

Signature · 2 lines
Best Regards
Masahiro Yamada
author
#2 lore raw

PATCH 1/1 mtd: nand: denali: Nuke unbounded loop

Quoted 1 line · Masahiro Yamada

Hi Marek,

Hi,

Quoted 71 lines

2015-12-20 11:59 GMT+09:00 Marek Vasut <[email protected]>:

Quoted 64 lines

This particular unbounded loop in the Denali NAND reset routine can lead to a system hang in case neither of the Timeout and Completed bits get set.

Refactor the code a bit so it's readable and implement timer so the loop is bounded instead. This way the complete hang can be prevented even if the NAND fails to reset.

Signed-off-by Marek Vasut <[email protected]>
Cc Masahiro Yamada <[email protected]>
Cc Scott Wood <[email protected]>

---

 drivers/mtd/nand/denali.c | 26 ++++++++++++++++++++------
 1 file changed, 20 insertions(+), 6 deletions(-)
drivers/mtd/nand/denali.c +7 -5
index 192be7d..8a8cca9 100644
@@ -199,6 +199,7 @@ static void reset_bank(struct denali_nand_info
 *denali)
 
 static uint32_t denali_nand_reset(struct denali_nand_info *denali)
 {
 
        int i;
 
+       u32 start, reg;
 
        for (i = 0; i < denali->max_banks; i++)
 
                writel(INTR_STATUS__RST_COMP | INTR_STATUS__TIME_OUT,

@@ -206,12 +207,25 @@ static uint32_t denali_nand_reset(struct
 denali_nand_info *denali)
 
        for (i = 0; i < denali->max_banks; i++) {
 
                writel(1 << i, denali->flash_reg + DEVICE_RESET);
 
-               while (!(readl(denali->flash_reg + INTR_STATUS(i)) &
-                       (INTR_STATUS__RST_COMP | INTR_STATUS__TIME_OUT)))
-                       if (readl(denali->flash_reg + INTR_STATUS(i)) &
-                               INTR_STATUS__TIME_OUT)
-                               debug("NAND Reset operation timed out on
 bank" -                                     " %d\n", i);
+
+               start = get_timer(0);
+               while (1) {
+                       reg = readl(denali->flash_reg + INTR_STATUS(i));
+                       if (reg & INTR_STATUS__TIME_OUT) {
+                               debug("NAND Reset operation timed out on

bank %d\n", + i);

drivers/mtd/nand/denali.c +4 -0
+                               break;
+                       }
+
+                       /* Reset completed and did not time out, all

good. */ + if (reg & INTR_STATUS__RST_COMP)

drivers/mtd/nand/denali.c +4 -0
+                               break;
+
+                       if (get_timer(start) > (CONFIG_SYS_HZ / 1000)) {
+                               debug("%s: Reset timed out!\n",

__func__); + break;

drivers/mtd/nand/denali.c

Quoted 2 lines
drivers/mtd/nand/denali.c +2 -0
+                       }
+               }

I feel it is too much here
about using get_timer() & CONFIG_SYS_HZ.

Quoted 1 line

How about iterating udelay(1) up to reasonable times?

The get_timer() provides more precise delay , in this case, it's 1 sec . Using just udelay() doesn't provide such precise control over the delay. Moreover, I believe the get_timer() method is the one agreed upon for implementing delays.

Quoted 1 line

See the wait_for_irq() function in this file.

I'd like to convert this one to wait_for_bit() once that hits mainline.

#3 lore raw

PATCH 1/1 mtd: nand: denali: Nuke unbounded loop

Hi Marek,

2015-12-21 19:16 GMT+09:00 Marek Vasut <[email protected]>:

Quoted 76 lines · Masahiro Yamada
Quoted 1 line

Hi Marek,

Hi,

Quoted 71 lines

2015-12-20 11:59 GMT+09:00 Marek Vasut <[email protected]>:

Quoted 64 lines

This particular unbounded loop in the Denali NAND reset routine can lead to a system hang in case neither of the Timeout and Completed bits get set.

Refactor the code a bit so it's readable and implement timer so the loop is bounded instead. This way the complete hang can be prevented even if the NAND fails to reset.

Signed-off-by Marek Vasut <[email protected]>
Cc Masahiro Yamada <[email protected]>
Cc Scott Wood <[email protected]>

---

 drivers/mtd/nand/denali.c | 26 ++++++++++++++++++++------
 1 file changed, 20 insertions(+), 6 deletions(-)
drivers/mtd/nand/denali.c +7 -5
index 192be7d..8a8cca9 100644
@@ -199,6 +199,7 @@ static void reset_bank(struct denali_nand_info
 *denali)
 
 static uint32_t denali_nand_reset(struct denali_nand_info *denali)
 {
 
        int i;
 
+       u32 start, reg;
 
        for (i = 0; i < denali->max_banks; i++)
 
                writel(INTR_STATUS__RST_COMP | INTR_STATUS__TIME_OUT,

@@ -206,12 +207,25 @@ static uint32_t denali_nand_reset(struct
 denali_nand_info *denali)
 
        for (i = 0; i < denali->max_banks; i++) {
 
                writel(1 << i, denali->flash_reg + DEVICE_RESET);
 
-               while (!(readl(denali->flash_reg + INTR_STATUS(i)) &
-                       (INTR_STATUS__RST_COMP | INTR_STATUS__TIME_OUT)))
-                       if (readl(denali->flash_reg + INTR_STATUS(i)) &
-                               INTR_STATUS__TIME_OUT)
-                               debug("NAND Reset operation timed out on
 bank" -                                     " %d\n", i);
+
+               start = get_timer(0);
+               while (1) {
+                       reg = readl(denali->flash_reg + INTR_STATUS(i));
+                       if (reg & INTR_STATUS__TIME_OUT) {
+                               debug("NAND Reset operation timed out on

bank %d\n", + i);

drivers/mtd/nand/denali.c +4 -0
+                               break;
+                       }
+
+                       /* Reset completed and did not time out, all

good. */ + if (reg & INTR_STATUS__RST_COMP)

drivers/mtd/nand/denali.c +4 -0
+                               break;
+
+                       if (get_timer(start) > (CONFIG_SYS_HZ / 1000)) {
+                               debug("%s: Reset timed out!\n",

__func__); + break;

drivers/mtd/nand/denali.c

Quoted 2 lines
drivers/mtd/nand/denali.c +2 -0
+                       }
+               }

I feel it is too much here
about using get_timer() & CONFIG_SYS_HZ.

Quoted 4 lines
Quoted 1 line

How about iterating udelay(1) up to reasonable times?

The get_timer() provides more precise delay , in this case, it's 1 sec . Using just udelay() doesn't provide such precise control over the delay.

OK. Why do you want to wait precisely 1 sec. Rationale for "1 sec", please?

Quoted 2 lines

Moreover, I believe the get_timer() method is the one agreed upon for implementing delays.

You do not have to use CONFIG_SYS_HZ.

As commented in lib/timer.c, get_timer() returns time in milliseconds.

Quoted 3 lines
Quoted 1 line

See the wait_for_irq() function in this file.

I'd like to convert this one to wait_for_bit() once that hits mainline.

No justice for the conversion.

This function just waits long enough, 1 sec, 2 sec, or whatever.

Signature · 2 lines
Best Regards
Masahiro Yamada
author
#4 lore raw

PATCH 1/1 mtd: nand: denali: Nuke unbounded loop

Quoted 81 lines · Masahiro Yamada

Hi Marek,

2015-12-21 19:16 GMT+09:00 Marek Vasut <[email protected]>:

Quoted 78 lines · Masahiro Yamada
Quoted 1 line

Hi Marek,

Hi,

Quoted 73 lines

2015-12-20 11:59 GMT+09:00 Marek Vasut <[email protected]>:

Quoted 66 lines

This particular unbounded loop in the Denali NAND reset routine can lead to a system hang in case neither of the Timeout and Completed bits get set.

Refactor the code a bit so it's readable and implement timer so the loop is bounded instead. This way the complete hang can be prevented even if the NAND fails to reset.

Signed-off-by Marek Vasut <[email protected]>
Cc Masahiro Yamada <[email protected]>
Cc Scott Wood <[email protected]>

---

 drivers/mtd/nand/denali.c | 26 ++++++++++++++++++++------
 1 file changed, 20 insertions(+), 6 deletions(-)
drivers/mtd/nand/denali.c +1 -2
index 192be7d..8a8cca9 100644
@@ -199,6 +199,7 @@ static void reset_bank(struct denali_nand_info
 *denali)
 
 static uint32_t denali_nand_reset(struct denali_nand_info *denali)
 {
 
        int i;
 
+       u32 start, reg;
 
        for (i = 0; i < denali->max_banks; i++)
 
                writel(INTR_STATUS__RST_COMP | INTR_STATUS__TIME_OUT,

@@ -206,12 +207,25 @@ static uint32_t denali_nand_reset(struct
 denali_nand_info *denali)
 
        for (i = 0; i < denali->max_banks; i++) {
 
                writel(1 << i, denali->flash_reg + DEVICE_RESET);
 
-               while (!(readl(denali->flash_reg + INTR_STATUS(i)) &
-                       (INTR_STATUS__RST_COMP |

INTR_STATUS__TIME_OUT))) - if
(readl(denali->flash_reg + INTR_STATUS(i)) & -
INTR_STATUS__TIME_OUT)
- debug("NAND Reset operation timed out
on bank" - " %d\n", i);

drivers/mtd/nand/denali.c +4 -0
+
+               start = get_timer(0);
+               while (1) {
+                       reg = readl(denali->flash_reg +

INTR_STATUS(i)); + if (reg &
INTR_STATUS__TIME_OUT) {
+ debug("NAND Reset operation timed out
on bank %d\n", + i);

drivers/mtd/nand/denali.c +4 -0
+                               break;
+                       }
+
+                       /* Reset completed and did not time out, all

good. */ + if (reg & INTR_STATUS__RST_COMP)

drivers/mtd/nand/denali.c +3 -0
+                               break;
+
+                       if (get_timer(start) > (CONFIG_SYS_HZ / 1000))

{ + debug("%s: Reset timed out!\n",
__func__); + break;

drivers/mtd/nand/denali.c

Quoted 2 lines
drivers/mtd/nand/denali.c +2 -0
+                       }
+               }

I feel it is too much here
about using get_timer() & CONFIG_SYS_HZ.

Quoted 7 lines
Quoted 4 lines
Quoted 1 line

How about iterating udelay(1) up to reasonable times?

The get_timer() provides more precise delay , in this case, it's 1 sec . Using just udelay() doesn't provide such precise control over the delay.

OK. Why do you want to wait precisely 1 sec. Rationale for "1 sec", please?

There isn't any, 1 second sounds about right for a timeout in my mind.

Quoted 7 lines
Quoted 2 lines

Moreover, I believe the get_timer() method is the one agreed upon for implementing delays.

You do not have to use CONFIG_SYS_HZ.

As commented in lib/timer.c, get_timer() returns time in milliseconds.

Ah, so you'd prefer just 1000 in there instead ?

Quoted 5 lines
Quoted 3 lines
Quoted 1 line

See the wait_for_irq() function in this file.

I'd like to convert this one to wait_for_bit() once that hits mainline.

No justice for the conversion.

It'd better to have one timeout function than multiple implementation of the same thing in multiple drivers, that's all.

Quoted 2 lines

This function just waits long enough, 1 sec, 2 sec, or whatever.

#5 lore raw

PATCH 1/1 mtd: nand: denali: Nuke unbounded loop

Hi Marek,

2015-12-21 19:45 GMT+09:00 Marek Vasut <[email protected]>:

Quoted 95 lines · Masahiro Yamada
Quoted 81 lines

Hi Marek,

2015-12-21 19:16 GMT+09:00 Marek Vasut <[email protected]>:

Quoted 78 lines · Masahiro Yamada
Quoted 1 line

Hi Marek,

Hi,

Quoted 73 lines

2015-12-20 11:59 GMT+09:00 Marek Vasut <[email protected]>:

Quoted 66 lines

This particular unbounded loop in the Denali NAND reset routine can lead to a system hang in case neither of the Timeout and Completed bits get set.

Refactor the code a bit so it's readable and implement timer so the loop is bounded instead. This way the complete hang can be prevented even if the NAND fails to reset.

Signed-off-by Marek Vasut <[email protected]>
Cc Masahiro Yamada <[email protected]>
Cc Scott Wood <[email protected]>

---

 drivers/mtd/nand/denali.c | 26 ++++++++++++++++++++------
 1 file changed, 20 insertions(+), 6 deletions(-)
drivers/mtd/nand/denali.c +1 -2
index 192be7d..8a8cca9 100644
@@ -199,6 +199,7 @@ static void reset_bank(struct denali_nand_info
 *denali)
 
 static uint32_t denali_nand_reset(struct denali_nand_info *denali)
 {
 
        int i;
 
+       u32 start, reg;
 
        for (i = 0; i < denali->max_banks; i++)
 
                writel(INTR_STATUS__RST_COMP | INTR_STATUS__TIME_OUT,

@@ -206,12 +207,25 @@ static uint32_t denali_nand_reset(struct
 denali_nand_info *denali)
 
        for (i = 0; i < denali->max_banks; i++) {
 
                writel(1 << i, denali->flash_reg + DEVICE_RESET);
 
-               while (!(readl(denali->flash_reg + INTR_STATUS(i)) &
-                       (INTR_STATUS__RST_COMP |

INTR_STATUS__TIME_OUT))) - if
(readl(denali->flash_reg + INTR_STATUS(i)) & -
INTR_STATUS__TIME_OUT)
- debug("NAND Reset operation timed out
on bank" - " %d\n", i);

drivers/mtd/nand/denali.c +4 -0
+
+               start = get_timer(0);
+               while (1) {
+                       reg = readl(denali->flash_reg +

INTR_STATUS(i)); + if (reg &
INTR_STATUS__TIME_OUT) {
+ debug("NAND Reset operation timed out
on bank %d\n", + i);

drivers/mtd/nand/denali.c +4 -0
+                               break;
+                       }
+
+                       /* Reset completed and did not time out, all

good. */ + if (reg & INTR_STATUS__RST_COMP)

drivers/mtd/nand/denali.c +3 -0
+                               break;
+
+                       if (get_timer(start) > (CONFIG_SYS_HZ / 1000))

{ + debug("%s: Reset timed out!\n",
__func__); + break;

drivers/mtd/nand/denali.c

Quoted 2 lines
drivers/mtd/nand/denali.c +2 -0
+                       }
+               }

I feel it is too much here
about using get_timer() & CONFIG_SYS_HZ.

Quoted 7 lines
Quoted 4 lines
Quoted 1 line

How about iterating udelay(1) up to reasonable times?

The get_timer() provides more precise delay , in this case, it's 1 sec . Using just udelay() doesn't provide such precise control over the delay.

OK. Why do you want to wait precisely 1 sec. Rationale for "1 sec", please?

There isn't any, 1 second sounds about right for a timeout in my mind.

Quoted 3 lines
Quoted 2 lines

Moreover, I believe the get_timer() method is the one agreed upon for implementing delays.

Quoted 6 lines
Quoted 4 lines

You do not have to use CONFIG_SYS_HZ.

As commented in lib/timer.c, get_timer() returns time in milliseconds.

Ah, so you'd prefer just 1000 in there instead ?

Yes.

Quoted 11 lines
Quoted 5 lines
Quoted 3 lines
Quoted 1 line

See the wait_for_irq() function in this file.

I'd like to convert this one to wait_for_bit() once that hits mainline.

No justice for the conversion.

It'd better to have one timeout function than multiple implementation of the same thing in multiple drivers, that's all.

Quoted 2 lines

This function just waits long enough, 1 sec, 2 sec, or whatever.

I noticed one thing I had missed.

While the CPU is stuck in udelay() function,
it cannot check the register flag that might have been already set. This possibly wastes a small piece of time slice.

So, I am OK with your way
(and also with your next patch for the wait_for_bit() conversion, if you like).

Go ahead!

Signature · 2 lines
Best Regards
Masahiro Yamada
author
#6 lore raw

PATCH 1/1 mtd: nand: denali: Nuke unbounded loop

Quoted 1 line · Masahiro Yamada

Hi Marek,

Hi!

Quoted 99 lines

2015-12-21 19:45 GMT+09:00 Marek Vasut <[email protected]>:

Quoted 98 lines · Masahiro Yamada
Quoted 83 lines

Hi Marek,

2015-12-21 19:16 GMT+09:00 Marek Vasut <[email protected]>:

Quoted 80 lines · Masahiro Yamada
Quoted 1 line

Hi Marek,

Hi,

Quoted 75 lines

2015-12-20 11:59 GMT+09:00 Marek Vasut <[email protected]>:

Quoted 68 lines

This particular unbounded loop in the Denali NAND reset routine can lead to a system hang in case neither of the Timeout and Completed bits get set.

Refactor the code a bit so it's readable and implement timer so the loop is bounded instead. This way the complete hang can be prevented even if the NAND fails to reset.

Signed-off-by Marek Vasut <[email protected]>
Cc Masahiro Yamada <[email protected]>
Cc Scott Wood <[email protected]>

---

 drivers/mtd/nand/denali.c | 26 ++++++++++++++++++++------
 1 file changed, 20 insertions(+), 6 deletions(-)
drivers/mtd/nand/denali.c +1 -1
index 192be7d..8a8cca9 100644
@@ -199,6 +199,7 @@ static void reset_bank(struct denali_nand_info
 *denali)
 
 static uint32_t denali_nand_reset(struct denali_nand_info *denali)
 {
 
        int i;
 
+       u32 start, reg;
 
        for (i = 0; i < denali->max_banks; i++)
 
                writel(INTR_STATUS__RST_COMP |
                INTR_STATUS__TIME_OUT,

@@ -206,12 +207,25 @@ static uint32_t denali_nand_reset(struct
 denali_nand_info *denali)
 
        for (i = 0; i < denali->max_banks; i++) {
 
                writel(1 << i, denali->flash_reg + DEVICE_RESET);
 
-               while (!(readl(denali->flash_reg + INTR_STATUS(i))

& - (INTR_STATUS__RST_COMP |
INTR_STATUS__TIME_OUT))) - if
(readl(denali->flash_reg + INTR_STATUS(i)) & -

INTR_STATUS__TIME_OUT)

- debug("NAND Reset operation timed
out on bank" - " %d\n", i); +

drivers/mtd/nand/denali.c +3 -0
+               start = get_timer(0);
+               while (1) {
+                       reg = readl(denali->flash_reg +

INTR_STATUS(i)); + if (reg &
INTR_STATUS__TIME_OUT) {
+ debug("NAND Reset operation timed
out on bank %d\n", + i);

drivers/mtd/nand/denali.c +4 -0
+                               break;
+                       }
+
+                       /* Reset completed and did not time out,

all good. */ + if (reg &
INTR_STATUS__RST_COMP) + break;

drivers/mtd/nand/denali.c +2 -0
+
+                       if (get_timer(start) > (CONFIG_SYS_HZ /

1000)) { + debug("%s: Reset timed
out!\n", __func__); + break;

drivers/mtd/nand/denali.c

Quoted 2 lines
drivers/mtd/nand/denali.c +2 -0
+                       }
+               }

I feel it is too much here
about using get_timer() & CONFIG_SYS_HZ.

Quoted 8 lines
Quoted 5 lines
Quoted 1 line

How about iterating udelay(1) up to reasonable times?

The get_timer() provides more precise delay , in this case, it's 1 sec . Using just udelay() doesn't provide such precise control over the delay.

OK. Why do you want to wait precisely 1 sec. Rationale for "1 sec", please?

There isn't any, 1 second sounds about right for a timeout in my mind.

Quoted 3 lines
Quoted 2 lines

Moreover, I believe the get_timer() method is the one agreed upon for implementing delays.

Quoted 8 lines
Quoted 6 lines
Quoted 4 lines

You do not have to use CONFIG_SYS_HZ.

As commented in lib/timer.c, get_timer() returns time in milliseconds.

Ah, so you'd prefer just 1000 in there instead ?

Yes.

Roger, makes sense, will do.

Quoted 10 lines
Quoted 10 lines
Quoted 6 lines
Quoted 4 lines
Quoted 1 line

See the wait_for_irq() function in this file.

I'd like to convert this one to wait_for_bit() once that hits mainline.

No justice for the conversion.

It'd better to have one timeout function than multiple implementation of the same thing in multiple drivers, that's all.

Quoted 8 lines
Quoted 2 lines
Quoted 2 lines

This function just waits long enough, 1 sec, 2 sec, or whatever.

I noticed one thing I had missed.

While the CPU is stuck in udelay() function,
it cannot check the register flag that might have been already set. This possibly wastes a small piece of time slice.

Well yeah, but that's such a small timeslice that it's not very important.

Quoted 5 lines

So, I am OK with your way
(and also with your next patch for the wait_for_bit() conversion, if you like).

Go ahead!

THanks ;-)