Author: stefanct
Date: Mon Nov  7 00:51:09 2011
New Revision: 1460
URL: http://flashrom.org/trac/flashrom/changeset/1460

Log:
ichspi: use a variable to distinguish ich generations instead of 
spi_programmer->type

The type member is enough most of the time to derive the wanted
information, but
 - not always (e.g. ich_set_bbar),
 - only available after registration, which we want to delay till the
   end of init, and
 - we really want to distinguish between chipset version-grained
   attributes which are not reflected by the registered programmer.

Hence this patch introduces a new static variable which is set up
early by the init functions and allows us to get rid of all "switch
(spi_programmer->type)" in ichspi.c. We reuse the enum introduced
for descriptor mode for the type of the new variable.

Previously magic numbers were passed by chipset_enable wrappers. Now
they use the enumeration items too. To get this working the enum
definition had to be moved to programmer.h.

Another noteworthy detail: previously we have checked for a valid
programmer/ich generation all over the place. I have removed those
checks and added one single check in the init method. Calling any
function of a programmer without executing the init method first, is
undefined behavior.

Signed-off-by: Stefan Tauner <[email protected]>
Acked-by: Carl-Daniel Hailfinger <[email protected]>

Modified:
   trunk/chipset_enable.c
   trunk/ich_descriptors.h
   trunk/ichspi.c
   trunk/programmer.h

Modified: trunk/chipset_enable.c
==============================================================================
--- trunk/chipset_enable.c      Fri Nov  4 22:35:26 2011        (r1459)
+++ trunk/chipset_enable.c      Mon Nov  7 00:51:09 2011        (r1460)
@@ -489,7 +489,7 @@
 }
 
 static int enable_flash_ich_dc_spi(struct pci_dev *dev, const char *name,
-                                  int ich_generation)
+                                  enum ich_chipset ich_generation)
 {
        int ret;
        uint8_t bbs, buc;
@@ -504,7 +504,7 @@
        static const char *const straps_names_unknown[] = { "unknown", 
"unknown", "unknown", "unknown" };
 
        switch (ich_generation) {
-       case 7:
+       case CHIPSET_ICH7:
                /* EP80579 may need further changes, but this is the least
                 * intrusive way to get correct BOOT Strap printing without
                 * changing the rest of its code path). */
@@ -513,13 +513,13 @@
                else
                        straps_names = straps_names_ich7_nm10;
                break;
-       case 8:
-       case 9:
-       case 10:
+       case CHIPSET_ICH8:
+       case CHIPSET_ICH9:
+       case CHIPSET_ICH10:
                straps_names = straps_names_ich8910;
                break;
-       case 11:
-       case 12:
+       case CHIPSET_5_SERIES_IBEX_PEAK:
+       case CHIPSET_6_SERIES_COUGAR_POINT:
                straps_names = straps_names_pch56;
                break;
        default:
@@ -557,7 +557,7 @@
         * on ICH7 when the southbridge is strapped to LPC
         */
        buses_supported = BUS_FWH;
-       if (ich_generation == 7) {
+       if (ich_generation == CHIPSET_ICH7) {
                if (bbs == 0x03) {
                        /* If strapped to LPC, no further SPI initialization is
                         * required. */
@@ -579,34 +579,34 @@
 
 static int enable_flash_ich7(struct pci_dev *dev, const char *name)
 {
-       return enable_flash_ich_dc_spi(dev, name, 7);
+       return enable_flash_ich_dc_spi(dev, name, CHIPSET_ICH7);
 }
 
 static int enable_flash_ich8(struct pci_dev *dev, const char *name)
 {
-       return enable_flash_ich_dc_spi(dev, name, 8);
+       return enable_flash_ich_dc_spi(dev, name, CHIPSET_ICH8);
 }
 
 static int enable_flash_ich9(struct pci_dev *dev, const char *name)
 {
-       return enable_flash_ich_dc_spi(dev, name, 9);
+       return enable_flash_ich_dc_spi(dev, name, CHIPSET_ICH9);
 }
 
 static int enable_flash_ich10(struct pci_dev *dev, const char *name)
 {
-       return enable_flash_ich_dc_spi(dev, name, 10);
+       return enable_flash_ich_dc_spi(dev, name, CHIPSET_ICH10);
 }
 
 /* Ibex Peak aka. 5 series & 3400 series */
 static int enable_flash_pch5(struct pci_dev *dev, const char *name)
 {
-       return enable_flash_ich_dc_spi(dev, name, 11);
+       return enable_flash_ich_dc_spi(dev, name, CHIPSET_5_SERIES_IBEX_PEAK);
 }
 
 /* Cougar Point aka. 6 series & c200 series */
 static int enable_flash_pch6(struct pci_dev *dev, const char *name)
 {
-       return enable_flash_ich_dc_spi(dev, name, 12);
+       return enable_flash_ich_dc_spi(dev, name, 
CHIPSET_6_SERIES_COUGAR_POINT);
 }
 
 static int via_no_byte_merge(struct pci_dev *dev, const char *name)

Modified: trunk/ich_descriptors.h
==============================================================================
--- trunk/ich_descriptors.h     Fri Nov  4 22:35:26 2011        (r1459)
+++ trunk/ich_descriptors.h     Mon Nov  7 00:51:09 2011        (r1460)
@@ -24,6 +24,7 @@
 #define __ICH_DESCRIPTORS_H__ 1
 
 #include <stdint.h>
+#include "programmer.h" /* for enum ich_chipset */
 
 /* FIXME: Replace with generic return codes */
 #define ICH_RET_OK     0
@@ -63,20 +64,6 @@
 #define ICH_FREG_BASE(flreg)  (((flreg) << 12) & 0x01fff000)
 #define ICH_FREG_LIMIT(flreg) (((flreg) >>  4) & 0x01fff000)
 
-/* Used to select the right descriptor printing function.
- * Currently only ICH8 and Ibex Peak are supported.
- */
-enum ich_chipset {
-       CHIPSET_ICH_UNKNOWN,
-       CHIPSET_ICH7 = 7,
-       CHIPSET_ICH8,
-       CHIPSET_ICH9,
-       CHIPSET_ICH10,
-       CHIPSET_5_SERIES_IBEX_PEAK,
-       CHIPSET_6_SERIES_COUGAR_POINT,
-       CHIPSET_7_SERIES_PANTHER_POINT
-};
-
 void prettyprint_ich_reg_vscc(uint32_t reg_val, int verbosity);
 
 struct ich_desc_content {

Modified: trunk/ichspi.c
==============================================================================
--- trunk/ichspi.c      Fri Nov  4 22:35:26 2011        (r1459)
+++ trunk/ichspi.c      Mon Nov  7 00:51:09 2011        (r1460)
@@ -172,6 +172,7 @@
 /* ICH SPI configuration lock-down. May be set during chipset enabling. */
 static int ichspi_lock = 0;
 
+static enum ich_chipset ich_generation = CHIPSET_ICH_UNKNOWN;
 uint32_t ichspi_bbar = 0;
 
 static void *ich_spibar = NULL;
@@ -454,23 +455,20 @@
                return -1;
        }
 
-       switch (spi_programmer->type) {
-       case SPI_CONTROLLER_ICH7:
-       case SPI_CONTROLLER_VIA:
+       switch (ich_generation) {
+       case CHIPSET_ICH7:
                preop = REGREAD16(ICH7_REG_PREOP);
                optype = REGREAD16(ICH7_REG_OPTYPE);
                opmenu[0] = REGREAD32(ICH7_REG_OPMENU);
                opmenu[1] = REGREAD32(ICH7_REG_OPMENU + 4);
                break;
-       case SPI_CONTROLLER_ICH9:
+       case CHIPSET_ICH8:
+       default:                /* Future version might behave the same */
                preop = REGREAD16(ICH9_REG_PREOP);
                optype = REGREAD16(ICH9_REG_OPTYPE);
                opmenu[0] = REGREAD32(ICH9_REG_OPMENU);
                opmenu[1] = REGREAD32(ICH9_REG_OPMENU + 4);
                break;
-       default:
-               msg_perr("%s: unsupported chipset\n", __func__);
-               return -1;
        }
 
        op->preop[0] = (uint8_t) preop;
@@ -529,9 +527,8 @@
        }
 
        msg_pdbg("\n%s: preop=%04x optype=%04x opmenu=%08x%08x\n", __func__, 
preop, optype, opmenu[0], opmenu[1]);
-       switch (spi_programmer->type) {
-       case SPI_CONTROLLER_ICH7:
-       case SPI_CONTROLLER_VIA:
+       switch (ich_generation) {
+       case CHIPSET_ICH7:
                /* Register undo only for enable_undo=1, i.e. first call. */
                if (enable_undo) {
                        rmmio_valw(ich_spibar + ICH7_REG_PREOP);
@@ -544,7 +541,8 @@
                mmio_writel(opmenu[0], ich_spibar + ICH7_REG_OPMENU);
                mmio_writel(opmenu[1], ich_spibar + ICH7_REG_OPMENU + 4);
                break;
-       case SPI_CONTROLLER_ICH9:
+       case CHIPSET_ICH8:
+       default:                /* Future version might behave the same */
                /* Register undo only for enable_undo=1, i.e. first call. */
                if (enable_undo) {
                        rmmio_valw(ich_spibar + ICH9_REG_PREOP);
@@ -557,9 +555,6 @@
                mmio_writel(opmenu[0], ich_spibar + ICH9_REG_OPMENU);
                mmio_writel(opmenu[1], ich_spibar + ICH9_REG_OPMENU + 4);
                break;
-       default:
-               msg_perr("%s: unsupported chipset\n", __func__);
-               return -1;
        }
 
        return 0;
@@ -569,16 +564,17 @@
  * Try to set BBAR (BIOS Base Address Register), but read back the value in 
case
  * it didn't stick.
  */
-static void ich_set_bbar(int ich_generation, uint32_t min_addr)
+static void ich_set_bbar(uint32_t min_addr)
 {
        int bbar_off;
        switch (ich_generation) {
-       case 7:
+       case CHIPSET_ICH7:
                bbar_off = 0x50;
                break;
-       case 8:
+       case CHIPSET_ICH8:
                msg_perr("BBAR offset is unknown on ICH8!\n");
                return;
+       case CHIPSET_ICH9:
        default:                /* Future version might behave the same */
                bbar_off = ICH9_REG_BBAR;
                break;
@@ -943,15 +939,12 @@
                return SPI_INVALID_LENGTH;
        }
 
-       switch (spi_programmer->type) {
-       case SPI_CONTROLLER_VIA:
-       case SPI_CONTROLLER_ICH7:
+       switch (ich_generation) {
+       case CHIPSET_ICH7:
                return ich7_run_opcode(op, offset, datalength, data, maxlength);
-       case SPI_CONTROLLER_ICH9:
+       case CHIPSET_ICH8:
+       default:                /* Future version might behave the same */
                return ich9_run_opcode(op, offset, datalength, data);
-       default:
-               /* If we ever get here, something really weird happened */
-               return -1;
        }
 }
 
@@ -1022,19 +1015,11 @@
            opcode->spi_type == SPI_OPCODE_TYPE_WRITE_WITH_ADDRESS) {
                addr = (writearr[1] << 16) |
                    (writearr[2] << 8) | (writearr[3] << 0);
-               switch (spi_programmer->type) {
-               case SPI_CONTROLLER_ICH7:
-               case SPI_CONTROLLER_VIA:
-               case SPI_CONTROLLER_ICH9:
-                       if (addr < ichspi_bbar) {
-                               msg_perr("%s: Address 0x%06x below allowed "
-                                        "range 0x%06x-0xffffff\n", __func__,
-                                        addr, ichspi_bbar);
-                               return SPI_INVALID_ADDRESS;
-                       }
-                       break;
-               default:
-                       break;
+               if (addr < ichspi_bbar) {
+                       msg_perr("%s: Address 0x%06x below allowed "
+                                "range 0x%06x-0xffffff\n", __func__,
+                                addr, ichspi_bbar);
+                       return SPI_INVALID_ADDRESS;
                }
        }
 
@@ -1316,7 +1301,7 @@
 };
 
 int ich_init_spi(struct pci_dev *dev, uint32_t base, void *rcrb,
-                       int ich_generation)
+                enum ich_chipset ich_gen)
 {
        int i;
        uint8_t old, new;
@@ -1324,15 +1309,16 @@
        uint32_t tmp;
        int desc_valid = 0;
 
+       ich_generation = ich_gen;
+
        switch (ich_generation) {
-       case 7:
-               spibar_offset = 0x3020;
-               break;
-       case 8:
+       case CHIPSET_ICH_UNKNOWN:
+               return -1;
+       case CHIPSET_ICH7:
+       case CHIPSET_ICH8:
                spibar_offset = 0x3020;
                break;
-       case 9:
-       case 10:
+       case CHIPSET_ICH9:
        default:                /* Future version might behave the same */
                spibar_offset = 0x3800;
                break;
@@ -1345,7 +1331,7 @@
        ich_spibar = rcrb + spibar_offset;
 
        switch (ich_generation) {
-       case 7:
+       case CHIPSET_ICH7:
                msg_pdbg("0x00: 0x%04x     (SPIS)\n",
                             mmio_readw(ich_spibar + 0));
                msg_pdbg("0x02: 0x%04x     (SPIC)\n",
@@ -1381,13 +1367,11 @@
                        msg_pinfo("WARNING: SPI Configuration Lockdown 
activated.\n");
                        ichspi_lock = 1;
                }
-               ich_set_bbar(ich_generation, 0);
+               ich_set_bbar(0);
                register_spi_programmer(&spi_programmer_ich7);
                ich_init_opcodes();
                break;
-       case 8:
-       case 9:
-       case 10:
+       case CHIPSET_ICH8:
        default:                /* Future version might behave the same */
                tmp2 = mmio_readw(ich_spibar + ICH9_REG_HSFS);
                msg_pdbg("0x04: 0x%04x (HSFS)\n", tmp2);
@@ -1447,7 +1431,7 @@
                             mmio_readl(ich_spibar + ICH9_REG_OPMENU));
                msg_pdbg("0x9C: 0x%08x (OPMENU+4)\n",
                             mmio_readl(ich_spibar + ICH9_REG_OPMENU + 4));
-               if (ich_generation == 8) {
+               if (ich_generation == CHIPSET_ICH8) {
                        tmp = mmio_readl(ich_spibar + ICH8_REG_VSCC);
                        msg_pdbg("0xC1: 0x%08x (VSCC)\n", tmp);
                        msg_pdbg("VSCC: ");
@@ -1469,7 +1453,7 @@
 
                        tmp = mmio_readl(ich_spibar + ICH9_REG_FPB);
                        msg_pdbg("0xD0: 0x%08x (FPB)\n", tmp);
-                       ich_set_bbar(ich_generation, 0);
+                       ich_set_bbar(0);
                }
 
                msg_pdbg("\n");
@@ -1524,6 +1508,7 @@
 
        /* Not sure if it speaks all these bus protocols. */
        buses_supported = BUS_LPC | BUS_FWH;
+       ich_generation = CHIPSET_ICH7;
        register_spi_programmer(&spi_programmer_via);
 
        msg_pdbg("0x00: 0x%04x     (SPIS)\n", mmio_readw(ich_spibar + 0));
@@ -1556,7 +1541,7 @@
                ichspi_lock = 1;
        }
 
-       ich_set_bbar(7, 0);
+       ich_set_bbar(0);
        ich_init_opcodes();
 
        return 0;

Modified: trunk/programmer.h
==============================================================================
--- trunk/programmer.h  Fri Nov  4 22:35:26 2011        (r1459)
+++ trunk/programmer.h  Mon Nov  7 00:51:09 2011        (r1460)
@@ -579,9 +579,20 @@
 
 /* ichspi.c */
 #if CONFIG_INTERNAL == 1
+enum ich_chipset {
+       CHIPSET_ICH_UNKNOWN,
+       CHIPSET_ICH7 = 7,
+       CHIPSET_ICH8,
+       CHIPSET_ICH9,
+       CHIPSET_ICH10,
+       CHIPSET_5_SERIES_IBEX_PEAK,
+       CHIPSET_6_SERIES_COUGAR_POINT,
+       CHIPSET_7_SERIES_PANTHER_POINT
+};
+
 extern uint32_t ichspi_bbar;
 int ich_init_spi(struct pci_dev *dev, uint32_t base, void *rcrb,
-                   int ich_generation);
+                enum ich_chipset ich_generation);
 int via_init_spi(struct pci_dev *dev);
 
 /* it85spi.c */

_______________________________________________
flashrom mailing list
[email protected]
http://www.flashrom.org/mailman/listinfo/flashrom

Reply via email to