Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41363 )
Change subject: device: avoid fixed resources hanging off pci domain device ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/41363/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/41363/2//COMMIT_MSG@9 PS2, Line 9: Some chipsets are adding resources to the pci domain device. As
Great question. I've been asking that myself. […]
I was just thinking about the pstack thing again. Perhaps we need to make it clearer how to provide linkage between a resource on a device and the struct bus. Right now we seem to be encoding that linkage with macros in the index field of struct resource.
/* Macros to generate index values for resources */ #define IOINDEX_SUBTRACTIVE(IDX, LINK) (0x10000000 + ((IDX) << 8) + LINK) #define IOINDEX_SUBTRACTIVE_LINK(IDX) (IDX & 0xff)
#define IOINDEX(IDX, LINK) (((LINK) << 16) + IDX) #define IOINDEX_LINK(IDX) ((IDX & 0xf0000) >> 16) #define IOINDEX_IDX(IDX) (IDX & 0xffff)
I honestly am very confused what these are supposed to mean and be used nor how they are related. I think I'd like to solidify on a concept of decoding for the resource windows.
IOINDEX_IDX isn't used at all in the tree. IOINDEX_LINK is used in the amd code, and it appears the index field is free to encode things however one sees fit. It seems hard to get consistency when index is such a free form field.
search_bus_resources() is the only user of IOINDEX_SUBTRACTIVE_LINK for traversing further down a bus of a child device where its resource link num matches. So there's something there to work with, but it's subtle. How bad would it be to do directly reference a struct bus from a struct resource?