Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39043 )
Change subject: lenovo/*: Fix DRAM init when WWAN card is present ......................................................................
Patch Set 7:
(2 comments)
I would prefer that we use a consistent decision to enable it across all stages of coreboot. If a specific card always needs power, the user shouldn't turn it off. What would be left to do is to move the current enabling/disabling from ramstage to romstage. e.g.
void h8_early_init(void) { /* comment why we might need it early */ h8_wwan_enable(h8_has_wwan(dev) && h8_wwan_nv_enable()); }
https://review.coreboot.org/c/coreboot/+/39043/7//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/39043/7//COMMIT_MSG@10 PS7, Line 10: support SMBus, is installed it pulls the lines low. Looks like such a card would not be spec compliant:
"Care should be taken in the design of both the input and output stages of SMBus devices, in order not to load the bus when their power plane is turned off, i.e. powered-down devices must provide no leakage path to ground."
Probably worth to mention here.
https://review.coreboot.org/c/coreboot/+/39043/7/src/mainboard/lenovo/t420/e... File src/mainboard/lenovo/t420/early_init.c:
https://review.coreboot.org/c/coreboot/+/39043/7/src/mainboard/lenovo/t420/e... PS7, Line 82: * ramstage will turn it off, in case it's not needed. How to tell if it's not needed? There may be more going on on the SMBus than just reading SPDs.