[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

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.

[Stefan Tauner]

On Fri, 22 Jul 2011 21:00:48 +0200
Carl-Daniel Hailfinger  wrote:
> 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
> 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

i did not check the facts, but it looks ok. maybe a concrete example
would be nice.
> 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;

fwh_idsel?
would increase the patch size though
> 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);

the "from to" message is a good idea imo, i am not sure if the verbosity
level is the right choice. most of the time parameters are just applied
without a message (but when there is an error). maybe msg_pdbg would be
better? OTOH if i specify something on the cli i would like to know if
it has changed anything or not (even in the default verbosity).
so i personally would leave it as it is, but i am not so sure what
carldani the output slayer would recommend ;)

apart from that, this does not compile on my machine:
error: format ‘%012llx’ expects type ‘long long unsigned int’, but argument 3 has type ‘uint64_t’

also the strtoul result is not checked for errors.
note from the man page:

Since strtoul() can legitimately return 0 or LONG_MAX (LLONG_MAX for
strtoull()) on both success and failure, the calling program should set
errno to  0  before  the call, and then determine if an error occurred
by checking whether errno has a nonzero value after the call.

you have also mentioned an idea to check for values >= (1<<48) this
could be combined to one check/message or be done in two (one
warning/info, one error).
> 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.

[Carl-Daniel Hailfinger]

Am 25.07.2011 01:34 schrieb Stefan Tauner:
> On Fri, 22 Jul 2011 21:00:48 +0200
> Carl-Daniel Hailfinger  wrote:
> > ===================================================================
> > --- flashrom-ich_fwh_idsel_programmer_param_docfix/flashrom.8	(Revision 1380)
> > +++ flashrom-ich_fwh_idsel_programmer_param_docfix/flashrom.8	(Arbeitskopie)
>
> i did not check the facts, but it looks ok. maybe a concrete example
> would be nice.

Done.
> > 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;
>
> fwh_idsel?
> would increase the patch size though

I created a separate fwh_idsel variable with narrower scope.
> > 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);
>
> the "from to" message is a good idea imo, i am not sure if the verbosity
> level is the right choice. most of the time parameters are just applied
> without a message (but when there is an error). maybe msg_pdbg would be
> better? OTOH if i specify something on the cli i would like to know if
> it has changed anything or not (even in the default verbosity).
> so i personally would leave it as it is, but i am not so sure what
> carldani the output slayer would recommend ;)

Downgraded to dbg, thanks for noticing this.
> apart from that, this does not compile on my machine:
> error: format ‘%012llx’ expects type ‘long long unsigned int’, but argument 3 has type ‘uint64_t’

Changed format specifier to PRIx64.
> also the strtoul result is not checked for errors.
> note from the man page:
>
> Since strtoul() can legitimately return 0 or LONG_MAX (LLONG_MAX for
> strtoull()) on both success and failure, the calling program should set
> errno to  0  before  the call, and then determine if an error occurred
> by checking whether errno has a nonzero value after the call.

Done.
> you have also mentioned an idea to check for values >= (1<<48) this
> could be combined to one check/message or be done in two (one
> warning/info, one error).

Both are errors now.

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

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,31 @@

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;
+		fwh_idsel = pci_read_long(dev, 0xd0);
+		fwh_idsel <<= 16;
+		fwh_idsel |= pci_read_word(dev, 0xd4);
+		msg_pdbg("\nSetting IDSEL from 0x%012" PRIx64 " ", 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;
+		}
+		msg_pdbg("to 0x%012llx for top 16 MB", 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.

[Stefan Tauner]

On Mon, 25 Jul 2011 08:48:37 +0200
Carl-Daniel Hailfinger  wrote:
> Signed-off-by: Carl-Daniel Hailfinger
> 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

having the example map something from the datasheet to the parameter is
excellent, thanks.
> 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,31 @@
>
> 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;
> fwh_idsel = pci_read_long(dev, 0xd0);
> fwh_idsel <<= 16;
> fwh_idsel |= pci_read_word(dev, 0xd4);
> msg_pdbg("\nSetting IDSEL from 0x%012" PRIx64 " ", 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;
> }
> msg_pdbg("to 0x%012llx for top 16 MB", fwh_idsel);

should use PRIx64 here too.
> 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.

the output in the case of an error is now:
Setting IDSEL from 0x000000004567 Error: fwh_idsel= specified, but value could not be converted.

using fwh_idsel_new, fwh_idsel_old and deferred output would fix
that.

[Carl-Daniel Hailfinger]

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

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.

[Carl-Daniel Hailfinger]

Am 26.07.2011 00:25 schrieb 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

Thanks for the Ack on IRC, I added the "." to the end of the message as
requested.
Committed in r1389.

Regards,
Carl-Daniel