Martijn Berger has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/39963 )
Change subject: nicintel: add I350 support ......................................................................
Patch Set 5:
(3 comments)
https://review.coreboot.org/c/flashrom/+/39963/1/nicintel_eeprom.c File nicintel_eeprom.c:
https://review.coreboot.org/c/flashrom/+/39963/1/nicintel_eeprom.c@103 PS1, Line 103: {PCI_VENDOR_ID_INTEL, 0x1522, NT, "Intel", "I350 Gigabit Network Connection External SGMII PHY"},
yes good catch
Done
https://review.coreboot.org/c/flashrom/+/39963/1/nicintel_eeprom.c@109 PS1, Line 109: switch (device_id)
Why use a switch? This could be simply: […]
Done
https://review.coreboot.org/c/flashrom/+/39963/1/nicintel_spi.c File nicintel_spi.c:
https://review.coreboot.org/c/flashrom/+/39963/1/nicintel_spi.c@277 PS1, Line 277: (dev->device_id & 0xfff0) == 0x1510
This check will be a false positive for 0x1517 and 0x151c
Done