FIx no mpu401 interface can cause hard freeze

PATCH In review Randy Dunlap 5 participants 5 messages Jul 3, 2006 – Jul 4, 2006 Sound

Share & tools

Randy Dunlap

author

2006-07-03 20:46 UTC #0 lore raw

PATCH 1/1 FIx no mpu401 interface can cause hard freeze

This patch fixes the remaining instances in our tree where a non- existent mpu401 interface can cause a hard freeze when i/o is issued.

This commit closes Malone #34831.

Bug https://launchpad.net/distros/ubuntu/+source/linux-source-2.6.15/+bug/34831

patch location:
http://www.kernel.org/git/?p=linux/kernel/git/bcollins/ubuntu-dapper.git;a=commitdiff;h=b422309cdd980cfefe99379796c04e961d3c1544

---
 sound/pci/emu10k1/emu10k1x.c  |   35 +++++++++++++++++++++++++----------
 sound/pci/emu10k1/emumpu401.c |   35 +++++++++++++++++++++++++----------
 2 files changed, 50 insertions(+), 20 deletions(-)

sound/pci/emu10k1/emu10k1x.c +25 ~~-10~~

@@ -1286,7 +1286,7 @@ static void snd_emu10k1x_midi_interrupt(
 	do_emu10k1x_midi_interrupt(emu, &emu->midi, status);
 }
 
-static void snd_emu10k1x_midi_cmd(struct emu10k1x * emu,
+static int snd_emu10k1x_midi_cmd(struct emu10k1x * emu,
 				  struct emu10k1x_midi *midi, unsigned char cmd, int ack)
 {
 	unsigned long flags;

@@ -1312,11 +1312,14 @@ static void snd_emu10k1x_midi_cmd(struct
 		ok = 1;
 	}
 	spin_unlock_irqrestore(&midi->input_lock, flags);
-	if (!ok)
+	if (!ok) {
 		snd_printk(KERN_ERR "midi_cmd: 0x%x failed at 0x%lx (status = 0x%x, data = 0x%x)!!!\n",
 			   cmd, emu->port,
 			   mpu401_read_stat(emu, midi),
 			   mpu401_read_data(emu, midi));
+		return 1;
+	}
+	return 0;
 }
 
 static int snd_emu10k1x_midi_input_open(struct snd_rawmidi_substream *substream)

@@ -1332,12 +1335,17 @@ static int snd_emu10k1x_midi_input_open(
 	midi->substream_input = substream;
 	if (!(midi->midi_mode & EMU10K1X_MIDI_MODE_OUTPUT)) {
 		spin_unlock_irqrestore(&midi->open_lock, flags);
-		snd_emu10k1x_midi_cmd(emu, midi, MPU401_RESET, 1);
-		snd_emu10k1x_midi_cmd(emu, midi, MPU401_ENTER_UART, 1);
+		if (snd_emu10k1x_midi_cmd(emu, midi, MPU401_RESET, 1))
+			goto error_out;
+		if (snd_emu10k1x_midi_cmd(emu, midi, MPU401_ENTER_UART, 1))
+			goto error_out;
 	} else {
 		spin_unlock_irqrestore(&midi->open_lock, flags);
 	}
 	return 0;
+
+error_out:
+	return -EIO;
 }
 
 static int snd_emu10k1x_midi_output_open(struct snd_rawmidi_substream *substream)

@@ -1353,12 +1361,17 @@ static int snd_emu10k1x_midi_output_open
 	midi->substream_output = substream;
 	if (!(midi->midi_mode & EMU10K1X_MIDI_MODE_INPUT)) {
 		spin_unlock_irqrestore(&midi->open_lock, flags);
-		snd_emu10k1x_midi_cmd(emu, midi, MPU401_RESET, 1);
-		snd_emu10k1x_midi_cmd(emu, midi, MPU401_ENTER_UART, 1);
+		if (snd_emu10k1x_midi_cmd(emu, midi, MPU401_RESET, 1))
+			goto error_out;
+		if (snd_emu10k1x_midi_cmd(emu, midi, MPU401_ENTER_UART, 1))
+			goto error_out;
 	} else {
 		spin_unlock_irqrestore(&midi->open_lock, flags);
 	}
 	return 0;
+
+error_out:
+	return -EIO;
 }
 
 static int snd_emu10k1x_midi_input_close(struct snd_rawmidi_substream *substream)

@@ -1366,6 +1379,7 @@ static int snd_emu10k1x_midi_input_close
 	struct emu10k1x *emu;
 	struct emu10k1x_midi *midi = substream->rmidi->private_data;
 	unsigned long flags;
+	int err = 0;
 
 	emu = midi->emu;
 	snd_assert(emu, return -ENXIO);

@@ -1375,11 +1389,11 @@ static int snd_emu10k1x_midi_input_close
 	midi->substream_input = NULL;
 	if (!(midi->midi_mode & EMU10K1X_MIDI_MODE_OUTPUT)) {
 		spin_unlock_irqrestore(&midi->open_lock, flags);
-		snd_emu10k1x_midi_cmd(emu, midi, MPU401_RESET, 0);
+		err = snd_emu10k1x_midi_cmd(emu, midi, MPU401_RESET, 0);
 	} else {
 		spin_unlock_irqrestore(&midi->open_lock, flags);
 	}
-	return 0;
+	return err;
 }
 
 static int snd_emu10k1x_midi_output_close(struct snd_rawmidi_substream *substream)

@@ -1387,6 +1401,7 @@ static int snd_emu10k1x_midi_output_clos
 	struct emu10k1x *emu;
 	struct emu10k1x_midi *midi = substream->rmidi->private_data;
 	unsigned long flags;
+	int err = 0;
 
 	emu = midi->emu;
 	snd_assert(emu, return -ENXIO);

@@ -1396,11 +1411,11 @@ static int snd_emu10k1x_midi_output_clos
 	midi->substream_output = NULL;
 	if (!(midi->midi_mode & EMU10K1X_MIDI_MODE_INPUT)) {
 		spin_unlock_irqrestore(&midi->open_lock, flags);
-		snd_emu10k1x_midi_cmd(emu, midi, MPU401_RESET, 0);
+		err = snd_emu10k1x_midi_cmd(emu, midi, MPU401_RESET, 0);
 	} else {
 		spin_unlock_irqrestore(&midi->open_lock, flags);
 	}
-	return 0;
+	return err;
 }
 
 static void snd_emu10k1x_midi_input_trigger(struct snd_rawmidi_substream *substream, int up)

sound/pci/emu10k1/emumpu401.c +25 ~~-10~~

@@ -116,7 +116,7 @@ static void snd_emu10k1_midi_interrupt2(
 	do_emu10k1_midi_interrupt(emu, &emu->midi2, status);
 }
 
-static void snd_emu10k1_midi_cmd(struct snd_emu10k1 * emu, struct snd_emu10k1_midi *midi, unsigned char cmd, int ack)
+static int snd_emu10k1_midi_cmd(struct snd_emu10k1 * emu, struct snd_emu10k1_midi *midi, unsigned char cmd, int ack)
 {
 	unsigned long flags;
 	int timeout, ok;

@@ -141,11 +141,14 @@ static void snd_emu10k1_midi_cmd(struct 
 		ok = 1;
 	}
 	spin_unlock_irqrestore(&midi->input_lock, flags);
-	if (!ok)
+	if (!ok) {
 		snd_printk(KERN_ERR "midi_cmd: 0x%x failed at 0x%lx (status = 0x%x, data = 0x%x)!!!\n",
 			   cmd, emu->port,
 			   mpu401_read_stat(emu, midi),
 			   mpu401_read_data(emu, midi));
+		return 1;
+	}
+	return 0;
 }
 
 static int snd_emu10k1_midi_input_open(struct snd_rawmidi_substream *substream)

@@ -161,12 +164,17 @@ static int snd_emu10k1_midi_input_open(s
 	midi->substream_input = substream;
 	if (!(midi->midi_mode & EMU10K1_MIDI_MODE_OUTPUT)) {
 		spin_unlock_irqrestore(&midi->open_lock, flags);
-		snd_emu10k1_midi_cmd(emu, midi, MPU401_RESET, 1);
-		snd_emu10k1_midi_cmd(emu, midi, MPU401_ENTER_UART, 1);
+		if (snd_emu10k1_midi_cmd(emu, midi, MPU401_RESET, 1))
+			goto error_out;
+		if (snd_emu10k1_midi_cmd(emu, midi, MPU401_ENTER_UART, 1))
+			goto error_out;
 	} else {
 		spin_unlock_irqrestore(&midi->open_lock, flags);
 	}
 	return 0;
+
+error_out:
+	return -EIO;
 }
 
 static int snd_emu10k1_midi_output_open(struct snd_rawmidi_substream *substream)

@@ -182,12 +190,17 @@ static int snd_emu10k1_midi_output_open(
 	midi->substream_output = substream;
 	if (!(midi->midi_mode & EMU10K1_MIDI_MODE_INPUT)) {
 		spin_unlock_irqrestore(&midi->open_lock, flags);
-		snd_emu10k1_midi_cmd(emu, midi, MPU401_RESET, 1);
-		snd_emu10k1_midi_cmd(emu, midi, MPU401_ENTER_UART, 1);
+		if (snd_emu10k1_midi_cmd(emu, midi, MPU401_RESET, 1))
+			goto error_out;
+		if (snd_emu10k1_midi_cmd(emu, midi, MPU401_ENTER_UART, 1))
+			goto error_out;
 	} else {
 		spin_unlock_irqrestore(&midi->open_lock, flags);
 	}
 	return 0;
+
+error_out:
+	return -EIO;
 }
 
 static int snd_emu10k1_midi_input_close(struct snd_rawmidi_substream *substream)

@@ -195,6 +208,7 @@ static int snd_emu10k1_midi_input_close(
 	struct snd_emu10k1 *emu;
 	struct snd_emu10k1_midi *midi = (struct snd_emu10k1_midi *)substream->rmidi->private_data;
 	unsigned long flags;
+	int err = 0;
 
 	emu = midi->emu;
 	snd_assert(emu, return -ENXIO);

@@ -204,11 +218,11 @@ static int snd_emu10k1_midi_input_close(
 	midi->substream_input = NULL;
 	if (!(midi->midi_mode & EMU10K1_MIDI_MODE_OUTPUT)) {
 		spin_unlock_irqrestore(&midi->open_lock, flags);
-		snd_emu10k1_midi_cmd(emu, midi, MPU401_RESET, 0);
+		err = snd_emu10k1_midi_cmd(emu, midi, MPU401_RESET, 0);
 	} else {
 		spin_unlock_irqrestore(&midi->open_lock, flags);
 	}
-	return 0;
+	return err;
 }
 
 static int snd_emu10k1_midi_output_close(struct snd_rawmidi_substream *substream)

@@ -216,6 +230,7 @@ static int snd_emu10k1_midi_output_close
 	struct snd_emu10k1 *emu;
 	struct snd_emu10k1_midi *midi = (struct snd_emu10k1_midi *)substream->rmidi->private_data;
 	unsigned long flags;
+	int err = 0;
 
 	emu = midi->emu;
 	snd_assert(emu, return -ENXIO);

@@ -225,11 +240,11 @@ static int snd_emu10k1_midi_output_close
 	midi->substream_output = NULL;
 	if (!(midi->midi_mode & EMU10K1_MIDI_MODE_INPUT)) {
 		spin_unlock_irqrestore(&midi->open_lock, flags);
-		snd_emu10k1_midi_cmd(emu, midi, MPU401_RESET, 0);
+		err = snd_emu10k1_midi_cmd(emu, midi, MPU401_RESET, 0);
 	} else {
 		spin_unlock_irqrestore(&midi->open_lock, flags);
 	}
-	return 0;
+	return err;
 }
 
 static void snd_emu10k1_midi_input_trigger(struct snd_rawmidi_substream *substream, int up)

Andrew Morton

2006-07-04 07:05 UTC #1 lore raw

Quoted 9 lines · On Mon, 03 Jul 2006 13:47:29 -0700 Randy Dunlap <[email protected]> wrote:

This patch fixes the remaining instances in our tree where a non- existent mpu401 interface can cause a hard freeze when i/o is issued.
This commit closes Malone #34831.
Bug https://launchpad.net/distros/ubuntu/+source/linux-source-2.6.15/+bug/34831
patch location:
http://www.kernel.org/git/?p=linux/kernel/git/bcollins/ubuntu-dapper.git;a=commitdiff;h=b422309cdd980cfefe99379796c04e961d3c1544

Do we know who wrote this patch?

Daniel T. Chen

2006-07-04 07:53 UTC #2 lore raw

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

[Apologies in advance for duplicated subscribers]

Quoted 3 lines · Andrew Morton

Quoted 1 line
http://www.kernel.org/git/?p=linux/kernel/git/bcollins/ubuntu-dapper.git;a=commitdiff;h=b422309cdd980cfefe99379796c04e961d3c1544
Do we know who wrote this patch?

It's mine. [0], [1], and [2] contain relevant context as well. The above commit is insufficient as a fix; the current Ubuntu Dapper kernel (2.6.15-25.43) does not have the patch from [2].

I have been unable to reproduce the hard freeze using Dapper's current kernel, but others commenting in [0] are able. The patch from [2] only seems to lessen the frequency of the freezes on certain hardware.

[0] http://launchpad.net/bugs/34831
[1] http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=376382
[2] https://bugtrack.alsa-project.org/alsa-bug/view.php?id=952

Thanks,
- --
Daniel T. Chen [email protected]
GPG key: www.sh.nu/~crimsun/pubkey.gpg.asc
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.2.2 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFEqh35e9GwFciKvaMRAuFhAJ9ez+8Uwy//GfTVyvUpulBPnTxQeQCgpCLB UAfNw1l1S9esliGWkvNA2so=
=RLDy
-----END PGP SIGNATURE-----

Clemens Ladisch

2006-07-04 09:09 UTC #3 lore raw

In reply to Randy Dunlap

Quoted 6 lines · Randy Dunlap

This patch fixes the remaining instances in our tree where a non- existent mpu401 interface can cause a hard freeze when i/o is issued.
sound/pci/emu10k1/emu10k1x.c
sound/pci/emu10k1/emumpu401.c

These drivers are for chips where we know that an MPU-401 interface exists.

Are there any freezes on these devices that can be resolved by this patch?

Regards,
Clemens

Takashi Iwai

2006-07-04 13:02 UTC #4 lore raw

At Tue, 04 Jul 2006 11:09:32 +0200,

Quoted 6 lines · Clemens Ladisch

Quoted 4 lines · Randy Dunlap
This patch fixes the remaining instances in our tree where a non- existent mpu401 interface can cause a hard freeze when i/o is issued.

Quoted 8 lines

Quoted 2 lines
sound/pci/emu10k1/emu10k1x.c
sound/pci/emu10k1/emumpu401.c
These drivers are for chips where we know that an MPU-401 interface exists.
Are there any freezes on these devices that can be resolved by this patch?

Maybe this fix is irrelevant. But it's anyway better to check the errors there.

My rough guess of the culprit is the spin deadlock through snd-rtctimer.

Takashi