[coreboot-gerrit] Change in coreboot[master]: soc/ti/am335x: Add MMC/SD driver

[Arthur Heymans (Code Review)]

Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44384 )

Change subject: soc/ti/am335x: Add MMC/SD driver
......................................................................

Patch Set 10: Code-Review+2

(2 comments)

https://review.coreboot.org/c/coreboot/+/44384/10/src/soc/ti/am335x/mmc.h 
File src/soc/ti/am335x/mmc.h:

https://review.coreboot.org/c/coreboot/+/44384/10/src/soc/ti/am335x/mmc.h@11 
PS10, Line 11: struct am335x_mmc {
             : 	uint8_t res1[0x110];
             : 	uint32_t sysconfig;
             : 	uint32_t sysstatus;
             : 	uint8_t res2[0x14];
             : 	uint32_t con;
             : 	uint32_t pwcnt;
             : 	uint32_t dll;
             : 	uint8_t res3[0xcc];
             : 	uint32_t blk;
             : 	uint32_t arg;
             : 	uint32_t cmd;
             : 	uint32_t rsp10;
             : 	uint32_t rsp32;
             : 	uint32_t rsp54;
             : 	uint32_t rsp76;
             : 	uint32_t data;
             : 	uint32_t pstate;
             : 	uint32_t hctl;
             : 	uint32_t sysctl;
             : 	uint32_t stat;
             : 	uint32_t ie;
             : 	uint8_t res4[0x4];
             : 	uint32_t ac12;
             : 	uint32_t capa;
             : 	uint32_t capa2;
             : 	uint8_t res5[0xc];
             : 	uint32_t admaes;
             : 	uint32_t admasal;
             : };
You might want to use the __packed directive?

https://review.coreboot.org/c/coreboot/+/44384/10/src/soc/ti/am335x/mmc.c 
File src/soc/ti/am335x/mmc.c:

https://review.coreboot.org/c/coreboot/+/44384/10/src/soc/ti/am335x/mmc.c@50 
PS10, Line 50: while (!(read32(&mmc->sysstatus) & SYSSTATUS_RESETDONE))
             : 		;
It would be nicer with timeouts.