[coreboot] flashrom: Winbond W83627DHG SPI, RDID 90, SST AAI & Intel D201GLY
Carl-Daniel Hailfinger
c-d.hailfinger.devel.2006 at gmx.net
Mon Jul 7 15:55:24 CEST 2008
On 07.07.2008 09:18, Peter Stuge wrote:
> Patch attached.
>
> The wbsio driver has been tested by rphlx to work correctly for probe
> and read on the Intel D201GLY board, but erase and write was not
> tested since he had no easy recovery strategy in case of failure. If
> anyone can test erase, write or both on a board with W83627DHG or
> compatible (please add ids to wbsio_get_spibase()) I would be most
> grateful. Comments, as always, appreciated.
>
Nice patch. Review follows:
> flashrom: Winbond SPI driver, RDID 90 probe, SST25VF040B.RDID90 chip, SST AAI write and Intel D201GLY board enable
>
> The Winbond superio SPI driver supports the maximum possible flash chip size
> of 8Mbit. Unfortunately Winbond left out the 4 top address bits in the
> programming interface, so there is no way to reach data below 4GB-1Mbyte.
> Chip erase still works.
>
I have not yet reviewed the Winbond driver itself, only the hooks for it
in generic code.
> Winbond superio SPI masters only support certain command types, so a new probe
> is needed at least for the SST25VF040B.
>
It should be possible to use Winbond command mode 3 (for partial match)
and command mode b (for cycling match) to emulate RDID.
If that doesn't work, we need a set of 3 IDs per SPI chip: RDID, REMS, RES.
> Add prettyprinting for the SST25VF040B status register.
>
> SST AAI is Auto Address Increment writing, a streamed write to the flash chip
> where the first write command sets a starting address, and following commands
> simply append data. Unfortunately not supported by Winbond SPI masters.
>
I believe AAI may be slower than per-page programming due to bigger
overhead.
Programming in AAI needs
1(command)+3(dummyaddr)+2(data)+(1(command)+2(data))*127(aaiwrites) =
387 cycles for 256 bytes
Programming in 16 byte chunks needs
(1(command)+3(address)+16(data))*16(chunks) = 320 cycles for 256 bytes
Programming in 64 byte chunks needs
(1(command)+3(address)+64(data))*4(chunks) = 272 cycles for 256 bytes
Programming in 256 byte chunks needs 1(command)+3(address)+256(data) =
260 cycles for 256 bytes
> Add an autodetected board enable for the mini-ITX Intel Desktop Board D201GLY.
> The PCI ids are 0:2.5 SiS IDE and 1:0.0 SiS VGA with Intel subsystem ids.
>
> Signed-off-by: Peter Stuge <peter at stuge.se>
>
Can you split this patch into multiple ones?
- Winbond SPI master support
- SST status prettyprinting support
- AAI support (which is chip-specific)
- D201GLY board enable
> Index: flash.h
> ===================================================================
> --- flash.h (revision 3418)
> +++ flash.h (working copy)
> @@ -378,7 +378,8 @@
> BUS_TYPE_ICH7_SPI,
> BUS_TYPE_ICH9_SPI,
> BUS_TYPE_IT87XX_SPI,
> - BUS_TYPE_VIA_SPI
> + BUS_TYPE_VIA_SPI,
> + BUS_TYPE_WBSIO_SPI
> } flashbus_t;
>
> extern flashbus_t flashbus;
> @@ -411,6 +412,7 @@
> extern char *lb_part, *lb_vendor;
>
> /* spi.c */
> +int probe_spi_rdid_90(struct flashchip *flash);
>
That's actually the REMS (read electronci manufacturer signature)
command, not a RDID variant. Please call it probe_spi_rems.
> int probe_spi_rdid(struct flashchip *flash);
> int probe_spi_rdid4(struct flashchip *flash);
> int probe_spi_res(struct flashchip *flash);
> @@ -424,6 +426,7 @@
> void spi_disable_blockprotect(void);
> void spi_byte_program(int address, uint8_t byte);
> void spi_nbyte_read(int address, uint8_t *bytes, int len);
> +int spi_aai_write(struct flashchip *flash, uint8_t *buf);
>
> /* 82802ab.c */
> int probe_82802ab(struct flashchip *flash);
> @@ -435,6 +438,12 @@
> int erase_29f040b(struct flashchip *flash);
> int write_29f040b(struct flashchip *flash, uint8_t *buf);
>
> +/* board_enable.c */
> +void w836xx_ext_enter(uint16_t port);
> +void w836xx_ext_leave(uint16_t port);
> +unsigned char wbsio_read(uint16_t index, uint8_t reg);
> +void wbsio_write(uint16_t index, uint8_t reg, uint8_t data);
> +
> /* ichspi.c */
> int ich_spi_command(unsigned int writecnt, unsigned int readcnt, const unsigned char *writearr, unsigned char *readarr);
> int ich_spi_read(struct flashchip *flash, uint8_t * buf);
> @@ -528,6 +537,12 @@
> /* w49f002u.c */
> int write_49f002(struct flashchip *flash, uint8_t *buf);
>
> +/* wbsio_spi.c */
> +int wbsio_check_for_spi(const char *name);
> +int wbsio_spi_command(unsigned int writecnt, unsigned int readcnt, const unsigned char *writearr, unsigned char *readarr);
> +int wbsio_spi_read(struct flashchip *flash, uint8_t *buf);
> +int wbsio_spi_write(struct flashchip *flash, uint8_t *buf);
> +
> /* stm50flw0x0x.c */
> int probe_stm50flw0x0x(struct flashchip *flash);
> int erase_stm50flw0x0x(struct flashchip *flash);
> Index: flashchips.c
> ===================================================================
> --- flashchips.c (revision 3418)
> +++ flashchips.c (working copy)
> @@ -69,6 +69,7 @@
> {"Spansion", "S25FL016A", SPANSION_ID, SPANSION_S25FL016A, 2048, 256, TEST_OK_PREW, probe_spi_rdid, spi_chip_erase_c7, spi_chip_write, spi_chip_read},
> {"SST", "SST25VF016B", SST_ID, SST_25VF016B, 2048, 256, TEST_UNTESTED, probe_spi_rdid, spi_chip_erase_c7, spi_chip_write, spi_chip_read},
> {"SST", "SST25VF040B", SST_ID, SST_25VF040B, 512, 256, TEST_UNTESTED, probe_spi_rdid, spi_chip_erase_c7, spi_chip_write, spi_chip_read},
> + {"SST", "SST25VF040B.RDID90", 0xbf, 0x8d, 512, 64 * 1024, TEST_OK_PR, probe_spi_rdid_90, spi_chip_erase_c7, spi_aai_write, spi_chip_read},
>
probe_spi_rems instead of probe_spi_rdid_90, but to be honest, having a
SPI controller which does not support RDID is a real PITA and needs a
structure redesign.
> {"SST", "SST28SF040A", SST_ID, SST_28SF040, 512, 256, TEST_UNTESTED, probe_28sf040, erase_28sf040, write_28sf040},
> {"SST", "SST29EE010", SST_ID, SST_29EE010, 128, 128, TEST_OK_PREW, probe_jedec, erase_chip_jedec, write_jedec},
> {"SST", "SST29LE010", SST_ID, SST_29LE010, 128, 128, TEST_UNTESTED, probe_jedec, erase_chip_jedec, write_jedec},
> Index: spi.c
> ===================================================================
> --- spi.c (revision 3418)
> +++ spi.c (working copy)
> @@ -41,6 +41,8 @@
> case BUS_TYPE_ICH9_SPI:
> case BUS_TYPE_VIA_SPI:
> return ich_spi_command(writecnt, readcnt, writearr, readarr);
> + case BUS_TYPE_WBSIO_SPI:
> + return wbsio_spi_command(writecnt, readcnt, writearr, readarr);
> default:
> printf_debug("%s called, but no SPI chipset/strapping detected\n", __FUNCTION__);
> }
> @@ -130,6 +132,25 @@
> return 0;
> }
>
> +int probe_spi_rdid_90(struct flashchip *flash) {
>
probe_spi_rems
> + unsigned char r[2];
>
please call this readarr for consistency
> + const unsigned char cmd[4] = {0x90, 0, 0, 0};
>
const unsigned char cmd[JEDEC_REMS_OUTSIZE] = {JEDEC_REMS, 0, 0, 0};
> +
> + if (spi_command(sizeof(cmd), 2, cmd, r))
>
if (spi_command(JEDEC_REMS_OUTSIZE, JEDEC_REMS_INSIZE, cmd, r))
> + return 0;
> +
> + if (!oddparity(r[0]))
> + printf_debug("0x90 RDID byte 0 parity violation.\n");
>
"REMS..."
> +
> + printf_debug("%s: id1 0x%02x, id2 0x%02x\n", __func__, r[0], r[1]);
> +
> + if (r[0] != flash->manufacture_id || r[1] != flash->model_id)
> + return 0;
> +
> + spi_prettyprint_status_register(flash);
> + return 1;
> +}
> +
> int probe_spi_rdid(struct flashchip *flash) {
> return probe_spi_rdid_generic(flash, 3);
> }
> @@ -142,6 +163,7 @@
> case BUS_TYPE_ICH7_SPI:
> case BUS_TYPE_ICH9_SPI:
> case BUS_TYPE_VIA_SPI:
> + case BUS_TYPE_WBSIO_SPI:
> return probe_spi_rdid_generic(flash, 4);
> default:
> printf_debug("4b ID not supported on this SPI controller\n");
> @@ -182,7 +204,7 @@
> uint8_t spi_read_status_register()
> {
> const unsigned char cmd[JEDEC_RDSR_OUTSIZE] = {JEDEC_RDSR};
> - unsigned char readarr[JEDEC_RDSR_INSIZE];
> + unsigned char readarr[2]; /* JEDEC_RDSR_INSIZE=1 but wbsio needs 2 */
>
Simply change JEDEC_RDSR_INSIZE to 2.
>
> /* Read Status Register */
> spi_command(sizeof(cmd), sizeof(readarr), cmd, readarr);
> @@ -243,6 +265,24 @@
> bpt[(status & 0x1c) >> 2]);
> }
>
> +void spi_prettyprint_status_register_sst25vf040b(uint8_t status)
> +{
> + const char *bpt[] = {
> + "none",
> + "0x70000-0x7ffff",
> + "0x60000-0x7ffff",
> + "0x40000-0x7ffff",
> + "all blocks", "all blocks", "all blocks", "all blocks"
> + };
> + printf_debug("Chip status register: Block Protect Write Disable "
> + "(BPL) is %sset\n", (status & (1 << 7)) ? "" : "not ");
> + printf_debug("Chip status register: Auto Address Increment Programming "
> + "(AAI) is %sset\n", (status & (1 << 6)) ? "" : "not ");
> + spi_prettyprint_status_register_common(status);
> + printf_debug("Resulting block protection : %s\n",
> + bpt[(status & 0x3c) >> 2]);
> +}
> +
> void spi_prettyprint_status_register(struct flashchip *flash)
> {
> uint8_t status;
> @@ -260,8 +300,15 @@
> spi_prettyprint_status_register_st_m25p(status);
> break;
> case SST_ID:
> - if (flash->model_id == SST_25VF016B)
> + switch (flash->model_id) {
> + case SST_25VF016B:
> spi_prettyprint_status_register_sst25vf016(status);
> + break;
> + case 0x8d:
> + case SST_25VF040B:
>
These case statements are very inconsistent. Please use #defines for all
of them.
> + spi_prettyprint_status_register_sst25vf040b(status);
> + break;
> + }
> break;
> }
> }
> @@ -384,6 +431,8 @@
> case BUS_TYPE_ICH9_SPI:
> case BUS_TYPE_VIA_SPI:
> return ich_spi_read(flash, buf);
> + case BUS_TYPE_WBSIO_SPI:
> + return wbsio_spi_read(flash, buf);
> default:
> printf_debug("%s called, but no SPI chipset/strapping detected\n", __FUNCTION__);
> }
> @@ -400,6 +449,8 @@
> case BUS_TYPE_ICH9_SPI:
> case BUS_TYPE_VIA_SPI:
> return ich_spi_write(flash, buf);
> + case BUS_TYPE_WBSIO_SPI:
> + return wbsio_spi_write(flash, buf);
> default:
> printf_debug("%s called, but no SPI chipset/strapping detected\n", __FUNCTION__);
> }
> @@ -407,3 +458,28 @@
> return 1;
> }
>
> +int spi_aai_write(struct flashchip *flash, uint8_t *buf) {
> + uint32_t pos = 2, size = flash->total_size * 1024;
> + unsigned char w[6] = {0xad, 0, 0, 0, buf[0], buf[1]};
>
Symbolic constants/#define please.
> + switch (flashbus) {
> + case BUS_TYPE_WBSIO_SPI:
>
This switch/case statement needs to be a bit more complicated: For IT87
SPI, something similar to AAI is possible if the number of bytes is 256
or less and if the addresses are below 512k.
> + fprintf(stderr, "%s: impossible through Winbond SPI masters, degrading to byte program\n", __func__);
> + return spi_chip_write(flash, buf);
> + default:
> + break;
> + }
> + spi_chip_erase_c7(flash);
>
Please use the corresponding pointer in struct flashchip.
> + spi_write_enable();
> + spi_command(6, 0, w, NULL);
>
Symbolic constants/#define please.
> + while (spi_read_status_register() & JEDEC_RDSR_BIT_WIP)
> + myusec_delay(5); /* SST25VF040B Tbp is max 10us */
> + while (pos < size) {
> + w[1] = buf[pos++];
> + w[2] = buf[pos++];
> + spi_command(3, 0, w, NULL);
> + while (spi_read_status_register() & JEDEC_RDSR_BIT_WIP)
> + myusec_delay(5); /* SST25VF040B Tbp is max 10us */
> + }
> + spi_write_disable();
> + return 0;
> +}
>
The function definitely needs more comments. The inner workings are not
obvious at first glance which suggests some rather clever programming.
> Index: Makefile
> ===================================================================
> --- Makefile (revision 3418)
> +++ Makefile (working copy)
> @@ -29,7 +29,7 @@
> w49f002u.o 82802ab.o pm49fl00x.o sst49lf040.o \
> sst49lfxxxc.o sst_fwhub.o layout.o cbtable.o flashchips.o \
> flashrom.o w39v080fa.o sharplhf00l04.o w29ee011.o spi.o it87spi.o \
> - ichspi.o
> + ichspi.o wbsio_spi.o
>
> all: pciutils dep $(PROGRAM)
>
> Index: wbsio_spi.c
> ===================================================================
> --- wbsio_spi.c (revision 0)
> +++ wbsio_spi.c (revision 0)
> @@ -0,0 +1,195 @@
> +/*
> + * This file is part of the flashrom project.
> + *
> + * Copyright (C) 2008 Peter Stuge <peter at stuge.se>
> + *
> + * 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
> + */
> +
> +#include <stdio.h>
> +#include <pci/pci.h>
> +#include <stdint.h>
> +#include <string.h>
> +#include "flash.h"
> +#include "spi.h"
> +
> +#define WBSIO_PORT1 0x2e
> +#define WBSIO_PORT2 0x4e
> +
> +static uint16_t wbsio_spibase = 0;
> +
> +static uint16_t wbsio_get_spibase(uint16_t port) {
> + uint8_t id;
> + uint16_t flashport = 0;
> +
> + w836xx_ext_enter(port);
> + id = wbsio_read(port, 0x20);
> + if(id != 0xa0) {
> + fprintf(stderr, "W83627 not found at 0x%x, id=0x%02x want=0xa0.\n", port, id);
> + goto done;
> + }
> +
> + if (0 == (wbsio_read(port, 0x24) & 2)) {
> + fprintf(stderr, "W83627 found at 0x%x, but SPI pins are not enabled. (CR[0x24] bit 1=0)\n", port);
> + goto done;
> + }
> +
> + wbsio_write(port, 0x07, 0x06);
> + if (0 == (wbsio_read(port, 0x30) & 1)) {
> + fprintf(stderr, "W83627 found at 0x%x, but SPI is not enabled. (LDN6[0x30] bit 0=0)\n", port);
> + goto done;
> + }
> +
> + flashport = (wbsio_read(port, 0x62) << 8) | wbsio_read(port, 0x63);
> +
> +done:
> + w836xx_ext_leave(port);
> + return flashport;
> +}
> +
> +int wbsio_check_for_spi(const char *name) {
> + if (0 == (wbsio_spibase = wbsio_get_spibase(WBSIO_PORT1)))
> + if (0 == (wbsio_spibase = wbsio_get_spibase(WBSIO_PORT2)))
> + return 1;
> +
> + printf_debug("wbsio_spibase = 0x%x\n", wbsio_spibase);
> + flashbus = BUS_TYPE_WBSIO_SPI;
> + return 0;
> +}
> +
> +/* W83627DHG has 11 command modes:
> + * 1=1 command only
> + * 2=1 command+1 data write
> + * 3=1 command+2 data read
> + * 4=1 command+3 address
> + * 5=1 command+3 address+1 data write
> + * 6=1 command+3 address+4 data write
> + * 7=1 command+3 address+1 dummy address inserted by wbsio+4 data read
> + * 8=1 command+3 address+1 data read
> + * 9=1 command+3 address+2 data read
> + * a=1 command+3 address+3 data read
> + * b=1 command+3 address+4 data read
> + */
> +int wbsio_spi_command(unsigned int writecnt, unsigned int readcnt, const unsigned char *writearr, unsigned char *readarr) {
> + int i;
> + uint8_t mode = 0;
> +
> + printf_debug("%s:", __func__);
> +
> + if (1 == writecnt && 0 == readcnt) {
> + mode = 0x10;
> + } else if (2 == writecnt && 0 == readcnt) {
> + OUTB(writearr[1], wbsio_spibase + 4);
> + printf_debug(" data=0x%02x", writearr[1]);
> + mode = 0x20;
> + } else if (1 == writecnt && 2 == readcnt) {
> + mode = 0x30;
> + } else if (4 == writecnt && 0 == readcnt) {
> + printf_debug(" addr=0x%02x", (writearr[1] & 0x0f));
> + for (i = 2; i < writecnt; i++) {
> + OUTB(writearr[i], wbsio_spibase + i);
> + printf_debug("%02x", writearr[i]);
> + }
> + mode = 0x40 | (writearr[1] & 0x0f);
> + } else if (5 == writecnt && 0 == readcnt) {
> + printf_debug(" addr=0x%02x", (writearr[1] & 0x0f));
> + for (i = 2; i < 4; i++) {
> + OUTB(writearr[i], wbsio_spibase + i);
> + printf_debug("%02x", writearr[i]);
> + }
> + OUTB(writearr[i], wbsio_spibase + i);
> + printf_debug(" data=0x%02x", writearr[i]);
> + mode = 0x50 | (writearr[1] & 0x0f);
> + } else if (8 == writecnt && 0 == readcnt) {
> + printf_debug(" addr=0x%02x", (writearr[1] & 0x0f));
> + for (i = 2; i < 4; i++) {
> + OUTB(writearr[i], wbsio_spibase + i);
> + printf_debug("%02x", writearr[i]);
> + }
> + printf_debug(" data=0x");
> + for (; i < writecnt; i++) {
> + OUTB(writearr[i], wbsio_spibase + i);
> + printf_debug("%02x", writearr[i]);
> + }
> + mode = 0x60 | (writearr[1] & 0x0f);
> + } else if (5 == writecnt && 4 == readcnt) {
> + /* XXX: TODO not supported by flashrom infrastructure!
> + * This mode, 7, discards the fifth byte in writecnt,
> + * but since we can not express that in flashrom, fail
> + * the operation for now.
> + */
> + ;
> + } else if (4 == writecnt && readcnt >= 1 && readcnt <= 4) {
> + printf_debug(" addr=0x%02x", (writearr[1] & 0x0f));
> + for (i = 2; i < writecnt; i++) {
> + OUTB(writearr[i], wbsio_spibase + i);
> + printf_debug("%02x", writearr[i]);
> + }
> + mode = ((7 + readcnt) << 4) | (writearr[1] & 0x0f);
> + }
> + printf_debug(" cmd=%02x mode=%02x\n", writearr[0], mode);
> +
> + if (!mode) {
> + fprintf(stderr, "%s: unsupported command type wr=%d rd=%d\n",
> + __func__, writecnt, readcnt);
> + return 1;
> + }
> +
> + OUTB(writearr[0], wbsio_spibase);
> + OUTB(mode, wbsio_spibase + 1);
> + myusec_delay(10);
> +
> + if (!readcnt)
> + return 0;
> +
> + printf_debug("%s: returning data =", __func__);
> + for (i = 0; i < readcnt; i++) {
> + readarr[i] = INB(wbsio_spibase + 4 + i);
> + printf_debug(" 0x%02x", readarr[i]);
> + }
> + printf_debug("\n");
> +
> + return 0;
> +}
> +
> +int wbsio_spi_read(struct flashchip *flash, uint8_t *buf) {
> + int size = flash->total_size * 1024;
> +
> + if (flash->total_size > 1024) {
> + fprintf(stderr, "%s: Winbond saved on 4 register bits, max chip size is 512 KB. :\\\n", __func__);
> + return 1;
> + }
> +
> + memcpy(buf, (const char *)flash->virtual_memory, size);
> + return 0;
> +}
> +
> +int wbsio_spi_write(struct flashchip *flash, uint8_t *buf) {
> + int pos, size = flash->total_size * 1024;
> +
> + if (flash->total_size > 1024) {
> + fprintf(stderr, "%s: Winbond saved on 4 register bits, max chip size is 512 KB. :\\\n", __func__);
> + return 1;
> + }
> +
> + flash->erase(flash);
> + spi_write_enable();
> + for (pos = 0; pos < size; pos++) {
> + spi_byte_program(pos, buf[pos]);
> + while (spi_read_status_register() & JEDEC_RDSR_BIT_WIP)
> + myusec_delay(10);
> + }
> + spi_write_disable();
> + return 0;
> +}
> Index: board_enable.c
> ===================================================================
> --- board_enable.c (revision 3418)
> +++ board_enable.c (working copy)
> @@ -35,32 +35,32 @@
> * Helper functions for many Winbond Super I/Os of the W836xx range.
> */
> /* Enter extended functions */
> -static void w836xx_ext_enter(uint16_t port)
> +void w836xx_ext_enter(uint16_t port)
> {
> OUTB(0x87, port);
> OUTB(0x87, port);
> }
>
> /* Leave extended functions */
> -static void w836xx_ext_leave(uint16_t port)
> +void w836xx_ext_leave(uint16_t port)
> {
> OUTB(0xAA, port);
> }
>
> /* General functions for reading/writing Winbond Super I/Os. */
> -static unsigned char wbsio_read(uint16_t index, uint8_t reg)
> +unsigned char wbsio_read(uint16_t index, uint8_t reg)
> {
> OUTB(reg, index);
> return INB(index + 1);
> }
>
> -static void wbsio_write(uint16_t index, uint8_t reg, uint8_t data)
> +void wbsio_write(uint16_t index, uint8_t reg, uint8_t data)
> {
> OUTB(reg, index);
> OUTB(data, index + 1);
> }
>
> -static void wbsio_mask(uint16_t index, uint8_t reg, uint8_t data, uint8_t mask)
> +void wbsio_mask(uint16_t index, uint8_t reg, uint8_t data, uint8_t mask)
> {
> uint8_t tmp;
>
> @@ -617,6 +617,8 @@
> NULL, NULL, "BioStar P4M80-M4", board_biostar_p4m80_m4},
> {0x1106, 0x3227, 0x1458, 0x5001, 0x10ec, 0x8139, 0x1458, 0xe000,
> NULL, NULL, "GIGABYTE GA-7VT600", board_biostar_p4m80_m4},
> + {0x1039, 0x5513, 0x8086, 0xd61f, 0x1039, 0x6330, 0x8086, 0xd61f,
> + NULL, NULL, "Intel Desktop Board D201GLY", wbsio_check_for_spi},
> {0, 0, 0, 0, 0, 0, 0, 0, NULL, NULL} /* Keep this */
> };
>
>
>
Regards,
Carl-Daniel
--
http://www.hailfinger.org/
More information about the coreboot
mailing list