Lee Leahy has posted comments on this change. ( https://review.coreboot.org/19211 )
Change subject: soc/intel/quark: Add SD/MMC test support ......................................................................
Patch Set 12:
(8 comments)
https://review.coreboot.org/#/c/19211/11/src/soc/intel/quark/Kconfig File src/soc/intel/quark/Kconfig:
PS11, Line 317: User must : also enable one or both of DRIVERS_STORAGE_SD or DRIVERS_STORAGE_MMC.
Should we have a 'depends on' statement for these instead of just adding it
A "depends on" is necessary since the intent is to make it easy to test on all Quark boards.
https://review.coreboot.org/#/c/19211/11/src/soc/intel/quark/Makefile.inc File src/soc/intel/quark/Makefile.inc:
PS11, Line 65:
Why so many stages? Is it doing useful things in all of them? It seems li
Removed extra stages and moved into romstage and ramstage
https://review.coreboot.org/#/c/19211/11/src/soc/intel/quark/storage_test.c File src/soc/intel/quark/storage_test.c:
PS11, Line 164: E)
I think we got rid of CONFIG_CAR_DRIVERS_STORAGE, so we should just be able
Removed CONFIG_CAR_DRIVERS_STORAGE
PS11, Line 170: RT((struct sd
We're in a !ENV_BOOTBLOCK #if section here, so this can't be true. Same wi
Removed ENV_BOOTBLOCK
PS11, Line 194: err);
For these printk statements, you could do something like this in console.h.
Defined LOG_DEBUG as shown above and defined STORAGE_DEBUG as BIOS_DEBUG
PS11, Line 219: name = storage_partition_name(media
No need for this if when calling display log, as everything in the function
Done
PS11, Line 232:
BIOS_SPEW?
Using STORAGE_DEBUG to output at same level as LOG_DEBUG.
PS11, Line 247: tr_t)(media + 1) + 0x7) &
again, i think we got rid of this.
Done