Attention is currently required from: Shelley Chen. Douglas Anderson has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/57057 )
Change subject: WIP: Herobrine: Initialize Pen and Audio i2c devices ......................................................................
Patch Set 2:
(4 comments)
File src/mainboard/google/herobrine/mainboard.c:
https://review.coreboot.org/c/coreboot/+/57057/comment/6ba182ae_a4b3b121 PS2, Line 74: qupv3_se_fw_load_and_init(QUPV3_1_SE4, SE_PROTOCOL_SPI, MIXED); /* ESIM SPI */ Drop this line. The eSIM was connected to SPI in a really early version of trogdor but (as far as I know) isn't now.
QUPV3_1_SE4 = 8 + 4 = 12. ...and QUP12 is actually supposed to be configured for i2c for talking to our TPM. This might be the source of some of our TPM problems that we were blaming on blobs?
https://review.coreboot.org/c/coreboot/+/57057/comment/ab2a163d_ee6b7a04 PS2, Line 76: QUPV3_1_SE6 1_SE6 = 8 + 6 = 14. spi14 pins are "gpio56", "gpio57", "gpio58", "gpio59". Schematic shows FP SPI as 44 - 47. That's spi11. 11 - 8 = 3 so this should be QUPV3_1_SE3.
https://review.coreboot.org/c/coreboot/+/57057/comment/4d9da488_63dd6177 PS2, Line 78: i2c_init(QUPV3_0_SE0, I2C_SPEED_FAST); /* Audio - QUP 0 */ Can you re-order? Seems like things are mostly sorted by QUP so this should be at the beginning?
Also, why this instead of `qupv3_se_fw_load_and_init()` like everything else in this file?
https://review.coreboot.org/c/coreboot/+/57057/comment/2b572cdf_4379a640 PS2, Line 79: i2c_init(QUPV3_1_SE7, I2C_SPEED_FAST); /* PEN - QUP 15 */ Let's drop this line. I was wrong, it's actually 13 and handled above as "Touch I2C"