[PATCH 0/3] OMAP clock and PM architecture fixes for 2.6.32-rc6

[PATCH 0/3] OMAP clock and PM architecture fixes for 2.6.32-rc6

[Paul Walmsley]

Hi,

this series contains fixes for some clock and PM architecture components
for the 2.6.32-rc series.

- Paul

---

Paul Walmsley (1):
      OMAP clock/hwmod: fix off-by-one errors

Rajendra Nayak (1):
      OMAP3 clock: Fix the DPLL freqsel computations

Tero Kristo (1):
      OMAP3: Clock: Fixed dpll3_m2x2 rate calculation


 arch/arm/mach-omap2/clock34xx.c  |    2 +-
 arch/arm/mach-omap2/clock34xx.h  |    4 ++--
 arch/arm/mach-omap2/cm.c         |    2 +-
 arch/arm/mach-omap2/omap_hwmod.c |   12 +++++-------
 arch/arm/mach-omap2/prcm.c       |    4 ++--
 5 files changed, 11 insertions(+), 13 deletions(-)

[PATCH 0/3] OMAP clock and PM architecture fixes for 2.6.32-rc6

[Paul Walmsley]

Hi,

this series contains fixes for some clock and PM architecture components
for the 2.6.32-rc series.

- Paul

---

Paul Walmsley (1):
      OMAP clock/hwmod: fix off-by-one errors

Rajendra Nayak (1):
      OMAP3 clock: Fix the DPLL freqsel computations

Tero Kristo (1):
      OMAP3: Clock: Fixed dpll3_m2x2 rate calculation


 arch/arm/mach-omap2/clock34xx.c  |    2 +-
 arch/arm/mach-omap2/clock34xx.h  |    4 ++--
 arch/arm/mach-omap2/cm.c         |    2 +-
 arch/arm/mach-omap2/omap_hwmod.c |   12 +++++-------
 arch/arm/mach-omap2/prcm.c       |    4 ++--
 5 files changed, 11 insertions(+), 13 deletions(-)

[PATCH 1/3] OMAP3 clock: Fix the DPLL freqsel computations

[Paul Walmsley]

From: Rajendra Nayak <rnayak@ti.com>

Fix the freqsel value computation. Use n instead of (n+1)

The formula in the TRM uses a zero-based N, hence the (n+1); however
at this point in the clock34xx.c code, N is one-based.

Hayati Bayrakdar <h-bayrakdar@ti.com> and Nishanth Menon <nm@ti.com> helped
track down this bug.

Signed-off-by: Rajendra Nayak <rnayak@ti.com>
Signed-off-by: Paul Walmsley <paul@pwsan.com>
[paul@pwsan.com: modified commit message]
Cc: Hayati Bayrakdar <h-bayrakdar@ti.com>
Cc: Nishanth Menon <nm@ti.com>
---
 arch/arm/mach-omap2/clock34xx.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-omap2/clock34xx.c b/arch/arm/mach-omap2/clock34xx.c
index c258f87..225c1a7 100644
--- a/arch/arm/mach-omap2/clock34xx.c
+++ b/arch/arm/mach-omap2/clock34xx.c
@@ -473,7 +473,7 @@ static u16 _omap3_dpll_compute_freqsel(struct clk *clk, u8 n)
 	unsigned long fint;
 	u16 f = 0;
 
-	fint = clk->dpll_data->clk_ref->rate / (n + 1);
+	fint = clk->dpll_data->clk_ref->rate / n;
 
 	pr_debug("clock: fint is %lu\n", fint);
 

[PATCH 1/3] OMAP3 clock: Fix the DPLL freqsel computations

[Paul Walmsley]

From: Rajendra Nayak <rnayak@ti.com>

Fix the freqsel value computation. Use n instead of (n+1)

The formula in the TRM uses a zero-based N, hence the (n+1); however
at this point in the clock34xx.c code, N is one-based.

Hayati Bayrakdar <h-bayrakdar@ti.com> and Nishanth Menon <nm@ti.com> helped
track down this bug.

Signed-off-by: Rajendra Nayak <rnayak@ti.com>
Signed-off-by: Paul Walmsley <paul@pwsan.com>
[paul at pwsan.com: modified commit message]
Cc: Hayati Bayrakdar <h-bayrakdar@ti.com>
Cc: Nishanth Menon <nm@ti.com>
---
 arch/arm/mach-omap2/clock34xx.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-omap2/clock34xx.c b/arch/arm/mach-omap2/clock34xx.c
index c258f87..225c1a7 100644
--- a/arch/arm/mach-omap2/clock34xx.c
+++ b/arch/arm/mach-omap2/clock34xx.c
@@ -473,7 +473,7 @@ static u16 _omap3_dpll_compute_freqsel(struct clk *clk, u8 n)
 	unsigned long fint;
 	u16 f = 0;
 
-	fint = clk->dpll_data->clk_ref->rate / (n + 1);
+	fint = clk->dpll_data->clk_ref->rate / n;
 
 	pr_debug("clock: fint is %lu\n", fint);
 

[APPLIED] [PATCH 1/3] OMAP3 clock: Fix the DPLL freqsel computations

[Tony Lindgren]

This patch has been applied to the linux-omap
by youw fwiendly patch wobot.

Branch in linux-omap: omap-fixes

Initial commit ID (Likely to change): 8c3b128f6b50f9b8a6b015afeead77fd6de366a6

PatchWorks
http://patchwork.kernel.org/patch/60367/

Git (Likely to change, and takes a while to get mirrored)
http://git.kernel.org/?p=linux/kernel/git/tmlind/linux-omap-2.6.git;a=commit;h=8c3b128f6b50f9b8a6b015afeead77fd6de366a6

[PATCH 2/3] OMAP3: Clock: Fixed dpll3_m2x2 rate calculation

[Paul Walmsley]

From: Tero Kristo <tero.kristo@nokia.com>

Current calculation does not take into account any changes to M2 divisor, and
thus when we change VDD2 OPP, dpll3_m2x2 rate does not change. Fixed by
re-routing dpll3_m2x2 parent to dpll3_m2.

Signed-off-by: Tero Kristo <tero.kristo@nokia.com>
Signed-off-by: Paul Walmsley <paul@pwsan.com>
---
 arch/arm/mach-omap2/clock34xx.h |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-omap2/clock34xx.h b/arch/arm/mach-omap2/clock34xx.h
index a1b3de7..8fe1bcb 100644
--- a/arch/arm/mach-omap2/clock34xx.h
+++ b/arch/arm/mach-omap2/clock34xx.h
@@ -489,9 +489,9 @@ static struct clk core_ck = {
 static struct clk dpll3_m2x2_ck = {
 	.name		= "dpll3_m2x2_ck",
 	.ops		= &clkops_null,
-	.parent		= &dpll3_x2_ck,
+	.parent		= &dpll3_m2_ck,
 	.clkdm_name	= "dpll3_clkdm",
-	.recalc		= &followparent_recalc,
+	.recalc		= &omap3_clkoutx2_recalc,
 };
 
 /* The PWRDN bit is apparently only available on 3430ES2 and above */

[PATCH 2/3] OMAP3: Clock: Fixed dpll3_m2x2 rate calculation

[Paul Walmsley]

From: Tero Kristo <tero.kristo@nokia.com>

Current calculation does not take into account any changes to M2 divisor, and
thus when we change VDD2 OPP, dpll3_m2x2 rate does not change. Fixed by
re-routing dpll3_m2x2 parent to dpll3_m2.

Signed-off-by: Tero Kristo <tero.kristo@nokia.com>
Signed-off-by: Paul Walmsley <paul@pwsan.com>
---
 arch/arm/mach-omap2/clock34xx.h |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-omap2/clock34xx.h b/arch/arm/mach-omap2/clock34xx.h
index a1b3de7..8fe1bcb 100644
--- a/arch/arm/mach-omap2/clock34xx.h
+++ b/arch/arm/mach-omap2/clock34xx.h
@@ -489,9 +489,9 @@ static struct clk core_ck = {
 static struct clk dpll3_m2x2_ck = {
 	.name		= "dpll3_m2x2_ck",
 	.ops		= &clkops_null,
-	.parent		= &dpll3_x2_ck,
+	.parent		= &dpll3_m2_ck,
 	.clkdm_name	= "dpll3_clkdm",
-	.recalc		= &followparent_recalc,
+	.recalc		= &omap3_clkoutx2_recalc,
 };
 
 /* The PWRDN bit is apparently only available on 3430ES2 and above */

[APPLIED] [PATCH 2/3] OMAP3: Clock: Fixed dpll3_m2x2 rate calculation

[Tony Lindgren]

This patch has been applied to the linux-omap
by youw fwiendly patch wobot.

Branch in linux-omap: omap-fixes

Initial commit ID (Likely to change): 2dcbc9ebbe46c92e748e6180133d10cce69acb4d

PatchWorks
http://patchwork.kernel.org/patch/60368/

Git (Likely to change, and takes a while to get mirrored)
http://git.kernel.org/?p=linux/kernel/git/tmlind/linux-omap-2.6.git;a=commit;h=2dcbc9ebbe46c92e748e6180133d10cce69acb4d

[PATCH 3/3] OMAP clock/hwmod: fix off-by-one errors

[Paul Walmsley]

Fix loop bailout off-by-one bugs reported by Juha Leppänen
<juha_motorsportcom@luukku.com>.

Signed-off-by: Paul Walmsley <paul@pwsan.com>
Cc: Juha Leppänen <juha_motorsportcom@luukku.com>
---
 arch/arm/mach-omap2/cm.c         |    2 +-
 arch/arm/mach-omap2/omap_hwmod.c |   12 +++++-------
 arch/arm/mach-omap2/prcm.c       |    4 ++--
 3 files changed, 8 insertions(+), 10 deletions(-)

diff --git a/arch/arm/mach-omap2/cm.c b/arch/arm/mach-omap2/cm.c
index 8eb2dab..0766e52 100644
--- a/arch/arm/mach-omap2/cm.c
+++ b/arch/arm/mach-omap2/cm.c
@@ -65,6 +65,6 @@ int omap2_cm_wait_module_ready(s16 prcm_mod, u8 idlest_id, u8 idlest_shift)
 	       (i++ < MAX_MODULE_READY_TIME))
 		udelay(1);
 
-	return (i < MAX_MODULE_READY_TIME) ? 0 : -EBUSY;
+	return (i <= MAX_MODULE_READY_TIME) ? 0 : -EBUSY;
 }
 
diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
index 633b216..a4a9518 100644
--- a/arch/arm/mach-omap2/omap_hwmod.c
+++ b/arch/arm/mach-omap2/omap_hwmod.c
@@ -759,14 +759,12 @@ static int _reset(struct omap_hwmod *oh)
 	_write_sysconfig(v, oh);
 
 	c = 0;
-	while (c < MAX_MODULE_RESET_WAIT &&
-	       !(omap_hwmod_readl(oh, oh->sysconfig->syss_offs) &
-		 SYSS_RESETDONE_MASK)) {
+	while (!(omap_hwmod_readl(oh, oh->sysconfig->syss_offs) &
+		 SYSS_RESETDONE_MASK) &&
+	       (c++ < MAX_MODULE_RESET_WAIT))
 		udelay(1);
-		c++;
-	}
 
-	if (c == MAX_MODULE_RESET_WAIT)
+	if (c > MAX_MODULE_RESET_WAIT)
 		WARN(1, "omap_hwmod: %s: failed to reset in %d usec\n",
 		     oh->name, MAX_MODULE_RESET_WAIT);
 	else
@@ -777,7 +775,7 @@ static int _reset(struct omap_hwmod *oh)
 	 * _wait_target_ready() or _reset()
 	 */
 
-	return (c == MAX_MODULE_RESET_WAIT) ? -ETIMEDOUT : 0;
+	return (c > MAX_MODULE_RESET_WAIT) ? -ETIMEDOUT : 0;
 }
 
 /**
diff --git a/arch/arm/mach-omap2/prcm.c b/arch/arm/mach-omap2/prcm.c
index 029d376..d486709 100644
--- a/arch/arm/mach-omap2/prcm.c
+++ b/arch/arm/mach-omap2/prcm.c
@@ -251,14 +251,14 @@ int omap2_cm_wait_idlest(void __iomem *reg, u32 mask, const char *name)
 	       (i++ < MAX_MODULE_ENABLE_WAIT))
 		udelay(1);
 
-	if (i < MAX_MODULE_ENABLE_WAIT)
+	if (i <= MAX_MODULE_ENABLE_WAIT)
 		pr_debug("cm: Module associated with clock %s ready after %d "
 			 "loops\n", name, i);
 	else
 		pr_err("cm: Module associated with clock %s didn't enable in "
 		       "%d tries\n", name, MAX_MODULE_ENABLE_WAIT);
 
-	return (i < MAX_MODULE_ENABLE_WAIT) ? 1 : 0;
+	return (i <= MAX_MODULE_ENABLE_WAIT) ? 1 : 0;
 };
 
 void __init omap2_set_globals_prcm(struct omap_globals *omap2_globals)


--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 3/3] OMAP clock/hwmod: fix off-by-one errors

[Paul Walmsley]

Fix loop bailout off-by-one bugs reported by Juha Lepp?nen
<juha_motorsportcom@luukku.com>.

Signed-off-by: Paul Walmsley <paul@pwsan.com>
Cc: Juha Lepp?nen <juha_motorsportcom@luukku.com>
---
 arch/arm/mach-omap2/cm.c         |    2 +-
 arch/arm/mach-omap2/omap_hwmod.c |   12 +++++-------
 arch/arm/mach-omap2/prcm.c       |    4 ++--
 3 files changed, 8 insertions(+), 10 deletions(-)

diff --git a/arch/arm/mach-omap2/cm.c b/arch/arm/mach-omap2/cm.c
index 8eb2dab..0766e52 100644
--- a/arch/arm/mach-omap2/cm.c
+++ b/arch/arm/mach-omap2/cm.c
@@ -65,6 +65,6 @@ int omap2_cm_wait_module_ready(s16 prcm_mod, u8 idlest_id, u8 idlest_shift)
 	       (i++ < MAX_MODULE_READY_TIME))
 		udelay(1);
 
-	return (i < MAX_MODULE_READY_TIME) ? 0 : -EBUSY;
+	return (i <= MAX_MODULE_READY_TIME) ? 0 : -EBUSY;
 }
 
diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
index 633b216..a4a9518 100644
--- a/arch/arm/mach-omap2/omap_hwmod.c
+++ b/arch/arm/mach-omap2/omap_hwmod.c
@@ -759,14 +759,12 @@ static int _reset(struct omap_hwmod *oh)
 	_write_sysconfig(v, oh);
 
 	c = 0;
-	while (c < MAX_MODULE_RESET_WAIT &&
-	       !(omap_hwmod_readl(oh, oh->sysconfig->syss_offs) &
-		 SYSS_RESETDONE_MASK)) {
+	while (!(omap_hwmod_readl(oh, oh->sysconfig->syss_offs) &
+		 SYSS_RESETDONE_MASK) &&
+	       (c++ < MAX_MODULE_RESET_WAIT))
 		udelay(1);
-		c++;
-	}
 
-	if (c == MAX_MODULE_RESET_WAIT)
+	if (c > MAX_MODULE_RESET_WAIT)
 		WARN(1, "omap_hwmod: %s: failed to reset in %d usec\n",
 		     oh->name, MAX_MODULE_RESET_WAIT);
 	else
@@ -777,7 +775,7 @@ static int _reset(struct omap_hwmod *oh)
 	 * _wait_target_ready() or _reset()
 	 */
 
-	return (c == MAX_MODULE_RESET_WAIT) ? -ETIMEDOUT : 0;
+	return (c > MAX_MODULE_RESET_WAIT) ? -ETIMEDOUT : 0;
 }
 
 /**
diff --git a/arch/arm/mach-omap2/prcm.c b/arch/arm/mach-omap2/prcm.c
index 029d376..d486709 100644
--- a/arch/arm/mach-omap2/prcm.c
+++ b/arch/arm/mach-omap2/prcm.c
@@ -251,14 +251,14 @@ int omap2_cm_wait_idlest(void __iomem *reg, u32 mask, const char *name)
 	       (i++ < MAX_MODULE_ENABLE_WAIT))
 		udelay(1);
 
-	if (i < MAX_MODULE_ENABLE_WAIT)
+	if (i <= MAX_MODULE_ENABLE_WAIT)
 		pr_debug("cm: Module associated with clock %s ready after %d "
 			 "loops\n", name, i);
 	else
 		pr_err("cm: Module associated with clock %s didn't enable in "
 		       "%d tries\n", name, MAX_MODULE_ENABLE_WAIT);
 
-	return (i < MAX_MODULE_ENABLE_WAIT) ? 1 : 0;
+	return (i <= MAX_MODULE_ENABLE_WAIT) ? 1 : 0;
 };
 
 void __init omap2_set_globals_prcm(struct omap_globals *omap2_globals)

Re: [PATCH 3/3] OMAP clock/hwmod: fix off-by-one errors

[Russell King - ARM Linux]

On Mon, Nov 16, 2009 at 06:36:55AM -0700, Paul Walmsley wrote:
> Fix loop bailout off-by-one bugs reported by Juha Leppänen
> <juha_motorsportcom@luukku.com>.

I'm not sure the new code is any easier to read than the old code.
And what with some checks post-loop being <= and others being >, it's
a recipe for cut'n'paste errors happening.

> diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
> index 633b216..a4a9518 100644
> --- a/arch/arm/mach-omap2/omap_hwmod.c
> +++ b/arch/arm/mach-omap2/omap_hwmod.c
> @@ -759,14 +759,12 @@ static int _reset(struct omap_hwmod *oh)
>  	_write_sysconfig(v, oh);
>  
>  	c = 0;
> -	while (c < MAX_MODULE_RESET_WAIT &&
> -	       !(omap_hwmod_readl(oh, oh->sysconfig->syss_offs) &
> -		 SYSS_RESETDONE_MASK)) {
> +	while (!(omap_hwmod_readl(oh, oh->sysconfig->syss_offs) &
> +		 SYSS_RESETDONE_MASK) &&
> +	       (c++ < MAX_MODULE_RESET_WAIT))
>  		udelay(1);
> -		c++;
> -	}
>  
> -	if (c == MAX_MODULE_RESET_WAIT)
> +	if (c > MAX_MODULE_RESET_WAIT)
>  		WARN(1, "omap_hwmod: %s: failed to reset in %d usec\n",
>  		     oh->name, MAX_MODULE_RESET_WAIT);
>  	else

How about:

	for (c = 0; c <= MAX_MODULE_RESET_WAIT; c++) {
		if (omap_hwmod_readl(oh, oh->sysconfig->syss_offs) &
		    SYSS_RESETDONE_MASK) {
			pr_debug("omap_hwmod: %s: reset in %d usec\n",
				oh->name, c);
			return 0;
		}
	}

	WARN(1, "omap_hwmod: %s: failed to reset in %d usec\n",
                     oh->name, c - 1);
	return -ETIMEDOUT;

?

Even better would be to turn this into a helper much like wait_event(),
which would stop silly mistakes happening.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 3/3] OMAP clock/hwmod: fix off-by-one errors

[Russell King - ARM Linux]

On Mon, Nov 16, 2009 at 06:36:55AM -0700, Paul Walmsley wrote:
> Fix loop bailout off-by-one bugs reported by Juha Lepp?nen
> <juha_motorsportcom@luukku.com>.

I'm not sure the new code is any easier to read than the old code.
And what with some checks post-loop being <= and others being >, it's
a recipe for cut'n'paste errors happening.

> diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
> index 633b216..a4a9518 100644
> --- a/arch/arm/mach-omap2/omap_hwmod.c
> +++ b/arch/arm/mach-omap2/omap_hwmod.c
> @@ -759,14 +759,12 @@ static int _reset(struct omap_hwmod *oh)
>  	_write_sysconfig(v, oh);
>  
>  	c = 0;
> -	while (c < MAX_MODULE_RESET_WAIT &&
> -	       !(omap_hwmod_readl(oh, oh->sysconfig->syss_offs) &
> -		 SYSS_RESETDONE_MASK)) {
> +	while (!(omap_hwmod_readl(oh, oh->sysconfig->syss_offs) &
> +		 SYSS_RESETDONE_MASK) &&
> +	       (c++ < MAX_MODULE_RESET_WAIT))
>  		udelay(1);
> -		c++;
> -	}
>  
> -	if (c == MAX_MODULE_RESET_WAIT)
> +	if (c > MAX_MODULE_RESET_WAIT)
>  		WARN(1, "omap_hwmod: %s: failed to reset in %d usec\n",
>  		     oh->name, MAX_MODULE_RESET_WAIT);
>  	else

How about:

	for (c = 0; c <= MAX_MODULE_RESET_WAIT; c++) {
		if (omap_hwmod_readl(oh, oh->sysconfig->syss_offs) &
		    SYSS_RESETDONE_MASK) {
			pr_debug("omap_hwmod: %s: reset in %d usec\n",
				oh->name, c);
			return 0;
		}
	}

	WARN(1, "omap_hwmod: %s: failed to reset in %d usec\n",
                     oh->name, c - 1);
	return -ETIMEDOUT;

?

Even better would be to turn this into a helper much like wait_event(),
which would stop silly mistakes happening.

Re: [PATCH 3/3] OMAP clock/hwmod: fix off-by-one errors

[Paul Walmsley]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 5855 bytes --]

Hi Russell,

On Mon, 16 Nov 2009, Russell King - ARM Linux wrote:

> On Mon, Nov 16, 2009 at 06:36:55AM -0700, Paul Walmsley wrote:
> > Fix loop bailout off-by-one bugs reported by Juha Leppänen
> > <juha_motorsportcom@luukku.com>.
> 
> I'm not sure the new code is any easier to read than the old code.
> And what with some checks post-loop being <= and others being >, it's
> a recipe for cut'n'paste errors happening.

...

> How about:
> 
> 	for (c = 0; c <= MAX_MODULE_RESET_WAIT; c++) {
> 		if (omap_hwmod_readl(oh, oh->sysconfig->syss_offs) &
> 		    SYSS_RESETDONE_MASK) {
> 			pr_debug("omap_hwmod: %s: reset in %d usec\n",
> 				oh->name, c);
> 			return 0;
> 		}
> 	}
> 
> 	WARN(1, "omap_hwmod: %s: failed to reset in %d usec\n",
>                      oh->name, c - 1);
> 	return -ETIMEDOUT;
> 
> ?
> 
> Even better would be to turn this into a helper much like wait_event(),
> which would stop silly mistakes happening.

Agreed.  How about this?  

arch/arm/plat-omap/include/plat/common.h probably isn't the right place 
for it, since it's not OMAP-specific...


- Paul


From 972b3bbf2c210086b862aedb35587484bcefe998 Mon Sep 17 00:00:00 2001
From: Paul Walmsley <paul@pwsan.com>
Date: Mon, 16 Nov 2009 06:13:15 -0700
Subject: [PATCH] OMAP clock/hwmod: fix off-by-one errors
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Fix loop bailout off-by-one bugs reported by Juha Leppänen
<juha_motorsportcom@luukku.com>.

This second version incorporates comments from Russell King
<rmk+kernel@arm.linux.org.uk>.  A new macro, 'omap_test_timeout', has
been created, with cleaner code, and existing code has been converted
to use it.

Signed-off-by: Paul Walmsley <paul@pwsan.com>
Cc: Juha Leppänen <juha_motorsportcom@luukku.com>
Cc: Russell King <rmk+kernel@arm.linux.org.uk>
---
 arch/arm/mach-omap2/cm.c                 |    7 ++++---
 arch/arm/mach-omap2/omap_hwmod.c         |   13 +++++--------
 arch/arm/mach-omap2/prcm.c               |    5 ++---
 arch/arm/plat-omap/include/plat/common.h |   20 ++++++++++++++++++++
 4 files changed, 31 insertions(+), 14 deletions(-)

diff --git a/arch/arm/mach-omap2/cm.c b/arch/arm/mach-omap2/cm.c
index 8eb2dab..58e4a1c 100644
--- a/arch/arm/mach-omap2/cm.c
+++ b/arch/arm/mach-omap2/cm.c
@@ -21,6 +21,8 @@
 
 #include <asm/atomic.h>
 
+#include <plat/common.h>
+
 #include "cm.h"
 #include "cm-regbits-24xx.h"
 #include "cm-regbits-34xx.h"
@@ -61,9 +63,8 @@ int omap2_cm_wait_module_ready(s16 prcm_mod, u8 idlest_id, u8 idlest_shift)
 	mask = 1 << idlest_shift;
 
 	/* XXX should be OMAP2 CM */
-	while (((cm_read_mod_reg(prcm_mod, cm_idlest_reg) & mask) != ena) &&
-	       (i++ < MAX_MODULE_READY_TIME))
-		udelay(1);
+	omap_test_timeout(((cm_read_mod_reg(prcm_mod, cm_idlest_reg) & mask) == ena),
+			  MAX_MODULE_READY_TIME, i);
 
 	return (i < MAX_MODULE_READY_TIME) ? 0 : -EBUSY;
 }
diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
index 633b216..7aaf5f1 100644
--- a/arch/arm/mach-omap2/omap_hwmod.c
+++ b/arch/arm/mach-omap2/omap_hwmod.c
@@ -45,6 +45,7 @@
 #include <linux/mutex.h>
 #include <linux/bootmem.h>
 
+#include <plat/common.h>
 #include <plat/cpu.h>
 #include <plat/clockdomain.h>
 #include <plat/powerdomain.h>
@@ -736,7 +737,7 @@ static int _wait_target_ready(struct omap_hwmod *oh)
 static int _reset(struct omap_hwmod *oh)
 {
 	u32 r, v;
-	int c;
+	int c = 0;
 
 	if (!oh->sysconfig ||
 	    !(oh->sysconfig->sysc_flags & SYSC_HAS_SOFTRESET) ||
@@ -758,13 +759,9 @@ static int _reset(struct omap_hwmod *oh)
 		return r;
 	_write_sysconfig(v, oh);
 
-	c = 0;
-	while (c < MAX_MODULE_RESET_WAIT &&
-	       !(omap_hwmod_readl(oh, oh->sysconfig->syss_offs) &
-		 SYSS_RESETDONE_MASK)) {
-		udelay(1);
-		c++;
-	}
+	omap_test_timeout((omap_hwmod_readl(oh, oh->sysconfig->syss_offs) &
+			   SYSS_RESETDONE_MASK),
+			  MAX_MODULE_RESET_WAIT, c);
 
 	if (c == MAX_MODULE_RESET_WAIT)
 		WARN(1, "omap_hwmod: %s: failed to reset in %d usec\n",
diff --git a/arch/arm/mach-omap2/prcm.c b/arch/arm/mach-omap2/prcm.c
index 029d376..7acd03d 100644
--- a/arch/arm/mach-omap2/prcm.c
+++ b/arch/arm/mach-omap2/prcm.c
@@ -247,9 +247,8 @@ int omap2_cm_wait_idlest(void __iomem *reg, u32 mask, const char *name)
 		BUG();
 
 	/* Wait for lock */
-	while (((__raw_readl(reg) & mask) != ena) &&
-	       (i++ < MAX_MODULE_ENABLE_WAIT))
-		udelay(1);
+	omap_test_timeout(((__raw_readl(reg) & mask) == ena),
+			  MAX_MODULE_ENABLE_WAIT, i);
 
 	if (i < MAX_MODULE_ENABLE_WAIT)
 		pr_debug("cm: Module associated with clock %s ready after %d "
diff --git a/arch/arm/plat-omap/include/plat/common.h b/arch/arm/plat-omap/include/plat/common.h
index 064f173..e977967 100644
--- a/arch/arm/plat-omap/include/plat/common.h
+++ b/arch/arm/plat-omap/include/plat/common.h
@@ -71,4 +71,24 @@ void omap2_set_globals_sdrc(struct omap_globals *);
 void omap2_set_globals_control(struct omap_globals *);
 void omap2_set_globals_prcm(struct omap_globals *);
 
+/**
+ * omap_test_timeout - busy-loop, testing a condition
+ * @cond: condition to test until it evaluates to true
+ * @timeout: maximum number of microseconds in the timeout
+ * @index: loop index (integer)
+ *
+ * Loop waiting for @cond to become true or until at least @timeout
+ * microseconds have passed.  To use, define some integer @index in the
+ * calling code.  After running, if @index == @timeout, then the loop has
+ * timed out.
+ */
+#define omap_test_timeout(cond, timeout, index)			\
+({								\
+	for (index = 0; index < timeout; index++) {		\
+		if (cond)					\
+			break;					\
+		udelay(1);					\
+	}							\
+})
+
 #endif /* __ARCH_ARM_MACH_OMAP_COMMON_H */
-- 
1.6.5.GIT

[PATCH 3/3] OMAP clock/hwmod: fix off-by-one errors

[Paul Walmsley]

Hi Russell,

On Mon, 16 Nov 2009, Russell King - ARM Linux wrote:

> On Mon, Nov 16, 2009 at 06:36:55AM -0700, Paul Walmsley wrote:
> > Fix loop bailout off-by-one bugs reported by Juha Lepp?nen
> > <juha_motorsportcom@luukku.com>.
> 
> I'm not sure the new code is any easier to read than the old code.
> And what with some checks post-loop being <= and others being >, it's
> a recipe for cut'n'paste errors happening.

...

> How about:
> 
> 	for (c = 0; c <= MAX_MODULE_RESET_WAIT; c++) {
> 		if (omap_hwmod_readl(oh, oh->sysconfig->syss_offs) &
> 		    SYSS_RESETDONE_MASK) {
> 			pr_debug("omap_hwmod: %s: reset in %d usec\n",
> 				oh->name, c);
> 			return 0;
> 		}
> 	}
> 
> 	WARN(1, "omap_hwmod: %s: failed to reset in %d usec\n",
>                      oh->name, c - 1);
> 	return -ETIMEDOUT;
> 
> ?
> 
> Even better would be to turn this into a helper much like wait_event(),
> which would stop silly mistakes happening.

Agreed.  How about this?  

arch/arm/plat-omap/include/plat/common.h probably isn't the right place 
for it, since it's not OMAP-specific...


- Paul


>From 972b3bbf2c210086b862aedb35587484bcefe998 Mon Sep 17 00:00:00 2001
From: Paul Walmsley <paul@pwsan.com>
Date: Mon, 16 Nov 2009 06:13:15 -0700
Subject: [PATCH] OMAP clock/hwmod: fix off-by-one errors
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Fix loop bailout off-by-one bugs reported by Juha Lepp?nen
<juha_motorsportcom@luukku.com>.

This second version incorporates comments from Russell King
<rmk+kernel@arm.linux.org.uk>.  A new macro, 'omap_test_timeout', has
been created, with cleaner code, and existing code has been converted
to use it.

Signed-off-by: Paul Walmsley <paul@pwsan.com>
Cc: Juha Lepp?nen <juha_motorsportcom@luukku.com>
Cc: Russell King <rmk+kernel@arm.linux.org.uk>
---
 arch/arm/mach-omap2/cm.c                 |    7 ++++---
 arch/arm/mach-omap2/omap_hwmod.c         |   13 +++++--------
 arch/arm/mach-omap2/prcm.c               |    5 ++---
 arch/arm/plat-omap/include/plat/common.h |   20 ++++++++++++++++++++
 4 files changed, 31 insertions(+), 14 deletions(-)

diff --git a/arch/arm/mach-omap2/cm.c b/arch/arm/mach-omap2/cm.c
index 8eb2dab..58e4a1c 100644
--- a/arch/arm/mach-omap2/cm.c
+++ b/arch/arm/mach-omap2/cm.c
@@ -21,6 +21,8 @@
 
 #include <asm/atomic.h>
 
+#include <plat/common.h>
+
 #include "cm.h"
 #include "cm-regbits-24xx.h"
 #include "cm-regbits-34xx.h"
@@ -61,9 +63,8 @@ int omap2_cm_wait_module_ready(s16 prcm_mod, u8 idlest_id, u8 idlest_shift)
 	mask = 1 << idlest_shift;
 
 	/* XXX should be OMAP2 CM */
-	while (((cm_read_mod_reg(prcm_mod, cm_idlest_reg) & mask) != ena) &&
-	       (i++ < MAX_MODULE_READY_TIME))
-		udelay(1);
+	omap_test_timeout(((cm_read_mod_reg(prcm_mod, cm_idlest_reg) & mask) == ena),
+			  MAX_MODULE_READY_TIME, i);
 
 	return (i < MAX_MODULE_READY_TIME) ? 0 : -EBUSY;
 }
diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
index 633b216..7aaf5f1 100644
--- a/arch/arm/mach-omap2/omap_hwmod.c
+++ b/arch/arm/mach-omap2/omap_hwmod.c
@@ -45,6 +45,7 @@
 #include <linux/mutex.h>
 #include <linux/bootmem.h>
 
+#include <plat/common.h>
 #include <plat/cpu.h>
 #include <plat/clockdomain.h>
 #include <plat/powerdomain.h>
@@ -736,7 +737,7 @@ static int _wait_target_ready(struct omap_hwmod *oh)
 static int _reset(struct omap_hwmod *oh)
 {
 	u32 r, v;
-	int c;
+	int c = 0;
 
 	if (!oh->sysconfig ||
 	    !(oh->sysconfig->sysc_flags & SYSC_HAS_SOFTRESET) ||
@@ -758,13 +759,9 @@ static int _reset(struct omap_hwmod *oh)
 		return r;
 	_write_sysconfig(v, oh);
 
-	c = 0;
-	while (c < MAX_MODULE_RESET_WAIT &&
-	       !(omap_hwmod_readl(oh, oh->sysconfig->syss_offs) &
-		 SYSS_RESETDONE_MASK)) {
-		udelay(1);
-		c++;
-	}
+	omap_test_timeout((omap_hwmod_readl(oh, oh->sysconfig->syss_offs) &
+			   SYSS_RESETDONE_MASK),
+			  MAX_MODULE_RESET_WAIT, c);
 
 	if (c == MAX_MODULE_RESET_WAIT)
 		WARN(1, "omap_hwmod: %s: failed to reset in %d usec\n",
diff --git a/arch/arm/mach-omap2/prcm.c b/arch/arm/mach-omap2/prcm.c
index 029d376..7acd03d 100644
--- a/arch/arm/mach-omap2/prcm.c
+++ b/arch/arm/mach-omap2/prcm.c
@@ -247,9 +247,8 @@ int omap2_cm_wait_idlest(void __iomem *reg, u32 mask, const char *name)
 		BUG();
 
 	/* Wait for lock */
-	while (((__raw_readl(reg) & mask) != ena) &&
-	       (i++ < MAX_MODULE_ENABLE_WAIT))
-		udelay(1);
+	omap_test_timeout(((__raw_readl(reg) & mask) == ena),
+			  MAX_MODULE_ENABLE_WAIT, i);
 
 	if (i < MAX_MODULE_ENABLE_WAIT)
 		pr_debug("cm: Module associated with clock %s ready after %d "
diff --git a/arch/arm/plat-omap/include/plat/common.h b/arch/arm/plat-omap/include/plat/common.h
index 064f173..e977967 100644
--- a/arch/arm/plat-omap/include/plat/common.h
+++ b/arch/arm/plat-omap/include/plat/common.h
@@ -71,4 +71,24 @@ void omap2_set_globals_sdrc(struct omap_globals *);
 void omap2_set_globals_control(struct omap_globals *);
 void omap2_set_globals_prcm(struct omap_globals *);
 
+/**
+ * omap_test_timeout - busy-loop, testing a condition
+ * @cond: condition to test until it evaluates to true
+ * @timeout: maximum number of microseconds in the timeout
+ * @index: loop index (integer)
+ *
+ * Loop waiting for @cond to become true or until@least @timeout
+ * microseconds have passed.  To use, define some integer @index in the
+ * calling code.  After running, if @index == @timeout, then the loop has
+ * timed out.
+ */
+#define omap_test_timeout(cond, timeout, index)			\
+({								\
+	for (index = 0; index < timeout; index++) {		\
+		if (cond)					\
+			break;					\
+		udelay(1);					\
+	}							\
+})
+
 #endif /* __ARCH_ARM_MACH_OMAP_COMMON_H */
-- 
1.6.5.GIT

RE: [PATCH 3/3] OMAP clock/hwmod: fix off-by-one errors

[Pandita, Vikram]

Paul

>-----Original Message-----
>From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-owner@vger.kernel.org] On Behalf Of Paul
>Walmsley
>Sent: Tuesday, November 17, 2009 11:39 AM
>To: Russell King - ARM Linux
>Cc: linux-arm-kernel@lists.infradead.org; Juha Leppänen; linux-omap@vger.kernel.org
>Subject: Re: [PATCH 3/3] OMAP clock/hwmod: fix off-by-one errors
<snip>
>
>+/**
>+ * omap_test_timeout - busy-loop, testing a condition
>+ * @cond: condition to test until it evaluates to true
>+ * @timeout: maximum number of microseconds in the timeout
>+ * @index: loop index (integer)
>+ *
>+ * Loop waiting for @cond to become true or until at least @timeout
>+ * microseconds have passed.  To use, define some integer @index in the
>+ * calling code.  After running, if @index == @timeout, then the loop has
>+ * timed out.
>+ */
>+#define omap_test_timeout(cond, timeout, index)			\
>+({								\
>+	for (index = 0; index < timeout; index++) {		\
>+		if (cond)					\
>+			break;					\
>+		udelay(1);					\
>+	}							\
>+})
>+

There is a similar kind of function implemented in USB host, that returns -ETIMEDOUT
Which makes more sense for failure case.

Maybe such a generic function for all omaps is needed.

Refer:
drivers/usb/host/ehci-hcd.c
static int handshake (struct ehci_hcd *ehci, void __iomem *ptr,
                      u32 mask, u32 done, int usec)
{
        u32     result;

        do {
                result = ehci_readl(ehci, ptr);
                if (result == ~(u32)0)          /* card removed */
                        return -ENODEV;
                result &= mask;
                if (result == done)
                        return 0;
                udelay (1);
                usec--;
        } while (usec > 0);
        return -ETIMEDOUT;
}


> #endif /* __ARCH_ARM_MACH_OMAP_COMMON_H */
>--
>1.6.5.GIT
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 3/3] OMAP clock/hwmod: fix off-by-one errors

[Pandita, Vikram]

Paul

>-----Original Message-----
From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-owner at vger.kernel.org] On Behalf Of Paul
>Walmsley
>Sent: Tuesday, November 17, 2009 11:39 AM
>To: Russell King - ARM Linux
>Cc: linux-arm-kernel at lists.infradead.org; Juha Lepp?nen; linux-omap at vger.kernel.org
>Subject: Re: [PATCH 3/3] OMAP clock/hwmod: fix off-by-one errors
<snip>
>
>+/**
>+ * omap_test_timeout - busy-loop, testing a condition
>+ * @cond: condition to test until it evaluates to true
>+ * @timeout: maximum number of microseconds in the timeout
>+ * @index: loop index (integer)
>+ *
>+ * Loop waiting for @cond to become true or until at least @timeout
>+ * microseconds have passed.  To use, define some integer @index in the
>+ * calling code.  After running, if @index == @timeout, then the loop has
>+ * timed out.
>+ */
>+#define omap_test_timeout(cond, timeout, index)			\
>+({								\
>+	for (index = 0; index < timeout; index++) {		\
>+		if (cond)					\
>+			break;					\
>+		udelay(1);					\
>+	}							\
>+})
>+

There is a similar kind of function implemented in USB host, that returns -ETIMEDOUT
Which makes more sense for failure case.

Maybe such a generic function for all omaps is needed.

Refer:
drivers/usb/host/ehci-hcd.c
static int handshake (struct ehci_hcd *ehci, void __iomem *ptr,
                      u32 mask, u32 done, int usec)
{
        u32     result;

        do {
                result = ehci_readl(ehci, ptr);
                if (result == ~(u32)0)          /* card removed */
                        return -ENODEV;
                result &= mask;
                if (result == done)
                        return 0;
                udelay (1);
                usec--;
        } while (usec > 0);
        return -ETIMEDOUT;
}


> #endif /* __ARCH_ARM_MACH_OMAP_COMMON_H */
>--
>1.6.5.GIT

Re: [PATCH 3/3] OMAP clock/hwmod: fix off-by-one errors

[Tony Lindgren]

* Pandita, Vikram <vikram.pandita@ti.com> [091117 09:50]:
> Paul
> 
> >-----Original Message-----
> >From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-owner@vger.kernel.org] On Behalf Of Paul
> >Walmsley
> >Sent: Tuesday, November 17, 2009 11:39 AM
> >To: Russell King - ARM Linux
> >Cc: linux-arm-kernel@lists.infradead.org; Juha Leppänen; linux-omap@vger.kernel.org
> >Subject: Re: [PATCH 3/3] OMAP clock/hwmod: fix off-by-one errors
> <snip>
> >
> >+/**
> >+ * omap_test_timeout - busy-loop, testing a condition
> >+ * @cond: condition to test until it evaluates to true
> >+ * @timeout: maximum number of microseconds in the timeout
> >+ * @index: loop index (integer)
> >+ *
> >+ * Loop waiting for @cond to become true or until at least @timeout
> >+ * microseconds have passed.  To use, define some integer @index in the
> >+ * calling code.  After running, if @index == @timeout, then the loop has
> >+ * timed out.
> >+ */
> >+#define omap_test_timeout(cond, timeout, index)			\
> >+({								\
> >+	for (index = 0; index < timeout; index++) {		\
> >+		if (cond)					\
> >+			break;					\
> >+		udelay(1);					\
> >+	}							\
> >+})
> >+
> 
> There is a similar kind of function implemented in USB host, that returns -ETIMEDOUT
> Which makes more sense for failure case.
> 
> Maybe such a generic function for all omaps is needed.
> 
> Refer:
> drivers/usb/host/ehci-hcd.c
> static int handshake (struct ehci_hcd *ehci, void __iomem *ptr,
>                       u32 mask, u32 done, int usec)
> {
>         u32     result;
> 
>         do {
>                 result = ehci_readl(ehci, ptr);
>                 if (result == ~(u32)0)          /* card removed */
>                         return -ENODEV;
>                 result &= mask;
>                 if (result == done)
>                         return 0;
>                 udelay (1);
>                 usec--;
>         } while (usec > 0);
>         return -ETIMEDOUT;
> }
> 
> 
> > #endif /* __ARCH_ARM_MACH_OMAP_COMMON_H */

Well we need to stick to minimal changes if we plan to send this in as a fix.
Anything more complex we should do the next merge window.

Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 3/3] OMAP clock/hwmod: fix off-by-one errors

[Tony Lindgren]

* Pandita, Vikram <vikram.pandita@ti.com> [091117 09:50]:
> Paul
> 
> >-----Original Message-----
> >From: linux-omap-owner at vger.kernel.org [mailto:linux-omap-owner at vger.kernel.org] On Behalf Of Paul
> >Walmsley
> >Sent: Tuesday, November 17, 2009 11:39 AM
> >To: Russell King - ARM Linux
> >Cc: linux-arm-kernel at lists.infradead.org; Juha Lepp?nen; linux-omap at vger.kernel.org
> >Subject: Re: [PATCH 3/3] OMAP clock/hwmod: fix off-by-one errors
> <snip>
> >
> >+/**
> >+ * omap_test_timeout - busy-loop, testing a condition
> >+ * @cond: condition to test until it evaluates to true
> >+ * @timeout: maximum number of microseconds in the timeout
> >+ * @index: loop index (integer)
> >+ *
> >+ * Loop waiting for @cond to become true or until at least @timeout
> >+ * microseconds have passed.  To use, define some integer @index in the
> >+ * calling code.  After running, if @index == @timeout, then the loop has
> >+ * timed out.
> >+ */
> >+#define omap_test_timeout(cond, timeout, index)			\
> >+({								\
> >+	for (index = 0; index < timeout; index++) {		\
> >+		if (cond)					\
> >+			break;					\
> >+		udelay(1);					\
> >+	}							\
> >+})
> >+
> 
> There is a similar kind of function implemented in USB host, that returns -ETIMEDOUT
> Which makes more sense for failure case.
> 
> Maybe such a generic function for all omaps is needed.
> 
> Refer:
> drivers/usb/host/ehci-hcd.c
> static int handshake (struct ehci_hcd *ehci, void __iomem *ptr,
>                       u32 mask, u32 done, int usec)
> {
>         u32     result;
> 
>         do {
>                 result = ehci_readl(ehci, ptr);
>                 if (result == ~(u32)0)          /* card removed */
>                         return -ENODEV;
>                 result &= mask;
>                 if (result == done)
>                         return 0;
>                 udelay (1);
>                 usec--;
>         } while (usec > 0);
>         return -ETIMEDOUT;
> }
> 
> 
> > #endif /* __ARCH_ARM_MACH_OMAP_COMMON_H */

Well we need to stick to minimal changes if we plan to send this in as a fix.
Anything more complex we should do the next merge window.

Tony

Re: [PATCH 3/3] OMAP clock/hwmod: fix off-by-one errors

[Tony Lindgren]

* Tony Lindgren <tony@atomide.com> [091117 14:24]:
> * Pandita, Vikram <vikram.pandita@ti.com> [091117 09:50]:
> > Paul
> > 
> > >-----Original Message-----
> > >From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-owner@vger.kernel.org] On Behalf Of Paul
> > >Walmsley
> > >Sent: Tuesday, November 17, 2009 11:39 AM
> > >To: Russell King - ARM Linux
> > >Cc: linux-arm-kernel@lists.infradead.org; Juha Leppänen; linux-omap@vger.kernel.org
> > >Subject: Re: [PATCH 3/3] OMAP clock/hwmod: fix off-by-one errors
> > <snip>
> > >
> > >+/**
> > >+ * omap_test_timeout - busy-loop, testing a condition
> > >+ * @cond: condition to test until it evaluates to true
> > >+ * @timeout: maximum number of microseconds in the timeout
> > >+ * @index: loop index (integer)
> > >+ *
> > >+ * Loop waiting for @cond to become true or until at least @timeout
> > >+ * microseconds have passed.  To use, define some integer @index in the
> > >+ * calling code.  After running, if @index == @timeout, then the loop has
> > >+ * timed out.
> > >+ */
> > >+#define omap_test_timeout(cond, timeout, index)			\
> > >+({								\
> > >+	for (index = 0; index < timeout; index++) {		\
> > >+		if (cond)					\
> > >+			break;					\
> > >+		udelay(1);					\
> > >+	}							\
> > >+})
> > >+
> > 
> > There is a similar kind of function implemented in USB host, that returns -ETIMEDOUT
> > Which makes more sense for failure case.
> > 
> > Maybe such a generic function for all omaps is needed.
> > 
> > Refer:
> > drivers/usb/host/ehci-hcd.c
> > static int handshake (struct ehci_hcd *ehci, void __iomem *ptr,
> >                       u32 mask, u32 done, int usec)
> > {
> >         u32     result;
> > 
> >         do {
> >                 result = ehci_readl(ehci, ptr);
> >                 if (result == ~(u32)0)          /* card removed */
> >                         return -ENODEV;
> >                 result &= mask;
> >                 if (result == done)
> >                         return 0;
> >                 udelay (1);
> >                 usec--;
> >         } while (usec > 0);
> >         return -ETIMEDOUT;
> > }
> > 
> > 
> > > #endif /* __ARCH_ARM_MACH_OMAP_COMMON_H */
> 
> Well we need to stick to minimal changes if we plan to send this in as a fix.
> Anything more complex we should do the next merge window.

To me it sounds like this can wait until the merge window,
so not adding to omap-fixes for now.

Regards,

Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 3/3] OMAP clock/hwmod: fix off-by-one errors

[Tony Lindgren]

* Tony Lindgren <tony@atomide.com> [091117 14:24]:
> * Pandita, Vikram <vikram.pandita@ti.com> [091117 09:50]:
> > Paul
> > 
> > >-----Original Message-----
> > >From: linux-omap-owner at vger.kernel.org [mailto:linux-omap-owner at vger.kernel.org] On Behalf Of Paul
> > >Walmsley
> > >Sent: Tuesday, November 17, 2009 11:39 AM
> > >To: Russell King - ARM Linux
> > >Cc: linux-arm-kernel at lists.infradead.org; Juha Lepp?nen; linux-omap at vger.kernel.org
> > >Subject: Re: [PATCH 3/3] OMAP clock/hwmod: fix off-by-one errors
> > <snip>
> > >
> > >+/**
> > >+ * omap_test_timeout - busy-loop, testing a condition
> > >+ * @cond: condition to test until it evaluates to true
> > >+ * @timeout: maximum number of microseconds in the timeout
> > >+ * @index: loop index (integer)
> > >+ *
> > >+ * Loop waiting for @cond to become true or until at least @timeout
> > >+ * microseconds have passed.  To use, define some integer @index in the
> > >+ * calling code.  After running, if @index == @timeout, then the loop has
> > >+ * timed out.
> > >+ */
> > >+#define omap_test_timeout(cond, timeout, index)			\
> > >+({								\
> > >+	for (index = 0; index < timeout; index++) {		\
> > >+		if (cond)					\
> > >+			break;					\
> > >+		udelay(1);					\
> > >+	}							\
> > >+})
> > >+
> > 
> > There is a similar kind of function implemented in USB host, that returns -ETIMEDOUT
> > Which makes more sense for failure case.
> > 
> > Maybe such a generic function for all omaps is needed.
> > 
> > Refer:
> > drivers/usb/host/ehci-hcd.c
> > static int handshake (struct ehci_hcd *ehci, void __iomem *ptr,
> >                       u32 mask, u32 done, int usec)
> > {
> >         u32     result;
> > 
> >         do {
> >                 result = ehci_readl(ehci, ptr);
> >                 if (result == ~(u32)0)          /* card removed */
> >                         return -ENODEV;
> >                 result &= mask;
> >                 if (result == done)
> >                         return 0;
> >                 udelay (1);
> >                 usec--;
> >         } while (usec > 0);
> >         return -ETIMEDOUT;
> > }
> > 
> > 
> > > #endif /* __ARCH_ARM_MACH_OMAP_COMMON_H */
> 
> Well we need to stick to minimal changes if we plan to send this in as a fix.
> Anything more complex we should do the next merge window.

To me it sounds like this can wait until the merge window,
so not adding to omap-fixes for now.

Regards,

Tony

Re: [PATCH 3/3] OMAP clock/hwmod: fix off-by-one errors

[Paul Walmsley]

On Wed, 18 Nov 2009, Tony Lindgren wrote:

> To me it sounds like this can wait until the merge window,
> so not adding to omap-fixes for now.

Indeed, there's no rush on this one, so let's set it up for 2.6.33.
I'll keep that macro in plat/system.h unless you or Russell have a better 
suggestion.  It might be worth looking at the rest of the Linux codebase 
to see if there are other potential users; maybe someone might be willing 
to pick up a "kernel-janitor" task for this?

Vikram, I agree with your comment.  Maybe someone can send a patch to 
clean up ehci-hcd.c and any other users of the same idiom to use this 
macro for 2.6.33?  Looks like also plat-omap/mailbox.c:__mbox_msg_send() 
and mach-omap2/powerdomain.c:pwrdm_wait_transition() would be good 
candidates for this, under arch/arm/*omap*.


- Paul

[PATCH 3/3] OMAP clock/hwmod: fix off-by-one errors

[Paul Walmsley]

On Wed, 18 Nov 2009, Tony Lindgren wrote:

> To me it sounds like this can wait until the merge window,
> so not adding to omap-fixes for now.

Indeed, there's no rush on this one, so let's set it up for 2.6.33.
I'll keep that macro in plat/system.h unless you or Russell have a better 
suggestion.  It might be worth looking at the rest of the Linux codebase 
to see if there are other potential users; maybe someone might be willing 
to pick up a "kernel-janitor" task for this?

Vikram, I agree with your comment.  Maybe someone can send a patch to 
clean up ehci-hcd.c and any other users of the same idiom to use this 
macro for 2.6.33?  Looks like also plat-omap/mailbox.c:__mbox_msg_send() 
and mach-omap2/powerdomain.c:pwrdm_wait_transition() would be good 
candidates for this, under arch/arm/*omap*.


- Paul