Attention is currently required from: Edward O'Callaghan, Angel Pons. Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/59281 )
Change subject: pcidev.c: Simplify by consolidating common logic ......................................................................
Patch Set 5:
(4 comments)
Commit Message:
https://review.coreboot.org/c/flashrom/+/59281/comment/d845c6f7_79082e8a PS5, Line 9: As observed by Angel Pos, It looks like `pci_filter_init()` typo: Pons
https://review.coreboot.org/c/flashrom/+/59281/comment/ee63a873_ceeff5e1 PS5, Line 10: initialises the device ID to -1 What is resulting action from this fact?
File pcidev.c:
https://review.coreboot.org/c/flashrom/+/59281/comment/bb6e80b4_5ae14b3a PS5, Line 166: struct pci_dev *pcidev_find_vendorclass(uint16_t vendor, uint16_t devclass) The new implementation tests only the first pci device from a vendor on it's class. The old implementation loops through all devices from a vendor.
https://review.coreboot.org/c/flashrom/+/59281/comment/5eb1d281_c9e35a88 PS5, Line 168: struct pci_dev *temp = pcidev_find(vendor, -1); Please comment here why to use -1 and it's special meaning. When reading the code later its easier to understand it without digging into the git log or libpci implementation. Maybe: /* libpci uses -1 as uninitialized filter value. Initializing only the vendor will give us... */
-
Thomas Heijligen (Code Review)