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 EFE39C0015E for ; Wed, 26 Jul 2023 12:32:08 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232554AbjGZMcH (ORCPT ); Wed, 26 Jul 2023 08:32:07 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:41518 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232553AbjGZMcH (ORCPT ); Wed, 26 Jul 2023 08:32:07 -0400 Received: from smtp-out1.suse.de (smtp-out1.suse.de [IPv6:2001:67c:2178:6::1c]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 47117E61; Wed, 26 Jul 2023 05:32:00 -0700 (PDT) Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out1.suse.de (Postfix) with ESMTPS id D7A2021CB9; Wed, 26 Jul 2023 12:31:58 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1690374718; 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=rQRJ4YwQA/6O0WXa/1Z5ActdMgPE2S7rN2kN4ZsnP5s=; b=sBohaG/6wQ0MNG9epgDlnJLCBjqa7F+dl7OQQwWTyzLvPMjFK9Za3zLY+0QNKFAGk8v4CU Q3BcKt+i+WNHpAgpsFU40HGoPIB8YUVDcdem+zT+KpIFphRtfOo6KbVm90W2/fWtjl3bWn H393pY7OIvuzH1hTOOJ+d+yz7fwj4bQ= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1690374718; 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=rQRJ4YwQA/6O0WXa/1Z5ActdMgPE2S7rN2kN4ZsnP5s=; b=6NQCnDnvw0PBrRBwW12lm/uW8aVoTQyAKwUdfFpg4zDta1dnUnd3kuke3mhlpepVyJLKsG BJFwhGeXeoBjnkBw== Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap2.suse-dmz.suse.de (Postfix) with ESMTPS id 91720139BD; Wed, 26 Jul 2023 12:31:58 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id x/X4Ij4SwWQdSAAAMHmgww (envelope-from ); Wed, 26 Jul 2023 12:31:58 +0000 Date: Wed, 26 Jul 2023 14:31:58 +0200 Message-ID: <87msziyhtt.wl-tiwai@suse.de> From: Takashi Iwai To: Wesley Cheng Cc: , , , , , , , , , , , , , , , , , , , , , , , , , , Subject: Re: [PATCH v4 18/32] sound: usb: Introduce QC USB SND offloading support In-Reply-To: <243ee81d-d46d-e05a-1fcd-35e6301a39cd@quicinc.com> References: <20230725023416.11205-1-quic_wcheng@quicinc.com> <20230725023416.11205-19-quic_wcheng@quicinc.com> <87bkg0v4ce.wl-tiwai@suse.de> <243ee81d-d46d-e05a-1fcd-35e6301a39cd@quicinc.com> User-Agent: Wanderlust/2.15.9 (Almost Unreal) Emacs/27.2 Mule/6.0 MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") Content-Type: text/plain; charset=US-ASCII Precedence: bulk List-ID: X-Mailing-List: linux-arm-msm@vger.kernel.org On Wed, 26 Jul 2023 00:59:57 +0200, Wesley Cheng wrote: > > >> +static int enable_audio_stream(struct snd_usb_substream *subs, > >> + snd_pcm_format_t pcm_format, > >> + unsigned int channels, unsigned int cur_rate, > >> + int datainterval) > >> +{ > > > > ... this implementation, I wonder whether it'd be better to modify and > > export snd_usb_hw_params() snd snd_usb_hw_free() to fit with qcom > > driver. Then you can avoid lots of open code. > > > > I think the problem is that snd_usb_hw_params assumes that we've > already done a PCM open on the PCM device created by USB SND. > However, with the offload path, we don't reference the USB PCM device, > but the one created by the platform sound card. Hence, I don't have > access to the snd_pcm_substream. > > I attempted to derive snd_pcm_substream from snd_usb_substream, but > since PCM open isn't run, it doesn't provide a valid structure. > > What do you think about adding a wrapper to snd_usb_hw_params? Have a > version that will take in snd_usb_substream, and another that is > registered to hw_params(). Yes, that's what I had in mind, too. > > In general, if you see a direct use of chip->mutex, it can be often > > done better in a different form. The use of an internal lock or such > > from an external driver is always fragile and error-prone. > > > > Also, the current open-code misses the potential race against the > > disconnection during the operation. In snd-usb-audio, it protects > > with snd_usb_lock_shutdown() and snd_usb_unlock_shutdown() pairs. > > > > I agree...I think then the best approach would be something like the > above, ie: > > int snd_usb_hw_params(struct snd_pcm_substream *substream, > struct snd_pcm_hw_params *hw_params) > { > struct snd_usb_substream *subs = substream->runtime->private_data; > > snd_usb_ep_attach(subs, hw_params); > ... > > int snd_usb_ep_attach(...) > { > //implementation of current code in snd_usb_hw_params() > } > EXPORT_SYMBOL(snd_usb_ep_attach); Yes, exactly something like that ;) thanks, Takashi