[flashrom] [PATCH] Fix VIA VX*** support.

Carl-Daniel Hailfinger c-d.hailfinger.devel.2006 at gmx.net
Wed Aug 1 00:16:05 CEST 2012


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 at ge.com>
> Signed-off-by: Stefan Tauner <stefan.tauner at 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 at 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

-- 
http://www.hailfinger.org/





More information about the flashrom mailing list