[U-Boot] [PATCH v3 4/4] mmc: sdhci: increase the timeout and udelay value

[Andy Fleming]

On Thu, Aug 30, 2012 at 7:24 PM, Jaehoon Chung <jh80.chung at samsung.com> wrote:
> Samsung-SoC is taken the too late to changing the interrupt status register.
> This patch is ensure to check the interrupt status register for Samsung-SoC.
>
> Signed-off-by: Jaehoon Chung <jh80.chung at samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park at samsung.com>
> ---


You should write, here, what you changed in v3.


>  drivers/mmc/sdhci.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c
> index ac39e48..d0b8d24 100644
> --- a/drivers/mmc/sdhci.c
> +++ b/drivers/mmc/sdhci.c
> @@ -83,7 +83,7 @@ static int sdhci_transfer_data(struct sdhci_host *host, struct mmc_data *data,
>  {
>         unsigned int stat, rdy, mask, timeout, block = 0;
>
> -       timeout = 10000;
> +       timeout = 100000;
>         rdy = SDHCI_INT_SPACE_AVAIL | SDHCI_INT_DATA_AVAIL;
>         mask = SDHCI_DATA_AVAILABLE | SDHCI_SPACE_AVAILABLE;
>         do {
> @@ -110,7 +110,7 @@ static int sdhci_transfer_data(struct sdhci_host *host, struct mmc_data *data,
>                 }
>  #endif
>                 if (timeout-- > 0)
> -                       udelay(10);
> +                       udelay(20);


... This change makes no sense.

Actually, this whole *function* makes no sense. It seems to me that if
you attempt to transfer more than 100000 blocks, it will fail. A
timeout variable should only be used to control the number of
iterations through a wait loop. This loop does more than wait, it also
executes an ongoing, multiblock transfer.

I'm not exactly sure what issue this change is solving, but extending
the delay, and increasing the number of iterations is *not* the
solution. You're just hiding the problem.

You need to figure out why your transfer failed, and then modify the
code to actually solve that problem. And I strongly think you need to
refactor this transfer loop so that it properly transfers all desired
blocks, and times out only when timeouts happen.

Andy