Duncan Laurie 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 14:
(4 comments)
https://review.coreboot.org/c/coreboot/+/46260/13/src/soc/intel/common/block... File src/soc/intel/common/block/pcie/rtd3/rtd3.c:
https://review.coreboot.org/c/coreboot/+/46260/13/src/soc/intel/common/block... PS13, Line 70:
ya it went beyond 96 chars, such long names..
Ack
https://review.coreboot.org/c/coreboot/+/46260/13/src/soc/intel/common/block... PS13, Line 73:
I think there is an implicit assumption that the pins are configured to retain their state in S0ix i […]
Ack
https://review.coreboot.org/c/coreboot/+/46260/13/src/soc/intel/common/block... PS13, Line 140: acpigen_write_if_lequal_op_op(LOCAL0_OP, LOCAL1_OP); : acpigen_write_return_op(ZERO_OP); : acpigen_pop_len(); /* If */ : acpigen_write_else(); : acpigen_write_return_op(ONE_OP); : acpigen_pop_len(); /* Else */
It should really just use the power enable to determine if the power is on. […]
Actually going to keep this and make 'enable gpio' optional as well, as long as one of enable/reset is provided. This is because the enable pin for RTS5261 was repurposed so it only has reset control...
https://review.coreboot.org/c/coreboot/+/46260/13/src/soc/intel/common/block... PS13, Line 153: , "_PR3" removing _PR3 here because technically these don't support D3hot since it completely removes power.