flashrom July 2013

flashrom@flashrom.org

31 participants
142 discussions

by Stelios Tsampas

Flashrom detects MS-7094 (http://www.msi.com/product/mb/K8T-Neo2-F-V2-0.html) as MS-6702E (http://www.msi.com/product/mb/K8T-Neo2-F--FIR.html) Note: flashrom successfully flashed MS-7094 even though it wasn't detected properly but it couldn't flash MS-6702E as it couldn't find a suitable erase command. Edit: Re-attached proper lspci output

6 years, 3 months

Re: [flashrom] [PATCH] dummyflasher.c: add an "empty" parameter and initialize with random bytes by default

by Carl-Daniel Hailfinger

Am 21.07.2011 17:54 schrieb Stefan Tauner: > previously the dummies were initialized to be empty (all ones), which makes writes skip > erasing altogether. with this patch the default is to fill it with random bytes instead and the > old behavior can be enforced with stating "empty=yes" on the command line. > > Signed-off-by: Stefan Tauner <stefan.tauner(a)student.tuwien.ac.at> > Does the following not work for you? dd if=/dev/urandom bs=1k count=4k of=oldimage.bin flashrom -p dummy:emulate=SST25VF032B,image=oldimage.bin -w newimage.bin Regards, Carl-Daniel -- http://www.hailfinger.org/

6 years, 3 months

Re: [flashrom] Add PICkit2 as an SPI programmer

by Stefan Tauner

On Sun, 24 Mar 2013 11:10:19 -0400 Justin Chevrier <jchevrier(a)gmail.com> wrote: > From the 'You can never have enough programmers' file, comes support > for another programmer type. Supporting the internal one becomes more painful every day, so expanding the support for external programmers is very welcomed, thanks! > Specifically it's the PICkit2. This patch was inspired by the code in > AVRDude (open source Atmel AVR > programmer) to support the PICkit2 written by Doug Brown [1]. The > Dediprog code in Flashrom was used > as the template for this code with some reference to the ft2232 code > as well. I left Carl-Daniel in > the copyright assignment as I thought it made the most sense as I > basically copied and pasted then > modified the code as needed. Hopefully that was the right thing to do. Sure, the copyright does not vanish when you copy some code to a new file, so all previous copyright notices should be copied over too (if one thinks they are useful at all... :) > A few notes: > > 1) The code makes no provision for externally powered targets (where > VDD from the PICkit can be connected to > the device and an external power source and it will detect external > power and not apply its own). > I'm assuming that code needs to be written for this to work and that > the PICkit itself doesn't make it happen > automagically. Not sure on that one I have *no* idea about that programmer or the protocol... but maybe that's what the equivalent of voltage_selector = 0 is for? > > 2) I've tested with PICkit2 firmware version 2.32. I suspect that > there are earlier versions of the > firmware that are not capable of talking SPI, but I'm not sure. Should > we include a warning if we detect > that a user is running on older firmware and ask users to submit if > their firmware version works > for them? Maybe... other programmers use their devices struct for such things. I don't think dediprog is a good example in this regard. > 3) I've only tested with the one easily accessible SPI chip I have on > hand (GigaDevice GD25Q80) > > Connections are as follows: > > PICkit2 Pin 1 - VPP/MCLR -> CS > PICkit2 Pin 2 - VDD -> Vcc > PICkit2 Pin 3 - GND -> Vss > PICkit2 Pin 4 - PGD -> SO > PICkit2 Pin 5 - PDC -> SCLK > PICkit2 Pin 6 - AUX -> SI Creating a wiki page would be welcomed. If you want an account to do that, just ask. > Feedback welcomed! Hopefully this is useful to someone. > > Thanks for the great tools as always guys. Thanks for your patch! The following is not a full review, but I mention a few things I spotted while skimming through. Feel free to ignore them (for now :) if you don't agree. > Index: Makefile > =================================================================== > --- Makefile (revision 1657) > +++ Makefile (working copy) > @@ -117,7 +117,7 @@ > else > override CONFIG_PONY_SPI = no > endif > -# Dediprog and FT2232 are not supported under DOS (missing USB support). > +# Dediprog, FT2232 and PICkit2 are not supported under DOS (missing USB support). > ifeq ($(CONFIG_DEDIPROG), yes) > UNSUPPORTED_FEATURES += CONFIG_DEDIPROG=yes > else > @@ -128,7 +128,12 @@ > else > override CONFIG_FT2232_SPI = no > endif > +ifeq ($(CONFIG_PICKIT2_SPI), yes) > +UNSUPPORTED_FEATURES += CONFIG_PICKIT2_SPI=yes > +else > +override CONFIG_PICKIT2_SPI = no > endif > +endif > > # FIXME: Should we check for Cygwin/MSVC as well? > ifeq ($(TARGET_OS), MinGW) > @@ -229,7 +234,7 @@ > else > override CONFIG_SERPROG = no > endif > -# Dediprog and FT2232 are not supported with libpayload (missing libusb support) > +# Dediprog, FT2232 and PICkit2 are not supported with libpayload (missing libusb support) > ifeq ($(CONFIG_DEDIPROG), yes) > UNSUPPORTED_FEATURES += CONFIG_DEDIPROG=yes > else > @@ -240,7 +245,12 @@ > else > override CONFIG_FT2232_SPI = no > endif > +ifeq ($(CONFIG_PICKIT2_SPI), yes) > +UNSUPPORTED_FEATURES += CONFIG_PICKIT2_SPI=yes > +else > +override CONFIG_PICKIT2_SPI = no > endif > +endif > > ifneq ($(TARGET_OS), Linux) > ifeq ($(CONFIG_LINUX_SPI), yes) > @@ -382,6 +392,9 @@ > # Enable Linux spidev interface by default. We disable it on non-Linux targets. > CONFIG_LINUX_SPI ?= yes > > +# Always enable PICkit2 SPI dongles for now. > +CONFIG_PICKIT2_SPI ?= yes > + > # Disable wiki printing by default. It is only useful if you have wiki access. > CONFIG_PRINT_WIKI ?= no > > @@ -549,6 +562,12 @@ > PROGRAMMER_OBJS += linux_spi.o > endif > > +ifeq ($(CONFIG_PICKIT2_SPI), yes) > +FEATURE_CFLAGS += -D'CONFIG_PICKIT2_SPI=1' > +PROGRAMMER_OBJS += pickit2_spi.o > +NEED_USB := yes > +endif > + > ifeq ($(NEED_SERIAL), yes) > LIB_OBJS += serial.o > endif > Index: flashrom.8 > =================================================================== > --- flashrom.8 (revision 1657) > +++ flashrom.8 (working copy) > @@ -220,6 +220,8 @@ > .sp > .BR "* linux_spi" " (for SPI flash ROMs accessible via /dev/spidevX.Y on Linux)" > .sp > +.BR "* pickit2_spi" " (for SPI flash ROMs accessible via Microchip PICkit2 on Linux)" Linux only? I guess that's a copy & paste error and it will work whereever flashrom and libusb work. > +.sp > Some programmers have optional or mandatory parameters which are described > in detail in the > .B PROGRAMMER SPECIFIC INFO TODO: explanation of programmer parameters in an detailed section. > Index: flashrom.c > =================================================================== > --- flashrom.c (revision 1657) > +++ flashrom.c (working copy) > @@ -308,6 +308,19 @@ > }, > #endif > > +#if CONFIG_PICKIT2_SPI == 1 > + { > + .name = "pickit2_spi", > + .type = OTHER, > + /* FIXME */ > + .devs.note = "Microchip PICkit2\n", > + .init = pickit2_spi_init, > + .map_flash_region = fallback_map, > + .unmap_flash_region = fallback_unmap, > + .delay = internal_delay, > + }, > +#endif > + > {0}, /* This entry corresponds to PROGRAMMER_INVALID. */ > }; > > Index: pickit2_spi.c > =================================================================== > --- pickit2_spi.c (revision 0) > +++ pickit2_spi.c (working copy) > @@ -0,0 +1,519 @@ > +/* > + * This file is part of the flashrom project. > + * > + * Copyright (C) 2010 Carl-Daniel Hailfinger > + * Copyright (C) 2013 Justin Chevrier > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation; version 2 of the License. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + * You should have received a copy of the GNU General Public License > + * along with this program; if not, write to the Free Software > + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA > + */ > + Links to or citations of relevant documentation should go here (or the to be written manpage section, if useful to users too), at least a reference to avrdude should be given IMHO, but feel free to be as verbose as the commit log. > +#include <stdio.h> > +#include <string.h> > +#include <limits.h> > +#include <errno.h> > +#include <usb.h> > +#include "flash.h" > +#include "chipdrivers.h" > +#include "programmer.h" > +#include "spi.h" > + > +static usb_dev_handle *pickit2_handle; > + > +#define DEFAULT_TIMEOUT 10000 which unit? when is it used? etc. > + > +#define CMD_LENGTH 64 > +#define ENDPOINT_OUT 0x01 > +#define ENDPOINT_IN 0x81 > + > +#define PICKIT2_VID 0x04D8 > +#define PICKIT2_PID 0x0033 > + > +#define CMD_GET_VERSION 0x76 > +#define CMD_SET_VDD 0xA0 > +#define CMD_SET_VPP 0xA1 > +#define CMD_READ_VDD_VPP 0xA3 > +#define CMD_EXEC_SCRIPT 0xA6 > +#define CMD_CLR_DLOAD_BUFF 0xA7 > +#define CMD_DOWNLOAD_DATA 0xA8 > +#define CMD_CLR_ULOAD_BUFF 0xA9 > +#define CMD_UPLOAD_DATA 0xAA > +#define CMD_END_OF_BUFFER 0xAD > + > +#define SCR_SPI_READ_BUF 0xC5 > +#define SCR_SPI_WRITE_BUF 0xC6 > +#define SCR_SET_AUX 0xCF > +#define SCR_LOOP 0xE9 > +#define SCR_SET_ICSP_CLK_PERIOD 0xEA > +#define SCR_SET_PINS 0xF3 > +#define SCR_BUSY_LED_OFF 0xF4 > +#define SCR_BUSY_LED_ON 0xF5 > +#define SCR_MCLR_GND_OFF 0xF6 > +#define SCR_MCLR_GND_ON 0xF7 > +#define SCR_VPP_PWM_OFF 0xF8 > +#define SCR_VPP_PWM_ON 0xF9 > +#define SCR_VPP_OFF 0xFA > +#define SCR_VPP_ON 0xFB > +#define SCR_VDD_OFF 0xFE > +#define SCR_VDD_ON 0xFF > + > +/* Might be useful for other USB devices as well. static for now. */ We need to sort stuff like this out, but that's not your problem. > +/* device parameter allows user to specify one device of multiple installed */ > +static struct usb_device *get_device_by_vid_pid(uint16_t vid, uint16_t pid, unsigned int device) > +{ > + struct usb_bus *bus; > + struct usb_device *dev; > + > + for (bus = usb_get_busses(); bus; bus = bus->next) > + for (dev = bus->devices; dev; dev = dev->next) > + if ((dev->descriptor.idVendor == vid) && > + (dev->descriptor.idProduct == pid)) { > + if (device == 0) > + return dev; > + device--; > + } > + > + return NULL; > +} > + > +static int pickit2_get_firmware_version(void) > +{ > + int ret; > + unsigned char command[CMD_LENGTH] = {CMD_GET_VERSION, CMD_END_OF_BUFFER}; > + > + ret = usb_interrupt_write(pickit2_handle, ENDPOINT_OUT, (char *)command, CMD_LENGTH, DEFAULT_TIMEOUT); > + ret = usb_interrupt_read(pickit2_handle, ENDPOINT_IN, (char *)command, CMD_LENGTH, DEFAULT_TIMEOUT); > + > + msg_pdbg("PICkit2 Firmware Version: %d.%d\n", (int)command[0], (int)command[1]); > + if (ret != CMD_LENGTH) { > + msg_perr("Command Get Firmware Version failed (%s)!\n", > + usb_strerror()); > + return 1; > + } > + > + return 0; > +} > + > +static int pickit2_set_spi_voltage(int millivolt) > +{ > + int ret; > + uint16_t voltage_selector; > […] > + voltage_selector = 1.8; that would be in the best case some "creative" usage of floating point constants. ;) > […] > + > + unsigned char command[CMD_LENGTH] = { I personally favor uint8_t instead of unsigned chars... > + CMD_SET_VDD, > + (uint8_t)(voltage_selector * 2048+672), ... especially if you cast to them anyway :) > + (uint8_t)((voltage_selector * 2048+672)/256), We normally add spaces around all binary ops. > + (uint8_t)(voltage_selector * 36), > + CMD_SET_VPP, > + 0x40, > + (uint8_t)(voltage_selector * 18.61), I am not sure I want to understand that protocol better, if this is correct. :) > + (uint8_t)(voltage_selector * 13), > + CMD_END_OF_BUFFER > + }; > + > + ret = usb_interrupt_write(pickit2_handle, ENDPOINT_OUT, (char *)command, CMD_LENGTH, DEFAULT_TIMEOUT); > + > + if (ret != CMD_LENGTH) { > + msg_perr("Command Set Voltage failed (%s)!\n", > + usb_strerror()); > + return 1; > + } > + > + return 0; > +} > + > + > +struct pickit2_spispeeds { > + const char *const name; > + const int speed; > +}; > + > +static const struct pickit2_spispeeds spispeeds[] = { > + { "1M", 0x1 }, > + { "500k", 0x2 }, > + { "333k", 0x3 }, > + { "250k", 0x4 }, > + { NULL, 0x0 }, > +}; > + > +static int pickit2_set_spi_speed(unsigned int spispeed_idx) > +{ > + int ret; > + > + msg_pdbg("SPI speed is %sHz\n", spispeeds[spispeed_idx].name); > + > + unsigned char command[CMD_LENGTH] = { > + CMD_EXEC_SCRIPT, > + 2, > + SCR_SET_ICSP_CLK_PERIOD, > + spispeed_idx, > + CMD_END_OF_BUFFER > + }; > + > + ret = usb_interrupt_write(pickit2_handle, ENDPOINT_OUT, (char *)command, CMD_LENGTH, DEFAULT_TIMEOUT); > + > + if (ret != CMD_LENGTH) { > + msg_perr("Command Set SPI Speed failed (%s)!\n", > + usb_strerror()); > + return 1; > + } > + > + return 0; > +} > + > +static int pickit2_spi_send_command(struct flashctx *flash, > + unsigned int writecnt, > + unsigned int readcnt, > + const unsigned char *writearr, > + unsigned char *readarr) > +{ > + int ret,i; > + > + unsigned char buf[CMD_LENGTH] = {CMD_DOWNLOAD_DATA, writecnt}; > + > + /* Maximum number of bytes per transaction (including command overhead) is 64. Lets play it safe > + * and always assume the worst case scenario of 20 bytes command overhead. > + */ > + msg_pspew("%s, writecnt=%i, readcnt=%i\n", __func__, writecnt, readcnt); > + if (writecnt + readcnt + 20 > CMD_LENGTH) { > + msg_perr("\nTotal packetsize (%i) is greater than 64 supported, aborting.\n", writecnt + readcnt + 20); > + return 1; > + } > + > + for (i = 2; i < writecnt + 2; i++) > + { The curly brace should be at the end of the "for line" according to our style. > + buf[i] = writearr[i - 2]; > + } > + > + buf[i++] = CMD_CLR_ULOAD_BUFF; > + buf[i++] = CMD_EXEC_SCRIPT; > + > + /* Determine script length based on number of bytes to be read or written */ > + if (writecnt == 1 && readcnt == 1) > + buf[i++] = 7; > + else if (writecnt == 1 || readcnt == 1) > + buf[i++] = 10; > + else > + buf[i++] = 13; > + > + /* Assert CS# */ > + buf[i++] = SCR_VPP_OFF; > + buf[i++] = SCR_MCLR_GND_ON; > + > + buf[i++] = SCR_SPI_WRITE_BUF; > + > + if (writecnt > 1) { > + buf[i++] = SCR_LOOP; > + buf[i++] = 1; /* Loop back one instruction */ > + buf[i++] = writecnt - 1; /* Number of times to loop */ > + } > + > + if (readcnt) > + buf[i++] = SCR_SPI_READ_BUF; > + > + if (readcnt > 1) { > + buf[i++] = SCR_LOOP; > + buf[i++] = 1; /* Loop back one instruction */ > + buf[i++] = readcnt - 1; /* Number of times to loop */ > + } > + > + /* De-assert CS# */ > + buf[i++] = SCR_MCLR_GND_OFF; > + buf[i++] = SCR_VPP_PWM_ON; > + buf[i++] = SCR_VPP_ON; > + > + buf[i++] = CMD_UPLOAD_DATA; > + buf[i++] = CMD_END_OF_BUFFER; > + > + ret = usb_interrupt_write(pickit2_handle, ENDPOINT_OUT, (char *)buf, CMD_LENGTH, DEFAULT_TIMEOUT); > + > + if (ret != CMD_LENGTH) { > + msg_perr("Send SPI failed, expected %i, got %i %s!\n", > + writecnt, ret, usb_strerror()); > + return 1; > + } > + > + if (readcnt) { > + memset(readarr, 0, readcnt); > + memset(buf, 0, sizeof(buf)); Is this really necessary? > + > + ret = usb_interrupt_read(pickit2_handle, ENDPOINT_IN, (char *)buf, CMD_LENGTH, DEFAULT_TIMEOUT); > + > + if (ret != CMD_LENGTH) { > + msg_perr("Receive SPI failed, expected %i, got %i %s!\n", > + readcnt, ret, usb_strerror()); > + return 1; > + } > + > + /* First byte indicates number of bytes transferred from upload buffer */ > + if (buf[0] != readcnt) { > + msg_perr("Unexpected number of bytes transferred, expected %i, got %i!\n", > + readcnt, ret); > + return 1; > + } > + > + /* Actual data starts at byte number two */ > + memcpy(readarr, &buf[1], readcnt); > + } > + > + return 0; > +} > + This is a candidate for a ultity "class" too, please add the "might be useful" comment here too: /* Might be useful for other USB devices as well. static for now. */ And maybe add a reference to dediprog (and likewise for other copied functions) so that we know immediately that they have been copied. > +static int parse_voltage(char *voltage) > +{ > + char *tmp = NULL; > + int i; > + int millivolt = 0, fraction = 0; > + > + if (!voltage || !strlen(voltage)) { > + msg_perr("Empty voltage= specified.\n"); > + return -1; > + } > + millivolt = (int)strtol(voltage, &tmp, 0); > + voltage = tmp; > + /* Handle "," and "." as decimal point. Everything after it is assumed > + * to be in decimal notation. > + */ > + if ((*voltage == '.') || (*voltage == ',')) { > + voltage++; > + for (i = 0; i < 3; i++) { > + fraction *= 10; > + /* Don't advance if the current character is invalid, > + * but continue multiplying. > + */ > + if ((*voltage < '0') || (*voltage > '9')) > + continue; > + fraction += *voltage - '0'; > + voltage++; > + } > + /* Throw away remaining digits. */ > + voltage += strspn(voltage, "0123456789"); > + } > + /* The remaining string must be empty or "mV" or "V". */ > + tolower_string(voltage); > + > + /* No unit or "V". */ > + if ((*voltage == '\0') || !strncmp(voltage, "v", 1)) { > + millivolt *= 1000; > + millivolt += fraction; > + } else if (!strncmp(voltage, "mv", 2) || > + !strncmp(voltage, "milliv", 6)) { > + /* No adjustment. fraction is discarded. */ > + } else { > + /* Garbage at the end of the string. */ > + msg_perr("Garbage voltage= specified.\n"); > + return -1; > + } > + return millivolt; > +} > + I think this function can be inlined without disadvantages. The comment is clear enough. OTOH it might be worth it if you can generify a pickit2_teardown or similar (the code in the shutdown function). > +static int pickit2_setup(void) > +{ > + int ret; > + /* Configure pins for correct directions and logic levels, > + * turn Vdd on, turn busy LED on and clear buffers > + */ > + unsigned char buf[CMD_LENGTH] = { > + CMD_EXEC_SCRIPT, > + 10, /* Script length */ > + SCR_SET_PINS, > + /* Bit-0=0(PDC Out), Bit-1=1(PGD In), Bit-2=0(PDC LL), Bit-3=0(PGD LL) */ > + 2, > + SCR_SET_AUX, > + /* Bit-0=0(Aux Out), Bit-1=0(Aux LL) */ > + 0, > + SCR_VDD_ON, > + SCR_MCLR_GND_OFF, /* Let CS# float */ > + SCR_VPP_PWM_ON, > + SCR_VPP_ON, /* Pull CS# high */ > + SCR_BUSY_LED_ON, > + CMD_CLR_DLOAD_BUFF, > + CMD_CLR_ULOAD_BUFF, > + CMD_END_OF_BUFFER > + }; > + > + ret = usb_interrupt_write(pickit2_handle, ENDPOINT_OUT, (char *)buf, CMD_LENGTH, DEFAULT_TIMEOUT); > + > + if (ret != CMD_LENGTH) { > + msg_perr("Command Setup failed (%s)!\n", > + usb_strerror()); > + return 1; > + } > + > + return 0; > +} > + > +[…] > + > +static int pickit2_shutdown(void *data) > +{ > + int ret; > + msg_pspew("%s\n", __func__); > + > + /* Set all pins to float and turn voltages off */ > + unsigned char command[CMD_LENGTH] = { > + CMD_EXEC_SCRIPT, > + 8, > + SCR_SET_PINS, > + /* Bit-0=1(PDC In), Bit-1=1(PGD In), Bit-2=0(PDC LL), Bit-3=0(PGD LL) */ > + 3, > + SCR_SET_AUX, > + /* Bit-0=1(Aux In), Bit-1=0(Aux LL) */ > + 1, > + SCR_MCLR_GND_OFF, > + SCR_VPP_OFF, > + SCR_VDD_OFF, > + SCR_BUSY_LED_OFF, > + CMD_END_OF_BUFFER > + }; > + > + ret = usb_interrupt_write(pickit2_handle, ENDPOINT_OUT, (char *)command, CMD_LENGTH, DEFAULT_TIMEOUT); > + > + if (ret != CMD_LENGTH) { > + msg_perr("Command Shutdown failed (%s)!\n", > + usb_strerror()); > + return 1; > + } > + > + if (usb_release_interface(pickit2_handle, 0)) { > + msg_perr("Could not release USB interface!\n"); > + return 1; > + } > + if (usb_close(pickit2_handle)) { > + msg_perr("Could not close USB device!\n"); > + return 1; > + } > + return 0; > […] -- Kind regards/Mit freundlichen Grüßen, Stefan Tauner

6 years, 3 months

Add timeout check to SPI bitbang bus request

by Carl-Daniel Hailfinger

Hi Iain, your Intel 82540EM which hangs on a flashrom run should spew "Timeout!" messages with this patch. It would be nice if you could tell us how long it waits for each chip probe so I can adjust the timeout value. Please note that this patch will _not_ fix the underlying issue, but I believe that an endless loop on possibly broken hardware is worse than aborting. Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006(a)gmx.net> Index: flashrom-bitbang_timeout/flash.h =================================================================== --- flashrom-bitbang_timeout/flash.h (Revision 1185) +++ flashrom-bitbang_timeout/flash.h (Arbeitskopie) @@ -35,6 +35,9 @@ #define ERROR_PTR ((void*)-1) +/* Error codes */ +#define TIMEOUT_ERROR -101 + typedef unsigned long chipaddr; int register_shutdown(void (*function) (void *data), void *data); Index: flashrom-bitbang_timeout/bitbang_spi.c =================================================================== --- flashrom-bitbang_timeout/bitbang_spi.c (Revision 1185) +++ flashrom-bitbang_timeout/bitbang_spi.c (Arbeitskopie) @@ -53,10 +53,11 @@ return bitbang_spi_master->get_miso(); } -static void bitbang_spi_request_bus(void) +static int bitbang_spi_request_bus(void) { if (bitbang_spi_master->request_bus) - bitbang_spi_master->request_bus(); + return bitbang_spi_master->request_bus(); + return 0; } static void bitbang_spi_release_bus(void) @@ -86,7 +87,7 @@ bitbang_spi_master = master; bitbang_spi_half_period = halfperiod; - /* FIXME: Run bitbang_spi_request_bus here or in programmer init? */ + /* FIXME: Run bitbang_spi_request_bus here or per command? */ bitbang_spi_set_cs(1); bitbang_spi_set_sck(0); bitbang_spi_set_mosi(0); @@ -131,13 +132,15 @@ int bitbang_spi_send_command(unsigned int writecnt, unsigned int readcnt, const unsigned char *writearr, unsigned char *readarr) { - int i; + int i, ret; /* FIXME: Run bitbang_spi_request_bus here or in programmer init? * Requesting and releasing the SPI bus is handled in here to allow the * programmer to use its own SPI engine for native accesses. */ - bitbang_spi_request_bus(); + ret = bitbang_spi_request_bus(); + if (ret) + return ret; bitbang_spi_set_cs(0); for (i = 0; i < writecnt; i++) bitbang_spi_readwrite_byte(writearr[i]); Index: flashrom-bitbang_timeout/nicintel_spi.c =================================================================== --- flashrom-bitbang_timeout/nicintel_spi.c (Revision 1185) +++ flashrom-bitbang_timeout/nicintel_spi.c (Arbeitskopie) @@ -58,6 +58,8 @@ // #define FL_BUSY 30 // #define FL_ER 31 +#define MAX_REQUEST_LOOPS 10000 + uint8_t *nicintel_spibar; const struct pcidev_status nics_intel_spi[] = { @@ -68,16 +70,27 @@ {}, }; -static void nicintel_request_spibus(void) +static void nicintel_release_spibus(void); + +static int nicintel_request_spibus(void) { uint32_t tmp; + int i = 0; tmp = pci_mmio_readl(nicintel_spibar + FLA); tmp |= 1 << FL_REQ; pci_mmio_writel(tmp, nicintel_spibar + FLA); /* Wait until we are allowed to use the SPI bus. */ - while (!(pci_mmio_readl(nicintel_spibar + FLA) & (1 << FL_GNT))) ; + while (!(pci_mmio_readl(nicintel_spibar + FLA) & (1 << FL_GNT))) { + if (i++ > MAX_REQUEST_LOOPS) { + nicintel_release_spibus(); + msg_perr("%s: Timeout! Aborting.\n", __func__); + return TIMEOUT_ERROR; + } + } + + return 0; } static void nicintel_release_spibus(void) Index: flashrom-bitbang_timeout/mcp6x_spi.c =================================================================== --- flashrom-bitbang_timeout/mcp6x_spi.c (Revision 1185) +++ flashrom-bitbang_timeout/mcp6x_spi.c (Arbeitskopie) @@ -40,22 +40,37 @@ #define MCP6X_SPI_REQUEST 0 #define MCP6X_SPI_GRANT 8 +#define MAX_REQUEST_LOOPS 10000 + void *mcp6x_spibar = NULL; /* Cached value of last GPIO state. */ static uint8_t mcp_gpiostate; -static void mcp6x_request_spibus(void) +static void mcp6x_release_spibus(void); + +static int mcp6x_request_spibus(void) { + int i = 0; + mcp_gpiostate = mmio_readb(mcp6x_spibar + 0x530); mcp_gpiostate |= 1 << MCP6X_SPI_REQUEST; mmio_writeb(mcp_gpiostate, mcp6x_spibar + 0x530); /* Wait until we are allowed to use the SPI bus. */ - while (!(mmio_readw(mcp6x_spibar + 0x530) & (1 << MCP6X_SPI_GRANT))) ; + while (!(mmio_readw(mcp6x_spibar + 0x530) & (1 << MCP6X_SPI_GRANT))) { + if (i++ > MAX_REQUEST_LOOPS) { + /* Update the cache. */ + mcp_gpiostate = mmio_readb(mcp6x_spibar + 0x530); + mcp6x_release_spibus(); + msg_perr("%s: Timeout! Aborting.\n", __func__); + return TIMEOUT_ERROR; + } + } /* Update the cache. */ mcp_gpiostate = mmio_readb(mcp6x_spibar + 0x530); + return 0; } static void mcp6x_release_spibus(void) Index: flashrom-bitbang_timeout/programmer.h =================================================================== --- flashrom-bitbang_timeout/programmer.h (Revision 1185) +++ flashrom-bitbang_timeout/programmer.h (Arbeitskopie) @@ -131,7 +131,7 @@ void (*set_sck) (int val); void (*set_mosi) (int val); int (*get_miso) (void); - void (*request_bus) (void); + int (*request_bus) (void); void (*release_bus) (void); }; -- http://www.hailfinger.org/

6 years, 3 months

Flashrom stuck using 100% cpu while probing for a chip

by GatoLoko

I've tried to run flashrom -V to see if it could identify the chip on my new board (AsRock M3A-GLAN), and it got stuck while probing for the first chip. I've tried commenting out that specific chip (AMIC A25L05PT) and then it gets stuck on the next one. Then commented out all AMIC chips, and it got stuck on the next one (the first ATMEL on the list) I've tried with an older flashrom (0.9.2-r1141). It skipped some AMD chips, then got stuck on AMIC again. Talked with "tlt" and "stefanct" on irc and they asked for a strace and a gdb backtrace. Strace produced no results. Gdb log is attached. "stefanct" tried and reproduced it on a machine with nicintel_spi, but warned it was with an unsupported nic: http://paste.flashrom.org/view.php?id=738 -- GatoLoko

6 years, 3 months

[PATCH] Refactor spi_read_status_register usage

by Carl-Daniel Hailfinger

spi_read_status_register() is used in open-coded loops everywhere just to check if SPI_SR_WIP bit cleared. The logic is missing a timeout detection, and we can save quite a lot of code by introducing a function which waits until SPI_SR_WIP is cleared or a timeout is reached. Untested. May explode. Unfinished. More functions need to be converted to the new style. Will change behaviour if the chip is too slow or hangs. Should prevent flashrom from hanging without any visible output. Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006(a)gmx.net> Index: flashrom-spi_rdsr_refactor/spi25.c =================================================================== --- flashrom-spi_rdsr_refactor/spi25.c (Revision 1653) +++ flashrom-spi_rdsr_refactor/spi25.c (Arbeitskopie) @@ -328,14 +328,9 @@ __func__); return result; } - /* Wait until the Write-In-Progress bit is cleared. - * This usually takes 1-85 s, so wait in 1 s steps. - */ - /* FIXME: We assume spi_read_status_register will never fail. */ - while (spi_read_status_register(flash) & SPI_SR_WIP) - programmer_delay(1000 * 1000); /* FIXME: Check the status register for errors. */ - return 0; + result = spi_wait_status_register_ready(flash, 120 * 1000 * 1000, JEDEC_CE_60); + return result; } int spi_chip_erase_62(struct flashctx *flash) @@ -365,14 +360,9 @@ __func__); return result; } - /* Wait until the Write-In-Progress bit is cleared. - * This usually takes 2-5 s, so wait in 100 ms steps. - */ - /* FIXME: We assume spi_read_status_register will never fail. */ - while (spi_read_status_register(flash) & SPI_SR_WIP) - programmer_delay(100 * 1000); /* FIXME: Check the status register for errors. */ - return 0; + result = spi_wait_status_register_ready(flash, 120 * 1000 * 1000, JEDEC_CE_62); + return result; } int spi_chip_erase_c7(struct flashctx *flash) @@ -401,14 +391,9 @@ msg_cerr("%s failed during command execution\n", __func__); return result; } - /* Wait until the Write-In-Progress bit is cleared. - * This usually takes 1-85 s, so wait in 1 s steps. - */ - /* FIXME: We assume spi_read_status_register will never fail. */ - while (spi_read_status_register(flash) & SPI_SR_WIP) - programmer_delay(1000 * 1000); /* FIXME: Check the status register for errors. */ - return 0; + result = spi_wait_status_register_ready(flash, 120 * 1000 * 1000, JEDEC_CE_C7); + return result; } int spi_block_erase_52(struct flashctx *flash, unsigned int addr, @@ -444,13 +429,9 @@ __func__, addr); return result; } - /* Wait until the Write-In-Progress bit is cleared. - * This usually takes 100-4000 ms, so wait in 100 ms steps. - */ - while (spi_read_status_register(flash) & SPI_SR_WIP) - programmer_delay(100 * 1000); /* FIXME: Check the status register for errors. */ - return 0; + result = spi_wait_status_register_ready(flash, 10 * 1000 * 1000, JEDEC_BE_52); + return result; } /* Block size is usually @@ -491,13 +472,9 @@ __func__, addr); return result; } - /* Wait until the Write-In-Progress bit is cleared. - * This usually takes 100-4000 ms, so wait in 100 ms steps. - */ - while (spi_read_status_register(flash) & SPI_SR_WIP) - programmer_delay(100 * 1000); /* FIXME: Check the status register for errors. */ - return 0; + result = spi_wait_status_register_ready(flash, 10 * 1000 * 1000, JEDEC_BE_D8); + return result; } /* Block size is usually @@ -536,13 +513,9 @@ __func__, addr); return result; } - /* Wait until the Write-In-Progress bit is cleared. - * This usually takes 100-4000 ms, so wait in 100 ms steps. - */ - while (spi_read_status_register(flash) & SPI_SR_WIP) - programmer_delay(100 * 1000); /* FIXME: Check the status register for errors. */ - return 0; + result = spi_wait_status_register_ready(flash, 10 * 1000 * 1000, JEDEC_BE_D7); + return result; } /* Sector size is usually 4k, though Macronix eliteflash has 64k */ @@ -579,13 +552,9 @@ __func__, addr); return result; } - /* Wait until the Write-In-Progress bit is cleared. - * This usually takes 15-800 ms, so wait in 10 ms steps. - */ - while (spi_read_status_register(flash) & SPI_SR_WIP) - programmer_delay(10 * 1000); /* FIXME: Check the status register for errors. */ - return 0; + result = spi_wait_status_register_ready(flash, 10 * 1000 * 1000, JEDEC_SE); + return result; } int spi_block_erase_50(struct flashctx *flash, unsigned int addr, unsigned int blocklen) @@ -619,13 +588,9 @@ msg_cerr("%s failed during command execution at address 0x%x\n", __func__, addr); return result; } - /* Wait until the Write-In-Progress bit is cleared. - * This usually takes 10 ms, so wait in 1 ms steps. - */ - while (spi_read_status_register(flash) & SPI_SR_WIP) - programmer_delay(1 * 1000); /* FIXME: Check the status register for errors. */ - return 0; + result = spi_wait_status_register_ready(flash, 10 * 1000 * 1000, JEDEC_BE_50); + return result; } int spi_block_erase_81(struct flashctx *flash, unsigned int addr, unsigned int blocklen) @@ -659,13 +624,9 @@ msg_cerr("%s failed during command execution at address 0x%x\n", __func__, addr); return result; } - /* Wait until the Write-In-Progress bit is cleared. - * This usually takes 8 ms, so wait in 1 ms steps. - */ - while (spi_read_status_register(flash) & SPI_SR_WIP) - programmer_delay(1 * 1000); /* FIXME: Check the status register for errors. */ - return 0; + result = spi_wait_status_register_ready(flash, 10 * 1000 * 1000, JEDEC_BE_81); + return result; } int spi_block_erase_60(struct flashctx *flash, unsigned int addr, Index: flashrom-spi_rdsr_refactor/spi25_statusreg.c =================================================================== --- flashrom-spi_rdsr_refactor/spi25_statusreg.c (Revision 1653) +++ flashrom-spi_rdsr_refactor/spi25_statusreg.c (Arbeitskopie) @@ -43,7 +43,6 @@ static int spi_write_status_register_flag(struct flashctx *flash, int status, const unsigned char enable_opcode) { int result; - int i = 0; /* * WRSR requires either EWSR or WREN depending on chip type. * The code below relies on the fact hat EWSR and WREN have the same @@ -78,17 +77,10 @@ /* WRSR performs a self-timed erase before the changes take effect. * This may take 50-85 ms in most cases, and some chips apparently * allow running RDSR only once. Therefore pick an initial delay of - * 100 ms, then wait in 10 ms steps until a total of 5 s have elapsed. + * 100 ms, then wait until a total of 5 s have elapsed. */ programmer_delay(100 * 1000); - while (spi_read_status_register(flash) & SPI_SR_WIP) { - if (++i > 490) { - msg_cerr("Error: WIP bit after WRSR never cleared\n"); - return TIMEOUT_ERROR; - } - programmer_delay(10 * 1000); - } - return 0; + return spi_wait_status_register_ready(flash, 4900 * 1000, JEDEC_WRSR); } int spi_write_status_register(struct flashctx *flash, int status) @@ -108,6 +100,28 @@ return ret; } +/* timeout is specified in usecs. */ +int spi_wait_status_register_ready(struct flashctx *flash, int timeout, uint8_t opcode) +{ + /* At least 1 usec between iterations. */ + int single_delay = (timeout / 100) ? : 1; + int elapsed = 0; + int halftime = 0; + + while (spi_read_status_register(flash) & SPI_SR_WIP) { + elapsed += single_delay; + if ((elapsed > timeout / 2) && !halftime) { + msg_cdbg("Debug: WIP bit after %02x didn't clear within %i us.\n", opcode, timeout/2); + } + if (elapsed >= timeout) { + msg_cerr("Error: WIP bit after %02x didn't clear within %i us.\n", opcode, timeout); + return TIMEOUT_ERROR; + } + programmer_delay(single_delay); + } + return 0; +} + uint8_t spi_read_status_register(struct flashctx *flash) { static const unsigned char cmd[JEDEC_RDSR_OUTSIZE] = { JEDEC_RDSR }; Index: flashrom-spi_rdsr_refactor/chipdrivers.h =================================================================== --- flashrom-spi_rdsr_refactor/chipdrivers.h (Revision 1653) +++ flashrom-spi_rdsr_refactor/chipdrivers.h (Arbeitskopie) @@ -60,6 +60,7 @@ /* spi25_statusreg.c */ uint8_t spi_read_status_register(struct flashctx *flash); +int spi_wait_status_register_ready(struct flashctx *flash, int timeout, uint8_t opcode); int spi_write_status_register(struct flashctx *flash, int status); int spi_prettyprint_status_register_plain(struct flashctx *flash); int spi_prettyprint_status_register_default_bp1(struct flashctx *flash); -- http://www.hailfinger.org/

6 years, 5 months

[PATCH 1/5] rayer_spi: Improve support for different pinouts

by Kyösti Mälkki

Create a list of programmer types with names. This list could be listed with flashrom -L in follow-up patches. Handle a bit in status register that is inverted, this will be used in different future programmer types. I did trivial change after receiving the Tested-by. Local variable storage is declared static and bit offset in register is uint8_t. Signed-off-by: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Tested-by: Maksim Kuleshov <mmcx(a)mail.ru> Acked-by: Kyösti Mälkki <kyosti.malkki(a)gmail.com> --- rayer_spi.c | 106 ++++++++++++++++++++++++++++++++++-------------------------- 1 file changed, 61 insertions(+), 45 deletions(-) diff --git a/rayer_spi.c b/rayer_spi.c index b312610..cbbd5ce 100644 --- a/rayer_spi.c +++ b/rayer_spi.c @@ -37,21 +37,48 @@ #include "programmer.h" #include "hwaccess.h" -enum rayer_type { - TYPE_RAYER, - TYPE_XILINX_DLC5, -}; - /* We have two sets of pins, out and in. The numbers for both sets are * independent and are bitshift values, not real pin numbers. * Default settings are for the RayeR hardware. */ -/* Pins for master->slave direction */ -static int rayer_cs_bit = 5; -static int rayer_sck_bit = 6; -static int rayer_mosi_bit = 7; -/* Pins for slave->master direction */ -static int rayer_miso_bit = 6; + +struct rayer_programmer { + const char * type; + const enum test_state status; + const char * description; + const void * private; +}; + +struct rayer_pinout { + uint8_t cs_bit; + uint8_t sck_bit; + uint8_t mosi_bit; + uint8_t miso_bit; + void (*preinit)(void *); + int (*shutdown)(void *); +}; + +static struct rayer_pinout rayer_spipgm = { + .cs_bit = 5, + .sck_bit = 6, + .mosi_bit = 7, + .miso_bit = 6, +}; + +static struct rayer_pinout xilinx_dlc5 = { + .cs_bit = 2, + .sck_bit = 1, + .mosi_bit = 0, + .miso_bit = 4, +}; + +static struct rayer_programmer rayer_spi_types[] = { + {"rayer", NT, "RayeR SPIPGM", &rayer_spipgm}, + {"xilinx", NT, "Xilinx Parallel Cable III (DLC 5)", &xilinx_dlc5}, + {0}, +}; + +static struct rayer_pinout *pinout = NULL; static uint16_t lpt_iobase; @@ -60,22 +87,22 @@ static uint8_t lpt_outbyte; static void rayer_bitbang_set_cs(int val) { - lpt_outbyte &= ~(1 << rayer_cs_bit); - lpt_outbyte |= (val << rayer_cs_bit); + lpt_outbyte &= ~(1 << pinout->cs_bit); + lpt_outbyte |= (val << pinout->cs_bit); OUTB(lpt_outbyte, lpt_iobase); } static void rayer_bitbang_set_sck(int val) { - lpt_outbyte &= ~(1 << rayer_sck_bit); - lpt_outbyte |= (val << rayer_sck_bit); + lpt_outbyte &= ~(1 << pinout->sck_bit); + lpt_outbyte |= (val << pinout->sck_bit); OUTB(lpt_outbyte, lpt_iobase); } static void rayer_bitbang_set_mosi(int val) { - lpt_outbyte &= ~(1 << rayer_mosi_bit); - lpt_outbyte |= (val << rayer_mosi_bit); + lpt_outbyte &= ~(1 << pinout->mosi_bit); + lpt_outbyte |= (val << pinout->mosi_bit); OUTB(lpt_outbyte, lpt_iobase); } @@ -83,8 +110,8 @@ static int rayer_bitbang_get_miso(void) { uint8_t tmp; - tmp = INB(lpt_iobase + 1); - tmp = (tmp >> rayer_miso_bit) & 0x1; + tmp = INB(lpt_iobase + 1) ^ 0x80; // bit.7 inverted + tmp = (tmp >> pinout->miso_bit) & 0x1; return tmp; } @@ -99,8 +126,8 @@ static const struct bitbang_spi_master bitbang_spi_master_rayer = { int rayer_spi_init(void) { + struct rayer_programmer *prog = rayer_spi_types; char *arg = NULL; - enum rayer_type rayer_type = TYPE_RAYER; /* Non-default port requested? */ arg = extract_programmer_param("iobase"); @@ -138,36 +165,20 @@ int rayer_spi_init(void) arg = extract_programmer_param("type"); if (arg) { - if (!strcasecmp(arg, "rayer")) { - rayer_type = TYPE_RAYER; - } else if (!strcasecmp(arg, "xilinx")) { - rayer_type = TYPE_XILINX_DLC5; - } else { + for (; prog->type; ++prog) { + if (! strcasecmp (arg, prog->type)) { + break; + } + } + if(! prog->type) { msg_perr("Error: Invalid device type specified.\n"); free(arg); return 1; } + free(arg); } - free(arg); - switch (rayer_type) { - case TYPE_RAYER: - msg_pdbg("Using RayeR SPIPGM pinout.\n"); - /* Bits for master->slave direction */ - rayer_cs_bit = 5; - rayer_sck_bit = 6; - rayer_mosi_bit = 7; - /* Bits for slave->master direction */ - rayer_miso_bit = 6; - break; - case TYPE_XILINX_DLC5: - msg_pdbg("Using Xilinx Parallel Cable III (DLC 5) pinout.\n"); - /* Bits for master->slave direction */ - rayer_cs_bit = 2; - rayer_sck_bit = 1; - rayer_mosi_bit = 0; - /* Bits for slave->master direction */ - rayer_miso_bit = 4; - } + msg_pinfo("Using %s pinout.\n", prog->description); + pinout = (struct rayer_pinout *) prog->private; if (rget_io_perms()) return 1; @@ -175,6 +186,11 @@ int rayer_spi_init(void) /* Get the initial value before writing to any line. */ lpt_outbyte = INB(lpt_iobase); + if (pinout->shutdown) + register_shutdown(pinout->shutdown, (void*)pinout); + if (pinout->preinit) + pinout->preinit(pinout); + if (bitbang_spi_init(&bitbang_spi_master_rayer)) return 1; -- 1.8.1.1

6 years, 10 months

Large FWH support

by Kyösti Mälkki

Hi I came across two problems using 2 MiB FWH flash on two different boards with old ICH4 and ICH6 chipsets: First, ICH3/4/5 do not implement fwh_idsel= parameter. If vendor bios had the default map of 1 MiB, it did not even detect 2 MiB flash chips. For this I have patches prepared already. Before: (idsel_failure.txt) Probing for SST SST49LF008C, 1024 kB: probe_82802ab: id1 0xbf, id2 0x5c Probing for SST SST49LF016C, 2048 kB: probe_82802ab: id1 0xff, id2 0xff, id1 parity violation, id1 is normal flash content, id2 is normal flash content After: (idsel_ok.txt) Probing for SST SST49LF008C, 1024 kB: probe_82802ab: id1 0xbf, id2 0x5c Probing for SST SST49LF016C, 2048 kB: probe_82802ab: id1 0xbf, id2 0x5c Found SST flash chip "SST49LF016C" (2048 kB, FWH) at physical address 0xffe00000. Second, a board with ICH6 had all except one 512 kiB range of flash disabled by vendor bios and it would not detect 1MiB and 2MiB flash chips. NOTE: the board actually had 512kiB flash for these logs, but you can see where ther problem is. Before: (decode-disabled.txt) 0xfff80000/0xffb80000 FWH decode enabled 0xfff00000/0xffb00000 FWH decode disabled 0xffe80000/0xffa80000 FWH decode disabled 0xffe00000/0xffa00000 FWH decode disabled ... Maximum FWH chip size: 0x80000 bytes ... Probing for SST SST49LF008C, 1024 kB: Chip size 1024 kB is bigger than supported size 512 kB of chipset/board/programmer for FWH interface, probe/read/erase/write may fail. probe_82802ab: id1 0xff, id2 0xff, id1 parity violation, id1 is normal flash content, id2 is normal flash content I used the following as a work-around for ICH6, but this should be derived from fwh_idsel= parameter: $ setpci -s 0:1f.0 0xd8.W=0xf0c0 After: (decode-enabled.txt) 0xfff80000/0xffb80000 FWH decode enabled 0xfff00000/0xffb00000 FWH decode enabled 0xffe80000/0xffa80000 FWH decode enabled 0xffe00000/0xffa00000 FWH decode enabled Maximum FWH chip size: 0x200000 bytes Regards, Kyösti Mälkki

6 years, 11 months

[PATCH] CONFIG_DEFAULT setting

by Carl-Daniel Hailfinger

Allow easy disabling of all programmers except selected ones by setting CONFIG_DEFAULT=no. Setting CONFIG_DEFAULT=yes has no effect. Usage example: # make CONFIG_DUMMY=yes CONFIG_NIC3COM=yes CONFIG_DEFAULT=no This will disable all programmers except dummy and nic3com. Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006(a)gmx.net> Index: flashrom-Makefile_config_default/Makefile =================================================================== --- flashrom-Makefile_config_default/Makefile (Revision 1623) +++ flashrom-Makefile_config_default/Makefile (Arbeitskopie) @@ -52,6 +52,10 @@ # Compilation will fail for unspecified values. CONFIG_DEFAULT_PROGRAMMER ?= PROGRAMMER_INVALID +# This parameter picks the default setting for CONFIG_ variables of programmers not specified explicitly +# on the command line. +CONFIG_DEFAULT ?= yes + # If your compiler spits out excessive warnings, run make WARNERROR=no # You shouldn't have to change this flag. WARNERROR ?= yes @@ -322,65 +326,65 @@ SVNDEF := -D'FLASHROM_VERSION="$(VERSION)"' # Always enable internal/onboard support for now. -CONFIG_INTERNAL ?= yes +CONFIG_INTERNAL ?= $(CONFIG_DEFAULT) # Always enable serprog for now. Needs to be disabled on Windows. -CONFIG_SERPROG ?= yes +CONFIG_SERPROG ?= $(CONFIG_DEFAULT) # RayeR SPIPGM hardware support -CONFIG_RAYER_SPI ?= yes +CONFIG_RAYER_SPI ?= $(CONFIG_DEFAULT) # PonyProg2000 SPI hardware support -CONFIG_PONY_SPI ?= yes +CONFIG_PONY_SPI ?= $(CONFIG_DEFAULT) # Always enable 3Com NICs for now. -CONFIG_NIC3COM ?= yes +CONFIG_NIC3COM ?= $(CONFIG_DEFAULT) # Enable NVIDIA graphics cards. Note: write and erase do not work properly. -CONFIG_GFXNVIDIA ?= yes +CONFIG_GFXNVIDIA ?= $(CONFIG_DEFAULT) # Always enable SiI SATA controllers for now. -CONFIG_SATASII ?= yes +CONFIG_SATASII ?= $(CONFIG_DEFAULT) # Highpoint (HPT) ATA/RAID controller support. # IMPORTANT: This code is not yet working! CONFIG_ATAHPT ?= no # Always enable FT2232 SPI dongles for now. -CONFIG_FT2232_SPI ?= yes +CONFIG_FT2232_SPI ?= $(CONFIG_DEFAULT) # Always enable dummy tracing for now. -CONFIG_DUMMY ?= yes +CONFIG_DUMMY ?= $(CONFIG_DEFAULT) # Always enable Dr. Kaiser for now. -CONFIG_DRKAISER ?= yes +CONFIG_DRKAISER ?= $(CONFIG_DEFAULT) # Always enable Realtek NICs for now. -CONFIG_NICREALTEK ?= yes +CONFIG_NICREALTEK ?= $(CONFIG_DEFAULT) # Disable National Semiconductor NICs until support is complete and tested. CONFIG_NICNATSEMI ?= no # Always enable Intel NICs for now. -CONFIG_NICINTEL ?= yes +CONFIG_NICINTEL ?= $(CONFIG_DEFAULT) # Always enable SPI on Intel NICs for now. -CONFIG_NICINTEL_SPI ?= yes +CONFIG_NICINTEL_SPI ?= $(CONFIG_DEFAULT) # Always enable SPI on OGP cards for now. -CONFIG_OGP_SPI ?= yes +CONFIG_OGP_SPI ?= $(CONFIG_DEFAULT) # Always enable Bus Pirate SPI for now. -CONFIG_BUSPIRATE_SPI ?= yes +CONFIG_BUSPIRATE_SPI ?= $(CONFIG_DEFAULT) # Disable Dediprog SF100 until support is complete and tested. CONFIG_DEDIPROG ?= no # Always enable Marvell SATA controllers for now. -CONFIG_SATAMV ?= yes +CONFIG_SATAMV ?= $(CONFIG_DEFAULT) # Enable Linux spidev interface by default. We disable it on non-Linux targets. -CONFIG_LINUX_SPI ?= yes +CONFIG_LINUX_SPI ?= $(CONFIG_DEFAULT) # Disable wiki printing by default. It is only useful if you have wiki access. CONFIG_PRINT_WIKI ?= no -- http://www.hailfinger.org/

6 years, 11 months

[PATCH] Remove exit call from physmap_common

by Niklas Söderlund

Only call path where exit is reached is from physmap(). Callers of physmap() that where not prepared to handle the ERROR_PTR return value have been handled. Signed-off-by: Niklas Söderlund <niso(a)kth.se> --- chipset_enable.c | 6 ++++++ drkaiser.c | 2 ++ gfxnvidia.c | 2 ++ ichspi.c | 2 ++ it85spi.c | 2 ++ mcp6x_spi.c | 2 ++ nicintel_spi.c | 2 ++ ogp_spi.c | 2 ++ physmap.c | 12 +++--------- satasii.c | 2 ++ sb600spi.c | 2 ++ 11 files changed, 27 insertions(+), 9 deletions(-) diff --git a/chipset_enable.c b/chipset_enable.c index fa79511..b1738ea 100644 --- a/chipset_enable.c +++ b/chipset_enable.c @@ -494,6 +494,8 @@ static int enable_flash_tunnelcreek(struct pci_dev *dev, const char *name) /* Map RCBA to virtual memory */ rcrb = physmap("ICH RCRB", tmp, 0x4000); + if (rcrb == ERROR_PTR) + return ERROR_FATAL; /* Test Boot BIOS Strap Status */ bnt = mmio_readl(rcrb + 0x3410); @@ -565,6 +567,8 @@ static int enable_flash_ich_dc_spi(struct pci_dev *dev, const char *name, /* Map RCBA to virtual memory */ rcrb = physmap("ICH RCRB", tmp, 0x4000); + if (rcrb == ERROR_PTR) + return ERROR_FATAL; gcs = mmio_readl(rcrb + 0x3410); msg_pdbg("GCS = 0x%x: ", gcs); @@ -1235,6 +1239,8 @@ static int get_flashbase_sc520(struct pci_dev *dev, const char *name) /* 1. Map MMCR */ mmcr = physmap("Elan SC520 MMCR", 0xfffef000, getpagesize()); + if (mmcr == ERROR_PTR) + return -1; /* 2. Scan PAR0 (0x88) - PAR15 (0xc4) for * BOOTCS region (PARx[31:29] = 100b)e diff --git a/drkaiser.c b/drkaiser.c index 83b8b3d..71981e8 100644 --- a/drkaiser.c +++ b/drkaiser.c @@ -82,6 +82,8 @@ int drkaiser_init(void) /* Map 128kB flash memory window. */ drkaiser_bar = physmap("Dr. Kaiser PC-Waechter flash memory", addr, DRKAISER_MEMMAP_SIZE); + if (drkaiser_bar == ERROR_PTR) + return 1; if (register_shutdown(drkaiser_shutdown, NULL)) return 1; diff --git a/gfxnvidia.c b/gfxnvidia.c index 9232677..9600675 100644 --- a/gfxnvidia.c +++ b/gfxnvidia.c @@ -102,6 +102,8 @@ int gfxnvidia_init(void) msg_pinfo("Detected NVIDIA I/O base address: 0x%x.\n", io_base_addr); nvidia_bar = physmap("NVIDIA", io_base_addr, GFXNVIDIA_MEMMAP_SIZE); + if (nvidia_bar == ERROR_PTR) + return 1; /* Must be done before rpci calls. */ if (register_shutdown(gfxnvidia_shutdown, NULL)) diff --git a/ichspi.c b/ichspi.c index bbe2b06..f6b77ad 100644 --- a/ichspi.c +++ b/ichspi.c @@ -1836,6 +1836,8 @@ int via_init_spi(struct pci_dev *dev, uint32_t mmio_base) int i; ich_spibar = physmap("VIA SPI MMIO registers", mmio_base, 0x70); + if (ich_spibar == ERROR_PTR) + return ERROR_FATAL; /* Do we really need no write enable? Like the LPC one at D17F0 0x40 */ /* Not sure if it speaks all these bus protocols. */ diff --git a/it85spi.c b/it85spi.c index 0b074eb..e15a287 100644 --- a/it85spi.c +++ b/it85spi.c @@ -262,6 +262,8 @@ static int it85xx_spi_common_init(struct superio s) * Major TODO here, and it will be a lot of work. */ base = (chipaddr)physmap("it85 communication", 0xFFFFF000, 0x1000); + if (base == ERROR_PTR) + return 1; msg_pdbg("%s():%d base=0x%08x\n", __func__, __LINE__, (unsigned int)base); ce_high = (unsigned char *)(base + 0xE00); /* 0xFFFFFE00 */ diff --git a/mcp6x_spi.c b/mcp6x_spi.c index ac40557..438e661 100644 --- a/mcp6x_spi.c +++ b/mcp6x_spi.c @@ -149,6 +149,8 @@ int mcp6x_spi_init(int want_spi) } /* Map the BAR. Bytewise/wordwise access at 0x530 and 0x540. */ mcp6x_spibar = physmap("NVIDIA MCP6x SPI", mcp6x_spibaraddr, 0x544); + if (mcp6x_spibar == ERROR_PTR) + return 1; #if 0 /* FIXME: Run the physunmap in a shutdown function. */ diff --git a/nicintel_spi.c b/nicintel_spi.c index 72b470b..6dc40f5 100644 --- a/nicintel_spi.c +++ b/nicintel_spi.c @@ -178,6 +178,8 @@ int nicintel_spi_init(void) nicintel_spibar = physmap("Intel Gigabit NIC w/ SPI flash", io_base_addr, MEMMAP_SIZE); + if (nicintel_spibar == ERROR_PTR) + return 1; /* Automatic restore of EECD on shutdown is not possible because EECD * does not only contain FLASH_WRITES_DISABLED|FLASH_WRITES_ENABLED, * but other bits with side effects as well. Those other bits must be diff --git a/ogp_spi.c b/ogp_spi.c index 9b596bf..b3963c2 100644 --- a/ogp_spi.c +++ b/ogp_spi.c @@ -138,6 +138,8 @@ int ogp_spi_init(void) return 1; ogp_spibar = physmap("OGP registers", io_base_addr, 4096); + if (ogp_spibar == ERROR_PTR) + return 1; if (register_shutdown(ogp_spi_shutdown, NULL)) return 1; diff --git a/physmap.c b/physmap.c index adce315..35dcfb8 100644 --- a/physmap.c +++ b/physmap.c @@ -211,13 +211,11 @@ void physunmap(void *virt_addr, size_t len) } #endif -#define PHYSMAP_NOFAIL 0 -#define PHYSMAP_MAYFAIL 1 #define PHYSMAP_RW 0 #define PHYSMAP_RO 1 static void *physmap_common(const char *descr, unsigned long phys_addr, - size_t len, int mayfail, int readonly) + size_t len, int readonly) { void *virt_addr; @@ -261,8 +259,6 @@ static void *physmap_common(const char *descr, unsigned long phys_addr, "and reboot, or reboot into\n" "single user mode.\n"); #endif - if (!mayfail) - exit(3); } return virt_addr; @@ -270,14 +266,12 @@ static void *physmap_common(const char *descr, unsigned long phys_addr, void *physmap(const char *descr, unsigned long phys_addr, size_t len) { - return physmap_common(descr, phys_addr, len, PHYSMAP_NOFAIL, - PHYSMAP_RW); + return physmap_common(descr, phys_addr, len, PHYSMAP_RW); } void *physmap_try_ro(const char *descr, unsigned long phys_addr, size_t len) { - return physmap_common(descr, phys_addr, len, PHYSMAP_MAYFAIL, - PHYSMAP_RO); + return physmap_common(descr, phys_addr, len, PHYSMAP_RO); } #if defined(__i386__) || defined(__x86_64__) diff --git a/satasii.c b/satasii.c index 838fff4..9f9bb7f 100644 --- a/satasii.c +++ b/satasii.c @@ -97,6 +97,8 @@ int satasii_init(void) } sii_bar = physmap("SATA SiI registers", addr, SATASII_MEMMAP_SIZE) + reg_offset; + if (sii_bar == ERROR_PTR) + return 1; /* Check if ROM cycle are OK. */ if ((id != 0x0680) && (!(pci_mmio_readl(sii_bar) & (1 << 26)))) diff --git a/sb600spi.c b/sb600spi.c index fe60aa9..a4ca20b 100644 --- a/sb600spi.c +++ b/sb600spi.c @@ -226,6 +226,8 @@ int sb600_probe_spi(struct pci_dev *dev) /* Physical memory has to be mapped at page (4k) boundaries. */ sb600_spibar = physmap("SB600 SPI registers", tmp & 0xfffff000, 0x1000); + if (sb600_spibar == ERROR_PTR) + return ERROR_FATAL; /* The low bits of the SPI base address are used as offset into * the mapped page. */ -- 1.7.12.4

6 years, 11 months

Jump to page:

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

flashrom July 2013