[flashrom] [PATCH 2/2] Error out when chipset/board doesn't decode full ROM

Uwe Hermann uwe at hermann-uwe.de
Thu Dec 17 10:52:23 CET 2009


On Fri, Oct 30, 2009 at 05:24:40AM +0100, Carl-Daniel Hailfinger wrote:
> Use the maximum decode size infrastructure.
> - Detect max FWH size for Intel
> 631xESB/632xESB/3100/ICH6/ICH7/ICH8/ICH9/ICH10.
> - Move IDSEL override before decode size checking for the chipsets
> listed above or flashrom will complain based on old values.
> - Adjust supported flash buses for the chipsets listed above (none of
> them supports LPC or Parallel).
> - Detect max parallel size for AMD/National Semiconductor CS5530.

Btw, this also applies CS5530A (which is mostly compatible to the CS5530).
I checked the CS5530A datasheet, your code works for both.


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

Acked-by: Uwe Hermann <uwe at hermann-uwe.de>


> @@ -215,48 +216,92 @@
>  	uint32_t fwh_conf;
>  	int i;
>  	char *idsel = NULL;
> +	int tmp;
> +	int max_decode_fwh_idsel = 0;
> +	int max_decode_fwh_decode = 0;
> +	int contiguous = 1;
>  
> -	/* Ignore all legacy ranges below 1 MB. */
> +	if (programmer_param)
> +		idsel = strstr(programmer_param, "fwh_idsel=");

> +
> +	if (idsel) {
> +		idsel += strlen("fwh_idsel=");
> +		fwh_conf = (uint32_t)strtoul(idsel, NULL, 0);

We should probably check the return value of strtoul() and handle errors
(this can be an extra follow-up patch).


> @@ -547,6 +598,24 @@
>  	reg8 |= BIOS_ROM_POSITIVE_DECODE;
>  	pci_write_byte(dev, DECODE_CONTROL_REG2, reg8);
>  
> +	reg8 = pci_read_byte(dev, CS5530_RESET_CONTROL_REG);
> +	if (reg8 & CS5530_ISA_MASTER) {
> +		/* We have A0-A23 available. */
> +		max_rom_decode.parallel = 16 * 1024 * 1024;
> +	} else {
> +		reg8 = pci_read_byte(dev, CS5530_USB_SHADOW_REG);
> +		if (reg8 & CS5530_ENABLE_SA2320) {
> +			/* We have A0-19, A20-A23 available. */
> +			max_rom_decode.parallel = 16 * 1024 * 1024;
> +		} else if (reg8 & CS5530_ENABLE_SA20) {
> +			/* We have A0-19, A20 available. */
> +			max_rom_decode.parallel = 2 * 1024 * 1024;
> +		} else {
> +			/* A20 and above are not active. */
> +			max_rom_decode.parallel = 1024 * 1024;

I think we should add a print after each of the cases, so that the user
can see how much this board can decode. IMHO we should just _always_
print that info for all chipsets/boards (if known), at least in -V.
In the CS5530 case it might also be interesting _why_ 16MB can be decoded
(as there are two cases which result in 16MB), so a print would be nice.

This is material for an extra patch, though.

On my test-board (GX1) the value was 16MB due to CS5530_ENABLE_SA2320, IIRC.


> @@ -1079,7 +1087,7 @@
>  	{0x10DE, 0x0030, 0x1043, 0x818a,  0x8086, 0x100E, 0x1043, 0x80EE, NULL,         NULL,          "ASUS",        "P5ND2-SLI Deluxe",   board_asus_p5nd2_sli},
>  	{0x1106, 0x3149, 0x1565, 0x3206,  0x1106, 0x3344, 0x1565, 0x1202, NULL,         NULL,          "Biostar",     "P4M80-M4",           it8705_rom_write_enable},
>  	{0x8086, 0x3590, 0x1028, 0x016c,  0x1000, 0x0030, 0x1028, 0x016c, NULL,         NULL,          "Dell",        "PowerEdge 1850",     ich5_gpio23_raise},
> -	{0x1106, 0x3038, 0x1019, 0x0996,  0x1106, 0x3177, 0x1019, 0x0996, NULL,         NULL,          "Elitegroup",  "K7VTA3",             it8705f_write_enable_2e},
> +	{0x1106, 0x3038, 0x1019, 0x0996,  0x1106, 0x3177, 0x1019, 0x0996, NULL,         NULL,          "Elitegroup",  "K7VTA3",             elitegroup_k7vta3},
>  	{0x1106, 0x3177, 0x1106, 0x3177,  0x1106, 0x3059, 0x1695, 0x3005, NULL,         NULL,          "EPoX",        "EP-8K5A2",           board_epox_ep_8k5a2},
>  	{0x10EC, 0x8139, 0x1695, 0x9001,  0x11C1, 0x5811, 0x1695, 0x9015, NULL,         NULL,          "EPoX",        "EP-8RDA3+",          board_epox_ep_8rda3plus},
>  	{0x8086, 0x7110,      0,      0,  0x8086, 0x7190,      0,      0, "epox",       "ep-bx3",      "EPoX",        "EP-BX3",             board_epox_ep_bx3},
> @@ -1105,7 +1113,7 @@
>  	{0x1106, 0x0571, 0x1462, 0x7120,       0,      0,      0,      0, "msi",        "kt4v",        "MSI",         "MS-6712 (KT4V)",     board_msi_kt4v},
>  	{0x8086, 0x2658, 0x1462, 0x7046,  0x1106, 0x3044, 0x1462, 0x046d, NULL,         NULL,          "MSI",         "MS-7046",            ich6_gpio19_raise},
>  	{0x10de, 0x005e,      0,      0,       0,      0,      0,      0, "msi",        "k8n-neo3",    "MSI",         "MS-7135 (K8N Neo3)", w83627thf_gpio4_4_raise_4e},
> -	{0x1106, 0x3104, 0x1297, 0xa238,  0x1106, 0x3059, 0x1297, 0xc063, NULL,         NULL,          "Shuttle",     "AK38N",              it8705f_write_enable_2e},
> +	{0x1106, 0x3104, 0x1297, 0xa238,  0x1106, 0x3059, 0x1297, 0xc063, NULL,         NULL,          "Shuttle",     "AK38N",              shuttle_ak38n},
>  	{0x10DE, 0x0050, 0x1297, 0x5036,  0x1412, 0x1724, 0x1297, 0x5036, NULL,         NULL,          "Shuttle",     "FN25",               board_shuttle_fn25},
>  	{0x1106, 0x3038, 0x0925, 0x1234,  0x1106, 0x3058, 0x15DD, 0x7609, NULL,         NULL,          "Soyo",        "SY-7VCA",            board_soyo_sy_7vca},
>  	{0x8086, 0x1076, 0x8086, 0x1176,  0x1106, 0x3059, 0x10f1, 0x2498, NULL,         NULL,          "Tyan",        "S2498 (Tomcat K7M)", board_asus_a7v8x_mx},

These two won't apply anymore, but it's easily fixable.


Uwe.
-- 
http://www.hermann-uwe.de  | http://www.randomprojects.org
http://www.crazy-hacks.org | http://www.unmaintained-free-software.org




More information about the flashrom mailing list