Attention is currently required from: Tim Wawrzynczak, Nick Vaccaro, Patrick Rudolph. Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52765 )
Change subject: device: Switch pci_dev_is_wake_source to take pci_devfn_t ......................................................................
Patch Set 1:
(2 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/52765/comment/ee4c1df1_4d705a87 PS1, Line 15: entire devicetree from SMM (only usage left : is when disabling HECI via SMM) This change seems fine. But, we really need to work on eliminating the tree topology (links) for non-ramstage stages. We can make use of aliases to skip walking the device tree. This will allow us to pull in only relevant parts of the device tree into any stage. We then don't have to worry about per-stage optimizations (I had to do something similar for bootblock some time back: https://review.coreboot.org/q/hashtag:%22dt-bootblock-simplification%22+(sta...))
File src/soc/intel/alderlake/elog.c:
https://review.coreboot.org/c/coreboot/+/52765/comment/794a9a40_633a76c8 PS1, Line 51: pci_dev_is_wake_source(pme_map[i].devfn) I don't think this usage is correct. Unfortunately, the `devfn` is overloaded in coreboot and this is not really the same as pci_devfn_t, even though line 15 says that it is (That is another thing to fix here).
1. Line 15 `devfn` should really be `unsigned int` as it expects `PCI_DEVFN()` format 2. `pci_dev_is_wake_source()` expects `pci_devfn_t` which would be something like `PCI_DEV(0, PCI_SLOT(pme_map[i].devfn), PCI_FUNC(pme_map[i].devfn))`