Tim Wawrzynczak 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 10:
(6 comments)
https://review.coreboot.org/c/coreboot/+/46260/10/src/soc/intel/common/block... File src/soc/intel/common/block/pcie/rtd3/chip.h:
https://review.coreboot.org/c/coreboot/+/46260/10/src/soc/intel/common/block... PS10, Line 11: unsigned int clock_pin; /* SRCCLK assigned to this root port */ This is available in the SoC chip info, means one less place to forget something 😊
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 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 */ : } Seems like this might be common enough to go in acipgen, wdyt?
https://review.coreboot.org/c/coreboot/+/46260/10/src/soc/intel/common/block... PS10, Line 76: ; /* If */
https://review.coreboot.org/c/coreboot/+/46260/10/src/soc/intel/common/block... PS10, Line 82: 320 I sure hope OSPM gets its own kthread........
https://review.coreboot.org/c/coreboot/+/46260/10/src/soc/intel/common/block... PS10, Line 90: ; /* If */
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's surprising to me that there isn't some kind of minimum time in reset here (I see the ASL didn't do it though *shrug*)