Nico Huber has uploaded this change for review. ( https://review.coreboot.org/21935
Change subject: nicintel_eeprom: Get rid of is_i210() ......................................................................
nicintel_eeprom: Get rid of is_i210()
Separate the two NIC classes explicitly and use discrete code paths cleared from fragile is_i210().
Only compile tested.
Change-Id: I59164ccf54afbbd64a0598282d13e80ff7fd6fa4 Signed-off-by: Nico Huber nico.h@gmx.de --- M flashrom.c M nicintel_eeprom.c M programmer.h 3 files changed, 72 insertions(+), 66 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/35/21935/1
diff --git a/flashrom.c b/flashrom.c index c600efc..8948857 100644 --- a/flashrom.c +++ b/flashrom.c @@ -312,7 +312,7 @@ { .name = "nicintel_eeprom", .type = PCI, - .devs.dev = nics_intel_ee, + .devs.note = "Several Intel 82580 and i210 NICs", .init = nicintel_ee_init, .map_flash_region = fallback_map, .unmap_flash_region = fallback_unmap, diff --git a/nicintel_eeprom.c b/nicintel_eeprom.c index c787580..1552548 100644 --- a/nicintel_eeprom.c +++ b/nicintel_eeprom.c @@ -80,17 +80,17 @@
#define UNPROG_DEVICE 0x1509
-/* - * Warning: is_i210() below makes assumptions on these PCI ids. - * It may have to be updated when this list is extended. - */ -const struct dev_entry nics_intel_ee[] = { +static const struct dev_entry nics_intel_ee_82580[] = { {PCI_VENDOR_ID_INTEL, 0x150e, OK, "Intel", "82580 Quad Gigabit Ethernet Controller (Copper)"}, {PCI_VENDOR_ID_INTEL, 0x150f, NT , "Intel", "82580 Quad Gigabit Ethernet Controller (Fiber)"}, {PCI_VENDOR_ID_INTEL, 0x1510, NT , "Intel", "82580 Quad Gigabit Ethernet Controller (Backplane)"}, {PCI_VENDOR_ID_INTEL, 0x1511, NT , "Intel", "82580 Quad Gigabit Ethernet Controller (Ext. PHY)"}, {PCI_VENDOR_ID_INTEL, 0x1511, NT , "Intel", "82580 Dual Gigabit Ethernet Controller (Copper)"}, {PCI_VENDOR_ID_INTEL, UNPROG_DEVICE, OK, "Intel", "Unprogrammed 82580 Quad/Dual Gigabit Ethernet Controller"}, + {0}, +}; + +static const struct dev_entry nics_intel_ee_i210[] = { {PCI_VENDOR_ID_INTEL, 0x1531, NT, "Intel", "I210 Gigabit Network Connection Unprogrammed"}, {PCI_VENDOR_ID_INTEL, 0x1532, NT, "Intel", "I211 Gigabit Network Connection Unprogrammed"}, {PCI_VENDOR_ID_INTEL, 0x1533, OK, "Intel", "I210 Gigabit Network Connection"}, @@ -100,11 +100,6 @@ {PCI_VENDOR_ID_INTEL, 0x1539, NT, "Intel", "I211 Gigabit Network Connection"}, {0}, }; - -static inline bool is_i210(uint16_t device_id) -{ - return (device_id & 0xfff0) == 0x1530; -}
static int nicintel_ee_probe_i210(struct flashctx *flash) { @@ -146,14 +141,6 @@ flash->chip->block_erasers->eraseblocks[0].count = (flash->chip->total_size * 1024) / (EE_PAGE_MASK + 1);
return 1; -} - -static int nicintel_ee_probe(struct flashctx *flash) -{ - if (is_i210(nicintel_pci->device_id)) - return nicintel_ee_probe_i210(flash); - - return nicintel_ee_probe_82580(flash); }
#define MAX_ATTEMPTS 10000000 @@ -284,6 +271,11 @@ return 0; }
+static int nicintel_ee_erase_i210(struct flashctx *flash, unsigned int addr, unsigned int len) +{ + return nicintel_ee_write_i210(flash, NULL, addr, len); +} + static int nicintel_ee_bitset(int reg, int bit, bool val) { uint32_t tmp; @@ -395,24 +387,24 @@ nicintel_ee_bitset(EEC, EE_REQ, 0); /* Give up direct access. */ return ret; } -static int nicintel_ee_write(struct flashctx *flash, const uint8_t *buf, unsigned int addr, unsigned int len) -{ - if (is_i210(nicintel_pci->device_id)) - return nicintel_ee_write_i210(flash, buf, addr, len);
- return nicintel_ee_write_82580(flash, buf, addr, len); +static int nicintel_ee_erase_82580(struct flashctx *flash, unsigned int addr, unsigned int len) +{ + return nicintel_ee_write_82580(flash, NULL, addr, len); }
-static int nicintel_ee_erase(struct flashctx *flash, unsigned int addr, unsigned int len) -{ - return nicintel_ee_write(flash, NULL, addr, len); -} - -static const struct opaque_master opaque_master_nicintel_ee = { - .probe = nicintel_ee_probe, +static const struct opaque_master opaque_master_nicintel_ee_82580 = { + .probe = nicintel_ee_probe_82580, .read = nicintel_ee_read, - .write = nicintel_ee_write, - .erase = nicintel_ee_erase, + .write = nicintel_ee_write_82580, + .erase = nicintel_ee_erase_82580, +}; + +static const struct opaque_master opaque_master_nicintel_ee_i210 = { + .probe = nicintel_ee_probe_i210, + .read = nicintel_ee_read, + .write = nicintel_ee_write_i210, + .erase = nicintel_ee_erase_i210, };
static int nicintel_ee_shutdown_i210(void *arg) @@ -435,7 +427,7 @@ return -1; }
-static int nicintel_ee_shutdown(void *eecp) +static int nicintel_ee_shutdown_82580(void *eecp) { uint32_t old_eec = *(uint32_t *)eecp; /* Request bitbanging and unselect the chip first to be safe. */ @@ -455,46 +447,61 @@
int nicintel_ee_init(void) { + struct pci_dev *dev; + if (rget_io_perms()) return 1;
- struct pci_dev *dev = pcidev_init(nics_intel_ee, PCI_BASE_ADDRESS_0); - if (!dev) - return 1; - - uint32_t io_base_addr = pcidev_readbar(dev, PCI_BASE_ADDRESS_0); - if (!io_base_addr) - return 1; - - nicintel_eebar = rphysmap("Intel Gigabit NIC w/ SPI EEPROM", - io_base_addr + (is_i210(dev->device_id) ? 0x12000 : 0), MEMMAP_SIZE); - if (!nicintel_eebar) - return 1; - - nicintel_pci = dev; - if ((dev->device_id != UNPROG_DEVICE) && ! is_i210(dev->device_id)) - { - uint32_t eec = pci_mmio_readl(nicintel_eebar + EEC); - - /* C.f. 3.3.1.5 for the detection mechanism (maybe? contradicting the EE_PRES definition), - * and 3.3.1.7 for possible recovery. */ - if (!(eec & BIT(EE_PRES))) { - msg_perr("Controller reports no EEPROM is present.\n"); + dev = pcidev_init(nics_intel_ee_82580, PCI_BASE_ADDRESS_0); + if (dev) { + uint32_t io_base_addr = pcidev_readbar(dev, PCI_BASE_ADDRESS_0); + if (!io_base_addr) return 1; + + nicintel_eebar = rphysmap("Intel Gigabit NIC w/ SPI EEPROM", io_base_addr, MEMMAP_SIZE); + if (!nicintel_eebar) + return 1; + + nicintel_pci = dev; + if ((dev->device_id != UNPROG_DEVICE)) { + uint32_t eec = pci_mmio_readl(nicintel_eebar + EEC); + + /* C.f. 3.3.1.5 for the detection mechanism (maybe? contradicting + the EE_PRES definition), + and 3.3.1.7 for possible recovery. */ + if (!(eec & BIT(EE_PRES))) { + msg_perr("Controller reports no EEPROM is present.\n"); + return 1; + } + + uint32_t *eecp = malloc(sizeof(uint32_t)); + if (eecp == NULL) + return 1; + *eecp = eec; + + if (register_shutdown(nicintel_ee_shutdown_82580, eecp)) + return 1; }
- uint32_t *eecp = malloc(sizeof(uint32_t)); - if (eecp == NULL) - return 1; - *eecp = eec; - - if (register_shutdown(nicintel_ee_shutdown, eecp)) - return 1; + return register_opaque_master(&opaque_master_nicintel_ee_82580); }
- if (is_i210(dev->device_id)) + dev = pcidev_init(nics_intel_ee_i210, PCI_BASE_ADDRESS_0); + if (dev) { + uint32_t io_base_addr = pcidev_readbar(dev, PCI_BASE_ADDRESS_0); + if (!io_base_addr) + return 1; + + nicintel_eebar = rphysmap("Intel Gigabit i210 NIC w/ emulated EEPROM", + io_base_addr + 0x12000, MEMMAP_SIZE); + if (!nicintel_eebar) + return 1; + if (register_shutdown(nicintel_ee_shutdown_i210, NULL)) return 1;
- return register_opaque_master(&opaque_master_nicintel_ee); + return register_opaque_master(&opaque_master_nicintel_ee_i210); + } + + return 1; } diff --git a/programmer.h b/programmer.h index a98b713..9d86804 100644 --- a/programmer.h +++ b/programmer.h @@ -437,7 +437,6 @@ /* nicintel_eeprom.c */ #if CONFIG_NICINTEL_EEPROM == 1 int nicintel_ee_init(void); -extern const struct dev_entry nics_intel_ee[]; #endif
/* ogp_spi.c */
-
Nico Huber (Code Review)