SST25VF016B (2MB) flash on m57sli (IT8716F).
I mounted a SST25VF016B 2MByte flash chip on the second SPI bios landpattern on the m57sli mobo (as per the m57sli tutorial). There as some problems with that, on the one side from the point of the LPC memory mapping options of the IT8716F (max. 512KB contiguous) and on the other side with the programming method(s) supported by the SST flash chip (only the byte-mode programming is compatible with the IT8716). Lets call them problem1 and problem2. The reason why I want the 2MB is that I would like to try direct Linux kernel as the payload. Problem1 and Problem2 have impact on the useability of flashrom, which I have sort-of solved (patch below). Problem1 also has impact on the elfboot.c and rom-stream.c in coreboot (linuxbios), which I'm still investigating (suggestions welcome!). Problem1 (for reading) is solved by NOT using the mmap method for reading the flash contents, but using outb() for sending the flash read commands (using a specific 25vf016 read function). Also the normal read command is only supported up to 25MHz by this chip, so I cannot use the 33MHz speed as used normally by spi.c. There is also a 'high speed' read command (0x0b), which inserts an extra dummy byte between address and data, but as the 8716 only allows max. 3 bytes read per io transfer, the gain (3 bytes per io transfer @ 16MHz versus 2 bytes per io transfer @ 33MHz) is negligible. Problem2 (for writing) is solved similarly: The mmap write method cannot be used effectively, because the flash chip needs a Write-enable command for each Byte-program (no block program concept in byte-program mode). The chip has a so-called AAI (auto address increment) programming mode instead, but that cannot be used with the 8716 chip, because it needs 6 byte and 3 byte io transfers, which it doesn't support (bah!). Not using the mmap writes also solves the limited LPC memory address window. So, there is also a 25vf016 specific write function. NOTE: there may be some offset because of other changes... Index: flash.h =================================================================== --- flash.h (revision 3051) +++ flash.h (working copy) @@ -297,6 +303,8 @@ void generic_spi_write_disable(); int generic_spi_chip_erase_c7(struct flashchip *flash); int generic_spi_chip_write(struct flashchip *flash, uint8_t *buf); +int sst25vf016_spi_chip_write(struct flashchip *flash, uint8_t *buf); +int sst25vf016_spi_chip_read(struct flashchip *flash, uint8_t *buf); /* 82802ab.c */ int probe_82802ab(struct flashchip *flash); Index: flashchips.c =================================================================== --- flashchips.c (revision 3051) +++ flashchips.c (working copy) @@ -57,7 +57,7 @@ {"SST25VF040B", SST_ID, SST_25VF040B, 512, 256, probe_spi, generic_spi_chip_erase_c7, generic_spi_chip_write}, {"SST25VF016B", SST_ID, SST_25VF016B, 2048, 256, - probe_spi, generic_spi_chip_erase_c7, generic_spi_chip_write}, + probe_spi, generic_spi_chip_erase_c7, sst25vf016_spi_chip_write, sst25vf016_spi_chip_read}, {"SST29EE020A", SST_ID, SST_29EE020A, 256, 128, probe_jedec, erase_chip_jedec, write_jedec}, {"SST28SF040A", SST_ID, SST_28SF040, 512, 256, Index: spi.c =================================================================== --- spi.c (revision 3051) +++ spi.c (working copy) @@ -77,6 +77,8 @@ #define JEDEC_RDSR_BIT_WIP (0x01 << 0) uint16_t it8716f_flashport = 0; +/* use fast 33MHz SPI (<>0) or slow 16MHz (0) */ +int fast_spi=1; void generic_spi_prettyprint_status_register(struct flashchip *flash); @@ -203,10 +205,10 @@ __FUNCTION__, writecnt); return 1; } - /* Start IO, 33MHz, readcnt input bytes, writecnt output bytes. Note: + /* Start IO, 33MHz (or 16), readcnt input bytes, writecnt output bytes. Note: * We can't use writecnt directly, but have to use a strange encoding. */ - outb((0x5 << 4) | ((readcnt & 0x3) << 2) | (writeenc), port); + outb(((0x4+(fast_spi?1:0)) << 4) | ((readcnt & 0x3) << 2) | (writeenc), port); do { busy = inb(port) & 0x80; } while (busy); @@ -314,6 +316,39 @@ "%sset\n", (status & (1 << 0)) ? "" : "not "); } +/* Prettyprint the status register. Works for + * SST 25VF016 + */ +void generic_spi_prettyprint_status_register_sst25vf016(uint8_t status) +{ + const char *bpt[]={ + "none", + "1F0000H-1FFFFFH", + "1E0000H-1FFFFFH", + "1C0000H-1FFFFFH", + "180000H-1FFFFFH", + "100000H-1FFFFFH", + "all","all" + }; + printf_debug("Chip status register: Block Protect Write Disable " + "(BPL) is %sset\n", (status & (1 << 7)) ? "" : "not "); + printf_debug("Chip status register: Auto Address Increment Programming " + "(AAI) is %sset\n", (status & (1 << 6)) ? "" : "not "); + printf_debug("Chip status register: Bit 5 (BP3) is " + "%sset\n", (status & (1 << 5)) ? "" : "not "); + printf_debug("Chip status register: Bit 4 (BP2) is " + "%sset\n", (status & (1 << 4)) ? "" : "not "); + printf_debug("Chip status register: Bit 3 (BP1) is " + "%sset\n", (status & (1 << 3)) ? "" : "not "); + printf_debug("Chip status register: Bit 2 (BP0) is " + "%sset\n", (status & (1 << 2)) ? "" : "not "); + printf_debug("Resulting block protection : %s\n", bpt[(status&0x1c)>>2]); + printf_debug("Chip status register: Write Enable Latch (WEL) is " + "%sset\n", (status & (1 << 1)) ? "" : "not "); + printf_debug("Chip status register: Write In Progress (BUSY) is " + "%sset\n", (status & (1 << 0)) ? "" : "not "); +} + void generic_spi_prettyprint_status_register(struct flashchip *flash) { uint8_t status; @@ -326,6 +361,10 @@ if ((flash->model_id & 0xff00) == 0x2000) generic_spi_prettyprint_status_register_st_m25p(status); break; + case SST_ID: + if (flash->model_id == SST_25VF016B) + generic_spi_prettyprint_status_register_sst25vf016(status); + break; } } @@ -392,7 +431,7 @@ generic_spi_write_enable(); outb(0x06 , it8716f_flashport + 1); - outb((3 << 4), it8716f_flashport); + outb(((2+(fast_spi?1:0)) << 4), it8716f_flashport); for (i = 0; i < 256; i++) { bios[256 * block + i] = buf[256 * block + i]; } @@ -410,6 +449,94 @@ it8716f_spi_page_program(block, buf, bios); } +/* + * This is according the SST25VF016 datasheet, who knows it is more + * generic that this... + */ +void sst25vf016_spi_write_status_register(int status) +{ + const unsigned char cmd[] = {0x01,(unsigned char)status}; + + /* Send WRSR (Write Status Register) */ + generic_spi_command(2, 0, cmd, NULL); + +} + +void sst25vf016_spi_byte_program(int address, uint8_t byte) +{ + const unsigned char cmd[5] = {0x02, + (address>>16)&0xff, + (address>>8)&0xff, + (address>>0)&0xff, + byte + }; + + /* Send Byte-Program */ + generic_spi_command(5, 0, cmd, NULL); + +} + +/* + * IT8716F only allows maximum of 512 kb SPI mapped to LPC memory cycles + * Program chip using firmware cycle byte programming. (SLOW!) + */ +int sst25vf016_spi_chip_write(struct flashchip *flash, uint8_t *buf) +{ + int total_size = 1024 * flash->total_size; + uint8_t status; + int i; + fast_spi=0; + + status = generic_spi_read_status_register(); + /* If there is block protection in effect, unprotect it first. */ + if ((status&0x3c)!=0) { + printf_debug("Some block protection in effect, disabling\n"); + generic_spi_write_enable(); + sst25vf016_spi_write_status_register(status&~0x3c); + } + for (i=0; i<total_size; i++) { + generic_spi_write_enable(); + sst25vf016_spi_byte_program(i,buf[i]); + //while (generic_spi_read_status_register() & JEDEC_RDSR_BIT_WIP) + usleep(10); + if (i%1024==0) fputc('b',stderr); + } + /* resume normal ops... */ + outb(0x20, it8716f_flashport); + return 0; +} + +void sst25vf016_spi_3byte_read(int address, uint8_t *bytes, int len) +{ + const unsigned char cmd[5] = {0x03, + (address>>16)&0xff, + (address>>8)&0xff, + (address>>0)&0xff, + }; + + /* Send Read */ + generic_spi_command(4, len, cmd, bytes); + +} + +/* + * IT8716F only allows maximum of 512 kb SPI mapped to LPC memory cycles + * Need to read this big flash using firmware cycles 3 byte at a time. + */ +int sst25vf016_spi_chip_read(struct flashchip *flash, uint8_t *buf) +{ + int total_size = 1024 * flash->total_size; + int i; + fast_spi=0; + + for (i = 0; i < total_size; i+=3) { + int toread=3; + if (total_size-i < toread) toread=total_size-i; + sst25vf016_spi_3byte_read(i, buf+i, toread); + } + return 0; +} + int generic_spi_chip_write(struct flashchip *flash, uint8_t *buf) { int total_size = 1024 * flash->total_size; int i;
Hi Ronald, first of all, welcome to coreboot. I see you invested a lot of time to work around the deficiencies of the IT8716F SPI translation function. You even did it the way I described the process a few weeks ago (but I had no time to implement it). Please see http://www.coreboot.org/Development_Guidelines#Sign-off_Procedure for details on patch submission. There are two things that block acceptance of your patch: - A "Signed-off-by:" line is missing. Please add one. - The patch does not apply because Evolution mangled it on your side. Either fix the configuration or attach the patch as plain text. On 17.01.2008 00:13, Ronald Hoogenboom wrote:
I mounted a SST25VF016B 2MByte flash chip on the second SPI bios landpattern on the m57sli mobo (as per the m57sli tutorial). There as some problems with that, on the one side from the point of the LPC memory mapping options of the IT8716F (max. 512KB contiguous) and on the other side with the programming method(s) supported by the SST flash chip (only the byte-mode programming is compatible with the IT8716). Lets call them problem1 and problem2.
The reason why I want the 2MB is that I would like to try direct Linux kernel as the payload.
Agreed.
Problem1 and Problem2 have impact on the useability of flashrom, which I have sort-of solved (patch below). Problem1 also has impact on the elfboot.c and rom-stream.c in coreboot (linuxbios), which I'm still investigating (suggestions welcome!).
I'd like to not handle this problem at all in coreboot v3. In v2, we can probably work around this issue by ensuring that all code until RAM is enabled has to be mapped in the contiguous 512 kByte area (which may correspond to the first or last 512 kB of the flash chip, nobody really knows and ITE are not answering my mails.
Problem1 (for reading) is solved by NOT using the mmap method for reading the flash contents, but using outb() for sending the flash read commands (using a specific 25vf016 read function). Also the normal read command is only supported up to 25MHz by this chip, so I cannot use the 33MHz speed as used normally by spi.c. There is also a 'high speed' read command (0x0b), which inserts an extra dummy byte between address and data, but as the 8716 only allows max. 3 bytes read per io transfer, the gain (3 bytes per io transfer @ 16MHz versus 2 bytes per io transfer @ 33MHz) is negligible.
Yes. We have to specify a maximum speed for each SPI chip in flashchips.c. Suggestions welcome.
Problem2 (for writing) is solved similarly: The mmap write method cannot be used effectively, because the flash chip needs a Write-enable command for each Byte-program (no block program concept in byte-program mode). The chip has a so-called AAI (auto address increment) programming mode instead, but that cannot be used with the 8716 chip, because it needs 6 byte and 3 byte io transfers, which it doesn't support (bah!). Not using the mmap writes also solves the limited LPC memory address window. So, there is also a 25vf016 specific write function.
The function is probably more specific to IT8716F and chips bigger than 512 kByte. Maybe rename the function and have the IT8716F writing code switch to you method if the chip is bigger than 512 kB?
NOTE: there may be some offset because of other changes...
Index: flash.h =================================================================== --- flash.h (revision 3051) +++ flash.h (working copy) @@ -297,6 +303,8 @@ void generic_spi_write_disable(); int generic_spi_chip_erase_c7(struct flashchip *flash); int generic_spi_chip_write(struct flashchip *flash, uint8_t *buf); +int sst25vf016_spi_chip_write(struct flashchip *flash, uint8_t *buf); +int sst25vf016_spi_chip_read(struct flashchip *flash, uint8_t *buf);
/* 82802ab.c */ int probe_82802ab(struct flashchip *flash); Index: flashchips.c =================================================================== --- flashchips.c (revision 3051) +++ flashchips.c (working copy) @@ -57,7 +57,7 @@ {"SST25VF040B", SST_ID, SST_25VF040B, 512, 256, probe_spi, generic_spi_chip_erase_c7, generic_spi_chip_write}, {"SST25VF016B", SST_ID, SST_25VF016B, 2048, 256, - probe_spi, generic_spi_chip_erase_c7, generic_spi_chip_write}, + probe_spi, generic_spi_chip_erase_c7, sst25vf016_spi_chip_write, sst25vf016_spi_chip_read}, {"SST29EE020A", SST_ID, SST_29EE020A, 256, 128, probe_jedec, erase_chip_jedec, write_jedec}, {"SST28SF040A", SST_ID, SST_28SF040, 512, 256, Index: spi.c =================================================================== --- spi.c (revision 3051) +++ spi.c (working copy) @@ -77,6 +77,8 @@ #define JEDEC_RDSR_BIT_WIP (0x01 << 0)
uint16_t it8716f_flashport = 0; +/* use fast 33MHz SPI (<>0) or slow 16MHz (0) */ +int fast_spi=1;
void generic_spi_prettyprint_status_register(struct flashchip *flash);
@@ -203,10 +205,10 @@ __FUNCTION__, writecnt); return 1; } - /* Start IO, 33MHz, readcnt input bytes, writecnt output bytes. Note: + /* Start IO, 33MHz (or 16), readcnt input bytes, writecnt output bytes. Note: * We can't use writecnt directly, but have to use a strange encoding. */ - outb((0x5 << 4) | ((readcnt & 0x3) << 2) | (writeenc), port); + outb(((0x4+(fast_spi?1:0)) << 4) | ((readcnt & 0x3) << 2) | (writeenc), port); do { busy = inb(port) & 0x80; } while (busy); @@ -314,6 +316,39 @@ "%sset\n", (status & (1 << 0)) ? "" : "not "); }
+/* Prettyprint the status register. Works for + * SST 25VF016 + */ +void generic_spi_prettyprint_status_register_sst25vf016(uint8_t status) +{ + const char *bpt[]={ + "none", + "1F0000H-1FFFFFH", + "1E0000H-1FFFFFH", + "1C0000H-1FFFFFH", + "180000H-1FFFFFH", + "100000H-1FFFFFH", + "all","all" + }; + printf_debug("Chip status register: Block Protect Write Disable " + "(BPL) is %sset\n", (status & (1 << 7)) ? "" : "not "); + printf_debug("Chip status register: Auto Address Increment Programming " + "(AAI) is %sset\n", (status & (1 << 6)) ? "" : "not "); + printf_debug("Chip status register: Bit 5 (BP3) is " + "%sset\n", (status & (1 << 5)) ? "" : "not "); + printf_debug("Chip status register: Bit 4 (BP2) is " + "%sset\n", (status & (1 << 4)) ? "" : "not "); + printf_debug("Chip status register: Bit 3 (BP1) is " + "%sset\n", (status & (1 << 3)) ? "" : "not "); + printf_debug("Chip status register: Bit 2 (BP0) is " + "%sset\n", (status & (1 << 2)) ? "" : "not "); + printf_debug("Resulting block protection : %s\n", bpt[(status&0x1c)>>2]); + printf_debug("Chip status register: Write Enable Latch (WEL) is " + "%sset\n", (status & (1 << 1)) ? "" : "not "); + printf_debug("Chip status register: Write In Progress (BUSY) is " + "%sset\n", (status & (1 << 0)) ? "" : "not "); +} + void generic_spi_prettyprint_status_register(struct flashchip *flash) { uint8_t status; @@ -326,6 +361,10 @@ if ((flash->model_id & 0xff00) == 0x2000) generic_spi_prettyprint_status_register_st_m25p(status); break; + case SST_ID: + if (flash->model_id == SST_25VF016B) + generic_spi_prettyprint_status_register_sst25vf016(status); + break; } }
@@ -392,7 +431,7 @@
generic_spi_write_enable(); outb(0x06 , it8716f_flashport + 1); - outb((3 << 4), it8716f_flashport); + outb(((2+(fast_spi?1:0)) << 4), it8716f_flashport); for (i = 0; i < 256; i++) { bios[256 * block + i] = buf[256 * block + i]; } @@ -410,6 +449,94 @@ it8716f_spi_page_program(block, buf, bios); }
+/* + * This is according the SST25VF016 datasheet, who knows it is more + * generic that this... + */ +void sst25vf016_spi_write_status_register(int status) +{ + const unsigned char cmd[] = {0x01,(unsigned char)status}; + + /* Send WRSR (Write Status Register) */ + generic_spi_command(2, 0, cmd, NULL); + +} + +void sst25vf016_spi_byte_program(int address, uint8_t byte) +{ + const unsigned char cmd[5] = {0x02, + (address>>16)&0xff, + (address>>8)&0xff, + (address>>0)&0xff, + byte + }; + + /* Send Byte-Program */ + generic_spi_command(5, 0, cmd, NULL); + +} + +/* + * IT8716F only allows maximum of 512 kb SPI mapped to LPC memory cycles + * Program chip using firmware cycle byte programming. (SLOW!) + */ +int sst25vf016_spi_chip_write(struct flashchip *flash, uint8_t *buf) +{ + int total_size = 1024 * flash->total_size; + uint8_t status; + int i; + fast_spi=0; + + status = generic_spi_read_status_register(); + /* If there is block protection in effect, unprotect it first. */ + if ((status&0x3c)!=0) { + printf_debug("Some block protection in effect, disabling\n"); + generic_spi_write_enable(); + sst25vf016_spi_write_status_register(status&~0x3c); + } + for (i=0; i<total_size; i++) { + generic_spi_write_enable(); + sst25vf016_spi_byte_program(i,buf[i]); + //while (generic_spi_read_status_register() & JEDEC_RDSR_BIT_WIP) + usleep(10); + if (i%1024==0) fputc('b',stderr); + } + /* resume normal ops... */ + outb(0x20, it8716f_flashport); + return 0; +} + +void sst25vf016_spi_3byte_read(int address, uint8_t *bytes, int len) +{ + const unsigned char cmd[5] = {0x03, + (address>>16)&0xff, + (address>>8)&0xff, + (address>>0)&0xff, + }; + + /* Send Read */ + generic_spi_command(4, len, cmd, bytes); + +} + +/* + * IT8716F only allows maximum of 512 kb SPI mapped to LPC memory cycles + * Need to read this big flash using firmware cycles 3 byte at a time. + */ +int sst25vf016_spi_chip_read(struct flashchip *flash, uint8_t *buf) +{ + int total_size = 1024 * flash->total_size; + int i; + fast_spi=0; + + for (i = 0; i < total_size; i+=3) { + int toread=3; + if (total_size-i < toread) toread=total_size-i; + sst25vf016_spi_3byte_read(i, buf+i, toread); + } + return 0; +} + int generic_spi_chip_write(struct flashchip *flash, uint8_t *buf) { int total_size = 1024 * flash->total_size; int i;
Please try to address the comments above and repost. Note: I suggest that the "maximum frequency" stuff could be handled in another patch because it requires redesign of some structures (needs discussion) and is also is logically another change. Regards, Carl-Daniel
Hi Ronald and Carl-Daniel, Great work - I want to use larger SPI chips on this board myself. On Thu, Jan 17, 2008 at 12:56:08AM +0100, Carl-Daniel Hailfinger wrote:
I see you invested a lot of time to work around the deficiencies of the IT8716F SPI translation function. You even did it the way I described the process a few weeks ago (but I had no time to implement it).
Problem1 and Problem2 have impact on the useability of flashrom, which I have sort-of solved (patch below). Problem1 also has impact on the elfboot.c and rom-stream.c in coreboot (linuxbios), which I'm still investigating (suggestions welcome!).
I'd like to not handle this problem at all in coreboot v3.
What do you propose for v3? Thanks, Ward. -- Ward Vandewege <ward@fsf.org> Free Software Foundation - Senior System Administrator
On 17.01.2008 01:05, Ward Vandewege wrote:
Hi Ronald and Carl-Daniel,
Great work - I want to use larger SPI chips on this board myself.
On Thu, Jan 17, 2008 at 12:56:08AM +0100, Carl-Daniel Hailfinger wrote:
I see you invested a lot of time to work around the deficiencies of the IT8716F SPI translation function. You even did it the way I described the process a few weeks ago (but I had no time to implement it).
Problem1 and Problem2 have impact on the useability of flashrom, which I have sort-of solved (patch below). Problem1 also has impact on the elfboot.c and rom-stream.c in coreboot (linuxbios), which I'm still investigating (suggestions welcome!).
I'd like to not handle this problem at all in coreboot v3.
What do you propose for v3?
Keep v3 clean, simply refuse to support such horrible interfaces in v3. Especially the lar walk code would explode to about twice the size if we wanted to support flash parts that can not be memmapped completely. There are workarounds for some of that stuff, but I'll keep them secret until someone manages to actually port a board with IT8176F SPI translation to v3. I hope that will never happen. Regards, Carl-Daniel
On 17.01.2008 00:13, Ronald Hoogenboom wrote:
I mounted a SST25VF016B 2MByte flash chip on the second SPI bios landpattern on the m57sli mobo (as per the m57sli tutorial). There as some problems with that[...]
Problem1 (for reading) is solved by NOT using the mmap method for reading the flash contents, but using outb() for sending the flash read commands (using a specific 25vf016 read function). Also the normal read command is only supported up to 25MHz by this chip, so I cannot use the 33MHz speed as used normally by spi.c. There is also a 'high speed' read command (0x0b), which inserts an extra dummy byte between address and data, but as the 8716 only allows max. 3 bytes read per io transfer, the gain (3 bytes per io transfer @ 16MHz versus 2 bytes per io transfer @ 33MHz) is negligible.
Please be aware that the M57SLI may read the reset vector and other really early stuff at 33 MHz, thereby causing read errors (sometimes single bit shifts) which are really hard to find. Regards, Carl-Daniel
On Thu, Jan 17, 2008 at 01:24:38AM +0100, Carl-Daniel Hailfinger wrote:
Please be aware that the M57SLI may read the reset vector and other really early stuff at 33 MHz, thereby causing read errors (sometimes single bit shifts) which are really hard to find.
English translation: SST25VF016B is not really compatible with the IT8716F superio. About right? //Peter
On 17.01.2008 02:29, Peter Stuge wrote:
On Thu, Jan 17, 2008 at 01:24:38AM +0100, Carl-Daniel Hailfinger wrote:
Please be aware that the M57SLI may read the reset vector and other really early stuff at 33 MHz, thereby causing read errors (sometimes single bit shifts) which are really hard to find.
English translation:
;-)
SST25VF016B is not really compatible with the IT8716F superio.
About right?
Mostly. If the M57SLI uses the IT8716F in 33 MHz mode at startup, the SST25VF016B is indeed incompatible. If IT8716F startup happens at 16 MHz, it will work. I have no idea about the real behaviour of the hardware. Regards, Carl-Daniel
Hi Ronald, On Thu, Jan 17, 2008 at 12:13:48AM +0100, Ronald Hoogenboom wrote:
Problem1 (for reading) is solved by NOT using the mmap method for reading the flash contents, but using outb() for sending the flash read commands (using a specific 25vf016 read function). Also the normal read command is only supported up to 25MHz by this chip, so I cannot use the 33MHz speed as used normally by spi.c. There is also a 'high speed' read command (0x0b), which inserts an extra dummy byte between address and data, but as the 8716 only allows max. 3 bytes read per io transfer, the gain (3 bytes per io transfer @ 16MHz versus 2 bytes per io transfer @ 33MHz) is negligible.
Can you elaborate a bit about that - I see the two read speeds in the datasheet, but how do you get to 16MHz in that last sentence? Thanks, Ward. -- Ward Vandewege <ward@fsf.org> Free Software Foundation - Senior System Administrator
On Thu, Jan 17, 2008 at 02:29:16AM +0100, Peter Stuge wrote:
On Thu, Jan 17, 2008 at 01:24:38AM +0100, Carl-Daniel Hailfinger wrote:
Please be aware that the M57SLI may read the reset vector and other really early stuff at 33 MHz, thereby causing read errors (sometimes single bit shifts) which are really hard to find.
English translation:
SST25VF016B is not really compatible with the IT8716F superio.
All right. So how about the Atmel AT45DB321D-SU, which is a 32Mbit (!) chip that seems to be able to do 33MHz reads in slow mode (and 66MHz in fast mode). Only about $3.66 at Mouser.com, and it's in stock. Here's the datasheet: http://www.atmel.com/dyn/resources/prod_documents/doc3597.pdf Good? Bad? Thanks, Ward. -- Ward Vandewege <ward@fsf.org> Free Software Foundation - Senior System Administrator
On Wed, Jan 16, 2008 at 08:57:52PM -0500, Ward Vandewege wrote:
English translation:
SST25VF016B is not really compatible with the IT8716F superio.
All right. So how about the Atmel AT45DB321D-SU
Unfortunately it has a different pinout than the flash chips we've looked at so far. //Peter
On Thu, Jan 17, 2008 at 03:03:48AM +0100, Peter Stuge wrote:
On Wed, Jan 16, 2008 at 08:57:52PM -0500, Ward Vandewege wrote:
English translation:
SST25VF016B is not really compatible with the IT8716F superio.
All right. So how about the Atmel AT45DB321D-SU
Unfortunately it has a different pinout than the flash chips we've looked at so far.
Oh, you're right, odd. It would work in a socket I guess, if that socket is wired up correctly. Hrm. Any other suggestions for large SPI/SOIC-8 chips? Thanks, Ward. -- Ward Vandewege <ward@fsf.org> Free Software Foundation - Senior System Administrator
On Wed, Jan 16, 2008 at 09:09:43PM -0500, Ward Vandewege wrote:
All right. So how about the Atmel AT45DB321D-SU
Unfortunately it has a different pinout than the flash chips we've looked at so far.
Oh, you're right, odd. It would work in a socket I guess, if that socket is wired up correctly. Hrm.
If the SPI commands for reading are the same then yeah, it should work.
Any other suggestions for large SPI/SOIC-8 chips?
Macronix MX25L8005M2C-15G Winbond W25X16VSSI Winbond W25X32VSSI Winbond will also make a 64Mbit flash but it didn't have a product number a few months ago. Problem with Winbond is they really only want to sell large quantities. I haven't even gotten a quote and delivery time back from my sales rep on a request I made in November. //Peter
On 17.01.2008 02:48, Ward Vandewege wrote:
Hi Ronald,
On Thu, Jan 17, 2008 at 12:13:48AM +0100, Ronald Hoogenboom wrote:
Problem1 (for reading) is solved by NOT using the mmap method for reading the flash contents, but using outb() for sending the flash read commands (using a specific 25vf016 read function). Also the normal read command is only supported up to 25MHz by this chip, so I cannot use the 33MHz speed as used normally by spi.c. There is also a 'high speed' read command (0x0b), which inserts an extra dummy byte between address and data, but as the 8716 only allows max. 3 bytes read per io transfer, the gain (3 bytes per io transfer @ 16MHz versus 2 bytes per io transfer @ 33MHz) is negligible.
Can you elaborate a bit about that - I see the two read speeds in the datasheet, but how do you get to 16MHz in that last sentence?
33 MHz /2 (at least that's how the IT8716F datasheet calls it. Regards, Carl-Daniel
On Thu, Jan 17, 2008 at 03:24:23AM +0100, Peter Stuge wrote:
On Wed, Jan 16, 2008 at 09:09:43PM -0500, Ward Vandewege wrote:
All right. So how about the Atmel AT45DB321D-SU
Unfortunately it has a different pinout than the flash chips we've looked at so far.
Oh, you're right, odd. It would work in a socket I guess, if that socket is wired up correctly. Hrm.
If the SPI commands for reading are the same then yeah, it should work.
Hmm, yeah, that's worth investigating I guess.
Any other suggestions for large SPI/SOIC-8 chips?
Macronix MX25L8005M2C-15G
Mouser/digikeys/jameco don't have this one.
Winbond W25X16VSSI Winbond W25X32VSSI
Problem with Winbond is they really only want to sell large quantities. I haven't even gotten a quote and delivery time back from my sales rep on a request I made in November.
Weird, yeah, minimum purchase is around $500 for those two - 270 units of the 16Mbit, or 180 units of the 32Mbit chip (this is digikeys, the others don't carry them). Maybe a group buy is in order... Thanks, Ward. -- Ward Vandewege <ward@fsf.org> Free Software Foundation - Senior System Administrator
Am Donnerstag, 17. Januar 2008 03:09:43 schrieb Ward Vandewege:
On Thu, Jan 17, 2008 at 03:03:48AM +0100, Peter Stuge wrote:
On Wed, Jan 16, 2008 at 08:57:52PM -0500, Ward Vandewege wrote:
English translation:
SST25VF016B is not really compatible with the IT8716F superio.
All right. So how about the Atmel AT45DB321D-SU
Unfortunately it has a different pinout than the flash chips we've looked at so far.
Oh, you're right, odd. It would work in a socket I guess, if that socket is wired up correctly. Hrm.
Any other suggestions for large SPI/SOIC-8 chips?
I've two Macronix here one is 8mbit with part number: MX25L8005 and the second one is an MX25L3205D.
Thanks, Ward.
regards, Harald
-- Ward Vandewege <ward@fsf.org> Free Software Foundation - Senior System Administrator
Do you think it would be possible to add write support for the MX25L8005 and the MX25L3205D to flashrom based on this patch? I'd be really glad to see write support for those chips. regards, Harald
On Wed, Jan 16, 2008 at 10:34:29PM -0500, Ward Vandewege wrote:
Any other suggestions for large SPI/SOIC-8 chips?
Macronix MX25L8005M2C-15G
Mouser/digikeys/jameco don't have this one.
I've tried to get quotes also for 16 and 32Mbit Macronix but they weren't available last time I asked.
Weird, yeah, minimum purchase is around $500 for those two - 270 units of the 16Mbit, or 180 units of the 32Mbit chip (this is digikeys, the others don't carry them).
Maybe a group buy is in order...
32Mbit is indeed nice. I've found what seems to be a good 16Mbit candidate that is readily available to me at a fair price without(!) moq. :) Spansion S25FL016A0LMFI011 http://www.spansion.com/products/S25FL016A.html http://www.spansion.com/datasheets/s25fl016a_00_c3.pdf //Peter
On 17.01.2008 14:32, Harald Gutmann wrote:
Do you think it would be possible to add write support for the MX25L8005 and the MX25L3205D to flashrom based on this patch?
Yes. The code is generic enough. But the order in which you write sectors to the chip may differ between both flashing methods. Regards, Carl-Daniel
On 17.01.2008 15:00, Peter Stuge wrote:
I've found what seems to be a good 16Mbit candidate that is readily available to me at a fair price without(!) moq. :)
Spansion S25FL016A0LMFI011 http://www.spansion.com/products/S25FL016A.html http://www.spansion.com/datasheets/s25fl016a_00_c3.pdf
Yes, that one should work fine. Regards, Carl-Daniel
On Thu, Jan 17, 2008 at 03:00:35PM +0100, Peter Stuge wrote:
I've tried to get quotes also for 16 and 32Mbit Macronix but they weren't available last time I asked.
16Mbit is now, but 32 and 64 are still not in production, only sampling. //Peter
On Thu, Jan 17, 2008 at 03:00:35PM +0100, Peter Stuge wrote:
I've found what seems to be a good 16Mbit candidate that is readily available to me at a fair price without(!) moq. :)
Spansion S25FL016A0LMFI011 http://www.spansion.com/products/S25FL016A.html http://www.spansion.com/datasheets/s25fl016a_00_c3.pdf
Nice! That one is available for $1.88 (1+) - $1.61 (100+) at jameco.com. I'll get a couple I think. 16Mbit is plenty big for an LAB payload with a useful busybox environment. Thanks for digging this one up. Thanks, Ward. -- Ward Vandewege <ward@fsf.org> Free Software Foundation - Senior System Administrator
On Thu, 2008-01-17 at 01:24 +0100, Carl-Daniel Hailfinger wrote:
On 17.01.2008 00:13, Ronald Hoogenboom wrote:
I mounted a SST25VF016B 2MByte flash chip on the second SPI bios ... 33MHz) is negligible.
Please be aware that the M57SLI may read the reset vector and other really early stuff at 33 MHz, thereby causing read errors (sometimes single bit shifts) which are really hard to find.
As far as I've seen the default io speed setting is 33/2, see the table in sect. 9.5.3.1 on page 78. So I hope (if the datasheet is correct) everything is done at low speed initially, which is GOOD ;-).
Regards, Carl-Daniel
What I've seen is that the memory mapped area from fff80000 to ffffffff is the top quarter of the 2MB flash rom, so it should be the intended contents at the correct location, as the linuxbios code is located there at the top and the elf payload at the bottom. Probably the IT8716 doesn't do any address translation at all and the flash chip just ignores the upper address bits (just like when block-reading past the top of the chips address space). brgds, Ronald.
On Thu, 2008-01-17 at 00:56 +0100, Carl-Daniel Hailfinger wrote:
Please see http://www.coreboot.org/Development_Guidelines#Sign-off_Procedure for details on patch submission. There are two things that block acceptance of your patch: - A "Signed-off-by:" line is missing. Please add one.
I'll try again now... Sorry for my ignorance (or failure to read the procedure).
The function is probably more specific to IT8716F and chips bigger than 512 kByte. Maybe rename the function and have the IT8716F writing code switch to you method if the chip is bigger than 512 kB?
I've made it a little more generic, but otoh, the spi.c is sort of less generic than the filename would suggest, quite IT8716F specific. I've made all spi flash chips over 512k point to the new functions and hope that they will all work for them too (I cannot test this). With the generic functions, it will definately not work because of the 8716's 512k limit, so it won't break anything that is not already broken... But the lack of block-write seems to me is quite sst chip specific (remember they have an AAI mode instead, which doesn't play well with the limited io transfer length options of the IT8716). It might be more appropriate to have an 'over512k_page_write' for more speed with chips that do support that. Also the byte program wait time is hard wired to 10 us (following the 25vf016b datasheet). And the block protect enablement might also NOT be so generic.
Please try to address the comments above and repost. Note: I suggest that the "maximum frequency" stuff could be handled in another patch because it requires redesign of some structures (needs discussion) and is also is logically another change.
For now, the frequency is set to 33/2 for the new large flash functions, pending a better approach. The reworked attached patch also includes the added winbond (previously known as Nexcom) chips. Signed-off-by: Ronald Hoogenboom <hoogenboom30@zonnet.nl> ---
Now I'm looking at how to make the rom_stream read the flash chip like in the over512k_read_chip. But I'm a bit stuck on the overal mechanics of the initialization process and how to get hold of the assigned io port for the SPI serial flash controller in the IT8716. This is supposed to be configured by the GPIO Config registers (LDN 07) at index 0x64 and 0x65, but in the pnp_dev_info array in it8716f/superio.c, there is nothing listed after IT8716F_GPIO, while at other indexes, there is. Does this mean that the superio.c file fails to configure this io port? Suppose that this port IS configured correcly, what means should my rom_stream use to retrieve this port address? Is there some doc about these pnp functions? Suppose I've figured it all out and created my new rom_stream_over512k (or so) object, how do I mingle it into the Config.lb files, so it will automatically get chosen when a ROM_SIZE of over 512k is selected in the target configuration (AND the superio is an IT8716F). In the targets/gigabyte/m57sli directory, there is a Config.lb.kernel file, which I used as a base for my build, but it only uses a 'fallback' and a 'failover' image. Why no 'normal' one? What is supposed to be the purpose of each image? (You see, I'm really missing some key parts of the big picture...) Ronald.
On 17.01.2008 22:25, Ronald Hoogenboom wrote:
On Thu, 2008-01-17 at 00:56 +0100, Carl-Daniel Hailfinger wrote:
Please see http://www.coreboot.org/Development_Guidelines#Sign-off_Procedure for details on patch submission. There are two things that block acceptance of your patch: - A "Signed-off-by:" line is missing. Please add one.
I'll try again now... Sorry for my ignorance (or failure to read the procedure).
No problem, thanks for fixing this up.
The function is probably more specific to IT8716F and chips bigger than 512 kByte. Maybe rename the function and have the IT8716F writing code switch to you method if the chip is bigger than 512 kB?
I've made it a little more generic, but otoh, the spi.c is sort of less generic than the filename would suggest, quite IT8716F specific.
Yes, but I always try to keep infrastructure generic enough to be able to support other SPI hosts later.
I've made all spi flash chips over 512k point to the new functions and hope that they will all work for them too (I cannot test this). With the generic functions, it will definately not work because of the 8716's 512k limit, so it won't break anything that is not already broken... But the lack of block-write seems to me is quite sst chip specific (remember they have an AAI mode instead, which doesn't play well with the limited io transfer length options of the IT8716). It might be more appropriate to have an 'over512k_page_write' for more speed with chips that do support that. Also the byte program wait time is hard wired to 10 us (following the 25vf016b datasheet). And the block protect enablement might also NOT be so generic.
By the way, the lack of block-write in the data sheet does not mean it will not work with the real chip.
Please try to address the comments above and repost. Note: I suggest that the "maximum frequency" stuff could be handled in another patch because it requires redesign of some structures (needs discussion) and is also is logically another change.
For now, the frequency is set to 33/2 for the new large flash functions, pending a better approach.
The reworked attached patch also includes the added winbond (previously known as Nexcom) chips.
Signed-off-by: Ronald Hoogenboom <hoogenboom30@zonnet.nl>
Nice. I have restructured your patch a bit, but I'd like to improve even further. Could you take a look at all those places where you added "const unsigned char cmd[] = XXX" and use #defines for commands like those at the top of spi.c? That would improve readability of the code. Please verify the code still works for you. Explicit mentioning of flash functions for chips >512 kByte has been converted to implicit decisions. Some cosmetic changes were done as well. Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@gmx.net> Index: flashrom-big/flash.h =================================================================== --- flashrom-big/flash.h (Revision 3051) +++ flashrom-big/flash.h (Arbeitskopie) @@ -234,7 +234,16 @@ #define TI_ID 0x97 /* Texas Instruments */ +/* + * W25X chips are SPI, first byte of device ID is memory type, second + * byte of device ID is related to log(bitsize). + */ #define WINBOND_ID 0xDA /* Winbond */ +#define WINBOND_NEX_ID 0xEF /* Winbond (ex Nexcom) serial flash devices */ +#define W_25X10 0x3011 +#define W_25X20 0x3012 +#define W_25X40 0x3013 +#define W_25X80 0x3014 #define W_29C011 0xC1 #define W_29C020C 0x45 #define W_29C040P 0x46 @@ -297,6 +306,7 @@ void generic_spi_write_disable(); int generic_spi_chip_erase_c7(struct flashchip *flash); int generic_spi_chip_write(struct flashchip *flash, uint8_t *buf); +int generic_spi_chip_read(struct flashchip *flash, uint8_t *buf); /* 82802ab.c */ int probe_82802ab(struct flashchip *flash); Index: flashrom-big/flashchips.c =================================================================== --- flashrom-big/flashchips.c (Revision 3051) +++ flashrom-big/flashchips.c (Arbeitskopie) @@ -51,13 +51,13 @@ {"MX29F002", MX_ID, MX_29F002, 256, 64 * 1024, probe_29f002, erase_29f002, write_29f002}, {"MX25L4005", MX_ID, MX_25L4005, 512, 256, - probe_spi, generic_spi_chip_erase_c7, generic_spi_chip_write}, + probe_spi, generic_spi_chip_erase_c7, generic_spi_chip_write, generic_spi_chip_read}, {"MX25L8005", MX_ID, MX_25L8005, 1024, 256, - probe_spi, generic_spi_chip_erase_c7, generic_spi_chip_write}, + probe_spi, generic_spi_chip_erase_c7, generic_spi_chip_write, generic_spi_chip_read}, {"SST25VF040B", SST_ID, SST_25VF040B, 512, 256, - probe_spi, generic_spi_chip_erase_c7, generic_spi_chip_write}, + probe_spi, generic_spi_chip_erase_c7, generic_spi_chip_write, generic_spi_chip_read}, {"SST25VF016B", SST_ID, SST_25VF016B, 2048, 256, - probe_spi, generic_spi_chip_erase_c7, generic_spi_chip_write}, + probe_spi, generic_spi_chip_erase_c7, generic_spi_chip_write, generic_spi_chip_read}, {"SST29EE020A", SST_ID, SST_29EE020A, 256, 128, probe_jedec, erase_chip_jedec, write_jedec}, {"SST28SF040A", SST_ID, SST_28SF040, 512, 256, @@ -122,6 +122,14 @@ probe_jedec, erase_chip_jedec, write_39sf020}, {"W39V080A", WINBOND_ID, W_39V080A, 1024, 64*1024, probe_jedec, erase_chip_jedec, write_39sf020}, + {"W25x10", WINBOND_NEX_ID, W_25X10, 128, 256, + probe_spi, generic_spi_chip_erase_c7, generic_spi_chip_write, generic_spi_chip_read}, + {"W25x20", WINBOND_NEX_ID, W_25X20, 256, 256, + probe_spi, generic_spi_chip_erase_c7, generic_spi_chip_write, generic_spi_chip_read}, + {"W25x40", WINBOND_NEX_ID, W_25X40, 512, 256, + probe_spi, generic_spi_chip_erase_c7, generic_spi_chip_write, generic_spi_chip_read}, + {"W25x80", WINBOND_NEX_ID, W_25X80, 1024, 256, + probe_spi, generic_spi_chip_erase_c7, generic_spi_chip_write, generic_spi_chip_read}, {"M29F002B", ST_ID, ST_M29F002B, 256, 64 * 1024, probe_jedec, erase_chip_jedec, write_jedec}, {"M50FW040", ST_ID, ST_M50FW040, 512, 64 * 1024, @@ -151,23 +159,23 @@ {"M29F040B", ST_ID, ST_M29F040B, 512, 64 * 1024, probe_29f040b, erase_29f040b, write_29f040b}, {"M25P05-A", ST_ID, ST_M25P05A, 64, 256, - probe_spi, generic_spi_chip_erase_c7, generic_spi_chip_write}, + probe_spi, generic_spi_chip_erase_c7, generic_spi_chip_write, generic_spi_chip_read}, {"M25P10-A", ST_ID, ST_M25P10A, 128, 256, - probe_spi, generic_spi_chip_erase_c7, generic_spi_chip_write}, + probe_spi, generic_spi_chip_erase_c7, generic_spi_chip_write, generic_spi_chip_read}, {"M25P20", ST_ID, ST_M25P20, 256, 256, - probe_spi, generic_spi_chip_erase_c7, generic_spi_chip_write}, + probe_spi, generic_spi_chip_erase_c7, generic_spi_chip_write, generic_spi_chip_read}, {"M25P40", ST_ID, ST_M25P40, 512, 256, - probe_spi, generic_spi_chip_erase_c7, generic_spi_chip_write}, + probe_spi, generic_spi_chip_erase_c7, generic_spi_chip_write, generic_spi_chip_read}, {"M25P80", ST_ID, ST_M25P80, 1024, 256, - probe_spi, generic_spi_chip_erase_c7, generic_spi_chip_write}, + probe_spi, generic_spi_chip_erase_c7, generic_spi_chip_write, generic_spi_chip_read}, {"M25P16", ST_ID, ST_M25P16, 2048, 256, - probe_spi, generic_spi_chip_erase_c7, generic_spi_chip_write}, + probe_spi, generic_spi_chip_erase_c7, generic_spi_chip_write, generic_spi_chip_read}, {"M25P32", ST_ID, ST_M25P32, 4096, 256, - probe_spi, generic_spi_chip_erase_c7, generic_spi_chip_write}, + probe_spi, generic_spi_chip_erase_c7, generic_spi_chip_write, generic_spi_chip_read}, {"M25P64", ST_ID, ST_M25P64, 8192, 256, - probe_spi, generic_spi_chip_erase_c7, generic_spi_chip_write}, + probe_spi, generic_spi_chip_erase_c7, generic_spi_chip_write, generic_spi_chip_read}, {"M25P128", ST_ID, ST_M25P128, 16384, 256, - probe_spi, generic_spi_chip_erase_c7, generic_spi_chip_write}, + probe_spi, generic_spi_chip_erase_c7, generic_spi_chip_write, generic_spi_chip_read}, {"82802ab", 137, 173, 512, 64 * 1024, probe_82802ab, erase_82802ab, write_82802ab}, {"82802ac", 137, 172, 1024, 64 * 1024, Index: flashrom-big/spi.c =================================================================== --- flashrom-big/spi.c (Revision 3051) +++ flashrom-big/spi.c (Arbeitskopie) @@ -77,6 +77,8 @@ #define JEDEC_RDSR_BIT_WIP (0x01 << 0) uint16_t it8716f_flashport = 0; +/* use fast 33MHz SPI (<>0) or slow 16MHz (0) */ +int fast_spi=1; void generic_spi_prettyprint_status_register(struct flashchip *flash); @@ -203,10 +205,10 @@ __FUNCTION__, writecnt); return 1; } - /* Start IO, 33MHz, readcnt input bytes, writecnt output bytes. Note: + /* Start IO, 33MHz (or 16), readcnt input bytes, writecnt output bytes. Note: * We can't use writecnt directly, but have to use a strange encoding. */ - outb((0x5 << 4) | ((readcnt & 0x3) << 2) | (writeenc), port); + outb(((0x4+(fast_spi?1:0)) << 4) | ((readcnt & 0x3) << 2) | (writeenc), port); do { busy = inb(port) & 0x80; } while (busy); @@ -314,6 +316,39 @@ "%sset\n", (status & (1 << 0)) ? "" : "not "); } +/* Prettyprint the status register. Works for + * SST 25VF016 + */ +void generic_spi_prettyprint_status_register_sst25vf016(uint8_t status) +{ + const char *bpt[]={ + "none", + "1F0000H-1FFFFFH", + "1E0000H-1FFFFFH", + "1C0000H-1FFFFFH", + "180000H-1FFFFFH", + "100000H-1FFFFFH", + "all","all" + }; + printf_debug("Chip status register: Block Protect Write Disable " + "(BPL) is %sset\n", (status & (1 << 7)) ? "" : "not "); + printf_debug("Chip status register: Auto Address Increment Programming " + "(AAI) is %sset\n", (status & (1 << 6)) ? "" : "not "); + printf_debug("Chip status register: Bit 5 (BP3) is " + "%sset\n", (status & (1 << 5)) ? "" : "not "); + printf_debug("Chip status register: Bit 4 (BP2) is " + "%sset\n", (status & (1 << 4)) ? "" : "not "); + printf_debug("Chip status register: Bit 3 (BP1) is " + "%sset\n", (status & (1 << 3)) ? "" : "not "); + printf_debug("Chip status register: Bit 2 (BP0) is " + "%sset\n", (status & (1 << 2)) ? "" : "not "); + printf_debug("Resulting block protection : %s\n", bpt[(status&0x1c)>>2]); + printf_debug("Chip status register: Write Enable Latch (WEL) is " + "%sset\n", (status & (1 << 1)) ? "" : "not "); + printf_debug("Chip status register: Write In Progress (BUSY) is " + "%sset\n", (status & (1 << 0)) ? "" : "not "); +} + void generic_spi_prettyprint_status_register(struct flashchip *flash) { uint8_t status; @@ -326,6 +361,10 @@ if ((flash->model_id & 0xff00) == 0x2000) generic_spi_prettyprint_status_register_st_m25p(status); break; + case SST_ID: + if (flash->model_id == SST_25VF016B) + generic_spi_prettyprint_status_register_sst25vf016(status); + break; } } @@ -392,7 +431,7 @@ generic_spi_write_enable(); outb(0x06 , it8716f_flashport + 1); - outb((3 << 4), it8716f_flashport); + outb(((2+(fast_spi?1:0)) << 4), it8716f_flashport); for (i = 0; i < 256; i++) { bios[256 * block + i] = buf[256 * block + i]; } @@ -410,11 +449,107 @@ it8716f_spi_page_program(block, buf, bios); } +/* + * This is according the SST25VF016 datasheet, who knows it is more + * generic that this... + */ +void spi_write_status_register(int status) +{ + const unsigned char cmd[] = {0x01,(unsigned char)status}; + + /* Send WRSR (Write Status Register) */ + generic_spi_command(2, 0, cmd, NULL); + +} + +void spi_byte_program(int address, uint8_t byte) +{ + const unsigned char cmd[5] = {0x02, + (address>>16)&0xff, + (address>>8)&0xff, + (address>>0)&0xff, + byte + }; + + /* Send Byte-Program */ + generic_spi_command(5, 0, cmd, NULL); + +} + +/* + * IT8716F only allows maximum of 512 kb SPI mapped to LPC memory cycles + * Program chip using firmware cycle byte programming. (SLOW!) + */ +int over512k_spi_chip_write(struct flashchip *flash, uint8_t *buf) +{ + int total_size = 1024 * flash->total_size; + uint8_t status; + int i; + fast_spi=0; + + status = generic_spi_read_status_register(); + /* If there is block protection in effect, unprotect it first. */ + if ((status&0x3c)!=0) { + printf_debug("Some block protection in effect, disabling\n"); + generic_spi_write_enable(); + spi_write_status_register(status&~0x3c); + } + for (i=0; i<total_size; i++) { + generic_spi_write_enable(); + spi_byte_program(i,buf[i]); + //while (generic_spi_read_status_register() & JEDEC_RDSR_BIT_WIP) + myusec_delay(10); + //if (i%1024==0) fputc('b',stderr); + } + /* resume normal ops... */ + outb(0x20, it8716f_flashport); + return 0; +} + +void spi_3byte_read(int address, uint8_t *bytes, int len) +{ + const unsigned char cmd[5] = {0x03, + (address>>16)&0xff, + (address>>8)&0xff, + (address>>0)&0xff, + }; + + /* Send Read */ + generic_spi_command(4, len, cmd, bytes); + +} + +/* + * IT8716F only allows maximum of 512 kb SPI mapped to LPC memory cycles + * Need to read this big flash using firmware cycles 3 byte at a time. + */ +int generic_spi_chip_read(struct flashchip *flash, uint8_t *buf) +{ + int total_size = 1024 * flash->total_size; + int i; + fast_spi=0; + + if (total_size > 512 * 1024) { + memcpy(buf, (const char *)flash->virtual_memory, total_size); + } else { + for (i = 0; i < total_size; i+=3) { + int toread=3; + if (total_size-i < toread) toread=total_size-i; + spi_3byte_read(i, buf+i, toread); + } + } + return 0; +} + int generic_spi_chip_write(struct flashchip *flash, uint8_t *buf) { int total_size = 1024 * flash->total_size; int i; - for (i = 0; i < total_size / 256; i++) { - generic_spi_page_program(i, buf, (uint8_t *)flash->virtual_memory); + if (total_size > 512 * 1024) { + over512k_spi_chip_write(flash, buf); + } else { + for (i = 0; i < total_size / 256; i++) { + generic_spi_page_program(i, buf, (uint8_t *)flash->virtual_memory); + } } return 0; }
On 18.01.2008 03:00, Carl-Daniel Hailfinger wrote:
Nice. I have restructured your patch a bit, but I'd like to improve even further. Could you take a look at all those places where you added "const unsigned char cmd[] = XXX" and use #defines for commands like those at the top of spi.c? That would improve readability of the code.
I was unclear. I mean stuff like this: void spi_write_status_register(int status) { const unsigned char cmd[] = {0x01,(unsigned char)status}; /* Send WRSR (Write Status Register) */ generic_spi_command(2, 0, cmd, NULL); } would probably be more readable if it looked like this: void spi_write_status_register(int status) { const unsigned char cmd[] = {JEDEC_WRSR,(unsigned char)status}; /* Send WRSR (Write Status Register) */ generic_spi_command(JEDEC_WRSR_OUTSIZE, JEDEC_WRSR_INSIZE, cmd, NULL); } Regards, Carl-Daniel
OK, SiS sent me this nice board with some 2 MB flash parts and I have not gotten much past step 1, "Program the flash", as it is a 2 MB flash. I had no idea what a mess the superio/flash interface was going to be. So, Ronald, I am most interested in your patch, hope it is ready soon! And, well, seems we have a problem with 2 MB parts. Ouch. I guess what I will do for now is use 512KB of the 2 MB part (i.e. program it with 4 512KB images). I hate to say this, and I agree with the desire to remain clean in v3, but we may have to find a way to deal with this mess. I hope not. It is hard to believe how brain-dead PC hardware can be, given that the same mistakes and lessons are learned (and forgotten) over and over again. Too bad. Anyway I am ready to test the patch tomorrow morning, I have a 1/2 vacation day and intend to put it to good use ... ron
On Jan 17, 2008 2:32 PM, Ronald Hoogenboom <ronald@zonnet.nl> wrote:
Now I'm looking at how to make the rom_stream read the flash chip like in the over512k_read_chip. But I'm a bit stuck on the overal mechanics of the initialization process and how to get hold of the assigned io port for the SPI serial flash controller in the IT8716. This is supposed to be configured by the GPIO Config registers (LDN 07) at index 0x64 and 0x65, but in the pnp_dev_info array in it8716f/superio.c, there is nothing listed after IT8716F_GPIO, while at other indexes, there is. Does this mean that the superio.c file fails to configure this io port?
yes, it means that the port is not configured. There is a lot of TODO in this file. It may be time to fill it in.
Suppose that this port IS configured correcly, what means should my rom_stream use to retrieve this port address?
I think you make a new stream type, let's called it bad_stream, that knows how to handle the addresses. The bad_stream would know that when the address is in a certain range, i.e. below the address (0-512KB), it will use I/O, and when it is above, it can use memory access. So in src/stream/Config.lb you add something like this: uses CONFIG_BADROM_PAYLOAD if CONFIG_BADROM_PAYLOAD object badrom_stream.o end
Is there some doc about these pnp functions?
no, sorry. We are doing better in v3 ... we know the lack of docs is a problem.
Suppose I've figured it all out and created my new rom_stream_over512k (or so) object, how do I mingle it into the Config.lb files, so it will automatically get chosen when a ROM_SIZE of over 512k is selected in the target configuration (AND the superio is an IT8716F).
I think you make those boards always use bad_stream (or whatever) in all cases, and not try to get the config tool to pick different types, as in the example above. But, it might be possible to do this: if CONFIG_BADROM_PAYLOAD if ROM_SIZE > 512*1024 object badrom_stream.o else object rom_stream.o end end Something like this might work. The it8761f is NOT the worst thing we have seen, it's just really, really bad. So I guess we have to put in another kludge for badly design chipsets. It's not the first time :-)
In the targets/gigabyte/m57sli directory, there is a Config.lb.kernel file, which I used as a base for my build, but it only uses a 'fallback' and a 'failover' image. Why no 'normal' one? What is supposed to be the purpose of each image? (You see, I'm really missing some key parts of the big picture...)
you're not missing anything. The picture, which was once, years ago, pretty clean, has gotten grafitti-tagged by many authors and the necessities of strange hardware. This will make no sense, but I'll try. Fallback is the image you can ALWAYS use, even when normal does not work. Normal is an image you can use unless something breaks, and you use fallback. So, what's the one image you must always have? fallback. What's the optional image? normal. So, when there is only going to be one image, it's called fallback. What people don't realize is that the images can have any name, so, for instance, on single image ROMs, they should really not use the name 'fallback' -- there is nothing sacred about the names. 'single' would do. failover -- that snuck in at some point and I think YH Lu understands why. This is a bit messy as the guys who first put the fallback/normal in never intended for bioses to be built WITHOUT fallback/normal. I relied heavily on the fallback stuff at LANL but have since hardly used it. Not all people think it is needed -- OLPC for example has explicitly rejected a need for a fallback bios image. It's an open question as to whether fallback bios images have real value for people. Let me know how your new stream code goes as I am going to need it, I think. ron
ron minnich wrote:
On Jan 17, 2008 2:32 PM, Ronald Hoogenboom <ronald@zonnet.nl> wrote:
Now I'm looking at how to make the rom_stream read the flash chip like in the over512k_read_chip. But I'm a bit stuck on the overal mechanics of the initialization process and how to get hold of the assigned io port for the SPI serial flash controller in the IT8716. This is supposed to be configured by the GPIO Config registers (LDN 07) at index 0x64 and 0x65, but in the pnp_dev_info array in it8716f/superio.c, there is nothing listed after IT8716F_GPIO, while at other indexes, there is. Does this mean that the superio.c file fails to configure this io port?
yes, it means that the port is not configured. There is a lot of TODO in this file. It may be time to fill it in.
Suppose that this port IS configured correcly, what means should my rom_stream use to retrieve this port address?
I think you make a new stream type, let's called it bad_stream, that knows how to handle the addresses. The bad_stream would know that when the address is in a certain range, i.e. below the address (0-512KB), it will use I/O, and when it is above, it can use memory access.
So in src/stream/Config.lb you add something like this:
uses CONFIG_BADROM_PAYLOAD
if CONFIG_BADROM_PAYLOAD object badrom_stream.o end
Is there some doc about these pnp functions?
no, sorry. We are doing better in v3 ... we know the lack of docs is a problem.
Suppose I've figured it all out and created my new rom_stream_over512k (or so) object, how do I mingle it into the Config.lb files, so it will automatically get chosen when a ROM_SIZE of over 512k is selected in the target configuration (AND the superio is an IT8716F).
I think you make those boards always use bad_stream (or whatever) in all cases, and not try to get the config tool to pick different types, as in the example above.
But, it might be possible to do this:
if CONFIG_BADROM_PAYLOAD if ROM_SIZE > 512*1024 object badrom_stream.o else object rom_stream.o end end
Something like this might work.
The it8761f is NOT the worst thing we have seen, it's just really, really bad.
So I guess we have to put in another kludge for badly design chipsets. It's not the first time :-)
In the targets/gigabyte/m57sli directory, there is a Config.lb.kernel file, which I used as a base for my build, but it only uses a 'fallback' and a 'failover' image. Why no 'normal' one? What is supposed to be the purpose of each image? (You see, I'm really missing some key parts of the big picture...)
you're not missing anything. The picture, which was once, years ago, pretty clean, has gotten grafitti-tagged by many authors and the necessities of strange hardware.
This will make no sense, but I'll try. Fallback is the image you can ALWAYS use, even when normal does not work. Normal is an image you can use unless something breaks, and you use fallback.
So, what's the one image you must always have? fallback. What's the optional image? normal. So, when there is only going to be one image, it's called fallback.
What people don't realize is that the images can have any name, so, for instance, on single image ROMs, they should really not use the name 'fallback' -- there is nothing sacred about the names. 'single' would do.
failover -- that snuck in at some point and I think YH Lu understands why.
This is a bit messy as the guys who first put the fallback/normal in never intended for bioses to be built WITHOUT fallback/normal. I relied heavily on the fallback stuff at LANL but have since hardly used it. Not all people think it is needed -- OLPC for example has explicitly rejected a need for a fallback bios image. It's an open question as to whether fallback bios images have real value for people.
Seems like this might be a good time to raise this question: what exactly is the difference between normal and fallback? I know part of the answer, but I don't think I know all of it. -Corey
On 18.01.2008 08:17, ron minnich wrote:
OK, SiS sent me this nice board with some 2 MB flash parts and I have not gotten much past step 1, "Program the flash", as it is a 2 MB flash. I had no idea what a mess the superio/flash interface was going to be.
So, Ronald, I am most interested in your patch, hope it is ready soon!
I think one or two more iterations and we should be ready to go.
And, well, seems we have a problem with 2 MB parts. Ouch. I guess what I will do for now is use 512KB of the 2 MB part (i.e. program it with 4 512KB images).
For now. We could certainly enable ROM shadowing in RAM if we can make some RAM appear directly below 4 GB.
I hate to say this, and I agree with the desire to remain clean in v3, but we may have to find a way to deal with this mess. I hope not. It is hard to believe how brain-dead PC hardware can be, given that the same mistakes and lessons are learned (and forgotten) over and over again. Too bad.
I have a mad scheme which would work for v3 without downsides (except for the fact that the boot block and initram have to be in the upper 512 kB, but that's the case anyway even without my scheme). Basically, we create a flat LAR-in-LAR structure. Before you start to barf, listen: Instead of storing real flash size at the top of ROM, we store a virtual size which corresponds to the start of the first LAR member in the top 512 kB block. This virtual size is what all LAR parsing uses until CAR has been disabled. Then we shadow the entire ROM somewhere in RAM and further accesses to the ROM will be directed there, including all LAR functions.
Anyway I am ready to test the patch tomorrow morning, I have a 1/2 vacation day and intend to put it to good use ...
Can you test the modified variant I posted as a reply to Ronald? It should work as well and fit a little better into flashrom structure. Regards, Carl-Daniel
On Jan 18, 2008 6:49 AM, Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@gmx.net> wrote:
I have a mad scheme which would work for v3 without downsides (except for the fact that the boot block and initram have to be in the upper 512 kB, but that's the case anyway even without my scheme). Basically, we create a flat LAR-in-LAR structure. Before you start to barf, listen: Instead of storing real flash size at the top of ROM, we store a virtual size which corresponds to the start of the first LAR member in the top 512 kB block. This virtual size is what all LAR parsing uses until CAR has been disabled. Then we shadow the entire ROM somewhere in RAM and further accesses to the ROM will be directed there, including all LAR functions.
That's fine, it's very simliar to what I had to do for the brain-dead chip that had a 64K hole in FLASH at (0-64K) -- had to shadow it at 0x2000000. We're going to see this problem again and again, we might as well figure it out now.
Can you test the modified variant I posted as a reply to Ronald? It should work as well and fit a little better into flashrom structure.
The top 512 flash fine, the lower 1.5M did not flash, but did erase, maybe. hard to say if readback worked. coreboot appears to not have booted, sadly. ron
On Fri, 2008-01-18 at 03:00 +0100, Carl-Daniel Hailfinger wrote:
On 17.01.2008 22:25, Ronald Hoogenboom wrote: ...
the limited io transfer length options of the IT8716). It might be more appropriate to have an 'over512k_page_write' for more speed with chips that do support that. Also the byte program wait time is hard wired to 10 us (following the 25vf016b datasheet). And the block protect enablement might also NOT be so generic.
By the way, the lack of block-write in the data sheet does not mean it will not work with the real chip.
No, it doesn't work! Way back when the mmapped mode was stil in flashboot ... then the program only wrote a few bytes here and there in the area > 0x180000, so this doesn't work properly, like the datasheet sais. Not all datasheets are false...
I'll have a look if it still works after your rework to the patch. Ronald.
OK, I've checked it again and fixed a few things, see the read function and the erase also has the block protect disable, and it works again. It takes just under a minute to flash the whole chip, which is still quite acceptable, I would say. It took WAY longer when the byte write was timed by usleep instead of the myusec_delay. Are there any more chips that need the block protect disable? Are these block protect bits always at the same position in the status register? Signed-off-by: Ronald Hoogenboom <hoogenboom30@zonnet.nl> ---
On Fri, 2008-01-18 at 03:13 +0100, Carl-Daniel Hailfinger wrote:
I was unclear. I mean stuff like this:
void spi_write_status_register(int status) { const unsigned char cmd[] = {0x01,(unsigned char)status};
would probably be more readable if it looked like this:
void spi_write_status_register(int status) { const unsigned char cmd[] = {JEDEC_WRSR,(unsigned char)status}; /* Send WRSR (Write Status Register) */ generic_spi_command(JEDEC_WRSR_OUTSIZE, JEDEC_WRSR_INSIZE, cmd, NULL); }
I think this is a matter of taste... But if the style is like this, I will comply. So I changed my patch again to do exactly the same, but with a few JEDEC_... defines.
Regards, Carl-Daniel
Regards, Ronald Signed-off-by: Ronald Hoogenboom <hoogenboom30@zonnet.nl> ---
Thanks for the explanations, however vague ;-). I thought I was the only one wandering in the dark.. On Thu, 2008-01-17 at 23:38 -0800, ron minnich wrote:
other indexes, there is. Does this mean that the superio.c file fails to
configure this io port?
yes, it means that the port is not configured. There is a lot of TODO in this file. It may be time to fill it in.
Might this also be the cause of the NVIDIA X11 driver not working yet? Can you just give me a push in the right direction how to 'fill it in'? How can I configure the SPI io port in LDN 07, index 0x64 and 0x65 and what io address to configure it to? Can I just pick 0x820 like the vendor bios does? Or do I ask it to the pnp functions (I guess)? What pnp function to use? Is there some more complete example superio chip that does similar things to do the copycat monkey-learns-trick? Hope the 1/2 day off was worth it... Ronald.
On Fri, 2008-01-18 at 09:39 -0800, ron minnich wrote:
On Jan 18, 2008 6:49 AM, Carl-Daniel Hailfinger
should work as well and fit a little better into flashrom structure.
The top 512 flash fine, the lower 1.5M did not flash, but did erase, maybe. hard to say if readback worked.
That is because the test for 512k+ was the wrong way around in the read function. Non-mapped memory also reads as ff (same as erased) using mmap method. The erase also needed the Block Protection to be disabled, on power-up the Block protection defaults to on (but, maybe not with the chips on your board.. What type?). See my re-worked and re-re-worked patches.
coreboot appears to not have booted, sadly.
ron
On 18.01.2008 21:09, Ronald Hoogenboom wrote:
OK, I've checked it again and fixed a few things, see the read function and the erase also has the block protect disable, and it works again.
Thanks! It seems I was half asleep while coding the if (size > 512k) part.
It takes just under a minute to flash the whole chip, which is still quite acceptable, I would say. It took WAY longer when the byte write was timed by usleep instead of the myusec_delay.
Yes, speed was not the primary concern when developing the code, it was mostly about getting the code into a working state with maximum adherence to the datasheets.
Are there any more chips that need the block protect disable? Are these block protect bits always at the same position in the status register?
As far as I know, almost all chips have the block protect bits in the same place in the status register. I can verify that later with some of the data sheets on my disk. Regards, Carl-Daniel
On Wed, Jan 16, 2008 at 10:34:29PM -0500, Ward Vandewege wrote:
Winbond W25X16VSSI Winbond W25X32VSSI
Problem with Winbond is they really only want to sell large quantities. I haven't even gotten a quote and delivery time back from my sales rep on a request I made in November.
What about Winbond W25X80VSSIG ? That's an 8Mbit chip which is avaible in small quantities. Thanks, Ward. -- Ward Vandewege <ward@fsf.org> Free Software Foundation - Senior System Administrator
On 18.01.2008 21:48, Ronald Hoogenboom wrote:
I think this is a matter of taste... But if the style is like this, I will comply. So I changed my patch again to do exactly the same, but with a few JEDEC_... defines.
Signed-off-by: Ronald Hoogenboom <hoogenboom30@zonnet.nl>
Thanks for reworking the code! I have factored out some common status registers to duplicate less code and hope the code still works. Could you please review it and tell me what you think? Ron (Minnich), can you test with your GA-2761GXDK? Regards, Carl-Daniel ------------------- Support SPI flash chips bigger than 512 kByte sitting behind IT8716F Super I/O performing LPC-to-SPI flash translation. Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@gmx.net> Signed-off-by: Ronald Hoogenboom <hoogenboom30@zonnet.nl> Index: flashrom-big/flash.h =================================================================== --- flashrom-big/flash.h (Revision 3051) +++ flashrom-big/flash.h (Arbeitskopie) @@ -234,7 +234,16 @@ #define TI_ID 0x97 /* Texas Instruments */ +/* + * W25X chips are SPI, first byte of device ID is memory type, second + * byte of device ID is related to log(bitsize). + */ #define WINBOND_ID 0xDA /* Winbond */ +#define WINBOND_NEX_ID 0xEF /* Winbond (ex Nexcom) serial flash devices */ +#define W_25X10 0x3011 +#define W_25X20 0x3012 +#define W_25X40 0x3013 +#define W_25X80 0x3014 #define W_29C011 0xC1 #define W_29C020C 0x45 #define W_29C040P 0x46 @@ -297,6 +306,7 @@ void generic_spi_write_disable(); int generic_spi_chip_erase_c7(struct flashchip *flash); int generic_spi_chip_write(struct flashchip *flash, uint8_t *buf); +int generic_spi_chip_read(struct flashchip *flash, uint8_t *buf); /* 82802ab.c */ int probe_82802ab(struct flashchip *flash); Index: flashrom-big/flashchips.c =================================================================== --- flashrom-big/flashchips.c (Revision 3051) +++ flashrom-big/flashchips.c (Arbeitskopie) @@ -51,13 +51,13 @@ {"MX29F002", MX_ID, MX_29F002, 256, 64 * 1024, probe_29f002, erase_29f002, write_29f002}, {"MX25L4005", MX_ID, MX_25L4005, 512, 256, - probe_spi, generic_spi_chip_erase_c7, generic_spi_chip_write}, + probe_spi, generic_spi_chip_erase_c7, generic_spi_chip_write, generic_spi_chip_read}, {"MX25L8005", MX_ID, MX_25L8005, 1024, 256, - probe_spi, generic_spi_chip_erase_c7, generic_spi_chip_write}, + probe_spi, generic_spi_chip_erase_c7, generic_spi_chip_write, generic_spi_chip_read}, {"SST25VF040B", SST_ID, SST_25VF040B, 512, 256, - probe_spi, generic_spi_chip_erase_c7, generic_spi_chip_write}, + probe_spi, generic_spi_chip_erase_c7, generic_spi_chip_write, generic_spi_chip_read}, {"SST25VF016B", SST_ID, SST_25VF016B, 2048, 256, - probe_spi, generic_spi_chip_erase_c7, generic_spi_chip_write}, + probe_spi, generic_spi_chip_erase_c7, generic_spi_chip_write, generic_spi_chip_read}, {"SST29EE020A", SST_ID, SST_29EE020A, 256, 128, probe_jedec, erase_chip_jedec, write_jedec}, {"SST28SF040A", SST_ID, SST_28SF040, 512, 256, @@ -122,6 +122,14 @@ probe_jedec, erase_chip_jedec, write_39sf020}, {"W39V080A", WINBOND_ID, W_39V080A, 1024, 64*1024, probe_jedec, erase_chip_jedec, write_39sf020}, + {"W25x10", WINBOND_NEX_ID, W_25X10, 128, 256, + probe_spi, generic_spi_chip_erase_c7, generic_spi_chip_write, generic_spi_chip_read}, + {"W25x20", WINBOND_NEX_ID, W_25X20, 256, 256, + probe_spi, generic_spi_chip_erase_c7, generic_spi_chip_write, generic_spi_chip_read}, + {"W25x40", WINBOND_NEX_ID, W_25X40, 512, 256, + probe_spi, generic_spi_chip_erase_c7, generic_spi_chip_write, generic_spi_chip_read}, + {"W25x80", WINBOND_NEX_ID, W_25X80, 1024, 256, + probe_spi, generic_spi_chip_erase_c7, generic_spi_chip_write, generic_spi_chip_read}, {"M29F002B", ST_ID, ST_M29F002B, 256, 64 * 1024, probe_jedec, erase_chip_jedec, write_jedec}, {"M50FW040", ST_ID, ST_M50FW040, 512, 64 * 1024, @@ -151,23 +159,23 @@ {"M29F040B", ST_ID, ST_M29F040B, 512, 64 * 1024, probe_29f040b, erase_29f040b, write_29f040b}, {"M25P05-A", ST_ID, ST_M25P05A, 64, 256, - probe_spi, generic_spi_chip_erase_c7, generic_spi_chip_write}, + probe_spi, generic_spi_chip_erase_c7, generic_spi_chip_write, generic_spi_chip_read}, {"M25P10-A", ST_ID, ST_M25P10A, 128, 256, - probe_spi, generic_spi_chip_erase_c7, generic_spi_chip_write}, + probe_spi, generic_spi_chip_erase_c7, generic_spi_chip_write, generic_spi_chip_read}, {"M25P20", ST_ID, ST_M25P20, 256, 256, - probe_spi, generic_spi_chip_erase_c7, generic_spi_chip_write}, + probe_spi, generic_spi_chip_erase_c7, generic_spi_chip_write, generic_spi_chip_read}, {"M25P40", ST_ID, ST_M25P40, 512, 256, - probe_spi, generic_spi_chip_erase_c7, generic_spi_chip_write}, + probe_spi, generic_spi_chip_erase_c7, generic_spi_chip_write, generic_spi_chip_read}, {"M25P80", ST_ID, ST_M25P80, 1024, 256, - probe_spi, generic_spi_chip_erase_c7, generic_spi_chip_write}, + probe_spi, generic_spi_chip_erase_c7, generic_spi_chip_write, generic_spi_chip_read}, {"M25P16", ST_ID, ST_M25P16, 2048, 256, - probe_spi, generic_spi_chip_erase_c7, generic_spi_chip_write}, + probe_spi, generic_spi_chip_erase_c7, generic_spi_chip_write, generic_spi_chip_read}, {"M25P32", ST_ID, ST_M25P32, 4096, 256, - probe_spi, generic_spi_chip_erase_c7, generic_spi_chip_write}, + probe_spi, generic_spi_chip_erase_c7, generic_spi_chip_write, generic_spi_chip_read}, {"M25P64", ST_ID, ST_M25P64, 8192, 256, - probe_spi, generic_spi_chip_erase_c7, generic_spi_chip_write}, + probe_spi, generic_spi_chip_erase_c7, generic_spi_chip_write, generic_spi_chip_read}, {"M25P128", ST_ID, ST_M25P128, 16384, 256, - probe_spi, generic_spi_chip_erase_c7, generic_spi_chip_write}, + probe_spi, generic_spi_chip_erase_c7, generic_spi_chip_write, generic_spi_chip_read}, {"82802ab", 137, 173, 512, 64 * 1024, probe_82802ab, erase_82802ab, write_82802ab}, {"82802ac", 137, 172, 1024, 64 * 1024, Index: flashrom-big/spi.c =================================================================== --- flashrom-big/spi.c (Revision 3051) +++ flashrom-big/spi.c (Arbeitskopie) @@ -76,9 +76,27 @@ #define JEDEC_RDSR_INSIZE 0x01 #define JEDEC_RDSR_BIT_WIP (0x01 << 0) +/* Write Status Register */ +#define JEDEC_WRSR 0x01 +#define JEDEC_WRSR_OUTSIZE 0x02 +#define JEDEC_WRSR_INSIZE 0x00 + +/* Read the memory */ +#define JEDEC_READ 0x03 +#define JEDEC_READ_OUTSIZE 0x04 +/* JEDEC_READ_INSIZE : any length */ + +/* Write memory byte */ +#define JEDEC_BYTE_PROGRAM 0x02 +#define JEDEC_BYTE_PROGRAM_OUTSIZE 0x05 +#define JEDEC_BYTE_PROGRAM_INSIZE 0x00 + uint16_t it8716f_flashport = 0; +/* use fast 33MHz SPI (<>0) or slow 16MHz (0) */ +int fast_spi=1; void generic_spi_prettyprint_status_register(struct flashchip *flash); +void spi_disable_blockprotect(void); /* Generic Super I/O helper functions */ uint8_t regval(uint16_t port, uint8_t reg) @@ -203,10 +221,10 @@ __FUNCTION__, writecnt); return 1; } - /* Start IO, 33MHz, readcnt input bytes, writecnt output bytes. Note: + /* Start IO, 33MHz (or 16), readcnt input bytes, writecnt output bytes. Note: * We can't use writecnt directly, but have to use a strange encoding. */ - outb((0x5 << 4) | ((readcnt & 0x3) << 2) | (writeenc), port); + outb(((0x4+(fast_spi?1:0)) << 4) | ((readcnt & 0x3) << 2) | (writeenc), port); do { busy = inb(port) & 0x80; } while (busy); @@ -290,6 +308,24 @@ return readarr[0]; } +/* Prettyprint the status register. Common definitions. + */ +void generic_spi_prettyprint_status_register_common(uint8_t status) +{ + printf_debug("Chip status register: Bit 5 / Block Protect 3 (BP3) is " + "%sset\n", (status & (1 << 5)) ? "" : "not "); + printf_debug("Chip status register: Bit 4 / Block Protect 2 (BP2) is " + "%sset\n", (status & (1 << 4)) ? "" : "not "); + printf_debug("Chip status register: Bit 3 / Block Protect 1 (BP1) is " + "%sset\n", (status & (1 << 3)) ? "" : "not "); + printf_debug("Chip status register: Bit 2 / Block Protect 0 (BP0) is " + "%sset\n", (status & (1 << 2)) ? "" : "not "); + printf_debug("Chip status register: Write Enable Latch (WEL) is " + "%sset\n", (status & (1 << 1)) ? "" : "not "); + printf_debug("Chip status register: Write In Progress (WIP/BUSY) is " + "%sset\n", (status & (1 << 0)) ? "" : "not "); +} + /* Prettyprint the status register. Works for * ST M25P series * MX MX25L series @@ -300,20 +336,31 @@ "(SRWD) is %sset\n", (status & (1 << 7)) ? "" : "not "); printf_debug("Chip status register: Bit 6 is " "%sset\n", (status & (1 << 6)) ? "" : "not "); - printf_debug("Chip status register: Bit 5 is " - "%sset\n", (status & (1 << 5)) ? "" : "not "); - printf_debug("Chip status register: Block Protect 2 (BP2) is " - "%sset\n", (status & (1 << 4)) ? "" : "not "); - printf_debug("Chip status register: Block Protect 1 (BP1) is " - "%sset\n", (status & (1 << 3)) ? "" : "not "); - printf_debug("Chip status register: Block Protect 0 (BP0) is " - "%sset\n", (status & (1 << 2)) ? "" : "not "); - printf_debug("Chip status register: Write Enable Latch (WEL) is " - "%sset\n", (status & (1 << 1)) ? "" : "not "); - printf_debug("Chip status register: Write In Progress (WIP) is " - "%sset\n", (status & (1 << 0)) ? "" : "not "); + generic_spi_prettyprint_status_register_common(status); } +/* Prettyprint the status register. Works for + * SST 25VF016 + */ +void generic_spi_prettyprint_status_register_sst25vf016(uint8_t status) +{ + const char *bpt[]={ + "none", + "1F0000H-1FFFFFH", + "1E0000H-1FFFFFH", + "1C0000H-1FFFFFH", + "180000H-1FFFFFH", + "100000H-1FFFFFH", + "all","all" + }; + printf_debug("Chip status register: Block Protect Write Disable " + "(BPL) is %sset\n", (status & (1 << 7)) ? "" : "not "); + printf_debug("Chip status register: Auto Address Increment Programming " + "(AAI) is %sset\n", (status & (1 << 6)) ? "" : "not "); + generic_spi_prettyprint_status_register_common(status); + printf_debug("Resulting block protection : %s\n", bpt[(status&0x1c)>>2]); +} + void generic_spi_prettyprint_status_register(struct flashchip *flash) { uint8_t status; @@ -326,6 +373,10 @@ if ((flash->model_id & 0xff00) == 0x2000) generic_spi_prettyprint_status_register_st_m25p(status); break; + case SST_ID: + if (flash->model_id == SST_25VF016B) + generic_spi_prettyprint_status_register_sst25vf016(status); + break; } } @@ -333,6 +384,7 @@ { const unsigned char cmd[] = JEDEC_CE_C7; + spi_disable_blockprotect(); generic_spi_write_enable(); /* Send CE (Chip Erase) */ generic_spi_command(JEDEC_CE_C7_OUTSIZE, JEDEC_CE_C7_INSIZE, cmd, NULL); @@ -392,7 +444,7 @@ generic_spi_write_enable(); outb(0x06 , it8716f_flashport + 1); - outb((3 << 4), it8716f_flashport); + outb(((2+(fast_spi?1:0)) << 4), it8716f_flashport); for (i = 0; i < 256; i++) { bios[256 * block + i] = buf[256 * block + i]; } @@ -410,11 +462,113 @@ it8716f_spi_page_program(block, buf, bios); } +/* + * This is according the SST25VF016 datasheet, who knows it is more + * generic that this... + */ +void spi_write_status_register(int status) +{ + const unsigned char cmd[] = {JEDEC_WRSR,(unsigned char)status}; + + /* Send WRSR (Write Status Register) */ + generic_spi_command(JEDEC_WRSR_OUTSIZE, JEDEC_WRSR_INSIZE, cmd, NULL); + +} + +void spi_byte_program(int address, uint8_t byte) +{ + const unsigned char cmd[JEDEC_BYTE_PROGRAM_OUTSIZE] = {JEDEC_BYTE_PROGRAM, + (address>>16)&0xff, + (address>>8)&0xff, + (address>>0)&0xff, + byte + }; + + /* Send Byte-Program */ + generic_spi_command(JEDEC_BYTE_PROGRAM_OUTSIZE, JEDEC_BYTE_PROGRAM_INSIZE, cmd, NULL); + +} + +void spi_disable_blockprotect(void) +{ + uint8_t status; + + status = generic_spi_read_status_register(); + /* If there is block protection in effect, unprotect it first. */ + if ((status & 0x3c) != 0) { + printf_debug("Some block protection in effect, disabling\n"); + generic_spi_write_enable(); + spi_write_status_register(status & ~0x3c); + } +} + +/* + * IT8716F only allows maximum of 512 kb SPI mapped to LPC memory cycles + * Program chip using firmware cycle byte programming. (SLOW!) + */ +int it8716f_over512k_spi_chip_write(struct flashchip *flash, uint8_t *buf) +{ + int total_size = 1024 * flash->total_size; + int i; + fast_spi=0; + + spi_disable_blockprotect(); + for (i=0; i<total_size; i++) { + generic_spi_write_enable(); + spi_byte_program(i,buf[i]); + //while (generic_spi_read_status_register() & JEDEC_RDSR_BIT_WIP) + myusec_delay(10); + //if (i%1024==0) fputc('b',stderr); + } + /* resume normal ops... */ + outb(0x20, it8716f_flashport); + return 0; +} + +void spi_3byte_read(int address, uint8_t *bytes, int len) +{ + const unsigned char cmd[JEDEC_READ_OUTSIZE] = {JEDEC_READ, + (address>>16)&0xff, + (address>>8)&0xff, + (address>>0)&0xff, + }; + + /* Send Read */ + generic_spi_command(JEDEC_READ_OUTSIZE, len, cmd, bytes); + +} + +/* + * IT8716F only allows maximum of 512 kb SPI mapped to LPC memory cycles + * Need to read this big flash using firmware cycles 3 byte at a time. + */ +int generic_spi_chip_read(struct flashchip *flash, uint8_t *buf) +{ + int total_size = 1024 * flash->total_size; + int i; + fast_spi=0; + + if (total_size > 512 * 1024) { + for (i = 0; i < total_size; i+=3) { + int toread=3; + if (total_size-i < toread) toread=total_size-i; + spi_3byte_read(i, buf+i, toread); + } + } else { + memcpy(buf, (const char *)flash->virtual_memory, total_size); + } + return 0; +} + int generic_spi_chip_write(struct flashchip *flash, uint8_t *buf) { int total_size = 1024 * flash->total_size; int i; - for (i = 0; i < total_size / 256; i++) { - generic_spi_page_program(i, buf, (uint8_t *)flash->virtual_memory); + if (total_size > 512 * 1024) { + it8716f_over512k_spi_chip_write(flash, buf); + } else { + for (i = 0; i < total_size / 256; i++) { + generic_spi_page_program(i, buf, (uint8_t *)flash->virtual_memory); + } } return 0; }
On Fri, 2008-01-18 at 23:31 +0100, Carl-Daniel Hailfinger wrote:
Thanks for reworking the code! I have factored out some common status registers to duplicate less code and hope the code still works. Could you please review it and tell me what you think?
Fine by me, works like a charm, the way I meant it to work ;-). Ronald.
On 19.01.2008 00:17, Ronald Hoogenboom wrote:
On Fri, 2008-01-18 at 23:31 +0100, Carl-Daniel Hailfinger wrote:
Thanks for reworking the code! I have factored out some common status registers to duplicate less code and hope the code still works. Could you please review it and tell me what you think?
Fine by me, works like a charm, the way I meant it to work ;-).
Thanks, checked in and added you as copyright holder. Revision 3061. Regards, Carl-Daniel
On 19.01.2008 01:08, Carl-Daniel Hailfinger wrote:
On 19.01.2008 00:17, Ronald Hoogenboom wrote:
On Fri, 2008-01-18 at 23:31 +0100, Carl-Daniel Hailfinger wrote:
Thanks for reworking the code! I have factored out some common status registers to duplicate less code and hope the code still works. Could you please review it and tell me what you think?
Fine by me, works like a charm, the way I meant it to work ;-).
Thanks, checked in and added you as copyright holder. Revision 3061.
What about the following patch on top of it? Make sure we delay writing the next byte long enough in SPI byte programming. This could even result in a speedup. Print a "b" for every kByte written to flash. Minor formatting changes. Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@gmx.net> Index: spi.c =================================================================== --- spi.c (Revision 3061) +++ spi.c (Arbeitskopie) @@ -516,12 +516,10 @@ for (i=0; i<total_size; i++) { generic_spi_write_enable(); spi_byte_program(i,buf[i]); - /* FIXME: We really should read the status register and delay - * accordingly. - */ - //while (generic_spi_read_status_register() & JEDEC_RDSR_BIT_WIP) + while (generic_spi_read_status_register() & JEDEC_RDSR_BIT_WIP) myusec_delay(10); - //if (i%1024==0) fputc('b',stderr); + if (i % 1024 == 0) + printf("b"); } /* resume normal ops... */ outb(0x20, it8716f_flashport); @@ -553,7 +551,8 @@ if (total_size > 512 * 1024) { for (i = 0; i < total_size; i+=3) { int toread=3; - if (total_size-i < toread) toread=total_size-i; + if (total_size - i < toread) + toread = total_size - i; spi_3byte_read(i, buf+i, toread); } } else { Regards, Carl-Daniel
On Sat, Jan 19, 2008 at 01:18:11AM +0100, Carl-Daniel Hailfinger wrote:
+ while (generic_spi_read_status_register() & JEDEC_RDSR_BIT_WIP) myusec_delay(10);
Indent this properly, please. //Peter
On 19.01.2008 01:23, Peter Stuge wrote:
On Sat, Jan 19, 2008 at 01:18:11AM +0100, Carl-Daniel Hailfinger wrote:
+ while (generic_spi_read_status_register() & JEDEC_RDSR_BIT_WIP) myusec_delay(10);
Indent this properly, please.
Will be done in the commit. The patch I posted was excluding whitespace changes to facilitate review. Regards, Carl-Daniel
Carl-Daniel Hailfinger wrote:
*/ - //while (generic_spi_read_status_register() & JEDEC_RDSR_BIT_WIP) + while (generic_spi_read_status_register() & JEDEC_RDSR_BIT_WIP) myusec_delay(10); - //if (i%1024==0) fputc('b',stderr); + if (i % 1024 == 0) + printf("b"); }
There is a problem with using printf like this and that is: stdout is line buffered. The 'b''s won't show until a newline is output or fflush is called. That's why I used fputc initially. If you do this with the 'pio mode' writing, you should do this for all methods an all chips for constistent behaviour, so that is why I commented it out initially. Ronald.
On 19.01.2008 19:34, Ronald Hoogenboom wrote:
Carl-Daniel Hailfinger wrote:
*/ - //while (generic_spi_read_status_register() & JEDEC_RDSR_BIT_WIP) + while (generic_spi_read_status_register() & JEDEC_RDSR_BIT_WIP) myusec_delay(10); - //if (i%1024==0) fputc('b',stderr); + if (i % 1024 == 0) + printf("b"); }
There is a problem with using printf like this and that is: stdout is line buffered. The 'b''s won't show until a newline is output or fflush is called. That's why I used fputc initially. If you do this with the 'pio mode' writing, you should do this for all methods an all chips for constistent behaviour, so that is why I commented it out initially.
Ah OK. I'll drop that part. Did you have time to test with the while (generic_spi_read_status_register() & .._WIP) enabled? It would be more correct to have it enabled. Regards, Carl-Daniel
On 18.01.2008 23:10, Ward Vandewege wrote:
On Wed, Jan 16, 2008 at 10:34:29PM -0500, Ward Vandewege wrote:
Winbond W25X16VSSI Winbond W25X32VSSI
Problem with Winbond is they really only want to sell large quantities. I haven't even gotten a quote and delivery time back from my sales rep on a request I made in November.
What about
Winbond W25X80VSSIG
? That's an 8Mbit chip which is avaible in small quantities.
Ronald Hoogenboom wrote that IT8716F startup clocks SPI at 16 MHz, so almost any SPI chip should work. We just have to make sure we don't accidentially change the speed of the IT8716F (which will happen if you simply run flashrom). So it is a case of "should work" for slower flash chips right now. Changing that to "will work even if flashrom is invoked" requires some flashrom surgery. For the Winbond W25X80VSSIG everything should work fine. It is fast enough. Regards, Carl-Daniel
Carl-Daniel Hailfinger wrote:
Did you have time to test with the while (generic_spi_read_status_register() & .._WIP) enabled? It would be more correct to have it enabled.
I had that in initially (that's why it was there, but commented out) but all it got me was slower programming times and the datasheet also suggests it is ok to just do a timed wait. But in the face of genericness for other chips, it is indeed more correct to do the check for the busy bit. The read status register will take at least: 16*(2/33) us = about 1 us (excluding the LPC latency, which is....?), so assuming that the first read status will show busy and the second will show ready, it is only 2 us slower: ~20%, so that should be acceptable. I'll do some checks on the hardware shortly.
Regards, Carl-Daniel
Regards back, Ronald.
On Sun, 2008-01-20 at 11:59 +0100, Ronald Hoogenboom wrote:
The read status register will take at least: 16*(2/33) us = about 1 us (excluding the LPC latency, which is....?), so assuming that the first read status will show busy and the second will show ready, it is only 2 us slower: ~20%, so that should be acceptable. I'll do some checks on the hardware shortly.
I've done the tests and the programming result (eg. the data read back from the flash) is correct with and without the status reading. With status reading, programming takes: 1'15 ~ 1'50 Without status reading, programming takes: 0'50 ~ 0'55 There is quite some variance in the time taken extra by reading the status, the lower bound is about the expected penalty (as calculated above), but the higher bound is not so easily explainable without better understanding. If you think this performance hit is acceptable, then go for it, put the status reading back in, I would say. Maybe it would be nice to have some kind of progress bar, like in 'rpm -Uhv', 'scp' or 'cdparanoia', at least while programming the flash, especially when the programming times get over a few minutes, as can be expected with these bigger and bigger serial flash devices. Maybe there should also be support for partial erasing and programming (depending on what the device allows). Ronald.
On 20.01.2008 12:48, Ronald Hoogenboom wrote:
On Sun, 2008-01-20 at 11:59 +0100, Ronald Hoogenboom wrote:
The read status register will take at least: 16*(2/33) us = about 1 us (excluding the LPC latency, which is....?), so assuming that the first read status will show busy and the second will show ready, it is only 2 us slower: ~20%, so that should be acceptable. I'll do some checks on the hardware shortly.
I've done the tests and the programming result (eg. the data read back
from the flash) is correct with and without the status reading. With status reading, programming takes: 1'15 ~ 1'50 Without status reading, programming takes: 0'50 ~ 0'55
Hm.
There is quite some variance in the time taken extra by reading the status, the lower bound is about the expected penalty (as calculated above), but the higher bound is not so easily explainable without better understanding. If you think this performance hit is acceptable, then go for it, put the status reading back in, I would say.
I think we can shorten the delay in the loop to about 5 us. However, even with 10 us delay we still can't explain the time needed to program the chip. In the case without status reading, we have exactly 5 bytes on the SPI bus for every byte written. At 16 MHz, 5 bytes take 2.5 us transfer time. Adding an unconditional 10 us delay (max. byte program time according to the datasheets) gives us a total of 12.5 us per byte written to flash. Looking at the time you needed to program the while flash chip, things don't add up. With 12.5 us per byte and 2 MByte chip size, the total programming time should be close to 25 seconds. That is half the time it takes in reality and tells us we have ~12.5 us overhead per transaction. This overhead can NOT be attributed to LPC cycles because we avoid LPC altogether in byte programming. Either there is some concurrent access to the ROM (SMM or other stuff) or the accumulated reads/writes to the I/O ports of the IT8716F take that long. Can you use oprofile against the current code to find out where we spend all of that overhead?
Maybe it would be nice to have some kind of progress bar, like in 'rpm -Uhv', 'scp' or 'cdparanoia', at least while programming the flash, especially when the programming times get over a few minutes, as can be expected with these bigger and bigger serial flash devices.
Yes, this would be nice to have. Some chips already implement a per-page progress indicator.
Maybe there should also be support for partial erasing and programming (depending on what the device allows).
The functions are there for most chips, but we currently have no way to tell flashrom about sector sizes. Since quite a few SPI chips have non-uniform sector sizes, you have to store an array or sector sizes for each chip. That gets tedious quickly. Regards, Carl-Daniel
On Sun, 2008-01-20 at 22:28 +0100, Carl-Daniel Hailfinger wrote:
I think we can shorten the delay in the loop to about 5 us. However, even with 10 us delay we still can't explain the time needed to program the chip. In the case without status reading, we have exactly 5 bytes on the SPI bus for every byte written. At 16 MHz, 5 bytes take 2.5 us transfer
I think you are forgetting the enable-write command, needed before each byte-program command. So that will be another 1 byte = 3.0 us total. But if you look at the it8716f_spi_command function, that will also before and after the command wait for the busy bit of the 8716's SPI controller. These actions will generate extra traffic on the LPC bus (LPC is the only datapath to the SB and CPU, so it has to!). We could optimize by omitting the wait for SPI ready when there is no data to be read, eg. readcnt==0. I'll have a look at what can be gained by that.
time. Adding an unconditional 10 us delay (max. byte program time according to the datasheets) gives us a total of 12.5 us per byte written to flash. Looking at the time you needed to program the while flash chip, things don't add up. With 12.5 us per byte and 2 MByte chip size, the total programming time should be close to 25 seconds. That is half the time it takes in reality and tells us we have ~12.5 us overhead per transaction. This overhead can NOT be attributed to LPC cycles because we avoid LPC altogether in byte programming. Either there is some concurrent access to the ROM (SMM or other stuff) or the accumulated reads/writes to the I/O ports of the IT8716F take that long.
IO MUST go through LPC, as that is the ONLY path to Southbridge.
Can you use oprofile against the current code to find out where we spend all of that overhead?
Maybe it would be nice to have some kind of progress bar, like in 'rpm -Uhv', 'scp' or 'cdparanoia', at least while programming the flash, especially when the programming times get over a few minutes, as can be expected with these bigger and bigger serial flash devices.
Yes, this would be nice to have. Some chips already implement a per-page progress indicator.
Maybe there should also be support for partial erasing and programming (depending on what the device allows).
The functions are there for most chips, but we currently have no way to tell flashrom about sector sizes. Since quite a few SPI chips have non-uniform sector sizes, you have to store an array or sector sizes for each chip. That gets tedious quickly.
Regards, Carl-Daniel
On Mon, 2008-01-21 at 00:24 +0100, Ronald Hoogenboom wrote:
(LPC is the only datapath to the SB and CPU, so it has to!). We could optimize by omitting the wait for SPI ready when there is no data to be read, eg. readcnt==0. I'll have a look at what can be gained by that.
Omitting it saves 10 seconds with the unconditional 10us delay, reducing to 40~45 secs. So I think it is worth putting it in. With the conditional delay, it is not so easily measurable, because of the natural variance much larger than 10 us... Patch follows... Signed-off-by: Ronald Hoogenboom <hoogenboom30@zonnet.nl> ---
On Mon, 2008-01-21 at 00:24 +0100, Ronald Hoogenboom wrote:
(LPC is the only datapath to the SB and CPU, so it has to!). We could optimize by omitting the wait for SPI ready when there is no data to be read, eg. readcnt==0. I'll have a look at what can be gained by that.
Omitting it saves 10 seconds with the unconditional 10us delay, reducing to 40~45 secs. So I think it is worth putting it in. With the conditional delay, it is not so easily measurable, because of the natural variance much larger than 10 us... Patch follows... Signed-off-by: Ronald Hoogenboom <hoogenboom30@zonnet.nl> ---
On 21.01.2008 00:47, Ronald Hoogenboom wrote:
On Mon, 2008-01-21 at 00:24 +0100, Ronald Hoogenboom wrote:
(LPC is the only datapath to the SB and CPU, so it has to!). We could optimize by omitting the wait for SPI ready when there is no data to be read, eg. readcnt==0. I'll have a look at what can be gained by that.
Omitting it saves 10 seconds with the unconditional 10us delay, reducing to 40~45 secs. So I think it is worth putting it in. With the conditional delay, it is not so easily measurable, because of the natural variance much larger than 10 us... Patch follows...
Signed-off-by: Ronald Hoogenboom <hoogenboom30@zonnet.nl>
Acked-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@gmx.net> Committed in r3068. Regards, Carl-Daniel
Harald: This patch should fix your problems writing to the chip. Use either "patch -l" or remove the // before //while (generic_spi_read_status_register() & JEDEC_RDSR_BIT_WIP) Ronald: I need an ack to commit this. On 20.01.2008 11:59, Ronald Hoogenboom wrote:
Carl-Daniel Hailfinger wrote:
Did you have time to test with the while (generic_spi_read_status_register() & .._WIP) enabled? It would be more correct to have it enabled.
I had that in initially (that's why it was there, but commented out) but all it got me was slower programming times and the datasheet also suggests it is ok to just do a timed wait. But in the face of genericness for other chips, it is indeed more correct to do the check for the busy bit.
Ronald/Harald, can you please ack the change? It is reproduced below (whichspace-damaged). Regards, Carl-Daniel Make sure we delay writing the next byte long enough in SPI byte programming. Minor formatting changes. Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@gmx.net> Index: spi.c =================================================================== --- spi.c (Revision 3068) +++ spi.c (Arbeitskopie) @@ -519,12 +519,8 @@ for (i=0; i<total_size; i++) { generic_spi_write_enable(); spi_byte_program(i,buf[i]); - /* FIXME: We really should read the status register and delay - * accordingly. - */ - //while (generic_spi_read_status_register() & JEDEC_RDSR_BIT_WIP) + while (generic_spi_read_status_register() & JEDEC_RDSR_BIT_WIP) myusec_delay(10); //if (i%1024==0) fputc('b',stderr); } Regards, Carl-Daniel
Just FYI, some luck writing this chip on the sis board ... The write had no errors. The -v failed. But on using the -r to read to a file, I find no differences between the file used for -w and the file used for -v! Next is to try to actually put the vga bios onto the image and see if, this time, I get something. I brought a post card home so I'm hoping to see something. ron
Am Dienstag, 22. Januar 2008 01:09:09 schrieben Sie:
Harald: This patch should fix your problems writing to the chip. Use either "patch -l" or remove the // before //while (generic_spi_read_status_register() & JEDEC_RDSR_BIT_WIP) I've updated my flashrom tree to revision 3068 and commented that line out. Now i can flash the chip, like Ronald says. I wrote the chip, and read the file again, and compared the start and the end of the file with hexdump/head/tail and diff. On the first/last 50000 lines there is no difference between the file which was written to the chip, and the file i've read with flashrom from the chip.
Any idea why verifying fails?
Ronald: I need an ack to commit this.
On 20.01.2008 11:59, Ronald Hoogenboom wrote:
Carl-Daniel Hailfinger wrote:
Did you have time to test with the while (generic_spi_read_status_register() & .._WIP) enabled? It would be more correct to have it enabled.
I had that in initially (that's why it was there, but commented out) but all it got me was slower programming times and the datasheet also suggests it is ok to just do a timed wait. But in the face of genericness for other chips, it is indeed more correct to do the check for the busy bit.
Ronald/Harald, can you please ack the change? It is reproduced below (whichspace-damaged).
Regards, Carl-Daniel
Make sure we delay writing the next byte long enough in SPI byte programming. Minor formatting changes.
Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@gmx.net> Acked-by: Harald Gutmann <harald.gutmann@gmx.net>
Index: spi.c =================================================================== --- spi.c (Revision 3068) +++ spi.c (Arbeitskopie) @@ -519,12 +519,8 @@ for (i=0; i<total_size; i++) { generic_spi_write_enable(); spi_byte_program(i,buf[i]); - /* FIXME: We really should read the status register and delay - * accordingly. - */ - //while (generic_spi_read_status_register() & JEDEC_RDSR_BIT_WIP) + while (generic_spi_read_status_register() & JEDEC_RDSR_BIT_WIP) myusec_delay(10); //if (i%1024==0) fputc('b',stderr); }
Regards, Carl-Daniel
Regards, Harald
On 22.01.2008 12:08, Harald Gutmann wrote:
Am Dienstag, 22. Januar 2008 01:09:09 schrieben Sie:
Harald: This patch should fix your problems writing to the chip. Use either "patch -l" or remove the // before //while (generic_spi_read_status_register() & JEDEC_RDSR_BIT_WIP)
I've updated my flashrom tree to revision 3068 and commented that line out. Now i can flash the chip, like Ronald says. I wrote the chip, and read the file again, and compared the start and the end of the file with hexdump/head/tail and diff. On the first/last 50000 lines there is no difference between the file which was written to the chip, and the file i've read with flashrom from the chip.
Any idea why verifying fails?
Yes. Will send a patch.
Ronald: I need an ack to commit this.
On 20.01.2008 11:59, Ronald Hoogenboom wrote:
Carl-Daniel Hailfinger wrote:
Did you have time to test with the while (generic_spi_read_status_register() & .._WIP) enabled? It would be more correct to have it enabled.
I had that in initially (that's why it was there, but commented out) but all it got me was slower programming times and the datasheet also suggests it is ok to just do a timed wait. But in the face of genericness for other chips, it is indeed more correct to do the check for the busy bit.
Ronald/Harald, can you please ack the change? It is reproduced below (whichspace-damaged).
Regards, Carl-Daniel
Make sure we delay writing the next byte long enough in SPI byte programming. Minor formatting changes.
Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@gmx.net>
Acked-by: Harald Gutmann <harald.gutmann@gmx.net>
Thanks, r3069. Regards, Carl-Daniel
On 22.01.2008 03:53, ron minnich wrote:
Just FYI, some luck writing this chip on the sis board ...
The write had no errors. The -v failed. But on using the -r to read to a file, I find no differences between the file used for -w and the file used for -v!
Yes, very old lingering bug in the verify code. Fixed with the patch I just sent to the list.
Next is to try to actually put the vga bios onto the image and see if, this time, I get something.
Should work.
I brought a post card home so I'm hoping to see something.
Good luck! Regards, Carl-Daniel
Carl-Daniel Hailfinger wrote:
Harald: This patch should fix your problems writing to the chip. Use either "patch -l" or remove the // before //while (generic_spi_read_status_register() & JEDEC_RDSR_BIT_WIP)
Ronald: I need an ack to commit this.
On 20.01.2008 11:59, Ronald Hoogenboom wrote:
Carl-Daniel Hailfinger wrote:
Did you have time to test with the while (generic_spi_read_status_register() & .._WIP) enabled? It would be more correct to have it enabled.
I had that in initially (that's why it was there, but commented out) but all it got me was slower programming times and the datasheet also suggests it is ok to just do a timed wait. But in the face of genericness for other chips, it is indeed more correct to do the check for the busy bit.
Ronald/Harald, can you please ack the change? It is reproduced below (whichspace-damaged).
Regards, Carl-Daniel
Make sure we delay writing the next byte long enough in SPI byte programming. Minor formatting changes.
Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@gmx.net>
Acked-by: Ronald Hoogenboom <hoogenboom30@zonnet.nl>
Finally, now I know it's working (at least the part that I'm patching here...), here is the patch that uses PIO mode read from SPI rom with lzma decompression. This patch allows direct out-of-SPI-flash boot of a Linux kernel. It circumvents the 512KB limitation in the IT8716f superio of memory mapping an SPI flash device by using a PIO method for reading the data. Limitation: no nrv2b support. Not so nice: hardcoded 0x820 SPI-IO port. Best regards, Ronald. Signed-off-by: Ronald Hoogenboom <hoogenboom30@zonnet.nl>
Hi Ronald, On Mon, Feb 18, 2008 at 10:45:23PM +0100, Ronald Hoogenboom wrote:
Finally, now I know it's working (at least the part that I'm patching here...), here is the patch that uses PIO mode read from SPI rom with lzma decompression.
This patch allows direct out-of-SPI-flash boot of a Linux kernel. It circumvents the 512KB limitation in the IT8716f superio of memory mapping an SPI flash device by using a PIO method for reading the data. Limitation: no nrv2b support.
I've tested this on an m57sli-s4 (soic/spi revision) with a 2MB SPI chip, and it boots. Great work!
Not so nice: hardcoded 0x820 SPI-IO port.
Is that superio specific? Maybe some sort of lookup table based on the superio in use would be a solution? At this point there would be just the one line I presume.
Signed-off-by: Ronald Hoogenboom <hoogenboom30@zonnet.nl>
I won't pretend to understand everything, but I have a few comments, see below.
Index: src/southbridge/nvidia/mcp55/mcp55_lpc.c =================================================================== --- src/southbridge/nvidia/mcp55/mcp55_lpc.c (revision 3103) +++ src/southbridge/nvidia/mcp55/mcp55_lpc.c (working copy) @@ -243,7 +243,7 @@ static void mcp55_lpc_read_resources(device_t dev) { struct resource *res; - unsigned long index; + //unsigned long index;
What's this change for? Unrelated to the rest of the patch? If it's general cleanup, please do this in a separate patch. <snip>
Index: src/mainboard/gigabyte/m57sli/Options.lb =================================================================== --- src/mainboard/gigabyte/m57sli/Options.lb (revision 3103) +++ src/mainboard/gigabyte/m57sli/Options.lb (working copy) @@ -42,7 +42,7 @@ uses ROM_IMAGE_SIZE uses ROM_SECTION_SIZE uses ROM_SECTION_OFFSET -uses CONFIG_ROM_PAYLOAD +uses CONFIG_PIOROM_PAYLOAD uses CONFIG_ROM_PAYLOAD_START uses CONFIG_COMPRESSED_PAYLOAD_NRV2B uses CONFIG_COMPRESSED_PAYLOAD_LZMA @@ -80,6 +80,7 @@ uses OBJCOPY uses CONFIG_CHIP_NAME uses CONFIG_CONSOLE_VGA +uses CONFIG_VGA_ROM_RUN uses CONFIG_USBDEBUG_DIRECT uses CONFIG_PCI_ROM_RUN uses HW_MEM_HOLE_SIZEK @@ -211,7 +212,8 @@
#VGA Console default CONFIG_CONSOLE_VGA=1 -default CONFIG_PCI_ROM_RUN=1 +default CONFIG_VGA_ROM_RUN=1 +default CONFIG_PCI_ROM_RUN=0
Unrelated to the rest of the patch? I'm not sure I want this default to change - it's nice to have any PCI roms run by default.
#default CONFIG_USBDEBUG_DIRECT=1
@@ -253,8 +255,8 @@ ## default MAINBOARD_PART_NUMBER="m57sli" default MAINBOARD_VENDOR="GIGABYTE" -default MAINBOARD_PCI_SUBSYSTEM_VENDOR_ID=0x1022 -default MAINBOARD_PCI_SUBSYSTEM_DEVICE_ID=0x2b80 +default MAINBOARD_PCI_SUBSYSTEM_VENDOR_ID=0x1458 +default MAINBOARD_PCI_SUBSYSTEM_DEVICE_ID=0xe000
Also unrelated to the rest of the patch? This does not seem to have any effect on my box. Also; the proprietary bios uses 0x1022; why are you changing this? This is great work; if you can respond to the comments above I think I can ack this soon. Thanks, Ward. -- Ward Vandewege <ward@fsf.org> Free Software Foundation - Senior System Administrator
Hi Ward, Thanks for reviewing my patch. I'm glad to hear that it got you a booting mainboard too. Most of the seemingly unrelated changes are a result of my quest to get my nvidia VGA to work. Also I'm quite new submitting patches and I'm not yet very familiar with what stuff is allowed to be in a patch like this and what not. Thanks for the advise in this. Please find my comments below. On Tue, 2008-02-19 at 23:34 -0500, Ward Vandewege wrote:
Hi Ronald, <snip>
Not so nice: hardcoded 0x820 SPI-IO port.
Is that superio specific? Maybe some sort of lookup table based on the superio in use would be a solution? At this point there would be just the one line I presume.
I was thinking this port number should be snarfed from the mainboard/gigabyte/m57sli/Config.lb file (see line 293: io 0x64 = 0x820), but I couldn't figure out how, so I hardcoded it to get going.
- unsigned long index; + //unsigned long index;
What's this change for? Unrelated to the rest of the patch? If it's general cleanup, please do this in a separate patch.
This was only to stop vim from going to that file every compile ;-). Just to get rid of an 'unused variable' warning. <snip>
+uses CONFIG_VGA_ROM_RUN <snip> -default CONFIG_PCI_ROM_RUN=1 +default CONFIG_VGA_ROM_RUN=1 +default CONFIG_PCI_ROM_RUN=0
Unrelated to the rest of the patch? I'm not sure I want this default to change - it's nice to have any PCI roms run by default.
You are right. But it might be good to at least give the user the option to choose to run only VGA ROM (get display going) and leave the rest to the OS.
-default MAINBOARD_PCI_SUBSYSTEM_VENDOR_ID=0x1022 -default MAINBOARD_PCI_SUBSYSTEM_DEVICE_ID=0x2b80 +default MAINBOARD_PCI_SUBSYSTEM_VENDOR_ID=0x1458 +default MAINBOARD_PCI_SUBSYSTEM_DEVICE_ID=0xe000
Also unrelated to the rest of the patch? This does not seem to have any effect on my box. Also; the proprietary bios uses 0x1022; why are you changing this?
My prop. BIOS does put the gigabyte subvendor ID in. When I changed it, I was only looking at MCP55 only (which gets the e000 DID from the prop. BIOS), I didn't realize the setting inpacted EVERY PCI card that can set subsystem ID's. Indeed it didn't have any effect with me either, but I do think that the 1458 vendor is more appropriate (for what that's worth...)
This is great work; if you can respond to the comments above I think I can ack this soon.
Thanks, please let me know what to leave out definitively or what to put in a separate patch and I'll send (a) new patch(es).
Thanks, Ward.
Regards, Ronald.
On Wed, 2008-02-20 at 23:21 +0100, Ronald Hoogenboom wrote:
On Tue, 2008-02-19 at 23:34 -0500, Ward Vandewege wrote: <...>
-default MAINBOARD_PCI_SUBSYSTEM_VENDOR_ID=0x1022 -default MAINBOARD_PCI_SUBSYSTEM_DEVICE_ID=0x2b80 +default MAINBOARD_PCI_SUBSYSTEM_VENDOR_ID=0x1458 +default MAINBOARD_PCI_SUBSYSTEM_DEVICE_ID=0xe000
Also unrelated to the rest of the patch? This does not seem to have any effect on my box. Also; the proprietary bios uses 0x1022; why are you changing this?
My prop. BIOS does put the gigabyte subvendor ID in. When I changed it, I was only looking at MCP55 only (which gets the e000 DID from the prop. BIOS), I didn't realize the setting inpacted EVERY PCI card that can set subsystem ID's. Indeed it didn't have any effect with me either, but I do think that the 1458 vendor is more appropriate (for what that's worth...)
I was totally bullshitting here... I changed it to make 'kudzu' (RedHat/Fedora's new-hardware detector) NOT re-configure my network interface when booted coreboot. These are the values put in by the proprietary bios for the PCI 00:08.0 (MCP55 Ethernet). Ronald.
On Sun, Feb 24, 2008 at 11:37:22PM +0100, Ronald Hoogenboom wrote:
My prop. BIOS does put the gigabyte subvendor ID in. When I changed it, I was only looking at MCP55 only (which gets the e000 DID from the prop. BIOS), I didn't realize the setting inpacted EVERY PCI card that can set subsystem ID's. Indeed it didn't have any effect with me either, but I do think that the 1458 vendor is more appropriate (for what that's worth...)
I was totally bullshitting here... I changed it to make 'kudzu' (RedHat/Fedora's new-hardware detector) NOT re-configure my network interface when booted coreboot. These are the values put in by the proprietary bios for the PCI 00:08.0 (MCP55 Ethernet).
OK; do you want to prepare a new patch? Thanks, Ward. -- Ward Vandewege <ward@fsf.org> Free Software Foundation - Senior System Administrator
On Mon, 2008-02-25 at 15:10 -0500, Ward Vandewege wrote:
'kudzu' (RedHat/Fedora's new-hardware detector) NOT re-configure my network interface when booted coreboot. These are the values put in by the proprietary bios for the PCI 00:08.0 (MCP55 Ethernet).
OK; do you want to prepare a new patch?
I would like to find a (semi-)nice way to get the SPI IO port number (now hardcoded 0x820) from the device tree settings. Do you have a suggestion? Otherwise I'll add yet another Config variable (not so nice). Ronald.
On 25.02.2008 22:36, Ronald Hoogenboom wrote:
On Mon, 2008-02-25 at 15:10 -0500, Ward Vandewege wrote:
'kudzu' (RedHat/Fedora's new-hardware detector) NOT re-configure my network interface when booted coreboot. These are the values put in
by
the proprietary bios for the PCI 00:08.0 (MCP55 Ethernet).
OK; do you want to prepare a new patch?
I would like to find a (semi-)nice way to get the SPI IO port number (now hardcoded 0x820) from the device tree settings. Do you have a suggestion? Otherwise I'll add yet another Config variable (not so nice).
Read it from the Super I/O? Regards, Carl-Daniel -- http://www.hailfinger.org/
On Mon, 2008-02-25 at 22:42 +0100, Carl-Daniel Hailfinger wrote:
On 25.02.2008 22:36, Ronald Hoogenboom wrote:
On Mon, 2008-02-25 at 15:10 -0500, Ward Vandewege wrote:
OK; do you want to prepare a new patch?
I would like to find a (semi-)nice way to get the SPI IO port number (now hardcoded 0x820) from the device tree settings. Do you have a suggestion? Otherwise I'll add yet another Config variable (not so nice).
Read it from the Super I/O?
I've found a nicer way: make an init_gpio in the superio.c that puts the port value in a global var if the PIOROM_STREAM is enabled. This patch allows direct out-of-SPI-flash boot of a Linux kernel. It circumvents the 512KB limitation in the IT8716f superio of memory mapping an SPI flash device by using a PIO method for reading the data. Limitation: no nrv2b support. Signed-off-by: Ronald Hoogenboom <hoogenboom30@zonnet.nl> Regards, Ronald
Hi Ronald, On Thu, Feb 28, 2008 at 12:05:26AM +0100, Ronald Hoogenboom wrote:
I've found a nicer way: make an init_gpio in the superio.c that puts the port value in a global var if the PIOROM_STREAM is enabled.
Uhm, ok, but this does not build for me (against r3127)? Looks like the actual definition of init_gpio is missing from your patch? I really want this to go into the tree. We'll get there eventually :) Thanks, Ward. -- Ward Vandewege <ward@fsf.org> Free Software Foundation - Senior System Administrator
On Thu, 2008-03-06 at 17:02 -0500, Ward Vandewege wrote:
Hi Ronald,
On Thu, Feb 28, 2008 at 12:05:26AM +0100, Ronald Hoogenboom wrote:
I've found a nicer way: make an init_gpio in the superio.c that puts the port value in a global var if the PIOROM_STREAM is enabled.
Uhm, ok, but this does not build for me (against r3127)? Looks like the actual definition of init_gpio is missing from your patch?
You are right! I must have deleted too much from the diff... I should not do so many changes on one source tree.
I really want this to go into the tree. We'll get there eventually :)
I've prepared a patch for r3129 WITH the definition of the static function.. This patch allows direct out-of-SPI-flash boot of a Linux kernel. It circumvents the 512KB limitation in the IT8716f superio of memory mapping an SPI flash device by using a PIO method for reading the data. Limitation: no nrv2b support. Signed-off-by: Ronald Hoogenboom <hoogenboom30@zonnet.nl> Regards, Ronald
Hi, On Mon, Mar 10, 2008 at 09:05:39AM +0100, Ronald Hoogenboom wrote:
I've prepared a patch for r3129 WITH the definition of the static function..
This is a great achievement, but I really miss more discussion of what was neccessary to do.
This patch allows direct out-of-SPI-flash boot of a Linux kernel. It circumvents the 512KB limitation in the IT8716f superio of memory mapping an SPI flash device by using a PIO method for reading the data. Limitation: no nrv2b support.
Please explain further. Why would a new reading method affect any compression methods? Not until I actually looked at the patch was there any mention of adding a "chunked" LZMA mode, and that was only in your (C) notice. What does this patch do, exactly, and how do your new bits work? //Peter
Hi Ronald, On Mon, Mar 10, 2008 at 09:05:39AM +0100, Ronald Hoogenboom wrote:
I've prepared a patch for r3129 WITH the definition of the static function..
This patch allows direct out-of-SPI-flash boot of a Linux kernel. It circumvents the 512KB limitation in the IT8716f superio of memory mapping an SPI flash device by using a PIO method for reading the data. Limitation: no nrv2b support.
Signed-off-by: Ronald Hoogenboom <hoogenboom30@zonnet.nl>
I've tested this on real hardware. It works - I can boot a 1MB image from a 2MB spi/soic chip, whereas without the patch the system does not boot with an image > 512KB. As far as I'm concerned: Acked-by: Ward Vandewege <ward@gnu.org> Let's see if we can get another ack before we commit though. Thanks, Ward. -- Ward Vandewege <ward@fsf.org> Free Software Foundation - Senior System Administrator
On 10.03.2008 09:05, Ronald Hoogenboom wrote:
This patch allows direct out-of-SPI-flash boot of a Linux kernel. It circumvents the 512KB limitation in the IT8716f superio of memory mapping an SPI flash device by using a PIO method for reading the data. Limitation: no nrv2b support.
Signed-off-by: Ronald Hoogenboom <hoogenboom30@zonnet.nl>
src/stream/piorom_stream.c: A missing license header. src/mainboard/gigabyte/m57sli/Options.lb: Changed to be SPI-only (rev 1.x boards will have problems). CONFIG_VGA_ROM_RUN settings are added (does not belong into this patch). src/lib/lzma_cb.c: This copies the incorrect copyright header from src/lib/lzma.c. I sent a patch to correct the copyright header (GPL v2 and later) more than half a year ago, IIRC Uwe vetoed the change because it did not introduce the long copyright+disclaimer header. It would be great if you used sizeof() instead of sizeof because it helps readability. static unsigned char scratchpad[15980]; static CLzmaDecoderState state; static UInt32 outSize; All three allocations were moved from the stack to rwdata. Is this intentional? src/lib/lzmadecode.c: You change the definition of RC_TEST for the normal case (different error code). Any reason for that? I think the big buffer for streaming LZMA decompression is not allocated anywhere, but it's already late at night, so I may be mistaken. Regards, Carl-Daniel -- http://www.hailfinger.org/
On Tue, 2008-03-11 at 01:12 +0100, Carl-Daniel Hailfinger wrote:
On 10.03.2008 09:05, Ronald Hoogenboom wrote:
This patch allows direct out-of-SPI-flash boot of a Linux kernel. It circumvents the 512KB limitation in the IT8716f superio of memory mapping an SPI flash device by using a PIO method for reading the data. Limitation: no nrv2b support.
Signed-off-by: Ronald Hoogenboom <hoogenboom30@zonnet.nl>
src/stream/piorom_stream.c: A missing license header.
I just imitated rom_stream.c, which hasn't got it either...
src/mainboard/gigabyte/m57sli/Options.lb: Changed to be SPI-only (rev 1.x boards will have problems).
Rev. 1.x boards would choose 512K rom, which changes the piorom_stream back to rom_stream (see the very first #if in piorom_stream). I agree this is ugly, but I couldn't make it work in the Options.lb with an 'if ROM_SIZE > 512*1024' - like construct.
CONFIG_VGA_ROM_RUN settings are added (does not belong into this patch).
True.
src/lib/lzma_cb.c:
This copies the incorrect copyright header from src/lib/lzma.c. I sent a patch to correct the copyright header (GPL v2 and later) more than half a year ago, IIRC Uwe vetoed the change because it did not introduce the long copyright+disclaimer header.
Where is a correct example of the header you would like to see?
It would be great if you used sizeof() instead of sizeof because it helps readability.
That's a matter of taste. I never use brackets on sizeof (except for types), because it makes it look like a function, which it isn't. But conforming is easy...
static unsigned char scratchpad[15980]; static CLzmaDecoderState state; static UInt32 outSize;
All three allocations were moved from the stack to rwdata. Is this intentional?
This was done, because I wanted to use the out-read mode of the lzma decoder too, but that became too complex... I think they can be moved back to the stack now, if that is prefered.
src/lib/lzmadecode.c:
You change the definition of RC_TEST for the normal case (different error code). Any reason for that?
No, this was just done while debugging and it remained there. I will remove that.
I think the big buffer for streaming LZMA decompression is not allocated anywhere, but it's already late at night, so I may be mistaken.
rom_stream also just dumps the decompressed result at address 16*2^20, so I figured that it would be OK if I did it too... (I must say, it made me frown when I saw it, but hey, it's only a short-lived intermediate and nobody else is using the memory, but coreboot...)
Regards, Carl-Daniel
Can you comment back, then I'll change things and recreate a patch. Thanks, Ronald.
On Wed, Mar 12, 2008 at 09:56:55PM +0100, Ronald Hoogenboom wrote:
On Tue, 2008-03-11 at 01:12 +0100, Carl-Daniel Hailfinger wrote:
On 10.03.2008 09:05, Ronald Hoogenboom wrote:
This patch allows direct out-of-SPI-flash boot of a Linux kernel. It circumvents the 512KB limitation in the IT8716f superio of memory mapping an SPI flash device by using a PIO method for reading the data. Limitation: no nrv2b support.
Signed-off-by: Ronald Hoogenboom <hoogenboom30@zonnet.nl>
src/stream/piorom_stream.c: A missing license header.
I just imitated rom_stream.c, which hasn't got it either...
src/mainboard/gigabyte/m57sli/Options.lb: Changed to be SPI-only (rev 1.x boards will have problems).
Rev. 1.x boards would choose 512K rom, which changes the piorom_stream back to rom_stream (see the very first #if in piorom_stream). I agree this is ugly, but I couldn't make it work in the Options.lb with an 'if ROM_SIZE > 512*1024' - like construct.
... except for those of us who've soldered a socket onto a rev1 board of course! I often use larger chips on a v1. So, this should really be spi-only. Carl-Daniel; was your e-mail a list of actual changes you had made, or just a list of 'things to be changed'? There was no patch attached but it kind of sounded like you had already fixed those things. Thanks, Ward. -- Ward Vandewege <ward@fsf.org> Free Software Foundation - Senior System Administrator
On Mon, 2008-03-17 at 16:45 -0400, Ward Vandewege wrote:
... except for those of us who've soldered a socket onto a rev1 board of course! I often use larger chips on a v1.
And, I presume the 512K limit doesn't apply for them...
So, this should really be spi-only.
Yes, but how? Having a separate V2-board target afterall? Or Config.lb files specific for V1/V2 boards, where the V1 uses rom_stream (CONFIG_ROM_PAYLOAD=1) and V2 uses piorom_stream (CONFIG_PIOROM_PAYLOAD=1)? Which is the least ugly.... Ronald.
On Tue, Mar 18, 2008 at 10:10:41PM +0100, Ronald Hoogenboom wrote:
On Mon, 2008-03-17 at 16:45 -0400, Ward Vandewege wrote:
... except for those of us who've soldered a socket onto a rev1 board of course! I often use larger chips on a v1.
And, I presume the 512K limit doesn't apply for them...
That's correct. It works at least with 1MB chips; I have not tried larger.
So, this should really be spi-only.
Yes, but how? Having a separate V2-board target afterall? Or Config.lb files specific for V1/V2 boards, where the V1 uses rom_stream (CONFIG_ROM_PAYLOAD=1) and V2 uses piorom_stream (CONFIG_PIOROM_PAYLOAD=1)? Which is the least ugly....
Hrm. I'd like to avoid splitting up the targets, but perhaps we have to. Carl-Daniel, thoughts? Thanks, Ward. -- Ward Vandewege <ward@fsf.org> Free Software Foundation - Senior System Administrator
On Tue, Mar 18, 2008 at 05:17:59PM -0400, Ward Vandewege wrote:
So, this should really be spi-only.
Yes, but how? Having a separate V2-board target afterall? Or Config.lb files specific for V1/V2 boards, where the V1 uses rom_stream (CONFIG_ROM_PAYLOAD=1) and V2 uses piorom_stream (CONFIG_PIOROM_PAYLOAD=1)? Which is the least ugly....
Hrm. I'd like to avoid splitting up the targets, but perhaps we have to.
I hope not. I fully agree that the code should _not_ be split if it's avoidable. Uwe. -- http://www.hermann-uwe.de | http://www.holsham-traders.de http://www.crazy-hacks.org | http://www.unmaintained-free-software.org
On Tue, 2008-03-18 at 22:29 +0100, Uwe Hermann wrote:
Yes, but how? Having a separate V2-board target afterall? Or Config.lb files specific for V1/V2 boards, where the V1 uses rom_stream (CONFIG_ROM_PAYLOAD=1) and V2 uses piorom_stream (CONFIG_PIOROM_PAYLOAD=1)? Which is the least ugly....
Hrm. I'd like to avoid splitting up the targets, but perhaps we have to.
I hope not. I fully agree that the code should _not_ be split if it's avoidable.
So, it's the options variant then... Default should be the ROM_PAYLOAD, I presume. That's OK for both V1 and V2 having the factory-fit 512K SPI. For the V2 board, I can create a Config-v2.lb that has a LAB/Linux payload and uses PIOROM_PAYLOAD in the targets/gigabyte/m57sli directory. All agree here? How does buildrom-devel handle situations like this? Ronald.
On 18.03.2008 22:17, Ward Vandewege wrote:
On Tue, Mar 18, 2008 at 10:10:41PM +0100, Ronald Hoogenboom wrote:
On Mon, 2008-03-17 at 16:45 -0400, Ward Vandewege wrote:
... except for those of us who've soldered a socket onto a rev1 board of course! I often use larger chips on a v1.
And, I presume the 512K limit doesn't apply for them...
That's correct. It works at least with 1MB chips; I have not tried larger.
So, this should really be spi-only.
Yes, but how? Having a separate V2-board target afterall? Or Config.lb files specific for V1/V2 boards, where the V1 uses rom_stream (CONFIG_ROM_PAYLOAD=1) and V2 uses piorom_stream (CONFIG_PIOROM_PAYLOAD=1)? Which is the least ugly....
Hrm. I'd like to avoid splitting up the targets, but perhaps we have to. Carl-Daniel, thoughts?
To be honest, most of this probably could be worked around a bit easier with less code changes if decompression wasn't changed at all. Is there any reason we can't copy (in the sense of "read byte from SPI, copy it somewhere") the ROM contents to a given location in RAM, then decompress it with normal lzma functions? That would reduce code differences to copy/nocopy and decompression source address. As a bonus, it would speed up booting a lot in the SPI case. Regards, Carl-Daniel -- http://www.hailfinger.org/
On Wed, 2008-03-19 at 00:46 +0100, Carl-Daniel Hailfinger wrote:
To be honest, most of this probably could be worked around a bit easier with less code changes if decompression wasn't changed at all. Is there any reason we can't copy (in the sense of "read byte from SPI, copy it somewhere") the ROM contents to a given location in RAM, then decompress it with normal lzma functions? That would reduce code differences to copy/nocopy and decompression source address. As a bonus, it would speed up booting a lot in the SPI case.
I don't see how "copy byte by byte from rom to ram and when all bytes are done, only then start decompressing" would be faster than "copy byte by byte from rom to ram and decompress while doing that". Can you explain, please? Besides the fact that we need yet-another-place-in-ram to do yet another copy. I was looking at the opposite: while decompressing, copy it directly to the place the elf-header says it should be, using the LZMA_OUT_READ mode. That should speed up the ELF loading in the normal rom_stream mode too. Ronald.
participants (9)
-
Carl-Daniel Hailfinger -
Corey Osgood -
Harald Gutmann -
Peter Stuge -
ron minnich -
Ronald Hoogenboom -
Ronald Hoogenboom -
Uwe Hermann -
Ward Vandewege