Re: 0.9.7c, cs4236b and kernel oops

[Takashi Iwai <tiwai@suse.de>]

[-- Attachment #1: Type: text/plain, Size: 1490 bytes --]

At Wed, 15 Oct 2003 16:21:57 +0200,
私 wrote:
> 
> At Wed, 15 Oct 2003 09:20:53 -0400,
> Dennis Veatch wrote:
> > 
> > On Wednesday 15 October 2003 06:33 am, Takashi Iwai wrote:
> > > At Tue, 14 Oct 2003 21:24:56 -0400,
> > >
> > > Dennis Veatch wrote:
> > > > This is a Dell GX1 with a cs4236b integrated sound card. Running kernel
> > > > version 2.4.20. I have tried recompiling the kernel any number of ways
> > > > making the sound stuff modules and part of the kernel all with the same
> > > > results. I have wiped out /lib/modules and /usr/src/linux entirely,
> > > > re-downloaded all the kernel stuff and still had the same results. If I
> > > > use the 0.9.6 version of alsa-driver there are no problems.
> > >
> > > is it a pnp device or non-pnp?
> > >
> > > there are no big changes in the cs423x codes.
> > > so the problem must be in either pnp layer or the change in the core
> > > part.
> > >
> > > if it's pnp card, please try to replace snd-pnp.o with the old one in
> > > 0.9.6.
> > >
> > >
> > 
> > Actually I am not sure if its pnp or not. During boot it says "isapnp: Card 
> > 'CS4236B' so I assume its pnp? 
>  
> yes.
> 
> > There is definitely something different in snd-pnp.o between version 0.9.6 and 
> > 0.9.7 on up. Tried your suggestion of copying snd-pnp.o (version 0.9.60) over 
> > the same of version 0.9.7c and there was no Oops.
> 
> ok, it looks like the pnp layer is the cause.
> i'll take a look.

could you try the attached patch?


Takashi

[-- Attachment #2: pnp-fix.dif --]
[-- Type: application/octet-stream, Size: 5334 bytes --]

Index: alsa-driver/support/pnp/pnp.c
===================================================================
RCS file: /suse/tiwai/cvs/alsa/alsa-driver/support/pnp/pnp.c,v
retrieving revision 1.11
diff -u -r1.11 pnp.c
--- alsa-driver/support/pnp/pnp.c	24 Sep 2003 15:59:31 -0000	1.11
+++ alsa-driver/support/pnp/pnp.c	16 Oct 2003 11:13:28 -0000
@@ -158,46 +158,51 @@
 int pnp_register_card_driver(struct pnp_card_driver * drv)
 {
 	unsigned short vendor, device;
+	unsigned short subvendor, subdevice;
 	unsigned int i, res = 0;
 	const struct pnp_card_device_id *cid;
 	struct pnp_card *card;
 	struct pnp_dev *dev;
 	struct pnp_card_driver_instance *ninst = NULL;
 	
+	if (! drv->probe) {
+		printk(KERN_ERR "pnp: no probe function!\n");
+		return -EINVAL;
+	}
+
 	for (cid = drv->id_table; cid->id[0] != '\0'; cid++) {
-	      __next_card:
-		card = NULL;
-		do {
-		      __next:
-			if (parse_id(cid->id, &vendor, &device) < 0)
-				break;
-			card = (struct pnp_card *)isapnp_find_card(vendor, device, (struct isapnp_card *)card);
-			if (card) {
-				if (ninst == NULL) {
-					ninst = kmalloc(sizeof(*ninst), GFP_KERNEL);
-					if (ninst == NULL)
-						return res > 0 ? (int)res : -ENOMEM;
-					memset(ninst, 0, sizeof(*ninst));
-					INIT_LIST_HEAD(&ninst->list);
-				}
-				for (i = 0; i < PNP_MAX_DEVICES; i++)
-					ninst->devs[i] = NULL;
-				for (i = 0; i < PNP_MAX_DEVICES && cid->devs[i].id[0] != '\0'; i++) {
-					if (parse_id(cid->devs[i].id, &vendor, &device) < 0) {
-						cid++;
-						goto __next_card;
-					}
-					dev = ninst->devs[i] = (struct pnp_dev *)isapnp_find_dev((struct isapnp_card *)card, vendor, device, NULL);
-					if (dev == NULL)
-						goto __next;
-					if (! dev->p.active) {
-						if (! (drv->flags & PNP_DRIVER_RES_DO_NOT_CHANGE)) {
-							pnp_activate_dev(dev);
-						}
-					} else {
-						if ((drv->flags & PNP_DRIVER_RES_DISABLE) == PNP_DRIVER_RES_DISABLE) {
-							pnp_disable_dev(dev);
-						}
+		if (parse_id(cid->id, &vendor, &device) < 0)
+			continue;
+		card = NULL; 
+		while ((card = (struct pnp_card *)isapnp_find_card(vendor, device, (struct isapnp_card *)card)) != NULL) {
+			if (ninst == NULL) {
+				ninst = kmalloc(sizeof(*ninst), GFP_KERNEL);
+				if (ninst == NULL)
+					return res > 0 ? (int)res : -ENOMEM;
+				memset(ninst, 0, sizeof(*ninst));
+				INIT_LIST_HEAD(&ninst->list);
+			}
+			for (i = 0; i < PNP_MAX_DEVICES; i++)
+				ninst->devs[i] = NULL;
+			for (i = 0; i < PNP_MAX_DEVICES && cid->devs[i].id[0] != '\0'; i++) {
+				if (parse_id(cid->devs[i].id, &subvendor, &subdevice) < 0)
+					goto __next_card;
+				dev = ninst->devs[i] = (struct pnp_dev *)isapnp_find_dev((struct isapnp_card *)card, subvendor, subdevice, NULL);
+				if (dev == NULL)
+					break;
+			}
+			if (i == PNP_MAX_DEVICES || !cid->devs[i].id[0]) {
+				/* all parsed successfully */
+				/* activate or deactivate devices before probing */
+				for (i = 0; i < PNP_MAX_DEVICES; i++) {
+					dev = ninst->devs[i];
+					if (! dev)
+						break;
+					if ((drv->flags & PNP_DRIVER_RES_DISABLE) != PNP_DRIVER_RES_DISABLE)
+						dev->p.activate((struct isapnp_dev *)dev);
+					else {
+						dev->p.deactivate((struct isapnp_dev *)dev);
+						dev->p.prepare((struct isapnp_dev *)dev);
 					}
 				}
 				ninst->link.card = card;
@@ -209,7 +214,9 @@
 					res++;
 				}
 			}
-		} while (card != NULL);
+		}
+	__next_card:
+		;
 	}
 
 	if (ninst != NULL)
@@ -227,7 +234,8 @@
 		inst = list_entry(p, struct pnp_card_driver_instance, list);
 		if (inst->link.driver == drv) {
 			list_del(p);
-			drv->remove(&inst->link);
+			if (drv->remove)
+				drv->remove(&inst->link);
 			kfree(inst);
 		}
 	}
@@ -321,14 +329,9 @@
 	}
 }
 
-/* FIXME: this function cannot be called many times.  the setting is cleared at each time */
 int pnp_manual_config_dev(struct pnp_dev *dev, struct pnp_resource_table *res, int mode)
 {
 	unsigned int idx;
-	int err;
-
-	/* prepare the isapnp */
-	err = dev->p.prepare((struct isapnp_dev *)dev);
 
 	for (idx = 0; idx < PNP_MAX_IRQ; idx++)
 		copy_resource(&dev->p.irq_resource[idx], &res->irq_resource[idx]);
@@ -344,30 +347,8 @@
 
 int pnp_activate_dev(struct pnp_dev *dev)
 {
-	struct pnp_resource_table *tmp;
-	unsigned int idx;
-
 	if (dev->p.active)
-		return 0; /* FIXME: should be -EBUSY but 2.6 pnp layer behaves like this */
-
-	/* reserve the manual configuration */
-	tmp = kmalloc(sizeof(*tmp), GFP_KERNEL);
-	if (! tmp)
-		return -ENOMEM;
-	pnp_init_resource_table(tmp);
-	for (idx = 0; idx < PNP_MAX_IRQ; idx++)
-		copy_resource(&tmp->irq_resource[idx], &dev->p.irq_resource[idx]);
-	for (idx = 0; idx < PNP_MAX_DMA; idx++)
-		copy_resource(&tmp->dma_resource[idx], &dev->p.dma_resource[idx]);
-	for (idx = 0; idx < PNP_MAX_PORT; idx++)
-		copy_resource(&tmp->port_resource[idx], &dev->p.resource[idx]);
-	for (idx = 0; idx < PNP_MAX_MEM; idx++)
-		copy_resource(&tmp->mem_resource[idx], &dev->p.resource[idx+8]);
-
-	/* restore the manual configuration again */
-	pnp_manual_config_dev(dev, tmp, 0);
-	kfree(tmp);
-
+		return 0; /* 2.6 pnp layer behaves like this */
 	return dev->p.activate((struct isapnp_dev *)dev);
 }
 
@@ -376,7 +357,6 @@
 	if (! dev->p.active)
 		return 0;
 	return dev->p.deactivate((struct isapnp_dev *)dev);
-	/* FIXME: do we need clean up the resources again? */
 }
 
 static int __init pnp_init(void)