[flashrom] [PATCH] Unify PCI init and let pcidev clean itself up.
Stefan Tauner
stefan.tauner at student.tuwien.ac.at
Wed Jan 2 15:43:48 CET 2013
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 at 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 at 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 at 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 at 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;
}
--
Kind regards, Stefan Tauner
More information about the flashrom
mailing list