Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46260 )
Change subject: soc/intel/common: Add PCIe Runtime D3 driver for ACPI ......................................................................
Patch Set 15: Code-Review+2
(6 comments)
https://review.coreboot.org/c/coreboot/+/46260/15/src/soc/intel/common/block... File src/soc/intel/common/block/pcie/rtd3/chip.h:
https://review.coreboot.org/c/coreboot/+/46260/15/src/soc/intel/common/block... PS15, Line 33: OS to apply security restrictions. Are these security restrictions specific to D3?
https://review.coreboot.org/c/coreboot/+/46260/15/src/soc/intel/common/block... PS15, Line 44: Disable the ACPI-driven L23 Under what conditions would a mainboard want to disable this?
https://review.coreboot.org/c/coreboot/+/46260/15/src/soc/intel/common/block... File src/soc/intel/common/block/pcie/rtd3/rtd3.c:
https://review.coreboot.org/c/coreboot/+/46260/15/src/soc/intel/common/block... PS15, Line 148: config->enable_gpio.pin_count Isn't enable gpio mandatory as per https://review.coreboot.org/c/coreboot/+/46260/15/src/soc/intel/common/block...
https://review.coreboot.org/c/coreboot/+/46260/15/src/soc/intel/common/block... PS15, Line 199: !config->enable_gpio.pin_count Is it okay not to provide the enable GPIO?
https://review.coreboot.org/c/coreboot/+/46260/15/src/soc/intel/common/block... PS15, Line 241: PCIE_HOTPLUG_IN_D3_UUID Is this true for all PCIe devices? Or would it be only for external facing ports? Given that there is a separate property for external facing ports, I think this can be true independent of whether the port is user visible.
If we are cutting power to the device completely, do we really support Hotplug in D3? Or is this something completely different?
https://review.coreboot.org/c/coreboot/+/46260/15/src/soc/intel/common/block... PS15, Line 281: PXSX Is this not present in kernel v5.4 in chromium tree? I was not able to find it. Also, does this apply only to storage devices?