Rizwan Qureshi 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 2:
(1 comment)
Patch Set 2: Code-Review+1
(1 comment)
I agree to the ASL code beside my inline question. Two remarks, though:
Why is it a workaround? Is it documented somewhere that it should behave differently?
errta is https://cdrdv2.intel.com/v1/dl/getContent/596808. Not sure if you have access. The issue is what is mentioned in the commit message. SDXC controller can operate the SD_PWR_EN as either an active high signal or an active low signal. This issue only happens when the SD_PW_EN is used as an active high signal.
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.
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)
What about other bits like enabling the output buffer? Are they […]
the method CTXS is initializing the fields by setting the "UpdateRule" as preserve, which should preserve all the bits which are not being modified.