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
> <[email protected]>.

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 [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to