Attention is currently required from: Furquan Shaikh, Francois Toguo Fotso, Tim Wawrzynczak, Nick Vaccaro, Karthik Ramasubramanian. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/57784 )
Change subject: device: Add pciexp_find_next_extended_cap function ......................................................................
Patch Set 4:
(7 comments)
File src/device/pciexp_device.c:
https://review.coreboot.org/c/coreboot/+/57784/comment/43476971_0afaa974 PS2, Line 2:
@Tim, Can you please check below code refactoring if it helps […]
Ack
https://review.coreboot.org/c/coreboot/+/57784/comment/c5a1d71d_6fe56a63 PS2, Line 34: unsigned int pciexp_find_next_extended_cap(struct device *dev, unsigned int cap, : unsigned int pos)
+1 to simplification. […]
Ack
https://review.coreboot.org/c/coreboot/+/57784/comment/9973dd13_139e16f4 PS2, Line 40: Read the passed-in capability to get the pointer to next
In `pci_s_find_next_capability()`, it looks like the function always starts from the PCI_CAPABILITY_ […]
Ack
https://review.coreboot.org/c/coreboot/+/57784/comment/ebc64bb9_837adb66 PS2, Line 42: this_cap_offset
Shouldn't this be checked for 0 before reading the cap?
Ack
https://review.coreboot.org/c/coreboot/+/57784/comment/4562dda5_5750a380 PS2, Line 45: if (!this_cap_offset || !this_cap) : return 0; :
This isn't possible given while requirements on line 44.
Ack
File src/device/pciexp_device.c:
https://review.coreboot.org/c/coreboot/+/57784/comment/2fdcc215_43bcd09b PS3, Line 17:
tab ?
Ack
https://review.coreboot.org/c/coreboot/+/57784/comment/7d05d0a1_3026cfa2 PS3, Line 37: }
Nit: Add a blank line between functions.
Ack