Thanks for the review. Next try.
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@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,23 @@ .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. The default value for ICH7 is given in the example below. +.sp +Example: +.B "flashrom -p internal:fwh_idsel=0x001122334567" +.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) @@ -30,6 +30,8 @@ #include <stdlib.h> #include <string.h> #include <unistd.h> +#include <inttypes.h> +#include <errno.h> #include "flash.h" #include "programmer.h"
@@ -311,15 +313,33 @@
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); + uint64_t fwh_idsel_old; + uint64_t fwh_idsel; + errno = 0; + /* Base 16, nothing else makes sense. */ + fwh_idsel = (uint64_t)strtoull(idsel, NULL, 16); + if (errno) { + msg_perr("Error: fwh_idsel= specified, but value could " + "not be converted.\n"); + goto idsel_garbage_out; + } + if (fwh_idsel & 0xffff000000000000ULL) { + msg_perr("Error: fwh_idsel= specified, but value had " + "unusued bits set.\n"); + goto idsel_garbage_out; + } + fwh_idsel_old = pci_read_long(dev, 0xd0); + fwh_idsel_old <<= 16; + fwh_idsel_old |= pci_read_word(dev, 0xd4); + 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); /* 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 value given.\n"); +idsel_garbage_out: free(idsel); /* FIXME: Return failure here once internal_init() starts * to care about the return value of the chipset enable.