Am 01.01.2013 18:29 schrieb Stefan Tauner:
Similarly to the previous PCI self-clean up patch this one allows to get rid of a huge number of programmer shutdown functions and makes introducing bugs harder. It adds a new function physmap_autocleanup() that takes care of unmapping at shutdown. Callers are changed where it makes sense.
Signed-off-by: Stefan Tauner stefan.tauner@student.tuwien.ac.at
Good work! The individual driver conversions were checked thoroughly and seem to be OK.
With the comments addressed, this is Acked-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net
Can you please change the name of physmap_autocleanup to rphysmap? That would be consistent with rmmio_write* and rpci_write* and be shorter as well (fixing some 112 columns limit issues).
diff --git a/it85spi.c b/it85spi.c index 0b074eb..b778436 100644 --- a/it85spi.c +++ b/it85spi.c @@ -262,6 +262,9 @@ static int it85xx_spi_common_init(struct superio s) * Major TODO here, and it will be a lot of work. */ base = (chipaddr)physmap("it85 communication", 0xFFFFF000, 0x1000);
- if (base == ERROR_PTR)
I somehow doubt that this really compiles. AFAICS you might not have noticed the type mismatch problem because of #if guards.
return 1;- msg_pdbg("%s():%d base=0x%08x\n", __func__, __LINE__, (unsigned int)base); ce_high = (unsigned char *)(base + 0xE00); /* 0xFFFFFE00 */
diff --git a/nicintel.c b/nicintel.c index 8481915..5463af2 100644 --- a/nicintel.c +++ b/nicintel.c @@ -81,19 +74,15 @@ int nicintel_init(void) */ addr = pcidev_init(PCI_BASE_ADDRESS_2, nics_intel);
- nicintel_bar = physmap("Intel NIC flash", addr, NICINTEL_MEMMAP_SIZE);
- nicintel_bar = physmap_autocleanup("Intel NIC flash", addr, NICINTEL_MEMMAP_SIZE); if (nicintel_bar == ERROR_PTR)
goto error_out_unmap;
return 1;/* FIXME: Using pcidev_dev _will_ cause pretty explosions in the future. */ addr = pcidev_readbar(pcidev_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);
- nicintel_control_bar = physmap_autocleanup("Intel NIC control/status reg", addr, NICINTEL_CONTROL_MEMMAP_SIZE);
112 column limit... not a problem with rphysmap.
if (nicintel_control_bar == ERROR_PTR)
goto error_out;if (register_shutdown(nicintel_shutdown, NULL)) return 1;
/* FIXME: This register is pretty undocumented in all publicly available
diff --git a/physmap.c b/physmap.c index 5aa9874..a9445d5 100644 --- a/physmap.c +++ b/physmap.c @@ -21,6 +21,7 @@ */
#include <unistd.h> +#include <stdbool.h>
Note: stdbool.h is only available in C99 compilers, which means this won't ever compile with MSVC. I don't really have a strong preference either way (and we use mingw/cygwin gcc for win* targets anyway).
#include <stdio.h> #include <stdlib.h> #include <string.h> @@ -214,13 +215,25 @@ void physunmap(void *virt_addr, size_t len) } #endif
-#define PHYSMAP_NOFAIL 0 -#define PHYSMAP_MAYFAIL 1 -#define PHYSMAP_RW 0 -#define PHYSMAP_RO 1
IMHO those #defines really helped readability. You can replace 0 and 1 by false/true if you want, but please keep the #defines.
+struct physmap_stutdown_data {
Maybe rename to undo_physmap_data for consistency with undo_pci_write_data.
- void *addr;
Use virt_addr to make it obvious that this is not a physical address, but the address where this was mapped into the address space of the flashrom process?
- size_t len;
+};
-static void *physmap_common(const char *descr, unsigned long phys_addr,
size_t len, int mayfail, int readonly)+static int physmap_shutdown(void *data)
Rename to undo_physmap for consistency with undo_pci_write?
+{
- if (data == NULL) {
msg_perr("%s: tried to shutdown without valid data!\n", __func__);return 1;- }
- struct physmap_stutdown_data *d = data;
Not all compilers support defining a new variable after code. Please move the line above to the head of the function.
- physunmap(d->addr, d->len);
- free(data);
- return 0;
+}
+static void *physmap_common(const char *descr, unsigned long phys_addr, size_t len, bool mayfail,
bool readonly, bool autocleanup){ void *virt_addr;
@@ -268,19 +281,40 @@ static void *physmap_common(const char *descr, unsigned long phys_addr, exit(3); }
- if (autocleanup) {
struct physmap_stutdown_data *d = malloc(sizeof(struct physmap_stutdown_data));
I wonder if that malloc should be moved to the start of physmap_common. It would detect OOM earlier, before we do any mapping.
if (d == NULL) {msg_perr("%s: Out of memory!\n", __func__);goto unmap_out;}d->addr = virt_addr;d->len = len;if (register_shutdown(physmap_shutdown, d) != 0) {msg_perr("%s: Could not register shutdown function!\n", __func__);goto unmap_out;}- }
- return virt_addr;
+unmap_out:
- physunmap(virt_addr, len);
- return ERROR_PTR;
}
void *physmap(const char *descr, unsigned long phys_addr, size_t len) {
- return physmap_common(descr, phys_addr, len, PHYSMAP_NOFAIL,
PHYSMAP_RW);
- return physmap_common(descr, phys_addr, len, false, false, false);
I really liked the #defines here because they increased readability.
+}
+void *physmap_autocleanup(const char *descr, unsigned long phys_addr, size_t len) +{
- return physmap_common(descr, phys_addr, len, false, false, true);
}
void *physmap_try_ro(const char *descr, unsigned long phys_addr, size_t len) {
- return physmap_common(descr, phys_addr, len, PHYSMAP_MAYFAIL,
PHYSMAP_RO);
- return physmap_common(descr, phys_addr, len, true, true, false);
}
#if defined(__i386__) || defined(__x86_64__)
Regards, Carl-Daniel
On Mon, 07 Jan 2013 01:46:16 +0100 Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net wrote:
Am 01.01.2013 18:29 schrieb Stefan Tauner:
Similarly to the previous PCI self-clean up patch this one allows to get rid of a huge number of programmer shutdown functions and makes introducing bugs harder. It adds a new function physmap_autocleanup() that takes care of unmapping at shutdown. Callers are changed where it makes sense.
Signed-off-by: Stefan Tauner stefan.tauner@student.tuwien.ac.at
Good work! The individual driver conversions were checked thoroughly and seem to be OK.
and then it was forgotten more or less, sorry ;) I had to rebase it (and solve small conflicts).
diff --git a/it85spi.c b/it85spi.c index 0b074eb..b778436 100644 --- a/it85spi.c +++ b/it85spi.c @@ -262,6 +262,9 @@ static int it85xx_spi_common_init(struct superio s) * Major TODO here, and it will be a lot of work. */ base = (chipaddr)physmap("it85 communication", 0xFFFFF000, 0x1000);
- if (base == ERROR_PTR)
I somehow doubt that this really compiles. AFAICS you might not have noticed the type mismatch problem because of #if guards.
right. I solved this by casting ERROR_PTR to chipaddr too, but I am not entirely sure that is safe/what we want.
+#include <stdbool.h>
Note: stdbool.h is only available in C99 compilers, which means this won't ever compile with MSVC. I don't really have a strong preference either way (and we use mingw/cygwin gcc for win* targets anyway).
Talk to me about MSVC again when they support C11. ;)
#include <stdio.h> #include <stdlib.h> #include <string.h> @@ -214,13 +215,25 @@ void physunmap(void *virt_addr, size_t len) } #endif
-#define PHYSMAP_NOFAIL 0 -#define PHYSMAP_MAYFAIL 1 -#define PHYSMAP_RW 0 -#define PHYSMAP_RO 1
IMHO those #defines really helped readability. You can replace 0 and 1 by false/true if you want, but please keep the #defines.
How often do you read these arguments? Exactly. :P Reverted anyway and added PHYSMAP_NOCLEANUP and PHYSMAP_CLEANUP respectively.
+struct physmap_stutdown_data {
Maybe rename to undo_physmap_data for consistency with undo_pci_write_data.
done
- void *addr;
Use virt_addr to make it obvious that this is not a physical address, but the address where this was mapped into the address space of the flashrom process?
done
- size_t len;
+};
-static void *physmap_common(const char *descr, unsigned long phys_addr,
size_t len, int mayfail, int readonly)+static int physmap_shutdown(void *data)
Rename to undo_physmap for consistency with undo_pci_write?
done
+{
- if (data == NULL) {
msg_perr("%s: tried to shutdown without valid data!\n", __func__);return 1;- }
- struct physmap_stutdown_data *d = data;
Not all compilers support defining a new variable after code. Please move the line above to the head of the function.
No. This is mandatory to be supported since C99 and helps keeping scopes small and uninitialized variables to a minimum which reduces bugs and readability for anybody not mentally fixed to C89 syntax.
- physunmap(d->addr, d->len);
- free(data);
- return 0;
+}
+static void *physmap_common(const char *descr, unsigned long phys_addr, size_t len, bool mayfail,
bool readonly, bool autocleanup){ void *virt_addr;
@@ -268,19 +281,40 @@ static void *physmap_common(const char *descr, unsigned long phys_addr, exit(3); }
- if (autocleanup) {
struct physmap_stutdown_data *d = malloc(sizeof(struct physmap_stutdown_data));I wonder if that malloc should be moved to the start of physmap_common. It would detect OOM earlier, before we do any mapping.
... and would fail in cases when we do not need to malloc at all and we would have to split the if... I don't think this makes sense for this kind of error (which hopefully will never be triggered).
But there is a real bug... we have to check mayfail and act accordingly. I have added this to the code at unmap_out.
New patch set will follow.
Similarly to the previous PCI self-clean up patch this one allows to get rid of a huge number of programmer shutdown functions and makes introducing bugs harder. It adds a new function rphysmap() that takes care of unmapping at shutdown. Callers are changed where it makes sense.
Signed-off-by: Stefan Tauner stefan.tauner@student.tuwien.ac.at Acked-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net --- chipset_enable.c | 12 ++++++----- drkaiser.c | 12 ++--------- gfxnvidia.c | 11 ++-------- ichspi.c | 4 +++- it85spi.c | 3 +++ mcp6x_spi.c | 15 +++++-------- nicintel.c | 22 +++---------------- nicintel_spi.c | 9 ++------ ogp_spi.c | 11 ++-------- physmap.c | 63 +++++++++++++++++++++++++++++++++++++++++++++--------- programmer.h | 1 + satamv.c | 11 +--------- satasii.c | 14 ++++-------- sb600spi.c | 6 ++++-- 14 files changed, 92 insertions(+), 102 deletions(-)
diff --git a/chipset_enable.c b/chipset_enable.c index 1e2df21..6562e1c 100644 --- a/chipset_enable.c +++ b/chipset_enable.c @@ -480,7 +480,9 @@ static int enable_flash_tunnelcreek(struct pci_dev *dev, const char *name) msg_pdbg("\nRoot Complex Register Block address = 0x%x\n", tmp);
/* Map RCBA to virtual memory */ - rcrb = physmap("ICH RCRB", tmp, 0x4000); + rcrb = rphysmap("ICH RCRB", tmp, 0x4000); + if (rcrb == ERROR_PTR) + return 1;
/* Test Boot BIOS Strap Status */ bnt = mmio_readl(rcrb + 0x3410); @@ -551,7 +553,9 @@ static int enable_flash_ich_dc_spi(struct pci_dev *dev, const char *name, msg_pdbg("Root Complex Register Block address = 0x%x\n", tmp);
/* Map RCBA to virtual memory */ - rcrb = physmap("ICH RCRB", tmp, 0x4000); + rcrb = rphysmap("ICH RCRB", tmp, 0x4000); + if (rcrb == ERROR_PTR) + return 1;
gcs = mmio_readl(rcrb + 0x3410); msg_pdbg("GCS = 0x%x: ", gcs); @@ -703,10 +707,8 @@ static int enable_flash_vt_vx(struct pci_dev *dev, const char *name) case 0x8410: /* VX900 */ mmio_base = pci_read_long(dev, 0xbc) << 8; mmio_base_physmapped = physmap("VIA VX MMIO register", mmio_base, SPI_CNTL_LEN); - if (mmio_base_physmapped == ERROR_PTR) { - physunmap(mmio_base_physmapped, SPI_CNTL_LEN); + if (mmio_base_physmapped == ERROR_PTR) return ERROR_FATAL; - }
/* Offset 0 - Bit 0 holds SPI Bus0 Enable Bit. */ spi_cntl = mmio_readl(mmio_base_physmapped) + 0x00; diff --git a/drkaiser.c b/drkaiser.c index b94d6dd..8c9fb6a 100644 --- a/drkaiser.c +++ b/drkaiser.c @@ -56,12 +56,6 @@ static const struct par_programmer par_programmer_drkaiser = { .chip_writen = fallback_chip_writen, };
-static int drkaiser_shutdown(void *data) -{ - physunmap(drkaiser_bar, DRKAISER_MEMMAP_SIZE); - return 0; -} - int drkaiser_init(void) { struct pci_dev *dev = NULL; @@ -80,10 +74,8 @@ int drkaiser_init(void) 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", - addr, DRKAISER_MEMMAP_SIZE); - - if (register_shutdown(drkaiser_shutdown, NULL)) + drkaiser_bar = rphysmap("Dr. Kaiser PC-Waechter flash memory", addr, DRKAISER_MEMMAP_SIZE); + if (drkaiser_bar == ERROR_PTR) return 1;
max_rom_decode.parallel = 128 * 1024; diff --git a/gfxnvidia.c b/gfxnvidia.c index d0a9feb..8f3aa44 100644 --- a/gfxnvidia.c +++ b/gfxnvidia.c @@ -77,12 +77,6 @@ static const struct par_programmer par_programmer_gfxnvidia = { .chip_writen = fallback_chip_writen, };
-static int gfxnvidia_shutdown(void *data) -{ - physunmap(nvidia_bar, GFXNVIDIA_MEMMAP_SIZE); - return 0; -} - int gfxnvidia_init(void) { struct pci_dev *dev = NULL; @@ -99,9 +93,8 @@ int gfxnvidia_init(void) io_base_addr += 0x300000; msg_pinfo("Detected NVIDIA I/O base address: 0x%x.\n", io_base_addr);
- nvidia_bar = physmap("NVIDIA", io_base_addr, GFXNVIDIA_MEMMAP_SIZE); - - if (register_shutdown(gfxnvidia_shutdown, NULL)) + nvidia_bar = rphysmap("NVIDIA", io_base_addr, GFXNVIDIA_MEMMAP_SIZE); + if (nvidia_bar == ERROR_PTR) return 1;
/* Allow access to flash interface (will disable screen). */ diff --git a/ichspi.c b/ichspi.c index 12f4584..abafacd 100644 --- a/ichspi.c +++ b/ichspi.c @@ -1845,7 +1845,9 @@ int via_init_spi(struct pci_dev *dev, uint32_t mmio_base) { int i;
- ich_spibar = physmap("VIA SPI MMIO registers", mmio_base, 0x70); + ich_spibar = rphysmap("VIA SPI MMIO registers", mmio_base, 0x70); + if (ich_spibar == ERROR_PTR) + return ERROR_FATAL; /* Do we really need no write enable? Like the LPC one at D17F0 0x40 */
/* Not sure if it speaks all these bus protocols. */ diff --git a/it85spi.c b/it85spi.c index 0b074eb..7efc680 100644 --- a/it85spi.c +++ b/it85spi.c @@ -262,6 +262,9 @@ static int it85xx_spi_common_init(struct superio s) * Major TODO here, and it will be a lot of work. */ base = (chipaddr)physmap("it85 communication", 0xFFFFF000, 0x1000); + if (base == (chipaddr)ERROR_PTR) + return 1; + msg_pdbg("%s():%d base=0x%08x\n", __func__, __LINE__, (unsigned int)base); ce_high = (unsigned char *)(base + 0xE00); /* 0xFFFFFE00 */ diff --git a/mcp6x_spi.c b/mcp6x_spi.c index ac40557..20e9bd8 100644 --- a/mcp6x_spi.c +++ b/mcp6x_spi.c @@ -135,25 +135,20 @@ int mcp6x_spi_init(int want_spi)
/* Accessing a NULL pointer BAR is evil. Don't do it. */ if (!mcp6x_spibaraddr && want_spi) { - msg_perr("Error: Chipset is strapped for SPI, but MCP SPI BAR " - "is invalid.\n"); + msg_perr("Error: Chipset is strapped for SPI, but MCP SPI BAR is invalid.\n"); return 1; } else if (!mcp6x_spibaraddr && !want_spi) { msg_pdbg("MCP SPI is not used.\n"); return 0; } else if (mcp6x_spibaraddr && !want_spi) { - msg_pdbg("Strange. MCP SPI BAR is valid, but chipset apparently" - " doesn't have SPI enabled.\n"); + msg_pdbg("Strange. MCP SPI BAR is valid, but chipset apparently doesn't have SPI enabled.\n"); /* FIXME: Should we enable SPI anyway? */ return 0; } /* Map the BAR. Bytewise/wordwise access at 0x530 and 0x540. */ - mcp6x_spibar = physmap("NVIDIA MCP6x SPI", mcp6x_spibaraddr, 0x544); - -#if 0 - /* FIXME: Run the physunmap in a shutdown function. */ - physunmap(mcp6x_spibar, 0x544); -#endif + mcp6x_spibar = rphysmap("NVIDIA MCP6x SPI", mcp6x_spibaraddr, 0x544); + if (mcp6x_spibar == ERROR_PTR) + return 1;
status = mmio_readw(mcp6x_spibar + 0x530); msg_pdbg("SPI control is 0x%04x, req=%i, gnt=%i\n", diff --git a/nicintel.c b/nicintel.c index 56678e7..305657c 100644 --- a/nicintel.c +++ b/nicintel.c @@ -59,13 +59,6 @@ static const struct par_programmer par_programmer_nicintel = { .chip_writen = fallback_chip_writen, };
-static int nicintel_shutdown(void *data) -{ - physunmap(nicintel_control_bar, NICINTEL_CONTROL_MEMMAP_SIZE); - physunmap(nicintel_bar, NICINTEL_MEMMAP_SIZE); - return 0; -} - int nicintel_init(void) { struct pci_dev *dev = NULL; @@ -83,18 +76,14 @@ int nicintel_init(void) return 1;
addr = pcidev_readbar(dev, PCI_BASE_ADDRESS_2); - nicintel_bar = physmap("Intel NIC flash", addr, NICINTEL_MEMMAP_SIZE); + nicintel_bar = rphysmap("Intel NIC flash", addr, NICINTEL_MEMMAP_SIZE); if (nicintel_bar == ERROR_PTR) - goto error_out_unmap; + return 1;
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); + nicintel_control_bar = rphysmap("Intel NIC control/status reg", addr, NICINTEL_CONTROL_MEMMAP_SIZE); if (nicintel_control_bar == ERROR_PTR) - goto error_out; - - if (register_shutdown(nicintel_shutdown, NULL)) return 1;
/* FIXME: This register is pretty undocumented in all publicly available @@ -112,11 +101,6 @@ int nicintel_init(void) register_par_programmer(&par_programmer_nicintel, BUS_PARALLEL);
return 0; - -error_out_unmap: - physunmap(nicintel_bar, NICINTEL_MEMMAP_SIZE); -error_out: - return 1; }
static void nicintel_chip_writeb(const struct flashctx *flash, uint8_t val, diff --git a/nicintel_spi.c b/nicintel_spi.c index 0045c09..b1bce6a 100644 --- a/nicintel_spi.c +++ b/nicintel_spi.c @@ -151,16 +151,12 @@ static int nicintel_spi_shutdown(void *data) { uint32_t tmp;
- /* Disable writes manually. See the comment about EECD in - * nicintel_spi_init() for details. - */ + /* Disable writes manually. See the comment about EECD in nicintel_spi_init() for details. */ tmp = pci_mmio_readl(nicintel_spibar + EECD); tmp &= ~FLASH_WRITES_ENABLED; tmp |= FLASH_WRITES_DISABLED; pci_mmio_writel(tmp, nicintel_spibar + EECD);
- physunmap(nicintel_spibar, MEMMAP_SIZE); - return 0; }
@@ -177,8 +173,7 @@ int nicintel_spi_init(void) 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); + nicintel_spibar = rphysmap("Intel Gigabit NIC w/ SPI flash", io_base_addr, MEMMAP_SIZE); /* Automatic restore of EECD on shutdown is not possible because EECD * does not only contain FLASH_WRITES_DISABLED|FLASH_WRITES_ENABLED, * but other bits with side effects as well. Those other bits must be diff --git a/ogp_spi.c b/ogp_spi.c index 0c09d6a..3aed6e8 100644 --- a/ogp_spi.c +++ b/ogp_spi.c @@ -97,12 +97,6 @@ static const struct bitbang_spi_master bitbang_spi_master_ogp = { .half_period = 0, };
-static int ogp_spi_shutdown(void *data) -{ - physunmap(ogp_spibar, 4096); - return 0; -} - int ogp_spi_init(void) { struct pci_dev *dev = NULL; @@ -137,9 +131,8 @@ int ogp_spi_init(void) 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)) + ogp_spibar = rphysmap("OGP registers", io_base_addr, 4096); + if (ogp_spibar == ERROR_PTR) return 1;
if (bitbang_spi_init(&bitbang_spi_master_ogp)) diff --git a/physmap.c b/physmap.c index 5aa9874..40dfa50 100644 --- a/physmap.c +++ b/physmap.c @@ -21,6 +21,7 @@ */
#include <unistd.h> +#include <stdbool.h> #include <stdio.h> #include <stdlib.h> #include <string.h> @@ -214,13 +215,32 @@ void physunmap(void *virt_addr, size_t len) } #endif
-#define PHYSMAP_NOFAIL 0 -#define PHYSMAP_MAYFAIL 1 -#define PHYSMAP_RW 0 -#define PHYSMAP_RO 1 +#define PHYSMAP_NOFAIL 0 +#define PHYSMAP_MAYFAIL 1 +#define PHYSMAP_RW 0 +#define PHYSMAP_RO 1 +#define PHYSMAP_NOCLEANUP 0 +#define PHYSMAP_CLEANUP 1
-static void *physmap_common(const char *descr, unsigned long phys_addr, - size_t len, int mayfail, int readonly) +struct undo_physmap_data { + void *virt_addr; + size_t len; +}; + +static int undo_physmap(void *data) +{ + if (data == NULL) { + msg_perr("%s: tried to physunmap without valid data!\n", __func__); + return 1; + } + struct undo_physmap_data *d = data; + physunmap(d->virt_addr, d->len); + free(data); + return 0; +} + +static void *physmap_common(const char *descr, unsigned long phys_addr, size_t len, bool mayfail, + bool readonly, bool autocleanup) { void *virt_addr;
@@ -268,19 +288,42 @@ static void *physmap_common(const char *descr, unsigned long phys_addr, exit(3); }
+ if (autocleanup) { + struct undo_physmap_data *d = malloc(sizeof(struct undo_physmap_data)); + if (d == NULL) { + msg_perr("%s: Out of memory!\n", __func__); + goto unmap_out; + } + + d->virt_addr = virt_addr; + d->len = len; + if (register_shutdown(undo_physmap, d) != 0) { + msg_perr("%s: Could not register shutdown function!\n", __func__); + goto unmap_out; + } + } + return virt_addr; +unmap_out: + physunmap(virt_addr, len); + if (!mayfail) + exit(3); + return ERROR_PTR; }
void *physmap(const char *descr, unsigned long phys_addr, size_t len) { - return physmap_common(descr, phys_addr, len, PHYSMAP_NOFAIL, - PHYSMAP_RW); + return physmap_common(descr, phys_addr, len, PHYSMAP_NOFAIL, PHYSMAP_RW, PHYSMAP_NOCLEANUP); +} + +void *rphysmap(const char *descr, unsigned long phys_addr, size_t len) +{ + return physmap_common(descr, phys_addr, len, PHYSMAP_NOFAIL, PHYSMAP_RW, PHYSMAP_CLEANUP); }
void *physmap_try_ro(const char *descr, unsigned long phys_addr, size_t len) { - return physmap_common(descr, phys_addr, len, PHYSMAP_MAYFAIL, - PHYSMAP_RO); + return physmap_common(descr, phys_addr, len, PHYSMAP_MAYFAIL, PHYSMAP_RO, PHYSMAP_NOCLEANUP); }
#if defined(__i386__) || defined(__x86_64__) diff --git a/programmer.h b/programmer.h index 51a8c80..9d28686 100644 --- a/programmer.h +++ b/programmer.h @@ -272,6 +272,7 @@ int processor_flash_enable(void);
/* physmap.c */ void *physmap(const char *descr, unsigned long phys_addr, size_t len); +void *rphysmap(const char *descr, unsigned long phys_addr, size_t len); void *physmap_try_ro(const char *descr, unsigned long phys_addr, size_t len); void physunmap(void *virt_addr, size_t len); #if CONFIG_INTERNAL == 1 diff --git a/satamv.c b/satamv.c index c3f27e7..b44e1cf 100644 --- a/satamv.c +++ b/satamv.c @@ -57,12 +57,6 @@ static const struct par_programmer par_programmer_satamv = { .chip_writen = fallback_chip_writen, };
-static int satamv_shutdown(void *data) -{ - physunmap(mv_bar, 0x20000); - return 0; -} - /* * Random notes: * FCE# Flash Chip Enable @@ -94,13 +88,10 @@ int satamv_init(void) return 1;
addr = pcidev_readbar(dev, PCI_BASE_ADDRESS_0); - mv_bar = physmap("Marvell 88SX7042 registers", addr, 0x20000); + mv_bar = rphysmap("Marvell 88SX7042 registers", addr, 0x20000); if (mv_bar == ERROR_PTR) return 1;
- if (register_shutdown(satamv_shutdown, NULL)) - return 1; - tmp = pci_mmio_readl(mv_bar + FLASH_PARAM); msg_pspew("Flash Parameters:\n"); msg_pspew("TurnOff=0x%01x\n", (tmp >> 0) & 0x7); diff --git a/satasii.c b/satasii.c index 72e35e5..37266eb 100644 --- a/satasii.c +++ b/satasii.c @@ -54,12 +54,6 @@ static const struct par_programmer par_programmer_satasii = { .chip_writen = fallback_chip_writen, };
-static int satasii_shutdown(void *data) -{ - physunmap(sii_bar, SATASII_MEMMAP_SIZE); - return 0; -} - static uint32_t satasii_wait_done(void) { uint32_t ctrl_reg; @@ -97,15 +91,15 @@ int satasii_init(void) reg_offset = 0x50; }
- sii_bar = physmap("SATA SiI registers", addr, SATASII_MEMMAP_SIZE) + reg_offset; + sii_bar = rphysmap("SATA SiI registers", addr, SATASII_MEMMAP_SIZE); + if (sii_bar == ERROR_PTR) + return 1; + sii_bar += reg_offset;
/* Check if ROM cycle are OK. */ if ((id != 0x0680) && (!(pci_mmio_readl(sii_bar) & (1 << 26)))) msg_pwarn("Warning: Flash seems unconnected.\n");
- if (register_shutdown(satasii_shutdown, NULL)) - return 1; - register_par_programmer(&par_programmer_satasii, BUS_PARALLEL);
return 0; diff --git a/sb600spi.c b/sb600spi.c index fe60aa9..9b48f2e 100644 --- a/sb600spi.c +++ b/sb600spi.c @@ -224,8 +224,10 @@ int sb600_probe_spi(struct pci_dev *dev) return 0;
/* Physical memory has to be mapped at page (4k) boundaries. */ - sb600_spibar = physmap("SB600 SPI registers", tmp & 0xfffff000, - 0x1000); + sb600_spibar = rphysmap("SB600 SPI registers", tmp & 0xfffff000, 0x1000); + if (sb600_spibar == ERROR_PTR) + return 1; + /* The low bits of the SPI base address are used as offset into * the mapped page. */
-
Carl-Daniel Hailfinger -
Stefan Tauner