Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38750 )
Change subject: soc/intel/skylake: Remove misleading BUG: messages ......................................................................
Patch Set 3:
(1 comment)
What I am doing here is check if a possible root port is on the devicetree. If it is a non NULL value is returned is it isn't a NULL value is returned and the PCI device shouldn't be accessed. Previously this returned the bug message which is fine for standard accesses but shouldn't be done if you just want to check if a device is on the tree.
Sorry, missed that there was a null check already.
For my understanding? How can I avoid the __SIMPLE_DEVICE__ stuff? The return is different in both cases (or is there an alternative I am not aware of?)
Kyösti worked on making the two cases compatible. I don't know the exact plan. While they are incompatible, one shouldn't use the same code for both cases. Beside the type conflict, they have very different environments. In ramstage, one is supposed to use the devicetree (at least after device enumeration, which is the case here, AFAICS). In SMM, there is no enumeration, hence all possible devices are tried.
If I wanted to implement this properly for ramstage, it would be a loop over bus 0:
for (dev = bus->children; dev; dev = dev->sibling) { if (dev->path.type != PCI) continue; if (PCI_SLOT(dev->path.pci.devfn) != PCH_DEV_SLOT_PCIE && ...) continue;
/* check this dev */ }
https://review.coreboot.org/c/coreboot/+/38750/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/38750/1//COMMIT_MSG@12 PS1, Line 12: This : is required because of a flaw in the PCH.
The PCIEXPWAK_STS bit doesn't get set so all rootports need to be scanned to determine if one of tha […]
Right, but that's only about the missing `if` there. I guess the function would still be there, to identify where the event occurred (not if). Note that the other platforms have a TODO there.