Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/39963 )
Change subject: nicintel: add I350 support ......................................................................
Patch Set 1:
(4 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@99 PS1, Line 99: Unprogrammed Please put this at the beginning of the string
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:
return device_id == 0x1509 || device_id == 0x151f;
https://review.coreboot.org/c/flashrom/+/39963/1/nicintel_eeprom.c@477 PS1, Line 477: uintptr_t io_base_addr = pcidev_readbar(dev, PCI_BASE_ADDRESS_0); This change is probably needed because most PCI devices on x86_64 use 32-bit BARs
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