flashrom

flashrom@flashrom.org

9164 discussions

lspci dumps of VT8237S with SPI
by Stefan Tauner 21 Aug '12

21 Aug '12

Here are two logs from Rudolf Marek… > Hi some very old dumps. new_lspci is coreboot. lspci xxxvvv is vendor > bios. the board uses SPI flash. -- Kind regards/Mit freundlichen Grüßen, Stefan Tauner

1 0

ASRock 4CoreDual-SATA2 R2.0: VT8237S flashing
by Carl-Daniel Hailfinger 21 Aug '12

21 Aug '12

Hi Jan-Hendrik, I noticed that you own an ASRock 4CoreDual-SATA2 R2.0 which has the VIA VT8237S chipset and a LPC/FWH BIOS flash chip. We flashrom developers are currently trying to cross-check the VT8237S documentation with real hardware settings, and it would be very helpful for us if you could mail us (just hit reply-all to send to the flashrom(a)flashrom.org mailing list) the output of the following command (as root): lspci -nnvvvxxx Just in case you've never heard about flashrom: flashrom is a tool to update your BIOS/EFI/coreboot from inside Linux/*BSD without the need to use a DOS boot disk. Your help is greatly appreciated. Regards, Carl-Daniel

2 1

NAND flash support
by jana1972＠centrum.cz 20 Aug '12

20 Aug '12

Is there NAND flash support available? Or is it a plan to add such support? By NAND I mean paralel NAND flash such as HY27US08121A or similar ( used in Xbox360 and other kind of boxes) BR, Ja.

2 1

Re: [flashrom] [PATCH] Refactor the -p internal:mainboard handling.
by Carl-Daniel Hailfinger 20 Aug '12

20 Aug '12

Am 13.08.2012 02:51 schrieb Stefan Tauner: > This patch gets rid of some global variables and makes lots of bits along > the code path that control the board enable execution more generic and > clearer. From now on flashrom also abort on a few occasions that should be > safer for the user. For example we abort if he specifies the mainboard (enable) > but we can not find the enable function. > > Parts of the board_match_cbname refactoring were done by Carl-Daniel. > > FIXME: manpage > > Signed-off-by: Stefan Tauner <stefan.tauner(a)student.tuwien.ac.at> > Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006(a)gmx.net> Acked-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006(a)gmx.net> with a few comments and if you fix the manpage. BIG FAT NOTE: You do not have to change all the stuff mentioned in the review (because that would mean I have to review the patch again), but it would be nice to have the review comments addressed in a followup patch. > --- > This might be kinda unreviewable... > i can probably split it up if need be, just tell me please. Big patch, but still... splitting it wouldn't have helped. > diff --git a/board_enable.c b/board_enable.c > index 9c16905..22d4072 100644 > --- a/board_enable.c > +++ b/board_enable.c > @@ -25,6 +25,7 @@ > */ > > #include <string.h> > +#include <stdlib.h> > #include "flash.h" > #include "programmer.h" > #include "hwaccess.h" > @@ -2443,41 +2444,63 @@ const struct board_match board_matches[] = { > { 0, 0, 0, 0, 0, 0, 0, 0, NULL, NULL, NULL, P3, NULL, NULL, 0, NT, NULL}, /* end marker */ > }; > > +/* Parse the <vendor>:<board> string specified by the user as part of -p internal:mainboard=<vendor>:<board>. > + * Parameters vendor and model will be overwritten. Returns 0 on success. > + * Note: strtok modifies the original string, so we work on a copy and allocate memory for the results. > + */ > +int board_parse_parameter(const char *boardstring, const char **vendor, const char **model) > +{ > + /* strtok may modify the original string. */ > + char *tempstr = strdup(boardstring); > + char *tempstr2 = NULL; > + strtok(tempstr, ":"); > + tempstr2 = strtok(NULL, ":"); > + if (tempstr == NULL || tempstr2 == NULL) { > + free(tempstr); While freeing only tempstr is correct, it's not obvious. > + msg_pinfo("Please supply the board vendor and model name with the " > + "-p internal:mainboard=<vendor>:<model> option.\n"); > + return 1; > + } > + *vendor = strdup(tempstr); > + *model = strdup(tempstr2); > + msg_pspew("-p internal:mainboard: vendor=\"%s\", model=\"%s\"\n", tempstr, tempstr2); > + free(tempstr); > + return 0; > +} > + > /* > - * Match boards on coreboot table gathered vendor and part name. > + * Match boards on vendor and model name. > + * Hint: the parameters can come either from the coreboot table or the command line (i.e. the user). Please add the following comment: vendor and model must be non-NULL. > * Require main PCI IDs to match too as extra safety. > */ > -static const struct board_match *board_match_cbname(const char *vendor, > - const char *part) > +static const struct board_match *board_match_name(const char *vendor, const char *model) > { > const struct board_match *board = board_matches; > const struct board_match *partmatch = NULL; > > for (; board->vendor_name; board++) { > - if (vendor && (!board->lb_vendor > - || strcasecmp(board->lb_vendor, vendor))) > + if (!board->lb_vendor || strcasecmp(board->lb_vendor, vendor)) > continue; > > - if (!board->lb_part || strcasecmp(board->lb_part, part)) > + if (!board->lb_part || strcasecmp(board->lb_part, model)) > continue; > > - if (!pci_dev_find(board->first_vendor, board->first_device)) > + if (!pci_dev_find(board->first_vendor, board->first_device)) { > + msg_pdbg("Odd. Board name \"%s\":\"%s\" matches, but first PCI device %04x:%04x " > + "doesn't.\n", vendor, model, board->first_vendor, board->first_device); Should we ask for a report here? Not sure, personal preference is not to ask. If yes, at which message level? > continue; > + } > > - if (board->second_vendor && > - !pci_dev_find(board->second_vendor, board->second_device)) > + if (!pci_dev_find(board->second_vendor, board->second_device)) { > + msg_pdbg("Odd. Board name \"%s\":\"%s\" matches, but second PCI device %04x:%04x " > + "doesn't.\n", vendor, model, board->second_vendor, board->second_device); > continue; > - > - if (vendor) > - return board; > + } > > if (partmatch) { > - /* a second entry has a matching part name */ > - msg_pinfo("AMBIGUOUS BOARD NAME: %s\n", part); > - msg_pinfo("At least vendors '%s' and '%s' match.\n", > - partmatch->lb_vendor, board->lb_vendor); > - msg_perr("Please use the full -p internal:mainboard=" > - "vendor:part syntax.\n"); > + /* More than one entry has a matching name. */ > + msg_perr("Board name \"%s\":\"%s\" and PCI IDs matched more than one board enable " > + "entry. Please report a bug at flashrom(a)flashrom.org\n", vendor, model); > return NULL; > } > partmatch = board; > @@ -2486,15 +2509,7 @@ static const struct board_match *board_match_cbname(const char *vendor, > if (partmatch) > return partmatch; > > - if (!partvendor_from_cbtable) { > - /* Only warn if the mainboard type was not gathered from the > - * coreboot table. If it was, the coreboot implementor is > - * expected to fix flashrom, too. > - */ > - msg_perr("\nUnknown vendor:board from -p internal:mainboard=" > - " programmer parameter:\n%s:%s\n\n", > - vendor, part); > - } > + msg_perr("No board enable found for vendor=\"%s\", model=\"%s\".\n", vendor, model); > return NULL; > } > > @@ -2534,9 +2549,10 @@ const static struct board_match *board_match_pci_ids(enum board_match_phase phas > > if (board->dmi_pattern) { > if (!has_dmi_support) { > - msg_perr("WARNING: Can't autodetect %s %s," > - " DMI info unavailable.\n", > + msg_perr("WARNING: Can't autodetect %s %s, DMI info unavailable.\n", > board->vendor_name, board->board_name); > + msg_pinfo("Please supply the board vendor and model name with the " > + "-p internal:mainboard=<vendor>:<model> option.\n"); > continue; > } else { > if (!dmi_match(board->dmi_pattern)) > @@ -2569,8 +2585,7 @@ static int unsafe_board_handler(const struct board_match *board) > return 1; > } > msg_pinfo("NOTE: Running an untested board enable procedure.\n" > - "Please report success/failure to flashrom(a)flashrom.org\n" > - "with your board name and SUCCESS or FAILURE in the subject.\n"); > + "Please report success/failure to flashrom(a)flashrom.org.\n"); > return 0; > } > > @@ -2606,29 +2621,29 @@ void board_handle_before_laptop(void) > board_handle_phase(P2); > } > > -int board_flash_enable(const char *vendor, const char *part) > +int board_flash_enable(const char *vendor, const char *model) > { > const struct board_match *board = NULL; > int ret = 0; > > - if (part) > - board = board_match_cbname(vendor, part); > - > - if (!board) > + if (vendor && model) { > + board = board_match_name(vendor, model); > + if (!board) /* if a board was given it has to match, else we abort here. */ > + return 1; > + } else { > board = board_match_pci_ids(P3); > + if (!board) /* i.e. there is just no board enable available for this board */ > + return 0; > + } > > - if (unsafe_board_handler(board)) > - board = NULL; > - > - if (board) { > + ret = unsafe_board_handler(board); Side note: unsafe_board_handler is a pretty dumb name. Yes, it's my fault. Still... check_unsafe_board_enable() or something similar would be a better name. > + if (!ret) { > if (board->max_rom_decode_parallel) > - max_rom_decode.parallel = > - board->max_rom_decode_parallel * 1024; > + max_rom_decode.parallel = board->max_rom_decode_parallel * 1024; > > if (board->enable != NULL) { > - msg_pinfo("Disabling flash write protection for " > - "board \"%s %s\"... ", board->vendor_name, > - board->board_name); > + msg_pinfo("Enabling full flash access for board \"%s %s\"... ", > + board->vendor_name, board->board_name); > > ret = board->enable(); > if (ret) > diff --git a/cbtable.c b/cbtable.c > index dde12ac..1f8b1c3 100644 > --- a/cbtable.c > +++ b/cbtable.c > @@ -24,57 +24,33 @@ > > #include <unistd.h> > #include <stdio.h> > -#include <stdlib.h> > #include <ctype.h> > #include <string.h> > #include "flash.h" > #include "programmer.h" > #include "coreboot_tables.h" > > -char *lb_part = NULL, *lb_vendor = NULL; > -int partvendor_from_cbtable = 0; > -static char *def_name = "DEFAULT"; > -static char *mainboard_vendor = NULL; > -static char *mainboard_part = NULL; > - > -/* Parse the [<vendor>:]<board> string specified by the user as part of > - * -p internal:mainboard=[<vendor>:]<board> and set lb_vendor and lb_part > - * to the extracted values. > - * Note: strtok modifies the original string, so we work on a copy and allocate > - * memory for lb_vendor and lb_part with strdup. > - */ > -void lb_vendor_dev_from_string(const char *boardstring) > -{ > - /* strtok may modify the original string. */ > - char *tempstr = strdup(boardstring); > - char *tempstr2 = NULL; > - strtok(tempstr, ":"); > - tempstr2 = strtok(NULL, ":"); > - if (tempstr2) { > - lb_vendor = strdup(tempstr); > - lb_part = strdup(tempstr2); > - } else { > - lb_vendor = NULL; > - lb_part = strdup(tempstr); > - } > - free(tempstr); > -} > +static char *cb_vendor = NULL, *cb_model = NULL; > > -int show_id(uint8_t *bios, int size) > +/* Tries to find coreboot IDs in the supplied image and compares them to the current IDs. > + * Returns... > + * -1 if IDs in the image do not match the IDs embedded in the current firmware, > + * 0 if the IDs could not be found in the image or if they match correctly. > + */ > +int cb_check_image(uint8_t *image, int size) cb_check_image_matches_board would be a better name IMHO. > { > + const char *image_vendor = NULL; > + const char *image_model = NULL; > unsigned int *walk; > unsigned int mb_part_offset, mb_vendor_offset; > char *mb_part, *mb_vendor; > > - mainboard_vendor = def_name; > - mainboard_part = def_name; > - > - walk = (unsigned int *)(bios + size - 0x10); > + walk = (unsigned int *)(image + size - 0x10); > walk--; The two lines above made me scream WTF?!? Since you're already fixing that code, can you please merge them into one? walk = (unsigned int *)(image + size - 0x14); > > if ((*walk) == 0 || ((*walk) & 0x3ff) != 0) { > - /* We might have an NVIDIA chipset BIOS which stores the ID information somewhere else. */ > - walk = (unsigned int *)(bios + size - 0x80); > + /* We might have an NVIDIA chipset which stores the ID information somewhere else. */ Actually, "BIOS" wasn't that wrong... let me rephrase the comment so that others may have a chance to understand the reason: /* Some NVIDIA chipsets store chipset soft straps (IIRC Hypertransport init info etc.) in flash at exactly the location where coreboot image size, coreboot vendor name pointer and coreboot board name pointer are usually stored. In this case coreboot uses an alternate location for the coreboot image data. */ > + walk = (unsigned int *)(image + size - 0x80); > walk--; And the two lines above need to be walk = (unsigned int *)(image + size - 0x84); > } > > @@ -88,49 +64,37 @@ int show_id(uint8_t *bios, int size) > mb_vendor_offset = *(walk - 2); > if ((*walk) == 0 || ((*walk) & 0x3ff) != 0 || (*walk) > size || > mb_part_offset > size || mb_vendor_offset > size) { > - msg_pinfo("Flash image seems to be a legacy BIOS. Disabling coreboot-related checks.\n"); > + msg_pdbg("Flash image seems to be a legacy BIOS. Disabling coreboot-related checks.\n"); > return 0; > } > > - mb_part = (char *)(bios + size - mb_part_offset); > - mb_vendor = (char *)(bios + size - mb_vendor_offset); > + mb_part = (char *)(image + size - mb_part_offset); > + mb_vendor = (char *)(image + size - mb_vendor_offset); > if (!isprint((unsigned char)*mb_part) || > !isprint((unsigned char)*mb_vendor)) { > - msg_pinfo("Flash image seems to have garbage in the ID location. Disabling checks.\n"); > + msg_pdbg("Flash image seems to have garbage in the ID location. Disabling checks.\n"); ...garbage in the coreboot ID location. Disabling coreboot-related checks. > return 0; > } > > msg_pdbg("coreboot last image size (not ROM size) is %d bytes.\n", *walk); > > - mainboard_part = strdup(mb_part); > - mainboard_vendor = strdup(mb_vendor); > - msg_pdbg("Manufacturer: %s\n", mainboard_vendor); > - msg_pdbg("Mainboard ID: %s\n", mainboard_part); > + image_vendor = strdup(mb_vendor); > + image_model = strdup(mb_part); If the image looks like a coreboot image according to our checks, but it's malformed (or not a coreboot image), checking string length with if (strnlen(mb_vendor, mb_vendor_offset) < mb_vendor_offset) and aborting otherwise may be a good idea to avoid segfaults. Can be done in a followup patch, though. > + msg_pdbg("Manufacturer: %s\n", image_vendor); > + msg_pdbg("Mainboard ID: %s\n", image_model); > > /* If these are not set, the coreboot table was not found. */ > - if (!lb_vendor || !lb_part) { > - msg_pinfo("Note: If the following flash access fails, try " > - "-p internal:mainboard=<vendor>:<mainboard>.\n"); > + if (!cb_vendor || !cb_model) > return 0; > - } > > /* These comparisons are case insensitive to make things a little less user^Werror prone. */ > - if (!strcasecmp(mainboard_vendor, lb_vendor) && > - !strcasecmp(mainboard_part, lb_part)) { > - msg_pdbg("This firmware image matches this mainboard.\n"); > + if (!strcasecmp(image_vendor, cb_vendor) && !strcasecmp(image_model, cb_model)) { > + msg_pdbg2("This coreboot image matches this mainboard.\n"); > } else { > - if (force_boardmismatch) { > - msg_pinfo("WARNING: This firmware image does not " > - "seem to fit to this machine - forcing it.\n"); > - } else { > - msg_pinfo("ERROR: Your firmware image (%s:%s) does not appear to\n" > - " be correct for the detected mainboard (%s:%s)\n\n" > - "Override with -p internal:boardmismatch=force to ignore the board name\n" > - "in the firmware image or override the detected mainboard with\n" > - "-p internal:mainboard=<vendor>:<mainboard>.\n\n", > - mainboard_vendor, mainboard_part, lb_vendor, lb_part); > - return 1; > - } > + msg_pinfo("WARNING: This coreboot image (%s:%s) does not appear to\n" > + " be correct for the detected mainboard (%s:%s).\n", > + image_vendor, image_model, cb_vendor, cb_model); > + return -1; > } > > return 0; > @@ -242,22 +206,15 @@ static void find_mainboard(struct lb_record *ptr, unsigned long addr) find_mainboard is a misnomer... it should be get_mainboard_from_cb_record or something like that, and the addr parameter was never used since the code was first committed. Followup patch, no need to complicate this one. > rec = (struct lb_mainboard *)ptr; > max_size = rec->size - sizeof(*rec); > msg_pdbg("Vendor ID: %.*s, part ID: %.*s\n", > - max_size - rec->vendor_idx, > - rec->strings + rec->vendor_idx, > - max_size - rec->part_number_idx, > - rec->strings + rec->part_number_idx); > - snprintf(vendor, 255, "%.*s", max_size - rec->vendor_idx, > - rec->strings + rec->vendor_idx); > - snprintf(part, 255, "%.*s", max_size - rec->part_number_idx, > - rec->strings + rec->part_number_idx); > - > - if (lb_part) { > - msg_pdbg("Overwritten by command line, vendor ID: %s, part ID: %s.\n", lb_vendor, lb_part); > - } else { > - partvendor_from_cbtable = 1; > - lb_part = strdup(part); > - lb_vendor = strdup(vendor); > - } > + max_size - rec->vendor_idx, > + rec->strings + rec->vendor_idx, > + max_size - rec->part_number_idx, > + rec->strings + rec->part_number_idx); > + snprintf(vendor, 255, "%.*s", max_size - rec->vendor_idx, rec->strings + rec->vendor_idx); > + snprintf(part, 255, "%.*s", max_size - rec->part_number_idx, rec->strings + rec->part_number_idx); > + > + cb_vendor = strdup(vendor); > + cb_model = strdup(part); > } > > static struct lb_record *next_record(struct lb_record *rec) > @@ -265,8 +222,7 @@ static struct lb_record *next_record(struct lb_record *rec) > return (struct lb_record *)(((char *)rec) + rec->size); > } > > -static void search_lb_records(struct lb_record *rec, struct lb_record *last, > - unsigned long addr) > +static void search_lb_records(struct lb_record *rec, struct lb_record *last, unsigned long addr) > { > struct lb_record *next; > int count; > @@ -284,7 +240,8 @@ static void search_lb_records(struct lb_record *rec, struct lb_record *last, > } > > #define BYTES_TO_MAP (1024*1024) > -int coreboot_init(void) > +/* returns 0 if the table was parsed successfully and cb_vendor/cb_model have been set. */ > +int cb_parse_table(const char **vendor, const char **model) > { > uint8_t *table_area; > unsigned long addr, start; > @@ -317,8 +274,7 @@ int coreboot_init(void) > physunmap(table_area, BYTES_TO_MAP); > table_area = physmap_try_ro("high tables", start, BYTES_TO_MAP); > if (ERROR_PTR == table_area) { > - msg_perr("Failed getting access to coreboot " > - "high tables.\n"); > + msg_perr("Failed getting access to coreboot high tables.\n"); > return -1; > } > lb_table = find_lb_table(table_area, 0x00000, 0x1000); > @@ -340,6 +296,7 @@ int coreboot_init(void) > lb_table->table_bytes, lb_table->table_checksum, > lb_table->table_entries); > search_lb_records(rec, last, addr + lb_table->header_bytes); > - > + *vendor = cb_vendor; > + *model = cb_model; I'm not totally happy with those static variables, but I can't offer a better suggestion (unless we want to extend struct flashctx with board/device matching info, but anything like that should be a separate patch. > return 0; > } > diff --git a/flashrom.c b/flashrom.c > index 9544155..c39c12c 100644 > --- a/flashrom.c > +++ b/flashrom.c > @@ -1814,11 +1814,15 @@ int doit(struct flashctx *flash, int force, const char *filename, int read_it, > } > > #if CONFIG_INTERNAL == 1 > - if (programmer == PROGRAMMER_INTERNAL) > - if (show_id(newcontents, size)) { > + if (programmer == PROGRAMMER_INTERNAL && cb_check_image(newcontents, size) < 0) { > + if (force_boardmismatch) { > + msg_pinfo("Proceeding anyway because user forced us to.\n"); > + } else { > + msg_perr("Aborting. You can override this with -p internal:boardmismatch=force.\n"); 112 column limit... > ret = 1; > goto out; > } > + } > #endif > } > > diff --git a/internal.c b/internal.c > index 3ecc348..b1686aa 100644 > --- a/internal.c > +++ b/internal.c > @@ -249,10 +256,20 @@ int internal_init(void) > } > > #if defined(__i386__) || defined(__x86_64__) > - /* We look at the cbtable first to see if we need a > - * mainboard specific flash enable sequence. > - */ > - coreboot_init(); > + if (cb_parse_table(&cb_vendor, &cb_model) == 0) { /* coreboot IDs valid */ > + /* if no -p internal:mainboard was given but there are valid coreboot IDs then use those */ Please capitalize first word and finish with full stop. Otherwise you'll see an instant followup commit by Uwe changing it. ;-) > + if (board_vendor == NULL || board_model == NULL) { Is it even possible that only one of board_vendor/board_model is NULL? > + board_vendor = cb_vendor; > + board_model = cb_model; > + } else if (strcasecmp(board_vendor, cb_vendor) || strcasecmp(board_model, cb_model)) { > + msg_pinfo("WARNING: The mainboard IDs set by -p internal:mainboard (%s:%s) do not\n" > + " match the current coreboot IDs of the mainboard (%s:%s).\n", > + board_vendor, board_model, cb_vendor, cb_model); > + if (!force_boardmismatch) > + return 1; > + msg_pinfo("Continuing anyway.\n"); > + } > + } > > dmi_init(); > > @@ -321,12 +338,11 @@ int internal_init(void) > init_superio_ite(); > #endif > > - board_flash_enable(lb_vendor, lb_part); > + if (board_flash_enable(board_vendor, board_model)) { > + msg_perr("Aborting to be safe.\n"); > + return 1; > + } > > - /* Even if chipset init returns an error code, we don't want to abort. > - * The error code might have been a warning only. The two lines above still make sense AFAICS. > - * Besides that, we don't check the board enable return code either. The line above can be killed as you did. > - */ > #if defined(__i386__) || defined(__x86_64__) || defined (__mips) > register_par_programmer(&par_programmer_internal, internal_buses_supported); > return 0; Thanks for cleaning up this code! Regards, Carl-Daniel -- http://www.hailfinger.org/

2 4

[commit] r1577 - trunk
by repository service 20 Aug '12

20 Aug '12

Author: stefanct Date: Mon Aug 20 02:24:22 2012 New Revision: 1577 URL: http://flashrom.org/trac/flashrom/changeset/1577 Log: Refactor the -p internal:mainboard handling. This patch gets rid of some global variables and makes lots of bits along the code path that control the board enable execution more generic and clearer. From now on flashrom aborts on a few more occasions that should be safer for the user. For example it aborts if the enable function for the specified mainboard (enable) can not be found. Parts of the board_match_cbname refactoring were done by Carl-Daniel. Signed-off-by: Stefan Tauner <stefan.tauner(a)student.tuwien.ac.at> Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006(a)gmx.net> Acked-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006(a)gmx.net> Modified: trunk/board_enable.c trunk/cbtable.c trunk/flashrom.8 trunk/flashrom.c trunk/internal.c trunk/programmer.h Modified: trunk/board_enable.c ============================================================================== --- trunk/board_enable.c Fri Aug 17 19:30:43 2012 (r1576) +++ trunk/board_enable.c Mon Aug 20 02:24:22 2012 (r1577) @@ -25,6 +25,7 @@ */ #include <string.h> +#include <stdlib.h> #include "flash.h" #include "programmer.h" #include "hwaccess.h" @@ -2443,41 +2444,64 @@ { 0, 0, 0, 0, 0, 0, 0, 0, NULL, NULL, NULL, P3, NULL, NULL, 0, NT, NULL}, /* end marker */ }; +/* Parse the <vendor>:<board> string specified by the user as part of -p internal:mainboard=<vendor>:<board>. + * Parameters vendor and model will be overwritten. Returns 0 on success. + * Note: strtok modifies the original string, so we work on a copy and allocate memory for the results. + */ +int board_parse_parameter(const char *boardstring, const char **vendor, const char **model) +{ + /* strtok may modify the original string. */ + char *tempstr = strdup(boardstring); + char *tempstr2 = NULL; + strtok(tempstr, ":"); + tempstr2 = strtok(NULL, ":"); + if (tempstr == NULL || tempstr2 == NULL) { + free(tempstr); + msg_pinfo("Please supply the board vendor and model name with the " + "-p internal:mainboard=<vendor>:<model> option.\n"); + return 1; + } + *vendor = strdup(tempstr); + *model = strdup(tempstr2); + msg_pspew("-p internal:mainboard: vendor=\"%s\", model=\"%s\"\n", tempstr, tempstr2); + free(tempstr); + return 0; +} + /* - * Match boards on coreboot table gathered vendor and part name. + * Match boards on vendor and model name. + * Hint: the parameters can come either from the coreboot table or the command line (i.e. the user). * Require main PCI IDs to match too as extra safety. + * vendor and model must be non-NULL! */ -static const struct board_match *board_match_cbname(const char *vendor, - const char *part) +static const struct board_match *board_match_name(const char *vendor, const char *model) { const struct board_match *board = board_matches; const struct board_match *partmatch = NULL; for (; board->vendor_name; board++) { - if (vendor && (!board->lb_vendor - || strcasecmp(board->lb_vendor, vendor))) + if (!board->lb_vendor || strcasecmp(board->lb_vendor, vendor)) continue; - if (!board->lb_part || strcasecmp(board->lb_part, part)) + if (!board->lb_part || strcasecmp(board->lb_part, model)) continue; - if (!pci_dev_find(board->first_vendor, board->first_device)) + if (!pci_dev_find(board->first_vendor, board->first_device)) { + msg_pdbg("Odd. Board name \"%s\":\"%s\" matches, but first PCI device %04x:%04x " + "doesn't.\n", vendor, model, board->first_vendor, board->first_device); continue; + } - if (board->second_vendor && - !pci_dev_find(board->second_vendor, board->second_device)) + if (!pci_dev_find(board->second_vendor, board->second_device)) { + msg_pdbg("Odd. Board name \"%s\":\"%s\" matches, but second PCI device %04x:%04x " + "doesn't.\n", vendor, model, board->second_vendor, board->second_device); continue; - - if (vendor) - return board; + } if (partmatch) { - /* a second entry has a matching part name */ - msg_pinfo("AMBIGUOUS BOARD NAME: %s\n", part); - msg_pinfo("At least vendors '%s' and '%s' match.\n", - partmatch->lb_vendor, board->lb_vendor); - msg_perr("Please use the full -p internal:mainboard=" - "vendor:part syntax.\n"); + /* More than one entry has a matching name. */ + msg_perr("Board name \"%s\":\"%s\" and PCI IDs matched more than one board enable " + "entry. Please report a bug at flashrom(a)flashrom.org\n", vendor, model); return NULL; } partmatch = board; @@ -2486,15 +2510,7 @@ if (partmatch) return partmatch; - if (!partvendor_from_cbtable) { - /* Only warn if the mainboard type was not gathered from the - * coreboot table. If it was, the coreboot implementor is - * expected to fix flashrom, too. - */ - msg_perr("\nUnknown vendor:board from -p internal:mainboard=" - " programmer parameter:\n%s:%s\n\n", - vendor, part); - } + msg_perr("No suitable board enable found for vendor=\"%s\", model=\"%s\".\n", vendor, model); return NULL; } @@ -2534,9 +2550,10 @@ if (board->dmi_pattern) { if (!has_dmi_support) { - msg_perr("WARNING: Can't autodetect %s %s," - " DMI info unavailable.\n", + msg_perr("WARNING: Can't autodetect %s %s, DMI info unavailable.\n", board->vendor_name, board->board_name); + msg_pinfo("Please supply the board vendor and model name with the " + "-p internal:mainboard=<vendor>:<model> option.\n"); continue; } else { if (!dmi_match(board->dmi_pattern)) @@ -2550,7 +2567,7 @@ return NULL; } -static int unsafe_board_handler(const struct board_match *board) +static int board_enable_safetycheck(const struct board_match *board) { if (!board) return 1; @@ -2559,18 +2576,15 @@ return 0; if (!force_boardenable) { - msg_pinfo("WARNING: Your mainboard is %s %s, but the mainboard-specific\n" - "code has not been tested, and thus will not be executed by default.\n" - "Depending on your hardware environment, erasing, writing or even probing\n" - "can fail without running the board specific code.\n\n" + msg_pinfo("WARNING: The mainboard-specific code for %s %s has not been tested,\n" + "and thus will not be executed by default. Depending on your hardware,\n" + "erasing, writing or even probing can fail without running this code.\n\n" "Please see the man page (section PROGRAMMER SPECIFIC INFO, subsection\n" - "\"internal programmer\") for details.\n", - board->vendor_name, board->board_name); + "\"internal programmer\") for details.\n", board->vendor_name, board->board_name); return 1; } msg_pinfo("NOTE: Running an untested board enable procedure.\n" - "Please report success/failure to flashrom(a)flashrom.org\n" - "with your board name and SUCCESS or FAILURE in the subject.\n"); + "Please report success/failure to flashrom(a)flashrom.org.\n"); return 0; } @@ -2581,12 +2595,12 @@ board = board_match_pci_ids(phase); - if (unsafe_board_handler(board)) - board = NULL; - if (!board) return 0; + if (board_enable_safetycheck(board)) + return 0; + if (!board->enable) { /* Not sure if there is a valid case for this. */ msg_perr("Board match found, but nothing to do?\n"); @@ -2606,36 +2620,37 @@ board_handle_phase(P2); } -int board_flash_enable(const char *vendor, const char *part) +int board_flash_enable(const char *vendor, const char *model) { const struct board_match *board = NULL; int ret = 0; - if (part) - board = board_match_cbname(vendor, part); - - if (!board) + if (vendor && model) { + board = board_match_name(vendor, model); + if (!board) /* if a board was given it has to match, else we abort here. */ + return 1; + } else { board = board_match_pci_ids(P3); + if (!board) /* i.e. there is just no board enable available for this board */ + return 0; + } - if (unsafe_board_handler(board)) - board = NULL; + if (board_enable_safetycheck(board)) + return 1; - if (board) { - if (board->max_rom_decode_parallel) - max_rom_decode.parallel = - board->max_rom_decode_parallel * 1024; - - if (board->enable != NULL) { - msg_pinfo("Disabling flash write protection for " - "board \"%s %s\"... ", board->vendor_name, - board->board_name); - - ret = board->enable(); - if (ret) - msg_pinfo("FAILED!\n"); - else - msg_pinfo("OK.\n"); - } + /* limit the maximum size of the parallel bus */ + if (board->max_rom_decode_parallel) + max_rom_decode.parallel = board->max_rom_decode_parallel * 1024; + + if (board->enable != NULL) { + msg_pinfo("Enabling full flash access for board \"%s %s\"... ", + board->vendor_name, board->board_name); + + ret = board->enable(); + if (ret) + msg_pinfo("FAILED!\n"); + else + msg_pinfo("OK.\n"); } return ret; Modified: trunk/cbtable.c ============================================================================== --- trunk/cbtable.c Fri Aug 17 19:30:43 2012 (r1576) +++ trunk/cbtable.c Mon Aug 20 02:24:22 2012 (r1577) @@ -24,57 +24,36 @@ #include <unistd.h> #include <stdio.h> -#include <stdlib.h> #include <ctype.h> #include <string.h> #include "flash.h" #include "programmer.h" #include "coreboot_tables.h" -char *lb_part = NULL, *lb_vendor = NULL; -int partvendor_from_cbtable = 0; -static char *def_name = "DEFAULT"; -static char *mainboard_vendor = NULL; -static char *mainboard_part = NULL; - -/* Parse the [<vendor>:]<board> string specified by the user as part of - * -p internal:mainboard=[<vendor>:]<board> and set lb_vendor and lb_part - * to the extracted values. - * Note: strtok modifies the original string, so we work on a copy and allocate - * memory for lb_vendor and lb_part with strdup. - */ -void lb_vendor_dev_from_string(const char *boardstring) -{ - /* strtok may modify the original string. */ - char *tempstr = strdup(boardstring); - char *tempstr2 = NULL; - strtok(tempstr, ":"); - tempstr2 = strtok(NULL, ":"); - if (tempstr2) { - lb_vendor = strdup(tempstr); - lb_part = strdup(tempstr2); - } else { - lb_vendor = NULL; - lb_part = strdup(tempstr); - } - free(tempstr); -} +static char *cb_vendor = NULL, *cb_model = NULL; -int show_id(uint8_t *bios, int size) +/* Tries to find coreboot IDs in the supplied image and compares them to the current IDs. + * Returns... + * -1 if IDs in the image do not match the IDs embedded in the current firmware, + * 0 if the IDs could not be found in the image or if they match correctly. + */ +int cb_check_image(uint8_t *image, int size) { + const char *image_vendor = NULL; + const char *image_model = NULL; unsigned int *walk; unsigned int mb_part_offset, mb_vendor_offset; char *mb_part, *mb_vendor; - mainboard_vendor = def_name; - mainboard_part = def_name; - - walk = (unsigned int *)(bios + size - 0x10); + walk = (unsigned int *)(image + size - 0x10); walk--; if ((*walk) == 0 || ((*walk) & 0x3ff) != 0) { - /* We might have an NVIDIA chipset BIOS which stores the ID information somewhere else. */ - walk = (unsigned int *)(bios + size - 0x80); + /* Some NVIDIA chipsets store chipset soft straps (IIRC Hypertransport init info etc.) in + * flash at exactly the location where coreboot image size, coreboot vendor name pointer and + * coreboot board name pointer are usually stored. In this case coreboot uses an alternate + * location for the coreboot image data. */ + walk = (unsigned int *)(image + size - 0x80); walk--; } @@ -88,49 +67,38 @@ mb_vendor_offset = *(walk - 2); if ((*walk) == 0 || ((*walk) & 0x3ff) != 0 || (*walk) > size || mb_part_offset > size || mb_vendor_offset > size) { - msg_pinfo("Flash image seems to be a legacy BIOS. Disabling coreboot-related checks.\n"); + msg_pdbg("Flash image seems to be a legacy BIOS. Disabling coreboot-related checks.\n"); return 0; } - mb_part = (char *)(bios + size - mb_part_offset); - mb_vendor = (char *)(bios + size - mb_vendor_offset); + mb_part = (char *)(image + size - mb_part_offset); + mb_vendor = (char *)(image + size - mb_vendor_offset); if (!isprint((unsigned char)*mb_part) || !isprint((unsigned char)*mb_vendor)) { - msg_pinfo("Flash image seems to have garbage in the ID location. Disabling checks.\n"); + msg_pdbg("Flash image seems to have garbage in the ID location. " + "Disabling coreboot-related checks.\n"); return 0; } msg_pdbg("coreboot last image size (not ROM size) is %d bytes.\n", *walk); - mainboard_part = strdup(mb_part); - mainboard_vendor = strdup(mb_vendor); - msg_pdbg("Manufacturer: %s\n", mainboard_vendor); - msg_pdbg("Mainboard ID: %s\n", mainboard_part); + image_vendor = strdup(mb_vendor); + image_model = strdup(mb_part); + msg_pdbg("Manufacturer: %s\n", image_vendor); + msg_pdbg("Mainboard ID: %s\n", image_model); /* If these are not set, the coreboot table was not found. */ - if (!lb_vendor || !lb_part) { - msg_pinfo("Note: If the following flash access fails, try " - "-p internal:mainboard=<vendor>:<mainboard>.\n"); + if (!cb_vendor || !cb_model) return 0; - } /* These comparisons are case insensitive to make things a little less user^Werror prone. */ - if (!strcasecmp(mainboard_vendor, lb_vendor) && - !strcasecmp(mainboard_part, lb_part)) { - msg_pdbg("This firmware image matches this mainboard.\n"); + if (!strcasecmp(image_vendor, cb_vendor) && !strcasecmp(image_model, cb_model)) { + msg_pdbg2("This coreboot image matches this mainboard.\n"); } else { - if (force_boardmismatch) { - msg_pinfo("WARNING: This firmware image does not " - "seem to fit to this machine - forcing it.\n"); - } else { - msg_pinfo("ERROR: Your firmware image (%s:%s) does not appear to\n" - " be correct for the detected mainboard (%s:%s)\n\n" - "Override with -p internal:boardmismatch=force to ignore the board name\n" - "in the firmware image or override the detected mainboard with\n" - "-p internal:mainboard=<vendor>:<mainboard>.\n\n", - mainboard_vendor, mainboard_part, lb_vendor, lb_part); - return 1; - } + msg_pinfo("WARNING: This coreboot image (%s:%s) does not appear to\n" + " be correct for the detected mainboard (%s:%s).\n", + image_vendor, image_model, cb_vendor, cb_model); + return -1; } return 0; @@ -242,22 +210,15 @@ rec = (struct lb_mainboard *)ptr; max_size = rec->size - sizeof(*rec); msg_pdbg("Vendor ID: %.*s, part ID: %.*s\n", - max_size - rec->vendor_idx, - rec->strings + rec->vendor_idx, - max_size - rec->part_number_idx, - rec->strings + rec->part_number_idx); - snprintf(vendor, 255, "%.*s", max_size - rec->vendor_idx, - rec->strings + rec->vendor_idx); - snprintf(part, 255, "%.*s", max_size - rec->part_number_idx, - rec->strings + rec->part_number_idx); + max_size - rec->vendor_idx, + rec->strings + rec->vendor_idx, + max_size - rec->part_number_idx, + rec->strings + rec->part_number_idx); + snprintf(vendor, 255, "%.*s", max_size - rec->vendor_idx, rec->strings + rec->vendor_idx); + snprintf(part, 255, "%.*s", max_size - rec->part_number_idx, rec->strings + rec->part_number_idx); - if (lb_part) { - msg_pdbg("Overwritten by command line, vendor ID: %s, part ID: %s.\n", lb_vendor, lb_part); - } else { - partvendor_from_cbtable = 1; - lb_part = strdup(part); - lb_vendor = strdup(vendor); - } + cb_vendor = strdup(vendor); + cb_model = strdup(part); } static struct lb_record *next_record(struct lb_record *rec) @@ -265,8 +226,7 @@ return (struct lb_record *)(((char *)rec) + rec->size); } -static void search_lb_records(struct lb_record *rec, struct lb_record *last, - unsigned long addr) +static void search_lb_records(struct lb_record *rec, struct lb_record *last, unsigned long addr) { struct lb_record *next; int count; @@ -284,7 +244,8 @@ } #define BYTES_TO_MAP (1024*1024) -int coreboot_init(void) +/* returns 0 if the table was parsed successfully and cb_vendor/cb_model have been set. */ +int cb_parse_table(const char **vendor, const char **model) { uint8_t *table_area; unsigned long addr, start; @@ -317,8 +278,7 @@ physunmap(table_area, BYTES_TO_MAP); table_area = physmap_try_ro("high tables", start, BYTES_TO_MAP); if (ERROR_PTR == table_area) { - msg_perr("Failed getting access to coreboot " - "high tables.\n"); + msg_perr("Failed getting access to coreboot high tables.\n"); return -1; } lb_table = find_lb_table(table_area, 0x00000, 0x1000); @@ -340,6 +300,7 @@ lb_table->table_bytes, lb_table->table_checksum, lb_table->table_entries); search_lb_records(rec, last, addr + lb_table->header_bytes); - + *vendor = cb_vendor; + *model = cb_model; return 0; } Modified: trunk/flashrom.8 ============================================================================== --- trunk/flashrom.8 Fri Aug 17 19:30:43 2012 (r1576) +++ trunk/flashrom.8 Mon Aug 20 02:24:22 2012 (r1577) @@ -259,7 +259,7 @@ identify the mainboard (which is the exception), or if you want to override the detected mainboard model, you can specify the mainboard using the .sp -.B " flashrom \-p internal:mainboard=[<vendor>:]<board>" +.B " flashrom \-p internal:mainboard=<vendor>:<board>" syntax. .sp See the 'Known boards' or 'Known laptops' section in the output Modified: trunk/flashrom.c ============================================================================== --- trunk/flashrom.c Fri Aug 17 19:30:43 2012 (r1576) +++ trunk/flashrom.c Mon Aug 20 02:24:22 2012 (r1577) @@ -1012,12 +1012,11 @@ "work, but to support all possible " "features"); - msg_cinfo(" we need to add them manually.\nYou " - "can help us by mailing us the output of " - "the following command to flashrom@flashrom." - "org: \n'flashrom -VV [plus the " - "-p/--programmer parameter (if needed)]" - "'\nThanks for your help!\n" + msg_cinfo(" we need to add them manually.\n" + "You can help us by mailing us the output of the following command to " + "flashrom@flashrom.org:\n" + "'flashrom -VV [plus the -p/--programmer parameter]'\n" + "Thanks for your help!\n" "===\n"); } @@ -1814,11 +1813,16 @@ } #if CONFIG_INTERNAL == 1 - if (programmer == PROGRAMMER_INTERNAL) - if (show_id(newcontents, size)) { + if (programmer == PROGRAMMER_INTERNAL && cb_check_image(newcontents, size) < 0) { + if (force_boardmismatch) { + msg_pinfo("Proceeding anyway because user forced us to.\n"); + } else { + msg_perr("Aborting. You can override this with " + "-p internal:boardmismatch=force.\n"); ret = 1; goto out; } + } #endif } Modified: trunk/internal.c ============================================================================== --- trunk/internal.c Fri Aug 17 19:30:43 2012 (r1576) +++ trunk/internal.c Mon Aug 20 02:24:22 2012 (r1577) @@ -169,6 +169,10 @@ #endif int force_laptop = 0; int not_a_laptop = 0; + const char *board_vendor = NULL; + const char *board_model = NULL; + const char *cb_vendor = NULL; + const char *cb_model = NULL; char *arg; arg = extract_programmer_param("boardenable"); @@ -217,7 +221,10 @@ arg = extract_programmer_param("mainboard"); if (arg && strlen(arg)) { - lb_vendor_dev_from_string(arg); + if (board_parse_parameter(arg, &board_vendor, &board_model)) { + free(arg); + return 1; + } } else if (arg && !strlen(arg)) { msg_perr("Missing argument for mainboard.\n"); free(arg); @@ -249,10 +256,20 @@ } #if defined(__i386__) || defined(__x86_64__) - /* We look at the cbtable first to see if we need a - * mainboard specific flash enable sequence. - */ - coreboot_init(); + if (cb_parse_table(&cb_vendor, &cb_model) == 0) { /* coreboot IDs valid */ + /* If no -p internal:mainboard was given but there are valid coreboot IDs then use those. */ + if (board_vendor == NULL || board_model == NULL) { + board_vendor = cb_vendor; + board_model = cb_model; + } else if (strcasecmp(board_vendor, cb_vendor) || strcasecmp(board_model, cb_model)) { + msg_pinfo("WARNING: The mainboard IDs set by -p internal:mainboard (%s:%s) do not\n" + " match the current coreboot IDs of the mainboard (%s:%s).\n", + board_vendor, board_model, cb_vendor, cb_model); + if (!force_boardmismatch) + return 1; + msg_pinfo("Continuing anyway.\n"); + } + } dmi_init(); @@ -321,11 +338,13 @@ init_superio_ite(); #endif - board_flash_enable(lb_vendor, lb_part); + if (board_flash_enable(board_vendor, board_model)) { + msg_perr("Aborting to be safe.\n"); + return 1; + } /* Even if chipset init returns an error code, we don't want to abort. * The error code might have been a warning only. - * Besides that, we don't check the board enable return code either. */ #if defined(__i386__) || defined(__x86_64__) || defined (__mips) register_par_programmer(&par_programmer_internal, internal_buses_supported); Modified: trunk/programmer.h ============================================================================== --- trunk/programmer.h Fri Aug 17 19:30:43 2012 (r1576) +++ trunk/programmer.h Mon Aug 20 02:24:22 2012 (r1577) @@ -246,6 +246,7 @@ #if CONFIG_INTERNAL == 1 /* board_enable.c */ +int board_parse_parameter(const char *boardstring, const char **vendor, const char **model); void w836xx_ext_enter(uint16_t port); void w836xx_ext_leave(uint16_t port); void probe_superio_winbond(void); @@ -255,7 +256,7 @@ void sio_mask(uint16_t port, uint8_t reg, uint8_t data, uint8_t mask); void board_handle_before_superio(void); void board_handle_before_laptop(void); -int board_flash_enable(const char *vendor, const char *part); +int board_flash_enable(const char *vendor, const char *model); /* chipset_enable.c */ int chipset_flash_enable(void); @@ -273,11 +274,8 @@ void cleanup_cpu_msr(void); /* cbtable.c */ -void lb_vendor_dev_from_string(const char *boardstring); -int show_id(uint8_t *bios, int size); -int coreboot_init(void); -extern char *lb_part, *lb_vendor; -extern int partvendor_from_cbtable; +int cb_parse_table(const char **vendor, const char **model); +int cb_check_image(uint8_t *bios, int size); /* dmi.c */ extern int has_dmi_support;

1 0

K9N6SGM-V
by Perpetuum Mobile (ukr.net) 19 Aug '12

19 Aug '12

http://www.msi.com/product/mb/K9N6SGM-V---K9N6PGM-FI---K9N6PGM- F.html#/?div=Detail

2 3

[PATCH] Bus Pirate init cleanup and fixes
by Carl-Daniel Hailfinger 17 Aug '12

17 Aug '12

The Bus Pirate firmware (at least v6.1 and earlier) can't handle UART input buffer overflow in BBIO mode, and sending a sequence of 0x00 too fast apparently triggers such an UART input buffer overflow. Wait 10 ms after sending each 0x00 byte during init to give the Bus Pirate enough time to handle the input. This fixes a Bus Pirate hang if the previous flashrom run was aborted by the user. The Bus Pirate firmware v6.1 and earlier use the wrong (too slow) SPI speed if more than 2 MHz are requested. Automatically downgrade SPI speed to 2 MHz for affected firmware versions. Detect Bus Pirate hardware and firmware versions to allow quirk handling. The Bus Pirate init sequence has lots of open-coded sequences which wait for a given string on the serial line. Refactor them into buspirate_wait_for_string(). User-visible changes: Fix hang, use 2 MHz SPI speed on buggy firmware. Note: This patch does _not_ include support for the fast access mode present in firmware 5.5 and later. Tested with Bus Pirate hardware v3a and v3b, firmwares 4.0, 5.2 and 6.2-beta. Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006(a)gmx.net> Index: flashrom-buspirate_initcleanup/buspirate_spi.c =================================================================== --- flashrom-buspirate_initcleanup/buspirate_spi.c (Revision 1546) +++ flashrom-buspirate_initcleanup/buspirate_spi.c (Arbeitskopie) @@ -83,7 +83,8 @@ msg_perr("Zero length command!\n"); return 1; } - msg_pspew("Sending"); + if (writecnt) + msg_pspew("Sending"); for (i = 0; i < writecnt; i++) msg_pspew(" 0x%02x", buf[i]); #ifdef FAKE_COMMUNICATION @@ -103,19 +104,32 @@ if (ret) return ret; #endif - msg_pspew(", receiving"); + if (readcnt) + msg_pspew(", receiving"); for (i = 0; i < readcnt; i++) msg_pspew(" 0x%02x", buf[i]); msg_pspew("\n"); return 0; } -static int buspirate_spi_send_command(struct flashctx *flash, - unsigned int writecnt, - unsigned int readcnt, - const unsigned char *writearr, - unsigned char *readarr); +static int buspirate_wait_for_string(unsigned char *buf, char *key) +{ + unsigned int keylen = strlen(key); + int ret; + ret = buspirate_sendrecv(buf, 0, keylen); + while (!ret) { + if (!memcmp(buf, key, keylen)) + return 0; + memmove(buf, buf + 1, keylen - 1); + ret = buspirate_sendrecv(buf + keylen - 1, 0, 1); + } + return ret; +} + +static int buspirate_spi_send_command(struct flashctx *flash, unsigned int writecnt, unsigned int readcnt, + const unsigned char *writearr, unsigned char *readarr); + static const struct spi_programmer spi_programmer_buspirate = { .type = SPI_CONTROLLER_BUSPIRATE, .max_data_read = 12, @@ -146,17 +160,15 @@ /* Exit raw SPI mode (enter raw bitbang mode) */ bp_commbuf[0] = 0x00; - ret = buspirate_sendrecv(bp_commbuf, 1, 5); - if (ret) + if ((ret = buspirate_sendrecv(bp_commbuf, 1, 0))) goto out_shutdown; - if (memcmp(bp_commbuf, "BBIO", 4)) { - msg_perr("Entering raw bitbang mode failed!\n"); - ret = 1; + if ((ret = buspirate_wait_for_string(bp_commbuf, "BBIO"))) goto out_shutdown; - } - msg_pdbg("Raw bitbang mode version %c\n", bp_commbuf[4]); - if (bp_commbuf[4] != '1') { - msg_perr("Can't handle raw bitbang mode version %c!\n", bp_commbuf[4]); + if ((ret = buspirate_sendrecv(bp_commbuf, 0, 1))) + goto out_shutdown; + msg_pdbg("Raw bitbang mode version %c\n", bp_commbuf[0]); + if (bp_commbuf[0] != '1') { + msg_perr("Can't handle raw bitbang mode version %c!\n", bp_commbuf[0]); ret = 1; goto out_shutdown; } @@ -181,18 +193,26 @@ return ret; } +#define BP_FWVERSION(a,b) ((a) << 8 | (b)) + int buspirate_spi_init(void) { char *dev = NULL; char *speed = NULL; + char *tmp; + unsigned int fw_version_major = 0; + unsigned int fw_version_minor = 0; int spispeed = 0x7; int ret = 0; int i; dev = extract_programmer_param("dev"); - if (!dev || !strlen(dev)) { - msg_perr("No serial device given. Use flashrom -p " - "buspirate_spi:dev=/dev/ttyUSB0\n"); + if (dev && !strlen(dev)) { + free(dev); + dev = NULL; + } + if (!dev) { + msg_perr("No serial device given. Use flashrom -p buspirate_spi:dev=/dev/ttyUSB0\n"); return 1; } @@ -209,9 +229,6 @@ } free(speed); - /* This works because speeds numbering starts at 0 and is contiguous. */ - msg_pdbg("SPI speed is %sHz\n", spispeeds[spispeed].name); - /* Default buffer size is 19: 16 bytes data, 3 bytes control. */ #define DEFAULT_BUFSIZE (16 + 3) bp_commbuf = malloc(DEFAULT_BUFSIZE); @@ -235,66 +252,118 @@ return 1; /* This is the brute force version, but it should work. */ - for (i = 0; i < 19; i++) { + for (i = 0; i < 20; i++) { /* Enter raw bitbang mode */ bp_commbuf[0] = 0x00; /* Send the command, don't read the response. */ ret = buspirate_sendrecv(bp_commbuf, 1, 0); if (ret) return ret; - /* Read any response and discard it. */ - sp_flush_incoming(); + /* The old way to handle responses from a Bus Pirate already in BBIO mode was to flush any + * response which came in over serial. Unfortunately that does not work reliably on Linux + * with FTDI USB-serial. + */ + //sp_flush_incoming(); + /* The Bus Pirate can't handle UART input buffer overflow in BBIO mode, and sending a sequence + * of 0x00 too fast apparently triggers such an UART input buffer overflow. + */ + usleep(10000); } - /* USB is slow. The Bus Pirate is even slower. Apparently the flush - * action above is too fast or too early. Some stuff still remains in - * the pipe after the flush above, and one additional flush is not - * sufficient either. Use a 1.5 ms delay inside the loop to make - * mostly sure that at least one USB frame had time to arrive. - * Looping only 5 times is not sufficient and causes the - * occasional failure. - * Folding the delay into the loop above is not reliable either. - */ - for (i = 0; i < 10; i++) { - usleep(1500); - /* Read any response and discard it. */ - sp_flush_incoming(); + /* We know that 20 commands of \0 should elicit at least one BBIO1 response. */ + if ((ret = buspirate_wait_for_string(bp_commbuf, "BBIO"))) + return ret; + + /* Reset the Bus Pirate. */ + bp_commbuf[0] = 0x0f; + /* Send the command, don't read the response. */ + if ((ret = buspirate_sendrecv(bp_commbuf, 1, 0))) + return ret; + if ((ret = buspirate_wait_for_string(bp_commbuf, "irate "))) + return ret; + /* Read the hardware version string. Last byte of the buffer is reserved for \0. */ + for (i = 0; i < DEFAULT_BUFSIZE - 1; i++) { + if ((ret = buspirate_sendrecv(bp_commbuf + i, 0, 1))) + return ret; + if (strchr("\r\n\t ", bp_commbuf[i])) + break; } + bp_commbuf[i] = '\0'; + msg_pdbg("Detected Bus Pirate hardware %s\n", bp_commbuf); + + if ((ret = buspirate_wait_for_string(bp_commbuf, "irmware "))) + return ret; + /* Read the firmware version string. Last byte of the buffer is reserved for \0. */ + for (i = 0; i < DEFAULT_BUFSIZE - 1; i++) { + if ((ret = buspirate_sendrecv(bp_commbuf + i, 0, 1))) + return ret; + if (strchr("\r\n\t ", bp_commbuf[i])) + break; + } + bp_commbuf[i] = '\0'; + msg_pdbg("Detected Bus Pirate firmware "); + if (bp_commbuf[0] != 'v') + msg_pdbg("(unknown version number format)\n"); + else if (!strchr("0123456789", bp_commbuf[1])) + msg_pdbg("(unknown version number format)"); + else { + fw_version_major = strtoul((char *)bp_commbuf + 1, &tmp, 10); + while ((*tmp != '\0') && !strchr("0123456789", *tmp)) + tmp++; + fw_version_minor = strtoul(tmp, NULL, 10); + msg_pdbg("%u.%u", fw_version_major, fw_version_minor); + } + msg_pdbg2(" (\"%s\")", bp_commbuf); + msg_pdbg("\n"); + + if ((ret = buspirate_wait_for_string(bp_commbuf, "HiZ>"))) + return ret; + + /* Workaround for broken speed settings in firmware 6.1 and older. */ + if (BP_FWVERSION(fw_version_major, fw_version_minor) < BP_FWVERSION(6, 2)) + if (spispeed > 0x4) { + msg_perr("Bus Pirate firmware 6.1 and older does not support SPI speeds above 2 MHz. " + "Limiting speed to 2 MHz.\n"); + msg_pinfo("It is recommended to upgrade to firmware 6.2 or newer.\n"); + spispeed = 0x4; + } + + /* Tell the user about missing SPI binary mode in firmware 2.3 and older. */ + if (BP_FWVERSION(fw_version_major, fw_version_minor) < BP_FWVERSION(2, 4)) { + msg_pinfo("Bus Pirate firmware 2.3 and older does not support binary SPI access.\n"); + msg_pinfo("Please upgrade to the latest firmware (at least 2.4).\n"); + return SPI_PROGRAMMER_ERROR; + } + + /* This works because speeds numbering starts at 0 and is contiguous. */ + msg_pdbg("SPI speed is %sHz\n", spispeeds[spispeed].name); + /* Enter raw bitbang mode */ - bp_commbuf[0] = 0x00; - ret = buspirate_sendrecv(bp_commbuf, 1, 5); - if (ret) + for (i = 0; i < 20; i++) { + bp_commbuf[0] = 0x00; + if ((ret = buspirate_sendrecv(bp_commbuf, 1, 0))) + return ret; + } + if ((ret = buspirate_wait_for_string(bp_commbuf, "BBIO"))) return ret; - if (memcmp(bp_commbuf, "BBIO", 4)) { - msg_perr("Entering raw bitbang mode failed!\n"); - msg_pdbg("Got %02x%02x%02x%02x%02x\n", - bp_commbuf[0], bp_commbuf[1], bp_commbuf[2], - bp_commbuf[3], bp_commbuf[4]); + if ((ret = buspirate_sendrecv(bp_commbuf, 0, 1))) + return ret; + msg_pdbg("Raw bitbang mode version %c\n", bp_commbuf[0]); + if (bp_commbuf[0] != '1') { + msg_perr("Can't handle raw bitbang mode version %c!\n", bp_commbuf[0]); return 1; } - msg_pdbg("Raw bitbang mode version %c\n", bp_commbuf[4]); - if (bp_commbuf[4] != '1') { - msg_perr("Can't handle raw bitbang mode version %c!\n", - bp_commbuf[4]); - return 1; - } /* Enter raw SPI mode */ bp_commbuf[0] = 0x01; - ret = buspirate_sendrecv(bp_commbuf, 1, 4); - if (ret) + ret = buspirate_sendrecv(bp_commbuf, 1, 0); + if ((ret = buspirate_wait_for_string(bp_commbuf, "SPI"))) return ret; - if (memcmp(bp_commbuf, "SPI", 3)) { - msg_perr("Entering raw SPI mode failed!\n"); - msg_pdbg("Got %02x%02x%02x%02x\n", - bp_commbuf[0], bp_commbuf[1], bp_commbuf[2], - bp_commbuf[3]); + if ((ret = buspirate_sendrecv(bp_commbuf, 0, 1))) + return ret; + msg_pdbg("Raw SPI mode version %c\n", bp_commbuf[0]); + if (bp_commbuf[0] != '1') { + msg_perr("Can't handle raw SPI mode version %c!\n", bp_commbuf[0]); return 1; } - msg_pdbg("Raw SPI mode version %c\n", bp_commbuf[3]); - if (bp_commbuf[3] != '1') { - msg_perr("Can't handle raw SPI mode version %c!\n", - bp_commbuf[3]); - return 1; - } /* Initial setup (SPI peripherals config): Enable power, CS high, AUX */ bp_commbuf[0] = 0x40 | 0xb; -- http://www.hailfinger.org/

1 2

[commit] r1576 - trunk
by repository service 17 Aug '12

17 Aug '12

Author: hailfinger Date: Fri Aug 17 19:30:43 2012 New Revision: 1576 URL: http://flashrom.org/trac/flashrom/changeset/1576 Log: Bus Pirate init cleanup and fixes. The Bus Pirate firmware (at least v6.1 and earlier) can't handle UART input buffer overflow in BBIO mode, and sending a sequence of 0x00 too fast apparently triggers such an UART input buffer overflow. Wait 10 ms after sending each 0x00 byte during init to give the Bus Pirate enough time to handle the input. This fixes a Bus Pirate hang if the previous flashrom run was aborted by the user. The Bus Pirate firmware v6.1 and earlier use the wrong (too slow) SPI speed if more than 2 MHz are requested. Automatically downgrade SPI speed to 2 MHz for affected firmware versions. Detect Bus Pirate hardware and firmware versions to allow quirk handling. The Bus Pirate init sequence has lots of open-coded sequences which wait for a given string on the serial line. Refactor them into buspirate_wait_for_string(). Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006(a)gmx.net> Acked-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006(a)gmx.net> Modified: trunk/buspirate_spi.c Modified: trunk/buspirate_spi.c ============================================================================== --- trunk/buspirate_spi.c Thu Aug 16 01:06:32 2012 (r1575) +++ trunk/buspirate_spi.c Fri Aug 17 19:30:43 2012 (r1576) @@ -84,7 +84,8 @@ msg_perr("Zero length command!\n"); return 1; } - msg_pspew("Sending"); + if (writecnt) + msg_pspew("Sending"); for (i = 0; i < writecnt; i++) msg_pspew(" 0x%02x", buf[i]); #ifdef FAKE_COMMUNICATION @@ -104,18 +105,31 @@ if (ret) return ret; #endif - msg_pspew(", receiving"); + if (readcnt) + msg_pspew(", receiving"); for (i = 0; i < readcnt; i++) msg_pspew(" 0x%02x", buf[i]); msg_pspew("\n"); return 0; } -static int buspirate_spi_send_command(struct flashctx *flash, - unsigned int writecnt, - unsigned int readcnt, - const unsigned char *writearr, - unsigned char *readarr); +static int buspirate_wait_for_string(unsigned char *buf, char *key) +{ + unsigned int keylen = strlen(key); + int ret; + + ret = buspirate_sendrecv(buf, 0, keylen); + while (!ret) { + if (!memcmp(buf, key, keylen)) + return 0; + memmove(buf, buf + 1, keylen - 1); + ret = buspirate_sendrecv(buf + keylen - 1, 0, 1); + } + return ret; +} + +static int buspirate_spi_send_command(struct flashctx *flash, unsigned int writecnt, unsigned int readcnt, + const unsigned char *writearr, unsigned char *readarr); static const struct spi_programmer spi_programmer_buspirate = { .type = SPI_CONTROLLER_BUSPIRATE, @@ -147,17 +161,15 @@ /* Exit raw SPI mode (enter raw bitbang mode) */ bp_commbuf[0] = 0x00; - ret = buspirate_sendrecv(bp_commbuf, 1, 5); - if (ret) + if ((ret = buspirate_sendrecv(bp_commbuf, 1, 0))) goto out_shutdown; - if (memcmp(bp_commbuf, "BBIO", 4)) { - msg_perr("Entering raw bitbang mode failed!\n"); - ret = 1; + if ((ret = buspirate_wait_for_string(bp_commbuf, "BBIO"))) goto out_shutdown; - } - msg_pdbg("Raw bitbang mode version %c\n", bp_commbuf[4]); - if (bp_commbuf[4] != '1') { - msg_perr("Can't handle raw bitbang mode version %c!\n", bp_commbuf[4]); + if ((ret = buspirate_sendrecv(bp_commbuf, 0, 1))) + goto out_shutdown; + msg_pdbg("Raw bitbang mode version %c\n", bp_commbuf[0]); + if (bp_commbuf[0] != '1') { + msg_perr("Can't handle raw bitbang mode version %c!\n", bp_commbuf[0]); ret = 1; goto out_shutdown; } @@ -182,18 +194,26 @@ return ret; } +#define BP_FWVERSION(a,b) ((a) << 8 | (b)) + int buspirate_spi_init(void) { char *dev = NULL; char *speed = NULL; + char *tmp; + unsigned int fw_version_major = 0; + unsigned int fw_version_minor = 0; int spispeed = 0x7; int ret = 0; int i; dev = extract_programmer_param("dev"); - if (!dev || !strlen(dev)) { - msg_perr("No serial device given. Use flashrom -p " - "buspirate_spi:dev=/dev/ttyUSB0\n"); + if (dev && !strlen(dev)) { + free(dev); + dev = NULL; + } + if (!dev) { + msg_perr("No serial device given. Use flashrom -p buspirate_spi:dev=/dev/ttyUSB0\n"); return 1; } @@ -210,9 +230,6 @@ } free(speed); - /* This works because speeds numbering starts at 0 and is contiguous. */ - msg_pdbg("SPI speed is %sHz\n", spispeeds[spispeed].name); - /* Default buffer size is 19: 16 bytes data, 3 bytes control. */ #define DEFAULT_BUFSIZE (16 + 3) bp_commbuf = malloc(DEFAULT_BUFSIZE); @@ -236,64 +253,116 @@ return 1; /* This is the brute force version, but it should work. */ - for (i = 0; i < 19; i++) { + for (i = 0; i < 20; i++) { /* Enter raw bitbang mode */ bp_commbuf[0] = 0x00; /* Send the command, don't read the response. */ ret = buspirate_sendrecv(bp_commbuf, 1, 0); if (ret) return ret; - /* Read any response and discard it. */ - sp_flush_incoming(); + /* The old way to handle responses from a Bus Pirate already in BBIO mode was to flush any + * response which came in over serial. Unfortunately that does not work reliably on Linux + * with FTDI USB-serial. + */ + //sp_flush_incoming(); + /* The Bus Pirate can't handle UART input buffer overflow in BBIO mode, and sending a sequence + * of 0x00 too fast apparently triggers such an UART input buffer overflow. + */ + usleep(10000); + } + /* We know that 20 commands of \0 should elicit at least one BBIO1 response. */ + if ((ret = buspirate_wait_for_string(bp_commbuf, "BBIO"))) + return ret; + + /* Reset the Bus Pirate. */ + bp_commbuf[0] = 0x0f; + /* Send the command, don't read the response. */ + if ((ret = buspirate_sendrecv(bp_commbuf, 1, 0))) + return ret; + if ((ret = buspirate_wait_for_string(bp_commbuf, "irate "))) + return ret; + /* Read the hardware version string. Last byte of the buffer is reserved for \0. */ + for (i = 0; i < DEFAULT_BUFSIZE - 1; i++) { + if ((ret = buspirate_sendrecv(bp_commbuf + i, 0, 1))) + return ret; + if (strchr("\r\n\t ", bp_commbuf[i])) + break; } - /* USB is slow. The Bus Pirate is even slower. Apparently the flush - * action above is too fast or too early. Some stuff still remains in - * the pipe after the flush above, and one additional flush is not - * sufficient either. Use a 1.5 ms delay inside the loop to make - * mostly sure that at least one USB frame had time to arrive. - * Looping only 5 times is not sufficient and causes the - * occasional failure. - * Folding the delay into the loop above is not reliable either. - */ - for (i = 0; i < 10; i++) { - usleep(1500); - /* Read any response and discard it. */ - sp_flush_incoming(); + bp_commbuf[i] = '\0'; + msg_pdbg("Detected Bus Pirate hardware %s\n", bp_commbuf); + + if ((ret = buspirate_wait_for_string(bp_commbuf, "irmware "))) + return ret; + /* Read the firmware version string. Last byte of the buffer is reserved for \0. */ + for (i = 0; i < DEFAULT_BUFSIZE - 1; i++) { + if ((ret = buspirate_sendrecv(bp_commbuf + i, 0, 1))) + return ret; + if (strchr("\r\n\t ", bp_commbuf[i])) + break; } - /* Enter raw bitbang mode */ - bp_commbuf[0] = 0x00; - ret = buspirate_sendrecv(bp_commbuf, 1, 5); - if (ret) + bp_commbuf[i] = '\0'; + msg_pdbg("Detected Bus Pirate firmware "); + if (bp_commbuf[0] != 'v') + msg_pdbg("(unknown version number format)"); + else if (!strchr("0123456789", bp_commbuf[1])) + msg_pdbg("(unknown version number format)"); + else { + fw_version_major = strtoul((char *)bp_commbuf + 1, &tmp, 10); + while ((*tmp != '\0') && !strchr("0123456789", *tmp)) + tmp++; + fw_version_minor = strtoul(tmp, NULL, 10); + msg_pdbg("%u.%u", fw_version_major, fw_version_minor); + } + msg_pdbg2(" (\"%s\")", bp_commbuf); + msg_pdbg("\n"); + + if ((ret = buspirate_wait_for_string(bp_commbuf, "HiZ>"))) return ret; - if (memcmp(bp_commbuf, "BBIO", 4)) { - msg_perr("Entering raw bitbang mode failed!\n"); - msg_pdbg("Got %02x%02x%02x%02x%02x\n", - bp_commbuf[0], bp_commbuf[1], bp_commbuf[2], - bp_commbuf[3], bp_commbuf[4]); - return 1; + + /* Tell the user about missing SPI binary mode in firmware 2.3 and older. */ + if (BP_FWVERSION(fw_version_major, fw_version_minor) < BP_FWVERSION(2, 4)) { + msg_pinfo("Bus Pirate firmware 2.3 and older does not support binary SPI access.\n"); + msg_pinfo("Please upgrade to the latest firmware (at least 2.4).\n"); + return SPI_PROGRAMMER_ERROR; + } + + /* Workaround for broken speed settings in firmware 6.1 and older. */ + if (BP_FWVERSION(fw_version_major, fw_version_minor) < BP_FWVERSION(6, 2)) + if (spispeed > 0x4) { + msg_perr("Bus Pirate firmware 6.1 and older does not support SPI speeds above 2 MHz. " + "Limiting speed to 2 MHz.\n"); + msg_pinfo("It is recommended to upgrade to firmware 6.2 or newer.\n"); + spispeed = 0x4; + } + + /* This works because speeds numbering starts at 0 and is contiguous. */ + msg_pdbg("SPI speed is %sHz\n", spispeeds[spispeed].name); + + /* Enter raw bitbang mode */ + for (i = 0; i < 20; i++) { + bp_commbuf[0] = 0x00; + if ((ret = buspirate_sendrecv(bp_commbuf, 1, 0))) + return ret; } - msg_pdbg("Raw bitbang mode version %c\n", bp_commbuf[4]); - if (bp_commbuf[4] != '1') { - msg_perr("Can't handle raw bitbang mode version %c!\n", - bp_commbuf[4]); + if ((ret = buspirate_wait_for_string(bp_commbuf, "BBIO"))) + return ret; + if ((ret = buspirate_sendrecv(bp_commbuf, 0, 1))) + return ret; + msg_pdbg("Raw bitbang mode version %c\n", bp_commbuf[0]); + if (bp_commbuf[0] != '1') { + msg_perr("Can't handle raw bitbang mode version %c!\n", bp_commbuf[0]); return 1; } /* Enter raw SPI mode */ bp_commbuf[0] = 0x01; - ret = buspirate_sendrecv(bp_commbuf, 1, 4); - if (ret) + ret = buspirate_sendrecv(bp_commbuf, 1, 0); + if ((ret = buspirate_wait_for_string(bp_commbuf, "SPI"))) return ret; - if (memcmp(bp_commbuf, "SPI", 3)) { - msg_perr("Entering raw SPI mode failed!\n"); - msg_pdbg("Got %02x%02x%02x%02x\n", - bp_commbuf[0], bp_commbuf[1], bp_commbuf[2], - bp_commbuf[3]); - return 1; - } - msg_pdbg("Raw SPI mode version %c\n", bp_commbuf[3]); - if (bp_commbuf[3] != '1') { - msg_perr("Can't handle raw SPI mode version %c!\n", - bp_commbuf[3]); + if ((ret = buspirate_sendrecv(bp_commbuf, 0, 1))) + return ret; + msg_pdbg("Raw SPI mode version %c\n", bp_commbuf[0]); + if (bp_commbuf[0] != '1') { + msg_perr("Can't handle raw SPI mode version %c!\n", bp_commbuf[0]); return 1; }

1 0

Re: [flashrom] [PATCH/WIP] AT45DB011D flashing support
by Aidan Thornton 17 Aug '12

17 Aug '12

Let's try that again, this time remembering to svn add the new file. Sorry about that On Fri, Aug 17, 2012 at 5:53 PM, Aidan Thornton <makosoft(a)gmail.com> wrote: > Hi, > > The attached patch is a somewhat messy attempt to add support for the > AT45DB011D and potentially other AT45DBxxxD Atmel DataFlash chips. > I've tested flashing the AT45DB011D-like integrated Flash on the > Xilinx XC3S50AN with it and it appears to work. Not really ready to > merge, but carldani asked me to post this so he could see if it could > be combined with his old incomplete code for non-power-of-2 flash > chips. Should in theory be able to read all AT45DBxxxD chips as-is and > write all chips that have 256/264-byte pages with suitable additions > to flashchips.c. > > Aidan

1 0

[PATCH/WIP] AT45DB011D flashing support
by Aidan Thornton 17 Aug '12

17 Aug '12

Hi, The attached patch is a somewhat messy attempt to add support for the AT45DB011D and potentially other AT45DBxxxD Atmel DataFlash chips. I've tested flashing the AT45DB011D-like integrated Flash on the Xilinx XC3S50AN with it and it appears to work. Not really ready to merge, but carldani asked me to post this so he could see if it could be combined with his old incomplete code for non-power-of-2 flash chips. Should in theory be able to read all AT45DBxxxD chips as-is and write all chips that have 256/264-byte pages with suitable additions to flashchips.c. Aidan

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

flashrom