Nico Huber has uploaded this change for review.
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 */
To view, visit change 21935. To unsubscribe, visit settings.