Hi,
Using pci_read_config8() inside mainboard.c causes a crash on the AMD DBM690T and Pistachio targets. AMD developers are seeing the crash as well and use pci_cf8_conf1.read8() instead which works fine.
Since I won't have access to my Asus M2A-VM board in the next two weeks, I'd appreciate if someone could test the patch below on a M2A-VM. I expect it to hang somewhere along these messages:
[...] enable_onboard_nic. enable_onboard_nic done. get_ide_dma66. get_ide_dma66 dev_find_slot(0:14.0) returned ....
If you can't recover from a non-booting image, DO NOT USE!
Signed-off-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net
Index: src/southbridge/amd/rs690/rs690.c =================================================================== --- src/southbridge/amd/rs690/rs690.c (Revision 3967) +++ src/southbridge/amd/rs690/rs690.c (Arbeitskopie) @@ -129,7 +129,7 @@ device_t nb_dev = 0, sb_dev = 0; int dev_ind;
- printk_info("rs690_enable: dev=0x%x, VID_DID=0x%x\n", dev, get_vid_did(dev)); + printk_info("rs690_enable: dev=%p, VID_DID=0x%x\n", dev, get_vid_did(dev));
nb_dev = dev_find_slot(0, PCI_DEVFN(0, 0)); if (!nb_dev) { Index: src/southbridge/amd/rs690/rs690_pcie.c =================================================================== --- src/southbridge/amd/rs690/rs690_pcie.c (Revision 3967) +++ src/southbridge/amd/rs690/rs690_pcie.c (Arbeitskopie) @@ -203,7 +203,7 @@ device_t sb_dev; struct southbridge_amd_rs690_config *cfg = (struct southbridge_amd_rs690_config *)nb_dev->chip_info; - printk_debug("gpp_sb_init nb_dev=0x%x, dev=0x%x, port=0x%x\n", nb_dev, dev, port); + printk_debug("gpp_sb_init nb_dev=0x%p, dev=0x%p, port=0x%x\n", nb_dev, dev, port);
/* init GPP core */ set_pcie_enable_bits(nb_dev, 0x20 | PCIE_CORE_INDEX_GPPSB, 1 << 8, Index: src/southbridge/amd/rs690/rs690_gfx.c =================================================================== --- src/southbridge/amd/rs690/rs690_gfx.c (Revision 3967) +++ src/southbridge/amd/rs690/rs690_gfx.c (Arbeitskopie) @@ -121,7 +121,7 @@ device_t k8_f0 = 0, k8_f2 = 0; device_t nb_dev = dev_find_slot(0, 0);
- printk_info("rs690_internal_gfx_enable dev=0x%x, nb_dev=0x%x.\n", dev, + printk_info("rs690_internal_gfx_enable dev=0x%p, nb_dev=0x%p.\n", dev, nb_dev);
/* set APERTURE_SIZE, 128M. */ @@ -417,7 +417,7 @@ struct southbridge_amd_rs690_config *cfg = (struct southbridge_amd_rs690_config *)nb_dev->chip_info;
- printk_info("rs690_gfx_init, nb_dev=0x%x, dev=0x%x, port=0x%x.\n", + printk_info("rs690_gfx_init, nb_dev=0x%p, dev=0x%p, port=0x%x.\n", nb_dev, dev, port);
/* step 0, REFCLK_SEL, skip A11 revision */ Index: src/southbridge/amd/sb600/sb600_sata.c =================================================================== --- src/southbridge/amd/sb600/sb600_sata.c (Revision 3967) +++ src/southbridge/amd/sb600/sb600_sata.c (Arbeitskopie) @@ -96,7 +96,7 @@ printk_spew("sata_bar2=%x\n", sata_bar2); /* 3040 */ printk_spew("sata_bar3=%x\n", sata_bar3); /* 3080 */ printk_spew("sata_bar4=%x\n", sata_bar4); /* 3000 */ - printk_spew("sata_bar5=%x\n", sata_bar5); /* e0309000 */ + printk_spew("sata_bar5=%p\n", sata_bar5); /* e0309000 */
/* Program the 2C to 0x43801002 */ dword = 0x43801002; Index: src/southbridge/amd/sb600/sb600_usb.c =================================================================== --- src/southbridge/amd/sb600/sb600_usb.c (Revision 3967) +++ src/southbridge/amd/sb600/sb600_usb.c (Arbeitskopie) @@ -94,7 +94,7 @@ /* pci_write_config32(dev, 0xf8, dword); */
usb2_bar0 = (u8 *) (pci_read_config32(dev, 0x10) & ~0xFF); - printk_info("usb2_bar0=%x\n", usb2_bar0); + printk_info("usb2_bar0=%p\n", usb2_bar0);
/* RPR5.4 Enables the USB PHY auto calibration resister to match 45ohm resistence */ dword = 0x00020F00; Index: src/southbridge/amd/sb600/sb600_hda.c =================================================================== --- src/southbridge/amd/sb600/sb600_hda.c (Revision 3967) +++ src/southbridge/amd/sb600/sb600_hda.c (Arbeitskopie) @@ -302,7 +302,7 @@ return;
base = (u8 *) ((u32)res->base); - printk_debug("base = %08x\n", base); + printk_debug("base = %p\n", base); codec_mask = codec_detect(base);
if (codec_mask) { Index: src/devices/device.c =================================================================== --- src/devices/device.c (Revision 3967) +++ src/devices/device.c (Arbeitskopie) @@ -358,7 +358,7 @@ base += size; printk_spew( - "%s %02x * [0x%08Lx - 0x%08Lx] %s\n", + "%s %02lx * [0x%08Lx - 0x%08Lx] %s\n", dev_path(dev), resource->index, resource->base, Index: src/devices/pci_ops.c =================================================================== --- src/devices/pci_ops.c (Revision 3967) +++ src/devices/pci_ops.c (Arbeitskopie) @@ -24,7 +24,7 @@ #include <device/pci_ids.h> #include <device/pci_ops.h>
-static struct bus *get_pbus(device_t dev) +struct bus *get_pbus(device_t dev) { struct bus *pbus = dev->bus; while(pbus && pbus->dev && !ops_pci_bus(pbus)) { Index: src/include/device/pci.h =================================================================== --- src/include/device/pci.h (Revision 3967) +++ src/include/device/pci.h (Arbeitskopie) @@ -99,5 +99,6 @@ } return bops; } +struct bus *get_pbus(device_t dev);
#endif /* PCI_H */ Index: src/cpu/x86/mtrr/mtrr.c =================================================================== --- src/cpu/x86/mtrr/mtrr.c (Revision 3967) +++ src/cpu/x86/mtrr/mtrr.c (Arbeitskopie) @@ -357,7 +357,7 @@ #endif } /* Allocate an msr */ - printk_spew(" Allocate an msr - basek = %08x, sizek = %08x,\n", basek, sizek); + printk_spew(" Allocate an msr - basek = %08lx, sizek = %08lx,\n", basek, sizek); state->range_startk = basek; state->range_sizek = sizek; } Index: src/cpu/x86/lapic/lapic_cpu_init.c =================================================================== --- src/cpu/x86/lapic/lapic_cpu_init.c (Revision 3967) +++ src/cpu/x86/lapic/lapic_cpu_init.c (Arbeitskopie) @@ -136,7 +136,7 @@ maxlvt = 4;
for (j = 1; j <= num_starts; j++) { - printk_spew("Sending STARTUP #%d to %u.\n", j, apicid); + printk_spew("Sending STARTUP #%d to %lu.\n", j, apicid); lapic_read_around(LAPIC_SPIV); lapic_write(LAPIC_ESR, 0); lapic_read(LAPIC_ESR); @@ -239,7 +239,7 @@ #warning "We may need to increase CONFIG_LB_MEM_TOPK, it need to be more than (0x100000+(20480 + STACK_SIZE)*CONFIG_MAX_CPU)\n" #endif if(stack_end > (CONFIG_LB_MEM_TOPK<<10)) { - printk_debug("start_cpu: Please increase the CONFIG_LB_MEM_TOPK more than %dK\n", stack_end>>10); + printk_debug("start_cpu: Please increase the CONFIG_LB_MEM_TOPK more than %luK\n", stack_end>>10); die("Can not go on\n"); } stack_end -= sizeof(struct cpu_info); Index: src/mainboard/amd/pistachio/resourcemap.c =================================================================== --- src/mainboard/amd/pistachio/resourcemap.c (Revision 3967) +++ src/mainboard/amd/pistachio/resourcemap.c (Arbeitskopie) @@ -17,7 +17,7 @@ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA */
-static void setup_pistachio_resource_map(void) +static void setup_mb_resource_map(void) { static const unsigned int register_values[] = { /* Careful set limit registers before base registers which contain the enables */ Index: src/mainboard/amd/pistachio/chip.h =================================================================== --- src/mainboard/amd/pistachio/chip.h (Revision 3967) +++ src/mainboard/amd/pistachio/chip.h (Arbeitskopie) @@ -21,6 +21,6 @@
struct mainboard_config { - unsigned long uma_size; /* How many UMA should be used in memory for TOP. */ + u32 uma_size; /* How many UMA should be used in memory for TOP. */ };
Index: src/mainboard/amd/pistachio/acpi/dsdt.asl =================================================================== --- src/mainboard/amd/pistachio/acpi/dsdt.asl (Revision 3967) +++ src/mainboard/amd/pistachio/acpi/dsdt.asl (Arbeitskopie) @@ -21,7 +21,7 @@ DefinitionBlock ( "DSDT.AML", /* Output filename */ "DSDT", /* Signature */ - 0x01, /* DSDT Revision */ + 0x02, /* DSDT Revision, needs to be 2 for 64bit */ "AMD ", /* OEMID */ "PISTACHI", /* TABLE ID */ 0x00010001 /* OEM Revision */ @@ -32,7 +32,6 @@ /* Data to be patched by the BIOS during POST */ /* FIXME the patching is not done yet! */ /* Memory related values */ - Name(TOM2, 0x0) /* Top of RAM memory above 4GB (>> 16) */ Name(LOMH, 0x0) /* Start of unused memory in C0000-E0000 range */ Name(PBAD, 0x0) /* Address of BIOS area (If TOM2 != 0, Addr >> 16) */ Name(PBLN, 0x0) /* Length of BIOS area */ @@ -1129,6 +1128,7 @@ /* Note: Only need HID on Primary Bus */ Device(PCI0) { External (TOM1) + External (TOM2) Name(_HID, EISAID("PNP0A03")) Name(_ADR, 0x00180000) /* Dev# = BSP Dev#, Func# = 0 */ Method(_BBN, 0) { /* Bus number = 0 */ Index: src/mainboard/amd/pistachio/mainboard.c =================================================================== --- src/mainboard/amd/pistachio/mainboard.c (Revision 3967) +++ src/mainboard/amd/pistachio/mainboard.c (Arbeitskopie) @@ -83,7 +83,6 @@ u16 word; u32 dword; device_t sm_dev; - struct bus pbus;
/* set adt7475 */ ADT7475_write_byte(0x40, 0x04); @@ -116,11 +115,11 @@ /* remote 2 therm temp limit (95C) */ ADT7475_write_byte(0x6c, 0x9f);
- /* PWM 1 minimum duty cycle (37%) */ + /* PWM 1 minimum duty cycle (37.6%) */ ADT7475_write_byte(0x64, 0x60); /* PWM 1 Maximum duty cycle (100%) */ ADT7475_write_byte(0x38, 0xff); - /* PWM 3 minimum duty cycle (37%) */ + /* PWM 3 minimum duty cycle (37.6%) */ ADT7475_write_byte(0x66, 0x60); /* PWM 3 Maximum Duty Cycle (100%) */ ADT7475_write_byte(0x3a, 0xff); @@ -167,28 +166,19 @@
/* GPM5 as GPIO not USB OC */ sm_dev = dev_find_slot(0, PCI_DEVFN(0x14, 0)); - dword = - pci_cf8_conf1.read32(&pbus, sm_dev->bus->secondary, - sm_dev->path.pci.devfn, 0x64); + dword = pci_read_config32(sm_dev, 0x64); dword |= 1 << 19; - pci_cf8_conf1.write32(&pbus, sm_dev->bus->secondary, - sm_dev->path.pci.devfn, 0x64, dword); + pci_write_config32(sm_dev, 0x64, dword);
/* Enable Client Management Index/Data registers */ - dword = - pci_cf8_conf1.read32(&pbus, sm_dev->bus->secondary, - sm_dev->path.pci.devfn, 0x78); + dword = pci_read_config32(sm_dev, 0x78); dword |= 1 << 11; /* Cms_enable */ - pci_cf8_conf1.write32(&pbus, sm_dev->bus->secondary, - sm_dev->path.pci.devfn, 0x78, dword); + pci_write_config32(sm_dev, 0x78, dword);
/* MiscfuncEnable */ - byte = - pci_cf8_conf1.read8(&pbus, sm_dev->bus->secondary, - sm_dev->path.pci.devfn, 0x41); + byte = pci_read_config8(sm_dev, 0x41); byte |= (1 << 5); - pci_cf8_conf1.write8(&pbus, sm_dev->bus->secondary, - sm_dev->path.pci.devfn, 0x41, byte); + pci_write_config8(sm_dev, 0x41, byte);
/* set GPM5 as input */ /* set index register 0C50h to 13h (miscellaneous control) */ @@ -228,12 +218,9 @@ pm2_iowrite(0x42, byte);
/* set GPIO 64 to input */ - word = - pci_cf8_conf1.read16(&pbus, sm_dev->bus->secondary, - sm_dev->path.pci.devfn, 0x56); + word = pci_read_config16(sm_dev, 0x56); word |= 1 << 7; - pci_cf8_conf1.write16(&pbus, sm_dev->bus->secondary, - sm_dev->path.pci.devfn, 0x56, word); + pci_write_config16(sm_dev, 0x56, word);
/* set GPIO 64 internal pull-up */ byte = pm2_ioread(0xf0); @@ -269,12 +256,12 @@ * enable the dedicated function in pistachio board. * This function called early than rs690_enable. *************************************************/ -void pistachio_enable(device_t dev) +void mb_enable(device_t dev) { struct mainboard_config *mainboard = (struct mainboard_config *)dev->chip_info;
- printk_info("Mainboard Pistachio Enable. dev=0x%x\n", dev); + printk_info("Mainboard " MAINBOARD_PART_NUMBER " Enable. dev=0x%x\n", dev);
#if (CONFIG_GFXUMA == 1) msr_t msr, msr2; @@ -339,6 +326,6 @@ * CONFIG_CHIP_NAME defined in Option.lb. */ struct chip_operations mainboard_ops = { - CHIP_NAME("AMD Pistachio Mainboard") - .enable_dev = pistachio_enable, + CHIP_NAME(MAINBOARD_VENDOR " " MAINBOARD_PART_NUMBER " Mainboard") + .enable_dev = mb_enable, }; Index: src/mainboard/amd/pistachio/cache_as_ram_auto.c =================================================================== --- src/mainboard/amd/pistachio/cache_as_ram_auto.c (Revision 3967) +++ src/mainboard/amd/pistachio/cache_as_ram_auto.c (Arbeitskopie) @@ -156,9 +156,8 @@ u32 bsp_apicid = 0; msr_t msr; struct cpuid_result cpuid1; - struct sys_info *sysinfo = - (struct sys_info *)(DCACHE_RAM_BASE + DCACHE_RAM_SIZE - - DCACHE_RAM_GLOBAL_VAR_SIZE); + struct sys_info *sysinfo = (struct sys_info *)(DCACHE_RAM_BASE + + DCACHE_RAM_SIZE - DCACHE_RAM_GLOBAL_VAR_SIZE);
if (bist == 0) { bsp_apicid = init_cpus(cpu_init_detectedx, sysinfo); @@ -178,7 +177,7 @@ report_bist_failure(bist); printk_debug("bsp_apicid=0x%x\n", bsp_apicid);
- setup_pistachio_resource_map(); + setup_mb_resource_map();
setup_coherent_ht_domain();
Index: src/mainboard/amd/dbm690t/Config.lb =================================================================== --- src/mainboard/amd/dbm690t/Config.lb (Revision 3967) +++ src/mainboard/amd/dbm690t/Config.lb (Arbeitskopie) @@ -197,7 +197,7 @@ #Define gfx_link_width, 0: x16, 1: x1, 2: x2, 3: x4, 4: x8, 5: x12 (not supported), 6: x16 chip northbridge/amd/amdk8/root_complex device apic_cluster 0 on - chip cpu/amd/socket_S1G1 + chip cpu/amd/socket_AM2 device apic 0 on end end end @@ -213,7 +213,7 @@ end end device pci 2.0 on end # PCIE P2P bridge (external graphics) 0x7913 - device pci 3.0 off end # PCIE P2P bridge 0x791b + #device pci 3.0 off end # PCIE P2P bridge 0x791b device pci 4.0 on end # PCIE P2P bridge 0x7914 device pci 5.0 on end # PCIE P2P bridge 0x7915 device pci 6.0 on end # PCIE P2P bridge 0x7916 @@ -257,9 +257,9 @@ device pci 14.3 on # LPC 0x438d chip superio/ite/it8712f device pnp 2e.0 off # Floppy - io 0x60 = 0x3f0 - irq 0x70 = 6 - drq 0x74 = 2 + #io 0x60 = 0x3f0 + #irq 0x70 = 6 + #drq 0x74 = 2 end device pnp 2e.1 on # Com1 io 0x60 = 0x3f8 Index: src/mainboard/amd/dbm690t/irq_tables.c =================================================================== --- src/mainboard/amd/dbm690t/irq_tables.c (Revision 3967) +++ src/mainboard/amd/dbm690t/irq_tables.c (Arbeitskopie) @@ -54,7 +54,7 @@ extern u8 bus_isa; extern u8 bus_rs690[8]; extern u8 bus_sb600[2]; -extern unsigned long sbdn_sb600; +extern u32 sbdn_sb600;
unsigned long write_pirq_routing_table(unsigned long addr) { @@ -73,7 +73,7 @@ addr &= ~15;
/* This table must be betweeen 0xf0000 & 0x100000 */ - printk_info("Writing IRQ routing tables to 0x%x...", addr); + printk_info("Writing IRQ routing tables to 0x%lx...", addr);
pirq = (void *)(addr); v = (u8 *) (addr); Index: src/mainboard/amd/dbm690t/resourcemap.c =================================================================== --- src/mainboard/amd/dbm690t/resourcemap.c (Revision 3967) +++ src/mainboard/amd/dbm690t/resourcemap.c (Arbeitskopie) @@ -17,7 +17,7 @@ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA */
-static void setup_dbm690t_resource_map(void) +static void setup_mb_resource_map(void) { static const unsigned int register_values[] = { /* Careful set limit registers before base registers which contain the enables */ Index: src/mainboard/amd/dbm690t/acpi_tables.c =================================================================== --- src/mainboard/amd/dbm690t/acpi_tables.c (Revision 3967) +++ src/mainboard/amd/dbm690t/acpi_tables.c (Arbeitskopie) @@ -254,7 +254,7 @@ memcpy((void *)dsdt, (void *)AmlCode, ((acpi_header_t *) AmlCode)->length); current += dsdt->length; - printk_debug("ACPI: * DSDT @ %08x Length %x\n", dsdt, dsdt->length); + printk_debug("ACPI: * DSDT @ %p Length %x\n", dsdt, dsdt->length); /* FADT */ printk_debug("ACPI: * FADT\n"); fadt = (acpi_fadt_t *) current; Index: src/mainboard/amd/dbm690t/acpi/dsdt.asl =================================================================== --- src/mainboard/amd/dbm690t/acpi/dsdt.asl (Revision 3967) +++ src/mainboard/amd/dbm690t/acpi/dsdt.asl (Arbeitskopie) @@ -19,12 +19,12 @@
/* DefinitionBlock Statement */ DefinitionBlock ( - "DSDT.AML", /* Output filename */ - "DSDT", /* Signature */ - 0x01, /* DSDT Revision */ - "AMD ", /* OEMID */ - "DBM690T ", /* TABLE ID */ - 0x00010001 /* OEM Revision */ + "DSDT.AML", /* Output filename */ + "DSDT", /* Signature */ + 0x02, /* DSDT Revision, needs to be 2 for 64bit */ + "AMD ", /* OEMID */ + "DBM690T ", /* TABLE ID */ + 0x00010001 /* OEM Revision */ ) { /* Start of ASL file */ /* Include ("debug.asl") */ /* Include global debug methods if needed */ @@ -32,7 +32,6 @@ /* Data to be patched by the BIOS during POST */ /* FIXME the patching is not done yet! */ /* Memory related values */ - Name(TOM2, 0x0) /* Top of RAM memory above 4GB (>> 16) */ Name(LOMH, 0x0) /* Start of unused memory in C0000-E0000 range */ Name(PBAD, 0x0) /* Address of BIOS area (If TOM2 != 0, Addr >> 16) */ Name(PBLN, 0x0) /* Length of BIOS area */ @@ -1130,6 +1129,7 @@ /* Note: Only need HID on Primary Bus */ Device(PCI0) { External (TOM1) + External (TOM2) Name(_HID, EISAID("PNP0A03")) Name(_ADR, 0x00180000) /* Dev# = BSP Dev#, Func# = 0 */ Method(_BBN, 0) { /* Bus number = 0 */ @@ -1476,7 +1476,7 @@ 0x0000, /* range minimum */ 0x0CF7, /* range maximum */ 0x0000, /* translation */ - 0x0CF8 /* Resource source index */ + 0x0CF8 /* length */ )
WORDIO(ResourceProducer, MinFixed, MaxFixed, PosDecode, EntireRange, @@ -1554,12 +1554,10 @@ /* Set size of memory from 1MB to TopMem */ Subtract(TOM1, 0x100000, DMLL)
- /* - * If(LNotEqual(TOM2, 0x00000000)){ - * Store(0x100000000,DMHB) DRAM from 4GB to TopMem2 - * Subtract(TOM2, 0x100000000, DMHL) - * } - */ + If(LNotEqual(TOM2, 0x00000000)){ + Store(0x100000000,DMHB) /* DRAM from 4GB to TopMem2 */ + Subtract(TOM2, 0x100000000, DMHL) + }
/* If there is no memory above 4GB, put the BIOS just below 4GB */ If(LEqual(TOM2, 0x00000000)){ Index: src/mainboard/amd/dbm690t/mainboard.c =================================================================== --- src/mainboard/amd/dbm690t/mainboard.c (Revision 3967) +++ src/mainboard/amd/dbm690t/mainboard.c (Arbeitskopie) @@ -61,7 +61,7 @@ { u8 byte;
- printk_info("enable_onboard_nic.\n"); + printk_info("%s.\n", __func__);
/* set index register 0C50h to 13h (miscellaneous control) */ outb(0x13, 0xC50); /* CMIndex */ @@ -87,6 +87,7 @@ byte = inb(0xC52); byte &= ~0x8; outb(byte, 0xC52); + printk_info("%s done.\n", __func__); }
/******************************************************** @@ -97,32 +98,36 @@ static void get_ide_dma66() { u8 byte; - /*u32 sm_dev, ide_dev; */ - device_t sm_dev, ide_dev; - struct bus pbus; + struct device *sm_dev; + struct device *ide_dev; + struct bus *pbus;
+ printk_info("%s.\n", __func__); sm_dev = dev_find_slot(0, PCI_DEVFN(0x14, 0)); + printk_info("%s dev_find_slot(0:14.0) returned sm_dev=%p, sm_dev->bus=%p.\n", __func__, sm_dev, sm_dev->bus);
- byte = - pci_cf8_conf1.read8(&pbus, sm_dev->bus->secondary, - sm_dev->path.pci.devfn, 0xA9); + printk_info("%s running get_pbus(sm_dev)\n", __func__); + pbus = get_pbus(sm_dev); + printk_info("%s pbus=%p, ops_pci_bus(pbus)=%p, &pci_cf8_conf1=%p.\n", __func__, pbus, ops_pci_bus(pbus), &pci_cf8_conf1); + if (ops_pci_bus(pbus) != &pci_cf8_conf1) + printk_info("%s ops_pci_bus(pbus) and &pci_cf8_conf1 do NOT match! Die?\n", __func__); + //byte = ops_pci_bus(pbus)->read8(pbus, sm_dev->bus->secondary, sm_dev->path.pci.devfn, 0xA9); + //byte = pci_cf8_conf1.read8(&pbus, sm_dev->bus->secondary, sm_dev->path.pci.devfn, 0xA9); + byte = pci_read_config8(sm_dev, 0xA9); + printk_info("%s survived first pci_read_config8\n", __func__); byte |= (1 << 5); /* Set Gpio9 as input */ - pci_cf8_conf1.write8(&pbus, sm_dev->bus->secondary, - sm_dev->path.pci.devfn, 0xA9, byte); + pci_write_config8(sm_dev, 0xA9, byte); + printk_info("%s survived first pci_write_config8\n", __func__);
ide_dev = dev_find_slot(0, PCI_DEVFN(0x14, 1)); - byte = - pci_cf8_conf1.read8(&pbus, ide_dev->bus->secondary, - ide_dev->path.pci.devfn, 0x56); + byte = pci_read_config8(ide_dev, 0x56); byte &= ~(7 << 0); - if ((1 << 5) & pci_cf8_conf1. - read8(&pbus, sm_dev->bus->secondary, sm_dev->path.pci.devfn, - 0xAA)) + if ((1 << 5) & pci_read_config8(sm_dev, 0xAA)) byte |= 2 << 0; /* mode 2 */ else byte |= 5 << 0; /* mode 5 */ - pci_cf8_conf1.write8(&pbus, ide_dev->bus->secondary, - ide_dev->path.pci.devfn, 0x56, byte); + pci_write_config8(ide_dev, 0x56, byte); + printk_info("%s done.\n", __func__); }
/* @@ -133,7 +138,6 @@ u8 byte; u16 word; device_t sm_dev; - struct bus pbus;
/* set ADT 7461 */ ADT7461_write_byte(0x0B, 0x50); /* Local Temperature Hight limit */ @@ -156,12 +160,9 @@
/* set GPIO 64 to input */ sm_dev = dev_find_slot(0, PCI_DEVFN(0x14, 0)); - word = - pci_cf8_conf1.read16(&pbus, sm_dev->bus->secondary, - sm_dev->path.pci.devfn, 0x56); + word = pci_read_config16(sm_dev, 0x56); word |= 1 << 7; - pci_cf8_conf1.write16(&pbus, sm_dev->bus->secondary, - sm_dev->path.pci.devfn, 0x56, word); + pci_write_config16(sm_dev, 0x56, word);
/* set GPIO 64 internal pull-up */ byte = pm2_ioread(0xf0); @@ -197,12 +198,12 @@ * enable the dedicated function in dbm690t board. * This function called early than rs690_enable. *************************************************/ -void dbm690t_enable(device_t dev) +void mb_enable(device_t dev) { struct mainboard_config *mainboard = (struct mainboard_config *)dev->chip_info;
- printk_info("Mainboard DBM690T Enable. dev=0x%x\n", dev); + printk_info("Mainboard " MAINBOARD_PART_NUMBER " Enable. dev=%p\n", dev);
#if (CONFIG_GFXUMA == 1) msr_t msr, msr2; @@ -236,7 +237,7 @@ }
uma_memory_base = msr.lo - uma_memory_size; /* TOP_MEM1 */ - printk_info("%s: uma size 0x%08lx, memory start 0x%08lx\n", + printk_info("%s: uma size 0x%08llx, memory start 0x%08llx\n", __func__, uma_memory_size, uma_memory_base);
/* TODO: TOP_MEM2 */ @@ -256,7 +257,7 @@ * in some circumstances we want the memory mentioned as reserved. */ #if (CONFIG_GFXUMA == 1) - printk_info("uma_memory_base=0x%lx, uma_memory_size=0x%lx \n", + printk_info("uma_memory_base=0x%llx, uma_memory_size=0x%llx \n", uma_memory_base, uma_memory_size); lb_add_memory_range(mem, LB_MEM_RESERVED, uma_memory_base, uma_memory_size); @@ -267,6 +268,6 @@ * CONFIG_CHIP_NAME defined in Option.lb. */ struct chip_operations mainboard_ops = { - CHIP_NAME("AMD DBM690T Mainboard") - .enable_dev = dbm690t_enable, + CHIP_NAME(MAINBOARD_VENDOR " " MAINBOARD_PART_NUMBER " Mainboard") + .enable_dev = mb_enable, }; Index: src/mainboard/amd/dbm690t/cache_as_ram_auto.c =================================================================== --- src/mainboard/amd/dbm690t/cache_as_ram_auto.c (Revision 3967) +++ src/mainboard/amd/dbm690t/cache_as_ram_auto.c (Arbeitskopie) @@ -32,6 +32,8 @@
#define DIMM0 0x50 #define DIMM1 0x51 +#define DIMM2 0x52 +#define DIMM3 0x53
#define ICS951462_ADDRESS 0x69 #define SMBUS_HUB 0x71 @@ -137,7 +139,7 @@ normal_image: post_code(0x23); __asm__ volatile ("jmp __normal_image": /* outputs */ - :"a" (bist), "b"(cpu_init_detectedx) /* inputs */); + :"a" (bist), "b"(cpu_init_detectedx)); /* inputs */
fallback_image: post_code(0x25); @@ -157,14 +159,14 @@
void real_main(unsigned long bist, unsigned long cpu_init_detectedx) { - static const u16 spd_addr[] = { DIMM0, 0, 0, 0, DIMM1, 0, 0, 0, }; + static const u16 spd_addr[] = { DIMM0, DIMM2, 0, 0, DIMM1, DIMM3, 0, 0, }; int needs_reset = 0; u32 bsp_apicid = 0; msr_t msr; struct cpuid_result cpuid1; - struct sys_info *sysinfo = (struct sys_info *)(DCACHE_RAM_BASE + DCACHE_RAM_SIZE - DCACHE_RAM_GLOBAL_VAR_SIZE); + struct sys_info *sysinfo = (struct sys_info *)(DCACHE_RAM_BASE + + DCACHE_RAM_SIZE - DCACHE_RAM_GLOBAL_VAR_SIZE);
- if (bist == 0) { bsp_apicid = init_cpus(cpu_init_detectedx, sysinfo); } @@ -181,7 +183,7 @@ report_bist_failure(bist); printk_debug("bsp_apicid=0x%x\n", bsp_apicid);
- setup_dbm690t_resource_map(); + setup_mb_resource_map();
setup_coherent_ht_domain();
Index: src/lib/malloc.c =================================================================== --- src/lib/malloc.c (Revision 3967) +++ src/lib/malloc.c (Arbeitskopie) @@ -27,7 +27,7 @@ { void *p;
- MALLOCDBG(("%s Enter, size %d, free_mem_ptr %p\n", __func__, size, free_mem_ptr)); + MALLOCDBG(("%s Enter, size %ld, free_mem_ptr 0x%08lx\n", __func__, size, free_mem_ptr)); if (size < 0) die("Error! malloc: Size < 0"); if (free_mem_ptr <= 0) Index: src/northbridge/amd/amdk8/amdk8_acpi.c =================================================================== --- src/northbridge/amd/amdk8/amdk8_acpi.c (Revision 3967) +++ src/northbridge/amd/amdk8/amdk8_acpi.c (Arbeitskopie) @@ -128,7 +128,7 @@ basek = resk(res->base); sizek = resk(res->size);
- printk_debug("set_srat_mem: dev %s, res->index=%04x startk=%08x, sizek=%08x\n", + printk_debug("set_srat_mem: dev %s, res->index=%04lx startk=%08lx, sizek=%08lx\n", dev_path(dev), res->index, basek, sizek); /* * 0-640K must be on node 0 Index: src/northbridge/amd/amdk8/raminit.c =================================================================== --- src/northbridge/amd/amdk8/raminit.c (Revision 3967) +++ src/northbridge/amd/amdk8/raminit.c (Arbeitskopie) @@ -1420,9 +1420,6 @@ die("min_cycle_time to low"); } print_spew(param->name); -#ifdef DRAM_MIN_CYCLE_TIME - print_debug(param->name); -#endif return param; }
Index: src/northbridge/amd/amdk8/northbridge.c =================================================================== --- src/northbridge/amd/amdk8/northbridge.c (Revision 3967) +++ src/northbridge/amd/amdk8/northbridge.c (Arbeitskopie) @@ -473,7 +473,7 @@ limit |= (nodeid & 7);
if (dev->link[link].bridge_ctrl & PCI_BRIDGE_CTL_VGA) { - printk_spew("%s, enabling legacy VGA IO forwarding for %s link %s\n", + printk_spew("%s, enabling legacy VGA IO forwarding for %s link 0x%x\n", __func__, dev_path(dev), link); base |= PCI_IO_BASE_VGA_EN; } Index: src/northbridge/amd/amdk8/raminit_f.c =================================================================== Index: src/arch/i386/include/arch/smp/mpspec.h =================================================================== --- src/arch/i386/include/arch/smp/mpspec.h (Revision 3967) +++ src/arch/i386/include/arch/smp/mpspec.h (Arbeitskopie) @@ -1,6 +1,8 @@ #ifndef __ASM_MPSPEC_H #define __ASM_MPSPEC_H
+#include <device/device.h> + #if HAVE_MP_TABLE==1
/* Index: targets/amd/dbm690t/Config-abuild.lb =================================================================== --- targets/amd/dbm690t/Config-abuild.lb (Revision 3967) +++ targets/amd/dbm690t/Config-abuild.lb (Arbeitskopie) @@ -7,9 +7,12 @@ option CROSS_COMPILE="CROSS_PREFIX" option HOSTCC="CROSS_HOSTCC"
+option DEFAULT_CONSOLE_LOGLEVEL = 9 +option MAXIMUM_CONSOLE_LOGLEVEL = 9 + __COMPRESSION__
-option ROM_SIZE=1024*1024 +option ROM_SIZE = 1024*1024 - 54784 romimage "normal" option USE_FALLBACK_IMAGE=0 option ROM_IMAGE_SIZE=0x20000
On Mon, Mar 2, 2009 at 6:57 AM, Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net wrote:
Hi,
Using pci_read_config8() inside mainboard.c causes a crash on the AMD DBM690T and Pistachio targets. AMD developers are seeing the crash as well and use pci_cf8_conf1.read8() instead which works fine.
Did this just start?
ron
On 02.03.2009 16:17, ron minnich wrote:
On Mon, Mar 2, 2009 at 6:57 AM, Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net wrote:
Hi,
Using pci_read_config8() inside mainboard.c causes a crash on the AMD DBM690T and Pistachio targets. AMD developers are seeing the crash as well and use pci_cf8_conf1.read8() instead which works fine.
Did this just start?
AFAIK it has been the case at least since AMD published their 690G/SB600 port, maybe even before that. For a fun time, grep over the tree for pci_cf8_conf1 and you'll see it is not constrained to mainboard code.
Regards, Carl-Daniel
On Mon, Mar 2, 2009 at 7:30 AM, Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net wrote:
AFAIK it has been the case at least since AMD published their 690G/SB600 port, maybe even before that. For a fun time, grep over the tree for pci_cf8_conf1 and you'll see it is not constrained to mainboard code.
ok, has anyone checked to see if it's the code that determines type 1/type 2 access going wrong somehow?
If so, there's an easy fix :-)
thanks
ron
On 02.03.2009 16:33, ron minnich wrote:
On Mon, Mar 2, 2009 at 7:30 AM, Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net wrote:
AFAIK it has been the case at least since AMD published their 690G/SB600 port, maybe even before that. For a fun time, grep over the tree for pci_cf8_conf1 and you'll see it is not constrained to mainboard code.
ok, has anyone checked to see if it's the code that determines type 1/type 2 access going wrong somehow?
Maybe. pci_set_method() is not called on any K8 platform. AFAICS type 1 is hardcoded there, so pci_set_method should not be necessary in theory. I don't know whether setting the access method happens early enough for the mainboard code.
If so, there's an easy fix :-)
Hopefully. Once Ward boots tomorrow with my debug patch, we'll know where exactly it hangs and probably also why.
Regards, Carl-Daniel
On 02.03.2009 16:40, Carl-Daniel Hailfinger wrote:
On 02.03.2009 16:33, ron minnich wrote:
On Mon, Mar 2, 2009 at 7:30 AM, Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net wrote:
AFAIK it has been the case at least since AMD published their 690G/SB600 port, maybe even before that. For a fun time, grep over the tree for pci_cf8_conf1 and you'll see it is not constrained to mainboard code.
ok, has anyone checked to see if it's the code that determines type 1/type 2 access going wrong somehow?
Maybe. pci_set_method() is not called on any K8 platform. AFAICS type 1 is hardcoded there, so pci_set_method should not be necessary in theory. I don't know whether setting the access method happens early enough for the mainboard code.
If so, there's an easy fix :-)
Hopefully. Once Ward boots tomorrow with my debug patch, we'll know where exactly it hangs and probably also why.
http://ward.vandewege.net/coreboot/m2a-vm/m2a-vm-with-fix-printk-format-warn...
Ouch. We hang inside get_pbus.
Ward, can you revert src/devices/pci_ops.c and apply this patch?
Index: src/devices/pci_ops.c =================================================================== --- src/devices/pci_ops.c (Revision 3967) +++ src/devices/pci_ops.c (Arbeitskopie) @@ -24,11 +24,15 @@ #include <device/pci_ids.h> #include <device/pci_ops.h>
-static struct bus *get_pbus(device_t dev) +struct bus *get_pbus(device_t dev) { + printk_spew("%s entered\n", __func__); struct bus *pbus = dev->bus; + printk_spew("%s before loop, pbus=%p, pbus->dev=%p, pbus->dev->ops=%p, pbus->children=%p\n", __func__, pbus, pbus ? pbus->dev : 0xdeadbeef, (pbus && pbus->dev) ? pbus->dev->ops : 0xdeadbeef, pbus ? pbus->children: 0xdeadbeef); while(pbus && pbus->dev && !ops_pci_bus(pbus)) { + printk_spew("%s inside loop begin, pbus=%p, pbus->dev=%p, pbus->dev->ops=%p, pbus->children=%p\n", __func__, pbus, pbus ? pbus->dev : 0xdeadbeef, (pbus && pbus->dev) ? pbus->dev->ops : 0xdeadbeef, pbus ? pbus->children: 0xdeadbeef); pbus = pbus->dev->bus; + printk_spew("%s inside loop end, pbus=%p, pbus->dev=%p, pbus->dev->ops=%p, pbus->children=%p\n", __func__, pbus, pbus ? pbus->dev : 0xdeadbeef, (pbus && pbus->dev) ? pbus->dev->ops : 0xdeadbeef, pbus ? pbus->children: 0xdeadbeef); } if (!pbus || !pbus->dev || !pbus->dev->ops || !pbus->dev->ops->ops_pci_bus) { printk_alert("%s Cannot find pci bus operations", dev_path(dev));
Regards, Carl-Daniel
On 03.03.2009 17:37, Carl-Daniel Hailfinger wrote:
On 02.03.2009 16:40, Carl-Daniel Hailfinger wrote:
On 02.03.2009 16:33, ron minnich wrote:
On Mon, Mar 2, 2009 at 7:30 AM, Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net wrote:
AFAIK it has been the case at least since AMD published their 690G/SB600 port, maybe even before that. For a fun time, grep over the tree for pci_cf8_conf1 and you'll see it is not constrained to mainboard code.
ok, has anyone checked to see if it's the code that determines type 1/type 2 access going wrong somehow?
Maybe. pci_set_method() is not called on any K8 platform. AFAICS type 1 is hardcoded there, so pci_set_method should not be necessary in theory. I don't know whether setting the access method happens early enough for the mainboard code.
If so, there's an easy fix :-)
Hopefully. Once Ward boots tomorrow with my debug patch, we'll know where exactly it hangs and probably also why.
http://ward.vandewege.net/coreboot/m2a-vm/m2a-vm-with-fix-printk-format-warn...
Ouch. We hang inside get_pbus.
Ward, can you revert src/devices/pci_ops.c again and apply this patch?
Index: src/devices/pci_ops.c =================================================================== --- src/devices/pci_ops.c (Revision 3967) +++ src/devices/pci_ops.c (Arbeitskopie) @@ -24,11 +24,19 @@ #include <device/pci_ids.h> #include <device/pci_ops.h>
-static struct bus *get_pbus(device_t dev) +struct bus *get_pbus(device_t dev) { + printk_spew("%s entered\n", __func__); struct bus *pbus = dev->bus; + printk_spew("%s before loop, pbus=%p, pbus->dev=%p, pbus->dev->ops=%p, pbus->children=%p\n", __func__, pbus, pbus ? pbus->dev : 0xdeadbeef, (pbus && pbus->dev) ? pbus->dev->ops : 0xdeadbeef, pbus ? pbus->children: 0xdeadbeef); while(pbus && pbus->dev && !ops_pci_bus(pbus)) { + printk_spew("%s inside loop begin, pbus=%p, pbus->dev=%p, pbus->dev->ops=%p, pbus->children=%p\n", __func__, pbus, pbus ? pbus->dev : 0xdeadbeef, (pbus && pbus->dev) ? pbus->dev->ops : 0xdeadbeef, pbus ? pbus->children: 0xdeadbeef); + if (pbus == pbus->dev->bus) { + printk_alert("%s stuck in endless loop for %s, breaking out\n", __func__, dev_path(dev)); + break; + } pbus = pbus->dev->bus; + printk_spew("%s inside loop end, pbus=%p, pbus->dev=%p, pbus->dev->ops=%p, pbus->children=%p\n", __func__, pbus, pbus ? pbus->dev : 0xdeadbeef, (pbus && pbus->dev) ? pbus->dev->ops : 0xdeadbeef, pbus ? pbus->children: 0xdeadbeef); } if (!pbus || !pbus->dev || !pbus->dev->ops || !pbus->dev->ops->ops_pci_bus) { printk_alert("%s Cannot find pci bus operations", dev_path(dev));
Regards, Carl-Daniel
I think I see the problem. The mainboard.c code is called during the chip_operations enable_dev stage very early before the static tree is setup with the device function pointers. The reason for this is to do chip device setup before the devices are scanned. For example the device may need to be enabled in the southbridge before it will be found in the normal scan or it may need to be hidden etc.
If you look at static.c you will see that the .ops = 0 for all the devices. That field gets updated in set_pci_ops() (or other device ops setup functions) which happens later in the device scan, after the chip enable_dev.
That is why we need to use the direct access pci functions and not the static tree pointer functions. I think that the fix is to add a comment explaining why you need to use the direct access functions at in the enable_dev function.
I hope that was clear. It is a bit easier to understand in V3 where the stages/phases are numbered.
Marc
On 03.03.2009 19:22, Marc Jones wrote:
I think I see the problem. The mainboard.c code is called during the chip_operations enable_dev stage very early before the static tree is setup with the device function pointers. The reason for this is to do chip device setup before the devices are scanned. For example the device may need to be enabled in the southbridge before it will be found in the normal scan or it may need to be hidden etc.
If you look at static.c you will see that the .ops = 0 for all the devices. That field gets updated in set_pci_ops() (or other device ops setup functions) which happens later in the device scan, after the chip enable_dev.
That is why we need to use the direct access pci functions and not the static tree pointer functions. I think that the fix is to add a comment explaining why you need to use the direct access functions at in the enable_dev function.
I hope that was clear. It is a bit easier to understand in V3 where the stages/phases are numbered.
Thanks. I still want to work around this.
Ward, can you please revert src/mainboard/amd/dbm690t/mainboard.c and try this?
Index: src/mainboard/amd/dbm690t/mainboard.c =================================================================== --- src/mainboard/amd/dbm690t/mainboard.c (Revision 3967) +++ src/mainboard/amd/dbm690t/mainboard.c (Arbeitskopie) @@ -61,7 +61,7 @@ { u8 byte;
- printk_info("enable_onboard_nic.\n"); + printk_info("%s.\n", __func__);
/* set index register 0C50h to 13h (miscellaneous control) */ outb(0x13, 0xC50); /* CMIndex */ @@ -87,6 +87,7 @@ byte = inb(0xC52); byte &= ~0x8; outb(byte, 0xC52); + printk_info("%s done.\n", __func__); }
/******************************************************** @@ -97,32 +98,47 @@ static void get_ide_dma66() { u8 byte; - /*u32 sm_dev, ide_dev; */ - device_t sm_dev, ide_dev; - struct bus pbus; + struct device *sm_dev; + struct device *ide_dev; + struct bus *pbus;
+ printk_info("%s.\n", __func__); sm_dev = dev_find_slot(0, PCI_DEVFN(0x14, 0)); + printk_info("%s dev_find_slot(0:14.0) returned sm_dev=%p, sm_dev->bus=%p.\n", __func__, sm_dev, sm_dev->bus);
- byte = - pci_cf8_conf1.read8(&pbus, sm_dev->bus->secondary, - sm_dev->path.pci.devfn, 0xA9); + pbus = dev->bus; + while(pbus && pbus->dev && !ops_pci_bus(pbus)) { + if (pbus == pbus->dev->bus) + break; + pbus = pbus->dev->bus; + } + if (pbus && pbus->dev && pbus->dev->ops) { + printk_info("%s fixing up root bus pci ops\n"); + bus->dev->ops->ops_pci_bus = &pci_cf8_conf1; + } + + printk_info("%s running get_pbus(sm_dev)\n", __func__); + pbus = get_pbus(sm_dev); + printk_info("%s pbus=%p, ops_pci_bus(pbus)=%p, &pci_cf8_conf1=%p.\n", __func__, pbus, ops_pci_bus(pbus), &pci_cf8_conf1); + if (ops_pci_bus(pbus) != &pci_cf8_conf1) + printk_info("%s ops_pci_bus(pbus) and &pci_cf8_conf1 do NOT match! Die?\n", __func__); + //byte = ops_pci_bus(pbus)->read8(pbus, sm_dev->bus->secondary, sm_dev->path.pci.devfn, 0xA9); + //byte = pci_cf8_conf1.read8(&pbus, sm_dev->bus->secondary, sm_dev->path.pci.devfn, 0xA9); + byte = pci_read_config8(sm_dev, 0xA9); + printk_info("%s survived first pci_read_config8\n", __func__); byte |= (1 << 5); /* Set Gpio9 as input */ - pci_cf8_conf1.write8(&pbus, sm_dev->bus->secondary, - sm_dev->path.pci.devfn, 0xA9, byte); + pci_write_config8(sm_dev, 0xA9, byte); + printk_info("%s survived first pci_write_config8\n", __func__);
ide_dev = dev_find_slot(0, PCI_DEVFN(0x14, 1)); - byte = - pci_cf8_conf1.read8(&pbus, ide_dev->bus->secondary, - ide_dev->path.pci.devfn, 0x56); + byte = pci_read_config8(ide_dev, 0x56); byte &= ~(7 << 0); - if ((1 << 5) & pci_cf8_conf1. - read8(&pbus, sm_dev->bus->secondary, sm_dev->path.pci.devfn, - 0xAA)) + if ((1 << 5) & pci_read_config8(sm_dev, 0xAA)) byte |= 2 << 0; /* mode 2 */ else byte |= 5 << 0; /* mode 5 */ - pci_cf8_conf1.write8(&pbus, ide_dev->bus->secondary, - ide_dev->path.pci.devfn, 0x56, byte); + pci_write_config8(ide_dev, 0x56, byte); + printk_info("%s done.\n", __func__); }
/* @@ -133,7 +149,6 @@ u8 byte; u16 word; device_t sm_dev; - struct bus pbus;
/* set ADT 7461 */ ADT7461_write_byte(0x0B, 0x50); /* Local Temperature Hight limit */ @@ -156,12 +171,9 @@
/* set GPIO 64 to input */ sm_dev = dev_find_slot(0, PCI_DEVFN(0x14, 0)); - word = - pci_cf8_conf1.read16(&pbus, sm_dev->bus->secondary, - sm_dev->path.pci.devfn, 0x56); + word = pci_read_config16(sm_dev, 0x56); word |= 1 << 7; - pci_cf8_conf1.write16(&pbus, sm_dev->bus->secondary, - sm_dev->path.pci.devfn, 0x56, word); + pci_write_config16(sm_dev, 0x56, word);
/* set GPIO 64 internal pull-up */ byte = pm2_ioread(0xf0); @@ -197,12 +209,12 @@ * enable the dedicated function in dbm690t board. * This function called early than rs690_enable. *************************************************/ -void dbm690t_enable(device_t dev) +void mb_enable(device_t dev) { struct mainboard_config *mainboard = (struct mainboard_config *)dev->chip_info;
- printk_info("Mainboard DBM690T Enable. dev=0x%x\n", dev); + printk_info("Mainboard " MAINBOARD_PART_NUMBER " Enable. dev=%p\n", dev);
#if (CONFIG_GFXUMA == 1) msr_t msr, msr2; @@ -236,7 +248,7 @@ }
uma_memory_base = msr.lo - uma_memory_size; /* TOP_MEM1 */ - printk_info("%s: uma size 0x%08lx, memory start 0x%08lx\n", + printk_info("%s: uma size 0x%08llx, memory start 0x%08llx\n", __func__, uma_memory_size, uma_memory_base);
/* TODO: TOP_MEM2 */ @@ -256,7 +268,7 @@ * in some circumstances we want the memory mentioned as reserved. */ #if (CONFIG_GFXUMA == 1) - printk_info("uma_memory_base=0x%lx, uma_memory_size=0x%lx \n", + printk_info("uma_memory_base=0x%llx, uma_memory_size=0x%llx \n", uma_memory_base, uma_memory_size); lb_add_memory_range(mem, LB_MEM_RESERVED, uma_memory_base, uma_memory_size); @@ -267,6 +279,6 @@ * CONFIG_CHIP_NAME defined in Option.lb. */ struct chip_operations mainboard_ops = { - CHIP_NAME("AMD DBM690T Mainboard") - .enable_dev = dbm690t_enable, + CHIP_NAME(MAINBOARD_VENDOR " " MAINBOARD_PART_NUMBER " Mainboard") + .enable_dev = mb_enable, };
Regards, Carl-Daniel
On 03.03.2009 23:29, Carl-Daniel Hailfinger wrote:
On 03.03.2009 19:22, Marc Jones wrote:
I think I see the problem. The mainboard.c code is called during the chip_operations enable_dev stage very early before the static tree is setup with the device function pointers. The reason for this is to do chip device setup before the devices are scanned. For example the device may need to be enabled in the southbridge before it will be found in the normal scan or it may need to be hidden etc.
If you look at static.c you will see that the .ops = 0 for all the devices. That field gets updated in set_pci_ops() (or other device ops setup functions) which happens later in the device scan, after the chip enable_dev.
That is why we need to use the direct access pci functions and not the static tree pointer functions. I think that the fix is to add a comment explaining why you need to use the direct access functions at in the enable_dev function.
I hope that was clear. It is a bit easier to understand in V3 where the stages/phases are numbered.
Thanks. I still want to work around this.
Ward, can you please revert src/mainboard/amd/dbm690t/mainboard.c and try this?
Argh. I sent the patch too early.
Index: src/mainboard/amd/dbm690t/mainboard.c =================================================================== --- src/mainboard/amd/dbm690t/mainboard.c (Revision 3967) +++ src/mainboard/amd/dbm690t/mainboard.c (Arbeitskopie) @@ -61,7 +61,7 @@ { u8 byte;
- printk_info("enable_onboard_nic.\n"); + printk_info("%s.\n", __func__);
/* set index register 0C50h to 13h (miscellaneous control) */ outb(0x13, 0xC50); /* CMIndex */ @@ -87,6 +87,7 @@ byte = inb(0xC52); byte &= ~0x8; outb(byte, 0xC52); + printk_info("%s done.\n", __func__); }
/******************************************************** @@ -97,32 +98,47 @@ static void get_ide_dma66() { u8 byte; - /*u32 sm_dev, ide_dev; */ - device_t sm_dev, ide_dev; - struct bus pbus; + struct device *sm_dev; + struct device *ide_dev; + struct bus *pbus;
+ printk_info("%s.\n", __func__); sm_dev = dev_find_slot(0, PCI_DEVFN(0x14, 0)); + printk_info("%s dev_find_slot(0:14.0) returned sm_dev=%p, sm_dev->bus=%p.\n", __func__, sm_dev, sm_dev->bus);
- byte = - pci_cf8_conf1.read8(&pbus, sm_dev->bus->secondary, - sm_dev->path.pci.devfn, 0xA9); + pbus = sm_dev->bus; + while(pbus && pbus->dev && !ops_pci_bus(pbus)) { + if (pbus == pbus->dev->bus) + break; + pbus = pbus->dev->bus; + } + if (pbus && pbus->dev && pbus->dev->ops) { + printk_info("%s fixing up root bus pci ops\n", __func__); + pbus->dev->ops->ops_pci_bus = &pci_cf8_conf1; + } + + printk_info("%s running get_pbus(sm_dev)\n", __func__); + pbus = get_pbus(sm_dev); + printk_info("%s pbus=%p, ops_pci_bus(pbus)=%p, &pci_cf8_conf1=%p.\n", __func__, pbus, ops_pci_bus(pbus), &pci_cf8_conf1); + if (ops_pci_bus(pbus) != &pci_cf8_conf1) + printk_info("%s ops_pci_bus(pbus) and &pci_cf8_conf1 do NOT match! Die?\n", __func__); + //byte = ops_pci_bus(pbus)->read8(pbus, sm_dev->bus->secondary, sm_dev->path.pci.devfn, 0xA9); + //byte = pci_cf8_conf1.read8(&pbus, sm_dev->bus->secondary, sm_dev->path.pci.devfn, 0xA9); + byte = pci_read_config8(sm_dev, 0xA9); + printk_info("%s survived first pci_read_config8\n", __func__); byte |= (1 << 5); /* Set Gpio9 as input */ - pci_cf8_conf1.write8(&pbus, sm_dev->bus->secondary, - sm_dev->path.pci.devfn, 0xA9, byte); + pci_write_config8(sm_dev, 0xA9, byte); + printk_info("%s survived first pci_write_config8\n", __func__);
ide_dev = dev_find_slot(0, PCI_DEVFN(0x14, 1)); - byte = - pci_cf8_conf1.read8(&pbus, ide_dev->bus->secondary, - ide_dev->path.pci.devfn, 0x56); + byte = pci_read_config8(ide_dev, 0x56); byte &= ~(7 << 0); - if ((1 << 5) & pci_cf8_conf1. - read8(&pbus, sm_dev->bus->secondary, sm_dev->path.pci.devfn, - 0xAA)) + if ((1 << 5) & pci_read_config8(sm_dev, 0xAA)) byte |= 2 << 0; /* mode 2 */ else byte |= 5 << 0; /* mode 5 */ - pci_cf8_conf1.write8(&pbus, ide_dev->bus->secondary, - ide_dev->path.pci.devfn, 0x56, byte); + pci_write_config8(ide_dev, 0x56, byte); + printk_info("%s done.\n", __func__); }
/* @@ -133,7 +149,6 @@ u8 byte; u16 word; device_t sm_dev; - struct bus pbus;
/* set ADT 7461 */ ADT7461_write_byte(0x0B, 0x50); /* Local Temperature Hight limit */ @@ -156,12 +171,9 @@
/* set GPIO 64 to input */ sm_dev = dev_find_slot(0, PCI_DEVFN(0x14, 0)); - word = - pci_cf8_conf1.read16(&pbus, sm_dev->bus->secondary, - sm_dev->path.pci.devfn, 0x56); + word = pci_read_config16(sm_dev, 0x56); word |= 1 << 7; - pci_cf8_conf1.write16(&pbus, sm_dev->bus->secondary, - sm_dev->path.pci.devfn, 0x56, word); + pci_write_config16(sm_dev, 0x56, word);
/* set GPIO 64 internal pull-up */ byte = pm2_ioread(0xf0); @@ -197,12 +209,12 @@ * enable the dedicated function in dbm690t board. * This function called early than rs690_enable. *************************************************/ -void dbm690t_enable(device_t dev) +void mb_enable(device_t dev) { struct mainboard_config *mainboard = (struct mainboard_config *)dev->chip_info;
- printk_info("Mainboard DBM690T Enable. dev=0x%x\n", dev); + printk_info("Mainboard " MAINBOARD_PART_NUMBER " Enable. dev=%p\n", dev);
#if (CONFIG_GFXUMA == 1) msr_t msr, msr2; @@ -236,7 +248,7 @@ }
uma_memory_base = msr.lo - uma_memory_size; /* TOP_MEM1 */ - printk_info("%s: uma size 0x%08lx, memory start 0x%08lx\n", + printk_info("%s: uma size 0x%08llx, memory start 0x%08llx\n", __func__, uma_memory_size, uma_memory_base);
/* TODO: TOP_MEM2 */ @@ -256,7 +268,7 @@ * in some circumstances we want the memory mentioned as reserved. */ #if (CONFIG_GFXUMA == 1) - printk_info("uma_memory_base=0x%lx, uma_memory_size=0x%lx \n", + printk_info("uma_memory_base=0x%llx, uma_memory_size=0x%llx \n", uma_memory_base, uma_memory_size); lb_add_memory_range(mem, LB_MEM_RESERVED, uma_memory_base, uma_memory_size); @@ -267,6 +279,6 @@ * CONFIG_CHIP_NAME defined in Option.lb. */ struct chip_operations mainboard_ops = { - CHIP_NAME("AMD DBM690T Mainboard") - .enable_dev = dbm690t_enable, + CHIP_NAME(MAINBOARD_VENDOR " " MAINBOARD_PART_NUMBER " Mainboard") + .enable_dev = mb_enable, };
On 03.03.2009 23:33, Carl-Daniel Hailfinger wrote:
On 03.03.2009 23:29, Carl-Daniel Hailfinger wrote:
On 03.03.2009 19:22, Marc Jones wrote:
I think I see the problem. The mainboard.c code is called during the chip_operations enable_dev stage very early before the static tree is setup with the device function pointers. The reason for this is to do chip device setup before the devices are scanned. For example the device may need to be enabled in the southbridge before it will be found in the normal scan or it may need to be hidden etc.
If you look at static.c you will see that the .ops = 0 for all the devices. That field gets updated in set_pci_ops() (or other device ops setup functions) which happens later in the device scan, after the chip enable_dev.
That is why we need to use the direct access pci functions and not the static tree pointer functions. I think that the fix is to add a comment explaining why you need to use the direct access functions at in the enable_dev function.
I hope that was clear. It is a bit easier to understand in V3 where the stages/phases are numbered.
Thanks. I still want to work around this.
Ward, can you please revert src/mainboard/amd/dbm690t/mainboard.c and try this?
Index: src/mainboard/amd/dbm690t/mainboard.c
--- src/mainboard/amd/dbm690t/mainboard.c (Revision 3967) +++ src/mainboard/amd/dbm690t/mainboard.c (Arbeitskopie)
Ward managed to boot his M2A-VM with this patch. Thanks for your patience, Ward! Boot log is here: http://ward.vandewege.net/coreboot/m2a-vm/m2a-vm-with-fix-printk-format-warn...
Separate patches will follow.
Regards, Carl-Daniel
On 03.03.2009 19:22, Marc Jones wrote:
I think I see the problem. The mainboard.c code is called during the chip_operations enable_dev stage very early before the static tree is setup with the device function pointers. The reason for this is to do chip device setup before the devices are scanned. For example the device may need to be enabled in the southbridge before it will be found in the normal scan or it may need to be hidden etc.
If you look at static.c you will see that the .ops = 0 for all the devices. That field gets updated in set_pci_ops() (or other device ops setup functions) which happens later in the device scan, after the chip enable_dev.
That is why we need to use the direct access pci functions and not the static tree pointer functions. I think that the fix is to add a comment explaining why you need to use the direct access functions at in the enable_dev function.
To be honest, I prefer a fix which is not an easily overlooked "don't do this" comment.
I hope that was clear. It is a bit easier to understand in V3 where the stages/phases are numbered.
Thanks for your explanation.
A lot (all?) of the DBM690T mainboard setup code does not muck with device visibility or bus setup and could easily be called at a time when the static device tree has been prepared sufficiently well.
Until someone can tell me how to call the mainboard specific setup code at a more opportune time, the following bandaid works well enough to boot without problems.
Signed-off-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net
Index: LinuxBIOSv2-asus_m2a-vm/src/mainboard/amd/dbm690t/mainboard.c =================================================================== --- LinuxBIOSv2-asus_m2a-vm/src/mainboard/amd/dbm690t/mainboard.c (Revision 3967) +++ LinuxBIOSv2-asus_m2a-vm/src/mainboard/amd/dbm690t/mainboard.c (Arbeitskopie) @@ -89,6 +89,31 @@ outb(byte, 0xC52); }
+/* + * This is a totally gross hack to be able to use pci_{read,write}_config* + * early during boot when the device tree is not yet set up completely. + */ +void devicetree_early_fixup(struct device *dev) +{ + struct bus *pbus = dev->bus; + while(pbus && pbus->dev && !ops_pci_bus(pbus)) { + if (pbus == pbus->dev->bus) + break; + pbus = pbus->dev->bus; + } + if (ops_pci_bus(pbus)) { + printk_info("%s not needed\n", __func__); + return; + } + if (pbus && pbus->dev && pbus->dev->ops) { + printk_info("%s fixing up root bus pci ops\n", __func__); + pbus->dev->ops->ops_pci_bus = &pci_cf8_conf1; + return; + } + printk_info("%s failed\n", __func__); + return; +} + /******************************************************** * dbm690t uses SB600 GPIO9 to detect IDE_DMA66. * IDE_DMA66 is routed to GPIO 9. So we read Gpio 9 to @@ -97,32 +122,25 @@ static void get_ide_dma66() { u8 byte; - /*u32 sm_dev, ide_dev; */ - device_t sm_dev, ide_dev; - struct bus pbus; + struct device *sm_dev; + struct device *ide_dev;
+ printk_info("%s.\n", __func__); sm_dev = dev_find_slot(0, PCI_DEVFN(0x14, 0)); + devicetree_early_fixup(sm_dev);
- byte = - pci_cf8_conf1.read8(&pbus, sm_dev->bus->secondary, - sm_dev->path.pci.devfn, 0xA9); + byte = pci_read_config8(sm_dev, 0xA9); byte |= (1 << 5); /* Set Gpio9 as input */ - pci_cf8_conf1.write8(&pbus, sm_dev->bus->secondary, - sm_dev->path.pci.devfn, 0xA9, byte); + pci_write_config8(sm_dev, 0xA9, byte);
ide_dev = dev_find_slot(0, PCI_DEVFN(0x14, 1)); - byte = - pci_cf8_conf1.read8(&pbus, ide_dev->bus->secondary, - ide_dev->path.pci.devfn, 0x56); + byte = pci_read_config8(ide_dev, 0x56); byte &= ~(7 << 0); - if ((1 << 5) & pci_cf8_conf1. - read8(&pbus, sm_dev->bus->secondary, sm_dev->path.pci.devfn, - 0xAA)) + if ((1 << 5) & pci_read_config8(sm_dev, 0xAA)) byte |= 2 << 0; /* mode 2 */ else byte |= 5 << 0; /* mode 5 */ - pci_cf8_conf1.write8(&pbus, ide_dev->bus->secondary, - ide_dev->path.pci.devfn, 0x56, byte); + pci_write_config8(ide_dev, 0x56, byte); }
/* @@ -133,7 +151,6 @@ u8 byte; u16 word; device_t sm_dev; - struct bus pbus;
/* set ADT 7461 */ ADT7461_write_byte(0x0B, 0x50); /* Local Temperature Hight limit */ @@ -156,12 +173,9 @@
/* set GPIO 64 to input */ sm_dev = dev_find_slot(0, PCI_DEVFN(0x14, 0)); - word = - pci_cf8_conf1.read16(&pbus, sm_dev->bus->secondary, - sm_dev->path.pci.devfn, 0x56); + word = pci_read_config16(sm_dev, 0x56); word |= 1 << 7; - pci_cf8_conf1.write16(&pbus, sm_dev->bus->secondary, - sm_dev->path.pci.devfn, 0x56, word); + pci_write_config16(sm_dev, 0x56, word);
/* set GPIO 64 internal pull-up */ byte = pm2_ioread(0xf0);
--- LinuxBIOSv2-asus_m2a-vm/src/mainboard/amd/dbm690t/mainboard.c (Revision 3967) +++ LinuxBIOSv2-asus_m2a-vm/src/mainboard/amd/dbm690t/mainboard.c (Arbeitskopie) @@ -89,6 +89,31 @@ outb(byte, 0xC52); }
+/* + * This is a totally gross hack to be able to use pci_{read,write}_config* + * early during boot when the device tree is not yet set up completely. + */ +void devicetree_early_fixup(struct device *dev) +{ + struct bus *pbus = dev->bus; + while(pbus && pbus->dev && !ops_pci_bus(pbus)) { + if (pbus == pbus->dev->bus) + break; + pbus = pbus->dev->bus; + } + if (ops_pci_bus(pbus)) { + printk_info("%s not needed\n", __func__); + return; + } + if (pbus && pbus->dev && pbus->dev->ops) { + printk_info("%s fixing up root bus pci ops\n", __func__); + pbus->dev->ops->ops_pci_bus = &pci_cf8_conf1; + return; + } + printk_info("%s failed\n", __func__); + return; +}
There has to be a better way to do this than a per-mainboard fix.
ron
I like the pbus fail nicely patch but I don't think that this device tree change in the mainboard code is a good idea.
Mainboard specific configuration is one area that coreboot doesn't handle very well. I think that the UMA setup is the only part that needs to be done before pci discovery. It would be ideal if we had a mainboard hook after discovery and before setup and enable. That probably isn't viable for v2. Maybe we should look at adding a mainboard device instead of chip to v3.
Marc
On Thu, Mar 5, 2009 at 7:32 AM, Marc Jones marcj303@gmail.com wrote:
Mainboard specific configuration is one area that coreboot doesn't handle very well. I think that the UMA setup is the only part that needs to be done before pci discovery. It would be ideal if we had a mainboard hook after discovery and before setup and enable. That probably isn't viable for v2. Maybe we should look at adding a mainboard device instead of chip to v3.
good idea, not sure what it would look like, but I'd love to see a suggestion.
ron
-
Carl-Daniel Hailfinger -
Marc Jones -
ron minnich