August 2014 - flashrom - mail.coreboot.org

Re: [flashrom] [PATCH] Add support for S25FL128P, S25FL129P and refine it for S25FL128S chips.
by Antonio Ospite Aug. 6, 2014

Aug. 6, 2014

On Mon, 4 Aug 2014 00:46:55 +0200 Stefan Tauner <stefan.tauner(a)alumni.tuwien.ac.at> wrote: > Additionally to the existing S25FL128S......0 definition this patch > adds S25FL128P......0, S25FL128P......1 and S25FL128S......1, as well as > S25FL129P......0 and S25FL129P......1 definitions. > S25FL12xP seem to be the predecessor families of S25FL128S. All > associated chips can not be distinguished with RDID alone. > > Besides the new chips, this patch also fixes the name of the previously > supported S25FL128S model with uniform 256 kB sectors > (S25FL128P......1 not 0) and adds the hybrid sector version (0) as well. > > Due to the shared IDs the user has to select the right chip manually > with the -c parameter. To make this even possible, this patch enlarges > the respective array for results to 6. > Hi Stefan, the chip I've got is a S25FL129P......0, logs attached and some comments inlined below. Thanks. > Signed-off-by: Stefan Tauner <stefan.tauner(a)alumni.tuwien.ac.at> Tested-by: Antonio Ospite <ao2(a)ao2.it> > --- > cli_classic.c | 2 +- > flashchips.c | 182 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++- > flashchips.h | 2 +- > 3 files changed, 181 insertions(+), 5 deletions(-) > > diff --git a/cli_classic.c b/cli_classic.c > index ee988a3..60b8b88 100644 > --- a/cli_classic.c > +++ b/cli_classic.c > @@ -94,7 +94,7 @@ int main(int argc, char *argv[]) > unsigned long size; > /* Probe for up to three flash chips. */ > const struct flashchip *chip = NULL; > - struct flashctx flashes[3] = {{0}}; > + struct flashctx flashes[6] = {{0}}; > struct flashctx *fill_flash; > const char *name; > int namelen, opt, i, j; > diff --git a/flashchips.c b/flashchips.c > index a1166e0..e587b28 100644 > --- a/flashchips.c > +++ b/flashchips.c > @@ -10389,7 +10389,69 @@ const struct flashchip flashchips[] = { > > { > .vendor = "Spansion", > - .name = "S25FL128S......0", /* uniform 256kB sectors */ > + .name = "S25FL128P......0", /* uniform 64 kB sectors */ > + .bustype = BUS_SPI, > + .manufacture_id = SPANSION_ID, > + .model_id = SPANSION_S25FL128, > + .total_size = 16384, > + .page_size = 256, > + .feature_bits = FEATURE_WRSR_WREN, > + .tested = TEST_UNTESTED, > + .probe = probe_spi_rdid, > + .probe_timing = TIMING_ZERO, > + .block_erasers = { > + { > + .eraseblocks = { {64 * 1024, 256} }, > + .block_erase = spi_block_erase_20, > + }, { > + .eraseblocks = { {64 * 1024, 256} }, > + .block_erase = spi_block_erase_d8, > + }, { > + .eraseblocks = { { 16384 * 1024, 1} }, > + .block_erase = spi_block_erase_60, > + }, { > + .eraseblocks = { { 16384 * 1024, 1} }, > + .block_erase = spi_block_erase_c7, > + } > + }, > + .printlock = spi_prettyprint_status_register_bp3_srwd, > + .unlock = spi_disable_blockprotect_bp3_srwd, > + .write = spi_chip_write_256, > + .read = spi_chip_read, /* Fast read (0x0B) supported */ > + .voltage = {2700, 3600}, > + }, > + > + { > + .vendor = "Spansion", > + .name = "S25FL128P......1", /* uniform 256kB sectors */ > + .bustype = BUS_SPI, > + .manufacture_id = SPANSION_ID, > + .model_id = SPANSION_S25FL128, > + .total_size = 16384, > + .page_size = 256, > + .feature_bits = FEATURE_WRSR_WREN, > + .tested = TEST_UNTESTED, > + .probe = probe_spi_rdid, > + .probe_timing = TIMING_ZERO, > + .block_erasers = { > + { > + .eraseblocks = { {256 * 1024, 64} }, > + .block_erase = spi_block_erase_d8, > + }, { > + .eraseblocks = { { 16384 * 1024, 1} }, > + .block_erase = spi_block_erase_c7, > + } > + }, > + .printlock = spi_prettyprint_status_register_bp2_srwd, > + .unlock = spi_disable_blockprotect_bp2_srwd, > + .write = spi_chip_write_256, > + .read = spi_chip_read, /* Fast read (0x0B) supported */ > + .voltage = {2700, 3600}, > + }, > + > + { > + .vendor = "Spansion", > + .name = "S25FL128S......0", /* hybrid: 32 (top or bottom) 4 kB sub-sectors + 64 kB sectors */ > .bustype = BUS_SPI, > .manufacture_id = SPANSION_ID, > .model_id = SPANSION_S25FL128, > @@ -10403,9 +10465,46 @@ const struct flashchip flashchips[] = { > .probe_timing = TIMING_ZERO, > .block_erasers = { > { > - .eraseblocks = { {4 * 1024, 4096} }, > - .block_erase = spi_block_erase_20, > + /* FIXME: erase opcodes 20h for (top or bottom) 4 kB parameter sectors only > + .eraseblocks = { > + {4 * 1024, 32}, > + {64 * 1024, 254}, // NOP > + }, > + .block_erase = NULL, > + }, { */ > + .eraseblocks = { { 64 * 1024, 256} }, > + .block_erase = spi_block_erase_d8, > + }, { > + .eraseblocks = { { 16384 * 1024, 1} }, > + .block_erase = spi_block_erase_60, > }, { > + .eraseblocks = { { 16384 * 1024, 1} }, > + .block_erase = spi_block_erase_c7, > + } > + }, > + .printlock = spi_prettyprint_status_register_bp2_ep_srwd, /* TODO: SR2 and many others */ > + .unlock = spi_disable_blockprotect_bp2_srwd, /* TODO: various other locks */ > + .write = spi_chip_write_256, /* Multi I/O supported */ > + .read = spi_chip_read, /* Fast read (0x0B) and multi I/O supported */ > + .voltage = {2700, 3600}, > + }, > + > + { > + .vendor = "Spansion", > + .name = "S25FL128S......1", /* uniform 256 kB sectors */ > + .bustype = BUS_SPI, > + .manufacture_id = SPANSION_ID, > + .model_id = SPANSION_S25FL128, > + .total_size = 16384, > + .page_size = 512, > + /* supports 4B addressing */ > + /* OTP: 1024B total, 32B reserved; read 0x4B; write 0x42 */ > + .feature_bits = FEATURE_WRSR_WREN | FEATURE_OTP, > + .tested = TEST_UNTESTED, > + .probe = probe_spi_rdid, > + .probe_timing = TIMING_ZERO, > + .block_erasers = { > + { > .eraseblocks = { {256 * 1024, 64} }, > .block_erase = spi_block_erase_d8, > }, { > @@ -10422,6 +10521,83 @@ const struct flashchip flashchips[] = { > .read = spi_chip_read, /* Fast read (0x0B) and multi I/O supported */ > .voltage = {2700, 3600}, > }, > + { > + .vendor = "Spansion", > + .name = "S25FL129P......0", /* hybrid: 32 (top or bottom) 4 kB sub-sectors + 64 kB sectors */ > + .bustype = BUS_SPI, > + .manufacture_id = SPANSION_ID, > + .model_id = SPANSION_S25FL128, > + .total_size = 16384, > + .page_size = 256, > + /* OTP: 506B total, 16B reserved; read 0x4B; write 0x42 */ > + .feature_bits = FEATURE_WRSR_WREN | FEATURE_OTP, > + .tested = TEST_UNTESTED, Probe and read work, and I tested write for all the supported block_erasers, each time with random data generated with: dd if=/dev/urandom count=$((16 * 1024 * 1024)) iflag=count_bytes of=random_data.bin > + .probe = probe_spi_rdid, > + .probe_timing = TIMING_ZERO, > + .block_erasers = { > + { > + /* FIXME: erase opcodes 20h for (top or bottom) 4 kB parameter sectors only > + .eraseblocks = { > + {4 * 1024, 32}, I also wanted to understand this "parameter sub-sector" thing a little better, it was not obvious to me what "(top or bottom)" meant in this context. Then I realized than sometimes these parameter sub-subsectors are at the start of the chip, sometimes they are at the end. Anyway, spi_block_erase_20 works with these 4KiB sub-sectors on the first two 64KiB sectors (i.e. up to 0x01f000-0x01ffff): ... 0x01f000-0x01ffff:EW, 0x020000-0x02ffff:EFAILED ... Just toe xperiment, I even tried to enable 4KiB sub-sectors for the whole flash but verification fails on the third 64KiB sector. ... 0x01f000-0x01ffff:EW, 0x020000-0x020fff:EFAILED ... > + {64 * 1024, 254}, // NOP as said verification fails here with spi_block_erase_20, is this because spi_block_erase_20 is a NOP from the third 64KiB sector on? If so, maybe the comment is a little too essential, it becomes clear only after you _know_ that it refers to opcode 20h. Maybe the comments can be modeled after the similar ones for some Intel chips (e.g. "25F160S33B8")? So if someone wanted to make this work should flashrom be changed so that it'd be possible to specify different eraser functions for different flash regions? Just curios. > + }, > + .block_erase = NULL, mention spi_block_erase_20 here and spi_block_erase_40 in the following item? > + }, { */ > + /* FIXME: erase opcodes 40h for (top or bottom) 2*4 kB parameter sectors only > + .eraseblocks = { > + {8 * 1024, 16}, > + {64 * 1024, 254}, // NOP > + }, > + .block_erase = NULL, I didn't play with this, because there's no spi_block_erase_40, but I guess the results would be consistent with the 4KiB blocks. > + }, { */ > + .eraseblocks = { { 64 * 1024, 256} }, > + .block_erase = spi_block_erase_d8, This one is OK. > + }, { > + .eraseblocks = { { 16384 * 1024, 1} }, > + .block_erase = spi_block_erase_60, This one is OK. > + }, { > + .eraseblocks = { { 16384 * 1024, 1} }, > + .block_erase = spi_block_erase_c7, This one is OK. > + } > + }, > + .printlock = spi_prettyprint_status_register_bp2_ep_srwd, /* TODO: Configuration register */ > + .unlock = spi_disable_blockprotect_bp2_srwd, > + .write = spi_chip_write_256, /* Multi I/O supported */ > + .read = spi_chip_read, /* Fast read (0x0B) and multi I/O supported */ > + .voltage = {2700, 3600}, > + }, > + > + { > + .vendor = "Spansion", > + .name = "S25FL128P......1", /* uniform 256 kB sectors */ ^ This should be "S25FL129P......1". > + .bustype = BUS_SPI, > + .manufacture_id = SPANSION_ID, > + .model_id = SPANSION_S25FL128, > + .total_size = 16384, > + .page_size = 512, > + /* OTP: 506B total, 16B reserved; read 0x4B; write 0x42 */ > + .feature_bits = FEATURE_WRSR_WREN | FEATURE_OTP, > + .tested = TEST_UNTESTED, > + .probe = probe_spi_rdid, > + .probe_timing = TIMING_ZERO, > + .block_erasers = { > + { > + .eraseblocks = { {256 * 1024, 64} }, > + .block_erase = spi_block_erase_d8, > + }, { > + .eraseblocks = { { 16384 * 1024, 1} }, > + .block_erase = spi_block_erase_60, > + }, { > + .eraseblocks = { { 16384 * 1024, 1} }, > + .block_erase = spi_block_erase_c7, > + } > + }, > + .printlock = spi_prettyprint_status_register_bp2_ep_srwd, /* TODO: Configuration register */ > + .unlock = spi_disable_blockprotect_bp2_srwd, > + .write = spi_chip_write_256, /* Multi I/O supported */ > + .read = spi_chip_read, /* Fast read (0x0B) and multi I/O supported */ > + .voltage = {2700, 3600}, > + }, > > { > .vendor = "SST", > diff --git a/flashchips.h b/flashchips.h > index 029bae8..b2c1d4d 100644 > --- a/flashchips.h > +++ b/flashchips.h > @@ -590,7 +590,7 @@ > #define SPANSION_S25FL016A 0x0214 > #define SPANSION_S25FL032A 0x0215 /* Same as S25FL032P, but the latter supports EDI and CFI */ > #define SPANSION_S25FL064A 0x0216 /* Same as S25FL064P, but the latter supports EDI and CFI */ > -#define SPANSION_S25FL128 0x2018 > +#define SPANSION_S25FL128 0x2018 /* Same ID for various S25FL128P, S25FL128S and S25FL129P (including dual-die S70FL256P) variants (EDI supported) */ > #define SPANSION_S25FL256 0x0219 > #define SPANSION_S25FL512 0x0220 > #define SPANSION_S25FL204 0x4013 > -- > Kind regards, Stefan Tauner > -- Antonio Ospite http://ao2.it A: Because it messes up the order in which people normally read text. See http://en.wikipedia.org/wiki/Posting_style Q: Why is top-posting such a bad thing?

2 1

[commit] r1838 - trunk
by repository service Aug. 6, 2014

Aug. 6, 2014

Author: stefanct Date: Wed Aug 6 16:36:27 2014 New Revision: 1838 URL: http://flashrom.org/trac/flashrom/changeset/1838 Log: Add support for S25FL128P, S25FL129P and refine it for S25FL128S chips. Additionally to the existing S25FL128S......0 definition this patch adds S25FL128P......0, S25FL128P......1 and S25FL128S......1, as well as S25FL129P......0 and S25FL129P......1 definitions. S25FL12xP seem to be the predecessor families of S25FL128S. All associated chips can not be distinguished with RDID alone. Besides the new chips, this patch also fixes the name of the previously supported S25FL128S model with uniform 256 kB sectors (S25FL128P......1 not 0) and adds the hybrid sector version (0) as well. Due to the shared IDs the user has to select the right chip manually with the -c parameter. To make this even possible, this patch enlarges the respective array for results to 6. Tested-by: Antonio Ospite <ao2(a)ao2.it> with a S25FL129P......0. Signed-off-by: Stefan Tauner <stefan.tauner(a)alumni.tuwien.ac.at> Acked-by: Stefan Tauner <stefan.tauner(a)alumni.tuwien.ac.at> Modified: trunk/cli_classic.c trunk/flashchips.c trunk/flashchips.h Modified: trunk/cli_classic.c ============================================================================== --- trunk/cli_classic.c Wed Aug 6 01:28:47 2014 (r1837) +++ trunk/cli_classic.c Wed Aug 6 16:36:27 2014 (r1838) @@ -94,7 +94,7 @@ unsigned long size; /* Probe for up to three flash chips. */ const struct flashchip *chip = NULL; - struct flashctx flashes[3] = {{0}}; + struct flashctx flashes[6] = {{0}}; struct flashctx *fill_flash; const char *name; int namelen, opt, i, j; Modified: trunk/flashchips.c ============================================================================== --- trunk/flashchips.c Wed Aug 6 01:28:47 2014 (r1837) +++ trunk/flashchips.c Wed Aug 6 16:36:27 2014 (r1838) @@ -10389,7 +10389,69 @@ { .vendor = "Spansion", - .name = "S25FL128S......0", /* uniform 256kB sectors */ + .name = "S25FL128P......0", /* uniform 64 kB sectors */ + .bustype = BUS_SPI, + .manufacture_id = SPANSION_ID, + .model_id = SPANSION_S25FL128, + .total_size = 16384, + .page_size = 256, + .feature_bits = FEATURE_WRSR_WREN, + .tested = TEST_UNTESTED, + .probe = probe_spi_rdid, + .probe_timing = TIMING_ZERO, + .block_erasers = { + { + .eraseblocks = { {64 * 1024, 256} }, + .block_erase = spi_block_erase_20, + }, { + .eraseblocks = { {64 * 1024, 256} }, + .block_erase = spi_block_erase_d8, + }, { + .eraseblocks = { { 16384 * 1024, 1} }, + .block_erase = spi_block_erase_60, + }, { + .eraseblocks = { { 16384 * 1024, 1} }, + .block_erase = spi_block_erase_c7, + } + }, + .printlock = spi_prettyprint_status_register_bp3_srwd, + .unlock = spi_disable_blockprotect_bp3_srwd, + .write = spi_chip_write_256, + .read = spi_chip_read, /* Fast read (0x0B) supported */ + .voltage = {2700, 3600}, + }, + + { + .vendor = "Spansion", + .name = "S25FL128P......1", /* uniform 256kB sectors */ + .bustype = BUS_SPI, + .manufacture_id = SPANSION_ID, + .model_id = SPANSION_S25FL128, + .total_size = 16384, + .page_size = 256, + .feature_bits = FEATURE_WRSR_WREN, + .tested = TEST_UNTESTED, + .probe = probe_spi_rdid, + .probe_timing = TIMING_ZERO, + .block_erasers = { + { + .eraseblocks = { {256 * 1024, 64} }, + .block_erase = spi_block_erase_d8, + }, { + .eraseblocks = { { 16384 * 1024, 1} }, + .block_erase = spi_block_erase_c7, + } + }, + .printlock = spi_prettyprint_status_register_bp2_srwd, + .unlock = spi_disable_blockprotect_bp2_srwd, + .write = spi_chip_write_256, + .read = spi_chip_read, /* Fast read (0x0B) supported */ + .voltage = {2700, 3600}, + }, + + { + .vendor = "Spansion", + .name = "S25FL128S......0", /* hybrid: 32 (top or bottom) 4 kB sub-sectors + 64 kB sectors */ .bustype = BUS_SPI, .manufacture_id = SPANSION_ID, .model_id = SPANSION_S25FL128, @@ -10403,9 +10465,48 @@ .probe_timing = TIMING_ZERO, .block_erasers = { { - .eraseblocks = { {4 * 1024, 4096} }, + /* This chip supports erasing of the 32 so-called "parameter sectors" with + * opcode 0x20. Trying to access an address outside these 4kB blocks does + * have no effect on the memory contents, but sets a flag in the SR. + .eraseblocks = { + {4 * 1024, 32}, + {64 * 1024, 254} // inaccessible + }, .block_erase = spi_block_erase_20, + }, { */ + .eraseblocks = { { 64 * 1024, 256} }, + .block_erase = spi_block_erase_d8, }, { + .eraseblocks = { { 16384 * 1024, 1} }, + .block_erase = spi_block_erase_60, + }, { + .eraseblocks = { { 16384 * 1024, 1} }, + .block_erase = spi_block_erase_c7, + } + }, + .printlock = spi_prettyprint_status_register_bp2_ep_srwd, /* TODO: SR2 and many others */ + .unlock = spi_disable_blockprotect_bp2_srwd, /* TODO: various other locks */ + .write = spi_chip_write_256, /* Multi I/O supported */ + .read = spi_chip_read, /* Fast read (0x0B) and multi I/O supported */ + .voltage = {2700, 3600}, + }, + + { + .vendor = "Spansion", + .name = "S25FL128S......1", /* uniform 256 kB sectors */ + .bustype = BUS_SPI, + .manufacture_id = SPANSION_ID, + .model_id = SPANSION_S25FL128, + .total_size = 16384, + .page_size = 512, + /* supports 4B addressing */ + /* OTP: 1024B total, 32B reserved; read 0x4B; write 0x42 */ + .feature_bits = FEATURE_WRSR_WREN | FEATURE_OTP, + .tested = TEST_UNTESTED, + .probe = probe_spi_rdid, + .probe_timing = TIMING_ZERO, + .block_erasers = { + { .eraseblocks = { {256 * 1024, 64} }, .block_erase = spi_block_erase_d8, }, { @@ -10421,6 +10522,86 @@ .write = spi_chip_write_256, /* Multi I/O supported */ .read = spi_chip_read, /* Fast read (0x0B) and multi I/O supported */ .voltage = {2700, 3600}, + }, + + { + .vendor = "Spansion", + .name = "S25FL129P......0", /* hybrid: 32 (top or bottom) 4 kB sub-sectors + 64 kB sectors */ + .bustype = BUS_SPI, + .manufacture_id = SPANSION_ID, + .model_id = SPANSION_S25FL128, + .total_size = 16384, + .page_size = 256, + /* OTP: 506B total, 16B reserved; read 0x4B; write 0x42 */ + .feature_bits = FEATURE_WRSR_WREN | FEATURE_OTP, + .tested = TEST_OK_PREW, + .probe = probe_spi_rdid, + .probe_timing = TIMING_ZERO, + .block_erasers = { + { + /* FIXME: This chip supports erasing of the 32 so-called "parameter sectors" with + * opcode 0x20. Trying to access an address outside these 4kB blocks does have no + * effect on the memory contents, but sets a flag in the SR. + .eraseblocks = { + {4 * 1024, 32}, + {64 * 1024, 254} // inaccessible + }, + .block_erase = spi_block_erase_20, + }, { */ + /* FIXME: Additionally it also supports erase opcode 40h for the respective 2*4 kB pairs + .eraseblocks = { + {8 * 1024, 16}, + {64 * 1024, 254} // inaccessible + }, + .block_erase = spi_block_erase_40, + }, { */ + .eraseblocks = { { 64 * 1024, 256} }, + .block_erase = spi_block_erase_d8, + }, { + .eraseblocks = { { 16384 * 1024, 1} }, + .block_erase = spi_block_erase_60, + }, { + .eraseblocks = { { 16384 * 1024, 1} }, + .block_erase = spi_block_erase_c7, + } + }, + .printlock = spi_prettyprint_status_register_bp2_ep_srwd, /* TODO: Configuration register */ + .unlock = spi_disable_blockprotect_bp2_srwd, + .write = spi_chip_write_256, /* Multi I/O supported */ + .read = spi_chip_read, /* Fast read (0x0B) and multi I/O supported */ + .voltage = {2700, 3600}, + }, + + { + .vendor = "Spansion", + .name = "S25FL129P......1", /* uniform 256 kB sectors */ + .bustype = BUS_SPI, + .manufacture_id = SPANSION_ID, + .model_id = SPANSION_S25FL128, + .total_size = 16384, + .page_size = 512, + /* OTP: 506B total, 16B reserved; read 0x4B; write 0x42 */ + .feature_bits = FEATURE_WRSR_WREN | FEATURE_OTP, + .tested = TEST_UNTESTED, + .probe = probe_spi_rdid, + .probe_timing = TIMING_ZERO, + .block_erasers = { + { + .eraseblocks = { {256 * 1024, 64} }, + .block_erase = spi_block_erase_d8, + }, { + .eraseblocks = { { 16384 * 1024, 1} }, + .block_erase = spi_block_erase_60, + }, { + .eraseblocks = { { 16384 * 1024, 1} }, + .block_erase = spi_block_erase_c7, + } + }, + .printlock = spi_prettyprint_status_register_bp2_ep_srwd, /* TODO: Configuration register */ + .unlock = spi_disable_blockprotect_bp2_srwd, + .write = spi_chip_write_256, /* Multi I/O supported */ + .read = spi_chip_read, /* Fast read (0x0B) and multi I/O supported */ + .voltage = {2700, 3600}, }, { Modified: trunk/flashchips.h ============================================================================== --- trunk/flashchips.h Wed Aug 6 01:28:47 2014 (r1837) +++ trunk/flashchips.h Wed Aug 6 16:36:27 2014 (r1838) @@ -590,7 +590,7 @@ #define SPANSION_S25FL016A 0x0214 #define SPANSION_S25FL032A 0x0215 /* Same as S25FL032P, but the latter supports EDI and CFI */ #define SPANSION_S25FL064A 0x0216 /* Same as S25FL064P, but the latter supports EDI and CFI */ -#define SPANSION_S25FL128 0x2018 +#define SPANSION_S25FL128 0x2018 /* Same ID for various S25FL128P, S25FL128S and S25FL129P (including dual-die S70FL256P) variants (EDI supported) */ #define SPANSION_S25FL256 0x0219 #define SPANSION_S25FL512 0x0220 #define SPANSION_S25FL204 0x4013

1 0

Re: [flashrom] [PATCH] ichspi: fix missing set_addr on erases and possible crossings of 256 B boundaries.
by Stefan Tauner Aug. 5, 2014

Aug. 5, 2014

On Sun, 3 Aug 2014 04:36:02 +0200 Stefan Tauner <stefan.tauner(a)alumni.tuwien.ac.at> wrote: > Apparently the erase function did never set any address before issuing the > erase commands. How could this ever work? > Also, according to PCH documentation crossing 256 byte boundaries is invalid > and may cause wraparound due to the flash chip's pages. Check for this on > reads as well as writes. > > Thanks to Vladimir 'φ-coder/phcoder' Serbinenko for noticing these issues > and providing the initial patch. > > Signed-off-by: Stefan Tauner <stefan.tauner(a)alumni.tuwien.ac.at> > Acked-by: Stefan Tauner <stefan.tauner(a)alumni.tuwien.ac.at> Committed in r1837. Thanks again, phcoder! -- Kind regards/Mit freundlichen Grüßen, Stefan Tauner

1 0

[commit] r1837 - trunk
by repository service Aug. 5, 2014

Aug. 5, 2014

Author: stefanct Date: Wed Aug 6 01:28:47 2014 New Revision: 1837 URL: http://flashrom.org/trac/flashrom/changeset/1837 Log: ichspi: fix missing set_addr on erases and possible crossings of 256 B boundaries. Apparently the erase function did never set any address before issuing the erase commands. How could this ever work? Also, according to PCH documentation crossing 256 byte boundaries is invalid and may cause wraparound due to the flash chip's pages. Check for this on reads as well as writes. Thanks to Vladimir 'φ-coder/phcoder' Serbinenko for noticing these issues and providing the initial patch. Signed-off-by: Stefan Tauner <stefan.tauner(a)alumni.tuwien.ac.at> Acked-by: Stefan Tauner <stefan.tauner(a)alumni.tuwien.ac.at> Modified: trunk/ichspi.c Modified: trunk/ichspi.c ============================================================================== --- trunk/ichspi.c Wed Aug 6 00:16:01 2014 (r1836) +++ trunk/ichspi.c Wed Aug 6 01:28:47 2014 (r1837) @@ -1272,6 +1272,7 @@ } msg_pdbg("Erasing %d bytes starting at 0x%06x.\n", len, addr); + ich_hwseq_set_addr(addr); /* make sure FDONE, FCERR, AEL are cleared by writing 1 to them */ REGWRITE16(ICH9_REG_HSFS, REGREAD16(ICH9_REG_HSFS)); @@ -1307,7 +1308,11 @@ REGWRITE16(ICH9_REG_HSFS, REGREAD16(ICH9_REG_HSFS)); while (len > 0) { + /* Obey programmer limit... */ block_len = min(len, flash->mst->opaque.max_data_read); + /* as well as flash chip page borders as demanded in the Intel datasheets. */ + block_len = min(block_len, 256 - (addr & 0xFF)); + ich_hwseq_set_addr(addr); hsfc = REGREAD16(ICH9_REG_HSFC); hsfc &= ~HSFC_FCYCLE; /* set read operation */ @@ -1345,7 +1350,10 @@ while (len > 0) { ich_hwseq_set_addr(addr); + /* Obey programmer limit... */ block_len = min(len, flash->mst->opaque.max_data_write); + /* as well as flash chip page borders as demanded in the Intel datasheets. */ + block_len = min(block_len, 256 - (addr & 0xFF)); ich_fill_data(buf, block_len, ICH9_REG_FDATA0); hsfc = REGREAD16(ICH9_REG_HSFC); hsfc &= ~HSFC_FCYCLE; /* clear operation */

1 0

[commit] r1836 - trunk
by repository service Aug. 5, 2014

Aug. 5, 2014

Author: stefanct Date: Wed Aug 6 00:16:01 2014 New Revision: 1836 URL: http://flashrom.org/trac/flashrom/changeset/1836 Log: linux_spi: properly convert pointers to kernel's u64. For arm64 with 32-bit userspace, pointers such as 0xff96ebf8 were incorrectly getting converted to u64_t 0xffffffffff96ebf8 in the spi_ioc_transfer struct which was causing ioctl()s to be rejected by the kernel. With this patch we first cast to uintptr_t (to avoid warnings on architectures where char * are not 64b wide) and then to uint64_t which is always big enough and does not produce warnings. This patch is taken from ChromiumOS' Change-Id: I5a15b4ca5d9657c3cb1ddccd42eafd91c852dd26 Signed-off-by: David Riley <davidriley(a)chromium.org> Reviewed-by: David Hendricks <dhendrix(a)chromium.org> Acked-by: Stefan Tauner <stefan.tauner(a)alumni.tuwien.ac.at> Modified: trunk/linux_spi.c Modified: trunk/linux_spi.c ============================================================================== --- trunk/linux_spi.c Sun Aug 3 16:15:14 2014 (r1835) +++ trunk/linux_spi.c Wed Aug 6 00:16:01 2014 (r1836) @@ -142,11 +142,11 @@ int iocontrol_code; struct spi_ioc_transfer msg[2] = { { - .tx_buf = (uint64_t)(ptrdiff_t)txbuf, + .tx_buf = (uint64_t)(uintptr_t)txbuf, .len = writecnt, }, { - .rx_buf = (uint64_t)(ptrdiff_t)rxbuf, + .rx_buf = (uint64_t)(uintptr_t)rxbuf, .len = readcnt, }, };

1 0

Re: [flashrom] Where is read lock typically implemented?
by Ross Amans Aug. 5, 2014

Aug. 5, 2014

David - The PCH is enforcing all these write / read protections. The SPI controller lives in the PCH. Ross On Tue, Aug 5, 2014 at 12:23 PM, David Hendricks <dhendrix(a)google.com> wrote: > On Tue, Aug 5, 2014 at 9:55 AM, TR Reardon <thomas_reardon(a)hotmail.com> > wrote: > >> I tried looking for an answer to this but a quick one didn't surface: >> when flash regions are protected, is this implemented by the flash chip >> itself or by the SPI logic (ie ichspi for Intel programmer)? >> > > It can be implemented by either, though in most cases it's the SPI > controller on Intel systems. > > -- > David Hendricks (dhendrix) > Systems Software Engineer, Google Inc. > > _______________________________________________ > flashrom mailing list > flashrom(a)flashrom.org > http://www.flashrom.org/mailman/listinfo/flashrom > -- Ross Amans Systems Engineer Newisys div of Sanmina 5385 Mark Dabling blvd Colorado Springs, CO 80918 719-884-8714 (work) 719-287-4446 (cell) CONFIDENTIALITY This e-mail message and any attachments thereto, is intended only for use by the addressee(s) named herein and may contain legally privileged and/or confidential information. If you are not the intended recipient of this e-mail message, you are hereby notified that any dissemination, distribution or copying of this e-mail message, and any attachments thereto, is strictly prohibited. If you have received this e-mail message in error, please immediately notify the sender and permanently delete the original and any copies of this email and any prints thereof. ABSENT AN EXPRESS STATEMENT TO THE CONTRARY HEREINABOVE, THIS E-MAIL IS NOT INTENDED AS A SUBSTITUTE FOR A WRITING. Notwithstanding the Uniform Electronic Transactions Act or the applicability of any other law of similar substance and effect, absent an express statement to the contrary hereinabove, this e-mail message its contents, and any attachments hereto are not intended to represent an offer or acceptance to enter into a contract and are not otherwise intended to bind the sender, Sanmina Corporation (or any of its subsidiaries), or any other person or entity.

1 0

Where is read lock typically implemented?
by TR Reardon Aug. 5, 2014

Aug. 5, 2014

I tried looking for an answer to this but a quick one didn't surface: when flash regions are protected, is this implemented by the flash chip itself or by the SPI logic (ie ichspi for Intel programmer)?

4 5

Re: [flashrom] Avoton patches for flashrom
by TR Reardon Aug. 5, 2014

Aug. 5, 2014

On Mon, Aug 4, 2014 at 7:49 PM, Stefan Tauner <stefan.tauner(a)alumni.tuwien.ac.at> wrote: > On Mon, 4 Aug 2014 19:18:04 -0400 > TR Reardon <...> wrote: > >> cool, just tried and was enable to read the descriptor (64K) and then >> the actual BIOS (6MB). I had to edit out the write-only check in >> cli_classic.c, assumed that was relatively safe. > > I have no idea what you are talking about, sorry. > Er... flashrom currently only allows using the layout codepaths in write mode; there is a check for this in cli_classic.c, which I disabled. The log I sent previously described two regions, a "descriptor" region at 0-ffff (64K), and the BIOS itself at 200000-7fffff (6M). When I use --verify, unlike --read, it fails. The final line of output (the only difference) is: Verifying flash... FAILED at 0x00000000! Expected=0xff, Found=0x00, failed byte count from 0x00000000-0x007fffff: 0x7d0bdf Logs attached

1 0

Re: [flashrom] Avoton patches for flashrom
by Stefan Tauner Aug. 4, 2014

Aug. 4, 2014

On Mon, 4 Aug 2014 19:18:04 -0400 TR Reardon <reardonia(a)gmail.com> wrote: > cool, just tried and was enable to read the descriptor (64K) and then > the actual BIOS (6MB). I had to edit out the write-only check in > cli_classic.c, assumed that was relatively safe. I have no idea what you are talking about, sorry. > The --verify function doesn't work per se, but the files I was able to > extract using --read matched. I'd be interested in details... like logs ;) > The BIOS files mostly seem to match what is posted by ASUS online. > That is, other than configuration areas which I don't understand. I > assume that when a tool like BUPDATER from Asus reads a BIOS update > file, it doesn't simply overwrite the entire BIOS area? I mean, there > are quite a few 64KB configuration areas containing things like the > motherboard name. Or is this relatively safe to overwrite? Only ASUS knows. It might be completely save to overwrite such areas if the firmware initializes them with a default configuration... but I would not try that without a way to recover (e.g. external programmer... which should be easy in your case because the flash is socketed). -- Kind regards/Mit freundlichen Grüßen, Stefan Tauner

1 0

Re: [flashrom] Avoton patches for flashrom
by Stefan Tauner Aug. 4, 2014

Aug. 4, 2014

On Mon, 4 Aug 2014 15:59:40 -0400 TR Reardon <reardonia(a)gmail.com> wrote: > Hi, I'm wondering if your patches from last fall for layout support for > read have been updated? I'd like to try with your Avoton patches but the > merge is non-trivial I have rebased my local branch to current HEAD for you. It compiles but I have not checked anything else -> no promises whatsoever. You can find it on my github: https://github.com/stefanct/flashrom/tree/layout They should not conflict with the Intel patches. Hope that helps. -- Kind regards/Mit freundlichen Grüßen, Stefan Tauner

2 1