Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34285 )
Change subject: soc/intel: Fix regression with hidden PCI devices ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34285/3/src/device/device_const.c File src/device/device_const.c:
https://review.coreboot.org/c/coreboot/+/34285/3/src/device/device_const.c@2... PS3, Line 243: BUG: %s requests hidden 00:%02x.%d\
Not everything that is reported will truly be a bug. In the output pasted by Maxim, following calls are fine since they check against !dev:
soc_peg_init_params pch_log_rp_wake_source
I'll make them explicitly call pcidev_path_on_root() instead. Maybe I mentioned earlier that I find soc/pci_devs.h most disgusting use of preprocessor. In one context macro expands to constant value, in other it's a function returning pointer...
On the other hand this is a true bug: me_read_config32. It does not check for NULL. But I think that would be a common problem. I see a lot of uses of PCH_DEV_* macro being passed as an argument to some function without any check.
The log should annoy one enough to add the NULL checks where necessary.
On the other hand, this is another problem: p2sb_get_device. Here the device seems to be hidden and the functions seem to be un-hiding it. If you plan to get rid of dev_find_slot() completely, what is your plan on supporting such devices?
Awww.. yuck. So "hiding" makes the device not respond to PCI ID register reads but *some* configuration registers can be accessed and written. Ever heard of PCI Specifications, Intel?
Well I'll probably have to rename dev_find_slot() then and leave it available.