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 12:
(6 comments)
https://review.coreboot.org/c/coreboot/+/46260/10/src/soc/intel/common/block... File src/soc/intel/common/block/pcie/rtd3/rtd3.c:
https://review.coreboot.org/c/coreboot/+/46260/10/src/soc/intel/common/block... PS10, Line 10: #include <intelblocks/pmc.h>
+#include <intelblocks/pmc_ipc. […]
Done
https://review.coreboot.org/c/coreboot/+/46260/10/src/soc/intel/common/block... PS10, Line 30: /* : * Delay up to wait_ms until provided register matches expected value. : * : * Local7 = segments : * While (Local7 > 0) : * { : * If (name == val) { : * Break : * } : * Sleep (wait_ms_segment) : * Local7++ : * } : */ : static void pcie_rtd3_acpi_delay(uint32_t wait_ms, const char *name, uint64_t val) : { : uint32_t wait_ms_segment = 1; : uint32_t segments = wait_ms; : : if (wait_ms > 32) { : wait_ms_segment = 16; : segments = wait_ms / 16; : } : : acpigen_write_store_int_to_op(segments, LOCAL7_OP); : acpigen_emit_byte(WHILE_OP); : acpigen_write_len_f(); : acpigen_emit_byte(LGREATER_OP); : acpigen_emit_byte(LOCAL7_OP); : acpigen_emit_byte(ZERO_OP); : if (name) { : acpigen_write_if_lequal_namestr_int(name, val); : acpigen_emit_byte(BREAK_OP); : acpigen_pop_len(); /* If */ : } : acpigen_write_sleep(wait_ms_segment); : acpigen_emit_byte(DECREMENT_OP); : acpigen_emit_byte(LOCAL7_OP); : acpigen_pop_len(); /* While */ : }
Ya I can do that.
Done
https://review.coreboot.org/c/coreboot/+/46260/10/src/soc/intel/common/block... PS10, Line 76: ;
/* If */
Done
https://review.coreboot.org/c/coreboot/+/46260/10/src/soc/intel/common/block... PS10, Line 90: ;
/* If */
Done
https://review.coreboot.org/c/coreboot/+/46260/10/src/soc/intel/common/block... PS10, Line 114: /* Assert power gpio. */ : acpigen_enable_tx_gpio(power_gpio); : : /* Enable CLKSRC for root port. */ : pmc_ipc_acpi_set_pci_clock(pcie_rp, clock_pin, true); : : /* De-assert reset gpio. */ : if (reset_gpio->pin_count) : acpigen_disable_tx_gpio(reset_gpio);
It does seems like there might be some sequencing violations without any delays. […]
Added some, still trying to debug the resume behavior...
https://review.coreboot.org/c/coreboot/+/46260/10/src/soc/intel/common/block... PS10, Line 266: RTD3
Looks like this has to be named PXSX for the kernel to recognize it. […]
Done