On Fri, Oct 23, 2009 at 03:33:33PM +0200, Carl-Daniel Hailfinger wrote: > OK, completely rewritten attempt. I'm reusing Uwe's changelog.
Most of it looks good, but there are some changeѕ required. - The checks have to be done later in the code, after the chip has been detected, so that we only check the sizes for the chip we actually find, not for all the chips we probe for. I moved the checks and simplified them a bit (I hope). - Use printf instead of printf_debug so that users will see the error without having to use -V. - Abort if the chip is bigger than the supported ROM decode size. Continuing will result in a non-bootable system in almost all cases. Adding a --force option later is fine with me, but I think the default should be to abort. Attached is an updated patch with the above changes. I tested it on the Shuttle AK38N (all operations), output looks like this: $ flashrom v0.9.1-r753 No coreboot table found. Found chipset "VIA VT8235", enabling flash write... OK. This chipset supports the following protocols: Non-SPI. Disabling flash write protection for board "Shuttle AK38N"... OK. Calibrating delay loop... OK. Found chip "SST SST39SF040" (512 KB, Parallel) at physical address 0xfff80000. Chipset/board/programmer only supports a max. size of 256 KB. ABORTING. I also tested that with a 256KB chip all operations still work fine. Additionally I tested a 512KB chip on ASUS P4B266 (FWH) with a (simulated) max. decode size of 256KB and 512KB to ensure that non-parallel chips also work OK with the code. The updated patch is Acked-by: Uwe Hermann <[email protected]> Feel free to commit. > @@ -977,6 +1004,8 @@ > size = flash->total_size * 1024; > buf = (uint8_t *) calloc(size, sizeof(char)); > > + // FIXME: Check for decode size again? > + > if (erase_it) { > if (flash->tested & TEST_BAD_ERASE) { > fprintf(stderr, "Erase is not working on this chip. "); Not sure about this one. Why would an additional check be needed? According to my tests there's no need for additional checks. Uwe. -- http://www.hermann-uwe.de | http://www.randomprojects.org http://www.crazy-hacks.org | http://www.unmaintained-free-software.org
Add a facility to check and report to the user the maximum supported decode size for chipsets and tested mainboards. The rationale is to warn users when they, for example, try to flash a 512KB parallel flash chip but their chipset only supports 256KB, or they try to flash 512KB and the chipset _does_ theoretically support 512KB but their special board doesn't wire all address lines and thus supports only 256 KB ROM chips at maximum. This has cost Uwe hours of debugging on some board already, until he figured out what was going on, we should try warn our users where possible about this (yes, this is "best effort", but still better than not to warn at all). Signed-off-by: Carl-Daniel Hailfinger <[email protected]> Index: flash.h =================================================================== --- flash.h (revision 753) +++ flash.h (working copy) @@ -352,9 +352,17 @@ void sio_mask(uint16_t port, uint8_t reg, uint8_t data, uint8_t mask); int board_flash_enable(const char *vendor, const char *part); +struct decode_sizes { + uint32_t parallel; + uint32_t lpc; + uint32_t fwh; + uint32_t spi; +}; + /* chipset_enable.c */ extern enum chipbustype buses_supported; int chipset_flash_enable(void); +extern struct decode_sizes max_rom_decode; extern unsigned long flashbase; Index: chipset_enable.c =================================================================== --- chipset_enable.c (revision 753) +++ chipset_enable.c (working copy) @@ -41,6 +41,12 @@ */ enum chipbustype buses_supported = CHIP_BUSTYPE_NONSPI; +struct decode_sizes max_rom_decode = { + .parallel = 0xffffffff, + .lpc = 0xffffffff, + .fwh = 0xffffffff, + .spi = 0xffffffff +}; extern int ichspi_lock; @@ -204,34 +210,73 @@ uint32_t fwh_conf; int i; char *idsel = NULL; + int tmp; + int max_decode_fwh_idsel = 0; + int max_decode_fwh_decode = 0; + int contiguous = 1; - /* Ignore all legacy ranges below 1 MB. */ + /* Ignore all legacy ranges below 1 MB. + * We currently only support flashing the chip which responds to + * IDSEL=0. To support IDSEL!=0, flashbase and decode size calculations + * have to be adjusted. + */ /* FWH_SEL1 */ fwh_conf = pci_read_long(dev, 0xd0); - for (i = 7; i >= 0; i--) + for (i = 7; i >= 0; i--) { + tmp = (fwh_conf >> (i * 4)) & 0xf; printf_debug("\n0x%08x/0x%08x FWH IDSEL: 0x%x", (0x1ff8 + i) * 0x80000, (0x1ff0 + i) * 0x80000, - (fwh_conf >> (i * 4)) & 0xf); + tmp); + if ((tmp == 0) && contiguous) { + max_decode_fwh_idsel = (8 - i) * 0x80000; + } else { + contiguous = 0; + } + } /* FWH_SEL2 */ fwh_conf = pci_read_word(dev, 0xd4); - for (i = 3; i >= 0; i--) + for (i = 3; i >= 0; i--) { + tmp = (fwh_conf >> (i * 4)) & 0xf; printf_debug("\n0x%08x/0x%08x FWH IDSEL: 0x%x", (0xff4 + i) * 0x100000, (0xff0 + i) * 0x100000, - (fwh_conf >> (i * 4)) & 0xf); + tmp); + if ((tmp == 0) && contiguous) { + max_decode_fwh_idsel = (8 - i) * 0x100000; + } else { + contiguous = 0; + } + } + contiguous = 1; /* FWH_DEC_EN1 */ fwh_conf = pci_read_word(dev, 0xd8); - for (i = 7; i >= 0; i--) + for (i = 7; i >= 0; i--) { + tmp = (fwh_conf >> (i + 0x8)) & 0x1; printf_debug("\n0x%08x/0x%08x FWH decode %sabled", (0x1ff8 + i) * 0x80000, (0x1ff0 + i) * 0x80000, - (fwh_conf >> (i + 0x8)) & 0x1 ? "en" : "dis"); - for (i = 3; i >= 0; i--) + tmp ? "en" : "dis"); + if ((tmp == 0) && contiguous) { + max_decode_fwh_decode = (8 - i) * 0x80000; + } else { + contiguous = 0; + } + } + for (i = 3; i >= 0; i--) { + tmp = (fwh_conf >> i) & 0x1; printf_debug("\n0x%08x/0x%08x FWH decode %sabled", (0xff4 + i) * 0x100000, (0xff0 + i) * 0x100000, - (fwh_conf >> i) & 0x1 ? "en" : "dis"); + tmp ? "en" : "dis"); + if ((tmp == 0) && contiguous) { + max_decode_fwh_decode = (8 - i) * 0x100000; + } else { + contiguous = 0; + } + } + max_rom_decode.fwh = min(max_decode_fwh_idsel, max_decode_fwh_decode); + printf_debug("\nMaximum FWH chip size: 0x%x bytes", max_rom_decode.fwh); if (programmer_param) idsel = strstr(programmer_param, "fwh_idsel="); @@ -244,6 +289,7 @@ printf("\nSetting IDSEL=0x%x for top 16 MB", fwh_conf); pci_write_long(dev, 0xd0, fwh_conf); pci_write_word(dev, 0xd4, fwh_conf); + /* FIXME: Decode settings are not changed. */ } return enable_flash_ich(dev, name, 0xdc); @@ -536,6 +582,9 @@ reg8 |= BIOS_ROM_POSITIVE_DECODE; pci_write_byte(dev, DECODE_CONTROL_REG2, reg8); + /* No idea which buses this limit applies to. */ + //max_rom_decode.lpc = 16 * 1024 * 1024; + return 0; } Index: flashrom.c =================================================================== --- flashrom.c (revision 753) +++ flashrom.c (working copy) @@ -401,7 +401,9 @@ struct flashchip *probe_flash(struct flashchip *first_flash, int force) { struct flashchip *flash; - unsigned long base = 0, size; + unsigned long base = 0; + unsigned int size = 0, tmpsize = 0xffffffff; + enum chipbustype buses_common = 0; char *tmp; for (flash = first_flash; flash && flash->name; flash++) { @@ -413,7 +415,8 @@ printf_debug("failed! flashrom has no probe function for this flash chip.\n"); continue; } - if (!(buses_supported & flash->bustype)) { + buses_common = buses_supported & flash->bustype; + if (!buses_common) { tmp = flashbuses_to_text(buses_supported); printf_debug("skipped. Host bus type %s ", tmp); free(tmp); @@ -449,6 +452,25 @@ flash->vendor, flash->name, flash->total_size, flashbuses_to_text(flash->bustype), base); + if ((buses_common & CHIP_BUSTYPE_PARALLEL) && + (max_rom_decode.parallel < flash->total_size * 1024)) + tmpsize = max_rom_decode.parallel; + if ((buses_common & CHIP_BUSTYPE_LPC) && + (max_rom_decode.lpc < flash->total_size * 1024)) + tmpsize = max_rom_decode.lpc; + if ((buses_common & CHIP_BUSTYPE_FWH) && + (max_rom_decode.fwh < flash->total_size * 1024)) + tmpsize = max_rom_decode.fwh; + if ((buses_common & CHIP_BUSTYPE_SPI) && + (max_rom_decode.spi < flash->total_size * 1024)) + tmpsize = max_rom_decode.spi; + if (tmpsize != 0xffffffff) { + printf("Chipset/board/programmer only supports " + "a max. size of %u KB. ABORTING.\n", tmpsize / 1024); + programmer_shutdown(); + exit(1); + } + return flash; } @@ -977,6 +999,8 @@ size = flash->total_size * 1024; buf = (uint8_t *) calloc(size, sizeof(char)); + // FIXME: Check for decode size again? + if (erase_it) { if (flash->tested & TEST_BAD_ERASE) { fprintf(stderr, "Erase is not working on this chip. "); Index: board_enable.c =================================================================== --- board_enable.c (revision 753) +++ board_enable.c (working copy) @@ -804,16 +804,24 @@ } /** - * Suited for: - * - Shuttle AK38N: VIA KT333CF + VIA VT8235 + ITE IT8705F - * - Elitegroup K7VTA3: VIA Apollo KT266/A/333 + VIA VT8235 + ITE IT8705F + * Suited for: Elitegroup K7VTA3: VIA Apollo KT266/A/333 + VIA VT8235 + ITE IT8705F */ -static int it8705f_write_enable_2e(const char *name) +static int elitegroup_k7vta3(const char *name) { + max_rom_decode.parallel = 256 * 1024; return it8705f_write_enable(0x2e, name); } /** + * Suited for: Shuttle AK38N: VIA KT333CF + VIA VT8235 + ITE IT8705F + */ +static int shuttle_ak38n(const char *name) +{ + max_rom_decode.parallel = 256 * 1024; + return it8705f_write_enable(0x2e, name); +} + +/** * Find the runtime registers of an SMSC Super I/O, after verifying its * chip ID. * @@ -1079,7 +1087,7 @@ {0x10DE, 0x0030, 0x1043, 0x818a, 0x8086, 0x100E, 0x1043, 0x80EE, NULL, NULL, "ASUS", "P5ND2-SLI Deluxe", board_asus_p5nd2_sli}, {0x1106, 0x3149, 0x1565, 0x3206, 0x1106, 0x3344, 0x1565, 0x1202, NULL, NULL, "Biostar", "P4M80-M4", it8705_rom_write_enable}, {0x8086, 0x3590, 0x1028, 0x016c, 0x1000, 0x0030, 0x1028, 0x016c, NULL, NULL, "Dell", "PowerEdge 1850", ich5_gpio23_raise}, - {0x1106, 0x3038, 0x1019, 0x0996, 0x1106, 0x3177, 0x1019, 0x0996, NULL, NULL, "Elitegroup", "K7VTA3", it8705f_write_enable_2e}, + {0x1106, 0x3038, 0x1019, 0x0996, 0x1106, 0x3177, 0x1019, 0x0996, NULL, NULL, "Elitegroup", "K7VTA3", elitegroup_k7vta3}, {0x1106, 0x3177, 0x1106, 0x3177, 0x1106, 0x3059, 0x1695, 0x3005, NULL, NULL, "EPoX", "EP-8K5A2", board_epox_ep_8k5a2}, {0x10EC, 0x8139, 0x1695, 0x9001, 0x11C1, 0x5811, 0x1695, 0x9015, NULL, NULL, "EPoX", "EP-8RDA3+", board_epox_ep_8rda3plus}, {0x8086, 0x7110, 0, 0, 0x8086, 0x7190, 0, 0, "epox", "ep-bx3", "EPoX", "EP-BX3", board_epox_ep_bx3}, @@ -1105,7 +1113,7 @@ {0x1106, 0x0571, 0x1462, 0x7120, 0, 0, 0, 0, "msi", "kt4v", "MSI", "MS-6712 (KT4V)", board_msi_kt4v}, {0x8086, 0x2658, 0x1462, 0x7046, 0x1106, 0x3044, 0x1462, 0x046d, NULL, NULL, "MSI", "MS-7046", ich6_gpio19_raise}, {0x10de, 0x005e, 0, 0, 0, 0, 0, 0, "msi", "k8n-neo3", "MSI", "MS-7135 (K8N Neo3)", w83627thf_gpio4_4_raise_4e}, - {0x1106, 0x3104, 0x1297, 0xa238, 0x1106, 0x3059, 0x1297, 0xc063, NULL, NULL, "Shuttle", "AK38N", it8705f_write_enable_2e}, + {0x1106, 0x3104, 0x1297, 0xa238, 0x1106, 0x3059, 0x1297, 0xc063, NULL, NULL, "Shuttle", "AK38N", shuttle_ak38n}, {0x10DE, 0x0050, 0x1297, 0x5036, 0x1412, 0x1724, 0x1297, 0x5036, NULL, NULL, "Shuttle", "FN25", board_shuttle_fn25}, {0x1106, 0x3038, 0x0925, 0x1234, 0x1106, 0x3058, 0x15DD, 0x7609, NULL, NULL, "Soyo", "SY-7VCA", board_soyo_sy_7vca}, {0x8086, 0x1076, 0x8086, 0x1176, 0x1106, 0x3059, 0x10f1, 0x2498, NULL, NULL, "Tyan", "S2498 (Tomcat K7M)", board_asus_a7v8x_mx},
_______________________________________________ flashrom mailing list [email protected] http://www.flashrom.org/mailman/listinfo/flashrom
