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
ICH3/4/5 also have FWH_SEL1/2 registers but at different addresses.
While FWH_DEC_EN1 is a 16bit register for ICH6, it was two separate 8bit registers on ICH4/5. So implement it with two byte reads instead.
Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com --- chipset_enable.c | 36 ++++++++++++++++++++++++++++-------- 1 file changed, 28 insertions(+), 8 deletions(-)
diff --git a/chipset_enable.c b/chipset_enable.c index 1e2df21..b17b295 100644 --- a/chipset_enable.c +++ b/chipset_enable.c @@ -325,14 +325,21 @@ static int enable_flash_ich_4e(struct pci_dev *dev, const char *name) return enable_flash_ich(dev, name, 0x4e); }
-static int enable_flash_ich_dc(struct pci_dev *dev, const char *name) +static int enable_flash_ich_fwh_decode(struct pci_dev *dev, const char *name) { uint32_t fwh_conf; + uint8_t fwh_sel1, fwh_sel2, fwh_dec_en_lo, fwh_dec_en_hi; int i, tmp; char *idsel = NULL; int max_decode_fwh_idsel = 0, max_decode_fwh_decode = 0; int contiguous = 1;
+ /* Register map from ICH6 onwards. */ + fwh_sel1 = 0xd0; + fwh_sel2 = 0xd4; + fwh_dec_en_lo = 0xd8; + fwh_dec_en_hi = 0xd9; + idsel = extract_programmer_param("fwh_idsel"); if (idsel && strlen(idsel)) { uint64_t fwh_idsel_old, fwh_idsel; @@ -349,14 +356,14 @@ static int enable_flash_ich_dc(struct pci_dev *dev, const char *name) "unused bits set.\n"); goto idsel_garbage_out; } - fwh_idsel_old = pci_read_long(dev, 0xd0); + fwh_idsel_old = pci_read_long(dev, fwh_sel1); fwh_idsel_old <<= 16; - fwh_idsel_old |= pci_read_word(dev, 0xd4); + fwh_idsel_old |= pci_read_word(dev, fwh_sel2); msg_pdbg("\nSetting IDSEL from 0x%012" PRIx64 " to " "0x%012" PRIx64 " for top 16 MB.", fwh_idsel_old, fwh_idsel); - rpci_write_long(dev, 0xd0, (fwh_idsel >> 16) & 0xffffffff); - rpci_write_word(dev, 0xd4, fwh_idsel & 0xffff); + rpci_write_long(dev, fwh_sel1, (fwh_idsel >> 16) & 0xffffffff); + rpci_write_word(dev, fwh_sel2, fwh_idsel & 0xffff); /* FIXME: Decode settings are not changed. */ } else if (idsel) { msg_perr("Error: fwh_idsel= specified, but no value given.\n"); @@ -372,7 +379,7 @@ idsel_garbage_out: * have to be adjusted. */ /* FWH_SEL1 */ - fwh_conf = pci_read_long(dev, 0xd0); + fwh_conf = pci_read_long(dev, fwh_sel1); for (i = 7; i >= 0; i--) { tmp = (fwh_conf >> (i * 4)) & 0xf; msg_pdbg("\n0x%08x/0x%08x FWH IDSEL: 0x%x", @@ -386,7 +393,7 @@ idsel_garbage_out: } } /* FWH_SEL2 */ - fwh_conf = pci_read_word(dev, 0xd4); + fwh_conf = pci_read_word(dev, fwh_sel2); for (i = 3; i >= 0; i--) { tmp = (fwh_conf >> (i * 4)) & 0xf; msg_pdbg("\n0x%08x/0x%08x FWH IDSEL: 0x%x", @@ -401,7 +408,9 @@ idsel_garbage_out: } contiguous = 1; /* FWH_DEC_EN1 */ - fwh_conf = pci_read_word(dev, 0xd8); + fwh_conf = pci_read_byte(dev, fwh_dec_en_hi); + fwh_conf <<= 8; + fwh_conf |= pci_read_byte(dev, fwh_dec_en_lo); for (i = 7; i >= 0; i--) { tmp = (fwh_conf >> (i + 0x8)) & 0x1; msg_pdbg("\n0x%08x/0x%08x FWH decode %sabled", @@ -429,6 +438,17 @@ idsel_garbage_out: max_rom_decode.fwh = min(max_decode_fwh_idsel, max_decode_fwh_decode); msg_pdbg("\nMaximum FWH chip size: 0x%x bytes", max_rom_decode.fwh);
+ return 0; +} + +static int enable_flash_ich_dc(struct pci_dev *dev, const char *name) +{ + int err; + + /* Configure FWH IDSEL decoder maps. */ + if ((err = enable_flash_ich_fwh_decode(dev, name)) != 0) + return err; + /* If we're called by enable_flash_ich_dc_spi, it will override * internal_buses_supported anyway. */
On Sat, 23 Feb 2013 23:11:32 +0200 Kyösti Mälkki kyosti.malkki@gmail.com wrote:
ICH3/4/5 also have FWH_SEL1/2 registers but at different addresses.
While FWH_DEC_EN1 is a 16bit register for ICH6, it was two separate 8bit registers on ICH4/5. So implement it with two byte reads instead.
Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com
Acked-by: Stefan Tauner stefan.tauner@student.tuwien.ac.at and committed in r1746.
Follow the style used from ICH7 onwards to pass ich_generation parameter to lower-level functions on older ICH chipsets too.
Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com --- chipset_enable.c | 74 ++++++++++++++++++++++++++++++++++++++------------------ programmer.h | 6 ++++- 2 files changed, 56 insertions(+), 24 deletions(-)
diff --git a/chipset_enable.c b/chipset_enable.c index b17b295..d1e4658 100644 --- a/chipset_enable.c +++ b/chipset_enable.c @@ -314,13 +314,8 @@ static int enable_flash_ich(struct pci_dev *dev, const char *name, uint8_t bios_ return 0; }
-static int enable_flash_ich_4e(struct pci_dev *dev, const char *name) +static int enable_flash_ich0(struct pci_dev *dev, const char *name) { - /* - * Note: ICH5 has registers similar to FWH_SEL1, FWH_SEL2 and - * FWH_DEC_EN1, but they are called FB_SEL1, FB_SEL2, FB_DEC_EN1 and - * FB_DEC_EN2. - */ internal_buses_supported = BUS_FWH; return enable_flash_ich(dev, name, 0x4e); } @@ -441,7 +436,35 @@ idsel_garbage_out: return 0; }
-static int enable_flash_ich_dc(struct pci_dev *dev, const char *name) +static int enable_flash_ich_4e(struct pci_dev *dev, const char *name, + enum ich_chipset ich_generation) +{ + /* + * Note: ICH5 has registers similar to FWH_SEL1, FWH_SEL2 and + * FWH_DEC_EN1, but they are called FB_SEL1, FB_SEL2, FB_DEC_EN1 and + * FB_DEC_EN2. + */ + internal_buses_supported = BUS_FWH; + return enable_flash_ich(dev, name, 0x4e); +} + +static int enable_flash_ich3(struct pci_dev *dev, const char *name) +{ + return enable_flash_ich_4e(dev, name, CHIPSET_ICH3); +} + +static int enable_flash_ich4(struct pci_dev *dev, const char *name) +{ + return enable_flash_ich_4e(dev, name, CHIPSET_ICH4); +} + +static int enable_flash_ich5(struct pci_dev *dev, const char *name) +{ + return enable_flash_ich_4e(dev, name, CHIPSET_ICH5); +} + +static int enable_flash_ich_dc(struct pci_dev *dev, const char *name, + enum ich_chipset ich_generation) { int err;
@@ -456,6 +479,11 @@ static int enable_flash_ich_dc(struct pci_dev *dev, const char *name) return enable_flash_ich(dev, name, 0xdc); }
+static int enable_flash_ich6(struct pci_dev *dev, const char *name) +{ + return enable_flash_ich_dc(dev, name, CHIPSET_ICH6); +} + static int enable_flash_poulsbo(struct pci_dev *dev, const char *name) { uint16_t old, new; @@ -562,7 +590,7 @@ static int enable_flash_ich_dc_spi(struct pci_dev *dev, const char *name, }
/* Enable Flash Writes */ - ret = enable_flash_ich_dc(dev, name); + ret = enable_flash_ich_dc(dev, name, ich_generation); if (ret == ERROR_FATAL) return ret;
@@ -1426,21 +1454,21 @@ const struct penable chipset_enables[] = { {0x8086, 0x1e5e, NT, "Intel", "HM70", enable_flash_pch7}, {0x8086, 0x1e5f, NT, "Intel", "NM70", enable_flash_pch7}, {0x8086, 0x2310, NT, "Intel", "DH89xxCC", enable_flash_pch7}, - {0x8086, 0x2410, OK, "Intel", "ICH", enable_flash_ich_4e}, - {0x8086, 0x2420, OK, "Intel", "ICH0", enable_flash_ich_4e}, - {0x8086, 0x2440, OK, "Intel", "ICH2", enable_flash_ich_4e}, - {0x8086, 0x244c, OK, "Intel", "ICH2-M", enable_flash_ich_4e}, - {0x8086, 0x2450, NT, "Intel", "C-ICH", enable_flash_ich_4e}, - {0x8086, 0x2480, OK, "Intel", "ICH3-S", enable_flash_ich_4e}, - {0x8086, 0x248c, OK, "Intel", "ICH3-M", enable_flash_ich_4e}, - {0x8086, 0x24c0, OK, "Intel", "ICH4/ICH4-L", enable_flash_ich_4e}, - {0x8086, 0x24cc, OK, "Intel", "ICH4-M", enable_flash_ich_4e}, - {0x8086, 0x24d0, OK, "Intel", "ICH5/ICH5R", enable_flash_ich_4e}, - {0x8086, 0x25a1, OK, "Intel", "6300ESB", enable_flash_ich_4e}, - {0x8086, 0x2640, OK, "Intel", "ICH6/ICH6R", enable_flash_ich_dc}, - {0x8086, 0x2641, OK, "Intel", "ICH6-M", enable_flash_ich_dc}, - {0x8086, 0x2642, NT, "Intel", "ICH6W/ICH6RW", enable_flash_ich_dc}, - {0x8086, 0x2670, OK, "Intel", "631xESB/632xESB/3100", enable_flash_ich_dc}, + {0x8086, 0x2410, OK, "Intel", "ICH", enable_flash_ich0}, + {0x8086, 0x2420, OK, "Intel", "ICH0", enable_flash_ich0}, + {0x8086, 0x2440, OK, "Intel", "ICH2", enable_flash_ich0}, + {0x8086, 0x244c, OK, "Intel", "ICH2-M", enable_flash_ich0}, + {0x8086, 0x2450, NT, "Intel", "C-ICH", enable_flash_ich0}, + {0x8086, 0x2480, OK, "Intel", "ICH3-S", enable_flash_ich3}, + {0x8086, 0x248c, OK, "Intel", "ICH3-M", enable_flash_ich3}, + {0x8086, 0x24c0, OK, "Intel", "ICH4/ICH4-L", enable_flash_ich4}, + {0x8086, 0x24cc, OK, "Intel", "ICH4-M", enable_flash_ich4}, + {0x8086, 0x24d0, OK, "Intel", "ICH5/ICH5R", enable_flash_ich5}, + {0x8086, 0x25a1, OK, "Intel", "6300ESB", enable_flash_ich5}, + {0x8086, 0x2640, OK, "Intel", "ICH6/ICH6R", enable_flash_ich6}, + {0x8086, 0x2641, OK, "Intel", "ICH6-M", enable_flash_ich6}, + {0x8086, 0x2642, NT, "Intel", "ICH6W/ICH6RW", enable_flash_ich6}, + {0x8086, 0x2670, OK, "Intel", "631xESB/632xESB/3100", enable_flash_ich6}, {0x8086, 0x27b0, OK, "Intel", "ICH7DH", enable_flash_ich7}, {0x8086, 0x27b8, OK, "Intel", "ICH7/ICH7R", enable_flash_ich7}, {0x8086, 0x27b9, OK, "Intel", "ICH7M", enable_flash_ich7}, diff --git a/programmer.h b/programmer.h index 51a8c80..ea7d456 100644 --- a/programmer.h +++ b/programmer.h @@ -540,7 +540,11 @@ int register_spi_programmer(const struct spi_programmer *programmer); /* The following enum is needed by ich_descriptor_tool and ich* code. */ enum ich_chipset { CHIPSET_ICH_UNKNOWN, - CHIPSET_ICH7 = 7, + CHIPSET_ICH3 = 3, + CHIPSET_ICH4, + CHIPSET_ICH5, + CHIPSET_ICH6, + CHIPSET_ICH7, CHIPSET_ICH8, CHIPSET_ICH9, CHIPSET_ICH10,
On Sat, 23 Feb 2013 23:12:32 +0200 Kyösti Mälkki kyosti.malkki@gmail.com wrote:
Follow the style used from ICH7 onwards to pass ich_generation parameter to lower-level functions on older ICH chipsets too.
Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com
Acked-by: Stefan Tauner stefan.tauner@student.tuwien.ac.at and committed in r1747.
Register locations are different from ICH6, but otherwise appear to have identical bit specifications and defaults.
Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com --- chipset_enable.c | 35 +++++++++++++++++++++++------------ 1 file changed, 23 insertions(+), 12 deletions(-)
diff --git a/chipset_enable.c b/chipset_enable.c index d1e4658..273de81 100644 --- a/chipset_enable.c +++ b/chipset_enable.c @@ -320,7 +320,8 @@ static int enable_flash_ich0(struct pci_dev *dev, const char *name) return enable_flash_ich(dev, name, 0x4e); }
-static int enable_flash_ich_fwh_decode(struct pci_dev *dev, const char *name) +static int enable_flash_ich_fwh_decode(struct pci_dev *dev, const char *name, + enum ich_chipset ich_generation) { uint32_t fwh_conf; uint8_t fwh_sel1, fwh_sel2, fwh_dec_en_lo, fwh_dec_en_hi; @@ -329,11 +330,20 @@ static int enable_flash_ich_fwh_decode(struct pci_dev *dev, const char *name) int max_decode_fwh_idsel = 0, max_decode_fwh_decode = 0; int contiguous = 1;
- /* Register map from ICH6 onwards. */ - fwh_sel1 = 0xd0; - fwh_sel2 = 0xd4; - fwh_dec_en_lo = 0xd8; - fwh_dec_en_hi = 0xd9; + if (ich_generation >= CHIPSET_ICH6) { + fwh_sel1 = 0xd0; + fwh_sel2 = 0xd4; + fwh_dec_en_lo = 0xd8; + fwh_dec_en_hi = 0xd9; + } else if (ich_generation >= CHIPSET_ICH3) { + fwh_sel1 = 0xe8; + fwh_sel2 = 0xee; + fwh_dec_en_lo = 0xf0; + fwh_dec_en_hi = 0xe3; + } else { + msg_perr("Error: FWH decode setting not implemented.\n"); + return ERROR_FATAL; + }
idsel = extract_programmer_param("fwh_idsel"); if (idsel && strlen(idsel)) { @@ -439,11 +449,12 @@ idsel_garbage_out: static int enable_flash_ich_4e(struct pci_dev *dev, const char *name, enum ich_chipset ich_generation) { - /* - * Note: ICH5 has registers similar to FWH_SEL1, FWH_SEL2 and - * FWH_DEC_EN1, but they are called FB_SEL1, FB_SEL2, FB_DEC_EN1 and - * FB_DEC_EN2. - */ + int err; + + /* Configure FWH IDSEL decoder maps. */ + if ((err = enable_flash_ich_fwh_decode(dev, name, ich_generation)) != 0) + return err; + internal_buses_supported = BUS_FWH; return enable_flash_ich(dev, name, 0x4e); } @@ -469,7 +480,7 @@ static int enable_flash_ich_dc(struct pci_dev *dev, const char *name, int err;
/* Configure FWH IDSEL decoder maps. */ - if ((err = enable_flash_ich_fwh_decode(dev, name)) != 0) + if ((err = enable_flash_ich_fwh_decode(dev, name, ich_generation)) != 0) return err;
/* If we're called by enable_flash_ich_dc_spi, it will override
On Sat, 23 Feb 2013 23:12:59 +0200 Kyösti Mälkki kyosti.malkki@gmail.com wrote:
Register locations are different from ICH6, but otherwise appear to have identical bit specifications and defaults.
Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com
Acked-by: Stefan Tauner stefan.tauner@student.tuwien.ac.at and committed in r1748.
On Sat, 23 Feb 2013 23:06:31 +0200 Kyösti Mälkki kyosti.malkki@gmail.com wrote:
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.
Thanks for your patches. Since Carl-Daniel has been pulling a Carl-Daniel on these again, I merged them today.
I took a look at the respective datasheets and noticed that ICH2 (and its derivative C-ICH) do also feature those registers. I have changed your patches accordingly before merging them. Even ICH and ICH0 do have similar registers but only for the "first" 4MB, which I have noted in a fixme (only).
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
Patches welcome, and they would not bitrot over half a year this time, I promise. ;)
-
Kyösti Mälkki -
Stefan Tauner