[MT7688] Stopping audio cases crash

Hi,

I'm using the LinkIt Smart 7688 SBC with the baseboard which comes with a WM8960 audio codec. I'm on the 23.05 branch, but I have tried 22.03 as well. This is my change set adding the audio definitions (inspired by this https://openwrt.org/toh/seeed/linkit7688#seeedstudio_breakout_for_linkit_smart_7688_v20):

diff --git a/target/linux/ramips/dts/mt7628an_mediatek_linkit-smart-7688.dts b/target/linux/ramips/dts/mt7628an_mediatek_linkit-smart-7688.dts
index 57624c302e..aa1191b2ed 100644
--- a/target/linux/ramips/dts/mt7628an_mediatek_linkit-smart-7688.dts
+++ b/target/linux/ramips/dts/mt7628an_mediatek_linkit-smart-7688.dts
@@ -46,6 +46,25 @@
 			linux,code = <KEY_WPS_BUTTON>;
 		};
 	};
+
+	sound {
+		compatible = "simple-audio-card";
+		simple-audio-card,name = "Audio-I2S";
+		simple-audio-card,format = "i2s";
+		simple-audio-card,bitclock-master = <&dailink0_master>;
+		simple-audio-card,frame-master = <&dailink0_master>;
+		simple-audio-card,widgets = "Headphone", "Headphones";
+		simple-audio-card,routing = "Headphones", "HP_L", "Headphones", "HP_R";
+		simple-audio-card,mclk-fs = <256>;
+
+		simple-audio-card,cpu {
+			sound-dai = <&i2s>;
+		};
+
+		dailink0_master: simple-audio-card,codec {
+			sound-dai = <&codec>;
+		};
+	};
 };
 
 &state_default {
@@ -59,11 +78,6 @@
 		function = "gpio";
 	};
 
-	i2s {
-		groups = "i2s";
-		function = "gpio";
-	};
-
 	spis {
 		groups = "spis";
 		function = "gpio";
@@ -132,6 +146,25 @@
 
 &i2c {
 	status = "okay";
+
+	codec: wm8960@1a {
+		#sound-dai-cells = <0>;
+		compatible = "wlf,wm8960";
+		reg = <0x1a>;
+
+		wlf,shared-lrclk;
+	};
+};
+
+&i2s {
+	#sound-dai-cells = <0>;
+	status = "okay";
+	pinctrl-names = "default";
+	pinctrl-0 = <&i2s_pins>;
+};
+
+&gdma {
+	status = "okay";
 };
 
 &uart1 {
diff --git a/target/linux/ramips/image/mt76x8.mk b/target/linux/ramips/image/mt76x8.mk
index 816009ac18..d0dfe3765b 100644
--- a/target/linux/ramips/image/mt76x8.mk
+++ b/target/linux/ramips/image/mt76x8.mk
@@ -375,7 +375,7 @@ define Device/mediatek_linkit-smart-7688
   IMAGE_SIZE := 32448k
   DEVICE_VENDOR := MediaTek
   DEVICE_MODEL := LinkIt Smart 7688
-  DEVICE_PACKAGES:= kmod-usb2 kmod-usb-ohci uboot-envtools kmod-sdhci-mt7620
+  DEVICE_PACKAGES:= kmod-usb2 kmod-usb-ohci uboot-envtools kmod-sdhci-mt7620 kmod-sound-mt7620
   SUPPORTED_DEVICES += linkits7688 linkits7688d
 endef
 TARGET_DEVICES += mediatek_linkit-smart-7688

Playback works well, e.g. using aplay, however, when I stop playback with Ctrl+C, the board immediately freezes and eventually reboots (watchdog, I guess). If I let the audio play to completion, everything works as expected. Does anyone have an idea what could be wrong here?

Skipping disabling the I2S transmitter (setting CFG0_TX_EN=0) in ralink_i2s_trigger fixes the issue, i.e. stopping the audio does not crash the system anymore. Could the crash be due to a pending DMA transfer trying to write to the TX FIFO when the transmitter is already disabled? If so, would this be a bug in the DAI driver or in the DMA driver?

hi @svenschwermer I have the same issue, and am trying to replicate what you did in what I'm assuming is the 835-asoc-add-mt7620-support.patch in ralink_i2s_trigger.

Are you saying that on TRIGGER_STOP/SUSPEND/PAUSE you simply set the val = mask, instead of val=0?

Am going to give it a try and see if it helps.

I can confirm that changing the patch to look like this works!

+static int ralink_i2s_trigger(struct snd_pcm_substream *substream, int cmd,
+               struct snd_soc_dai *dai)
+{
+       struct ralink_i2s *i2s = snd_soc_dai_get_drvdata(dai);
+       unsigned int mask, val;
+
+       if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
+               mask = I2S_REG_CFG0_TX_EN;
+       else
+               mask = I2S_REG_CFG0_RX_EN;
+
+       switch (cmd) {
+       case SNDRV_PCM_TRIGGER_START:
+       case SNDRV_PCM_TRIGGER_RESUME:
+       case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
+               val = mask;
+               break;
+       case SNDRV_PCM_TRIGGER_STOP:
+       case SNDRV_PCM_TRIGGER_SUSPEND:
+       case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
+               val = mask;
+               break;
+       default:
+               return -EINVAL;
+       }
+
+       regmap_update_bits(i2s->regmap, I2S_REG_CFG0, mask, val);
+
+       return 0;
+}

This is the original code

+static int ralink_i2s_trigger(struct snd_pcm_substream *substream, int cmd,
+               struct snd_soc_dai *dai)
+{
+       struct ralink_i2s *i2s = snd_soc_dai_get_drvdata(dai);
+       unsigned int mask, val;
+
+       if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
+               mask = I2S_REG_CFG0_TX_EN;
+       else
+               mask = I2S_REG_CFG0_RX_EN;
+
+       switch (cmd) {
+       case SNDRV_PCM_TRIGGER_START:
+       case SNDRV_PCM_TRIGGER_RESUME:
+       case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
+               val = mask;
+               break;
+       case SNDRV_PCM_TRIGGER_STOP:
+       case SNDRV_PCM_TRIGGER_SUSPEND:
+       case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
+               val = 0;
+               break;
+       default:
+               return -EINVAL;
+       }
+
+       regmap_update_bits(i2s->regmap, I2S_REG_CFG0, mask, val);
+
+       return 0;
+}