[flashrom] [PATCH] Split out erase region walking
Split erase region walking out of erase_flash.
That allows us to use erase region walking for a combined erase/write
action, and is a prerequisite for partial flashing.
Signed-off-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2...@gmx.net
Index: flashrom-walk_eraseregions/flashrom.c
===
--- flashrom-walk_eraseregions/flashrom.c (Revision 1072)
+++ flashrom-walk_eraseregions/flashrom.c (Arbeitskopie)
@@ -1164,14 +1164,36 @@
return ret;
}
-int erase_flash(struct flashchip *flash)
+static int walk_eraseregions(struct flashchip *flash, int erasefunction, int
(*do_something) (struct flashchip *flash, unsigned int addr, unsigned int len))
{
- int i, j, k, ret = 0, found = 0;
+ int i, j;
+ unsigned int done = 0;
unsigned int start, len;
+ struct block_eraser eraser = flash-block_erasers[erasefunction];
+ for (i = 0; i NUM_ERASEREGIONS; i++) {
+ /* count==0 for all automatically initialized array
+* members so the loop below won't be executed for them.
+*/
+ for (j = 0; j eraser.eraseblocks[i].count; j++) {
+ start = done + eraser.eraseblocks[i].size * j;
+ len = eraser.eraseblocks[i].size;
+ msg_cdbg(0x%06x-0x%06x, , start,
+start + len - 1);
+ if (do_something(flash, start, len))
+ return 1;
+ }
+ done += eraser.eraseblocks[i].count *
+ eraser.eraseblocks[i].size;
+ }
+ return 0;
+}
+int erase_flash(struct flashchip *flash)
+{
+ int k, ret = 0, found = 0;
+
msg_cinfo(Erasing flash chip... );
for (k = 0; k NUM_ERASEFUNCTIONS; k++) {
- unsigned int done = 0;
struct block_eraser eraser = flash-block_erasers[k];
msg_cdbg(Looking at blockwise erase function %i... , k);
@@ -1194,24 +1216,7 @@
}
found = 1;
msg_cdbg(trying... );
- for (i = 0; i NUM_ERASEREGIONS; i++) {
- /* count==0 for all automatically initialized array
-* members so the loop below won't be executed for them.
-*/
- for (j = 0; j eraser.eraseblocks[i].count; j++) {
- start = done + eraser.eraseblocks[i].size * j;
- len = eraser.eraseblocks[i].size;
- msg_cdbg(0x%06x-0x%06x, , start,
-start + len - 1);
- ret = eraser.block_erase(flash, start, len);
- if (ret)
- break;
- }
- if (ret)
- break;
- done += eraser.eraseblocks[i].count *
- eraser.eraseblocks[i].size;
- }
+ ret = walk_eraseregions(flash, k, eraser.block_erase);
msg_cdbg(\n);
/* If everything is OK, don't try another erase function. */
if (!ret)
--
http://www.hailfinger.org/
___
flashrom mailing list
flashrom@flashrom.org
http://www.flashrom.org/mailman/listinfo/flashrom
Re: [flashrom] Support for HP Compaq 6820s laptop
Hello Andrew, In my case it contains what seems to be a windows dll (begins with MZ and contains This program cannot be run in DOS mode etc...), doesn't look like a bootblock. I found out that it could somehow be linked with slic 2.1, a feature used to transparently validate Windows on OEMs. Sorry no jumper in sight on my laptop. But HP flash upgrades (and doesn't downgrade) that zone indeed. Anyway not reflashing that zone doesn't seem do make any problem when flashing another bios version. Thanks for your interest. Best regards, Julius -- Ursprüngliche Nachricht: Betreff: Re: [flashrom] Support for HP Compaq 6820s laptop Datum: Samstag, 10. Juli 2010, 10:41:58 Von: Andrew Goodbody aj...@elfringham.co.uk An: apricot...@gmail.com, flashrom@flashrom.org On 10/07/2010 01:18, apricot...@gmail.com wrote: Hello, Just messed up a little bit with the code. Seems that the whole region f- f is write-protected. No idea what is actually stored in it and why it is write-protected apart it being rewritten by the HP flash utility when upgrading (but not when downgrading). So didn't find out anything, sad :| Best regards, Julius The top 64KB (or 16KB on older boards) is often used to contain the 'bootblock' by an OEM BIOS. This contains just enough code to verify the rest of the BIOS image and if a problem is detected to launch a means to flash in a new image. It is very often protected with a GPIO or jumper to control a write protect line to the flash device. When updating an OEM BIOS containing a bootblock it would not normally update the bootblock ever as that would risk making the recovery mechanism unusable so I am surprised when you say that the HP flash utlility updates it on an upgrade. Andrew ___ flashrom mailing list flashrom@flashrom.org http://www.flashrom.org/mailman/listinfo/flashrom
[flashrom] test of Winbond W25Q80BV
Hi, I've tested a Winbond W25Q80BV (W25Q80BVDAIG/25Q80BVAIG) socketed on a Asus M4A87TD/USB3. Read, erase, write and verify worked fine. Attached is flashrom -V output. Jonathan Kollasch flashrom v0.9.2-r1072 on NetBSD 5.1_RC3 (i386), built with libpci 3.1.3, GCC 4.1.3 20080704 prerelease (NetBSD nb2 20081120), little endian flashrom is free software, get the source code at http://www.flashrom.org Calibrating delay loop... OS timer resolution is 1 usecs, 1505M loops per second, 10 myus = 10 us, 100 myus = 100 us, 1000 myus = 1001 us, 1 myus = 10003 us, 4 myus = 5 us, OK. Initializing internal programmer No coreboot table found. dmidecode execution unsucessfull - continuing without DMI info Found ITE Super I/O, id 8721 Found chipset AMD SB700/SB710/SB750, enabling flash write... chipset PCI ID is 1002:439d, SPI base address is at 0xfec1 AltSpiCSEnable=0, SpiRomEnable=1, AbortEnable=0 PrefetchEnSPIFromIMC=0, PrefetchEnSPIFromHost=1, SpiOpEnInLpcMode=1 SpiArbEnable=1, SpiAccessMacRomEn=1, SpiHostAccessRomEn=1, ArbWaitCount=7, SpiBridgeDisable=1, DropOneClkOnRd=0 GPIO11 used for SPI_DO GPIO12 used for SPI_DI GPIO31 used for SPI_HOLD GPIO32 used for SPI_CS GPIO47 used for SPI_CLK ROM strap override is not active OK. This chipset supports the following protocols: LPC,FWH,SPI. SuperI/O ID 8721 is not on the controller list. Probing for AMD Am29F010A/B, 128 KB: skipped. Probing for AMD Am29F002(N)BB, 256 KB: skipped. Probing for AMD Am29F002(N)BT, 256 KB: skipped. Probing for AMD Am29F016D, 2048 KB: skipped. Probing for AMD Am29F040B, 512 KB: skipped. Probing for AMD Am29F080B, 1024 KB: skipped. Probing for AMD Am29LV040B, 512 KB: skipped. Probing for AMD Am29LV081B, 1024 KB: skipped. Probing for ASD AE49F2008, 256 KB: skipped. Probing for Atmel AT25DF021, 256 KB: probe_spi_rdid_generic: id1 0xef, id2 0x4014 Probing for Atmel AT25DF041A, 512 KB: probe_spi_rdid_generic: id1 0xef, id2 0x4014 Probing for Atmel AT25DF081, 1024 KB: probe_spi_rdid_generic: id1 0xef, id2 0x4014 Probing for Atmel AT25DF161, 2048 KB: probe_spi_rdid_generic: id1 0xef, id2 0x4014 Probing for Atmel AT25DF321, 4096 KB: probe_spi_rdid_generic: id1 0xef, id2 0x4014 Probing for Atmel AT25DF321A, 4096 KB: probe_spi_rdid_generic: id1 0xef, id2 0x4014 Probing for Atmel AT25DF641, 8192 KB: probe_spi_rdid_generic: id1 0xef, id2 0x4014 Probing for Atmel AT25F512B, 64 KB: probe_spi_rdid_generic: id1 0xef, id2 0x4014 Probing for Atmel AT25FS010, 128 KB: probe_spi_rdid_generic: id1 0xef, id2 0x4014 Probing for Atmel AT25FS040, 512 KB: probe_spi_rdid_generic: id1 0xef, id2 0x4014 Probing for Atmel AT26DF041, 512 KB: probe_spi_rdid_generic: id1 0xef, id2 0x4014 Probing for Atmel AT26DF081A, 1024 KB: probe_spi_rdid_generic: id1 0xef, id2 0x4014 Probing for Atmel AT26DF161, 2048 KB: probe_spi_rdid_generic: id1 0xef, id2 0x4014 Probing for Atmel AT26DF161A, 2048 KB: probe_spi_rdid_generic: id1 0xef, id2 0x4014 Probing for Atmel AT26F004, 512 KB: probe_spi_rdid_generic: id1 0xef, id2 0x4014 Probing for Atmel AT29C512, 64 KB: skipped. Probing for Atmel AT29C010A, 128 KB: skipped. Probing for Atmel AT29C020, 256 KB: skipped. Probing for Atmel AT29C040A, 512 KB: skipped. Probing for Atmel AT45CS1282, 16896 KB: probe_spi_rdid_generic: id1 0xef, id2 0x4014 Probing for Atmel AT45DB011D, 128 KB: probe_spi_rdid_generic: id1 0xef, id2 0x4014 Probing for Atmel AT45DB021D, 256 KB: probe_spi_rdid_generic: id1 0xef, id2 0x4014 Probing for Atmel AT45DB041D, 512 KB: probe_spi_rdid_generic: id1 0xef, id2 0x4014 Probing for Atmel AT45DB081D, 1024 KB: probe_spi_rdid_generic: id1 0xef, id2 0x4014 Probing for Atmel AT45DB161D, 2048 KB: probe_spi_rdid_generic: id1 0xef, id2 0x4014 Probing for Atmel AT45DB321C, 4224 KB: probe_spi_rdid_generic: id1 0xef, id2 0x4014 Probing for Atmel AT45DB321D, 4096 KB: probe_spi_rdid_generic: id1 0xef, id2 0x4014 Probing for Atmel AT45DB642D, 8192 KB: probe_spi_rdid_generic: id1 0xef, id2 0x4014 Probing for Atmel AT49BV512, 64 KB: skipped. Probing for Atmel AT49F020, 256 KB: skipped. Probing for Atmel AT49F002(N), 256 KB: skipped. Probing for Atmel AT49F002(N)T, 256 KB: skipped. Probing for AMIC A25L40PT, 512 KB: probe_spi_rdid_generic: id1 0xef, id2 0x4014 Probing for AMIC A25L40PU, 512 KB: probe_spi_rdid_generic: id1 0xef, id2 0x4014 Probing for AMIC A29002B, 256 KB: skipped. Probing for AMIC A29002T, 256 KB: skipped. Probing for AMIC A29040B, 512 KB: skipped. Probing for AMIC A49LF040A, 512 KB: probe_jedec_common: id1 0x65, id2 0x10, id1 parity violation, id1 is normal flash content, id2 is normal flash content Probing for EMST F49B002UA, 256 KB: skipped. Probing for EMST F25L008A, 1024 KB: probe_spi_rdid_generic: id1 0xef, id2 0x4014 Probing for Eon EN25B05, 64 KB: probe_spi_rdid_generic: id1 0xef, id2 0x4014 Probing for Eon EN25B05T, 64 KB: probe_spi_rdid_generic: id1 0xef, id2 0x4014 Probing for Eon EN25B10, 128 KB: probe_spi_rdid_generic: id1 0xef, id2 0x4014 Probing for Eon EN25B10T, 128 KB:
[flashrom] [commit] r1073 - trunk
Author: hailfinger
Date: Sat Jul 10 18:56:32 2010
New Revision: 1073
URL: http://flashrom.org/trac/coreboot/changeset/1073
Log:
Autodetect the ITE IT8705 Super I/O and enable flash writes if it
performs LPC-Parallel translation.
Remove board enables which triggered the IT8705 write enable manually.
Change the IT87 SPI special case to cover IT87 LPC-SPI and
LPC-Parallel translation.
Signed-off-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2...@gmx.net
Tested on Syntax SV266A.
Acked-by: Paul Menzel paulepan...@users.sourceforge.net
Tested on Shuttle AK38N, all operations work fine.
Acked-by: Uwe Hermann u...@hermann-uwe.de
Modified:
trunk/board_enable.c
trunk/flash.h
trunk/internal.c
trunk/it87spi.c
Modified: trunk/board_enable.c
==
--- trunk/board_enable.cThu Jul 8 12:13:37 2010(r1072)
+++ trunk/board_enable.cSat Jul 10 18:56:32 2010(r1073)
@@ -390,31 +390,92 @@
}
/**
- *
+ * Suited for all boards with ITE IT8705F.
+ * The SIS950 Super I/O probably requires a similar flash write enable.
*/
-static int it8705f_write_enable(uint8_t port)
+int it8705f_write_enable(uint8_t port)
{
+ uint8_t tmp;
+ int ret = 0;
+
enter_conf_mode_ite(port);
- sio_mask(port, 0x24, 0x04, 0x04); /* Flash ROM I/F Writes Enable */
+ tmp = sio_read(port, 0x24);
+ /* Check if at least one flash segment is enabled. */
+ if (tmp 0xf0) {
+ /* The IT8705F will respond to LPC cycles and translate them. */
+ buses_supported = CHIP_BUSTYPE_PARALLEL;
+ /* Flash ROM I/F Writes Enable */
+ tmp |= 0x04;
+ msg_pdbg(Enabling IT8705F flash ROM interface write.\n);
+ if (tmp 0x02) {
+ /* The data sheet contradicts itself about max size. */
+ max_rom_decode.parallel = 1024 * 1024;
+ msg_pinfo(IT8705F with very unusual settings. Please
+ send the output of \flashrom -V\ to \n
+ flashrom@flashrom.org to help us finish
+ support for your Super I/O. Thanks.\n);
+ ret = 1;
+ } else if (tmp 0x08) {
+ max_rom_decode.parallel = 512 * 1024;
+ } else {
+ max_rom_decode.parallel = 256 * 1024;
+ }
+ /* Safety checks. The data sheet is unclear here: Segments 1+3
+* overlap, no segment seems to cover top - 1MB to top - 512kB.
+* We assume that certain combinations make no sense.
+*/
+ if (((tmp 0x02) !(tmp 0x08)) || /* 1 MB en, 512 kB dis */
+ (!(tmp 0x10)) || /* 128 kB dis */
+ (!(tmp 0x40))) { /* 256/512 kB dis */
+ msg_perr(Inconsistent IT8705F decode size!\n);
+ ret = 1;
+ }
+ if (sio_read(port, 0x25) != 0) {
+ msg_perr(IT8705F flash data pins disabled!\n);
+ ret = 1;
+ }
+ if (sio_read(port, 0x26) != 0) {
+ msg_perr(IT8705F flash address pins 0-7 disabled!\n);
+ ret = 1;
+ }
+ if (sio_read(port, 0x27) != 0) {
+ msg_perr(IT8705F flash address pins 8-15 disabled!\n);
+ ret = 1;
+ }
+ if ((sio_read(port, 0x29) 0x10) != 0) {
+ msg_perr(IT8705F flash write enable pin disabled!\n);
+ ret = 1;
+ }
+ if ((sio_read(port, 0x29) 0x08) != 0) {
+ msg_perr(IT8705F flash chip select pin disabled!\n);
+ ret = 1;
+ }
+ if ((sio_read(port, 0x29) 0x04) != 0) {
+ msg_perr(IT8705F flash read strobe pin disabled!\n);
+ ret = 1;
+ }
+ if ((sio_read(port, 0x29) 0x03) != 0) {
+ msg_perr(IT8705F flash address pins 16-17
disabled!\n);
+ /* Not really an error if you use flash chips smaller
+* than 256 kByte, but such a configuration is unlikely.
+*/
+ ret = 1;
+ }
+ msg_pdbg(Maximum IT8705F parallel flash decode size is %u.\n,
+ max_rom_decode.parallel);
+ if (ret) {
+ msg_pinfo(Not enabling IT8705F flash write.\n);
+ } else {
+ sio_write(port, 0x24, tmp);
+ }
+ } else {
+ msg_pdbg(No IT8705F flash segment enabled.\n);
+ /* Not sure if this is an error or not. */
+
Re: [flashrom] [PATCH] Add IT8705 autodetection, Shuttle AK38N info
On 10.07.2010 17:38, Uwe Hermann wrote: On Fri, Jul 09, 2010 at 06:39:40PM +0200, Carl-Daniel Hailfinger wrote: Autodetect the ITE IT8705 Super I/O and enable flash writes if it ^ Missing 'F' in the commit message. performs LPC-Parallel translation. Remove board enables which triggered the IT8705 write enable manually. ^ Ditto Sorry, I got that wrong during cut-n-paste. Change the IT87 SPI special case to cover IT87 LPC-SPI and LPC-Parallel translation. Signed-off-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2...@gmx.net Tested on Syntax SV266A. Acked-by: Paul Menzel paulepan...@users.sourceforge.net Tested on Shuttle AK38N, all operations work fine. Acked-by: Uwe Hermann u...@hermann-uwe.de Retested this version on AK38N, still works fine. Acked-by: Uwe Hermann u...@hermann-uwe.de Thanks, committed in r1073. Please feel free to commit. I will do a review against the datasheet when time permits, but no need to wait for that, the code has been tested and reviewed by multiple people now, I don't expect issues. I changed the code as you suggested. I'll attach flashrom and superiotool output on Shuttle AK38N for reference. Thanks. Regards, Carl-Daniel -- http://www.hailfinger.org/ ___ flashrom mailing list flashrom@flashrom.org http://www.flashrom.org/mailman/listinfo/flashrom
Re: [flashrom] [PATCH] Convert SPI chips to partial write
Am Freitag, den 09.07.2010, 19:34 +0200 schrieb Carl-Daniel Hailfinger:
-int spi_chip_write_1(struct flashchip *flash, uint8_t *buf)
+/* real chunksize is 1, logical chunksize is 1 */
+int spi_chip_write_1_new(struct flashchip *flash, uint8_t *buf, int start,
int len)
I think spi_chip_write_1_range makes a better name than
spi_chip_write_1_new. But if your plan is to remove the old
spi_chip_write_1 function and rename this function at the same time to
spi_chip_write_1, I don't really mind the temporary name.
-int it8716f_spi_chip_write_256(struct flashchip *flash, uint8_t *buf)
+int it8716f_spi_chip_write_256(struct flashchip *flash, uint8_t *buf, int
start, int len)
{
[...]
+#ifdef TODO
+#warning This function needs to be converted to partial write
+ if ((programmer == PROGRAMMER_IT87SPI) || (flash-total_size * 1024
512 * 1024)) {
+#endif
+ spi_chip_write_1_new(flash, buf, start, len);
+#ifdef TODO
+#warning This function needs to be converted to partial write
[...]
+#endif
So this function will fall back to single-byte writes every time now
until it is changed in a later patch to support ranges. Did I understand
the patch correctly? I think this is acceptable to keep the patch size
manageable.
===
--- flashrom-partial_write_spi_intermediate/flashchips.c (Revision 1072)
+++ flashrom-partial_write_spi_intermediate/flashchips.c (Arbeitskopie)
@@ -1392,7 +1392,7 @@
.block_erase = spi_block_erase_c7,
}
},
- .write = spi_aai_write,
+ .write = spi_chip_write_1,
.read = read_memmapped,
},
You do this change to accommodate for the changed signature of
spi_aai_write, I think. But you lose the information that this chip used
AAI writes this way. Would adding a comment like /* AAI capable */ be
sensible?
@@ -197,7 +197,8 @@
* Program chip using page (256 bytes) programming.
* Some SPI masters can't do this, they use single byte programming instead.
*/
-int spi_chip_write_256(struct flashchip *flash, uint8_t *buf)
+/* real chunksize is up to 256, logical chunksize is 256 */
+int spi_chip_write_256_new(struct flashchip *flash, uint8_t *buf, int start,
int len)
{
if (!spi_programmer[spi_controller].write_256) {
msg_perr(%s called, but SPI page write is unsupported on this
Oops! The comment says that single-byte programming is used on
non-256-capable masters, but the code does not handle the fallback to
single bytes. I also am unable to find any other place in flashrom that
checks for the block write capability before calling this function. Is
that a bug?
+/* Wrapper function until the generic code is converted to partial writes. */
+int spi_chip_write_256(struct flashchip *flash, uint8_t *buf)
+{
+ int ret;
+
+ spi_disable_blockprotect();
+ msg_pinfo(Erasing flash before programming... );
+ if (erase_flash(flash)) {
+ msg_perr(ERASE FAILED!\n);
+ return -1;
+ }
+ msg_pinfo(done.\n);
+ msg_pinfo(Programming flash... );
+ ret = spi_chip_write_256_new(flash, buf, 0, flash-total_size * 1024);
+ if (!ret)
+ msg_pinfo(done.\n);
+ else
+ msg_pinfo(\n);
+ return ret;
+}
This reminds me of patch 1371. I'm all for getting that patch in before
this patch and not move around the erase-on-write logic just to kill it.
I will send a review soon.
-int wbsio_spi_write_1(struct flashchip *flash, uint8_t *buf)
+int wbsio_spi_write_1(struct flashchip *flash, uint8_t *buf, int start, int
len)
{
- int size = flash-total_size * 1024;
-
- if (size 1024 * 1024) {
+ if (flash-total_size * 1024 1024 * 1024) {
msg_perr(%s: Winbond saved on 4 register bits so max chip size
is 1024 KB!\n, __func__);
return 1;
}
- return spi_chip_write_1(flash, buf);
+ return spi_chip_write_1_new(flash, buf, start, len);
}
As soon as we have explicit erase, this check is too late in the write
function. Don't we even have the same problem even at read time?
Remaining stuff looks fine.
Regards,
Michael Karcher
___
flashrom mailing list
flashrom@flashrom.org
http://www.flashrom.org/mailman/listinfo/flashrom
Re: [flashrom] [PATCH] Convert SPI chips to partial write
Am Samstag, den 10.07.2010, 19:41 +0200 schrieb Carl-Daniel Hailfinger:
On 10.07.2010 19:21, Michael Karcher wrote:
I think spi_chip_write_1_range makes a better name than
spi_chip_write_1_new. But if your plan is to remove the old
spi_chip_write_1 function and rename this function at the same time to
spi_chip_write_1, I don't really mind the temporary name.
Yes, the plan is to kill the wrapper ASAP once the other chips are
converted as well.
OK, sounds sensible.
===
--- flashrom-partial_write_spi_intermediate/flashchips.c (Revision 1072)
+++ flashrom-partial_write_spi_intermediate/flashchips.c (Arbeitskopie)
@@ -1392,7 +1392,7 @@
.block_erase = spi_block_erase_c7,
}
},
- .write = spi_aai_write,
+ .write = spi_chip_write_1,
.read = read_memmapped,
},
You do this change to accommodate for the changed signature of
spi_aai_write, I think. But you lose the information that this chip used
AAI writes this way. Would adding a comment like /* AAI capable */ be
sensible?
AFAIK all write_1 chips can do AAI. Right now AAI is broken and I'm
waiting for an ack for patch 1548. Switching to write_1 instead of AAI
allows me to avoid creating a wrapper for AAI.
What is missing for 1548? Looks like just the formal ack, as the patch
seems to already be tested by den_m. In that case, I would do a review
of that patch.
-int spi_chip_write_256(struct flashchip *flash, uint8_t *buf)
+/* real chunksize is up to 256, logical chunksize is 256 */
+int spi_chip_write_256_new(struct flashchip *flash, uint8_t *buf, int
start, int len)
{
if (!spi_programmer[spi_controller].write_256) {
msg_perr(%s called, but SPI page write is unsupported on this
Oops! The comment says that single-byte programming is used on
non-256-capable masters, but the code does not handle the fallback to
single bytes. I also am unable to find any other place in flashrom that
checks for the block write capability before calling this function. Is
that a bug?
Mh. That's very non-obvious. The spi_programmer array has a .write_256
member for every controller. In case the controller can't do anything
except 1-byte writes, we set
.write_256 = spi_chip_write_1_new
Not every controller. SPI_CONTROLLER_DUMMY really has NULL in this
field. Your patch does not change that. And please kill the test if we
are sure that write_256 is always non-null.
This reminds me of patch 1371. I'm all for getting that patch in before
this patch and not move around the erase-on-write logic just to kill it.
I will send a review soon.
This patch is basically a part of patch 1371 with less bugs, more
consistency and working backwards compatibility to make merging and
reviewing easier.
I would consider it just the other way around. This patch is about
partial write, and includes parts of the semantically unrelated patch
about having erase outside of the write functions. Of course, one can
also add the write area infrastructure before removing the erase code
from the write code, but it seemed to make sense to me to first yank out
erase and ad-hoc partial write from the chip driver's write functions,
then convert them to a partial-write interface and finally put
everything below the generalized erase-block walking.
I don't like the introduction of two new wrappers either, but those are
a lot less of a problem than the existing per-controller wrappers and
removing them later will be a lot less invasive than 1371.
If your plan is to base a new 1371 on top of this patch, I'm perfectly
fine with these wrappers.
As soon as we have explicit erase, this check is too late in the write
function. Don't we even have the same problem even at read time?
Yes, this should be handled with max_rom_decode instead. I will send a
followup patch for max_rom_decode.spi in wbsio and kill the
wbsio-specific read+write.
Great!
Regards,
Michael Karcher
___
flashrom mailing list
flashrom@flashrom.org
http://www.flashrom.org/mailman/listinfo/flashrom
Re: [flashrom] [PATCH] Hook up SPI unlock to infrastructure
Am Samstag, den 10.07.2010, 03:05 +0200 schrieb Carl-Daniel Hailfinger:
-int spi_disable_blockprotect(void);
+int spi_disable_blockprotect(struct flashchip *flash);
This change makes sense, but you don't use the flash parameter yet.
@@ -1392,6 +1408,7 @@
.block_erase = spi_block_erase_c7,
}
},
+ .unlock = spi_disable_blockprotect,
.write = spi_chip_write_1,
.read = read_memmapped,
},
OUCH! blame me for committing that! read must not be read_memmapped.
Should I submit a fixup patch for that or do you want to fix that in one
of your patches?
-int spi_disable_blockprotect(void)
+int spi_disable_blockprotect(struct flashchip *flash)
{
uint8_t status;
int result;
@@ -855,6 +843,11 @@
msg_cerr(spi_write_status_register failed\n);
return result;
}
+ status = spi_read_status_register();
+ if ((status 0x3c) != 0) {
+ msg_cerr(Block protection could not be disabled!\n);
+ /* Should we error out here? */
Good question. As long as we have no partial write, we really should
error out here. But when we get partial writes, it would be great to
have flashrom being able to flash the lower part of the chip even if the
upper part is write protected. This would need major code changes,
though (unlocking would need to get the range to be unlocked), so for
now erroring out seems like the best option and as soon as not erroring
out might be sensible, we need to touch the code anyway.
Regards,
Michael Karcher
___
flashrom mailing list
flashrom@flashrom.org
http://www.flashrom.org/mailman/listinfo/flashrom
Re: [flashrom] [PATCH] Move implicit erase out of chip drivers, clean up
On 10.07.2010 20:16, Michael Karcher wrote:
Am Samstag, den 22.05.2010, 03:26 +0200 schrieb Carl-Daniel Hailfinger:
[as requested on IRC, this is not a full review, but two things not
directly related to the patch stand out I don't want to leave
uncommented]
int it8716f_spi_chip_write_256(struct flashchip *flash, uint8_t *buf)
{
-int total_size = 1024 * flash-total_size;
int i;
/*
* IT8716F only allows maximum of 512 kb SPI chip size for memory
* mapped access.
*/
-if ((programmer == PROGRAMMER_IT87SPI) || (total_size 512 * 1024)) {
+if ((programmer == PROGRAMMER_IT87SPI) || (flash-total_size * 1024
512 * 1024)) {
why do you have to test for the programmer type here? It seems like
it8716f* functions are only ever called if programmer is
PROGRAMMER_IT87SPI.
Historical baggage, and a corner case. PROGRAMMER_IT87SPI means someone
used -p it87_spi instead of -p internal. That's for machines which
used it87_spi before we had autodetection (and used board enabled
instead), and it also is for machines where the IT87* Super I/O does not
perform flash translation, but where someone hooked up a flash chip
anyway and wants to use IT87 in pure command mode and avoid memmapped mode.
To be honest, I think we can kill that special case. I don't know of
anyone who actually uses it nowadays.
+/* real chunksize is 1, logical chunksize is 64k */
int write_coreboot_m29f400bt(struct flashchip *flash, uint8_t *buf)
{
chipaddr bios = flash-virtual_memory;
The M29F400 stuff is completely broken. We use write_coreboot_m29f400bt
everywhere and write_m29f400bt is dead code. But
write_coreboot_m29f400bt does just write the lower half of the chip
(below the dashed line in the diagram).
I think I'll just kill that function.
Regards,
Carl-Daniel
--
http://www.hailfinger.org/
___
flashrom mailing list
flashrom@flashrom.org
http://www.flashrom.org/mailman/listinfo/flashrom
[flashrom] [commit] r1074 - trunk
Author: mkarcher
Date: Sat Jul 10 21:34:15 2010
New Revision: 1074
URL: http://flashrom.org/trac/coreboot/changeset/1074
Log:
Fix read function for EMST F25L008A
SPI chips never should use read_memmapped. The SPI master code might
decide that read_memmapped is fine for this chip, though, in a lower
layer.
Signed-off-by: Michael Karcher flash...@mkarcher.dialup.fu-berlin.de
Acked-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2...@gmx.net
Modified:
trunk/flashchips.c
Modified: trunk/flashchips.c
==
--- trunk/flashchips.c Sat Jul 10 18:56:32 2010(r1073)
+++ trunk/flashchips.c Sat Jul 10 21:34:15 2010(r1074)
@@ -1393,7 +1393,7 @@
}
},
.write = spi_aai_write,
- .read = read_memmapped,
+ .read = spi_chip_read,
},
{
___
flashrom mailing list
flashrom@flashrom.org
http://www.flashrom.org/mailman/listinfo/flashrom
Re: [flashrom] [PATCH] Hook up SPI unlock to infrastructure
Am Samstag, den 10.07.2010, 21:26 +0200 schrieb Carl-Daniel Hailfinger: + .unlock = spi_disable_blockprotect, .write = spi_chip_write_1, .read = read_memmapped, OUCH! blame me for committing that! read must not be read_memmapped. Should I submit a fixup patch for that or do you want to fix that in one of your patches? Can you fix it up? Such a change is Acked-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2...@gmx.net r1074. Regards, Michael Karcher ___ flashrom mailing list flashrom@flashrom.org http://www.flashrom.org/mailman/listinfo/flashrom
[flashrom] [PATCH] Use max_rom_decode for wbsio_spi
Use the max_rom_decode infrastructure for wbsio_spi instead of
open-coding a variant which only aborts after it is too late.
Signed-off-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2...@gmx.net
diff -ur flashrom-partial_write_spi_intermediate/flash.h
flashrom-wbsio_max_decode/flash.h
--- flashrom-partial_write_spi_intermediate/flash.h 2010-07-11
00:05:36.0 +0200
+++ flashrom-wbsio_max_decode/flash.h 2010-07-11 00:15:31.0 +0200
@@ -716,7 +716,6 @@
int wbsio_spi_send_command(unsigned int writecnt, unsigned int readcnt,
const unsigned char *writearr, unsigned char *readarr);
int wbsio_spi_read(struct flashchip *flash, uint8_t *buf, int start, int len);
-int wbsio_spi_write_1(struct flashchip *flash, uint8_t *buf, int start, int
len);
/* serprog.c */
int serprog_init(void);
diff -ur flashrom-partial_write_spi_intermediate/spi.c
flashrom-wbsio_max_decode/spi.c
--- flashrom-partial_write_spi_intermediate/spi.c 2010-07-11
00:09:26.0 +0200
+++ flashrom-wbsio_max_decode/spi.c 2010-07-11 00:16:22.0 +0200
@@ -80,7 +80,7 @@
.command = wbsio_spi_send_command,
.multicommand = default_spi_send_multicommand,
.read = wbsio_spi_read,
- .write_256 = wbsio_spi_write_1,
+ .write_256 = spi_chip_write_1_new,
},
#endif
#endif
diff -ur flashrom-partial_write_spi_intermediate/wbsio_spi.c
flashrom-wbsio_max_decode/wbsio_spi.c
--- flashrom-partial_write_spi_intermediate/wbsio_spi.c 2010-07-09
19:09:49.0 +0200
+++ flashrom-wbsio_max_decode/wbsio_spi.c 2010-07-11 00:18:38.0
+0200
@@ -69,6 +69,9 @@
buses_supported |= CHIP_BUSTYPE_SPI;
spi_controller = SPI_CONTROLLER_WBSIO;
+ msg_pdbg(%s: Winbond saved on 4 register bits so max chip size is
+1024 KB!\n, __func__);
+ max_rom_decode.spi = 1024 * 1024;
return 0;
}
@@ -179,24 +182,7 @@
int wbsio_spi_read(struct flashchip *flash, uint8_t *buf, int start, int len)
{
- int size = flash-total_size * 1024;
-
- if (size 1024 * 1024) {
- msg_perr(%s: Winbond saved on 4 register bits so max chip size
is 1024 KB!\n, __func__);
- return 1;
- }
-
return read_memmapped(flash, buf, start, len);
}
-int wbsio_spi_write_1(struct flashchip *flash, uint8_t *buf, int start, int
len)
-{
- if (flash-total_size * 1024 1024 * 1024) {
- msg_perr(%s: Winbond saved on 4 register bits so max chip size
is 1024 KB!\n, __func__);
- return 1;
- }
-
- return spi_chip_write_1_new(flash, buf, start, len);
-}
-
#endif
--
http://www.hailfinger.org/
___
flashrom mailing list
flashrom@flashrom.org
http://www.flashrom.org/mailman/listinfo/flashrom
Re: [flashrom] [PATCH] Convert SPI chips to partial write
On 10.07.2010 21:20, Carl-Daniel Hailfinger wrote:
On 10.07.2010 20:15, Michael Karcher wrote:
Am Samstag, den 10.07.2010, 19:41 +0200 schrieb Carl-Daniel Hailfinger:
The spi_programmer array has a .write_256
member for every controller. In case the controller can't do anything
except 1-byte writes, we set
.write_256 = spi_chip_write_1_new
Not every controller. SPI_CONTROLLER_DUMMY really has NULL in this
field. Your patch does not change that. And please kill the test if we
are sure that write_256 is always non-null.
Right. I wanted to add a write_256 function to the dummy programmer
anyway. I'll send an updated patch.
Side note: If no SPI controller is selected, write_256 is empty as well,
but I'd say that's not a problem because it should never happen.
If your plan is to base a new 1371 on top of this patch, I'm perfectly
fine with these wrappers.
Thanks. That's indeed the plan.
New version, should hopefully address all comments.
Convert SPI chips to partial write, but wrap the write functions in a
compat layer to allow converting the rest of flashrom later.
I actually have patches for most of the remaining conversion, but I
wanted to get this out and reviewed first.
Compile tested, but that's it. I would really appreciate it if someone
could test this on a machine with SPI flash.
Signed-off-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2...@gmx.net
Index: flashrom-partial_write_spi_intermediate/flash.h
===
--- flashrom-partial_write_spi_intermediate/flash.h (Revision 1074)
+++ flashrom-partial_write_spi_intermediate/flash.h (Arbeitskopie)
@@ -456,6 +456,7 @@
int dummy_spi_send_command(unsigned int writecnt, unsigned int readcnt,
const unsigned char *writearr, unsigned char *readarr);
int dummy_spi_read(struct flashchip *flash, uint8_t *buf, int start, int len);
+int dummy_spi_write_256(struct flashchip *flash, uint8_t *buf, int start, int
len);
#endif
/* nic3com.c */
@@ -529,7 +530,7 @@
int ft2232_spi_init(void);
int ft2232_spi_send_command(unsigned int writecnt, unsigned int readcnt, const
unsigned char *writearr, unsigned char *readarr);
int ft2232_spi_read(struct flashchip *flash, uint8_t *buf, int start, int len);
-int ft2232_spi_write_256(struct flashchip *flash, uint8_t *buf);
+int ft2232_spi_write_256(struct flashchip *flash, uint8_t *buf, int start, int
len);
/* bitbang_spi.c */
extern int bitbang_spi_half_period;
@@ -537,7 +538,7 @@
int bitbang_spi_init(void);
int bitbang_spi_send_command(unsigned int writecnt, unsigned int readcnt,
const unsigned char *writearr, unsigned char *readarr);
int bitbang_spi_read(struct flashchip *flash, uint8_t *buf, int start, int
len);
-int bitbang_spi_write_256(struct flashchip *flash, uint8_t *buf);
+int bitbang_spi_write_256(struct flashchip *flash, uint8_t *buf, int start,
int len);
/* buspirate_spi.c */
struct buspirate_spispeeds {
@@ -548,7 +549,7 @@
int buspirate_spi_shutdown(void);
int buspirate_spi_send_command(unsigned int writecnt, unsigned int readcnt,
const unsigned char *writearr, unsigned char *readarr);
int buspirate_spi_read(struct flashchip *flash, uint8_t *buf, int start, int
len);
-int buspirate_spi_write_256(struct flashchip *flash, uint8_t *buf);
+int buspirate_spi_write_256(struct flashchip *flash, uint8_t *buf, int start,
int len);
/* dediprog.c */
int dediprog_init(void);
@@ -668,7 +669,7 @@
/* Optimized functions for this programmer */
int (*read)(struct flashchip *flash, uint8_t *buf, int start, int len);
- int (*write_256)(struct flashchip *flash, uint8_t *buf);
+ int (*write_256)(struct flashchip *flash, uint8_t *buf, int start, int
len);
};
extern enum spi_controller spi_controller;
@@ -689,7 +690,7 @@
int ich_spi_send_command(unsigned int writecnt, unsigned int readcnt,
const unsigned char *writearr, unsigned char *readarr);
int ich_spi_read(struct flashchip *flash, uint8_t *buf, int start, int len);
-int ich_spi_write_256(struct flashchip *flash, uint8_t * buf);
+int ich_spi_write_256(struct flashchip *flash, uint8_t * buf, int start, int
len);
int ich_spi_send_multicommand(struct spi_command *cmds);
/* it87spi.c */
@@ -701,13 +702,13 @@
int it8716f_spi_send_command(unsigned int writecnt, unsigned int readcnt,
const unsigned char *writearr, unsigned char *readarr);
int it8716f_spi_chip_read(struct flashchip *flash, uint8_t *buf, int start,
int len);
-int it8716f_spi_chip_write_256(struct flashchip *flash, uint8_t *buf);
+int it8716f_spi_chip_write_256(struct flashchip *flash, uint8_t *buf, int
start, int len);
/* sb600spi.c */
int sb600_spi_send_command(unsigned int writecnt, unsigned int readcnt,
const unsigned char *writearr, unsigned char *readarr);
int sb600_spi_read(struct flashchip *flash, uint8_t *buf, int
