From patchwork Fri Oct 6 13:19:32 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Takashi Iwai X-Patchwork-Id: 9989275 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 6578560247 for ; Fri, 6 Oct 2017 13:19:45 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 6EF1428CA4 for ; Fri, 6 Oct 2017 13:19:45 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 625C728D9D; Fri, 6 Oct 2017 13:19:45 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-1.9 required=2.0 tests=BAYES_00, RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.1 Received: from alsa0.perex.cz (alsa0.perex.cz [77.48.224.243]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id A07B528CA4 for ; Fri, 6 Oct 2017 13:19:43 +0000 (UTC) Received: from alsa0.perex.cz (localhost [127.0.0.1]) by alsa0.perex.cz (Postfix) with ESMTP id 08D692675BB; Fri, 6 Oct 2017 15:19:39 +0200 (CEST) X-Original-To: alsa-devel@alsa-project.org Delivered-To: alsa-devel@alsa-project.org Received: by alsa0.perex.cz (Postfix, from userid 1000) id 2ED9F2675DE; Fri, 6 Oct 2017 15:19:37 +0200 (CEST) Received: from mx1.suse.de (mx2.suse.de [195.135.220.15]) by alsa0.perex.cz (Postfix) with ESMTP id 0B769266991 for ; Fri, 6 Oct 2017 15:19:34 +0200 (CEST) X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (charybdis-ext.suse.de [195.135.220.254]) by mx1.suse.de (Postfix) with ESMTP id 6CD41AB1A; Fri, 6 Oct 2017 13:19:34 +0000 (UTC) Date: Fri, 06 Oct 2017 15:19:32 +0200 Message-ID: From: Takashi Iwai To: Kuninori Morimoto In-Reply-To: <87bmlw6880.wl%kuninori.morimoto.gx@renesas.com> References: <874lsnniv3.wl%kuninori.morimoto.gx@renesas.com> <87d17akgqr.wl%kuninori.morimoto.gx@renesas.com> <87h8vpc1xe.wl%kuninori.morimoto.gx@renesas.com> <87bmlw6880.wl%kuninori.morimoto.gx@renesas.com> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI/1.14.6 (Maruoka) FLIM/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL/10.8 Emacs/25.3 (x86_64-suse-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Cc: Linux-ALSA , Mark Brown Subject: Re: [alsa-devel] [PATCH] ASoC: rsnd: stop all working stream when .remove X-BeenThere: alsa-devel@alsa-project.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: "Alsa-devel mailing list for ALSA developers - http://www.alsa-project.org" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: alsa-devel-bounces@alsa-project.org Sender: alsa-devel-bounces@alsa-project.org X-Virus-Scanned: ClamAV using ClamSMTP 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);