Refactor MCP SPI detection: - Set supported buses based on ISA bridge reg 0x8a - Use MCP55 chipset enable only if LPC is detected - Allow LPC on MCP61 - Eliminate duplicated code where possible
Signed-off-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net
Index: flashrom-mcp_spi_detect_refactor/chipset_enable.c =================================================================== --- flashrom-mcp_spi_detect_refactor/chipset_enable.c (Revision 905) +++ flashrom-mcp_spi_detect_refactor/chipset_enable.c (Arbeitskopie) @@ -1054,30 +1054,39 @@ return 0; }
-/** - * The MCP67 code is guesswork based on cleanroom reverse engineering. - * Due to that, it only reads info and doesn't change any settings. - * It is assumed that LPC chips need the MCP55 code and SPI chips need the - * code provided in this function. Until we know for sure, call - * enable_flash_mcp55 from this function. Warning: enable_flash_mcp55 - * might make SPI flash inaccessible. The same caveat applies to SPI init - * for LPC flash. +/* This is a shot in the dark. Even if the code is totally bogus for some + * chipsets, users will at least start to send in reports. */ -static int enable_flash_mcp67(struct pci_dev *dev, const char *name) +static int enable_flash_mcp6x_7x_common(struct pci_dev *dev, const char *name) { - int result = 0; uint8_t byte; uint16_t status; + char *busname; uint32_t mcp_spibaraddr; void *mcp_spibar; struct pci_dev *smbusdev;
+ msg_pinfo("This chipset is not really supported yet. Guesswork...\n"); + /* dev is the ISA bridge. No idea what the stuff below does. */ byte = pci_read_byte(dev, 0x8a); - msg_pdbg("ISA bridge reg 0x8a contents: 0x%02x, bit 6 is %i, bit 5 is " - "%i\n", byte, (byte >> 6) & 0x1, (byte >> 5) & 0x1); - msg_pdbg("Guessed flash bus type is %s\n", ((byte >> 5) & 0x3) == 0x2 ? - "SPI" : "unknown, probably LPC"); + msg_pdbg("ISA/LPC bridge reg 0x8a contents: 0x%02x, bit 6 is %i, bit 5 " + "is %i\n", byte, (byte >> 6) & 0x1, (byte >> 5) & 0x1); + switch ((byte >> 5) & 0x3) { + case 0x0: + buses_supported = CHIP_BUSTYPE_LPC; + break; + case 0x2: + buses_supported = CHIP_BUSTYPE_SPI; + break; + default: + buses_supported = CHIP_BUSTYPE_UNKNOWN; + break; + } + busname = flashbuses_to_text(buses_supported); + msg_pdbg("Guessed flash bus type is %s\n", busname); + free(busname); + /* Disable the write code for now until we have more info. */ #if 0 byte |= (1 << 6); @@ -1088,8 +1097,9 @@ /* Look for the SMBus device (SMBus PCI class) */ smbusdev = pci_dev_find_vendorclass(0x10de, 0x0c05); if (!smbusdev) { - msg_perr("ERROR: SMBus device not found. Aborting.\n"); - exit(1); + msg_perr("ERROR: SMBus device not found.\n"); + buses_supported = CHIP_BUSTYPE_NONE; + return 1; } msg_pdbg("Found SMBus device %04x:%04x at %02x:%02x:%01x\n", smbusdev->vendor_id, smbusdev->device_id, @@ -1108,7 +1118,7 @@ msg_pdbg("after clearing low bits BAR is at 0x%08x\n", mcp_spibaraddr);
/* Accessing a NULL pointer BAR is evil. Don't do it. */ - if (mcp_spibaraddr) { + if (mcp_spibaraddr && (buses_supported == CHIP_BUSTYPE_SPI)) { /* Map the BAR. Bytewise/wordwise access at 0x530 and 0x540. */ mcp_spibar = physmap("MCP67 SPI", mcp_spibaraddr, 0x544);
@@ -1125,54 +1135,81 @@ status, status & 0x1, (status >> 8) & 0x1); /* FIXME: Remove the physunmap once the SPI driver exists. */ physunmap(mcp_spibar, 0x544); + } else if (!mcp_spibaraddr && (buses_supported & CHIP_BUSTYPE_SPI)) { + msg_pdbg("Strange. MCP SPI BAR is invalid.\n"); + buses_supported &= ~CHIP_BUSTYPE_SPI; + } else if (mcp_spibaraddr && !(buses_supported & CHIP_BUSTYPE_SPI)) { + msg_pdbg("Strange. MCP SPI BAR is valid, but chipset apparently" + " doesn't support SPI.\n"); } else { - msg_pdbg("Strange. MCP67 SPI BAR is invalid.\n"); + msg_pdbg("MCP SPI not used.\n"); } msg_pinfo("Please send the output of "flashrom -V" to " "flashrom@flashrom.org to help us finish support for your " "chipset. Thanks.\n");
- /* Not sure if this is still correct. No docs as usual. */ - result = enable_flash_mcp55(dev, name); + return 0; +}
+/** + * The MCP67 code is guesswork based on cleanroom reverse engineering. + * Due to that, it only reads info and doesn't change any settings. + * It is assumed that LPC chips need the MCP55 code and SPI chips need the + * code provided in this function. Until we know for sure, call + * enable_flash_mcp55 from this function. Warning: enable_flash_mcp55 + * might make SPI flash inaccessible. The same caveat applies to SPI init + * for LPC flash. + */ +static int enable_flash_mcp67(struct pci_dev *dev, const char *name) +{ + int result = 0; + + result = enable_flash_mcp6x_7x_common(dev, name); + if (result) + return result; + + /* Not sure if this is correct. No docs as usual. */ + switch (buses_supported) { + case CHIP_BUSTYPE_LPC: + result = enable_flash_mcp55(dev, name); + break; + case CHIP_BUSTYPE_SPI: + msg_pinfo("SPI on this chipset is not supported yet.\n"); + buses_supported = CHIP_BUSTYPE_NONE; + break; + default: + msg_pinfo("Something went wrong with bus type detection.\n"); + buses_supported = CHIP_BUSTYPE_NONE; + break; + } + return result; }
-/* This is a shot in the dark. Even if the code is totally bogus for some - * chipsets, users will at least start to send in reports. - */ static int enable_flash_mcp7x(struct pci_dev *dev, const char *name) { - uint8_t byte; - uint32_t mcp_spibaraddr; - struct pci_dev *smbusdev; + int result = 0;
- msg_pinfo("This chipset is not really supported yet. Guesswork...\n"); + result = enable_flash_mcp6x_7x_common(dev, name); + if (result) + return result;
- /* dev is the ISA bridge. No idea what the stuff below does. */ - byte = pci_read_byte(dev, 0x8a); - msg_pdbg("ISA/LPC bridge reg 0x8a contents: 0x%02x, bit 6 is %i, bit 5 " - "is %i\n", byte, (byte >> 6) & 0x1, (byte >> 5) & 0x1); - - /* Look for the SMBus device (SMBus PCI class) */ - smbusdev = pci_dev_find_vendorclass(0x10de, 0x0c05); - if (!smbusdev) { - msg_perr("ERROR: SMBus device not found. Aborting.\n"); - exit(1); + /* Not sure if this is correct. No docs as usual. */ + switch (buses_supported) { + case CHIP_BUSTYPE_LPC: + msg_pinfo("LPC on this chipset is not supported yet.\n"); + break; + case CHIP_BUSTYPE_SPI: + msg_pinfo("SPI on this chipset is not supported yet.\n"); + buses_supported = CHIP_BUSTYPE_NONE; + break; + default: + msg_pinfo("Something went wrong with bus type detection.\n"); + buses_supported = CHIP_BUSTYPE_NONE; + break; } - msg_pdbg("Found SMBus device %04x:%04x at %02x:%02x:%01x\n", - smbusdev->vendor_id, smbusdev->device_id, - smbusdev->bus, smbusdev->dev, smbusdev->func);
- /* Locate the BAR where the SPI interface lives. */ - mcp_spibaraddr = pci_read_long(smbusdev, 0x74); - msg_pdbg("SPI BAR is at 0x%08x, ", mcp_spibaraddr); - - msg_pinfo("Please send the output of "flashrom -V" to " - "flashrom@flashrom.org to help us finish support for your " - "chipset. Thanks.\n"); - - return 0; + return result; }
static int enable_flash_ht1000(struct pci_dev *dev, const char *name) @@ -1314,10 +1351,10 @@ {0x10de, 0x0365, OK, "NVIDIA", "MCP55", enable_flash_mcp55}, /* LPC */ {0x10de, 0x0366, OK, "NVIDIA", "MCP55", enable_flash_mcp55}, /* LPC */ {0x10de, 0x0367, OK, "NVIDIA", "MCP55", enable_flash_mcp55}, /* Pro */ - {0x10de, 0x03e0, NT, "NVIDIA", "MCP61", enable_flash_mcp7x}, - {0x10de, 0x03e1, NT, "NVIDIA", "MCP61", enable_flash_mcp7x}, - {0x10de, 0x03e2, NT, "NVIDIA", "MCP61", enable_flash_mcp7x}, - {0x10de, 0x03e3, NT, "NVIDIA", "MCP61", enable_flash_mcp7x}, + {0x10de, 0x03e0, NT, "NVIDIA", "MCP61", enable_flash_mcp67}, + {0x10de, 0x03e1, NT, "NVIDIA", "MCP61", enable_flash_mcp67}, + {0x10de, 0x03e2, NT, "NVIDIA", "MCP61", enable_flash_mcp67}, + {0x10de, 0x03e3, NT, "NVIDIA", "MCP61", enable_flash_mcp67}, {0x10de, 0x0440, NT, "NVIDIA", "MCP65", enable_flash_mcp7x}, {0x10de, 0x0441, NT, "NVIDIA", "MCP65", enable_flash_mcp7x}, {0x10de, 0x0442, NT, "NVIDIA", "MCP65", enable_flash_mcp7x},
Am Mittwoch, den 17.02.2010, 15:19 +0100 schrieb Carl-Daniel Hailfinger:
Refactor MCP SPI detection:
- Set supported buses based on ISA bridge reg 0x8a
- Use MCP55 chipset enable only if LPC is detected
- Allow LPC on MCP61
- Eliminate duplicated code where possible
Sounds great.
/* dev is the ISA bridge. No idea what the stuff below does. */ byte = pci_read_byte(dev, 0x8a);
- msg_pdbg("ISA bridge reg 0x8a contents: 0x%02x, bit 6 is %i, bit 5 is "
"%i\n", byte, (byte >> 6) & 0x1, (byte >> 5) & 0x1);- msg_pdbg("Guessed flash bus type is %s\n", ((byte >> 5) & 0x3) == 0x2 ?
"SPI" : "unknown, probably LPC");
- msg_pdbg("ISA/LPC bridge reg 0x8a contents: 0x%02x, bit 6 is %i, bit 5 "
"is %i\n", byte, (byte >> 6) & 0x1, (byte >> 5) & 0x1);- switch ((byte >> 5) & 0x3) {
- case 0x0:
buses_supported = CHIP_BUSTYPE_LPC;break;- case 0x2:
buses_supported = CHIP_BUSTYPE_SPI;break;- default:
buses_supported = CHIP_BUSTYPE_UNKNOWN;break;- }
- busname = flashbuses_to_text(buses_supported);
- msg_pdbg("Guessed flash bus type is %s\n", busname);
- free(busname);
Hmm. I think there was a report with 0x1 for LPC, but I have to recheck that.
@@ -1088,8 +1097,9 @@ /* Look for the SMBus device (SMBus PCI class) */ smbusdev = pci_dev_find_vendorclass(0x10de, 0x0c05); if (!smbusdev) {
msg_perr("ERROR: SMBus device not found. Aborting.\n");exit(1);
msg_perr("ERROR: SMBus device not found.\n");buses_supported = CHIP_BUSTYPE_NONE;return 1;
We don't need the SMBus device for LPC flashing, unless GPIOs are to be toggled.
- } else if (mcp_spibaraddr && !(buses_supported & CHIP_BUSTYPE_SPI)) {
msg_pdbg("Strange. MCP SPI BAR is valid, but chipset apparently"" doesn't support SPI.\n");
The chipset might support SPI, it's just not enabled. Maybe more like "flash chip apperently is not SPI".
+/**
- The MCP67 code is guesswork based on cleanroom reverse engineering.
- Due to that, it only reads info and doesn't change any settings.
- It is assumed that LPC chips need the MCP55 code and SPI chips need the
For the MCP61, I have seen the MCP55 code in an board enable.
Regards, Michael Karcher
Am Mittwoch, den 17.02.2010, 15:50 +0100 schrieb Michael Karcher:
- case 0x0:
buses_supported = CHIP_BUSTYPE_LPC;break;- case 0x2:
buses_supported = CHIP_BUSTYPE_SPI;break;Hmm. I think there was a report with 0x1 for LPC, but I have to recheck that.
I was wrong.
Regards, Michael Karcher
On 17.02.2010 15:50, Michael Karcher wrote:
Am Mittwoch, den 17.02.2010, 15:19 +0100 schrieb Carl-Daniel Hailfinger:
Refactor MCP SPI detection:
- Set supported buses based on ISA bridge reg 0x8a
- Use MCP55 chipset enable only if LPC is detected
- Allow LPC on MCP61
- Eliminate duplicated code where possible
Sounds great.
Thanks for the review. I reworked code flow and messages and it should hopefully be mergeable now.
Signed-off-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net
Index: flashrom-mcp_spi_detect_refactor/chipset_enable.c =================================================================== --- flashrom-mcp_spi_detect_refactor/chipset_enable.c (Revision 905) +++ flashrom-mcp_spi_detect_refactor/chipset_enable.c (Arbeitskopie) @@ -1054,31 +1054,41 @@ return 0; }
-/** - * The MCP67 code is guesswork based on cleanroom reverse engineering. - * Due to that, it only reads info and doesn't change any settings. - * It is assumed that LPC chips need the MCP55 code and SPI chips need the - * code provided in this function. Until we know for sure, call - * enable_flash_mcp55 from this function. Warning: enable_flash_mcp55 - * might make SPI flash inaccessible. The same caveat applies to SPI init - * for LPC flash. +/* This is a shot in the dark. Even if the code is totally bogus for some + * chipsets, users will at least start to send in reports. */ -static int enable_flash_mcp67(struct pci_dev *dev, const char *name) +static int enable_flash_mcp6x_7x_common(struct pci_dev *dev, const char *name) { - int result = 0; + int ret = 0; uint8_t byte; uint16_t status; + char *busname; uint32_t mcp_spibaraddr; void *mcp_spibar; struct pci_dev *smbusdev;
+ msg_pinfo("This chipset is not really supported yet. Guesswork...\n"); + /* dev is the ISA bridge. No idea what the stuff below does. */ byte = pci_read_byte(dev, 0x8a); - msg_pdbg("ISA bridge reg 0x8a contents: 0x%02x, bit 6 is %i, bit 5 is " - "%i\n", byte, (byte >> 6) & 0x1, (byte >> 5) & 0x1); - msg_pdbg("Guessed flash bus type is %s\n", ((byte >> 5) & 0x3) == 0x2 ? - "SPI" : "unknown, probably LPC"); - /* Disable the write code for now until we have more info. */ + msg_pdbg("ISA/LPC bridge reg 0x8a contents: 0x%02x, bit 6 is %i, bit 5 " + "is %i\n", byte, (byte >> 6) & 0x1, (byte >> 5) & 0x1); + switch ((byte >> 5) & 0x3) { + case 0x0: + buses_supported = CHIP_BUSTYPE_LPC; + break; + case 0x2: + buses_supported = CHIP_BUSTYPE_SPI; + break; + default: + buses_supported = CHIP_BUSTYPE_UNKNOWN; + break; + } + busname = flashbuses_to_text(buses_supported); + msg_pdbg("Guessed flash bus type is %s\n", busname); + free(busname); + + /* Force enable SPI and disable LPC? Not a good idea. */ #if 0 byte |= (1 << 6); byte &= ~(1 << 5); @@ -1088,8 +1098,15 @@ /* Look for the SMBus device (SMBus PCI class) */ smbusdev = pci_dev_find_vendorclass(0x10de, 0x0c05); if (!smbusdev) { - msg_perr("ERROR: SMBus device not found. Aborting.\n"); - exit(1); + if (buses_supported & CHIP_BUSTYPE_SPI) { + msg_perr("ERROR: SMBus device not found. Not enabling " + "SPI.\n"); + buses_supported &= ~CHIP_BUSTYPE_SPI; + ret = 1; + } else { + msg_pinfo("Odd. SMBus device not found.\n"); + } + goto out_msg; } msg_pdbg("Found SMBus device %04x:%04x at %02x:%02x:%01x\n", smbusdev->vendor_id, smbusdev->device_id, @@ -1108,7 +1125,7 @@ msg_pdbg("after clearing low bits BAR is at 0x%08x\n", mcp_spibaraddr);
/* Accessing a NULL pointer BAR is evil. Don't do it. */ - if (mcp_spibaraddr) { + if (mcp_spibaraddr && (buses_supported == CHIP_BUSTYPE_SPI)) { /* Map the BAR. Bytewise/wordwise access at 0x530 and 0x540. */ mcp_spibar = physmap("MCP67 SPI", mcp_spibaraddr, 0x544);
@@ -1125,54 +1142,83 @@ status, status & 0x1, (status >> 8) & 0x1); /* FIXME: Remove the physunmap once the SPI driver exists. */ physunmap(mcp_spibar, 0x544); + } else if (!mcp_spibaraddr && (buses_supported & CHIP_BUSTYPE_SPI)) { + msg_pdbg("Strange. MCP SPI BAR is invalid.\n"); + buses_supported &= ~CHIP_BUSTYPE_SPI; + } else if (mcp_spibaraddr && !(buses_supported & CHIP_BUSTYPE_SPI)) { + msg_pdbg("Strange. MCP SPI BAR is valid, but chipset apparently" + " doesn't have SPI enabled.\n"); } else { - msg_pdbg("Strange. MCP67 SPI BAR is invalid.\n"); + msg_pdbg("MCP SPI is not used.\n"); } +out_msg: msg_pinfo("Please send the output of "flashrom -V" to " "flashrom@flashrom.org to help us finish support for your " "chipset. Thanks.\n");
- /* Not sure if this is still correct. No docs as usual. */ - result = enable_flash_mcp55(dev, name); + return ret; +}
+/** + * The MCP61/MCP67 code is guesswork based on cleanroom reverse engineering. + * Due to that, it only reads info and doesn't change any settings. + * It is assumed that LPC chips need the MCP55 code and SPI chips need the + * code provided in enable_flash_mcp6x_7x_common. Until we know for sure, call + * enable_flash_mcp55 from this function only if enable_flash_mcp6x_7x_common + * indicates the flash chip is LPC. Warning: enable_flash_mcp55 + * might make SPI flash inaccessible. The same caveat applies to SPI init + * for LPC flash. + */ +static int enable_flash_mcp67(struct pci_dev *dev, const char *name) +{ + int result = 0; + + result = enable_flash_mcp6x_7x_common(dev, name); + if (result) + return result; + + /* Not sure if this is correct. No docs as usual. */ + switch (buses_supported) { + case CHIP_BUSTYPE_LPC: + result = enable_flash_mcp55(dev, name); + break; + case CHIP_BUSTYPE_SPI: + msg_pinfo("SPI on this chipset is not supported yet.\n"); + buses_supported = CHIP_BUSTYPE_NONE; + break; + default: + msg_pinfo("Something went wrong with bus type detection.\n"); + buses_supported = CHIP_BUSTYPE_NONE; + break; + } + return result; }
-/* This is a shot in the dark. Even if the code is totally bogus for some - * chipsets, users will at least start to send in reports. - */ static int enable_flash_mcp7x(struct pci_dev *dev, const char *name) { - uint8_t byte; - uint32_t mcp_spibaraddr; - struct pci_dev *smbusdev; + int result = 0;
- msg_pinfo("This chipset is not really supported yet. Guesswork...\n"); + result = enable_flash_mcp6x_7x_common(dev, name); + if (result) + return result;
- /* dev is the ISA bridge. No idea what the stuff below does. */ - byte = pci_read_byte(dev, 0x8a); - msg_pdbg("ISA/LPC bridge reg 0x8a contents: 0x%02x, bit 6 is %i, bit 5 " - "is %i\n", byte, (byte >> 6) & 0x1, (byte >> 5) & 0x1); - - /* Look for the SMBus device (SMBus PCI class) */ - smbusdev = pci_dev_find_vendorclass(0x10de, 0x0c05); - if (!smbusdev) { - msg_perr("ERROR: SMBus device not found. Aborting.\n"); - exit(1); + /* Not sure if this is correct. No docs as usual. */ + switch (buses_supported) { + case CHIP_BUSTYPE_LPC: + msg_pinfo("LPC on this chipset is not supported yet.\n"); + break; + case CHIP_BUSTYPE_SPI: + msg_pinfo("SPI on this chipset is not supported yet.\n"); + buses_supported = CHIP_BUSTYPE_NONE; + break; + default: + msg_pinfo("Something went wrong with bus type detection.\n"); + buses_supported = CHIP_BUSTYPE_NONE; + break; } - msg_pdbg("Found SMBus device %04x:%04x at %02x:%02x:%01x\n", - smbusdev->vendor_id, smbusdev->device_id, - smbusdev->bus, smbusdev->dev, smbusdev->func);
- /* Locate the BAR where the SPI interface lives. */ - mcp_spibaraddr = pci_read_long(smbusdev, 0x74); - msg_pdbg("SPI BAR is at 0x%08x, ", mcp_spibaraddr); - - msg_pinfo("Please send the output of "flashrom -V" to " - "flashrom@flashrom.org to help us finish support for your " - "chipset. Thanks.\n"); - - return 0; + return result; }
static int enable_flash_ht1000(struct pci_dev *dev, const char *name) @@ -1314,10 +1360,10 @@ {0x10de, 0x0365, OK, "NVIDIA", "MCP55", enable_flash_mcp55}, /* LPC */ {0x10de, 0x0366, OK, "NVIDIA", "MCP55", enable_flash_mcp55}, /* LPC */ {0x10de, 0x0367, OK, "NVIDIA", "MCP55", enable_flash_mcp55}, /* Pro */ - {0x10de, 0x03e0, NT, "NVIDIA", "MCP61", enable_flash_mcp7x}, - {0x10de, 0x03e1, NT, "NVIDIA", "MCP61", enable_flash_mcp7x}, - {0x10de, 0x03e2, NT, "NVIDIA", "MCP61", enable_flash_mcp7x}, - {0x10de, 0x03e3, NT, "NVIDIA", "MCP61", enable_flash_mcp7x}, + {0x10de, 0x03e0, NT, "NVIDIA", "MCP61", enable_flash_mcp67}, + {0x10de, 0x03e1, NT, "NVIDIA", "MCP61", enable_flash_mcp67}, + {0x10de, 0x03e2, NT, "NVIDIA", "MCP61", enable_flash_mcp67}, + {0x10de, 0x03e3, NT, "NVIDIA", "MCP61", enable_flash_mcp67}, {0x10de, 0x0440, NT, "NVIDIA", "MCP65", enable_flash_mcp7x}, {0x10de, 0x0441, NT, "NVIDIA", "MCP65", enable_flash_mcp7x}, {0x10de, 0x0442, NT, "NVIDIA", "MCP65", enable_flash_mcp7x},
Am Donnerstag, den 18.02.2010, 03:46 +0100 schrieb Carl-Daniel Hailfinger:
- } else if (!mcp_spibaraddr && (buses_supported & CHIP_BUSTYPE_SPI)) {
msg_pdbg("Strange. MCP SPI BAR is invalid.\n");buses_supported &= ~CHIP_BUSTYPE_SPI;
You might want to set ret to 1 here.
Nevertheless Acked-by: Michael Karcher flashrom@mkarcher.dialup.fu-berlin.de You can use the Ack for either version (with or with ret=1 at the point quoted above).
Regards, Michael Karcher
On 18.02.2010 09:06, Michael Karcher wrote:
Am Donnerstag, den 18.02.2010, 03:46 +0100 schrieb Carl-Daniel Hailfinger:
- } else if (!mcp_spibaraddr && (buses_supported & CHIP_BUSTYPE_SPI)) {
msg_pdbg("Strange. MCP SPI BAR is invalid.\n");buses_supported &= ~CHIP_BUSTYPE_SPI;You might want to set ret to 1 here.
Done.
Acked-by: Michael Karcher flashrom@mkarcher.dialup.fu-berlin.de
Thanks. New version follows.
Refactor MCP SPI detection: - Set supported buses based on ISA bridge reg 0x8a - Use MCP55 chipset enable only if LPC is detected - Allow LPC on MCP61 - Eliminate duplicated code where possible
Signed-off-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net Acked-by: Michael Karcher flashrom@mkarcher.dialup.fu-berlin.de
Index: flashrom-mcp_spi_detect_refactor/chipset_enable.c =================================================================== --- flashrom-mcp_spi_detect_refactor/chipset_enable.c (Revision 905) +++ flashrom-mcp_spi_detect_refactor/chipset_enable.c (Arbeitskopie) @@ -1054,31 +1054,41 @@ return 0; }
-/** - * The MCP67 code is guesswork based on cleanroom reverse engineering. - * Due to that, it only reads info and doesn't change any settings. - * It is assumed that LPC chips need the MCP55 code and SPI chips need the - * code provided in this function. Until we know for sure, call - * enable_flash_mcp55 from this function. Warning: enable_flash_mcp55 - * might make SPI flash inaccessible. The same caveat applies to SPI init - * for LPC flash. +/* This is a shot in the dark. Even if the code is totally bogus for some + * chipsets, users will at least start to send in reports. */ -static int enable_flash_mcp67(struct pci_dev *dev, const char *name) +static int enable_flash_mcp6x_7x_common(struct pci_dev *dev, const char *name) { - int result = 0; + int ret = 0; uint8_t byte; uint16_t status; + char *busname; uint32_t mcp_spibaraddr; void *mcp_spibar; struct pci_dev *smbusdev;
+ msg_pinfo("This chipset is not really supported yet. Guesswork...\n"); + /* dev is the ISA bridge. No idea what the stuff below does. */ byte = pci_read_byte(dev, 0x8a); - msg_pdbg("ISA bridge reg 0x8a contents: 0x%02x, bit 6 is %i, bit 5 is " - "%i\n", byte, (byte >> 6) & 0x1, (byte >> 5) & 0x1); - msg_pdbg("Guessed flash bus type is %s\n", ((byte >> 5) & 0x3) == 0x2 ? - "SPI" : "unknown, probably LPC"); - /* Disable the write code for now until we have more info. */ + msg_pdbg("ISA/LPC bridge reg 0x8a contents: 0x%02x, bit 6 is %i, bit 5 " + "is %i\n", byte, (byte >> 6) & 0x1, (byte >> 5) & 0x1); + switch ((byte >> 5) & 0x3) { + case 0x0: + buses_supported = CHIP_BUSTYPE_LPC; + break; + case 0x2: + buses_supported = CHIP_BUSTYPE_SPI; + break; + default: + buses_supported = CHIP_BUSTYPE_UNKNOWN; + break; + } + busname = flashbuses_to_text(buses_supported); + msg_pdbg("Guessed flash bus type is %s\n", busname); + free(busname); + + /* Force enable SPI and disable LPC? Not a good idea. */ #if 0 byte |= (1 << 6); byte &= ~(1 << 5); @@ -1088,8 +1098,15 @@ /* Look for the SMBus device (SMBus PCI class) */ smbusdev = pci_dev_find_vendorclass(0x10de, 0x0c05); if (!smbusdev) { - msg_perr("ERROR: SMBus device not found. Aborting.\n"); - exit(1); + if (buses_supported & CHIP_BUSTYPE_SPI) { + msg_perr("ERROR: SMBus device not found. Not enabling " + "SPI.\n"); + buses_supported &= ~CHIP_BUSTYPE_SPI; + ret = 1; + } else { + msg_pinfo("Odd. SMBus device not found.\n"); + } + goto out_msg; } msg_pdbg("Found SMBus device %04x:%04x at %02x:%02x:%01x\n", smbusdev->vendor_id, smbusdev->device_id, @@ -1108,7 +1125,7 @@ msg_pdbg("after clearing low bits BAR is at 0x%08x\n", mcp_spibaraddr);
/* Accessing a NULL pointer BAR is evil. Don't do it. */ - if (mcp_spibaraddr) { + if (mcp_spibaraddr && (buses_supported == CHIP_BUSTYPE_SPI)) { /* Map the BAR. Bytewise/wordwise access at 0x530 and 0x540. */ mcp_spibar = physmap("MCP67 SPI", mcp_spibaraddr, 0x544);
@@ -1125,54 +1142,84 @@ status, status & 0x1, (status >> 8) & 0x1); /* FIXME: Remove the physunmap once the SPI driver exists. */ physunmap(mcp_spibar, 0x544); + } else if (!mcp_spibaraddr && (buses_supported & CHIP_BUSTYPE_SPI)) { + msg_pdbg("Strange. MCP SPI BAR is invalid.\n"); + buses_supported &= ~CHIP_BUSTYPE_SPI; + ret = 1; + } else if (mcp_spibaraddr && !(buses_supported & CHIP_BUSTYPE_SPI)) { + msg_pdbg("Strange. MCP SPI BAR is valid, but chipset apparently" + " doesn't have SPI enabled.\n"); } else { - msg_pdbg("Strange. MCP67 SPI BAR is invalid.\n"); + msg_pdbg("MCP SPI is not used.\n"); } +out_msg: msg_pinfo("Please send the output of "flashrom -V" to " "flashrom@flashrom.org to help us finish support for your " "chipset. Thanks.\n");
- /* Not sure if this is still correct. No docs as usual. */ - result = enable_flash_mcp55(dev, name); + return ret; +}
+/** + * The MCP61/MCP67 code is guesswork based on cleanroom reverse engineering. + * Due to that, it only reads info and doesn't change any settings. + * It is assumed that LPC chips need the MCP55 code and SPI chips need the + * code provided in enable_flash_mcp6x_7x_common. Until we know for sure, call + * enable_flash_mcp55 from this function only if enable_flash_mcp6x_7x_common + * indicates the flash chip is LPC. Warning: enable_flash_mcp55 + * might make SPI flash inaccessible. The same caveat applies to SPI init + * for LPC flash. + */ +static int enable_flash_mcp67(struct pci_dev *dev, const char *name) +{ + int result = 0; + + result = enable_flash_mcp6x_7x_common(dev, name); + if (result) + return result; + + /* Not sure if this is correct. No docs as usual. */ + switch (buses_supported) { + case CHIP_BUSTYPE_LPC: + result = enable_flash_mcp55(dev, name); + break; + case CHIP_BUSTYPE_SPI: + msg_pinfo("SPI on this chipset is not supported yet.\n"); + buses_supported = CHIP_BUSTYPE_NONE; + break; + default: + msg_pinfo("Something went wrong with bus type detection.\n"); + buses_supported = CHIP_BUSTYPE_NONE; + break; + } + return result; }
-/* This is a shot in the dark. Even if the code is totally bogus for some - * chipsets, users will at least start to send in reports. - */ static int enable_flash_mcp7x(struct pci_dev *dev, const char *name) { - uint8_t byte; - uint32_t mcp_spibaraddr; - struct pci_dev *smbusdev; + int result = 0;
- msg_pinfo("This chipset is not really supported yet. Guesswork...\n"); + result = enable_flash_mcp6x_7x_common(dev, name); + if (result) + return result;
- /* dev is the ISA bridge. No idea what the stuff below does. */ - byte = pci_read_byte(dev, 0x8a); - msg_pdbg("ISA/LPC bridge reg 0x8a contents: 0x%02x, bit 6 is %i, bit 5 " - "is %i\n", byte, (byte >> 6) & 0x1, (byte >> 5) & 0x1); - - /* Look for the SMBus device (SMBus PCI class) */ - smbusdev = pci_dev_find_vendorclass(0x10de, 0x0c05); - if (!smbusdev) { - msg_perr("ERROR: SMBus device not found. Aborting.\n"); - exit(1); + /* Not sure if this is correct. No docs as usual. */ + switch (buses_supported) { + case CHIP_BUSTYPE_LPC: + msg_pinfo("LPC on this chipset is not supported yet.\n"); + break; + case CHIP_BUSTYPE_SPI: + msg_pinfo("SPI on this chipset is not supported yet.\n"); + buses_supported = CHIP_BUSTYPE_NONE; + break; + default: + msg_pinfo("Something went wrong with bus type detection.\n"); + buses_supported = CHIP_BUSTYPE_NONE; + break; } - msg_pdbg("Found SMBus device %04x:%04x at %02x:%02x:%01x\n", - smbusdev->vendor_id, smbusdev->device_id, - smbusdev->bus, smbusdev->dev, smbusdev->func);
- /* Locate the BAR where the SPI interface lives. */ - mcp_spibaraddr = pci_read_long(smbusdev, 0x74); - msg_pdbg("SPI BAR is at 0x%08x, ", mcp_spibaraddr); - - msg_pinfo("Please send the output of "flashrom -V" to " - "flashrom@flashrom.org to help us finish support for your " - "chipset. Thanks.\n"); - - return 0; + return result; }
static int enable_flash_ht1000(struct pci_dev *dev, const char *name) @@ -1314,10 +1361,10 @@ {0x10de, 0x0365, OK, "NVIDIA", "MCP55", enable_flash_mcp55}, /* LPC */ {0x10de, 0x0366, OK, "NVIDIA", "MCP55", enable_flash_mcp55}, /* LPC */ {0x10de, 0x0367, OK, "NVIDIA", "MCP55", enable_flash_mcp55}, /* Pro */ - {0x10de, 0x03e0, NT, "NVIDIA", "MCP61", enable_flash_mcp7x}, - {0x10de, 0x03e1, NT, "NVIDIA", "MCP61", enable_flash_mcp7x}, - {0x10de, 0x03e2, NT, "NVIDIA", "MCP61", enable_flash_mcp7x}, - {0x10de, 0x03e3, NT, "NVIDIA", "MCP61", enable_flash_mcp7x}, + {0x10de, 0x03e0, NT, "NVIDIA", "MCP61", enable_flash_mcp67}, + {0x10de, 0x03e1, NT, "NVIDIA", "MCP61", enable_flash_mcp67}, + {0x10de, 0x03e2, NT, "NVIDIA", "MCP61", enable_flash_mcp67}, + {0x10de, 0x03e3, NT, "NVIDIA", "MCP61", enable_flash_mcp67}, {0x10de, 0x0440, NT, "NVIDIA", "MCP65", enable_flash_mcp7x}, {0x10de, 0x0441, NT, "NVIDIA", "MCP65", enable_flash_mcp7x}, {0x10de, 0x0442, NT, "NVIDIA", "MCP65", enable_flash_mcp7x},
On 18.02.2010 13:22, Carl-Daniel Hailfinger wrote:
Refactor MCP SPI detection:
- Set supported buses based on ISA bridge reg 0x8a
- Use MCP55 chipset enable only if LPC is detected
- Allow LPC on MCP61
- Eliminate duplicated code where possible
Committed in r906.
Regards, Carl-Daniel
-
Carl-Daniel Hailfinger -
Michael Karcher