| Submitter | Carl-Daniel Hailfinger |
|---|---|
| Date | 2009-08-27 23:28:53 |
| Message ID | <4A9716B5.1000503@gmx.net> |
| Download | mbox | patch |
| Permalink | /patch/214/ |
| State | Accepted |
| Commit | r4646 |
| Headers | show |
Comments
On 28.08.2009 01:28, Carl-Daniel Hailfinger wrote: > Until r4340, any usage of pci_{read,write}_config{8,16,32} in > coreboot_ram before the device tree was set up resulted in either a > silent hang or a NULL pointer dereference. I changed the code in r4340 > to die() properly with a loud error message. That still was not perfect, > but at least it allowed people to see why their new ports died. > Still, die() is not something developers like to see, and thus a patch > to automatically pick a sensible default instead of dying was created. > Of course, handling PCI access method selection automatically for > fallback purposes has certain limitations before the device tree is set > up. We only check if conf1 works and use conf2 as fallback. No further > tests are done. > > This patch enables cleanups and readability improvements in early > coreboot_ram code: > Without this patch: > dword = pci_cf8_conf1.read32(&pbus, sm_dev->bus->secondary, > sm_dev->path.pci.devfn, 0x64); > With this patch: > dword = pci_read_config32(sm_dev, 0x64); > The advantage is obvious. > > Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@gmx.net> > Peter asked for a short summary what the patch does. This it it: If no pci access method has been set for the device tree so far (e.g. during early coreboot_ram), pci_{read,write}_config{8,16,32} will die(). This patch changes pci_{read,write}_config{8,16,32} to use the existing PCI access method autodetection infrastructure instead of die()ing. Acked-by: Peter Stuge <peter@stuge.se> and committed in r4646. Regards, Carl-Daniel
Patch
Index: LinuxBIOSv2-asus_m2a-vm/src/devices/pci_ops.c =================================================================== --- LinuxBIOSv2-asus_m2a-vm/src/devices/pci_ops.c (Revision 4604) +++ LinuxBIOSv2-asus_m2a-vm/src/devices/pci_ops.c (Arbeitskopie) @@ -25,6 +25,9 @@ #include <device/pci_ids.h> #include <device/pci_ops.h> +/* The only consumer of the return value of get_pbus() is ops_pci_bus(). + * ops_pci_bus() can handle being passed NULL and auto-picks working ops. + */ static struct bus *get_pbus(device_t dev) { struct bus *pbus = NULL; @@ -44,8 +47,9 @@ pbus = pbus->dev->bus; } if (!pbus || !pbus->dev || !pbus->dev->ops || !pbus->dev->ops->ops_pci_bus) { - printk_emerg("%s: Cannot find pci bus operations.\n", dev_path(dev)); - die(""); + /* This can happen before the device tree is set up completely. */ + //printk_emerg("%s: Cannot find pci bus operations.\n", dev_path(dev)); + pbus = NULL; } return pbus; } Index: LinuxBIOSv2-asus_m2a-vm/src/include/device/pci_ops.h =================================================================== --- LinuxBIOSv2-asus_m2a-vm/src/include/device/pci_ops.h (Revision 4604) +++ LinuxBIOSv2-asus_m2a-vm/src/include/device/pci_ops.h (Arbeitskopie) @@ -21,4 +21,7 @@ void pci_mmio_write_config32(device_t dev, unsigned where, uint32_t val); #endif +/* This function lives in pci_ops_auto.c */ +const struct pci_bus_operations *pci_remember_direct(void); + #endif /* PCI_OPS_H */ Index: LinuxBIOSv2-asus_m2a-vm/src/include/device/pci.h =================================================================== --- LinuxBIOSv2-asus_m2a-vm/src/include/device/pci.h (Revision 4604) +++ LinuxBIOSv2-asus_m2a-vm/src/include/device/pci.h (Arbeitskopie) @@ -97,6 +97,8 @@ if (bus && bus->dev && bus->dev->ops) { bops = bus->dev->ops->ops_pci_bus; } + if (!bops) + bops = pci_remember_direct(); return bops; } Index: LinuxBIOSv2-asus_m2a-vm/src/arch/i386/lib/pci_ops_auto.c =================================================================== --- LinuxBIOSv2-asus_m2a-vm/src/arch/i386/lib/pci_ops_auto.c (Revision 4604) +++ LinuxBIOSv2-asus_m2a-vm/src/arch/i386/lib/pci_ops_auto.c (Arbeitskopie) @@ -41,6 +41,8 @@ return 0; } +struct pci_bus_operations *pci_bus_fallback_ops = NULL; + const struct pci_bus_operations *pci_check_direct(void) { unsigned int tmp; @@ -81,11 +83,18 @@ return NULL; } +const struct pci_bus_operations *pci_remember_direct(void) +{ + if (!pci_bus_fallback_ops) + pci_bus_fallback_ops = pci_check_direct(); + return pci_bus_fallback_ops; +} + /** Set the method to be used for PCI, type I or type II */ void pci_set_method(device_t dev) { printk_info("Finding PCI configuration type.\n"); - dev->ops->ops_pci_bus = pci_check_direct(); + dev->ops->ops_pci_bus = pci_remember_direct(); post_code(0x5f); }
Until r4340, any usage of pci_{read,write}_config{8,16,32} in coreboot_ram before the device tree was set up resulted in either a silent hang or a NULL pointer dereference. I changed the code in r4340 to die() properly with a loud error message. That still was not perfect, but at least it allowed people to see why their new ports died. Still, die() is not something developers like to see, and thus a patch to automatically pick a sensible default instead of dying was created. Of course, handling PCI access method selection automatically for fallback purposes has certain limitations before the device tree is set up. We only check if conf1 works and use conf2 as fallback. No further tests are done. This patch enables cleanups and readability improvements in early coreboot_ram code: Without this patch: dword = pci_cf8_conf1.read32(&pbus, sm_dev->bus->secondary, sm_dev->path.pci.devfn, 0x64); With this patch: dword = pci_read_config32(sm_dev, 0x64); The advantage is obvious. Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@gmx.net>