[coreboot] pci_read_config8 crash in mainboard.c
Carl-Daniel Hailfinger
c-d.hailfinger.devel.2006 at gmx.net
Tue Mar 3 23:33:54 CET 2009
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,
};
--
http://www.hailfinger.org/
More information about the coreboot
mailing list