ASoC: meson: aiu-fifo: prevent sleeping when spinlock is held
PATCH 1/1 ASoC: meson: aiu-fifo: prevent sleeping when spinlock is held
dai_links in the gx-card are atomic which means that the DAI trigger callback is called with the spinlock held. This means that usage of potentially sleeping functions (e.g. mutexes) is not allowed here.
'snd_soc_component_write'/'snd_soc_component_read' take the component's io_mutex on entry.
Replace usage of these functions in:
- 'aiu_fifo_i2s_trigger'
- 'fifo_spdif_trigger'
- 'aiu_fifo_pointer'
This is meant to resolve the following runtime bug found while starting I2S playback on an OdroidC2 device where the kernel was built enabling CONFIG_DEBUG_ATOMIC_SLEEP and CONFIG_PROVE_LOCKING:
Playback device is hw:0,0
Stream parameters are 44100Hz, S16_LE, 2 channels Sine wave rate is 1000.0000Hz
Rate set to 44100Hz (requested 44100Hz) Buffer size range from 128 to 262144
Period size range from 64 [ 21.012074] BUG: sleeping function called from invalid context at kernel/locking/mutex.c:623
[ 21.018009] in_atomic(): 1, irqs_disabled(): 1, non_block: 0, pid: 267, name: speaker-test
[ 21.026184] preempt_count: 1, expected: 0
[ 21.030151] RCU nest depth: 0, expected: 0
[ 21.034205] locks held by speaker-test/267: 1, last CPU#3:
[ 21.039638] #0: ffff000003dbd910 (&group->lock#2){....}-{3:3}, at: snd_pcm_stream_lock_irq+0x40/0x60
[ 21.048782] irq event stamp: 5274
[ 21.052059] hardirqs last enabled at (5273): [<ffff800081494240>] _raw_spin_unlock_irq+0x38/0x80
[ 21.060856] hardirqs last disabled at (5274): [<ffff8000814935c8>] _raw_spin_lock_irq+0x80/0x90
[ 21.069481] softirqs last enabled at (5238): [<ffff8000800f9aec>] handle_softirqs+0x584/0x5e0
[ 21.078020] softirqs last disabled at (5231): [<ffff800080010324>] __do_softirq+0x1c/0x28
[ 21.086133] CPU: 3 UID: 0 PID: 267 Comm: speaker-test Not tainted 7.3.0 #2 PREEMPT
[ 21.086141] Hardware name: Hardkernel ODROID-C2 (DT)
[ 21.086145] Call trace:
[ 21.086149] show_stack+0x20/0x40 (C)
[ 21.086160] dump_stack_lvl+0x6c/0x98
[ 21.086171] dump_stack+0x18/0x24
[ 21.086179] __might_resched+0x160/0x230
[ 21.086189] __might_sleep+0x50/0x90
[ 21.086197] __mutex_lock+0x64/0x8a0
[ 21.086206] mutex_lock_nested+0x2c/0x40
[ 21.086215] snd_soc_component_write+0x38/0x80
[ 21.086227] aiu_fifo_i2s_trigger+0x74/0x90
[ 21.086237] soc_dai_trigger.part.0+0x44/0x90
[ 21.086246] snd_soc_pcm_dai_trigger+0x174/0x280
[ 21.086255] soc_pcm_trigger+0xe4/0x1f0
[ 21.086265] dpcm_dai_trigger_fe_be+0x30/0xb0
[ 21.086276] dpcm_fe_dai_do_trigger+0x11c/0x1a0
[ 21.086287] dpcm_fe_dai_trigger+0x40/0x60
[ 21.086296] snd_pcm_do_start+0x4c/0x80
[ 21.086307] snd_pcm_action_single+0x50/0xc0
[ 21.086316] snd_pcm_action+0x88/0xb0
[ 21.086325] snd_pcm_start+0x2c/0x40
[ 21.086335] __snd_pcm_lib_xfer+0x660/0x810
[ 21.086345] snd_pcm_common_ioctl+0xa20/0x17e0
[ 21.086356] snd_pcm_ioctl+0x3c/0x60
[ 21.086366] __arm64_sys_ioctl+0xac/0x110
[ 21.086376] invoke_syscall+0x5c/0x120
[ 21.086386] el0_svc_common.constprop.0+0x48/0xf0
[ 21.086395] do_el0_svc+0x24/0x40
[ 21.086404] el0_svc+0x54/0x3b0
[ 21.086414] el0t_64_sync_handler+0xa0/0xf0
[ 21.086424] el0t_64_sync+0x198/0x1a0---
I found this issue while working on [1].
This patch is independent of [1] and they can go in in any order. They are fixing the same problem in different modules.
[1]: https://lore.kernel.org/all/20261021-gx-formatter-use-spinlock-v1-1-71ebc9224c03@baylibre.com/
--- sound/soc/meson/aiu-fifo-i2s.c | 15 ++++++++++++--- sound/soc/meson/aiu-fifo-spdif.c | 14 ++++++++++---- sound/soc/meson/aiu-fifo.c | 6 +++++- 3 files changed, 27 insertions(+), 8 deletions(-)
sound/soc/meson/aiu-fifo-i2s.c +12 -3
@@ -5,6 +5,7 @@#include <linux/bitfield.h> #include <linux/clk.h> +#include <linux/regmap.h> #include <sound/pcm_params.h> #include <sound/soc.h> #include <sound/soc-dai.h>
@@ -48,14 +49,22 @@ static int aiu_fifo_i2s_trigger(struct snd_pcm_substream *substream, int cmd, struct snd_soc_dai *dai) {struct snd_soc_component *component = dai->component; +unsigned int val; +int ret;switch (cmd) {case SNDRV_PCM_TRIGGER_START :case SNDRV_PCM_TRIGGER_RESUME :case SNDRV_PCM_TRIGGER_PAUSE_RELEASE : -snd_soc_component_write (component,AIU_RST_SOFT , -AIU_RST_SOFT_I2S_FAST ); -snd_soc_component_read (component,AIU_I2S_SYNC ); + ret =regmap_write (component->regmap,AIU_RST_SOFT ,AIU_RST_SOFT_I2S_FAST ); +if (ret <0 ) +return snd_soc_ret (component->dev, ret, +"Failed to write register on trigger" ); + + ret =regmap_read (component->regmap,AIU_I2S_SYNC , &val); +if (ret <0 ) +return snd_soc_ret (component->dev, ret, +"Failed to read register on trigger" );break ; }
sound/soc/meson/aiu-fifo-spdif.c +10 -4
@@ -69,15 +69,21 @@ static int fifo_spdif_trigger(struct snd_pcm_substream *substream, int cmd,case SNDRV_PCM_TRIGGER_START :case SNDRV_PCM_TRIGGER_RESUME :case SNDRV_PCM_TRIGGER_PAUSE_RELEASE : -snd_soc_component_write (component,AIU_RST_SOFT , -AIU_RST_SOFT_958_FAST ); + ret =regmap_write (component->regmap,AIU_RST_SOFT ,AIU_RST_SOFT_958_FAST ); +if (ret <0 ) +return snd_soc_ret (component->dev, ret, +"Failed to write register on trigger" ); +fifo_spdif_dcu_enable (component, true);break ;case SNDRV_PCM_TRIGGER_SUSPEND :case SNDRV_PCM_TRIGGER_PAUSE_PUSH :case SNDRV_PCM_TRIGGER_STOP : -snd_soc_component_write (component,AIU_RST_SOFT , -AIU_RST_SOFT_958_FAST ); + ret =regmap_write (component->regmap,AIU_RST_SOFT ,AIU_RST_SOFT_958_FAST ); +if (ret <0 ) +return snd_soc_ret (component->dev, ret, +"Failed to write register on trigger" ); +fifo_spdif_dcu_enable (component, false);break ;default :
sound/soc/meson/aiu-fifo.c +5 -1
@@ -5,6 +5,7 @@#include <linux/bitfield.h> #include <linux/clk.h> +#include <linux/regmap.h> #include <linux/dma-mapping.h> #include <sound/pcm_params.h> #include <sound/soc.h>
@@ -37,8 +38,11 @@ snd_pcm_uframes_t aiu_fifo_pointer(struct snd_soc_component *component,struct aiu_fifo *fifo =snd_soc_dai_dma_data_get_playback (dai);struct snd_pcm_runtime *runtime = substream->runtime;unsigned int addr; +int ret; - addr =snd_soc_component_read (component, fifo->mem_offset +AIU_MEM_RD ); + ret =regmap_read (component->regmap, fifo->mem_offset +AIU_MEM_RD , &addr); +if (ret <0 ) +return snd_soc_ret (component->dev, ret,"Failed to read FIFO pointer" );return bytes_to_frames (runtime, addr - (unsigned int )runtime->dma_addr); }
---
base-commit: 6628e7d4c6b25c8d18f1345e3a20d21829aae7e4
change-id: 20261111-aiu-fifo-fix-sleep-on-spinlock-0ecec8d64a9b
Best regards,
Signature · 1 line
Valerio Setti <[email protected]>