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 <[email protected]>
> Signed-off-by: Stefan Tauner <[email protected]>

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 <[email protected]>
>   * 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/


_______________________________________________
flashrom mailing list
[email protected]
http://www.flashrom.org/mailman/listinfo/flashrom

Reply via email to