David Brownell wrote:
> Oh, and one more issue likely to appear in a public review:
> there are a couple un-bounded loops, which should get terminated.
> Appended is a patch I've been running with, which at least gets
> rid of the "system hangs" aspect of such loop (even if there's
> no particularly good cleanup).
>
> I hope you're planning to send this up for review soon, so that
> it will be in the 2.6.30 merge queue when the merge window opens...
> host->bus_mode = ios->bus_mode;
> if (ios->power_mode == MMC_POWER_UP) {
> + unsigned long timeout = jiffies + msecs_to_jiffies(50);
> + bool lose = true;
> +
> /* Send clock cycles, poll completion */
> writel(0, host->base + DAVINCI_MMCARGHL);
> writel(MMCCMD_INITCK, host->base + DAVINCI_MMCCMD);
> - while (!(readl(host->base + DAVINCI_MMCST0) & MMCST0_RSPDNE))
> + while (time_before(jiffies, timeout)) {
> + u32 tmp = readl(host->base + DAVINCI_MMCST0);
> +
> + if (tmp & MMCST0_RSPDNE) {
> + lose = false;
> + break;
> + }
> cpu_relax();
> + }
> + if (lose)
> + dev_warn(mmc_dev(host->mmc), "powerup timeout\n");
> }
I agree to this and we should incorporate it. It will help us to get out of
while loop.
> @@ -907,6 +919,7 @@ static inline int handle_core_command(
> int end_transfer = 0;
> unsigned int qstatus = status;
> struct mmc_data *data = host->data;
> + unsigned count = 0;
>
> /* handle FIFO first when using PIO for data */
> while (host->bytes_left && (status & (MMCST0_DXRDY | MMCST0_DRRDY))) {
> @@ -914,7 +927,12 @@ static inline int handle_core_command(
> status = readl(host->base + DAVINCI_MMCST0);
> if (!status)
> break;
> +
> qstatus |= status;
> + if (count++ > 40) {
> + dev_dbg(mmc_dev(host->mmc), "PIO timeout\n");
> + break;
> + }
> }
>
> if (qstatus & MMCST0_DATDNE) {
I think it has potential bug. DAVINCI_MMCST0 register has been read (assume
that either MMCST0_DXRDY or MMCST0_DRRDY was also set) and at same time it will
break if count has reached 41. There will be no write to FIFO and so
MMCST0_DXRDY/MMCST0_DRRDY will not be set again and so no next interrupt which
could cause hang. In order to fix this bug, we need to check count value before
DAVINCI_MMCST0 register read. But, I feel we should not add any PIO timeout
here. I checked on EVM and count has never increased more than 1 for high speed
card.
_______________________________________________
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
