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