On 29.06.2008 01:04, Stefan Reinauer wrote:
> See patch. It's a first attempt, but I think we should get this in in
> small enough step by step improvements.
>
> First attempt to clean up SPI probing and create a common
> construct: the flash bus.
>
> At some point the flash bus will be part of struct flashchip.
>
> Signed-off-by: Stefan Reinauer <[EMAIL PROTECTED]>
>
Nice, but I'd like a small design change:
* Keep a list of available buses: flashbuses[]
* Call all functions with an additional parameter: flashbus
Per-bus probing is a lot easier with that change.
The cleanups unrelated to flash bus support are
Acked-by: Carl-Daniel Hailfinger <[EMAIL PROTECTED]>
For the rest, please consider my design change suggestions.
> Index: flash.h
> ===================================================================
> --- flash.h (revision 3393)
> +++ flash.h (working copy)
> @@ -370,10 +370,18 @@
> /* chipset_enable.c */
> int chipset_flash_enable(void);
> void print_supported_chipsets(void);
> -extern int ich7_detected;
> -extern int ich9_detected;
> -extern void *ich_spibar;
>
> +typedef enum {
>
BUS_TYPE_FWH
BUS_TYPE_PARALLEL
> + BUS_TYPE_LPC,
> + BUS_TYPE_ICH7_SPI,
> + BUS_TYPE_ICH9_SPI,
> + BUS_TYPE_IT87XX_SPI,
> + BUS_TYPE_VIA_SPI
> +} flashbus_t;
>
> +
> +extern flashbus_t flashbus;
>
See my initial comment:
extern flashbus_t flashbuses[];
> +extern void *spibar;
> +
> /* Physical memory mapping device */
> #if defined (__sun) && (defined(__i386) || defined(__amd64))
> # define MEM_DEV "/dev/xsvc"
> Index: it87spi.c
> ===================================================================
> --- it87spi.c (revision 3393)
> +++ it87spi.c (working copy)
> @@ -3,6 +3,7 @@
> *
> * Copyright (C) 2007, 2008 Carl-Daniel Hailfinger
> * Copyright (C) 2008 Ronald Hoogenboom <[EMAIL PROTECTED]>
> + * Copyright (C) 2008 coresystems GmbH
> *
> * 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
> @@ -112,8 +113,13 @@
> int it87xx_probe_spi_flash(const char *name)
>
int it87xx_probe_spi_flash(flashbus_t flashbus, const char *name)
> {
> it8716f_flashport = find_ite_spi_flash_port(ITE_SUPERIO_PORT1);
> +
> if (!it8716f_flashport)
> it8716f_flashport = find_ite_spi_flash_port(ITE_SUPERIO_PORT2);
> +
> + if (it8716f_flashport)
> + flashbus = BUS_TYPE_IT87XX_SPI;
> +
> return (!it8716f_flashport);
> }
>
> Index: spi.c
> ===================================================================
> --- spi.c (revision 3393)
> +++ spi.c (working copy)
> @@ -34,13 +34,15 @@
>
> int spi_command(unsigned int writecnt, unsigned int readcnt, const unsigned
> char *writearr, unsigned char *readarr)
>
int spi_command(flashbus_t flashbus, unsigned int writecnt, unsigned int
readcnt, const unsigned char *writearr, unsigned char *readarr)
> {
> - if (it8716f_flashport)
> + switch (flashbus) {
> + case BUS_TYPE_IT87XX_SPI:
> return it8716f_spi_command(writecnt, readcnt, writearr,
> readarr);
> - else if (ich7_detected)
> + case BUS_TYPE_ICH7_SPI:
> + case BUS_TYPE_ICH9_SPI:
> return ich_spi_command(writecnt, readcnt, writearr, readarr);
> - else if (ich9_detected)
> - return ich_spi_command(writecnt, readcnt, writearr, readarr);
> - printf_debug("%s called, but no SPI chipset detected\n", __FUNCTION__);
> + default:
> + printf_debug("%s called, but no SPI chipset/strapping
> detected\n", __FUNCTION__);
> + }
> return 1;
> }
>
> @@ -299,11 +301,17 @@
>
> void spi_page_program(int block, uint8_t *buf, uint8_t *bios)
>
void spi_page_program(flashbus_t flashbus, int block, uint8_t *buf, uint8_t
*bios)
> {
> - if (it8716f_flashport) {
> + switch (flashbus) {
> + case BUS_TYPE_IT87XX_SPI:
> it8716f_spi_page_program(block, buf, bios);
> - return;
> + break;
> + case BUS_TYPE_ICH7_SPI:
> + case BUS_TYPE_ICH9_SPI:
> + printf_debug("%s called, but not implemented for ICH\n",
> __FUNCTION__);
> + break;
> + default:
> + printf_debug("%s called, but no SPI chipset/strapping
> detected\n", __FUNCTION__);
> }
> - printf_debug("%s called, but no SPI chipset detected\n", __FUNCTION__);
> }
>
> /*
> @@ -358,25 +366,32 @@
>
> int spi_chip_read(struct flashchip *flash, uint8_t *buf)
>
same here.
> {
> - if (it8716f_flashport)
> +
> + switch (flashbus) {
> + case BUS_TYPE_IT87XX_SPI:
> return it8716f_spi_chip_read(flash, buf);
> - else if (ich7_detected)
> - return ich_spi_read(flash, buf);
> - else if (ich9_detected)
> + case BUS_TYPE_ICH7_SPI:
> + case BUS_TYPE_ICH9_SPI:
> return ich_spi_read(flash, buf);
> - printf_debug("%s called, but no SPI chipset detected\n", __FUNCTION__);
> + default:
> + printf_debug("%s called, but no SPI chipset/strapping
> detected\n", __FUNCTION__);
> + }
> +
> return 1;
> }
>
> int spi_chip_write(struct flashchip *flash, uint8_t *buf)
>
same here
> {
> - if (it8716f_flashport)
> + switch (flashbus) {
> + case BUS_TYPE_IT87XX_SPI:
> return it8716f_spi_chip_write(flash, buf);
> - else if (ich7_detected)
> - return ich_spi_write(flash, buf);
> - else if (ich9_detected)
> + case BUS_TYPE_ICH7_SPI:
> + case BUS_TYPE_ICH9_SPI:
> return ich_spi_write(flash, buf);
> - printf_debug("%s called, but no SPI chipset detected\n", __FUNCTION__);
> + default:
> + printf_debug("%s called, but no SPI chipset/strapping
> detected\n", __FUNCTION__);
> + }
> +
> return 1;
> }
>
> Index: chipset_enable.c
> ===================================================================
> --- chipset_enable.c (revision 3393)
> +++ chipset_enable.c (working copy)
> @@ -35,6 +35,17 @@
> #include <unistd.h>
> #include "flash.h"
>
> +/**
> + * flashrom defaults to LPC flash devices. If a known SPI controller is found
> + * and the SPI strappings are set, this will be overwritten by the probing
> code.
> + *
> + * Eventually, this will become an array when multiple flash support works.
> + */
> +
> +flashbus_t flashbus = BUS_TYPE_LPC;
>
flashbus_t flashbuses[MAX_BUSES] = {BUS_TYPE_LPC, };
int flashbus_count=1;
(and matching changes below)
> +void *spibar = NULL;
> +
> +
> static int enable_flash_ali_m1533(struct pci_dev *dev, const char *name)
> {
> uint8_t tmp;
> @@ -124,7 +135,7 @@
> * Note: Accesses to FFFF0000-FFFFFFFF are always forwarded to ISA.
> * Set bit 2: BIOSCS# Write Enable (1=enable, 0=disable).
> */
> - new = old | 0x2c4;
> + new = old | 0x02c4;
>
> if (new == old)
> return 0;
> @@ -185,68 +196,106 @@
> return enable_flash_ich(dev, name, 0xdc);
> }
>
> -void *ich_spibar = NULL;
> +#define ICH_STRAP_RSVD 0x00
> +#define ICH_STRAP_SPI 0x01
> +#define ICH_STRAP_PCI 0x02
> +#define ICH_STRAP_LPC 0x03
>
> -static int enable_flash_ich_dc_spi(struct pci_dev *dev, const char *name,
> unsigned long spibar)
> +static int enable_flash_ich_dc_spi(struct pci_dev *dev, const char *name,
> int ich_generation)
> {
> + int ret, i;
> uint8_t old, new, bbs, buc;
> + uint16_t spibar_offset;
> uint32_t tmp, gcs;
> void *rcrb;
> + static const char *straps_names[] = { "reserved", "SPI", "PCI", "LPC" };
> +
> + /* Enable Flash Writes */
> + ret = enable_flash_ich_dc(dev, name);
>
> - /* Read the Root Complex Base Address Register (RCBA) */
> - tmp = pci_read_long(dev, 0xf0);
> + /* Get physical address of Root Complex Register Block */
> + tmp = pci_read_long(dev, 0xf0) & 0xffffc000;
> + printf_debug("\nRoot Complex Register Block address = 0x%x\n", tmp);
>
> - /* Calculate the Root Complex Register Block address */
> - tmp &= 0xffffc000;
> - printf_debug("\nRoot Complex Register Block address = 0x%x\n", tmp);
> + /* Map RCBA to virtual memory */
> rcrb = mmap(0, 0x4000, PROT_READ | PROT_WRITE, MAP_SHARED, fd_mem,
> (off_t)tmp);
> if (rcrb == MAP_FAILED) {
> perror("Can't mmap memory using " MEM_DEV);
> exit(1);
> }
> - printf_debug("GCS address = 0x%x\n", tmp + 0x3410);
> +
> gcs = *(volatile uint32_t *)(rcrb + 0x3410);
> printf_debug("GCS = 0x%x: ", gcs);
> printf_debug("BIOS Interface Lock-Down: %sabled, ",
> (gcs & 0x1) ? "en" : "dis");
> bbs = (gcs >> 10) & 0x3;
> - printf_debug("BOOT BIOS Straps: 0x%x (%s)\n", bbs,
> - (bbs == 0x3) ? "LPC" : ((bbs == 0x2) ? "PCI" : "SPI"));
> + printf_debug("BOOT BIOS Straps: 0x%x (%s)\n", bbs, straps_names[bbs]);
>
> buc = *(volatile uint8_t *)(rcrb + 0x3414);
> printf_debug("Top Swap : %s\n", (buc & 1)?"enabled (A16 inverted)":"not
> enabled");
>
> + if (bbs == ICH_STRAP_LPC) {
> + /* No further SPI initialization required */
> + return ret;
> + }
> +
> + switch (ich_generation) {
> + case 7:
> + flashbus = BUS_TYPE_ICH7_SPI;
> + spibar_offset = 0x3020;
> + break;
> + case 8:
> + flashbus = BUS_TYPE_ICH9_SPI;
> + spibar_offset = 0x3020;
> + break;
> + case 9:
> + default: /* Future version might behave the same */
> + flashbus = BUS_TYPE_ICH9_SPI;
> + spibar_offset = 0x3800;
> + break;
> + }
> +
> /* SPIBAR is at RCRB+0x3020 for ICH[78] and RCRB+0x3800 for ICH9. */
> - printf_debug("SPIBAR = 0x%x + 0x%04x\n", tmp, (uint16_t)spibar);
> + printf_debug("SPIBAR = 0x%x + 0x%04x\n", tmp, spibar_offset);
>
> - // Assign Virtual Address
> - ich_spibar = rcrb + spibar;
> + /* Assign Virtual Address */
> + spibar = rcrb + spibar_offset;
>
> - if (ich7_detected) {
> - int i;
> - printf_debug("0x00: 0x%04x (SPIS)\n", *(uint16_t
> *)(ich_spibar + 0));
> - printf_debug("0x02: 0x%04x (SPIC)\n", *(uint16_t
> *)(ich_spibar + 2));
> - printf_debug("0x04: 0x%08x (SPIA)\n", *(uint32_t *)(ich_spibar
> + 4));
> + switch (flashbus) {
> + case BUS_TYPE_ICH7_SPI:
> + printf_debug("0x00: 0x%04x (SPIS)\n", *(uint16_t *)(spibar
> + 0));
> + printf_debug("0x02: 0x%04x (SPIC)\n", *(uint16_t *)(spibar
> + 2));
> + printf_debug("0x04: 0x%08x (SPIA)\n", *(uint32_t *)(spibar +
> 4));
> for (i=0; i < 8; i++) {
> int offs;
> offs = 8 + (i * 8);
> - printf_debug("0x%02x: 0x%08x (SPID%d)\n", offs,
> *(uint32_t *)(ich_spibar + offs), i);
> - printf_debug("0x%02x: 0x%08x (SPID%d+4)\n", offs+4,
> *(uint32_t *)(ich_spibar + offs +4), i);
> + printf_debug("0x%02x: 0x%08x (SPID%d)\n", offs,
> *(uint32_t *)(spibar + offs), i);
> + printf_debug("0x%02x: 0x%08x (SPID%d+4)\n", offs+4,
> *(uint32_t *)(spibar + offs +4), i);
> }
> - printf_debug("0x50: 0x%08x (BBAR)\n", *(uint32_t *)(ich_spibar
> + 0x50));
> - printf_debug("0x54: 0x%04x (PREOP)\n", *(uint16_t
> *)(ich_spibar + 0x54));
> - printf_debug("0x56: 0x%04x (OPTYPE)\n", *(uint16_t
> *)(ich_spibar + 0x56));
> - printf_debug("0x58: 0x%08x (OPMENU)\n", *(uint32_t
> *)(ich_spibar + 0x58));
> - printf_debug("0x5c: 0x%08x (OPMENU+4)\n", *(uint32_t
> *)(ich_spibar + 0x5c));
> + printf_debug("0x50: 0x%08x (BBAR)\n", *(uint32_t *)(spibar +
> 0x50));
> + printf_debug("0x54: 0x%04x (PREOP)\n", *(uint16_t *)(spibar
> + 0x54));
> + printf_debug("0x56: 0x%04x (OPTYPE)\n", *(uint16_t
> *)(spibar + 0x56));
> + printf_debug("0x58: 0x%08x (OPMENU)\n", *(uint32_t *)(spibar +
> 0x58));
> + printf_debug("0x5c: 0x%08x (OPMENU+4)\n", *(uint32_t *)(spibar
> + 0x5c));
> for (i=0; i < 4; i++) {
> int offs;
> offs = 0x60 + (i * 4);
> - printf_debug("0x%02x: 0x%08x (PBR%d)\n", offs,
> *(uint32_t *)(ich_spibar + offs), i);
> + printf_debug("0x%02x: 0x%08x (PBR%d)\n", offs,
> *(uint32_t *)(spibar + offs), i);
> }
> printf_debug("\n");
> - if ( (*(uint16_t *)ich_spibar) & (1 << 15)) {
> + if ( (*(uint16_t *)spibar) & (1 << 15)) {
> printf("WARNING: SPI Configuration Lockdown
> activated.\n");
> }
> + break;
> + case BUS_TYPE_ICH9_SPI:
> + /* TODO: Add dumping function for ICH8/ICH9, or drop the
> + * whole SPIBAR dumping from chipset_enable.c - There's
> + * inteltool for this task already.
> + */
> + break;
> + default:
> + /* Nothing */
> + break;
> }
>
> old = pci_read_byte(dev, 0xdc);
> @@ -257,37 +306,30 @@
> case 1:
> case 2:
> printf_debug("prefetching %sabled, caching %sabled, ",
> - (new & 0x2) ? "en" : "dis", (new & 0x1) ? "dis" : "en");
> + (new & 0x2) ? "en" : "dis",
> + (new & 0x1) ? "dis" : "en");
> break;
> default:
> printf_debug("invalid prefetching/caching settings, ");
> break;
> }
> - return enable_flash_ich_dc(dev, name);
> +
> + return ret;
> }
>
> -/* Flag for ICH7 SPI register block */
> -int ich7_detected = 0;
> -
> static int enable_flash_ich7(struct pci_dev *dev, const char *name)
> {
> - ich7_detected = 1;
> - return enable_flash_ich_dc_spi(dev, name, 0x3020);
> + return enable_flash_ich_dc_spi(dev, name, 7);
> }
>
> -/* Flag for ICH8/ICH9 SPI register block */
> -int ich9_detected = 0;
> -
> static int enable_flash_ich8(struct pci_dev *dev, const char *name)
> {
> - ich9_detected = 1;
> - return enable_flash_ich_dc_spi(dev, name, 0x3020);
> + return enable_flash_ich_dc_spi(dev, name, 8);
> }
>
> static int enable_flash_ich9(struct pci_dev *dev, const char *name)
> {
> - ich9_detected = 1;
> - return enable_flash_ich_dc_spi(dev, name, 0x3800);
> + return enable_flash_ich_dc_spi(dev, name, 9);
> }
>
> static int enable_flash_vt823x(struct pci_dev *dev, const char *name)
> Index: ichspi.c
> ===================================================================
> --- ichspi.c (revision 3393)
> +++ ichspi.c (working copy)
> @@ -123,20 +123,20 @@
> static inline uint32_t REGREAD32(int X)
> {
> volatile uint32_t regval;
> - regval = *(volatile uint32_t *) ((uint8_t *) ich_spibar + X);
> + regval = *(volatile uint32_t *) ((uint8_t *) spibar + X);
> return regval;
> }
>
> static inline uint16_t REGREAD16(int X)
> {
> volatile uint16_t regval;
> - regval = *(volatile uint16_t *) ((uint8_t *) ich_spibar + X);
> + regval = *(volatile uint16_t *) ((uint8_t *) spibar + X);
> return regval;
> }
>
> -#define REGWRITE32(X,Y) (*(uint32_t *)((uint8_t *)ich_spibar+X)=Y)
> -#define REGWRITE16(X,Y) (*(uint16_t *)((uint8_t *)ich_spibar+X)=Y)
> -#define REGWRITE8(X,Y) (*(uint8_t *)((uint8_t *)ich_spibar+X)=Y)
> +#define REGWRITE32(X,Y) (*(uint32_t *)((uint8_t *)spibar+X)=Y)
> +#define REGWRITE16(X,Y) (*(uint16_t *)((uint8_t *)spibar+X)=Y)
> +#define REGWRITE8(X,Y) (*(uint8_t *)((uint8_t *)spibar+X)=Y)
>
> /* Common SPI functions */
> static int program_opcodes(OPCODES * op);
> @@ -167,58 +167,50 @@
> int program_opcodes(OPCODES * op)
> {
> uint8_t a;
> - uint16_t temp16;
> - uint32_t temp32;
> + uint16_t preop, optype;
> + uint32_t opmenu[2];
>
> /* Program Prefix Opcodes */
> - temp16 = 0;
> + preop = 0;
> /* 0:7 Prefix Opcode 1 */
> - temp16 = (op->preop[0]);
> + preop = (op->preop[0]);
> /* 8:16 Prefix Opcode 2 */
> - temp16 |= ((uint16_t) op->preop[1]) << 8;
> - if (ich7_detected) {
> - REGWRITE16(ICH7_REG_PREOP, temp16);
> - } else if (ich9_detected) {
> - REGWRITE16(ICH9_REG_PREOP, temp16);
> - }
> -
> + preop |= ((uint16_t) op->preop[1]) << 8;
> +
> /* Program Opcode Types 0 - 7 */
> - temp16 = 0;
> + optype = 0;
> for (a = 0; a < 8; a++) {
> - temp16 |= ((uint16_t) op->opcode[a].spi_type) << (a * 2);
> + optype |= ((uint16_t) op->opcode[a].spi_type) << (a * 2);
> }
> -
> - if (ich7_detected) {
> - REGWRITE16(ICH7_REG_OPTYPE, temp16);
> - } else if (ich9_detected) {
> - REGWRITE16(ICH9_REG_OPTYPE, temp16);
> - }
> -
> -
> +
> /* Program Allowable Opcodes 0 - 3 */
> - temp32 = 0;
> + opmenu[0] = 0;
> for (a = 0; a < 4; a++) {
> - temp32 |= ((uint32_t) op->opcode[a].opcode) << (a * 8);
> + opmenu[0] |= ((uint32_t) op->opcode[a].opcode) << (a * 8);
> }
>
> - if (ich7_detected) {
> - REGWRITE32(ICH7_REG_OPMENU, temp32);
> - } else if (ich9_detected) {
> - REGWRITE32(ICH9_REG_OPMENU, temp32);
> - }
> -
> -
> /*Program Allowable Opcodes 4 - 7 */
> - temp32 = 0;
> + opmenu[1] = 0;
> for (a = 4; a < 8; a++) {
> - temp32 |=
> - ((uint32_t) op->opcode[a].opcode) << ((a - 4) * 8);
> + opmenu[1] |= ((uint32_t) op->opcode[a].opcode) << ((a - 4) * 8);
> }
>
> - if (ich7_detected) {
> - REGWRITE32(ICH7_REG_OPMENU + 4, temp32);
> - } else if (ich9_detected) {
> - REGWRITE32(ICH9_REG_OPMENU + 4, temp32);
> + switch (flashbus) {
> + case BUS_TYPE_ICH7_SPI:
> + REGWRITE16(ICH7_REG_PREOP, preop);
> + REGWRITE16(ICH7_REG_OPTYPE, optype);
> + REGWRITE32(ICH7_REG_OPMENU, opmenu[0]);
> + REGWRITE32(ICH7_REG_OPMENU + 4, opmenu[1]);
> + break;
> + case BUS_TYPE_ICH9_SPI:
> + REGWRITE16(ICH9_REG_PREOP, preop);
> + REGWRITE16(ICH9_REG_OPTYPE, optype);
> + REGWRITE32(ICH9_REG_OPMENU, opmenu[0]);
> + REGWRITE32(ICH9_REG_OPMENU + 4, opmenu[1]);
> + break;
> + default:
> + printf_debug("%s: unsupported chipset\n", __FUNCTION__);
> + return -1;
> }
>
> return 0;
> @@ -332,6 +324,7 @@
> uint8_t datalength, uint8_t * data)
> {
> int write_cmd = 0;
> + int timeout;
> uint32_t temp32;
> uint32_t a;
>
> @@ -402,10 +395,13 @@
> REGWRITE32(ICH9_REG_SSFS, temp32);
>
> /*wait for cycle complete */
> - while ((REGREAD32(ICH9_REG_SSFS) & SSFS_CDS) == 0) {
> - /*TODO; Do something that this can't lead into an endless loop.
> but some
> - * commands may cause this to be last more than 30 seconds */
> + timeout = 1000 * 60; // 60s is a looong timeout.
> + while (((REGREAD32(ICH9_REG_SSFS) & SSFS_CDS) == 0) && --timeout) {
> + myusec_delay(1000);
> }
> + if (!timeout) {
> + printf_debug("timeout\n");
> + }
>
> if ((REGREAD32(ICH9_REG_SSFS) & SSFS_FCERR) != 0) {
> printf_debug("Transaction error!\n");
> @@ -430,10 +426,13 @@
> static int run_opcode(uint8_t nr, OPCODE op, uint32_t offset,
> uint8_t datalength, uint8_t * data)
> {
> - if (ich7_detected)
> + switch (flashbus) {
> + case BUS_TYPE_ICH7_SPI:
> return ich7_run_opcode(nr, op, offset, datalength, data);
> - else if (ich9_detected) {
> + case BUS_TYPE_ICH9_SPI:
> return ich9_run_opcode(nr, op, offset, datalength, data);
> + default:
> + printf_debug("%s: unsupported chipset\n", __FUNCTION__);
> }
>
> /* If we ever get here, something really weird happened */
>
Regards,
Carl-Daniel
--
http://www.hailfinger.org/
--
coreboot mailing list
[email protected]
http://www.coreboot.org/mailman/listinfo/coreboot
