[flashrom] [commit] r1644 - trunk

repository service svn at flashrom.org
Sun Jan 6 00:52:47 CET 2013


Author: hailfinger
Date: Sun Jan  6 00:52:45 2013
New Revision: 1644
URL: http://flashrom.org/trac/flashrom/changeset/1644

Log:
Decouple BAR reading from pci device init, handle errors gracefully.

pcidev_init() now returns struct pci_device * instead of a BAR stored in
PCI config space. This allows for real error checking instead of having
exit(1) everywhere in pcidev.c.
Thanks to Niklas Söderlund for coming up with the original error
handling patch which was slightly modified and folded into this patch.
Move the declaration of struct pci_device in programmer.h before the
first user.

Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net>
Acked-by: Stefan Tauner <stefan.tauner at student.tuwien.ac.at>

Modified:
   trunk/atahpt.c
   trunk/drkaiser.c
   trunk/gfxnvidia.c
   trunk/nic3com.c
   trunk/nicintel.c
   trunk/nicintel_spi.c
   trunk/nicnatsemi.c
   trunk/nicrealtek.c
   trunk/ogp_spi.c
   trunk/pcidev.c
   trunk/programmer.h
   trunk/satamv.c
   trunk/satasii.c

Modified: trunk/atahpt.c
==============================================================================
--- trunk/atahpt.c	Fri Jan  4 23:54:07 2013	(r1643)
+++ trunk/atahpt.c	Sun Jan  6 00:52:45 2013	(r1644)
@@ -58,17 +58,22 @@
 
 int atahpt_init(void)
 {
+	struct pci_dev *dev = NULL;
 	uint32_t reg32;
 
 	if (rget_io_perms())
 		return 1;
 
-	io_base_addr = pcidev_init(PCI_BASE_ADDRESS_4, ata_hpt);
+	dev = pcidev_init(ata_hpt, PCI_BASE_ADDRESS_4);
+	if (!dev)
+		return 1;
+
+	io_base_addr = pcidev_readbar(dev, PCI_BASE_ADDRESS_4);
 
 	/* Enable flash access. */
-	reg32 = pci_read_long(pcidev_dev, REG_FLASH_ACCESS);
+	reg32 = pci_read_long(dev, REG_FLASH_ACCESS);
 	reg32 |= (1 << 24);
-	rpci_write_long(pcidev_dev, REG_FLASH_ACCESS, reg32);
+	rpci_write_long(dev, REG_FLASH_ACCESS, reg32);
 
 	register_par_programmer(&par_programmer_atahpt, BUS_PARALLEL);
 

Modified: trunk/drkaiser.c
==============================================================================
--- trunk/drkaiser.c	Fri Jan  4 23:54:07 2013	(r1643)
+++ trunk/drkaiser.c	Sun Jan  6 00:52:45 2013	(r1644)
@@ -64,17 +64,20 @@
 
 int drkaiser_init(void)
 {
+	struct pci_dev *dev = NULL;
 	uint32_t addr;
 
 	if (rget_io_perms())
 		return 1;
 
-	/* No need to check for errors, pcidev_init() will not return in case of errors. */
-	addr = pcidev_init(PCI_BASE_ADDRESS_2, drkaiser_pcidev);
+	dev = pcidev_init(drkaiser_pcidev, PCI_BASE_ADDRESS_2);
+	if (!dev)
+		return 1;
+
+	addr = pcidev_readbar(dev, PCI_BASE_ADDRESS_2);
 
 	/* Write magic register to enable flash write. */
-	rpci_write_word(pcidev_dev, PCI_MAGIC_DRKAISER_ADDR,
-			PCI_MAGIC_DRKAISER_VALUE);
+	rpci_write_word(dev, PCI_MAGIC_DRKAISER_ADDR, PCI_MAGIC_DRKAISER_VALUE);
 
 	/* Map 128kB flash memory window. */
 	drkaiser_bar = physmap("Dr. Kaiser PC-Waechter flash memory",

Modified: trunk/gfxnvidia.c
==============================================================================
--- trunk/gfxnvidia.c	Fri Jan  4 23:54:07 2013	(r1643)
+++ trunk/gfxnvidia.c	Sun Jan  6 00:52:45 2013	(r1644)
@@ -85,14 +85,17 @@
 
 int gfxnvidia_init(void)
 {
+	struct pci_dev *dev = NULL;
 	uint32_t reg32;
 
 	if (rget_io_perms())
 		return 1;
 
-	/* No need to check for errors, pcidev_init() will not return in case of errors. */
-	io_base_addr = pcidev_init(PCI_BASE_ADDRESS_0, gfx_nvidia);
+	dev = pcidev_init(gfx_nvidia, PCI_BASE_ADDRESS_0);
+	if (!dev)
+		return 1;
 
+	io_base_addr = pcidev_readbar(dev, PCI_BASE_ADDRESS_0);
 	io_base_addr += 0x300000;
 	msg_pinfo("Detected NVIDIA I/O base address: 0x%x.\n", io_base_addr);
 
@@ -102,9 +105,9 @@
 		return 1;
 
 	/* Allow access to flash interface (will disable screen). */
-	reg32 = pci_read_long(pcidev_dev, 0x50);
+	reg32 = pci_read_long(dev, 0x50);
 	reg32 &= ~(1 << 0);
-	rpci_write_long(pcidev_dev, 0x50, reg32);
+	rpci_write_long(dev, 0x50, reg32);
 
 	/* Write/erase doesn't work. */
 	programmer_may_write = 0;

Modified: trunk/nic3com.c
==============================================================================
--- trunk/nic3com.c	Fri Jan  4 23:54:07 2013	(r1643)
+++ trunk/nic3com.c	Sun Jan  6 00:52:45 2013	(r1644)
@@ -86,13 +86,18 @@
 
 int nic3com_init(void)
 {
+	struct pci_dev *dev = NULL;
+
 	if (rget_io_perms())
 		return 1;
 
-	/* No need to check for errors, pcidev_init() will not return in case of errors. */
-	io_base_addr = pcidev_init(PCI_BASE_ADDRESS_0, nics_3com);
+	dev = pcidev_init(nics_3com, PCI_BASE_ADDRESS_0);
+	if (!dev)
+		return 1;
+
+	io_base_addr = pcidev_readbar(dev, PCI_BASE_ADDRESS_0);
 
-	id = pcidev_dev->device_id;
+	id = dev->device_id;
 
 	/* 3COM 3C90xB cards need a special fixup. */
 	if (id == 0x9055 || id == 0x9001 || id == 0x9004 || id == 0x9005

Modified: trunk/nicintel.c
==============================================================================
--- trunk/nicintel.c	Fri Jan  4 23:54:07 2013	(r1643)
+++ trunk/nicintel.c	Sun Jan  6 00:52:45 2013	(r1644)
@@ -68,6 +68,7 @@
 
 int nicintel_init(void)
 {
+	struct pci_dev *dev = NULL;
 	uintptr_t addr;
 
 	/* Needed only for PCI accesses on some platforms.
@@ -76,17 +77,17 @@
 	if (rget_io_perms())
 		return 1;
 
-	/* No need to check for errors, pcidev_init() will not return in case of errors.
-	 * FIXME: BAR2 is not available if the device uses the CardBus function.
-	 */
-	addr = pcidev_init(PCI_BASE_ADDRESS_2, nics_intel);
+	/* FIXME: BAR2 is not available if the device uses the CardBus function. */
+	dev = pcidev_init(nics_intel, PCI_BASE_ADDRESS_2);
+	if (!dev)
+		return 1;
 
+	addr = pcidev_readbar(dev, PCI_BASE_ADDRESS_2);
 	nicintel_bar = physmap("Intel NIC flash", addr, NICINTEL_MEMMAP_SIZE);
 	if (nicintel_bar == ERROR_PTR)
 		goto error_out_unmap;
 
-	/* FIXME: Using pcidev_dev _will_ cause pretty explosions in the future. */
-	addr = pcidev_readbar(pcidev_dev, PCI_BASE_ADDRESS_0);
+	addr = pcidev_readbar(dev, PCI_BASE_ADDRESS_0);
 	/* FIXME: This is not an aligned mapping. Use 4k? */
 	nicintel_control_bar = physmap("Intel NIC control/status reg",
 	                               addr, NICINTEL_CONTROL_MEMMAP_SIZE);

Modified: trunk/nicintel_spi.c
==============================================================================
--- trunk/nicintel_spi.c	Fri Jan  4 23:54:07 2013	(r1643)
+++ trunk/nicintel_spi.c	Sun Jan  6 00:52:45 2013	(r1644)
@@ -166,14 +166,17 @@
 
 int nicintel_spi_init(void)
 {
+	struct pci_dev *dev = NULL;
 	uint32_t tmp;
 
 	if (rget_io_perms())
 		return 1;
 
-	/* No need to check for errors, pcidev_init() will not return in case of errors. */
-	io_base_addr = pcidev_init(PCI_BASE_ADDRESS_0, nics_intel_spi);
+	dev = pcidev_init(nics_intel_spi, PCI_BASE_ADDRESS_0);
+	if (!dev)
+		return 1;
 
+	io_base_addr = pcidev_readbar(dev, PCI_BASE_ADDRESS_0);
 	nicintel_spibar = physmap("Intel Gigabit NIC w/ SPI flash",
 				  io_base_addr, MEMMAP_SIZE);
 	/* Automatic restore of EECD on shutdown is not possible because EECD

Modified: trunk/nicnatsemi.c
==============================================================================
--- trunk/nicnatsemi.c	Fri Jan  4 23:54:07 2013	(r1643)
+++ trunk/nicnatsemi.c	Sun Jan  6 00:52:45 2013	(r1644)
@@ -52,22 +52,19 @@
 		.chip_writen		= fallback_chip_writen,
 };
 
-static int nicnatsemi_shutdown(void *data)
-{
-	pci_cleanup(pacc);
-	return 0;
-}
-
 int nicnatsemi_init(void)
 {
+	struct pci_dev *dev = NULL;
+
 	if (rget_io_perms())
 		return 1;
 
-	io_base_addr = pcidev_init(PCI_BASE_ADDRESS_0, nics_natsemi);
-
-	if (register_shutdown(nicnatsemi_shutdown, NULL))
+	dev = pcidev_init(nics_natsemi, PCI_BASE_ADDRESS_0);
+	if (!dev)
 		return 1;
 
+	io_base_addr = pcidev_readbar(dev, PCI_BASE_ADDRESS_0);
+
 	/* The datasheet shows address lines MA0-MA16 in one place and MA0-MA15
 	 * in another. My NIC has MA16 connected to A16 on the boot ROM socket
 	 * so I'm assuming it is accessible. If not then next line wants to be

Modified: trunk/nicrealtek.c
==============================================================================
--- trunk/nicrealtek.c	Fri Jan  4 23:54:07 2013	(r1643)
+++ trunk/nicrealtek.c	Sun Jan  6 00:52:45 2013	(r1644)
@@ -59,16 +59,19 @@
 
 int nicrealtek_init(void)
 {
+	struct pci_dev *dev = NULL;
+
 	if (rget_io_perms())
 		return 1;
 
-	/* No need to check for errors, pcidev_init() will not return in case of errors. */
-	io_base_addr = pcidev_init(PCI_BASE_ADDRESS_0, nics_realtek);
-	if (register_shutdown(nicrealtek_shutdown, NULL))
+	dev = pcidev_init(nics_realtek, PCI_BASE_ADDRESS_0);
+	if (!dev)
 		return 1;
 
+	io_base_addr = pcidev_readbar(dev, PCI_BASE_ADDRESS_0);
+
 	/* Beware, this ignores the vendor ID! */
-	switch (pcidev_dev->device_id) {
+	switch (dev->device_id) {
 	case 0x8139: /* RTL8139 */
 	case 0x1211: /* SMC 1211TX */
 	default:
@@ -81,6 +84,9 @@
 		break;
 	}
 
+	if (register_shutdown(nicrealtek_shutdown, NULL))
+		return 1;
+
 	register_par_programmer(&par_programmer_nicrealtek, BUS_PARALLEL);
 
 	return 0;

Modified: trunk/ogp_spi.c
==============================================================================
--- trunk/ogp_spi.c	Fri Jan  4 23:54:07 2013	(r1643)
+++ trunk/ogp_spi.c	Sun Jan  6 00:52:45 2013	(r1644)
@@ -105,6 +105,7 @@
 
 int ogp_spi_init(void)
 {
+	struct pci_dev *dev = NULL;
 	char *type;
 
 	type = extract_programmer_param("rom");
@@ -131,8 +132,11 @@
 	if (rget_io_perms())
 		return 1;
 
-	io_base_addr = pcidev_init(PCI_BASE_ADDRESS_0, ogp_spi);
+	dev = pcidev_init(ogp_spi, PCI_BASE_ADDRESS_0);
+	if (!dev)
+		return 1;
 
+	io_base_addr = pcidev_readbar(dev, PCI_BASE_ADDRESS_0);
 	ogp_spibar = physmap("OGP registers", io_base_addr, 4096);
 
 	if (register_shutdown(ogp_spi_shutdown, NULL))

Modified: trunk/pcidev.c
==============================================================================
--- trunk/pcidev.c	Fri Jan  4 23:54:07 2013	(r1643)
+++ trunk/pcidev.c	Sun Jan  6 00:52:45 2013	(r1644)
@@ -27,7 +27,6 @@
 
 uint32_t io_base_addr;
 struct pci_access *pacc;
-struct pci_dev *pcidev_dev = NULL;
 
 enum pci_bartype {
 	TYPE_MEMBAR,
@@ -156,7 +155,6 @@
 
 static int pcidev_shutdown(void *data)
 {
-	pcidev_dev = NULL;
 	if (pacc == NULL) {
 		msg_perr("%s: Tried to cleanup an invalid PCI context!\n"
 			 "Please report a bug at flashrom at flashrom.org\n", __func__);
@@ -181,18 +179,24 @@
 	return 0;
 }
 
-uintptr_t pcidev_init(int bar, const struct dev_entry *devs)
+/* pcidev_init gets an array of allowed PCI device IDs and returns a pointer to struct pci_dev iff exactly one
+ * match was found. If the "pci=bb:dd.f" programmer parameter was specified, a match is only considered if it
+ * also matches the specified bus:device.function.
+ * For convenience, this function also registers its own undo handlers.
+ */
+struct pci_dev *pcidev_init(const struct dev_entry *devs, int bar)
 {
 	struct pci_dev *dev;
+	struct pci_dev *found_dev = NULL;
 	struct pci_filter filter;
 	char *pcidev_bdf;
 	char *msg = NULL;
 	int found = 0;
 	int i;
-	uintptr_t addr = 0, curaddr = 0;
+	uintptr_t addr = 0;
 
-	if(pci_init_common() != 0)
-		return 1;
+	if (pci_init_common() != 0)
+		return NULL;
 	pci_filter_init(pacc, &filter);
 
 	/* Filter by bb:dd.f (if supplied by the user). */
@@ -200,7 +204,7 @@
 	if (pcidev_bdf != NULL) {
 		if ((msg = pci_filter_parse_slot(&filter, pcidev_bdf))) {
 			msg_perr("Error: %s\n", msg);
-			exit(1);
+			return NULL;
 		}
 	}
 	free(pcidev_bdf);
@@ -230,8 +234,7 @@
 			 * just those with a valid BAR.
 			 */
 			if ((addr = pcidev_readbar(dev, bar)) != 0) {
-				curaddr = addr;
-				pcidev_dev = dev;
+				found_dev = dev;
 				found++;
 			}
 		}
@@ -240,14 +243,14 @@
 	/* Only continue if exactly one supported PCI dev has been found. */
 	if (found == 0) {
 		msg_perr("Error: No supported PCI device found.\n");
-		exit(1);
+		return NULL;
 	} else if (found > 1) {
 		msg_perr("Error: Multiple supported PCI devices found. Use 'flashrom -p xxxx:pci=bb:dd.f' \n"
 			 "to explicitly select the card with the given BDF (PCI bus, device, function).\n");
-		exit(1);
+		return NULL;
 	}
 
-	return curaddr;
+	return found_dev;
 }
 
 enum pci_write_type {

Modified: trunk/programmer.h
==============================================================================
--- trunk/programmer.h	Fri Jan  4 23:54:07 2013	(r1643)
+++ trunk/programmer.h	Sun Jan  6 00:52:45 2013	(r1644)
@@ -160,8 +160,25 @@
 	unsigned int half_period;
 };
 
-#if CONFIG_INTERNAL == 1
+#if NEED_PCI == 1
 struct pci_dev;
+
+/* pcidev.c */
+// FIXME: These need to be local, not global
+extern uint32_t io_base_addr;
+extern struct pci_access *pacc;
+int pci_init_common(void);
+uintptr_t pcidev_readbar(struct pci_dev *dev, int bar);
+struct pci_dev *pcidev_init(const struct dev_entry *devs, int bar);
+/* rpci_write_* are reversible writes. The original PCI config space register
+ * contents will be restored on shutdown.
+ */
+int rpci_write_byte(struct pci_dev *dev, int reg, uint8_t data);
+int rpci_write_word(struct pci_dev *dev, int reg, uint16_t data);
+int rpci_write_long(struct pci_dev *dev, int reg, uint32_t data);
+#endif
+
+#if CONFIG_INTERNAL == 1
 struct penable {
 	uint16_t vendor_id;
 	uint16_t device_id;
@@ -232,23 +249,6 @@
 void myusec_calibrate_delay(void);
 void internal_delay(int usecs);
 
-#if NEED_PCI == 1
-/* pcidev.c */
-// FIXME: These need to be local, not global
-extern uint32_t io_base_addr;
-extern struct pci_access *pacc;
-extern struct pci_dev *pcidev_dev;
-int pci_init_common(void);
-uintptr_t pcidev_readbar(struct pci_dev *dev, int bar);
-uintptr_t pcidev_init(int bar, const struct dev_entry *devs);
-/* rpci_write_* are reversible writes. The original PCI config space register
- * contents will be restored on shutdown.
- */
-int rpci_write_byte(struct pci_dev *dev, int reg, uint8_t data);
-int rpci_write_word(struct pci_dev *dev, int reg, uint16_t data);
-int rpci_write_long(struct pci_dev *dev, int reg, uint32_t data);
-#endif
-
 #if CONFIG_INTERNAL == 1
 /* board_enable.c */
 int board_parse_parameter(const char *boardstring, const char **vendor, const char **model);

Modified: trunk/satamv.c
==============================================================================
--- trunk/satamv.c	Fri Jan  4 23:54:07 2013	(r1643)
+++ trunk/satamv.c	Sun Jan  6 00:52:45 2013	(r1644)
@@ -81,6 +81,7 @@
  */
 int satamv_init(void)
 {
+	struct pci_dev *dev = NULL;
 	uintptr_t addr;
 	uint32_t tmp;
 
@@ -88,11 +89,11 @@
 		return 1;
 
 	/* BAR0 has all internal registers memory mapped. */
-	/* No need to check for errors, pcidev_init() will not return in case
-	 * of errors.
-	 */
-	addr = pcidev_init(PCI_BASE_ADDRESS_0, satas_mv);
+	dev = pcidev_init(satas_mv, PCI_BASE_ADDRESS_0);
+	if (!dev)
+		return 1;
 
+	addr = pcidev_readbar(dev, PCI_BASE_ADDRESS_0);
 	mv_bar = physmap("Marvell 88SX7042 registers", addr, 0x20000);
 	if (mv_bar == ERROR_PTR)
 		return 1;
@@ -143,8 +144,7 @@
 	pci_rmmio_writel(tmp, mv_bar + GPIO_PORT_CONTROL);
 
 	/* Get I/O BAR location. */
-	tmp = pci_read_long(pcidev_dev, PCI_BASE_ADDRESS_2) &
-	      PCI_BASE_ADDRESS_IO_MASK;
+	tmp = pcidev_readbar(dev, PCI_BASE_ADDRESS_2);
 	/* Truncate to reachable range.
 	 * FIXME: Check if the I/O BAR is actually reachable.
 	 * This is an arch specific check.

Modified: trunk/satasii.c
==============================================================================
--- trunk/satasii.c	Fri Jan  4 23:54:07 2013	(r1643)
+++ trunk/satasii.c	Sun Jan  6 00:52:45 2013	(r1644)
@@ -76,21 +76,24 @@
 
 int satasii_init(void)
 {
+	struct pci_dev *dev = NULL;
 	uint32_t addr;
 	uint16_t reg_offset;
 
 	if (rget_io_perms())
 		return 1;
 
-	pcidev_init(PCI_BASE_ADDRESS_0, satas_sii);
+	dev = pcidev_init(satas_sii, PCI_BASE_ADDRESS_0);
+	if (!dev)
+		return 1;
 
-	id = pcidev_dev->device_id;
+	id = dev->device_id;
 
 	if ((id == 0x3132) || (id == 0x3124)) {
-		addr = pci_read_long(pcidev_dev, PCI_BASE_ADDRESS_0) & ~0x07;
+		addr = pcidev_readbar(dev, PCI_BASE_ADDRESS_0);
 		reg_offset = 0x70;
 	} else {
-		addr = pci_read_long(pcidev_dev, PCI_BASE_ADDRESS_5) & ~0x07;
+		addr = pcidev_readbar(dev, PCI_BASE_ADDRESS_5);
 		reg_offset = 0x50;
 	}
 




More information about the flashrom mailing list