On Mon, Nov 12, 2007 at 11:24:37PM +0100, Rudolf Marek wrote:
> Hello,
>
> Attached patch fine-tunes the V-link bus between K8T890 and VT8237R and set 
> it to 8X transfer rate (up to 1066 MB/s) similar code placed here would be 
> needed for VT8237A/S etc. Using VIA recommended values despite they are for 
> K8T890CF, this is K8T890CE (still dont know what is exactly different).
>
> This patch enables the parity error reporting on V-Link, so it enables NMI 
> generation for the SERR# errors. The NMI may not be generated, maybe port 
> 61h
> needs some tuning too.
>
> Signed-off-by: Rudolf Marek <[EMAIL PROTECTED]>

Thanks, r2958 with some changes, see below.


> +#include <device/device.h>
> +#include <device/pci.h>
> +#include <device/pci_ops.h>
> +#include <device/pci_ids.h>
> +#include <console/console.h>
> +#include <cpu/x86/msr.h>
> +#include <cpu/amd/mtrr.h>

I dropped

  #include <device/pci_ops.h>
  #include <cpu/x86/msr.h>
  #include <cpu/amd/mtrr.h>

as they're not needed.


> +
> +static void error_enable(struct device *dev)
> +{
> +     /*
> +      * bit0 - Enable V-link parity error reporting in 0x50 bit0 (RWC)
> +      * bit6 - Parity Error/SERR# Report Through V-Link to SB
> +      * bit7 - Parity Error/SERR# Report Through NMI
> +      */
> +     pci_write_config8(dev, 0x58, 0x81);
> +}
> +
> +static struct device_operations error_ops = {
> +     .read_resources = pci_dev_read_resources,
> +     .set_resources = pci_dev_set_resources,
> +     .enable_resources = pci_dev_enable_resources,
> +     .enable = error_enable,
> +     .ops_pci = 0,
> +};
> +
> +static const struct pci_driver northbridge_driver __pci_driver = {
> +     .ops = &error_ops,
> +     .vendor = PCI_VENDOR_ID_VIA,
> +     .device = PCI_DEVICE_ID_VIA_K8T890CE_1,

This should probably be renamed to PCI_DEVICE_ID_VIA_K8T890CE_ERROR later.


> +};
> Index: src/southbridge/via/k8t890/k8t890_ctrl.c
> ===================================================================
> --- src/southbridge/via/k8t890/k8t890_ctrl.c  (revision 2953)
> +++ src/southbridge/via/k8t890/k8t890_ctrl.c  (working copy)
> @@ -23,6 +23,48 @@
>  #include <device/pci_ids.h>
>  #include <console/console.h>
>  
> +static void ctrl_init(struct device *dev)
> +{
> +     u8 reg;
> +     device_t devsb = dev_find_device(PCI_VENDOR_ID_VIA,
> +                                      PCI_DEVICE_ID_VIA_VT8237R_LPC, 0);
> +
> +     /* Setup the V-Link for VT8237R, 8X mode */
> +     if (devsb) {

Do we even need this? 

The
        .init = ctrl_init,

and the

        static const struct pci_driver northbridge_driver __pci_driver = {
                .ops = &ctrl_ops,
                .vendor = PCI_VENDOR_ID_VIA,
                .device = PCI_DEVICE_ID_VIA_K8T890CE_7,
        };

will only call this function for the PCI_DEVICE_ID_VIA_K8T890CE_7 device,
no need for explicit checks. Are the registers accessed from
PCI_DEVICE_ID_VIA_K8T890CE_7 only?

Also, why check for VT8237R's LPC device in the K8T890 code? Drop it?


> +
> +     /* for K8T890CF VIA recommends what is in VIA column, AW is award 8X
> +      *                                              REG     DEF     AW      
> VIA-8X  VIA-4X
> +      * NB V-Link Manual Driving Control strobe      0xb5    0x46    0x46    
> 0x88    0x88
> +      * NB V-Link Manual Driving Control - Data      0xb6    0x46    0x46    
> 0x88    0x88
> +      * NB V-Link Receiving Strobe Delay             0xb7    0x02    0x02    
> 0x61    0x01
> +      * NB V-Link Compensation Control bit4,0 (b5,b6)0xb4    0x10    0x10    
> 0x11    0x11
> +      * SB V-Link Strobe Drive Control               0xb9    0x00    0xa5    
> 0x98    0x98
> +      * SB V-Link Data drive Control????             0xba    0x00    0xbb    
> 0x77    0x77
> +      * SB V-Link Receive Strobe Delay????           0xbb    0x04    0x11    
> 0x11    0x11
> +      * SB V-Link Compensation Control bit0 (use b9) 0xb8    0x00    0x01    
> 0x01    0x01
> +      * V-Link CKG Control                           0xb0    0x05    0x05    
> 0x06    0x03
> +      * V-Link CKG Control                           0xb1    0x05    0x05    
> 0x01    0x03
> +      */
> +             pci_write_config8(dev, 0xb5, 0x88);
> +             pci_write_config8(dev, 0xb6, 0x88);
> +             pci_write_config8(dev, 0xb7, 0x61);
> +             reg = pci_read_config8(dev, 0xb4);
> +             reg |= 0x11;
> +             pci_write_config8(dev, 0xb4, reg);
> +             pci_write_config8(dev, 0xb9, 0x98);
> +             pci_write_config8(dev, 0xba, 0x77);
> +             pci_write_config8(dev, 0xbb, 0x11);
> +             reg = pci_read_config8(dev, 0xb8);
> +             reg |= 0x1;
> +             pci_write_config8(dev, 0xb8, reg);
> +             pci_write_config8(dev, 0xb0, 0x06);
> +             pci_write_config8(dev, 0xb1, 0x01);
> +
> +             /* Program V-link 8X 16bit full duplex, parity enabled */
> +             pci_write_config8(dev, 0x48, 0xa3);
> +     }

I've refactored this a bit, there should be no semantic changes though.


Uwe.
-- 
http://www.hermann-uwe.de  | http://www.holsham-traders.de
http://www.crazy-hacks.org | http://www.unmaintained-free-software.org

Attachment: signature.asc
Description: Digital signature

-- 
linuxbios mailing list
linuxbios@linuxbios.org
http://www.linuxbios.org/mailman/listinfo/linuxbios

Reply via email to