Patchwork Refactor MCP SPI detection

login
register
about
Submitter Carl-Daniel Hailfinger
Date 2010-02-17 14:19:29
Message ID <4B7BFAF1.2030806@gmx.net>
Download mbox | patch
Permalink /patch/932/
State Superseded
Headers show

Comments

Carl-Daniel Hailfinger - 2010-02-17 14:19:29
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>
Michael Karcher - 2010-02-17 14:50:37
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
Michael Karcher - 2010-02-17 15:48:21
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

Patch

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},