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 <[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.
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
--
linuxbios mailing list
linuxbios@linuxbios.org
http://www.linuxbios.org/mailman/listinfo/linuxbios
