Joey: This patch applies to current svn HEAD.
Patch is untested, but it should work and not spit out any new error messages.
RFC: Are those checks with abort a good idea or do they break stuff? exit(1) is not a nice thing for libflashrom, but then again, it's probably better than exploding. I'm also unsure about the parameter order of mmio_readn.
The IT87 SPI driver has one quirk to speed up reading and writing: If a flash chip is 512 kByte or less, the flash chip can be completely mapped in memory and both read and write accesses are faster that way.
The current IT87 SPI code did use the parallel programmer interface for memory mapped reads and writes, but that's the wrong abstraction. It has been fixed to use mmio_read*/mmio_write* for that purpose.
Introduce sanity checks for all SPI/Parallel-style accesses before a possibly undefined union member is dereferenced.
Signed-off-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net
Index: flashrom-fix_par_programmer_access_from_spi/hwaccess.c =================================================================== --- flashrom-fix_par_programmer_access_from_spi/hwaccess.c (Revision 1510) +++ flashrom-fix_par_programmer_access_from_spi/hwaccess.c (Arbeitskopie) @@ -169,6 +169,12 @@ return *(volatile uint32_t *) addr; }
+void mmio_readn(void *addr, uint8_t *buf, size_t len) +{ + memcpy(buf, addr, len); + return; +} + void mmio_le_writeb(uint8_t val, void *addr) { mmio_writeb(cpu_to_le8(val), addr); Index: flashrom-fix_par_programmer_access_from_spi/flash.h =================================================================== --- flashrom-fix_par_programmer_access_from_spi/flash.h (Revision 1510) +++ flashrom-fix_par_programmer_access_from_spi/flash.h (Arbeitskopie) @@ -247,6 +247,7 @@ void print_banner(void); void list_programmers_linebreak(int startcol, int cols, int paren); int selfcheck(void); +void shutdown_and_exit(int exitcode); int doit(struct flashctx *flash, int force, const char *filename, int read_it, int write_it, int erase_it, int verify_it); int read_buf_from_file(unsigned char *buf, unsigned long size, const char *filename); int write_buf_to_file(unsigned char *buf, unsigned long size, const char *filename); Index: flashrom-fix_par_programmer_access_from_spi/it87spi.c =================================================================== --- flashrom-fix_par_programmer_access_from_spi/it87spi.c (Revision 1510) +++ flashrom-fix_par_programmer_access_from_spi/it87spi.c (Arbeitskopie) @@ -330,7 +330,7 @@ OUTB(0x06, it8716f_flashport + 1); OUTB(((2 + (fast_spi ? 1 : 0)) << 4), it8716f_flashport); for (i = 0; i < flash->page_size; i++) - chip_writeb(flash, buf[i], bios + start + i); + mmio_writeb(buf[i], (void *)(bios + start + i)); OUTB(0, it8716f_flashport); /* Wait until the Write-In-Progress bit is cleared. * This usually takes 1-10 ms, so wait in 1 ms steps. @@ -356,7 +356,7 @@ if ((flash->total_size * 1024 > 512 * 1024)) { spi_read_chunked(flash, buf, start, len, 3); } else { - read_memmapped(flash, buf, start, len); + mmio_readn((void *)(flash->virtual_memory + start), buf, len); }
return 0; Index: flashrom-fix_par_programmer_access_from_spi/internal.c =================================================================== --- flashrom-fix_par_programmer_access_from_spi/internal.c (Revision 1510) +++ flashrom-fix_par_programmer_access_from_spi/internal.c (Arbeitskopie) @@ -387,6 +387,6 @@ static void internal_chip_readn(const struct flashctx *flash, uint8_t *buf, const chipaddr addr, size_t len) { - memcpy(buf, (void *)addr, len); + mmio_readn((void *)addr, buf, len); return; } Index: flashrom-fix_par_programmer_access_from_spi/spi.c =================================================================== --- flashrom-fix_par_programmer_access_from_spi/spi.c (Revision 1510) +++ flashrom-fix_par_programmer_access_from_spi/spi.c (Arbeitskopie) @@ -34,12 +34,22 @@ unsigned int readcnt, const unsigned char *writearr, unsigned char *readarr) { + if (!(flash->pgm->buses_supported & BUS_SPI)) { + msg_perr("%s called for wrong programmer type! Please report " + "a bug at flashrom@flashrom.org.\n", __func__); + shutdown_and_exit(1); + } return flash->pgm->spi.command(flash, writecnt, readcnt, writearr, readarr); }
int spi_send_multicommand(struct flashctx *flash, struct spi_command *cmds) { + if (!(flash->pgm->buses_supported & BUS_SPI)) { + msg_perr("%s called for wrong programmer type! Please report " + "a bug at flashrom@flashrom.org.\n", __func__); + shutdown_and_exit(1); + } return flash->pgm->spi.multicommand(flash, cmds); }
@@ -78,7 +88,13 @@ int default_spi_read(struct flashctx *flash, uint8_t *buf, unsigned int start, unsigned int len) { - unsigned int max_data = flash->pgm->spi.max_data_read; + unsigned int max_data; + if (!(flash->pgm->buses_supported & BUS_SPI)) { + msg_perr("%s called for wrong programmer type! Please report " + "a bug at flashrom@flashrom.org.\n", __func__); + shutdown_and_exit(1); + } + max_data = flash->pgm->spi.max_data_read; if (max_data == MAX_DATA_UNSPECIFIED) { msg_perr("%s called, but SPI read chunk size not defined " "on this hardware. Please report a bug at " @@ -91,7 +107,13 @@ int default_spi_write_256(struct flashctx *flash, uint8_t *buf, unsigned int start, unsigned int len) { - unsigned int max_data = flash->pgm->spi.max_data_write; + unsigned int max_data; + if (!(flash->pgm->buses_supported & BUS_SPI)) { + msg_perr("%s called for wrong programmer type! Please report " + "a bug at flashrom@flashrom.org.\n", __func__); + shutdown_and_exit(1); + } + max_data = flash->pgm->spi.max_data_write; if (max_data == MAX_DATA_UNSPECIFIED) { msg_perr("%s called, but SPI write chunk size not defined " "on this hardware. Please report a bug at " @@ -106,6 +128,11 @@ { unsigned int addrbase = 0;
+ if (!(flash->pgm->buses_supported & BUS_SPI)) { + msg_perr("%s called for wrong programmer type! Please report " + "a bug at flashrom@flashrom.org.\n", __func__); + shutdown_and_exit(1); + } /* Check if the chip fits between lowest valid and highest possible * address. Highest possible address with the current SPI implementation * means 0xffffff, the highest unsigned 24bit number. @@ -138,6 +165,11 @@ int spi_chip_write_256(struct flashctx *flash, uint8_t *buf, unsigned int start, unsigned int len) { + if (!(flash->pgm->buses_supported & BUS_SPI)) { + msg_perr("%s called for wrong programmer type! Please report " + "a bug at flashrom@flashrom.org.\n", __func__); + shutdown_and_exit(1); + } return flash->pgm->spi.write_256(flash, buf, start, len); }
@@ -148,6 +180,11 @@ */ uint32_t spi_get_valid_read_addr(struct flashctx *flash) { + if (!(flash->pgm->buses_supported & BUS_SPI)) { + msg_perr("%s called for wrong programmer type! Please report " + "a bug at flashrom@flashrom.org.\n", __func__); + shutdown_and_exit(1); + } switch (flash->pgm->spi.type) { #if CONFIG_INTERNAL == 1 #if defined(__i386__) || defined(__x86_64__) Index: flashrom-fix_par_programmer_access_from_spi/flashrom.c =================================================================== --- flashrom-fix_par_programmer_access_from_spi/flashrom.c (Revision 1510) +++ flashrom-fix_par_programmer_access_from_spi/flashrom.c (Arbeitskopie) @@ -357,43 +357,83 @@
void chip_writeb(const struct flashctx *flash, uint8_t val, chipaddr addr) { + if (!(flash->pgm->buses_supported & BUS_NONSPI)) { + msg_perr("%s called for wrong programmer type! Please report " + "a bug at flashrom@flashrom.org.\n", __func__); + shutdown_and_exit(1); + } flash->pgm->par.chip_writeb(flash, val, addr); }
void chip_writew(const struct flashctx *flash, uint16_t val, chipaddr addr) { + if (!(flash->pgm->buses_supported & BUS_NONSPI)) { + msg_perr("%s called for wrong programmer type! Please report " + "a bug at flashrom@flashrom.org.\n", __func__); + shutdown_and_exit(1); + } flash->pgm->par.chip_writew(flash, val, addr); }
void chip_writel(const struct flashctx *flash, uint32_t val, chipaddr addr) { + if (!(flash->pgm->buses_supported & BUS_NONSPI)) { + msg_perr("%s called for wrong programmer type! Please report " + "a bug at flashrom@flashrom.org.\n", __func__); + shutdown_and_exit(1); + } flash->pgm->par.chip_writel(flash, val, addr); }
void chip_writen(const struct flashctx *flash, uint8_t *buf, chipaddr addr, size_t len) { + if (!(flash->pgm->buses_supported & BUS_NONSPI)) { + msg_perr("%s called for wrong programmer type! Please report " + "a bug at flashrom@flashrom.org.\n", __func__); + shutdown_and_exit(1); + } flash->pgm->par.chip_writen(flash, buf, addr, len); }
uint8_t chip_readb(const struct flashctx *flash, const chipaddr addr) { + if (!(flash->pgm->buses_supported & BUS_NONSPI)) { + msg_perr("%s called for wrong programmer type! Please report " + "a bug at flashrom@flashrom.org.\n", __func__); + shutdown_and_exit(1); + } return flash->pgm->par.chip_readb(flash, addr); }
uint16_t chip_readw(const struct flashctx *flash, const chipaddr addr) { + if (!(flash->pgm->buses_supported & BUS_NONSPI)) { + msg_perr("%s called for wrong programmer type! Please report " + "a bug at flashrom@flashrom.org.\n", __func__); + shutdown_and_exit(1); + } return flash->pgm->par.chip_readw(flash, addr); }
uint32_t chip_readl(const struct flashctx *flash, const chipaddr addr) { + if (!(flash->pgm->buses_supported & BUS_NONSPI)) { + msg_perr("%s called for wrong programmer type! Please report " + "a bug at flashrom@flashrom.org.\n", __func__); + shutdown_and_exit(1); + } return flash->pgm->par.chip_readl(flash, addr); }
void chip_readn(const struct flashctx *flash, uint8_t *buf, chipaddr addr, size_t len) { + if (!(flash->pgm->buses_supported & BUS_NONSPI)) { + msg_perr("%s called for wrong programmer type! Please report " + "a bug at flashrom@flashrom.org.\n", __func__); + shutdown_and_exit(1); + } flash->pgm->par.chip_readn(flash, buf, addr, len); }
@@ -1700,6 +1740,15 @@ return 0; }
+/* FIXME: Totally unacceptable for libflashrom, but it's better than a simple + * exit(1) for most places which just exit(1) for now. + */ +void shutdown_and_exit(int exitcode) +{ + programmer_shutdown(); + exit(exitcode); +} + /* This function signature is horrible. We need to design a better interface, * but right now it allows us to split off the CLI code. * Besides that, the function itself is a textbook example of abysmal code flow. Index: flashrom-fix_par_programmer_access_from_spi/programmer.h =================================================================== --- flashrom-fix_par_programmer_access_from_spi/programmer.h (Revision 1510) +++ flashrom-fix_par_programmer_access_from_spi/programmer.h (Arbeitskopie) @@ -312,6 +312,7 @@ uint8_t mmio_readb(void *addr); uint16_t mmio_readw(void *addr); uint32_t mmio_readl(void *addr); +void mmio_readn(void *addr, uint8_t *buf, size_t len); void mmio_le_writeb(uint8_t val, void *addr); void mmio_le_writew(uint16_t val, void *addr); void mmio_le_writel(uint32_t val, void *addr);
On Thu, 01 Mar 2012 01:42:27 +0100 Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net wrote:
RFC: Are those checks with abort a good idea or do they break stuff? exit(1) is not a nice thing for libflashrom, but then again, it's probably better than exploding.
at least the spi functions are able to return an error, why exit instead?!
Am 01.03.2012 01:51 schrieb Stefan Tauner:
On Thu, 01 Mar 2012 01:42:27 +0100 Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net wrote:
RFC: Are those checks with abort a good idea or do they break stuff? exit(1) is not a nice thing for libflashrom, but then again, it's probably better than exploding.
at least the spi functions are able to return an error, why exit instead?!
Not all the way down the calling graph. spi_read_status_register() returns the status register itself (or garbage in case there was an error). Regards, Carl-Daniel
Hi Johan,
thanks for testing!
Am 01.03.2012 08:21 schrieb Johan Svensson:
Hello Carl-Daniel,
It works perfectly well with this patch. No segfault, no new error message and the output file is populated with data.
Thank you very much for awesome support!
Johan
This would qualify the patch as tested, but is the style in which it solves the problem (indirection via mmio_readn) the right way to do this? Comments from a reviewer perspective appreciated.
Regards, Carl-Daniel
Am Donnerstag, den 01.03.2012, 01:42 +0100 schrieb Carl-Daniel Hailfinger:
RFC: Are those checks with abort a good idea or do they break stuff? exit(1) is not a nice thing for libflashrom, but then again, it's probably better than exploding.
I don't really feel like checking that at runtime is a good idea, but until someone proposes a not too hacky plan how to check that at compile time, this definitely has some merits, so it won't be a reason to not ack for me.
I'm also unsure about the parameter order of mmio_readn.
Yeah, that one is tricky, as it does not match memcpy, which might be surprising. OTOH, the "read" in the function name implies that the order might be different, as you really "read" what is pointed to by the first argument. So this one is OK in some sense. Something up to discussion is the length parameter. Traditionally in C, the length is passed immediately after the pointer it applies to. Now that still does not help us, as the source and the target buffer have the same size for obvious reason, so attaching the size to either buffer is OK ("read a block of $len bytes at physical address $addr and store it in $buf" vs. "read data from physical address $addr and fill the buffer $buf of size $len with it). Finally, the length can also be seen as detached from either buffer: "Copy $len bytes, with the source being at physical address $addr and the destination being at $buf", which would also suggest putting size last to me. So for me
read(src, size, dest) read(src, dest, size)
both make sense in that regard. The variant you use now (the second) has some extra bonus in being semantically equivalent to the read system call. This semantic equivalence is not a silver bullet, though, as our write functions don't model the "file handle first" order of the system calls (with the physical address corresponding to the physical address.
So while I can't really push you into either direction, maybe you prefer one of the ways to decide on a order that much that you can decide yourself.
The current IT87 SPI code did use the parallel programmer interface for memory mapped reads and writes, but that's the wrong abstraction. It has been fixed to use mmio_read*/mmio_write* for that purpose.
I like that.
Introduce sanity checks for all SPI/Parallel-style accesses before a possibly undefined union member is dereferenced.
See above.
Signed-off-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net
Acked-by: Michael Karcher flashrom@mkarcher.dialup.fu-berlin.de
Regards, Michael Karcher
Hi,
this is the same patch, but without the runtime sanity checks.
Am 01.03.2012 20:15 schrieb Michael Karcher:
Am Donnerstag, den 01.03.2012, 01:42 +0100 schrieb Carl-Daniel Hailfinger:
RFC: Are those checks with abort a good idea or do they break stuff? exit(1) is not a nice thing for libflashrom, but then again, it's probably better than exploding.
I don't really feel like checking that at runtime is a good idea, but until someone proposes a not too hacky plan how to check that at compile time, this definitely has some merits, so it won't be a reason to not ack for me.
I'm also unsure about the parameter order of mmio_readn.
Yeah, that one is tricky, as it does not match memcpy, which might be surprising. OTOH, the "read" in the function name implies that the order might be different, as you really "read" what is pointed to by the first argument. So this one is OK in some sense. Something up to discussion is the length parameter. Traditionally in C, the length is passed immediately after the pointer it applies to. Now that still does not help us, as the source and the target buffer have the same size for obvious reason, so attaching the size to either buffer is OK ("read a block of $len bytes at physical address $addr and store it in $buf" vs. "read data from physical address $addr and fill the buffer $buf of size $len with it). Finally, the length can also be seen as detached from either buffer: "Copy $len bytes, with the source being at physical address $addr and the destination being at $buf", which would also suggest putting size last to me. So for me
read(src, size, dest) read(src, dest, size)
both make sense in that regard. The variant you use now (the second) has some extra bonus in being semantically equivalent to the read system call. This semantic equivalence is not a silver bullet, though, as our write functions don't model the "file handle first" order of the system calls (with the physical address corresponding to the physical address.
So while I can't really push you into either direction, maybe you prefer one of the ways to decide on a order that much that you can decide yourself.
OK then, it stays like it is in the first version of the patch.
The current IT87 SPI code did use the parallel programmer interface for memory mapped reads and writes, but that's the wrong abstraction. It has been fixed to use mmio_read*/mmio_write* for that purpose.
I like that.
Introduce sanity checks for all SPI/Parallel-style accesses before a possibly undefined union member is dereferenced.
See above.
Signed-off-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net
Acked-by: Michael Karcher flashrom@mkarcher.dialup.fu-berlin.de
Thanks for the review!
The IT87 SPI driver has one quirk to speed up reading and writing: If a flash chip is 512 kByte or less, the flash chip can be completely mapped in memory and both read and write accesses are faster that way.
The current IT87 SPI code did use the parallel programmer interface for memory mapped reads and writes, but that's the wrong abstraction. It has been fixed to use mmio_read*/mmio_write* for that purpose. Switch internal_chip_readn to use mmio_readn as proper abstraction.
Kudos to Michael Karcher for spotting the bug.
Reported-by: Johan Svensson flashrom.js@crypt.se Tested-by: Johan Svensson flashrom.js@crypt.se
Signed-off-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net Acked-by: Michael Karcher flashrom@mkarcher.dialup.fu-berlin.de
Index: flashrom-fix_par_programmer_access_from_spi/hwaccess.c =================================================================== --- flashrom-fix_par_programmer_access_from_spi/hwaccess.c (Revision 1510) +++ flashrom-fix_par_programmer_access_from_spi/hwaccess.c (Arbeitskopie) @@ -169,6 +169,12 @@ return *(volatile uint32_t *) addr; }
+void mmio_readn(void *addr, uint8_t *buf, size_t len) +{ + memcpy(buf, addr, len); + return; +} + void mmio_le_writeb(uint8_t val, void *addr) { mmio_writeb(cpu_to_le8(val), addr); Index: flashrom-fix_par_programmer_access_from_spi/it87spi.c =================================================================== --- flashrom-fix_par_programmer_access_from_spi/it87spi.c (Revision 1510) +++ flashrom-fix_par_programmer_access_from_spi/it87spi.c (Arbeitskopie) @@ -330,7 +330,7 @@ OUTB(0x06, it8716f_flashport + 1); OUTB(((2 + (fast_spi ? 1 : 0)) << 4), it8716f_flashport); for (i = 0; i < flash->page_size; i++) - chip_writeb(flash, buf[i], bios + start + i); + mmio_writeb(buf[i], (void *)(bios + start + i)); OUTB(0, it8716f_flashport); /* Wait until the Write-In-Progress bit is cleared. * This usually takes 1-10 ms, so wait in 1 ms steps. @@ -356,7 +356,7 @@ if ((flash->total_size * 1024 > 512 * 1024)) { spi_read_chunked(flash, buf, start, len, 3); } else { - read_memmapped(flash, buf, start, len); + mmio_readn((void *)(flash->virtual_memory + start), buf, len); }
return 0; Index: flashrom-fix_par_programmer_access_from_spi/internal.c =================================================================== --- flashrom-fix_par_programmer_access_from_spi/internal.c (Revision 1510) +++ flashrom-fix_par_programmer_access_from_spi/internal.c (Arbeitskopie) @@ -387,6 +387,6 @@ static void internal_chip_readn(const struct flashctx *flash, uint8_t *buf, const chipaddr addr, size_t len) { - memcpy(buf, (void *)addr, len); + mmio_readn((void *)addr, buf, len); return; } Index: flashrom-fix_par_programmer_access_from_spi/programmer.h =================================================================== --- flashrom-fix_par_programmer_access_from_spi/programmer.h (Revision 1510) +++ flashrom-fix_par_programmer_access_from_spi/programmer.h (Arbeitskopie) @@ -312,6 +312,7 @@ uint8_t mmio_readb(void *addr); uint16_t mmio_readw(void *addr); uint32_t mmio_readl(void *addr); +void mmio_readn(void *addr, uint8_t *buf, size_t len); void mmio_le_writeb(uint8_t val, void *addr); void mmio_le_writew(uint16_t val, void *addr); void mmio_le_writel(uint32_t val, void *addr);
Third time's a charm.
Michael Karcher pointed out that I had inexplicably overlooked wbsio.
Fix parallel-style programmer access from ITE IT87/Winbond W83627 SPI
The ITE IT87 SPI driver uses a trick to speed up reading and writing: If a flash chip is 512 kByte or less, the flash chip can be completely mapped in memory and both read and write accesses are faster that way. The current IT87 SPI code did use the parallel programmer interface for memory mapped reads and writes, but that's the wrong abstraction. It has been fixed to use mmio_read*/mmio_write* for that purpose.
The Winbond W83627 SPI driver uses the same trick in its read path for all supported chip sizes. Fix it the same way.
Switch internal_chip_readn to use mmio_readn as proper abstraction.
Kudos to Michael Karcher for spotting the bugs.
Reported-by: Johan Svensson flashrom.js@crypt.se Signed-off-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net Acked-by: Michael Karcher flashrom@mkarcher.dialup.fu-berlin.de Tested-by: Johan Svensson flashrom.js@crypt.se
Index: flashrom-fix_par_programmer_access_from_spi/hwaccess.c =================================================================== --- flashrom-fix_par_programmer_access_from_spi/hwaccess.c (Revision 1510) +++ flashrom-fix_par_programmer_access_from_spi/hwaccess.c (Arbeitskopie) @@ -169,6 +169,12 @@ return *(volatile uint32_t *) addr; }
+void mmio_readn(void *addr, uint8_t *buf, size_t len) +{ + memcpy(buf, addr, len); + return; +} + void mmio_le_writeb(uint8_t val, void *addr) { mmio_writeb(cpu_to_le8(val), addr); Index: flashrom-fix_par_programmer_access_from_spi/it87spi.c =================================================================== --- flashrom-fix_par_programmer_access_from_spi/it87spi.c (Revision 1510) +++ flashrom-fix_par_programmer_access_from_spi/it87spi.c (Arbeitskopie) @@ -330,7 +330,7 @@ OUTB(0x06, it8716f_flashport + 1); OUTB(((2 + (fast_spi ? 1 : 0)) << 4), it8716f_flashport); for (i = 0; i < flash->page_size; i++) - chip_writeb(flash, buf[i], bios + start + i); + mmio_writeb(buf[i], (void *)(bios + start + i)); OUTB(0, it8716f_flashport); /* Wait until the Write-In-Progress bit is cleared. * This usually takes 1-10 ms, so wait in 1 ms steps. @@ -356,7 +356,7 @@ if ((flash->total_size * 1024 > 512 * 1024)) { spi_read_chunked(flash, buf, start, len, 3); } else { - read_memmapped(flash, buf, start, len); + mmio_readn((void *)(flash->virtual_memory + start), buf, len); }
return 0; Index: flashrom-fix_par_programmer_access_from_spi/internal.c =================================================================== --- flashrom-fix_par_programmer_access_from_spi/internal.c (Revision 1510) +++ flashrom-fix_par_programmer_access_from_spi/internal.c (Arbeitskopie) @@ -387,6 +387,6 @@ static void internal_chip_readn(const struct flashctx *flash, uint8_t *buf, const chipaddr addr, size_t len) { - memcpy(buf, (void *)addr, len); + mmio_readn((void *)addr, buf, len); return; } Index: flashrom-fix_par_programmer_access_from_spi/wbsio_spi.c =================================================================== --- flashrom-fix_par_programmer_access_from_spi/wbsio_spi.c (Revision 1510) +++ flashrom-fix_par_programmer_access_from_spi/wbsio_spi.c (Arbeitskopie) @@ -202,7 +202,8 @@ static int wbsio_spi_read(struct flashctx *flash, uint8_t *buf, unsigned int start, unsigned int len) { - return read_memmapped(flash, buf, start, len); + mmio_readn((void *)(flash->virtual_memory + start), buf, len); + return 0; }
#endif Index: flashrom-fix_par_programmer_access_from_spi/programmer.h =================================================================== --- flashrom-fix_par_programmer_access_from_spi/programmer.h (Revision 1510) +++ flashrom-fix_par_programmer_access_from_spi/programmer.h (Arbeitskopie) @@ -312,6 +312,7 @@ uint8_t mmio_readb(void *addr); uint16_t mmio_readw(void *addr); uint32_t mmio_readl(void *addr); +void mmio_readn(void *addr, uint8_t *buf, size_t len); void mmio_le_writeb(uint8_t val, void *addr); void mmio_le_writew(uint16_t val, void *addr); void mmio_le_writel(uint32_t val, void *addr);
Am 01.03.2012 23:15 schrieb Carl-Daniel Hailfinger:
Fix parallel-style programmer access from ITE IT87/Winbond W83627 SPI
The ITE IT87 SPI driver uses a trick to speed up reading and writing: If a flash chip is 512 kByte or less, the flash chip can be completely mapped in memory and both read and write accesses are faster that way. The current IT87 SPI code did use the parallel programmer interface for memory mapped reads and writes, but that's the wrong abstraction. It has been fixed to use mmio_read*/mmio_write* for that purpose.
The Winbond W83627 SPI driver uses the same trick in its read path for all supported chip sizes. Fix it the same way.
Switch internal_chip_readn to use mmio_readn as proper abstraction.
Kudos to Michael Karcher for spotting the bugs.
Reported-by: Johan Svensson flashrom.js@crypt.se Signed-off-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net Acked-by: Michael Karcher flashrom@mkarcher.dialup.fu-berlin.de Tested-by: Johan Svensson flashrom.js@crypt.se
And committed in r1511.
Regards, Carl-Daniel
-
Carl-Daniel Hailfinger -
Michael Karcher -
Stefan Tauner