Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31951 )
Change subject: device/pciexp_device: Add set_L1_ss_latency() for pciexp device ......................................................................
Patch Set 2:
(2 comments)
https://review.coreboot.org/#/c/31951/1/src/device/pciexp_device.c File src/device/pciexp_device.c:
https://review.coreboot.org/#/c/31951/1/src/device/pciexp_device.c@470 PS1, Line 470: PCIE_LTR_MAX_NO_SNOOP_LATENCY_VALUE << 16 |
sorry i didn't get you. […]
Specially, if you would add this function in global space, it is a false assumption that every caller will want the same latency values programmed;
Thus this would become:
void pciex_set_L1_ss_max_latency(dev, offset, max_no_snoop, max_snoop) { pci_write_config32(dev, offset, (max_no_snoop << 16) | max_snoop); }
There is just no point in doing that... By aliasing I mean inventing a new function that does nothing else but calls an already existing function after combining two u16 to u32.
https://review.coreboot.org/#/c/31951/2/src/soc/intel/common/pcie.c File src/soc/intel/common/pcie.c:
https://review.coreboot.org/#/c/31951/2/src/soc/intel/common/pcie.c@26 PS2, Line 26: #define PCIE_LTR_MAX_SNOOP_LATENCY_3140US 0x1003 The defines are fine, but as I believe this is PCIe standard configuration register, these can go in device/pciexp.h. Comment above them which PCIe capability they are associated with.
To be precise, it is 3 x 1048576, so 3146 US rounded.