diff mbox series

ALSA: cmipci: Allow disabling MPU port via module option

Message ID 20191217081448.1144-1-tiwai@suse.de (mailing list archive)
State New, archived
Headers show
Series ALSA: cmipci: Allow disabling MPU port via module option | expand

Commit Message

Takashi Iwai Dec. 17, 2019, 8:14 a.m. UTC
Patrick May reported that his sound card with CMI8378 chip causes a
crash / reboot when accessing the MIDI port that isn't actually
present on the board.  Moreover, despite of the documentation,
passing mpu_port=0 doesn't disable the MIDI port on this board.

It implies that the chip is a newer revision and the MPU401 port is
integrated and mapped on the PCI register.  For this chip model, the
driver enables the MPU port unconditionally, so far.

Although fixing the unexpected reboot would be the best solution, it's
not so trivial to identify the cause.  So, as a plan B, this patch
extends the existing mpu_port option usage to allow disabling the port
by specifying the value 0, just like we applied for fm_port option in
commit 2f24d159d5ac ("[ALSA] cmipci - Allow to disable integrated FM
port").  As default, the MPU port is still enabled, but user can pass
mpu_port=0 to disable it.

Reported-and-tested-by: Patrick May <dusthillresident@gmail.com>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 Documentation/sound/alsa-configuration.rst | 3 ++-
 sound/pci/cmipci.c                         | 5 +++--
 2 files changed, 5 insertions(+), 3 deletions(-)

Comments

Clemens Ladisch Dec. 17, 2019, 4:18 p.m. UTC | #1
Takashi Iwai wrote:
> Patrick May reported that his sound card with CMI8378 chip causes a
> crash / reboot when accessing the MIDI port that isn't actually
> present on the board.  [...]
> Although fixing the unexpected reboot would be the best solution, it's
> not so trivial to identify the cause.

There was a similar problem in the CMI8788 driver:
https://git.kernel.org/linus/f1bc07af9a9e
https://bugtrack.alsa-project.org/alsa-bug/view.php?id=4496 (does this still exist?)

When reading a register repeatedly, the chip sometimes does not
complete the PCI transaction.  Apparently, mainboard PCI chipsets
ignore this error, but PCIe/PCI bridges generate a machine check
exception in this case.

When writing MIDI data to a MPU-401-like device, the driver regularly
polls the status register.  I expect that this will blow up with any
C-Media chip on a card with a PCIe/PCI bridge, regardless of whether
a MIDI port exists.

(I wonder if this can be triggered with snd_pcm_avail() in a loop.
Maybe it happens only for byte accesses, or nobody has yet found out.)

> As default, the MPU port is still enabled

It might be a good idea to disable it by default if the parent
bridge is an ASM1083.


Regards,
Clemens
diff mbox series

Patch

diff --git a/Documentation/sound/alsa-configuration.rst b/Documentation/sound/alsa-configuration.rst
index 02aacd69ab96..392875a1b94e 100644
--- a/Documentation/sound/alsa-configuration.rst
+++ b/Documentation/sound/alsa-configuration.rst
@@ -495,7 +495,8 @@  Module for C-Media CMI8338/8738/8768/8770 PCI sound cards.
 mpu_port
     port address of MIDI interface (8338 only):
     0x300,0x310,0x320,0x330 = legacy port,
-    0 = disable (default)
+    1 = integrated PCI port (default on 8738),
+    0 = disable
 fm_port
     port address of OPL-3 FM synthesizer (8x38 only):
     0x388 = legacy port,
diff --git a/sound/pci/cmipci.c b/sound/pci/cmipci.c
index 4bfab21c53f4..266c4cf28b78 100644
--- a/sound/pci/cmipci.c
+++ b/sound/pci/cmipci.c
@@ -42,7 +42,7 @@  MODULE_SUPPORTED_DEVICE("{{C-Media,CMI8738},"
 static int index[SNDRV_CARDS] = SNDRV_DEFAULT_IDX;	/* Index 0-MAX */
 static char *id[SNDRV_CARDS] = SNDRV_DEFAULT_STR;	/* ID for this card */
 static bool enable[SNDRV_CARDS] = SNDRV_DEFAULT_ENABLE_PNP;	/* Enable switches */
-static long mpu_port[SNDRV_CARDS];
+static long mpu_port[SNDRV_CARDS] = {[0 ... (SNDRV_CARDS-1)] = 1};
 static long fm_port[SNDRV_CARDS] = {[0 ... (SNDRV_CARDS-1)]=1};
 static bool soft_ac3[SNDRV_CARDS] = {[0 ... (SNDRV_CARDS-1)]=1};
 #ifdef SUPPORT_JOYSTICK
@@ -3132,7 +3132,8 @@  static int snd_cmipci_create(struct snd_card *card, struct pci_dev *pci,
 	if (cm->chip_version >= 39) {
 		val = snd_cmipci_read_b(cm, CM_REG_MPU_PCI + 1);
 		if (val != 0x00 && val != 0xff) {
-			iomidi = cm->iobase + CM_REG_MPU_PCI;
+			if (mpu_port[dev])
+				iomidi = cm->iobase + CM_REG_MPU_PCI;
 			integrated_midi = 1;
 		}
 	}