Patchwork Fix VIA VX*** support.

login
register
about
Submitter Stefan Tauner
Date 2012-07-31 18:52:57
Message ID <1343760777-6313-1-git-send-email-stefan.tauner@student.tuwien.ac.at>
Download mbox | patch
Permalink /patch/3699/
State Superseded
Headers show

Comments

Stefan Tauner - 2012-07-31 18:52:57
Helge Wagner's patch that added VIA VX900 chipset support made me look
closer at the datasheets which led to some concise documentation about
newer VIA chipsets: http://flashrom.org/VIA

Based on that this patch adds full support for VX800/VX820, VX855/VX875
and VX900, including SPI and LPC. VT8237S was not changed (SPI support
only) because there is no public datasheet and it is not clear how to
distinguish between LPC and SPI strapping and investigations in (NDAed)
documents have not brought up anything conclusively.

enable_flash_vt823x could probably be enhanced too due to various
yet ignored LPC options of the chipset.

Signed-off-by: Helge Wagner <Helge.Wagner@ge.com>
Signed-off-by: Stefan Tauner <stefan.tauner@student.tuwien.ac.at>
---
New version with tiny changes for 0.9.6...
test status is NT and arguable... but since there are so few VIA users
out there anyway i think we wont get too many mails anyway.
 
 chipset_enable.c |   80 +++++++++++++++++++++++++++++++++++++++++++++++-------
 ichspi.c         |   11 +++-----
 programmer.h     |    2 +-
 3 files changed, 75 insertions(+), 18 deletions(-)
Carl-Daniel Hailfinger - 2012-07-31 22:16:05
Am 31.07.2012 20:52 schrieb Stefan Tauner:
> Helge Wagner's patch that added VIA VX900 chipset support made me look
> closer at the datasheets which led to some concise documentation about
> newer VIA chipsets: http://flashrom.org/VIA

Thanks, that was very helpful. I have updated the wiki with new information.


> Based on that this patch adds full support for VX800/VX820, VX855/VX875
> and VX900, including SPI and LPC. VT8237S was not changed (SPI support
> only) because there is no public datasheet and it is not clear how to
> distinguish between LPC and SPI strapping and investigations in (NDAed)
> documents have not brought up anything conclusively.
>
> enable_flash_vt823x could probably be enhanced too due to various
> yet ignored LPC options of the chipset.
>
> Signed-off-by: Helge Wagner <Helge.Wagner@ge.com>
> Signed-off-by: Stefan Tauner <stefan.tauner@student.tuwien.ac.at>

The more I learn about VIA, the less I'm inclined to change the
behaviour of flashrom substantially directly before a release.
I would like to get a modified version of this patch merged after 0.9.6.


> New version with tiny changes for 0.9.6...
> test status is NT and arguable... but since there are so few VIA users
> out there anyway i think we wont get too many mails anyway.
>  
>  chipset_enable.c |   80 +++++++++++++++++++++++++++++++++++++++++++++++-------
>  ichspi.c         |   11 +++-----
>  programmer.h     |    2 +-
>  3 files changed, 75 insertions(+), 18 deletions(-)
>
> diff --git a/chipset_enable.c b/chipset_enable.c
> index 936d7b8..0974427 100644
> --- a/chipset_enable.c
> +++ b/chipset_enable.c
> @@ -6,6 +6,7 @@
>   * Copyright (C) 2006 Uwe Hermann <uwe@hermann-uwe.de>
>   * Copyright (C) 2007,2008,2009 Carl-Daniel Hailfinger
>   * Copyright (C) 2009 Kontron Modular Computers GmbH
> + * Copyright (C) 2011, 2012 Stefan Tauner
>   *
>   * This program is free software; you can redistribute it and/or modify
>   * it under the terms of the GNU General Public License as published by
> @@ -508,12 +509,6 @@ static int enable_flash_tunnelcreek(struct pci_dev *dev, const char *name)
>  	return ret;
>  }
>  
> -static int enable_flash_vt8237s_spi(struct pci_dev *dev, const char *name)
> -{
> -	/* Do we really need no write enable? */
> -	return via_init_spi(dev);
> -}
> -
>  static int enable_flash_ich_dc_spi(struct pci_dev *dev, const char *name,
>  				   enum ich_chipset ich_generation)
>  {
> @@ -681,16 +676,80 @@ static int enable_flash_vt823x(struct pci_dev *dev, const char *name)
>  		return -1;
>  	}
>  
> -	if (dev->device_id == 0x3227) { /* VT8237R */
> +	if (dev->device_id == 0x3227) { /* VT8237 */

If you change that comment, please change it to VT8237/VT8237R.


>  		/* All memory cycles, not just ROM ones, go to LPC. */
>  		val = pci_read_byte(dev, 0x59);
>  		val &= ~0x80;
>  		rpci_write_byte(dev, 0x59, val);
>  	}
>  
> +	internal_buses_supported = BUS_LPC | BUS_FWH;

It's not that easy. Some older VIA chipsets do not support ROMs on
LPC/FWH, or LPC/FWH at all. And unfortunately, not all VIA chipsets
allow you to read the ROM type straps. Please kill this hunk for now.

VT8231 can speak Parallel/LPC
VT8233 can speak Parallel/LPC
VT8233A ???
VT8235 can speak Parallel/LPC/FWH
VT8237A ???
VT8237R can speak LPC/FWH (and is identical to VT8237)
VT8237S can speak LPC/FWH/SPI
CX700 can speak LPC/FWH
VX[89]* can speak LPC/FWH/SPI

Adding that information to the chipset enable functions should be done
in a separate patch. I can do that if you want... it's pretty tricky.


>  	return 0;
>  }
>  
> +static int enable_flash_vt_vx(struct pci_dev *dev, const char *name)
> +{
> +	struct pci_dev *south_north = pci_dev_find(0x1106, 0xa353);
> +	if (south_north == NULL) {
> +		msg_perr("Could not find South-North Module Interface Control device!\n");
> +		return ERROR_FATAL;
> +	}
> +
> +	msg_pdbg("Strapped to ");
> +	if ((pci_read_byte(south_north, 0x56) & 0x01) == 0) {
> +		msg_pdbg("LPC.\n");
> +		return enable_flash_vt823x(dev, name);
> +	}
> +	msg_pdbg("SPI.\n");
> +
> +	uint32_t mmio_base;
> +	void *mmio_base_physmapped;
> +	uint32_t spi_cntl;
> +	#define SPI_CNTL_LEN 0x08
> +	uint32_t spi0_mm_base = 0;
> +	switch(dev->device_id) {
> +		case 0x8353: /* VX800/VX820 */
> +			spi0_mm_base = pci_read_long(dev, 0xbc) << 8;
> +			break;
> +		case 0x8409: /* VX855/VX875 */
> +		case 0x8410: /* VX900 */
> +			mmio_base = pci_read_long(dev, 0xbc) << 8;
> +			mmio_base_physmapped = physmap("VIA VX MMIO register", mmio_base, SPI_CNTL_LEN);
> +			if (mmio_base_physmapped == ERROR_PTR) {
> +				physunmap(mmio_base_physmapped, SPI_CNTL_LEN);
> +				return ERROR_FATAL;
> +			}
> +
> +			/* Offset 0 - Bit 0 holds SPI Bus0 Enable Bit. */
> +			spi_cntl = mmio_readl(mmio_base_physmapped) + 0x00;
> +			if ((spi_cntl & 0x01) == 0) {
> +				msg_pdbg ("SPI Bus0 disabled!\n");
> +				physunmap(mmio_base_physmapped, SPI_CNTL_LEN);
> +				return ERROR_FATAL;
> +			}
> +			/* Offset 1-3 has  SPI Bus Memory Map Base Address: */
> +			spi0_mm_base = spi_cntl & 0xFFFFFF00;
> +
> +			/* Offset 4 - Bit 0 holds SPI Bus1 Enable Bit. */
> +			spi_cntl = mmio_readl(mmio_base_physmapped) + 0x04;
> +			if ((spi_cntl & 0x01) == 1)
> +				msg_pdbg2("SPI Bus1 is enabled too.\n");

We don't handle that anywhere, so it should probably spit out some
really loud warning.


> +
> +			physunmap(mmio_base_physmapped, SPI_CNTL_LEN);
> +			break;
> +		default:
> +			msg_perr("%s: Unsupported chipset %x:%x!\n", __func__, dev->vendor_id, dev->device_id);
> +			return ERROR_FATAL;
> +	}
> +
> +	return via_init_spi(dev, spi0_mm_base);
> +}
> +
> +static int enable_flash_vt8237s_spi(struct pci_dev *dev, const char *name)

This function needs to die.


> +{
> +	return via_init_spi(dev, pci_read_long(dev, 0xbc) << 8);
> +}
> +
>  static int enable_flash_cs5530(struct pci_dev *dev, const char *name)
>  {
>  	uint8_t reg8;
> @@ -1263,7 +1322,7 @@ const struct penable chipset_enables[] = {
>  	{0x1106, 0x0586, OK, "VIA", "VT82C586A/B",	enable_flash_amd8111},
>  	{0x1106, 0x0596, OK, "VIA", "VT82C596",		enable_flash_amd8111},
>  	{0x1106, 0x0686, OK, "VIA", "VT82C686A/B",	enable_flash_amd8111},
> -	{0x1106, 0x3074, OK, "VIA", "VT8233",		enable_flash_vt823x},
> +	{0x1106, 0x3074, OK, "VIA", "VT8233/VT8237R",	enable_flash_vt823x},

No, that's very likely just a datasheet bug (compare table 4 in VT8237R
datasheet 2.06 to the values mentioned in the detailed register
description for the Bus Control device). Please undo this hunk.


>  	{0x1106, 0x3147, OK, "VIA", "VT8233A",		enable_flash_vt823x},
>  	{0x1106, 0x3177, OK, "VIA", "VT8235",		enable_flash_vt823x},
>  	{0x1106, 0x3227, OK, "VIA", "VT8237",		enable_flash_vt823x},
> @@ -1271,8 +1330,9 @@ const struct penable chipset_enables[] = {
>  	{0x1106, 0x3372, OK, "VIA", "VT8237S",		enable_flash_vt8237s_spi},
>  	{0x1106, 0x8231, NT, "VIA", "VT8231",		enable_flash_vt823x},
>  	{0x1106, 0x8324, OK, "VIA", "CX700",		enable_flash_vt823x},
> -	{0x1106, 0x8353, OK, "VIA", "VX800/VX820",	enable_flash_vt8237s_spi},
> -	{0x1106, 0x8409, OK, "VIA", "VX855/VX875",	enable_flash_vt823x},
> +	{0x1106, 0x8353, NT, "VIA", "VX800/VX820",	enable_flash_vt_vx},
> +	{0x1106, 0x8409, NT, "VIA", "VX855/VX875",	enable_flash_vt_vx},
> +	{0x1106, 0x8410, NT, "VIA", "VX900",		enable_flash_vt_vx},
>  	{0x1166, 0x0200, OK, "Broadcom", "OSB4",	enable_flash_osb4},
>  	{0x1166, 0x0205, OK, "Broadcom", "HT-1000",	enable_flash_ht1000},
>  	{0x17f3, 0x6030, OK, "RDC", "R8610/R3210",	enable_flash_rdc_r8610},
> diff --git a/ichspi.c b/ichspi.c
> index 0223ae3..44d659b 100644
> --- a/ichspi.c
> +++ b/ichspi.c
> @@ -1844,19 +1844,16 @@ static const struct spi_programmer spi_programmer_via = {
>  	.write_aai = default_spi_write_aai,
>  };
>  
> -int via_init_spi(struct pci_dev *dev)
> +int via_init_spi(struct pci_dev *dev, uint32_t mmio_base)
>  {
> -	uint32_t mmio_base;
>  	int i;
>  
> -	mmio_base = (pci_read_long(dev, 0xbc)) << 8;
> -	msg_pdbg("MMIO base at = 0x%x\n", mmio_base);
> -	ich_spibar = physmap("VT8237S MMIO registers", mmio_base, 0x70);
> +	ich_spibar = physmap("VIA SPI MMIO registers", mmio_base, 0x70);
> +	/* Do we really need no write enable? Like the LPC one at D17F0 0x40 */
>  
> -	/* Not sure if it speaks all these bus protocols. */
> -	internal_buses_supported = BUS_LPC | BUS_FWH;
>  	ich_generation = CHIPSET_ICH7;
>  	register_spi_programmer(&spi_programmer_via);
> +	internal_buses_supported = BUS_SPI;

Please undo the internal_buses_supported change here.
So far, all VIA chipsets with SPI also support LPC/FWH. We could either
call enable_flash_vt823x() on all SPI-capable chipsets as well, and have
LPC/FWH/SPI sorted out automatically by flashrom, or we could try to
read the ROM straps, which may or may not be readable or documented.


>  
>  	msg_pdbg("0x00: 0x%04x     (SPIS)\n", mmio_readw(ich_spibar + 0));
>  	msg_pdbg("0x02: 0x%04x     (SPIC)\n", mmio_readw(ich_spibar + 2));
> diff --git a/programmer.h b/programmer.h
> index 5109ed9..ec29ed8 100644
> --- a/programmer.h
> +++ b/programmer.h
> @@ -561,7 +561,7 @@ enum ich_chipset {
>  extern uint32_t ichspi_bbar;
>  int ich_init_spi(struct pci_dev *dev, uint32_t base, void *rcrb,
>  		 enum ich_chipset ich_generation);
> -int via_init_spi(struct pci_dev *dev);
> +int via_init_spi(struct pci_dev *dev, uint32_t mmio_base);
>  
>  /* it85spi.c */
>  int it85xx_spi_init(struct superio s);

Regards,
Carl-Daniel

Patch

diff --git a/chipset_enable.c b/chipset_enable.c
index 936d7b8..0974427 100644
--- a/chipset_enable.c
+++ b/chipset_enable.c
@@ -6,6 +6,7 @@ 
  * Copyright (C) 2006 Uwe Hermann <uwe@hermann-uwe.de>
  * Copyright (C) 2007,2008,2009 Carl-Daniel Hailfinger
  * Copyright (C) 2009 Kontron Modular Computers GmbH
+ * Copyright (C) 2011, 2012 Stefan Tauner
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -508,12 +509,6 @@  static int enable_flash_tunnelcreek(struct pci_dev *dev, const char *name)
 	return ret;
 }
 
-static int enable_flash_vt8237s_spi(struct pci_dev *dev, const char *name)
-{
-	/* Do we really need no write enable? */
-	return via_init_spi(dev);
-}
-
 static int enable_flash_ich_dc_spi(struct pci_dev *dev, const char *name,
 				   enum ich_chipset ich_generation)
 {
@@ -681,16 +676,80 @@  static int enable_flash_vt823x(struct pci_dev *dev, const char *name)
 		return -1;
 	}
 
-	if (dev->device_id == 0x3227) { /* VT8237R */
+	if (dev->device_id == 0x3227) { /* VT8237 */
 		/* All memory cycles, not just ROM ones, go to LPC. */
 		val = pci_read_byte(dev, 0x59);
 		val &= ~0x80;
 		rpci_write_byte(dev, 0x59, val);
 	}
 
+	internal_buses_supported = BUS_LPC | BUS_FWH;
 	return 0;
 }
 
+static int enable_flash_vt_vx(struct pci_dev *dev, const char *name)
+{
+	struct pci_dev *south_north = pci_dev_find(0x1106, 0xa353);
+	if (south_north == NULL) {
+		msg_perr("Could not find South-North Module Interface Control device!\n");
+		return ERROR_FATAL;
+	}
+
+	msg_pdbg("Strapped to ");
+	if ((pci_read_byte(south_north, 0x56) & 0x01) == 0) {
+		msg_pdbg("LPC.\n");
+		return enable_flash_vt823x(dev, name);
+	}
+	msg_pdbg("SPI.\n");
+
+	uint32_t mmio_base;
+	void *mmio_base_physmapped;
+	uint32_t spi_cntl;
+	#define SPI_CNTL_LEN 0x08
+	uint32_t spi0_mm_base = 0;
+	switch(dev->device_id) {
+		case 0x8353: /* VX800/VX820 */
+			spi0_mm_base = pci_read_long(dev, 0xbc) << 8;
+			break;
+		case 0x8409: /* VX855/VX875 */
+		case 0x8410: /* VX900 */
+			mmio_base = pci_read_long(dev, 0xbc) << 8;
+			mmio_base_physmapped = physmap("VIA VX MMIO register", mmio_base, SPI_CNTL_LEN);
+			if (mmio_base_physmapped == ERROR_PTR) {
+				physunmap(mmio_base_physmapped, SPI_CNTL_LEN);
+				return ERROR_FATAL;
+			}
+
+			/* Offset 0 - Bit 0 holds SPI Bus0 Enable Bit. */
+			spi_cntl = mmio_readl(mmio_base_physmapped) + 0x00;
+			if ((spi_cntl & 0x01) == 0) {
+				msg_pdbg ("SPI Bus0 disabled!\n");
+				physunmap(mmio_base_physmapped, SPI_CNTL_LEN);
+				return ERROR_FATAL;
+			}
+			/* Offset 1-3 has  SPI Bus Memory Map Base Address: */
+			spi0_mm_base = spi_cntl & 0xFFFFFF00;
+
+			/* Offset 4 - Bit 0 holds SPI Bus1 Enable Bit. */
+			spi_cntl = mmio_readl(mmio_base_physmapped) + 0x04;
+			if ((spi_cntl & 0x01) == 1)
+				msg_pdbg2("SPI Bus1 is enabled too.\n");
+
+			physunmap(mmio_base_physmapped, SPI_CNTL_LEN);
+			break;
+		default:
+			msg_perr("%s: Unsupported chipset %x:%x!\n", __func__, dev->vendor_id, dev->device_id);
+			return ERROR_FATAL;
+	}
+
+	return via_init_spi(dev, spi0_mm_base);
+}
+
+static int enable_flash_vt8237s_spi(struct pci_dev *dev, const char *name)
+{
+	return via_init_spi(dev, pci_read_long(dev, 0xbc) << 8);
+}
+
 static int enable_flash_cs5530(struct pci_dev *dev, const char *name)
 {
 	uint8_t reg8;
@@ -1263,7 +1322,7 @@  const struct penable chipset_enables[] = {
 	{0x1106, 0x0586, OK, "VIA", "VT82C586A/B",	enable_flash_amd8111},
 	{0x1106, 0x0596, OK, "VIA", "VT82C596",		enable_flash_amd8111},
 	{0x1106, 0x0686, OK, "VIA", "VT82C686A/B",	enable_flash_amd8111},
-	{0x1106, 0x3074, OK, "VIA", "VT8233",		enable_flash_vt823x},
+	{0x1106, 0x3074, OK, "VIA", "VT8233/VT8237R",	enable_flash_vt823x},
 	{0x1106, 0x3147, OK, "VIA", "VT8233A",		enable_flash_vt823x},
 	{0x1106, 0x3177, OK, "VIA", "VT8235",		enable_flash_vt823x},
 	{0x1106, 0x3227, OK, "VIA", "VT8237",		enable_flash_vt823x},
@@ -1271,8 +1330,9 @@  const struct penable chipset_enables[] = {
 	{0x1106, 0x3372, OK, "VIA", "VT8237S",		enable_flash_vt8237s_spi},
 	{0x1106, 0x8231, NT, "VIA", "VT8231",		enable_flash_vt823x},
 	{0x1106, 0x8324, OK, "VIA", "CX700",		enable_flash_vt823x},
-	{0x1106, 0x8353, OK, "VIA", "VX800/VX820",	enable_flash_vt8237s_spi},
-	{0x1106, 0x8409, OK, "VIA", "VX855/VX875",	enable_flash_vt823x},
+	{0x1106, 0x8353, NT, "VIA", "VX800/VX820",	enable_flash_vt_vx},
+	{0x1106, 0x8409, NT, "VIA", "VX855/VX875",	enable_flash_vt_vx},
+	{0x1106, 0x8410, NT, "VIA", "VX900",		enable_flash_vt_vx},
 	{0x1166, 0x0200, OK, "Broadcom", "OSB4",	enable_flash_osb4},
 	{0x1166, 0x0205, OK, "Broadcom", "HT-1000",	enable_flash_ht1000},
 	{0x17f3, 0x6030, OK, "RDC", "R8610/R3210",	enable_flash_rdc_r8610},
diff --git a/ichspi.c b/ichspi.c
index 0223ae3..44d659b 100644
--- a/ichspi.c
+++ b/ichspi.c
@@ -1844,19 +1844,16 @@  static const struct spi_programmer spi_programmer_via = {
 	.write_aai = default_spi_write_aai,
 };
 
-int via_init_spi(struct pci_dev *dev)
+int via_init_spi(struct pci_dev *dev, uint32_t mmio_base)
 {
-	uint32_t mmio_base;
 	int i;
 
-	mmio_base = (pci_read_long(dev, 0xbc)) << 8;
-	msg_pdbg("MMIO base at = 0x%x\n", mmio_base);
-	ich_spibar = physmap("VT8237S MMIO registers", mmio_base, 0x70);
+	ich_spibar = physmap("VIA SPI MMIO registers", mmio_base, 0x70);
+	/* Do we really need no write enable? Like the LPC one at D17F0 0x40 */
 
-	/* Not sure if it speaks all these bus protocols. */
-	internal_buses_supported = BUS_LPC | BUS_FWH;
 	ich_generation = CHIPSET_ICH7;
 	register_spi_programmer(&spi_programmer_via);
+	internal_buses_supported = BUS_SPI;
 
 	msg_pdbg("0x00: 0x%04x     (SPIS)\n", mmio_readw(ich_spibar + 0));
 	msg_pdbg("0x02: 0x%04x     (SPIC)\n", mmio_readw(ich_spibar + 2));
diff --git a/programmer.h b/programmer.h
index 5109ed9..ec29ed8 100644
--- a/programmer.h
+++ b/programmer.h
@@ -561,7 +561,7 @@  enum ich_chipset {
 extern uint32_t ichspi_bbar;
 int ich_init_spi(struct pci_dev *dev, uint32_t base, void *rcrb,
 		 enum ich_chipset ich_generation);
-int via_init_spi(struct pci_dev *dev);
+int via_init_spi(struct pci_dev *dev, uint32_t mmio_base);
 
 /* it85spi.c */
 int it85xx_spi_init(struct superio s);