u-boot 2010.12 (Jan. 11 2011) breaks in QEMU

Bug #703094 reported by Paul Larson
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Linaro QEMU
Fix Released
High
Peter Maydell

Bug Description

This didn't fail before the update to the new version of u-boot.
Partial boot log attached:

U-Boot 2010.12 (Jan 11 2011 - 04:46:52) OMAP35XX-GP ES3.1, CPU-OPP2, L3-165MHz, Max CPU Clock 600 mHz OMAP3 Beagle board + LPDDR/NAND I2C: ready DRAM: 128 MiB NAND: 256 MiB MMC: OMAP SD/MMC: 0
*** Warning - bad CRC, using default environment
ERROR : Unsupport USB mode Check that mini-B USB cable is attached to the device In: serial Out: serial Err: serial Beagle Rev C4 No EEPROM on expansion board Die ID #51454d5551454d555400000051454d55
Hit any key to stop autoboot: 10  9  8  7  6  5  4  3  2  1  0 mmc_send_cmd: timedout waiting for stat! ** Partition 1 not valid on device 0 ** ** Unable to use mmc 0:1 for fatload **
mmc_send_cmd: timedout waiting for stat! ** Can't read from device 0 ** ** Unable to use mmc 0:1 for fatload ** Booting from nand ... NAND read: device 0 offset 0x280000, size 0x400000
4194304 bytes read: OK Wrong Image Format for bootm command ERROR: can't get kernel image!

Loïc Minier (lool)
Changed in qemu-maemo:
importance: Undecided → High
Changed in qemu-maemo:
assignee: nobody → Peter Maydell (pmaydell)
Loïc Minier (lool)
affects: qemu-maemo → qemu-linaro
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.

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

Sorry, firefox and/or launchpad have mangled the indent on the code there; it was fine in the textedit box before I sent it...

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

The fix for this is now in the qemu-linaro tree at:
http://git.linaro.org/gitweb?p=qemu/qemu-linaro.git;a=summary

specifically this commit:
 http://git.linaro.org/gitweb?p=qemu/qemu-linaro.git;a=commit;h=cdc2888b37ae42b303855853d802e928397a3787

and will be in the upcoming qemu-linaro release.

Loïc Minier (lool)
Changed in qemu-linaro:
status: New → Fix Committed
Peter Maydell (pmaydell)
Changed in qemu-linaro:
status: Fix Committed → Triaged
status: Triaged → Fix Released
milestone: none → 2011.02-rc2
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Related blueprints

Remote bug watches

Bug watches keep track of this bug in other bug trackers.