[flashrom] [PATCH] Fix ICH FWH IDSEL setting

[Carl-Daniel Hailfinger]

Fix ICH FWH IDSEL setting with the fwh_idsel= internal programmer parameter.
The code took 32 bits of input and wrote them to an 48 bit register,
duplicating some values.
Document the fwh_idsel= parameter in the man page.

Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net>

Index: flashrom-ich_fwh_idsel_programmer_param_docfix/flashrom.8
===================================================================
--- flashrom-ich_fwh_idsel_programmer_param_docfix/flashrom.8	(Revision 1380)
+++ flashrom-ich_fwh_idsel_programmer_param_docfix/flashrom.8	(Arbeitskopie)
@@ -298,6 +298,20 @@
 .B it87spi
 programmer.
 .sp
+If you have an Intel chipset with an ICH6 or later southbridge and if you want
+to set specific IDSEL values for a non-default flash chip or an embedded
+controller (EC), you can use the
+.sp
+.B "  flashrom \-p internal:fwh_idsel=value"
+.sp
+syntax where value is the 48-bit hexadecimal raw value to be written in the
+IDSEL registers of the Intel southbridge. The upper 32 bits use one hex digit
+each per 512 kB range between 0xffc00000 and 0xffffffff, and the lower 16 bits
+use one hex digit each per 1024 kB range between 0xff400000 and 0xff7fffff.
+The rightmost hex digit corresponds with the lowest address range. All address
+ranges have a corresponding sister range 4 MB below with identical IDSEL
+settings.
+.sp
 Using flashrom on laptops is dangerous and may easily make your hardware
 unusable (see also the
 .B BUGS
Index: flashrom-ich_fwh_idsel_programmer_param_docfix/chipset_enable.c
===================================================================
--- flashrom-ich_fwh_idsel_programmer_param_docfix/chipset_enable.c	(Revision 1380)
+++ flashrom-ich_fwh_idsel_programmer_param_docfix/chipset_enable.c	(Arbeitskopie)
@@ -301,7 +301,7 @@
 
 static int enable_flash_ich_dc(struct pci_dev *dev, const char *name)
 {
-	uint32_t fwh_conf;
+	uint64_t fwh_conf;
 	int i;
 	char *idsel = NULL;
 	int tmp;
@@ -311,15 +311,18 @@
 
 	idsel = extract_programmer_param("fwh_idsel");
 	if (idsel && strlen(idsel)) {
-		fwh_conf = (uint32_t)strtoul(idsel, NULL, 0);
-
-		/* FIXME: Need to undo this on shutdown. */
-		msg_pinfo("\nSetting IDSEL=0x%x for top 16 MB", fwh_conf);
-		rpci_write_long(dev, 0xd0, fwh_conf);
-		rpci_write_word(dev, 0xd4, fwh_conf);
+		fwh_conf = pci_read_long(dev, 0xd0);
+		fwh_conf <<= 16;
+		fwh_conf |= pci_read_word(dev, 0xd4);
+		msg_pinfo("\nSetting IDSEL from 0x%012llx ", fwh_conf);
+		/* Base 16, nothing else makes sense. */
+		fwh_conf = (uint64_t)strtoull(idsel, NULL, 16);
+		msg_pinfo("to 0x%012llx for top 16 MB", fwh_conf);
+		rpci_write_long(dev, 0xd0, (fwh_conf >> 16) & 0xffffffff);
+		rpci_write_word(dev, 0xd4, fwh_conf & 0xffff);
 		/* FIXME: Decode settings are not changed. */
 	} else if (idsel) {
-		msg_perr("Error: idsel= specified, but no number given.\n");
+		msg_perr("Error: fwh_idsel= specified, but no number given.\n");
 		free(idsel);
 		/* FIXME: Return failure here once internal_init() starts
 		 * to care about the return value of the chipset enable.


-- 
http://www.hailfinger.org/