Re: [PATCH] OMAP: MMC: replace infinite loops with timeouts (was Re: [PATCH] OMAP: MMC: recover from transfer failures - Resend)

Adrian Hunter
2009-02-09T15:51:34Z

Jean Pihet wrote:
> Hi,
>
>>>> On Thu, 5 Feb 2009, Andrew Morton wrote:
>>>>> An infinite loop which assumes the hardware is perfect is always a
>>>>> worry. But I see the driver already does that, so we're no worse
>>>>> off..
>>> Do you want a finite loop with udelay in it? I located 4 places were this
>>> could be used. If so I can generate a new patch for that.
>> Even if Andrew doesn't, I'd sure like it. (the finite bit at least) :)
> Ok here is a patch that replaces the infinite loops with a timeout version.
> This patch applies on top of the previous one I sent ('[PATCH] OMAP: MMC:
> recover from transfer failures - Resend'). Is that OK?
>

What about making use of a function like this:

static void reset_host_controller(struct mmc_omap_host *host, unsigned bit)
{
unsigned long i, limit = loops_per_jiffy;

OMAP_HSMMC_WRITE(host->base, SYSCTL,
OMAP_HSMMC_READ(host->base, SYSCTL) | bit);
for (i = 0; i < limit; i++) {
if (!(OMAP_HSMMC_READ(host->base, SYSCTL) & bit))
return;
cpu_relax();
}
dev_err(mmc_dev(host->mmc), "%s: timeout waiting on software reset\n",
mmc_hostname(host->mmc));
}

And then just put:

reset_host_controller(host, SRD);

and

reset_host_controller(host, SRC);

in the right places.

>> Related, who is the maintainer of this driver? Tony? I'd like to have
>> someone who checks patches before I queue them up.
>>
>>
>> Rgds
>
>>From 5ee867d09efe22a903ac7373a05e9e047bad6544 Mon Sep 17 00:00:00 2001
> From: Jean Pihet <jpihet@mvista.com>
> Date: Fri, 6 Feb 2009 16:42:51 +0100
> Subject: [PATCH] OMAP: MMC: replace infinite loops with timeouts
>
> Replace the 'while() ;' with a timeout
>
> Signed-off-by: Jean Pihet <jpihet@mvista.com>
> ---
> drivers/mmc/host/omap_hsmmc.c | 56
> +++++++++++++++++++++++++++++++++--------
> 1 files changed, 45 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
> index 1ac6918..7ddc77e 100644
> --- a/drivers/mmc/host/omap_hsmmc.c
> +++ b/drivers/mmc/host/omap_hsmmc.c
> @@ -102,6 +102,8 @@
> #define OMAP_MMC_DATADIR_READ 1
> #define OMAP_MMC_DATADIR_WRITE 2
> #define MMC_TIMEOUT_MS 20
> +#define MMC_TIMEOUT_WAIT 100 /* Active wait duration, in usec */
> +#define MMC_TIMEOUT_WAIT_LOOPS ((MMC_TIMEOUT_MS * 1000) / MMC_TIMEOUT_WAIT)
> #define OMAP_MMC_MASTER_CLOCK 96000000
> #define DRIVER_NAME "mmci-omap-hs"
>
> @@ -384,6 +386,7 @@ static irqreturn_t mmc_omap_irq(int irq, void *dev_id)
> struct mmc_omap_host *host = dev_id;
> struct mmc_data *data;
> int end_cmd = 0, end_trans = 0, status;
> + unsigned long timeout_counter;
>
> if (host->cmd == NULL && host->data == NULL) {
> OMAP_HSMMC_WRITE(host->base, STAT,
> @@ -406,9 +409,17 @@ static irqreturn_t mmc_omap_irq(int irq, void *dev_id)
> OMAP_HSMMC_WRITE(host->base, SYSCTL,
> OMAP_HSMMC_READ(host->base,
> SYSCTL) |
> SRC);
> - while (OMAP_HSMMC_READ(host->base,
> - SYSCTL) & SRC)
> - ;
> + timeout_counter = 0;
> + while ((OMAP_HSMMC_READ(host->base,
> + SYSCTL) & SRC)
> + && (timeout_counter++ <
> +
> MMC_TIMEOUT_WAIT_LOOPS))
> +
> udelay(MMC_TIMEOUT_WAIT);
> +
> + if (OMAP_HSMMC_READ(host->base,
> + SYSCTL) & SRC)
> + dev_err(mmc_dev(host->mmc),
> + "Timeout waiting on
> SRC\n");
>
> host->cmd->error = -ETIMEDOUT;
> } else {
> @@ -421,9 +432,17 @@ static irqreturn_t mmc_omap_irq(int irq, void *dev_id)
> OMAP_HSMMC_WRITE(host->base, SYSCTL,
> OMAP_HSMMC_READ(host->base,
> SYSCTL) | SRD);
> - while (OMAP_HSMMC_READ(host->base,
> - SYSCTL) & SRD)
> - ;
> + timeout_counter = 0;
> + while ((OMAP_HSMMC_READ(host->base,
> + SYSCTL) & SRD)
> + && (timeout_counter++ <
> +
> MMC_TIMEOUT_WAIT_LOOPS))
> + udelay(MMC_TIMEOUT_WAIT);
> +
> + if (OMAP_HSMMC_READ(host->base,
> + SYSCTL) & SRD)
> + dev_err(mmc_dev(host->mmc),
> + "Timeout waiting on SRD\n");
> }
> }
> if ((status & DATA_TIMEOUT) ||
> @@ -436,9 +455,17 @@ static irqreturn_t mmc_omap_irq(int irq, void *dev_id)
> OMAP_HSMMC_WRITE(host->base, SYSCTL,
> OMAP_HSMMC_READ(host->base,
> SYSCTL) | SRD);
> - while (OMAP_HSMMC_READ(host->base,
> - SYSCTL) & SRD)
> - ;
> + timeout_counter = 0;
> + while ((OMAP_HSMMC_READ(host->base,
> + SYSCTL) & SRD)
> + && (timeout_counter++ <
> +
> MMC_TIMEOUT_WAIT_LOOPS))
> + udelay(MMC_TIMEOUT_WAIT);
> +
> + if (OMAP_HSMMC_READ(host->base,
> + SYSCTL) & SRD)
> + dev_err(mmc_dev(host->mmc),
> + "Timeout waiting on SRD\n");
> end_trans = 1;
> }
> }
> @@ -524,6 +551,7 @@ static void mmc_omap_detect(struct work_struct *work)
> {
> struct mmc_omap_host *host = container_of(work, struct mmc_omap_host,
> mmc_carddetect_work);
> + unsigned long timeout;
>
> sysfs_notify(&host->mmc->class_dev.kobj, NULL, "cover_switch");
> if (host->carddetect) {
> @@ -531,8 +559,14 @@ static void mmc_omap_detect(struct work_struct *work)
> } else {
> OMAP_HSMMC_WRITE(host->base, SYSCTL,
> OMAP_HSMMC_READ(host->base, SYSCTL) | SRD);
> - while (OMAP_HSMMC_READ(host->base, SYSCTL) & SRD)
> - ;
> + /* Wait till the SRD bit is reset */
> + timeout = jiffies + msecs_to_jiffies(MMC_TIMEOUT_MS);
> + while ((OMAP_HSMMC_READ(host->base, SYSCTL) & SRD)
> + && time_before(jiffies, timeout))
> + msleep(1);
> +
> + if (OMAP_HSMMC_READ(host->base, SYSCTL) & SRD)
> + dev_err(mmc_dev(host->mmc), "Timeout waiting on
> SRD\n");
>
> mmc_detect_change(host->mmc, (HZ * 50) / 1000);
> }
> --
> 1.6.0.1.42.gf8c9c
>