rmmod snd-es1968 crashes my laptop...

rmmod snd-es1968 crashes my laptop...

[Karsten Wiese]

...if the us428 is running. 
the us428 (via uhci module) uses exactly the same irq 5 as the snd-es1968 
here.
If the us428 is deactivated, that is there are only snd-es1968 related 
interrupts on irq5, "rmmod snd-es1968" is ok.

This prevents the crash here (2.4.26 kernel, planet CCRMA fc1):
>>>>>>
--- alsa-kernel/pci/es1968.c	3 May 2004 17:34:48 -0000	1.66
+++ alsa-kernel/pci/es1968.c	13 Jun 2004 17:01:32 -0000
@@ -2477,7 +2477,7 @@
 		kfree_nocheck(chip->res_joystick);
 	}
 #endif
-	snd_es1968_set_acpi(chip, ACPI_D3);
+	//snd_es1968_set_acpi(chip, ACPI_D3);
 	chip->master_switch = NULL;
 	chip->master_volume = NULL;
 	if (chip->res_io_port) {
<<<<<<

I don't know, if this is the correct solution.
Let me know, if you need more details or tests.

thanks,
Karsten



-------------------------------------------------------
This SF.Net email is sponsored by The 2004 JavaOne(SM) Conference
Learn from the experts at JavaOne(SM), Sun's Worldwide Java Developer
Conference, June 28 - July 1 at the Moscone Center in San Francisco, CA
REGISTER AND SAVE! http://java.sun.com/javaone/sf Priority Code NWMGYKND

Re: rmmod snd-es1968 crashes my laptop...

[Takashi Iwai]

At Sun, 13 Jun 2004 20:23:48 +0200,
Karsten Wiese wrote:
> 
> ...if the us428 is running. 
> the us428 (via uhci module) uses exactly the same irq 5 as the snd-es1968 
> here.
> If the us428 is deactivated, that is there are only snd-es1968 related 
> interrupts on irq5, "rmmod snd-es1968" is ok.
> 
> This prevents the crash here (2.4.26 kernel, planet CCRMA fc1):
> >>>>>>
> --- alsa-kernel/pci/es1968.c	3 May 2004 17:34:48 -0000	1.66
> +++ alsa-kernel/pci/es1968.c	13 Jun 2004 17:01:32 -0000
> @@ -2477,7 +2477,7 @@
>  		kfree_nocheck(chip->res_joystick);
>  	}
>  #endif
> -	snd_es1968_set_acpi(chip, ACPI_D3);
> +	//snd_es1968_set_acpi(chip, ACPI_D3);
>  	chip->master_switch = NULL;
>  	chip->master_volume = NULL;
>  	if (chip->res_io_port) {
> <<<<<<
> 
> I don't know, if this is the correct solution.
> Let me know, if you need more details or tests.

I also don't know whether it's correct :)
ESS chip is always in tons of mystery.

Does the driver work if you reload the module again?
How about suspend/resume?


Takashi


-------------------------------------------------------
This SF.Net email is sponsored by the new InstallShield X.
>From Windows to Linux, servers to mobile, InstallShield X is the
one installation-authoring solution that does it all. Learn more and
evaluate today! http://www.installshield.com/Dev2Dev/0504

Re: rmmod snd-es1968 crashes my laptop...

[Karsten Wiese]

Am Montag 14 Juni 2004 15:39 schrieb Takashi Iwai:
> At Sun, 13 Jun 2004 20:23:48 +0200,
>
> Karsten Wiese wrote:
> > ...if the us428 is running.
> > the us428 (via uhci module) uses exactly the same irq 5 as the snd-es1968
> > here.
> > If the us428 is deactivated, that is there are only snd-es1968 related
> > interrupts on irq5, "rmmod snd-es1968" is ok.
> >
> > This prevents the crash here (2.4.26 kernel, planet CCRMA fc1):
> >
> > --- alsa-kernel/pci/es1968.c	3 May 2004 17:34:48 -0000	1.66
> > +++ alsa-kernel/pci/es1968.c	13 Jun 2004 17:01:32 -0000
> > @@ -2477,7 +2477,7 @@
> >  		kfree_nocheck(chip->res_joystick);
> >  	}
> >  #endif
> > -	snd_es1968_set_acpi(chip, ACPI_D3);
> > +	//snd_es1968_set_acpi(chip, ACPI_D3);
> >  	chip->master_switch = NULL;
> >  	chip->master_volume = NULL;
> >  	if (chip->res_io_port) {
> > <<<<<<
> >
> > I don't know, if this is the correct solution.
> > Let me know, if you need more details or tests.
>
> I also don't know whether it's correct :)
> ESS chip is always in tons of mystery.
>
> Does the driver work if you reload the module again?

Yes.

> How about suspend/resume?

"apmsleep --suspend +1:00" is OK! 
At least the devices (Maestro2 & us428) are fully functional after wakeup.
I made shure by inserting snd_printd in  es1968_resume & es1968_suspend t5hat 
those are called in the process.
the us428 gets restarted by hotplug, I guess.
The apps I tested (Ecasound & KDE's artsd) didn't survive though.

This patch keeps the "snd_es1968_set_acpi(chip, ACPI_D3);":

>>>>>>
Index: es1968.c
===================================================================
RCS file: /cvsroot/alsa/alsa-kernel/pci/es1968.c,v
retrieving revision 1.66
diff -u -r1.66 es1968.c
--- es1968.c	3 May 2004 17:34:48 -0000	1.66
+++ es1968.c	14 Jun 2004 22:15:20 -0000
@@ -2477,7 +2477,6 @@
 		kfree_nocheck(chip->res_joystick);
 	}
 #endif
-	snd_es1968_set_acpi(chip, ACPI_D3);
 	chip->master_switch = NULL;
 	chip->master_volume = NULL;
 	if (chip->res_io_port) {
@@ -2486,6 +2485,7 @@
 	}
 	if (chip->irq >= 0)
 		free_irq(chip->irq, (void *)chip);
+	snd_es1968_set_acpi(chip, ACPI_D3);
 	snd_magic_kfree(chip);
 	return 0;
 }
<<<<<<

I think its OK to commit with comment:

	- Prevent shared Interrupts from other devices
	  causing crash on device free.

(Analysis:
the crash was induced when the usb interrupt was still reached to 
snd_es1968_interrupt:
the value "event = inb(chip->io_port + 0x1A)" will possibly be != 0 cause the 
device went into high Z state, so snd_es1968_interrupt falsely tries to 
handle the alien interrupt....
by telling kernel that the module doesn't handle interrupts any more before 
switching the device into powersave mode we keep the system sane.
or so....)

thanks,
Karsten



-------------------------------------------------------
This SF.Net email is sponsored by The 2004 JavaOne(SM) Conference
Learn from the experts at JavaOne(SM), Sun's Worldwide Java Developer
Conference, June 28 - July 1 at the Moscone Center in San Francisco, CA
REGISTER AND SAVE! http://java.sun.com/javaone/sf Priority Code NWMGYKND

Re: Re: rmmod snd-es1968 crashes my laptop...

[Takashi Iwai]

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

At Tue, 15 Jun 2004 00:50:59 +0200,
Karsten Wiese wrote:
> 
> Am Montag 14 Juni 2004 15:39 schrieb Takashi Iwai:
> > At Sun, 13 Jun 2004 20:23:48 +0200,
> This patch keeps the "snd_es1968_set_acpi(chip, ACPI_D3);":
(snip)
> I think its OK to commit with comment:
> 
> 	- Prevent shared Interrupts from other devices
> 	  causing crash on device free.
> 
> (Analysis:
> the crash was induced when the usb interrupt was still reached to 
> snd_es1968_interrupt:
> the value "event = inb(chip->io_port + 0x1A)" will possibly be != 0 cause the 
> device went into high Z state, so snd_es1968_interrupt falsely tries to 
> handle the alien interrupt....
> by telling kernel that the module doesn't handle interrupts any more before 
> switching the device into powersave mode we keep the system sane.
> or so....)

Then we should move free_irq() itself before releasing other
resources.  calling snd_es1968_set_acpi() after release io's is
invalid.

How about the attached patch?


Takashi

[-- Attachment #2: Type: text/plain, Size: 779 bytes --]

Index: alsa-kernel/pci/es1968.c
===================================================================
RCS file: /suse/tiwai/cvs/alsa/alsa-kernel/pci/es1968.c,v
retrieving revision 1.66
diff -u -r1.66 es1968.c
--- alsa-kernel/pci/es1968.c	26 Apr 2004 10:58:07 -0000	1.66
+++ alsa-kernel/pci/es1968.c	15 Jun 2004 11:00:02 -0000
@@ -2470,6 +2470,8 @@
 		outw(0, chip->io_port + ESM_PORT_HOST_IRQ); /* disable IRQ */
 	}
 
+	if (chip->irq >= 0)
+		free_irq(chip->irq, (void *)chip);
 #ifdef SUPPORT_JOYSTICK
 	if (chip->res_joystick) {
 		gameport_unregister_port(&chip->gameport);
@@ -2484,8 +2486,6 @@
 		release_resource(chip->res_io_port);
 		kfree_nocheck(chip->res_io_port);
 	}
-	if (chip->irq >= 0)
-		free_irq(chip->irq, (void *)chip);
 	snd_magic_kfree(chip);
 	return 0;
 }

Re: Re: rmmod snd-es1968 crashes my laptop...

[Karsten Wiese]

Am Dienstag 15 Juni 2004 13:04 schrieb Takashi Iwai:
> At Tue, 15 Jun 2004 00:50:59 +0200,
>
> Karsten Wiese wrote:
> > Am Montag 14 Juni 2004 15:39 schrieb Takashi Iwai:
> > > At Sun, 13 Jun 2004 20:23:48 +0200,
> >
> > This patch keeps the "snd_es1968_set_acpi(chip, ACPI_D3);":
>
> (snip)
>
> > I think its OK to commit with comment:
> >
> > 	- Prevent shared Interrupts from other devices
> > 	  causing crash on device free.
> >
> > (Analysis:
> > the crash was induced when the usb interrupt was still reached to
> > snd_es1968_interrupt:
> > the value "event = inb(chip->io_port + 0x1A)" will possibly be != 0 cause
> > the device went into high Z state, so snd_es1968_interrupt falsely tries
> > to handle the alien interrupt....
> > by telling kernel that the module doesn't handle interrupts any more
> > before switching the device into powersave mode we keep the system sane.
> > or so....)
>
> Then we should move free_irq() itself before releasing other
> resources.  calling snd_es1968_set_acpi() after release io's is
> invalid.
>
> How about the attached patch?

Agreed. Works fine here.

Karsten


-------------------------------------------------------
This SF.Net email is sponsored by The 2004 JavaOne(SM) Conference
Learn from the experts at JavaOne(SM), Sun's Worldwide Java Developer
Conference, June 28 - July 1 at the Moscone Center in San Francisco, CA
REGISTER AND SAVE! http://java.sun.com/javaone/sf Priority Code NWMGYKND