Re: [coreboot] pci_read_config8 crash in mainboard.c

[Carl-Daniel Hailfinger]

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

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);