[U-Boot] [PATCH] mtd: nand: denali: Nuke unbounded loop

[Masahiro Yamada]

Hi Marek,



2015-12-20 11:59 GMT+09:00 Marek Vasut <marex at denx.de>:
> 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 <marex at denx.de>
> Cc: Masahiro Yamada <yamada.masahiro at socionext.com>
> Cc: Scott Wood <scottwood at freescale.com>
> ---
>  drivers/mtd/nand/denali.c | 26 ++++++++++++++++++++------
>  1 file changed, 20 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/mtd/nand/denali.c b/drivers/mtd/nand/denali.c
> index 192be7d..8a8cca9 100644
> --- a/drivers/mtd/nand/denali.c
> +++ b/drivers/mtd/nand/denali.c
> @@ -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;
> +                       }
> +               }


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.



-- 
Best Regards
Masahiro Yamada