[flashrom] [PATCH] Refactor remaining write wrappers
Carl-Daniel Hailfinger
c-d.hailfinger.devel.2006 at gmx.net
Mon Oct 11 14:35:58 CEST 2010
On 10.10.2010 15:19, Uwe Hermann wrote:
> On Sat, Oct 09, 2010 at 11:59:24PM +0200, Carl-Daniel Hailfinger wrote:
>
>> Refactor remaining write wrappers.
>> Kill duplicated code.
>>
>> Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net>
>>
>
> Acked-by: Uwe Hermann <uwe at hermann-uwe.de>
>
Thanks for the review. Here is an extended patch which has some more
refactoring to make the switchover to partial write less painful. Parts
have already been reviewed by Uwe.
If possible, please test this code on all chip classes, one per write
function listed below.
- write_82802ab()
- write_jedec()
- write_jedec_1()
- write_m29f400bt()
- write_28sf040()
Testing of SPI chips is not needed. They are not touched in this patch.
Refactor remaining write wrappers.
Kill duplicated code.
Annotate write functions with their chunk size.
Mark Fujitsu MBM29F400BC and ST M29F400BB as untested because their
write code no longer uses a broken layout.
Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net>
Index: flashrom-partial_write_inner_function_cleanup_annotate/jedec.c
===================================================================
--- flashrom-partial_write_inner_function_cleanup_annotate/jedec.c (Revision 1209)
+++ flashrom-partial_write_inner_function_cleanup_annotate/jedec.c (Arbeitskopie)
@@ -92,6 +92,25 @@
msg_cdbg("%s: excessive loops, i=0x%x\n", __func__, i);
}
+static int getaddrmask(struct flashchip *flash)
+{
+ switch (flash->feature_bits & FEATURE_ADDR_MASK) {
+ case FEATURE_ADDR_FULL:
+ return MASK_FULL;
+ break;
+ case FEATURE_ADDR_2AA:
+ return MASK_2AA;
+ break;
+ case FEATURE_ADDR_AAA:
+ return MASK_AAA;
+ break;
+ default:
+ msg_cerr("%s called with unknown mask\n", __func__);
+ return 0;
+ break;
+ }
+}
+
static void start_program_jedec_common(struct flashchip *flash, unsigned int mask)
{
chipaddr bios = flash->virtual_memory;
@@ -317,12 +336,15 @@
return failed;
}
-int write_sector_jedec_common(struct flashchip *flash, uint8_t *src, int start, int len, unsigned int mask)
+int write_sector_jedec_common(struct flashchip *flash, uint8_t *src, int start, int len)
{
int i, failed = 0;
chipaddr dst = flash->virtual_memory + start;
chipaddr olddst;
+ int mask;
+ mask = getaddrmask(flash);
+
olddst = dst;
for (i = 0; i < len; i++) {
if (write_byte_program_jedec_common(flash, src, dst, mask))
@@ -335,14 +357,17 @@
return failed;
}
-int write_page_write_jedec_common(struct flashchip *flash, uint8_t *src, int start, int page_size, unsigned int mask)
+int write_page_write_jedec_common(struct flashchip *flash, uint8_t *src, int start, int page_size)
{
int i, tried = 0, failed;
uint8_t *s = src;
chipaddr bios = flash->virtual_memory;
chipaddr dst = bios + start;
chipaddr d = dst;
+ int mask;
+ mask = getaddrmask(flash);
+
retry:
/* Issue JEDEC Start Program command */
start_program_jedec_common(flash, mask);
@@ -373,49 +398,55 @@
return failed;
}
-static int getaddrmask(struct flashchip *flash)
+/*
+ * Write a part of the flash chip.
+ * FIXME: Use the chunk code from Michael Karcher instead.
+ * This function is a slightly modified copy of spi_write_chunked.
+ * Each page is written separately in chunks with a maximum size of chunksize.
+ */
+int write_jedec_pages(struct flashchip *flash, uint8_t *buf, int start, int len)
{
- switch (flash->feature_bits & FEATURE_ADDR_MASK) {
- case FEATURE_ADDR_FULL:
- return MASK_FULL;
- break;
- case FEATURE_ADDR_2AA:
- return MASK_2AA;
- break;
- case FEATURE_ADDR_AAA:
- return MASK_AAA;
- break;
- default:
- msg_cerr("%s called with unknown mask\n", __func__);
- return 0;
- break;
- }
-}
-
-int write_jedec(struct flashchip *flash, uint8_t *buf)
-{
- int mask;
- int i, failed = 0;
- int total_size = flash->total_size * 1024;
+ int i, starthere, lenhere;
+ /* FIXME: page_size is the wrong variable. We need max_writechunk_size
+ * in struct flashchip to do this properly. All chips using
+ * write_jedec have page_size set to max_writechunk_size, so
+ * we're OK for now.
+ */
int page_size = flash->page_size;
- mask = getaddrmask(flash);
+ /* Warning: This loop has a very unusual condition and body.
+ * The loop needs to go through each page with at least one affected
+ * byte. The lowest page number is (start / page_size) since that
+ * division rounds down. The highest page number we want is the page
+ * where the last byte of the range lives. That last byte has the
+ * address (start + len - 1), thus the highest page number is
+ * (start + len - 1) / page_size. Since we want to include that last
+ * page as well, the loop condition uses <=.
+ */
+ for (i = start / page_size; i <= (start + len - 1) / page_size; i++) {
+ /* Byte position of the first byte in the range in this page. */
+ /* starthere is an offset to the base address of the chip. */
+ starthere = max(start, i * page_size);
+ /* Length of bytes in the range in this page. */
+ lenhere = min(start + len, (i + 1) * page_size) - starthere;
- for (i = 0; i < total_size / page_size; i++) {
- if (write_page_write_jedec_common(flash, buf + i * page_size, i * page_size, page_size, mask))
- failed = 1;
+ if (write_page_write_jedec_common(flash, buf + starthere - start, starthere, lenhere))
+ return 1;
}
- return failed;
+ return 0;
}
+/* chunksize is page_size */
+int write_jedec(struct flashchip *flash, uint8_t *buf)
+{
+ return write_jedec_pages(flash, buf, 0, flash->total_size * 1024);
+}
+
+/* chunksize is 1 */
int write_jedec_1(struct flashchip *flash, uint8_t * buf)
{
- int mask;
-
- mask = getaddrmask(flash);
-
- return write_sector_jedec_common(flash, buf, 0, flash->total_size * 1024, mask);
+ return write_sector_jedec_common(flash, buf, 0, flash->total_size * 1024);
}
/* erase chip with block_erase() prototype */
Index: flashrom-partial_write_inner_function_cleanup_annotate/sst49lfxxxc.c
===================================================================
--- flashrom-partial_write_inner_function_cleanup_annotate/sst49lfxxxc.c (Revision 1209)
+++ flashrom-partial_write_inner_function_cleanup_annotate/sst49lfxxxc.c (Arbeitskopie)
@@ -75,15 +75,3 @@
}
return 0;
}
-
-int write_49lfxxxc(struct flashchip *flash, uint8_t *buf)
-{
- chipaddr bios = flash->virtual_memory;
-
- write_lockbits_49lfxxxc(flash, 0);
- write_page_82802ab(flash, buf, 0, flash->total_size * 1024);
-
- chip_writeb(0xFF, bios);
-
- return 0;
-}
Index: flashrom-partial_write_inner_function_cleanup_annotate/82802ab.c
===================================================================
--- flashrom-partial_write_inner_function_cleanup_annotate/82802ab.c (Revision 1209)
+++ flashrom-partial_write_inner_function_cleanup_annotate/82802ab.c (Arbeitskopie)
@@ -160,6 +160,7 @@
return 0;
}
+/* chunksize is 1 */
int write_82802ab(struct flashchip *flash, uint8_t *buf)
{
return write_page_82802ab(flash, buf, 0, flash->total_size * 1024);
Index: flashrom-partial_write_inner_function_cleanup_annotate/flashchips.c
===================================================================
--- flashrom-partial_write_inner_function_cleanup_annotate/flashchips.c (Revision 1209)
+++ flashrom-partial_write_inner_function_cleanup_annotate/flashchips.c (Arbeitskopie)
@@ -2995,7 +2995,7 @@
.total_size = 512,
.page_size = 64 * 1024,
.feature_bits = FEATURE_ADDR_SHIFTED | FEATURE_EITHER_RESET,
- .tested = TEST_BAD_WRITE, /* Implicit eraseblock layout in write_m29f400bt is broken. */
+ .tested = TEST_UNTESTED,
.probe = probe_m29f400bt,
.probe_timing = TIMING_IGNORED, /* routine don't use probe_timing (m29f400bt.c) */
.block_erasers =
@@ -3013,7 +3013,7 @@
.block_erase = block_erase_chip_m29f400bt,
},
},
- .write = NULL,
+ .write = write_m29f400bt,
.read = read_memmapped,
},
@@ -5107,6 +5107,7 @@
.block_erase = erase_chip_28sf040,
}
},
+ .unlock = unprotect_28sf040,
.write = write_28sf040,
.read = read_memmapped,
},
@@ -5564,7 +5565,7 @@
}
},
.unlock = unlock_49lfxxxc,
- .write = write_49lfxxxc,
+ .write = write_82802ab,
.read = read_memmapped,
},
@@ -5627,7 +5628,7 @@
}
},
.unlock = unlock_49lfxxxc,
- .write = write_49lfxxxc,
+ .write = write_82802ab,
.read = read_memmapped,
},
@@ -5659,7 +5660,7 @@
}
},
.unlock = unlock_49lfxxxc,
- .write = write_49lfxxxc,
+ .write = write_82802ab,
.read = read_memmapped,
},
@@ -5837,7 +5838,7 @@
}
},
.unlock = unlock_49lfxxxc,
- .write = write_49lfxxxc,
+ .write = write_82802ab,
.read = read_memmapped,
},
@@ -6315,7 +6316,7 @@
.total_size = 512,
.page_size = 64 * 1024,
.feature_bits = FEATURE_ADDR_SHIFTED | FEATURE_EITHER_RESET,
- .tested = TEST_BAD_WRITE, /* Implicit eraseblock layout in write_m29f400bt is broken. */
+ .tested = TEST_UNTESTED,
.probe = probe_m29f400bt,
.probe_timing = TIMING_IGNORED, /* routine doesn't use probe_timing (m29f400bt.c) */
.block_erasers =
@@ -6333,7 +6334,7 @@
.block_erase = block_erase_chip_m29f400bt,
}
},
- .write = NULL,
+ .write = write_m29f400bt,
.read = read_memmapped,
},
{
Index: flashrom-partial_write_inner_function_cleanup_annotate/sst28sf040.c
===================================================================
--- flashrom-partial_write_inner_function_cleanup_annotate/sst28sf040.c (Revision 1209)
+++ flashrom-partial_write_inner_function_cleanup_annotate/sst28sf040.c (Arbeitskopie)
@@ -30,7 +30,7 @@
#define RESET 0xFF
#define READ_ID 0x90
-static void protect_28sf040(struct flashchip *flash)
+int protect_28sf040(struct flashchip *flash)
{
chipaddr bios = flash->virtual_memory;
@@ -41,9 +41,11 @@
chip_readb(bios + 0x041B);
chip_readb(bios + 0x0419);
chip_readb(bios + 0x040A);
+
+ return 0;
}
-static void unprotect_28sf040(struct flashchip *flash)
+int unprotect_28sf040(struct flashchip *flash)
{
chipaddr bios = flash->virtual_memory;
@@ -54,12 +56,15 @@
chip_readb(bios + 0x041B);
chip_readb(bios + 0x0419);
chip_readb(bios + 0x041A);
+
+ return 0;
}
int erase_sector_28sf040(struct flashchip *flash, unsigned int address, unsigned int sector_size)
{
chipaddr bios = flash->virtual_memory;
+ /* This command sequence is very similar to erase_block_82802ab. */
chip_writeb(AUTO_PG_ERASE1, bios);
chip_writeb(AUTO_PG_ERASE2, bios + address);
@@ -101,10 +106,8 @@
{
chipaddr bios = flash->virtual_memory;
- unprotect_28sf040(flash);
chip_writeb(CHIP_ERASE, bios);
chip_writeb(CHIP_ERASE, bios);
- protect_28sf040(flash);
programmer_delay(10);
toggle_ready_jedec(bios);
@@ -116,15 +119,10 @@
return 0;
}
+/* chunksize is 1 */
int write_28sf040(struct flashchip *flash, uint8_t *buf)
{
- unprotect_28sf040(flash);
-
- write_sector_28sf040(flash, buf, 0, flash->total_size * 1024);
-
- protect_28sf040(flash);
-
- return 0;
+ return write_sector_28sf040(flash, buf, 0, flash->total_size * 1024);
}
int erase_chip_28sf040(struct flashchip *flash, unsigned int addr, unsigned int blocklen)
Index: flashrom-partial_write_inner_function_cleanup_annotate/m29f400bt.c
===================================================================
--- flashrom-partial_write_inner_function_cleanup_annotate/m29f400bt.c (Revision 1209)
+++ flashrom-partial_write_inner_function_cleanup_annotate/m29f400bt.c (Arbeitskopie)
@@ -140,6 +140,7 @@
return erase_m29f400bt(flash);
}
+/* chunksize is 1 */
int write_m29f400bt(struct flashchip *flash, uint8_t *buf)
{
return write_page_m29f400bt(flash, buf, 0, flash->total_size * 1024);
Index: flashrom-partial_write_inner_function_cleanup_annotate/chipdrivers.h
===================================================================
--- flashrom-partial_write_inner_function_cleanup_annotate/chipdrivers.h (Revision 1209)
+++ flashrom-partial_write_inner_function_cleanup_annotate/chipdrivers.h (Arbeitskopie)
@@ -86,8 +86,8 @@
int erase_sector_jedec(struct flashchip *flash, unsigned int page, unsigned int pagesize);
int erase_block_jedec(struct flashchip *flash, unsigned int page, unsigned int blocksize);
int erase_chip_block_jedec(struct flashchip *flash, unsigned int page, unsigned int blocksize);
-int write_sector_jedec_common(struct flashchip *flash, uint8_t *src, int start, int len, unsigned int mask);
-int write_page_write_jedec_common(struct flashchip *flash, uint8_t *src, int start, int page_size, unsigned int mask);
+int write_sector_jedec_common(struct flashchip *flash, uint8_t *src, int start, int len);
+int write_page_write_jedec_common(struct flashchip *flash, uint8_t *src, int start, int page_size);
/* m29f400bt.c */
int probe_m29f400bt(struct flashchip *flash);
@@ -106,10 +106,11 @@
int erase_sector_28sf040(struct flashchip *flash, unsigned int address, unsigned int sector_size);
int write_28sf040(struct flashchip *flash, uint8_t *buf);
int write_sector_28sf040(struct flashchip *flash, uint8_t *src, int start, int len);
+int unprotect_28sf040(struct flashchip *flash);
+int protect_28sf040(struct flashchip *flash);
/* sst49lfxxxc.c */
int erase_sector_49lfxxxc(struct flashchip *flash, unsigned int address, unsigned int sector_size);
-int write_49lfxxxc(struct flashchip *flash, uint8_t *buf);
int unlock_49lfxxxc(struct flashchip *flash);
/* sst_fwhub.c */
--
http://www.hailfinger.org/
More information about the flashrom
mailing list