As promised to Stefan, here's the flashrom patch I've been talking about.
Add additional SPI sector erase and chip erase command functions to flashrom. Not all chips support all commands, so allow the implementer to select the matching function. Fix a layering violation in ICH SPI code to be less bad. Still not perfect, but the new code is shorter, more generic and architecturally more sound. TODO (in a separate patch): - move the generic sector erase code to spi.c - decide which erase command to use based on info about the chip - create a generic spi_erase_all_sectors function which calls the generic sector erase function
Signed-off-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net
Index: flashrom-spi_erase/flash.h =================================================================== --- flashrom-spi_erase/flash.h (Revision 3471) +++ flashrom-spi_erase/flash.h (Arbeitskopie) @@ -417,7 +417,10 @@ int spi_command(unsigned int writecnt, unsigned int readcnt, const unsigned char *writearr, unsigned char *readarr); void spi_write_enable(); void spi_write_disable(); +int spi_chip_erase_60(struct flashchip *flash); int spi_chip_erase_c7(struct flashchip *flash); +int spi_block_erase_52(const struct flashchip *flash, unsigned long addr); +int spi_block_erase_d8(const struct flashchip *flash, unsigned long addr); int spi_chip_write(struct flashchip *flash, uint8_t *buf); int spi_chip_read(struct flashchip *flash, uint8_t *buf); uint8_t spi_read_status_register(); Index: flashrom-spi_erase/spi.c =================================================================== --- flashrom-spi_erase/spi.c (Revision 3471) +++ flashrom-spi_erase/spi.c (Arbeitskopie) @@ -266,6 +266,22 @@ } } +int spi_chip_erase_60(struct flashchip *flash) +{ + const unsigned char cmd[JEDEC_CE_60_OUTSIZE] = {JEDEC_CE_60}; + + spi_disable_blockprotect(); + spi_write_enable(); + /* Send CE (Chip Erase) */ + spi_command(sizeof(cmd), 0, cmd, NULL); + /* Wait until the Write-In-Progress bit is cleared. + * This usually takes 1-85 s, so wait in 1 s steps. + */ + while (spi_read_status_register() & JEDEC_RDSR_BIT_WIP) + sleep(1); + return 0; +} + int spi_chip_erase_c7(struct flashchip *flash) { const unsigned char cmd[JEDEC_CE_C7_OUTSIZE] = {JEDEC_CE_C7}; @@ -282,6 +298,24 @@ return 0; }
+int spi_block_erase_52(const struct flashchip *flash, unsigned long addr) +{ + unsigned char cmd[JEDEC_BE_52_OUTSIZE] = {JEDEC_BE_52}; + + cmd[1] = (addr & 0x00ff0000) >> 16; + cmd[2] = (addr & 0x0000ff00) >> 8; + cmd[3] = (addr & 0x000000ff); + spi_write_enable(); + /* Send BE (Block Erase) */ + spi_command(sizeof(cmd), 0, cmd, NULL); + /* Wait until the Write-In-Progress bit is cleared. + * This usually takes 100-4000 ms, so wait in 100 ms steps. + */ + while (spi_read_status_register() & JEDEC_RDSR_BIT_WIP) + usleep(100 * 1000); + return 0; +} + /* Block size is usually * 64k for Macronix * 32k for SST Index: flashrom-spi_erase/ichspi.c =================================================================== --- flashrom-spi_erase/ichspi.c (Revision 3471) +++ flashrom-spi_erase/ichspi.c (Arbeitskopie) @@ -452,17 +452,8 @@
static int ich_spi_erase_block(struct flashchip *flash, int offset) { - printf_debug("ich_spi_erase_block: offset=%d, sectors=%d\n", - offset, 1); - - if (run_opcode(2, curopcodes->opcode[2], offset, 0, NULL) != 0) { - printf_debug("Error erasing sector at 0x%x", offset); - return -1; - } - - printf("DONE BLOCK 0x%x\n", offset); - - return 0; + /* FIMXE: call the chip-specific spi_block_erase_XX instead. */ + return spi_block_erase_d8(flash, offset); }
static int ich_spi_read_page(struct flashchip *flash, uint8_t * buf, int offset, int maxdata)
On 05.08.2008 01:58, Carl-Daniel Hailfinger wrote:
As promised to Stefan, here's the flashrom patch I've been talking about.
Add additional SPI sector erase and chip erase command functions to flashrom. Not all chips support all commands, so allow the implementer to select the matching function. Fix a layering violation in ICH SPI code to be less bad. Still not perfect, but the new code is shorter, more generic and architecturally more sound. TODO (in a separate patch):
- move the generic sector erase code to spi.c
- decide which erase command to use based on info about the chip
- create a generic spi_erase_all_sectors function which calls the
generic sector erase function
Thanks to Stefan for reviewing and commenting. New patch follows.
Regards, Carl-Daniel
Signed-off-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net
Index: flashrom-spi_erase/flash.h =================================================================== --- flashrom-spi_erase/flash.h (Revision 3718) +++ flashrom-spi_erase/flash.h (Arbeitskopie) @@ -424,8 +424,11 @@ const unsigned char *writearr, unsigned char *readarr); void spi_write_enable(); void spi_write_disable(); +int spi_chip_erase_60(struct flashchip *flash); int spi_chip_erase_c7(struct flashchip *flash); int spi_chip_erase_d8(struct flashchip *flash); +int spi_block_erase_52(const struct flashchip *flash, unsigned long addr); +int spi_block_erase_d8(const struct flashchip *flash, unsigned long addr); int spi_chip_write(struct flashchip *flash, uint8_t *buf); int spi_chip_read(struct flashchip *flash, uint8_t *buf); uint8_t spi_read_status_register(); Index: flashrom-spi_erase/spi.c =================================================================== --- flashrom-spi_erase/spi.c (Revision 3718) +++ flashrom-spi_erase/spi.c (Arbeitskopie) @@ -271,6 +271,22 @@ } }
+int spi_chip_erase_60(struct flashchip *flash) +{ + const unsigned char cmd[JEDEC_CE_60_OUTSIZE] = {JEDEC_CE_60}; + + spi_disable_blockprotect(); + spi_write_enable(); + /* Send CE (Chip Erase) */ + spi_command(sizeof(cmd), 0, cmd, NULL); + /* Wait until the Write-In-Progress bit is cleared. + * This usually takes 1-85 s, so wait in 1 s steps. + */ + while (spi_read_status_register() & JEDEC_RDSR_BIT_WIP) + sleep(1); + return 0; +} + int spi_chip_erase_c7(struct flashchip *flash) { const unsigned char cmd[JEDEC_CE_C7_OUTSIZE] = { JEDEC_CE_C7 }; @@ -287,6 +303,24 @@ return 0; }
+int spi_block_erase_52(const struct flashchip *flash, unsigned long addr) +{ + unsigned char cmd[JEDEC_BE_52_OUTSIZE] = {JEDEC_BE_52}; + + cmd[1] = (addr & 0x00ff0000) >> 16; + cmd[2] = (addr & 0x0000ff00) >> 8; + cmd[3] = (addr & 0x000000ff); + spi_write_enable(); + /* Send BE (Block Erase) */ + spi_command(sizeof(cmd), 0, cmd, NULL); + /* Wait until the Write-In-Progress bit is cleared. + * This usually takes 100-4000 ms, so wait in 100 ms steps. + */ + while (spi_read_status_register() & JEDEC_RDSR_BIT_WIP) + usleep(100 * 1000); + return 0; +} + /* Block size is usually * 64k for Macronix * 32k for SST Index: flashrom-spi_erase/ichspi.c =================================================================== --- flashrom-spi_erase/ichspi.c (Revision 3718) +++ flashrom-spi_erase/ichspi.c (Arbeitskopie) @@ -154,7 +154,6 @@ int offset, int maxdata); static int ich_spi_write_page(struct flashchip *flash, uint8_t * bytes, int offset, int maxdata); -static int ich_spi_erase_block(struct flashchip *flash, int offset);
OPCODES O_ST_M25P = { { @@ -449,20 +448,6 @@ return -1; }
-static int ich_spi_erase_block(struct flashchip *flash, int offset) -{ - printf_debug("ich_spi_erase_block: offset=%d, sectors=%d\n", offset, 1); - - if (run_opcode(2, curopcodes->opcode[2], offset, 0, NULL) != 0) { - printf_debug("Error erasing sector at 0x%x", offset); - return -1; - } - - printf("DONE BLOCK 0x%x\n", offset); - - return 0; -} - static int ich_spi_read_page(struct flashchip *flash, uint8_t * buf, int offset, int maxdata) { @@ -566,7 +551,11 @@ printf("Programming page: \n");
for (i = 0; i < total_size / erase_size; i++) { - rc = ich_spi_erase_block(flash, i * erase_size); + /* FIMXE: call the chip-specific spi_block_erase_XX instead. + * For this, we need to add a block erase function to + * struct flashchip. + */ + rc = spi_block_erase_d8(flash, i * erase_size); if (rc) { printf("Error erasing block at 0x%x\n", i); break;
Carl-Daniel Hailfinger wrote:
On 05.08.2008 01:58, Carl-Daniel Hailfinger wrote:
As promised to Stefan, here's the flashrom patch I've been talking about.
Add additional SPI sector erase and chip erase command functions to flashrom. Not all chips support all commands, so allow the implementer to select the matching function. Fix a layering violation in ICH SPI code to be less bad. Still not perfect, but the new code is shorter, more generic and architecturally more sound. TODO (in a separate patch):
- move the generic sector erase code to spi.c
- decide which erase command to use based on info about the chip
- create a generic spi_erase_all_sectors function which calls the
generic sector erase function
Thanks to Stefan for reviewing and commenting. New patch follows.
Regards, Carl-Daniel
Signed-off-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net
Acked-by: Stefan Reinauer stepan@coresystems.de
- if (run_opcode(2, curopcodes->opcode[2], offset, 0, NULL) != 0) {
This won't apply cleanly with the version in the tree though, I dropped the index thing as per our discussion earlier.
rc = ich_spi_erase_block(flash, i * erase_size);
/* FIMXE: call the chip-specific spi_block_erase_XX instead.
* For this, we need to add a block erase function to
* struct flashchip.
*/
rc = spi_block_erase_d8(flash, i * erase_size);
I agree with the observation. But I'm not sure what the struct flashchip should look like.
When I wrote /dev/bios in 1998 I had both a page size (128-256 bytes for example) and a block size (8-192k for example) and /dev/bios would try to choose the appropriate function for your chip. Some chips have non-linear block sizes, so I also added an array with the size of each block. Plus a bitfield describing whether to use page size or block size erases and writes.
So, for some random intel chip, the block sizes would look like this:
{ "28F001BX-T", 0x9489, 12, 128, 1, 128, 3, (int []) { 0,112,116,120,128 } },
For some ancient source code browsing fun go to http://tracker.coreboot.org/trac/openbios/browser/openbios-devel/utils/devbi...
On 03.11.2008 00:39, Stefan Reinauer wrote:
Carl-Daniel Hailfinger wrote:
On 05.08.2008 01:58, Carl-Daniel Hailfinger wrote:
As promised to Stefan, here's the flashrom patch I've been talking about.
Add additional SPI sector erase and chip erase command functions to flashrom. Not all chips support all commands, so allow the implementer to select the matching function. Fix a layering violation in ICH SPI code to be less bad. Still not perfect, but the new code is shorter, more generic and architecturally more sound. TODO (in a separate patch):
- move the generic sector erase code to spi.c
- decide which erase command to use based on info about the chip
- create a generic spi_erase_all_sectors function which calls the
generic sector erase function
Thanks to Stefan for reviewing and commenting. New patch follows.
Regards, Carl-Daniel
Signed-off-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net
Acked-by: Stefan Reinauer stepan@coresystems.de
Thanks, committed in r3722.
- if (run_opcode(2, curopcodes->opcode[2], offset, 0, NULL) != 0) {
This won't apply cleanly with the version in the tree though, I dropped the index thing as per our discussion earlier.
Yes, I updated the patch.
rc = ich_spi_erase_block(flash, i * erase_size);
/* FIMXE: call the chip-specific spi_block_erase_XX instead.
* For this, we need to add a block erase function to
* struct flashchip.
*/
rc = spi_block_erase_d8(flash, i * erase_size);
I agree with the observation. But I'm not sure what the struct flashchip should look like.
When I wrote /dev/bios in 1998 I had both a page size (128-256 bytes for example) and a block size (8-192k for example) and /dev/bios would try to choose the appropriate function for your chip. Some chips have non-linear block sizes, so I also added an array with the size of each block. Plus a bitfield describing whether to use page size or block size erases and writes.
So, for some random intel chip, the block sizes would look like this:
{ "28F001BX-T", 0x9489, 12, 128, 1, 128, 3, (int []) { 0,112,116,120,128 } },
For some ancient source code browsing fun go to http://tracker.coreboot.org/trac/openbios/browser/openbios-devel/utils/devbi...
Let's continue the discussion about per-sector flashing in the existing thread about it with subject [coreboot] [RFC] flashrom: sector-based flashing
Regards, Carl-Daniel