Previously the internal programmer used its own code to initialize pcilib. This patch extracts the common code from the internal programmer and pcidev_init() into pcidev_init_common(). This fixes the non-existent PCI cleanup of the internal programmer and adds an additional safety by checking for an already existing PCI context.
We got a nice shutdown function registration infrastructure, but did not use it very wisely. Instead we added shutdown functions to a myriad of programmers unnecessarily. In this patch we get rid of those that do only call pci_cleanup(pacc) by adding a shutdown function the pcidev.c itself that gets registered by pcidev_init().
Signed-off-by: Stefan Tauner stefan.tauner@student.tuwien.ac.at --- atahpt.c | 10 ---------- drkaiser.c | 3 +-- gfxnvidia.c | 6 +----- internal.c | 6 ++---- nic3com.c | 2 +- nicintel.c | 5 +---- nicintel_spi.c | 2 +- nicrealtek.c | 3 +-- ogp_spi.c | 2 -- pcidev.c | 37 ++++++++++++++++++++++++++++++++++--- programmer.h | 1 + satamv.c | 7 +------ satasii.c | 1 - 13 files changed, 44 insertions(+), 41 deletions(-)
diff --git a/atahpt.c b/atahpt.c index 461191d..d19cb75 100644 --- a/atahpt.c +++ b/atahpt.c @@ -56,13 +56,6 @@ static const struct par_programmer par_programmer_atahpt = { .chip_writen = fallback_chip_writen, };
-static int atahpt_shutdown(void *data) -{ - /* Flash access is disabled automatically by PCI restore. */ - pci_cleanup(pacc); - return 0; -} - int atahpt_init(void) { uint32_t reg32; @@ -72,9 +65,6 @@ int atahpt_init(void)
io_base_addr = pcidev_init(PCI_BASE_ADDRESS_4, ata_hpt);
- if (register_shutdown(atahpt_shutdown, NULL)) - return 1; - /* Enable flash access. */ reg32 = pci_read_long(pcidev_dev, REG_FLASH_ACCESS); reg32 |= (1 << 24); diff --git a/drkaiser.c b/drkaiser.c index e461061..a6eca1c 100644 --- a/drkaiser.c +++ b/drkaiser.c @@ -59,8 +59,6 @@ static const struct par_programmer par_programmer_drkaiser = { static int drkaiser_shutdown(void *data) { physunmap(drkaiser_bar, DRKAISER_MEMMAP_SIZE); - /* Flash write is disabled automatically by PCI restore. */ - pci_cleanup(pacc); return 0; }
@@ -71,6 +69,7 @@ int drkaiser_init(void) 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);
/* Write magic register to enable flash write. */ diff --git a/gfxnvidia.c b/gfxnvidia.c index 624fd7c..a994d68 100644 --- a/gfxnvidia.c +++ b/gfxnvidia.c @@ -80,10 +80,6 @@ static const struct par_programmer par_programmer_gfxnvidia = { static int gfxnvidia_shutdown(void *data) { physunmap(nvidia_bar, GFXNVIDIA_MEMMAP_SIZE); - /* Flash interface access is disabled (and screen enabled) automatically - * by PCI restore. - */ - pci_cleanup(pacc); return 0; }
@@ -94,6 +90,7 @@ int gfxnvidia_init(void) 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);
io_base_addr += 0x300000; @@ -101,7 +98,6 @@ int gfxnvidia_init(void)
nvidia_bar = physmap("NVIDIA", io_base_addr, GFXNVIDIA_MEMMAP_SIZE);
- /* Must be done before rpci calls. */ if (register_shutdown(gfxnvidia_shutdown, NULL)) return 1;
diff --git a/internal.c b/internal.c index eda4d59..8dd844d 100644 --- a/internal.c +++ b/internal.c @@ -245,10 +245,8 @@ int internal_init(void) internal_buses_supported = BUS_NONSPI;
/* Initialize PCI access for flash enables */ - pacc = pci_alloc(); /* Get the pci_access structure */ - /* Set all options you want -- here we stick with the defaults */ - pci_init(pacc); /* Initialize the PCI library */ - pci_scan_bus(pacc); /* We want to get the list of devices */ + if (pci_init_common() != 0) + return 1;
if (processor_flash_enable()) { msg_perr("Processor detection/init failed.\n" diff --git a/nic3com.c b/nic3com.c index 05eada6..4ec6193 100644 --- a/nic3com.c +++ b/nic3com.c @@ -81,7 +81,6 @@ static int nic3com_shutdown(void *data) OUTL(internal_conf, io_base_addr + INTERNAL_CONFIG); }
- pci_cleanup(pacc); return 0; }
@@ -90,6 +89,7 @@ int nic3com_init(void) 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);
id = pcidev_dev->device_id; diff --git a/nicintel.c b/nicintel.c index d210d78..8481915 100644 --- a/nicintel.c +++ b/nicintel.c @@ -63,7 +63,6 @@ static int nicintel_shutdown(void *data) { physunmap(nicintel_control_bar, NICINTEL_CONTROL_MEMMAP_SIZE); physunmap(nicintel_bar, NICINTEL_MEMMAP_SIZE); - pci_cleanup(pacc); return 0; }
@@ -77,8 +76,7 @@ int nicintel_init(void) if (rget_io_perms()) return 1;
- /* No need to check for errors, pcidev_init() will not return in case - * of errors. + /* 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); @@ -117,7 +115,6 @@ int nicintel_init(void) error_out_unmap: physunmap(nicintel_bar, NICINTEL_MEMMAP_SIZE); error_out: - pci_cleanup(pacc); return 1; }
diff --git a/nicintel_spi.c b/nicintel_spi.c index 325e61c..f61c2b1 100644 --- a/nicintel_spi.c +++ b/nicintel_spi.c @@ -160,7 +160,6 @@ static int nicintel_spi_shutdown(void *data) pci_mmio_writel(tmp, nicintel_spibar + EECD);
physunmap(nicintel_spibar, MEMMAP_SIZE); - pci_cleanup(pacc);
return 0; } @@ -172,6 +171,7 @@ int nicintel_spi_init(void) 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);
nicintel_spibar = physmap("Intel Gigabit NIC w/ SPI flash", diff --git a/nicrealtek.c b/nicrealtek.c index 3c3b261..8349b42 100644 --- a/nicrealtek.c +++ b/nicrealtek.c @@ -54,7 +54,6 @@ static const struct par_programmer par_programmer_nicrealtek = { static int nicrealtek_shutdown(void *data) { /* FIXME: We forgot to disable software access again. */ - pci_cleanup(pacc); return 0; }
@@ -63,8 +62,8 @@ int nicrealtek_init(void) 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)) return 1;
diff --git a/ogp_spi.c b/ogp_spi.c index d1bb12f..6fb1a77 100644 --- a/ogp_spi.c +++ b/ogp_spi.c @@ -100,8 +100,6 @@ static const struct bitbang_spi_master bitbang_spi_master_ogp = { static int ogp_spi_shutdown(void *data) { physunmap(ogp_spibar, 4096); - pci_cleanup(pacc); - return 0; }
diff --git a/pcidev.c b/pcidev.c index 1a26e99..f2c8827 100644 --- a/pcidev.c +++ b/pcidev.c @@ -154,6 +154,33 @@ uintptr_t pcidev_readbar(struct pci_dev *dev, int bar) return (uintptr_t)addr; }
+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@flashrom.org\n", __func__); + return 1; + } + pci_cleanup(pacc); + return 0; +} + +int pci_init_common(void) +{ + if (pacc != NULL) { + msg_perr("%s: Tried to allocate a new PCI context, but there is still an old one!\n" + "Please report a bug at flashrom@flashrom.org\n", __func__); + return 1; + } + pacc = pci_alloc(); /* Get the pci_access structure */ + pci_init(pacc); /* Initialize the PCI library */ + if (register_shutdown(pcidev_shutdown, NULL)) + return 1; + pci_scan_bus(pacc); /* We want to get the list of devices */ + return 0; +} + uintptr_t pcidev_init(int bar, const struct dev_entry *devs) { struct pci_dev *dev; @@ -164,9 +191,8 @@ uintptr_t pcidev_init(int bar, const struct dev_entry *devs) int i; uintptr_t addr = 0, curaddr = 0;
- pacc = pci_alloc(); /* Get the pci_access structure */ - pci_init(pacc); /* Initialize the PCI library */ - pci_scan_bus(pacc); /* We want to get the list of devices */ + if(pci_init_common() != 0) + return 1; pci_filter_init(pacc, &filter);
/* Filter by bb:dd.f (if supplied by the user). */ @@ -244,6 +270,11 @@ struct undo_pci_write_data { int undo_pci_write(void *p) { struct undo_pci_write_data *data = p; + if (pacc == NULL) { + msg_perr("%s: Tried to undo PCI writes without a valid PCI context!\n" + "Please report a bug at flashrom@flashrom.org\n", __func__); + return 1; + } msg_pdbg("Restoring PCI config space for %02x:%02x:%01x reg 0x%02x\n", data->dev.bus, data->dev.dev, data->dev.func, data->reg); switch (data->type) { diff --git a/programmer.h b/programmer.h index 8de42bc..4302809 100644 --- a/programmer.h +++ b/programmer.h @@ -238,6 +238,7 @@ void internal_delay(int usecs); 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 diff --git a/satamv.c b/satamv.c index c0c1ffa..46a0e2d 100644 --- a/satamv.c +++ b/satamv.c @@ -60,7 +60,6 @@ static const struct par_programmer par_programmer_satamv = { static int satamv_shutdown(void *data) { physunmap(mv_bar, 0x20000); - pci_cleanup(pacc); return 0; }
@@ -96,7 +95,7 @@ int satamv_init(void)
mv_bar = physmap("Marvell 88SX7042 registers", addr, 0x20000); if (mv_bar == ERROR_PTR) - goto error_out; + return 1;
if (register_shutdown(satamv_shutdown, NULL)) return 1; @@ -159,10 +158,6 @@ int satamv_init(void) register_par_programmer(&par_programmer_satamv, BUS_PARALLEL);
return 0; - -error_out: - pci_cleanup(pacc); - return 1; }
/* BAR2 (MEM) can map NVRAM and flash. We set it to flash in the init function. diff --git a/satasii.c b/satasii.c index 7b94203..730c420 100644 --- a/satasii.c +++ b/satasii.c @@ -57,7 +57,6 @@ static const struct par_programmer par_programmer_satasii = { static int satasii_shutdown(void *data) { physunmap(sii_bar, SATASII_MEMMAP_SIZE); - pci_cleanup(pacc); return 0; }