Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31445 )
Change subject: soc/intel/cannonlake: Add a power control workaround for SD controller ......................................................................
Patch Set 8:
(1 comment)
Why the additional Kconfig? Can't we simply read the pad mode in _PS3 and do what needs to be done if it was set to native1?
Pad mode will be set to Native1 irrespective. This WA has to be applied only when SD_PW_EN is used as an active high signal.
What I meant is, that we can detect that case by looking at the register, even the active high setting. Or is that con- figured somewhere else?
https://review.coreboot.org/#/c/31445/2/src/soc/intel/cannonlake/acpi/scs.as... File src/soc/intel/cannonlake/acpi/scs.asl:
https://review.coreboot.org/#/c/31445/2/src/soc/intel/cannonlake/acpi/scs.as... PS2, Line 134: CTXS(SD_PWR_EN_PIN)
the method CTXS is initializing the fields by setting the "UpdateRule" as preserve, which should pre […]
That's neither what I meant nor how CTXS does it. It does preserve the bits (but not by the UpdateRule but rather by manually masking) and that is my concern. It means the state of the other bits before calling GPMO has to be the state that we need them to be after cal- ling CTXS. And the question is, is that guaranteed or does this only work by chance (e.g. depend on bits set in the board's initial GPIO config)?