flashrom February 2010

flashrom@flashrom.org

55 participants
173 discussions

[patch] NetBSD/amd64 build fix (w/ actual patch)
by Jonathan A. Kollasch 14 Feb '10

14 Feb '10

Use uname -p instead of -m on NetBSD so we get the right architecture library name. Signed-off-by: Jonathan A. Kollasch <jakllsch(a)kollasch.net> ---

2 1

[commit] r903 - trunk
by repository service 14 Feb '10

14 Feb '10

Author: hailfinger Date: Sun Feb 14 02:00:36 2010 New Revision: 903 URL: http://flashrom.org/trac/coreboot/changeset/903 Log: Use uname -p instead of -m on NetBSD so we get the right architecture library name. Signed-off-by: Jonathan A. Kollasch <jakllsch(a)kollasch.net> Acked-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006(a)gmx.net> Modified: trunk/Makefile Modified: trunk/Makefile ============================================================================== --- trunk/Makefile Sun Feb 14 00:41:01 2010 (r902) +++ trunk/Makefile Sun Feb 14 02:00:36 2010 (r903) @@ -183,7 +183,7 @@ PROGRAMMER_OBJS += pcidev.o physmap.o hwaccess.o ifeq ($(OS_ARCH), NetBSD) LIBS += -lpciutils # The libpci we want. -LIBS += -l$(shell uname -m) # For (i386|x86_64)_iopl(2). +LIBS += -l$(shell uname -p) # For (i386|x86_64)_iopl(2). endif endif

1 0

Re: [flashrom] [coreboot] flashrom on MCP73
by Carl-Daniel Hailfinger 14 Feb '10

14 Feb '10

Hi Fabrício, [Followup to flashrom(a)flashrom.org please] some of us are interested in reverse engineering flashrom SPI support for your chipset. Do you still have that board available? On 15.01.2009 18:12, Fabrício Ceolin wrote: > On Wed, Jan 14, 2009 at 9:36 PM, Peter Stuge <peter(a)stuge.se> wrote: > >> Fabrício Ceolin wrote: >> >>> We are trying to make flashrom work on http://www.ecs.com.tw/ECSWebSite/Products/ProductsDetail.aspx?detailid=858&… >>> >>> Chipset MCP73 / SPI chip Macronix MX25L8005. >>> >>> We made modifications using a enable_flash_mcp55 function, but >>> does't work. >>> >> First you have to figure out what chip is the SPI bus master. >> > We made these tests. There are a superio, but the bios chip isn't connect to > it. > The bios chip is directly connected to the chipset MCP73. > > So, I need to the datasheet of MCP73 to implement it. > Did you get any datasheets from Nvidia? Regards, Carl-Daniel -- Developer quote of the week: "We are juggling too many chainsaws and flaming arrows and tigers."

1 1

Erasing on Asus M2N68-VM
by Michael Karcher 14 Feb '10

14 Feb '10

Hello developers, this is from Code found in the BIOS. Enable bitbanging SPI interface: - ISA brigde (10de:0548, at 00:01.0) needs 8A.6 set and 8A.5 cleared - SMBus controller (10de:0542, at 00:01.1, so same chip) contains base address of something at register 0x74 (DWORD). Bitbanging register is at 0x530 inside that block. The base address has been seen as 0xfec80000 on one board. To select the BIOS ROM: - Set bit 0 in the bitbang register - Wait for bit 8 to be set (this is the *only* word access to the bitbang register, everything els is bytewise) - Clear bit 1 in the bitbang register To deselect the BIOS ROM: - Set bit 1 in the bitbang register - Clear bit 0 in the bitbang register To do SPI transfers: - bit 2 is clock (read/write) - bit 3 is MOSI (to flash) - bit 4 is MISO (from flash) That's basically everything. No warranty that this doesn't interfere with anything ACPI-like while the system is running, of course. Regards, Michael Karcher

5 10

[commit] r902 - trunk
by repository service 14 Feb '10

14 Feb '10

Author: hailfinger Date: Sun Feb 14 00:41:01 2010 New Revision: 902 URL: http://flashrom.org/trac/coreboot/changeset/902 Log: Add SPI mode diagnostics for all post-MCP55 (nForce 5) chipsets from Nvidia. Huge thanks to Michael Karcher for reverse engineering the MCP67 chipset and writing a spec. Due to this, we were able to use the chinese wall technique for 100% clean room reverse engineering. This patch doesn't touch any of the new registers, it only reads them. Assuming that read has no side effects, this patch is a no-op and safe. We need "flashrom -V" output from all post-MCP55 (nForce 5) chipset boards. Please indicate if your board uses SPI flash or LPC flash (if you know it). Note: That output is only helpful if it is created with patched flashrom and if is from the first run of flashrom after a cold boot (reset or Ctrl-Alt-Del is not sufficient). There is a pattern based on which we can probably detect which flash type is present on the board. Thanks to Alessandro Polverini for testing earlier iterations of this patch. Note: The MCP67 should work. I guessed that the other recent Nvidia chipsets would work in a similar way, and created a simplified do-nothing catchall chipset enable function which dumps some info and instructs the user to send more info. Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006(a)gmx.net> Acked-by: Michael Karcher <flashrom(a)mkarcher.dialup.fu-berlin.de> Modified: trunk/chipset_enable.c Modified: trunk/chipset_enable.c ============================================================================== --- trunk/chipset_enable.c Sat Feb 13 20:22:11 2010 (r901) +++ trunk/chipset_enable.c Sun Feb 14 00:41:01 2010 (r902) @@ -1054,6 +1054,127 @@ return 0; } +/** + * The MCP67 code is guesswork based on cleanroom reverse engineering. + * Due to that, it only reads info and doesn't change any settings. + * It is assumed that LPC chips need the MCP55 code and SPI chips need the + * code provided in this function. Until we know for sure, call + * enable_flash_mcp55 from this function. Warning: enable_flash_mcp55 + * might make SPI flash inaccessible. The same caveat applies to SPI init + * for LPC flash. + */ +static int enable_flash_mcp67(struct pci_dev *dev, const char *name) +{ + int result = 0; + uint8_t byte; + uint16_t status; + uint32_t mcp_spibaraddr; + void *mcp_spibar; + struct pci_dev *smbusdev; + + /* dev is the ISA bridge. No idea what the stuff below does. */ + byte = pci_read_byte(dev, 0x8a); + msg_pdbg("ISA bridge reg 0x8a contents: 0x%02x, bit 6 is %i, bit 5 is " + "%i\n", byte, (byte >> 6) & 0x1, (byte >> 5) & 0x1); + msg_pdbg("Guessed flash bus type is %s\n", ((byte >> 5) & 0x3) == 0x2 ? + "SPI" : "unknown, probably LPC"); + /* Disable the write code for now until we have more info. */ +#if 0 + byte |= (1 << 6); + byte &= ~(1 << 5); + pci_write_byte(dev, 0x8a, byte); +#endif + + /* Look for the SMBus device (SMBus PCI class) */ + smbusdev = pci_dev_find_vendorclass(0x10de, 0x0c05); + if (!smbusdev) { + msg_perr("ERROR: SMBus device not found. Aborting.\n"); + exit(1); + } + msg_pdbg("Found SMBus device %04x:%04x at %02x:%02x:%01x\n", + smbusdev->vendor_id, smbusdev->device_id, + smbusdev->bus, smbusdev->dev, smbusdev->func); + + /* Locate the BAR where the SPI interface lives. */ + mcp_spibaraddr = pci_read_long(smbusdev, 0x74); + msg_pdbg("SPI BAR is at 0x%08x, ", mcp_spibaraddr); + /* We hope this has native alignment. We know the SPI interface (well, + * a set of GPIOs that is connected to SPI flash) is at offset 0x530, + * so we expect a size of at least 0x800. Clear the lower bits. + * It is entirely possible that the BAR is 64k big and the low bits are + * reserved for an entirely different purpose. + */ + mcp_spibaraddr &= ~0x7ff; + msg_pdbg("after clearing low bits BAR is at 0x%08x\n", mcp_spibaraddr); + + /* Accessing a NULL pointer BAR is evil. Don't do it. */ + if (mcp_spibaraddr) { + /* Map the BAR. Bytewise/wordwise access at 0x530 and 0x540. */ + mcp_spibar = physmap("MCP67 SPI", mcp_spibaraddr, 0x544); + +/* Guessed. If this is correct, migrate to a separate MCP67 SPI driver. */ +#define MCP67_SPI_CS (1 << 1) +#define MCP67_SPI_SCK (1 << 2) +#define MCP67_SPI_MOSI (1 << 3) +#define MCP67_SPI_MISO (1 << 4) +#define MCP67_SPI_ENABLE (1 << 0) +#define MCP67_SPI_IDLE (1 << 8) + + status = mmio_readw(mcp_spibar + 0x530); + msg_pdbg("SPI control is 0x%04x, enable=%i, idle=%i\n", + status, status & 0x1, (status >> 8) & 0x1); + /* FIXME: Remove the physunmap once the SPI driver exists. */ + physunmap(mcp_spibar, 0x544); + } else { + msg_pdbg("Strange. MCP67 SPI BAR is invalid.\n"); + } + msg_pinfo("Please send the output of \"flashrom -V\" to " + "flashrom(a)flashrom.org to help us finish support for your " + "chipset. Thanks.\n"); + + /* Not sure if this is still correct. No docs as usual. */ + result = enable_flash_mcp55(dev, name); + + return result; +} + +/* This is a shot in the dark. Even if the code is totally bogus for some + * chipsets, users will at least start to send in reports. + */ +static int enable_flash_mcp7x(struct pci_dev *dev, const char *name) +{ + uint8_t byte; + uint32_t mcp_spibaraddr; + struct pci_dev *smbusdev; + + msg_pinfo("This chipset is not really supported yet. Guesswork...\n"); + + /* dev is the ISA bridge. No idea what the stuff below does. */ + byte = pci_read_byte(dev, 0x8a); + msg_pdbg("ISA/LPC bridge reg 0x8a contents: 0x%02x, bit 6 is %i, bit 5 " + "is %i\n", byte, (byte >> 6) & 0x1, (byte >> 5) & 0x1); + + /* Look for the SMBus device (SMBus PCI class) */ + smbusdev = pci_dev_find_vendorclass(0x10de, 0x0c05); + if (!smbusdev) { + msg_perr("ERROR: SMBus device not found. Aborting.\n"); + exit(1); + } + msg_pdbg("Found SMBus device %04x:%04x at %02x:%02x:%01x\n", + smbusdev->vendor_id, smbusdev->device_id, + smbusdev->bus, smbusdev->dev, smbusdev->func); + + /* Locate the BAR where the SPI interface lives. */ + mcp_spibaraddr = pci_read_long(smbusdev, 0x74); + msg_pdbg("SPI BAR is at 0x%08x, ", mcp_spibaraddr); + + msg_pinfo("Please send the output of \"flashrom -V\" to " + "flashrom(a)flashrom.org to help us finish support for your " + "chipset. Thanks.\n"); + + return 0; +} + static int enable_flash_ht1000(struct pci_dev *dev, const char *name) { uint8_t byte; @@ -1193,7 +1314,22 @@ {0x10de, 0x0365, OK, "NVIDIA", "MCP55", enable_flash_mcp55}, /* LPC */ {0x10de, 0x0366, OK, "NVIDIA", "MCP55", enable_flash_mcp55}, /* LPC */ {0x10de, 0x0367, OK, "NVIDIA", "MCP55", enable_flash_mcp55}, /* Pro */ - {0x10de, 0x0548, OK, "NVIDIA", "MCP67", enable_flash_mcp55}, + {0x10de, 0x03e0, NT, "NVIDIA", "MCP61", enable_flash_mcp7x}, + {0x10de, 0x03e1, NT, "NVIDIA", "MCP61", enable_flash_mcp7x}, + {0x10de, 0x03e2, NT, "NVIDIA", "MCP61", enable_flash_mcp7x}, + {0x10de, 0x03e3, NT, "NVIDIA", "MCP61", enable_flash_mcp7x}, + {0x10de, 0x0440, NT, "NVIDIA", "MCP65", enable_flash_mcp7x}, + {0x10de, 0x0441, NT, "NVIDIA", "MCP65", enable_flash_mcp7x}, + {0x10de, 0x0442, NT, "NVIDIA", "MCP65", enable_flash_mcp7x}, + {0x10de, 0x0443, NT, "NVIDIA", "MCP65", enable_flash_mcp7x}, + {0x10de, 0x0548, OK, "NVIDIA", "MCP67", enable_flash_mcp67}, + {0x10de, 0x075c, NT, "NVIDIA", "MCP78S", enable_flash_mcp7x}, + {0x10de, 0x075d, NT, "NVIDIA", "MCP78S", enable_flash_mcp7x}, + {0x10de, 0x07d7, NT, "NVIDIA", "MCP73", enable_flash_mcp7x}, + {0x10de, 0x0aac, NT, "NVIDIA", "MCP79", enable_flash_mcp7x}, + {0x10de, 0x0aad, NT, "NVIDIA", "MCP79", enable_flash_mcp7x}, + {0x10de, 0x0aae, NT, "NVIDIA", "MCP79", enable_flash_mcp7x}, + {0x10de, 0x0aaf, NT, "NVIDIA", "MCP79", enable_flash_mcp7x}, {0x1039, 0x0496, NT, "SiS", "85C496+497", enable_flash_sis85c496}, {0x1039, 0x0406, NT, "SiS", "501/5101/5501", enable_flash_sis501}, {0x1039, 0x5511, NT, "SiS", "5511", enable_flash_sis5511},

1 0

[PATCH] Support for chip: Intel E28F004S5
by Sean Nelson 13 Feb '10

13 Feb '10

Adds support for the Intel E28F004S5 flash chip. Signed-off-by: Sean Nelson <audiohacked(a)gmail.com>

2 4

[commit] r901 - trunk
by repository service 13 Feb '10

13 Feb '10

Author: hailfinger Date: Sat Feb 13 20:22:11 2010 New Revision: 901 URL: http://flashrom.org/trac/coreboot/changeset/901 Log: Kill an erroneous .erase introduced in r900. 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/flashchips.c Modified: trunk/flashchips.c ============================================================================== --- trunk/flashchips.c Sat Feb 13 19:41:53 2010 (r900) +++ trunk/flashchips.c Sat Feb 13 20:22:11 2010 (r901) @@ -2342,7 +2342,6 @@ .tested = TEST_UNTESTED, .probe = probe_82802ab, .probe_timing = TIMING_ZERO, /* Datasheet has no timing info specified */ - .erase = NULL, .block_erasers = { {

1 0

[commit] r900 - trunk
by repository service 13 Feb '10

13 Feb '10

Author: snelson Date: Sat Feb 13 19:41:53 2010 New Revision: 900 URL: http://flashrom.org/trac/coreboot/changeset/900 Log: Adds support for the Intel E28F004S5 flash chip. Signed-off-by: Sean Nelson <audiohacked(a)gmail.com> Acked-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006(a)gmx.net> Modified: trunk/flashchips.c trunk/flashchips.h Modified: trunk/flashchips.c ============================================================================== --- trunk/flashchips.c Fri Feb 12 20:37:25 2010 (r899) +++ trunk/flashchips.c Sat Feb 13 19:41:53 2010 (r900) @@ -2333,6 +2333,29 @@ { .vendor = "Intel", + .name = "28F004S5", + .bustype = CHIP_BUSTYPE_PARALLEL, + .manufacture_id = INTEL_ID, + .model_id = E_28F004S5, + .total_size = 512, + .page_size = 256, + .tested = TEST_UNTESTED, + .probe = probe_82802ab, + .probe_timing = TIMING_ZERO, /* Datasheet has no timing info specified */ + .erase = NULL, + .block_erasers = + { + { + .eraseblocks = { {64 * 1024, 8} }, + .block_erase = erase_82802ab_block, + }, + }, + .write = write_82802ab, + .read = read_memmapped, + }, + + { + .vendor = "Intel", .name = "82802AB", .bustype = CHIP_BUSTYPE_FWH, .manufacture_id = INTEL_ID, Modified: trunk/flashchips.h ============================================================================== --- trunk/flashchips.h Fri Feb 12 20:37:25 2010 (r899) +++ trunk/flashchips.h Sat Feb 13 19:41:53 2010 (r900) @@ -254,6 +254,9 @@ #define INTEL_ID 0x89 /* Intel */ #define I_82802AB 0xAD #define I_82802AC 0xAC +#define E_28F004S5 0xA7 +#define E_28F008S5 0xA6 +#define E_28F016S5 0xAA #define P28F001BXT 0x94 /* 28F001BX-T */ #define P28F001BXB 0x95 /* 28F001BX-B */ #define SHARP_LH28F008SA 0xA2 /* Sharp chip, Intel Vendor ID */

1 0

Re: [flashrom] [PATCH] [rfc/demo] implementation of the Thinkpad T20/T21/T22 host
by Carl-Daniel Hailfinger 13 Feb '10

13 Feb '10

On 16.01.2010 13:54, Michael Karcher wrote: > This mainboard uses external pins for banking the ROM chip, which seems > to be impossible to do with the "internal" programmer, yet I had to clone > most of the init stuff from internal.c . Hints for a clean implementation > will be appreciated. > It would be possible to make a copy of the selected struct programmer array element before running programmer_init, and have the programmer-specific programmer_init change the function pointers on the fly. In your case, we'd change all chip_{read,write}[bwln] and {un,}map_flash_region pointers inside a board enable function. With register_shutdown it should be possible to have your shutdown function run in the intended place. > --- > Makefile | 2 +- > flash.h | 11 ++++++ > flashrom.c | 19 ++++++++++ > t20.c | 108 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > 4 files changed, 139 insertions(+), 1 deletions(-) > create mode 100644 t20.c > > diff --git a/Makefile b/Makefile > index 501f65a..cdd45d1 100644 > --- a/Makefile > +++ b/Makefile > @@ -101,7 +101,7 @@ CONFIG_PRINT_WIKI ?= no > > ifeq ($(CONFIG_INTERNAL), yes) > FEATURE_CFLAGS += -D'INTERNAL_SUPPORT=1' > -PROGRAMMER_OBJS += chipset_enable.o board_enable.o cbtable.o it87spi.o ichspi.o sb600spi.o wbsio_spi.o > +PROGRAMMER_OBJS += chipset_enable.o board_enable.o cbtable.o it87spi.o ichspi.o sb600spi.o t20.o wbsio_spi.o > NEED_PCI := yes > endif > > diff --git a/flash.h b/flash.h > index 9a25c70..77b0d48 100644 > --- a/flash.h > +++ b/flash.h > @@ -58,6 +58,9 @@ enum programmer { > #if INTERNAL_SUPPORT == 1 > PROGRAMMER_IT87SPI, > #endif > +#if INTERNAL_SUPPORT == 1 > + PROGRAMMER_TPT20, > +#endif > #if FT2232_SPI_SUPPORT == 1 > PROGRAMMER_FT2232SPI, > #endif > @@ -435,6 +438,14 @@ uint8_t drkaiser_chip_readb(const chipaddr addr); > extern struct pcidev_status drkaiser_pcidev[]; > #endif > > +/* t20.c */ > +#if INTERNAL_SUPPORT == 1 > +int t20_init(void); > +int t20_shutdown(void); > +void t20_chip_writeb(uint8_t val, chipaddr addr); > +uint8_t t20_chip_readb(const chipaddr addr); > +#endif > + > /* satasii.c */ > #if SATASII_SUPPORT == 1 > int satasii_init(void); > diff --git a/flashrom.c b/flashrom.c > index db44c2f..7f05eeb 100644 > --- a/flashrom.c > +++ b/flashrom.c > @@ -226,6 +226,25 @@ const struct programmer_entry programmer_table[] = { > }, > #endif > > +#if INTERNAL_SUPPORT == 1 > + { > + .name = "t20", > + .init = t20_init, > + .shutdown = t20_shutdown, > + .map_flash_region = fallback_map, > + .unmap_flash_region = fallback_unmap, > + .chip_readb = t20_chip_readb, > + .chip_readw = fallback_chip_readw, > + .chip_readl = fallback_chip_readl, > + .chip_readn = fallback_chip_readn, > + .chip_writeb = t20_chip_writeb, > + .chip_writew = fallback_chip_writew, > + .chip_writel = fallback_chip_writel, > + .chip_writen = fallback_chip_writen, > + .delay = internal_delay, > + }, > +#endif > + > #if FT2232_SPI_SUPPORT == 1 > { > .name = "ft2232spi", > diff --git a/t20.c b/t20.c > new file mode 100644 > index 0000000..d6841cc > --- /dev/null > +++ b/t20.c > @@ -0,0 +1,108 @@ > +/* > + * This file is part of the flashrom project. > + * > + * Copyright (C) 2010 Michael Karcher <flashrom(a)mkarcher.dialup.fu-berlin.de> > + * > + * 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; either version 2 of the License, or > + * (at your option) any later version. > + * > + * 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 > + */ > + > +/* Base address of register IND1 in the Field IMGA in the DSDT */ > +#define IMGA1_BASE 0x15EE > It would be great if we could get this info from a live system, and not have to rely on hardcoding it (may depend on BIOS version, installed hardware etc.). No idea if it is possible to ask ACPI about that value (at least without including a big ACPI parser in flashrom). > + > +#include "flash.h" > + > +static uint8_t *t20_base; > + > +#define MAX_COUNT 20 > + > +static void imga1_mask(uint8_t index, uint8_t mask, uint8_t value) > This function can fail. Should it return int instead of void? > +{ > + uint16_t temp; > + int count = 0; > + /* Be careful, as ACPI might use these registers too, so check > + that the index is still OK. */ > Comment style nitpick: Please start the second and following lines of a multiline comment with * and terminate such comments on a separate line. I don't see the "Be careful" motto mentioned above in the code below. If ACPI accesses IMGA1_BASE, we will pretty sure mess up ACPI accesses there. It's a race condition, and since we are not in control of ACPI, there is nothing we can do to reduce impact on the ACPI code. Besides that, there is a time window between the last INW and the final OUTW where anything can happen. > + do { > + OUTB(index, IMGA1_BASE); > + temp = INW(IMGA1_BASE); > OUTB, but INW. Is there a reason for the size mismatch? > + } > + while ((temp & 0xff) != index && ++count < MAX_COUNT); > Minor coding style nitpick: The while statement should be on the same line as the closing brace. > + if(count == MAX_COUNT) > + { > + printf("error: IMGA1 index stuck at %02x\n", temp & 0xff); > + return; > + } > + temp &= ~(mask << 8); > OK, maybe I've stared at such code too often, but given that mask is uint8_t and shifting happens inside brackets, isn't the compiler free to treat the calculation inside brackets as yielding an uint8_t as well, causing the contents of the bracket to be always zero, or to wrap around the bitshift at an 8-bit boundary, causing the contents of the bracket to be unshifted? I would have expected a cast of mask to uint16_t. > + temp |= (value & mask) << 8; > + OUTW(temp, IMGA1_BASE); > To be honest, the code flow in this function seems obvious at first, and on a second look it becomes an example of clever (maybe even too clever) coding. A few comments about the behaviour of IMGA1_BASE would be very appreciated. So far it seems that IMGA1_BASE is used to refer to two registers, namely an index/value pair. The OUTW is a trick to get an atomic write of index+value (is that guaranteed to be atomic by the x86 ISA?), a similar explanation probably applies to the INW. > +} > + > +int t20_init(void) > +{ > + pacc = pci_alloc(); > + pci_init(pacc); > + pci_scan_bus(pacc); /* We want to get the list of devices */ > + get_io_perms(); > I'm not 100% sure if the ordering above is correct. On some fringe platform, we might have to get IO permission before scanning the PCI bus. This is only a minor nitpick, not a hard requirement. > + /* ID of the graphics chip in T20/T21/T22. The mainboards are > + identical in these models */ > + if (!pci_card_find(0x5333, 0x8c12, 0x1014, 0x017f)) > + { > + msg_perr("This is not a ThinkPad T20/T21/T22\n"); > + return 1; > + } > IMHO the matching is too loose here. I'd like to see an additional DMI match and/or a second PCI device match. > + if (chipset_flash_enable() == -2) { > + printf("WARNING: No chipset found. Flash detection " > + "will most likely fail.\n"); > + } > What if the chipset enable returns -1? > + > + imga1_mask(0, 4, 4); /* Enable flash writes */ > + t20_base = physmap("IBM T20 mainboard flash", 0xFFFE0000, 0x20000); > A comment about the address used for physmap would be very nice, even if it is just "The EC maps a window of the flash chip to 4G-0x20000". > + buses_supported = CHIP_BUSTYPE_PARALLEL; > + return 0; > +} > + > +int t20_shutdown(void) > +{ > + physunmap(t20_base, 0x20000); > + imga1_mask(0, 4, 4); /* Disable flash writes */ > + release_io_perms(); > + pci_cleanup(pacc); > + return 0; > +} > + > +static unsigned int currentbank = -1U; > You could move currentbank inside switchtobank. Not sure if that is a good idea, though. > + > +static void switchtobank(unsigned int banknum) > +{ > + if (banknum > 3) > + { > + msg_perr("Bad bank number %d\n", banknum); > + return; > + } > + if(banknum != currentbank) > + imga1_mask(0, 3, banknum); > + currentbank = banknum; > +} > + > +void t20_chip_writeb(uint8_t val, chipaddr addr) > +{ > + switchtobank(addr >> 17); > + mmio_writeb(val, t20_base + (addr & 0x1FFFF)); > +} > + > +uint8_t t20_chip_readb(chipaddr addr) > +{ > + switchtobank(addr >> 17); > + return mmio_readb(t20_base + (addr & 0x1FFFF)); > +} > Overall, I like the patch and will probably ack it next time. Regards, Carl-Daniel -- Developer quote of the year: "We are juggling too many chainsaws and flaming arrows and tigers."

1 0

[patch] NetBSD/amd64 build fix
by Jonathan A. Kollasch 13 Feb '10

13 Feb '10

Use uname -p instead of -m on NetBSD so we get the right architecture library name. Signed-off-by: Jonathan A. Kollasch <jakllsch(a)kollasch.net> ---

1 0

← Newer
1
...
8
9
10
11
12
13
14
...
18
Older →

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

flashrom February 2010