Morimoto-san,
sorry for the late reply. It took time until I digest all pending stuff after vacation.
On Wed, 27 Sep 2017 07:14:21 +0200, Kuninori Morimoto wrote:
Hi Takashi
snd_pcm_drop() has this check
if (runtime->status->state == SNDRV_PCM_STATE_OPEN || runtime->status->state == SNDRV_PCM_STATE_DISCONNECTED) return -EBADFD;
(snip)
Then, 2nd issue happen on snd_pcm_stop(). it will call snd_pcm_do_stop(), and it has
if (substream->runtime->trigger_master == substream && snd_pcm_running(substream)) substream->ops->trigger(substream, SNDRV_PCM_TRIGGER_STOP);
I used your patch, and modified above, then, hot-unplug during playback stops correctly without kernel panic. snd_pcm_drop() and snd_pcm_do_stop() care about SNDRV_PCM_STATE_DISCONNECTED on this patch. I think it means, "it should be stopped immediately if it was disconnected". But, I don't know this is OK or Not.
I added my local patch on this mail. Maybe we want to separate this patch into few small patches. but can you review this ? It is including
- your patch
- snd_pcm_stop() snd_pcm_do_stop() care DISCONNECTED
That needs a bit more investigation. When the device is disconnected, not all drivers expect that further PCM operations are done for non-existing devices. We might need either some flag to allow/prefer the stop-after-disconnection, or rethink whether we should actually stop at snd_pcm_dev_disconnect() like below.
- ASoC version of snd_card_disconnect_sync()
- user driver (= rsnd) uses snd_soc_card_disconnect_sync()
Yes, these should be split.
thanks,
Takashi
--- diff --git a/sound/core/pcm.c b/sound/core/pcm.c index 7eadb7fd8074..054e47ad23ed 100644 --- a/sound/core/pcm.c +++ b/sound/core/pcm.c @@ -1152,6 +1152,10 @@ static int snd_pcm_dev_disconnect(struct snd_device *device) for (substream = pcm->streams[cidx].substream; substream; substream = substream->next) { snd_pcm_stream_lock_irq(substream); if (substream->runtime) { + if (snd_pcm_running(substream)) + snd_pcm_stop(substream, + SNDRV_PCM_STATE_DISCONNECTED); + /* to be sure, set the state unconditionally */ substream->runtime->status->state = SNDRV_PCM_STATE_DISCONNECTED; wake_up(&substream->runtime->sleep); wake_up(&substream->runtime->tsleep);