Petr Cvek has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35678 )
Change subject: intel/i945,i82801gx: Refactor early PCI bridge reset ......................................................................
Patch Set 2:
(3 comments)
It seems these link detection problems may be caused by the card itself. Only first run without mdelay failed (and it started to work after car replug). All runs with mdelay succeeded. Gonna do more tests tomorrow with PCIe/PCIe port multiplier. A PCIe/PCI adapter with PCI S3 trio card was detected (with and without delay).
https://review.coreboot.org/c/coreboot/+/35678/1/src/northbridge/intel/i945/... File src/northbridge/intel/i945/early_init.c:
https://review.coreboot.org/c/coreboot/+/35678/1/src/northbridge/intel/i945/... PS1, Line 556: pci_s_deassert_secondary_reset(p2peg);
Conventional PCI had Trst 1 ms minimum for RST# asserted and Trhfa 1 second when add-on hardware mus […]
OK so new infos:
I've removed HDD and RX460 GPU and added some old radeon HD4550, which I'm using for testings. I've managed to fail the detection, but after I've replugged the card it started to work. So I assume there may have been some oxidation on contacts, dry caps or something like this. Other restarts (powerdown, reset) seemed to work regardless on delay/nondelay.
A multiple powerdown/resets with delay/nondelay versions on RX460 worked always.
If a test with mdelay(10) between the old variant of assert and deassert suffices, then I can say it works. For your complete patchset I didn't have a time yet to merge it with my tree. So I guess this topic is not urgent.
ad Trhfa: yeah and according to PCI EXPRESS BASE SPECIFICATION, REV. 3.0 the value must be respected for PCIe/PCI(-X) bridge too.
https://review.coreboot.org/c/coreboot/+/35678/1/src/northbridge/intel/i945/... PS1, Line 591: printk(BIOS_DEBUG, "PCIe link training ...");
This deserves a rewrite, lines 615-625 repeat 591-601. […]
yeah the whole function should be optimized. I've some ideas for later (not relevant with reset, but with IGD init).
https://review.coreboot.org/c/coreboot/+/35678/1/src/northbridge/intel/i945/... PS1, Line 876: mdelay(200); If there is a PCI(e/-X) specification requirement for an assertion to have some duration. Maybe the easiest way is to move all mdelays into pci_s_assert_secondary_reset() function. It will shorten this patch and we can solve the problem with delay later.