Comment 1 for bug 703094

Revision history for this message
Peter Maydell (pmaydell) wrote :

The new u-boot has a completely different implementation of the MMC driver, which does this:

http://git.linaro.org/gitweb?p=boot/u-boot-linaro-stable.git;a=blob;f=drivers/mmc/omap_hsmmc.c;hb=eb9a28f699667919bf140bdabdf37c25be725c79#l282

in abridged form:

 294 while (size) {
 296 do {
 297 mmc_stat = readl(&mmc_base->stat);
 303 } while (mmc_stat == 0);
 308 if (mmc_stat & BRR_MASK) {
 311 writel(readl(&mmc_base->stat) | BRR_MASK,
 312 &mmc_base->stat);
                                [read the data from the FIFO]
 318 }
 320 if (mmc_stat & BWR_MASK)
 321 writel(readl(&mmc_base->stat) | BWR_MASK,
 322 &mmc_base->stat);
 324 if (mmc_stat & TC_MASK) {
 325 writel(readl(&mmc_base->stat) | TC_MASK,
 326 &mmc_base->stat);
 327 break;
 328 }
 329 }

which is (lines 324..238) attempting to detect and clear the TC (transfer complete) status bit. However since TC is only set when the FIFO is fully drained (happens in 313-317) but the local variable mmc_stat is still the value we first read at line 297, we don't notice that TC is set and never clear it (we will stop the loop when size becomes zero anyway).

On hardware this doesn't matter because the next time we send a command we first write -1 to STAT (clearing all bits in it) (line 180) before waiting for it to read as zeroes, so the stray TC bit is cleared then. However the qemu model of the MMC controller effectively refuses to clear a bit in STAT if you've never seen it (ie if you have not read the STAT register since that bit was set). This means that (since we didn't read STAT after TC got set) we don't clear TC when u-boot writes -1 to STAT, and so STAT never goes to zero, and u-boot times out and complains (line 184).

I think qemu is incorrect in not clearing unread STAT bits -- I can't find any justification for it in the TRM -- but on the other hand the code was clearly carefully written to behave that way, which suggests it was intentional. If I just remove this code the Linaro snapshot boots to a shell prompt, though...

I'd argue that the u-boot code probably isn't doing what the author intended it to do, but it's not actually doing anything illegal.