[LinuxBIOS] [PATCH] K8T890 V-link fine-tune

[Rudolf Marek]

Uwe Hermann wrote:
> 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 <r.marek at assembler.cz>
> 
> 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.


hmm aha cut and paste ;) Thanks.


> 
> 
>> +
>> +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?

We check because all those values are specific for the SB & NB combination,
for other SBs this should be extended for VT8237A VT8237S VT8237 (without plus 
R) and VT8251 too.


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

Well I really need it like it was, because I can add those numbers for other via 
SB... Image there some kind of case statement with different PCIids or just 
block with the find_pci_device...

Rudolf