From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 35D0DC433EF for ; Tue, 15 Mar 2022 15:34:49 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1349660AbiCOPf7 (ORCPT ); Tue, 15 Mar 2022 11:35:59 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:40270 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231708AbiCOPf5 (ORCPT ); Tue, 15 Mar 2022 11:35:57 -0400 Received: from smtp-out2.suse.de (smtp-out2.suse.de [195.135.220.29]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id A592B53B7F for ; Tue, 15 Mar 2022 08:34:45 -0700 (PDT) Received: from relay2.suse.de (relay2.suse.de [149.44.160.134]) by smtp-out2.suse.de (Postfix) with ESMTP id 531F41F391; Tue, 15 Mar 2022 15:34:44 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1647358484; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=5ha63cA1iMjHX8s87c7n2kolHfqFbmLzNe1WLNjs1WE=; b=bMCllI0OEAhpTR/xo/nQZpGembmiqDKLvCjpZsLaRwk38/hIQOlo2N0vLq7HJPuH9lzS6C Uq1Uc2Ce5qyLN0s4/EHpmvktOGsUCXnAcSspEMS4bvDPOhARI/cHcN9Jt2JPrtKBDAY4Cz Jtb/OpIJ0w5dzanOsSGBN6/ilO/CG10= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1647358484; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=5ha63cA1iMjHX8s87c7n2kolHfqFbmLzNe1WLNjs1WE=; b=IYqjuc4KM10+WvhWGIkJ3JehgCkXUN0ALfdgcnbvvFSNq9VIgyt/PUFiRFAJLN0bTkDd4x /X7ED/9OJsRhglDw== Received: from alsa1.suse.de (alsa1.suse.de [10.160.4.42]) by relay2.suse.de (Postfix) with ESMTP id F14DBA3B81; Tue, 15 Mar 2022 15:34:43 +0000 (UTC) Date: Tue, 15 Mar 2022 16:34:43 +0100 Message-ID: From: Takashi Iwai To: Arnaud POULIQUEN Cc: Daniel Palmer , "broonie@kernel.org" , "tiwai@suse.com" , "alsa-devel@alsa-project.org" , "linux-kernel@vger.kernel.org" Subject: Re: [PATCH] ASoC: sti: sti_uniperif: Remove driver In-Reply-To: References: <20220315091319.3351522-1-daniel@0x0f.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") Content-Type: text/plain; charset=US-ASCII Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, 15 Mar 2022 16:27:40 +0100, Arnaud POULIQUEN wrote: > > > > > ST Restricted > > > -----Original Message----- > > From: Takashi Iwai > > Sent: mardi 15 mars 2022 15:35 > > To: Arnaud POULIQUEN > > Cc: Daniel Palmer ; broonie@kernel.org; tiwai@suse.com; > > alsa-devel@alsa-project.org; linux-kernel@vger.kernel.org > > Subject: Re: [PATCH] ASoC: sti: sti_uniperif: Remove driver > > > > On Tue, 15 Mar 2022 14:15:20 +0100, > > Arnaud POULIQUEN wrote: > > > > > > Hello, > > > > > > > > > ST Restricted > > > > > > > -----Original Message----- > > > > From: Takashi Iwai > > > > Sent: mardi 15 mars 2022 11:28 > > > > To: Daniel Palmer > > > > Cc: broonie@kernel.org; tiwai@suse.com; Arnaud POULIQUEN > > > > ; alsa-devel@alsa-project.org; linux- > > > > kernel@vger.kernel.org > > > > Subject: Re: [PATCH] ASoC: sti: sti_uniperif: Remove driver > > > > > > > > On Tue, 15 Mar 2022 10:13:19 +0100, > > > > Daniel Palmer wrote: > > > > > > > > > > This driver seems to be in the "only good for attracting bot > > > > > generated patches" phase of it's life. > > > > > > > > > > It doesn't seem like anyone actually tested the patches that have > > > > > been applied in the last few years as uni_reader_irq_handler() had > > > > > a dead lock added to it (it locks the stream, then calls > > > > > snd_pcm_stop_xrun() which will also lock the stream). > > > > > > > > Mea culpa, that was an obvious deadlock I overlooked in the patch > > > > series. > > > > > > > > > Seems best just to remove it. > > > > > > > > > > Signed-off-by: Daniel Palmer > > > > > --- > > > > > I've never used this driver, don't have the hardware etc. > > > > > I just noticed that this looks broken when debugging my own > > > > > driver that uses snd_pcm_stop_xrun() and was looking at other > > > > > users to see if I was using it wrong and noticed this was the > > > > > only place that locked the stream before calling > > > > > snd_pcm_stop_xrun(). > > > > > > > > > > There are probably some other bits of the driver that should be > > > > > removed but I didn't look that hard. > > > > > > > > > > TL;DR; This driver seems broken, seems like nobody uses it. > > > > > Maybe it should be deleted? > > > > > > > > Yeah, that looks dead. > > > > > > > > > > The platform is still used for instance: > > > https://lore.kernel.org/all/1d95209f-9cb4-47a3-2696-7a93df7cdc05@foss. > > > st.com/ > > > > > > So please do not remove the driver > > > > Ah, it's always good to see a vital sign! > > > > > The issue has not been detected because it is related to an error that > > > occurs only when we reach the limit of the platform, with application > > > that stop the stream at same time. > > > So almost no chance to occur. > > > > > > > OTOH, if anyone really wants to keep the stuff, please revert the > > > > commit dc865fb9e7c2251c9585ff6a7bf185d499db13e4. > > > > > > Yes reverting the commit is one solution. > > > The other is to clean-up the snd_pcm_stream_lock/ > > > snd_pcm_stream_unlock in the Handler. > > > > That would work, but maybe it's safer to keep that lock, as the state change > > isn't protected by irq_lock but only implicitly by stream lock in start/stop > > callbacks. > > You are right, trying to use the snd_pcm_stop_xrun needs deeper update > that could introduce regression. > It seems wiser to revert your commit dc865fb9e7c2 as you propose. OK, I'm going to submit the revert. thanks, Takashi