Re: MediaGX sound freezes 2.3.99-pre

From: Alan Curry (pacman-kernel@cqc.com)
Date: Sat Jul 22 2000 - 03:48:37 EST


Alessandro Zummo writes the following:
>sorry, not so much time to work on it. have you tried to snoop out
>the differences between .12 and .13 that could cause the
>system to hang?

OK, I've narrowed it down to a few small fragments of the patch, most of
which deal with the addition of a "struct resource". Getting rid of that
struct seems to be the key to making my sound work. I've configured out all
drivers except IDE (my root partition) and SB, to avoid having to track this
struct resource change in a hundred places.

Here's the diff that breaks my sound. I don't know what to make of it,
considering it doesn't touch drivers/sound at all. It's getting to the point
that I can't break it down much further without getting compile errors. I
hope it's small enough that someone can look it over and come up with a
theory on why it would cause lockups. This diff applies cleanly to 2.3.13 if
applied -R (reverse).

I'm open to the possibility that it's a hardware flaw, but if it is a
hardware flaw, all the 2.0 and 2.2 kernels dealt with a lot better...

diff -ruN linux/arch/i386/kernel/bios32.c linuxcp/arch/i386/kernel/bios32.c
--- linux/arch/i386/kernel/bios32.c Sat Jul 22 02:09:55 2000
+++ linuxcp/arch/i386/kernel/bios32.c Sat Jul 22 02:07:02 2000
@@ -907,12 +907,17 @@
                 if ((try & PCI_BASE_ADDRESS_IO_MASK) != addr) {
                         addr = 0;
                         printk("PCI: Address setup failed, got %04x\n", try);
- } else
- dev->base_address[idx] = try;
+ } else {
+ struct resource *res = dev->resource + idx;
+ res->start = addr;
+ res->end = addr + size - 1;
+ res->flags |= PCI_BASE_ADDRESS_IO_MASK;
+ }
         }
         if (!addr) {
                 pcibios_write_config_dword(bus, devfn, reg, 0);
- dev->base_address[idx] = 0;
+ dev->resource[idx].start = 0;
+ dev->resource[idx].flags = 0;
         }
         pcibios_write_config_word(bus, devfn, PCI_COMMAND, cmd);
 }
@@ -939,7 +944,7 @@
                             e->vendor == d->vendor &&
                             e->device == d->device &&
                             e->class == d->class &&
- !memcmp(e->base_address, d->base_address, sizeof(e->base_address)))
+ !memcmp(e->resource, d->resource, sizeof(e->resource)))
                                 break;
                 if (!e)
                         return;
@@ -1060,7 +1065,7 @@
         int i;
 
         for(i=0; i<4; i++)
- d->base_address[i] |= PCI_BASE_ADDRESS_SPACE_IO;
+ d->resource[i].flags |= PCI_BASE_ADDRESS_SPACE_IO;
 }
 
 struct dev_ex {
@@ -1112,11 +1117,12 @@
                  */
                 has_io = has_mem = 0;
                 for(i=0; i<6; i++) {
- unsigned long a = dev->base_address[i];
+ struct resource *res = dev->resource + i;
+ unsigned long a = res->flags;
                         if (a & PCI_BASE_ADDRESS_SPACE_IO) {
+ unsigned long addr = res->start;
                                 has_io = 1;
- a &= PCI_BASE_ADDRESS_IO_MASK;
- if (!a || a == PCI_BASE_ADDRESS_IO_MASK)
+ if (!addr || addr == PCI_BASE_ADDRESS_IO_MASK)
                                         pcibios_fixup_io_addr(dev, i);
                         } else if (a & PCI_BASE_ADDRESS_MEM_MASK)
                                 has_mem = 1;
diff -ruN linux/drivers/block/ide-pci.c linuxcp/drivers/block/ide-pci.c
--- linux/drivers/block/ide-pci.c Sat Jul 22 02:14:06 2000
+++ linuxcp/drivers/block/ide-pci.c Sat Jul 22 02:07:02 2000
@@ -257,20 +257,29 @@
         switch(dev->device) {
                 case PCI_DEVICE_ID_TTI_HPT343:
                         {
- int i;
                                 unsigned short pcicmd = 0;
- unsigned long hpt34xIoBase = dev->base_address[4] & PCI_BASE_ADDRESS_IO_MASK;
 
                                 pci_write_config_byte(dev, 0x80, 0x00);
- dev->base_address[0] = (hpt34xIoBase + 0x20);
- dev->base_address[1] = (hpt34xIoBase + 0x34);
- dev->base_address[2] = (hpt34xIoBase + 0x28);
- dev->base_address[3] = (hpt34xIoBase + 0x3c);
- for(i=0; i<4; i++)
- dev->base_address[i] |= PCI_BASE_ADDRESS_SPACE_IO;
-
                                 pci_read_config_word(dev, PCI_COMMAND, &pcicmd);
                                 if (!(pcicmd & PCI_COMMAND_MEMORY)) {
+ /*
+ * FIXME - this is too ugly, and looks senseless.
+ * Why not just use resource[4]?
+ *
+ * This was a cleaner/quicker way to get the ioports
+ * that the are not decode do to a flaw in the chipset
+ * design.
+ */
+
+ int i;
+ unsigned long hpt34xIoBase = dev->resource[4].start;
+
+ dev->resource[0].start = (hpt34xIoBase + 0x20);
+ dev->resource[1].start = (hpt34xIoBase + 0x34);
+ dev->resource[2].start = (hpt34xIoBase + 0x28);
+ dev->resource[3].start = (hpt34xIoBase + 0x3c);
+ for(i=0; i<4; i++)
+ dev->resource[i].flags |= PCI_BASE_ADDRESS_SPACE_IO;
                                         pci_write_config_byte(dev, PCI_LATENCY_TIMER, 0x20);
                                 } else {
                                         pci_write_config_byte(dev, PCI_LATENCY_TIMER, 0xF0);
@@ -375,11 +384,15 @@
         /*
          * Setup base registers for IDE command/control spaces for each interface:
          */
- for (reg = 0; reg < 4; reg++)
- if (!dev->base_address[reg]) {
+ for (reg = 0; reg < 4; reg++) {
+ struct resource *res = dev->resource + reg;
+ if (!(res->flags & PCI_BASE_ADDRESS_SPACE_IO))
+ continue;
+ if (!res->start) {
                         printk("%s: Missing I/O address #%d\n", name, reg);
                         return 1;
                 }
+ }
         return 0;
 }
 
@@ -478,8 +491,9 @@
                 if (IDE_PCI_DEVID_EQ(d->devid, DEVID_HPT366) && (port))
                         return;
                 if ((dev->class >> 8) != PCI_CLASS_STORAGE_IDE || (dev->class & (port ? 4 : 1)) != 0) {
- ctl = dev->base_address[(2*port)+1] & PCI_BASE_ADDRESS_IO_MASK;
- base = dev->base_address[2*port] & ~7;
+ /* FIXME! This really should check that it really gets the IO/MEM part right! */
+ ctl = dev->resource[(2*port)+1].start;
+ base = dev->resource[2*port].start;
                 }
                 if ((ctl && !base) || (base && !ctl)) {
                         printk("%s: inconsistent baseregs (BIOS) for port %d, skipping\n", d->name, port);
diff -ruN linux/drivers/pci/pci.c linuxcp/drivers/pci/pci.c
--- linux/drivers/pci/pci.c Sat Jul 22 02:17:14 2000
+++ linuxcp/drivers/pci/pci.c Sat Jul 22 02:07:02 2000
@@ -29,8 +29,9 @@
 
 struct pci_bus pci_root;
 struct pci_dev *pci_devices = NULL;
+int pci_reverse __initdata = 0;
+
 static struct pci_dev **pci_last_dev_p = &pci_devices;
-static int pci_reverse __initdata = 0;
 
 struct pci_dev *
 pci_find_slot(unsigned int bus, unsigned int devfn)
@@ -143,25 +144,56 @@
         unsigned int reg;
         u32 l;
 
- for(reg=0; reg<howmany; reg++) {
+ for(reg=0; reg < howmany; reg++) {
+ struct resource *res = dev->resource + reg;
+ unsigned int mask, newval, size;
+
+ res->name = dev->name;
                 pci_read_config_dword(dev, PCI_BASE_ADDRESS_0 + (reg << 2), &l);
                 if (l == 0xffffffff)
                         continue;
- dev->base_address[reg] = l;
+
+ pci_write_config_dword(dev, PCI_BASE_ADDRESS_0 + (reg << 2), 0xffffffff);
+ pci_read_config_dword(dev, PCI_BASE_ADDRESS_0 + (reg << 2), &newval);
+ pci_write_config_dword(dev, PCI_BASE_ADDRESS_0 + (reg << 2), l);
+
+ mask = PCI_BASE_ADDRESS_MEM_MASK;
+ if (l & PCI_BASE_ADDRESS_SPACE_IO)
+ mask = PCI_BASE_ADDRESS_IO_MASK;
+
+ newval &= mask;
+ if (!newval)
+ continue;
+
+ res->start = l & mask;
+ res->flags = l & ~mask;
+
+ size = 1;
+ do {
+ size <<= 1;
+ } while (!(size & newval));
+
+ /* 64-bit memory? */
                 if ((l & (PCI_BASE_ADDRESS_SPACE | PCI_BASE_ADDRESS_MEM_TYPE_MASK))
                     == (PCI_BASE_ADDRESS_SPACE_MEMORY | PCI_BASE_ADDRESS_MEM_TYPE_64)) {
+ unsigned int high;
                         reg++;
- pci_read_config_dword(dev, PCI_BASE_ADDRESS_0 + (reg << 2), &l);
- if (l) {
+ pci_read_config_dword(dev, PCI_BASE_ADDRESS_0 + (reg << 2), &high);
+ if (high) {
 #if BITS_PER_LONG == 64
- dev->base_address[reg-1] |= ((unsigned long) l) << 32;
+ res->start |= ((unsigned long) high) << 32;
 #else
                                 printk("PCI: Unable to handle 64-bit address for device %02x:%02x\n",
                                         dev->bus->number, dev->devfn);
- dev->base_address[reg-1] = 0;
+ res->flags = 0;
+ res->start = 0;
+ res->end = 0;
+ continue;
 #endif
                         }
                 }
+ res->end = res->start + size - 1;
+ request_resource((l & PCI_BASE_ADDRESS_SPACE_IO) ? &ioport_resource : &iomem_resource, res);
         }
 }
 
diff -ruN linux/drivers/pci/proc.c linuxcp/drivers/pci/proc.c
--- linux/drivers/pci/proc.c Sat Jul 22 02:18:42 2000
+++ linuxcp/drivers/pci/proc.c Sat Jul 22 02:07:02 2000
@@ -11,6 +11,7 @@
 #include <linux/pci.h>
 #include <linux/proc_fs.h>
 #include <linux/init.h>
+
 #include <asm/uaccess.h>
 #include <asm/byteorder.h>
 
@@ -246,14 +247,15 @@
                         dev->vendor,
                         dev->device,
                         dev->irq);
- for(i=0; i<6; i++)
+ for(i=0; i<6; i++) {
                         len += sprintf(buf+len,
 #if BITS_PER_LONG == 32
                                                 "\t%08lx",
 #else
                                                 "\t%016lx",
 #endif
- dev->base_address[i]);
+ dev->resource[i].start | (dev->resource[i].flags & 0xf));
+ }
                 len += sprintf(buf+len,
 #if BITS_PER_LONG == 32
                                         "\t%08lx",
diff -ruN linux/include/linux/pci.h linuxcp/include/linux/pci.h
--- linux/include/linux/pci.h Sat Jul 22 02:19:19 2000
+++ linuxcp/include/linux/pci.h Sat Jul 22 02:07:02 2000
@@ -1245,7 +1245,7 @@
         /* Base registers for this device, can be adjusted by
          * pcibios_fixup() as necessary.
          */
- unsigned long base_address[6];
+ struct resource resource[6];
         unsigned long rom_address;
 };
 
diff -ruN linux/kernel/resource.c linuxcp/kernel/resource.c
--- linux/kernel/resource.c Sat Jul 22 02:24:00 2000
+++ linuxcp/kernel/resource.c Sat Jul 22 02:07:02 2000
@@ -12,9 +12,13 @@
 #include <linux/init.h>
 #include <linux/malloc.h>
 
+#include <asm/spinlock.h>
+
 struct resource ioport_resource = { "PCI IO", 0x0000, 0xFFFF };
 struct resource iomem_resource = { "PCI mem", 0x00000000, 0xFFFFFFFF };
 
+static rwlock_t resource_lock = RW_LOCK_UNLOCKED;
+
 /*
  * This generates reports for /proc/ioports and /proc/memory
  */
@@ -47,25 +51,30 @@
 int get_resource_list(struct resource *root, char *buf, int size)
 {
         char *fmt;
+ int retval;
 
         fmt = " %08lx-%08lx : %s\n";
         if (root == &ioport_resource)
                 fmt = " %04lx-%04lx : %s\n";
- return do_resource_list(root->child, fmt, 8, buf, buf + size) - buf;
+ read_lock(&resource_lock);
+ retval = do_resource_list(root->child, fmt, 8, buf, buf + size) - buf;
+ read_unlock(&resource_lock);
+ return retval;
 }
 
-int request_resource(struct resource *root, struct resource *new)
+/* Return the conflict entry if you can't request it */
+static struct resource * __request_resource(struct resource *root, struct resource *new)
 {
         unsigned long start = new->start;
         unsigned long end = new->end;
         struct resource *tmp, **p;
 
         if (end < start)
- return -EINVAL;
+ return root;
         if (start < root->start)
- return -EINVAL;
+ return root;
         if (end > root->end)
- return -EINVAL;
+ return root;
         p = &root->child;
         for (;;) {
                 tmp = *p;
@@ -73,15 +82,25 @@
                         new->sibling = tmp;
                         *p = new;
                         new->parent = root;
- return 0;
+ return NULL;
                 }
                 p = &tmp->sibling;
                 if (tmp->end < start)
                         continue;
- return -EBUSY;
+ return tmp;
         }
 }
 
+int request_resource(struct resource *root, struct resource *new)
+{
+ struct resource *conflict;
+
+ write_lock(&resource_lock);
+ conflict = __request_resource(root, new);
+ write_unlock(&resource_lock);
+ return conflict ? -EBUSY : 0;
+}
+
 int release_resource(struct resource *old)
 {
         struct resource *tmp, **p;
@@ -101,6 +120,18 @@
         return -EINVAL;
 }
 
+/*
+ * This is compatibility stuff for IO resources.
+ *
+ * Note how this, unlike the above, knows about
+ * the IO flag meanings (busy etc).
+ *
+ * Request-region creates a new busy region.
+ *
+ * Check-region returns non-zero if the area is already busy
+ *
+ * Release-region releases a matching busy region.
+ */
 struct resource * __request_region(struct resource *parent, unsigned long start, unsigned long n, const char *name)
 {
         struct resource *res = kmalloc(sizeof(*res), GFP_KERNEL);
@@ -110,21 +141,33 @@
                 res->name = name;
                 res->start = start;
                 res->end = start + n - 1;
- if (request_resource(parent, res) != 0) {
+ res->flags = IORESOURCE_BUSY;
+
+ write_lock(&resource_lock);
+
+ for (;;) {
+ struct resource *conflict;
+
+ conflict = __request_resource(parent, res);
+ if (!conflict)
+ break;
+ if (conflict != parent) {
+ parent = conflict;
+ if (!(conflict->flags & IORESOURCE_BUSY))
+ continue;
+ }
+
+ /* Uhhuh, that didn't work out.. */
                         kfree(res);
                         res = NULL;
+ break;
                 }
+ write_unlock(&resource_lock);
         }
         return res;
 }
 
 /*
- * Compatibility cruft.
- *
- * Check-region returns non-zero if something already exists.
- *
- * Release-region releases an anonymous region that matches
- * the IO port range.
  */
 int __check_region(struct resource *parent, unsigned long start, unsigned long n)
 {
@@ -152,7 +195,13 @@
 
                 if (!res)
                         break;
- if (res->start == start && res->end == end) {
+ if (res->start <= start && res->end >= end) {
+ if (!(res->flags & IORESOURCE_BUSY)) {
+ p = &res->child;
+ continue;
+ }
+ if (res->start != start || res->end != end)
+ break;
                         *p = res->sibling;
                         kfree(res);
                         return;

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu
Please read the FAQ at http://www.tux.org/lkml/



This archive was generated by hypermail 2b29 : Sun Jul 23 2000 - 21:00:18 EST