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 X-Spam-Level: X-Spam-Status: No, score=-11.4 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH, MAILING_LIST_MULTI,NICE_REPLY_A,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 0A65AC41604 for ; Tue, 6 Oct 2020 04:43:58 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id BDAEA2075A for ; Tue, 6 Oct 2020 04:43:57 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726118AbgJFEn4 (ORCPT ); Tue, 6 Oct 2020 00:43:56 -0400 Received: from wout4-smtp.messagingengine.com ([64.147.123.20]:44985 "EHLO wout4-smtp.messagingengine.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725806AbgJFEn4 (ORCPT ); Tue, 6 Oct 2020 00:43:56 -0400 Received: from compute5.internal (compute5.nyi.internal [10.202.2.45]) by mailout.west.internal (Postfix) with ESMTP id B6BA53DF; Tue, 6 Oct 2020 00:43:54 -0400 (EDT) Received: from mailfrontend2 ([10.202.2.163]) by compute5.internal (MEProxy); Tue, 06 Oct 2020 00:43:55 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sholland.org; h= subject:to:cc:references:from:message-id:date:mime-version :in-reply-to:content-type:content-transfer-encoding; s=fm3; bh=X 2uGAJogJbE61MC4c1/0d4db+rheQyrn5sPt3B2jfek=; b=C04hKOuPIfiwZn1kb pvrDuEopDqCt7XKTr2jV16rzmpjCMkhbcvTOWnFw8aGUnMJfO9U47YETUyXoP/VZ fu39eqeyt0PzOuvqeQjPO+dDux86UCRvcirx26Jd8MP8QECyeJhQcU41nKvN5i6r 1/zfjtDn9rfePb4aCbaw2E4ipIdS+0C2Plit5jR2MJzOMFPTLGUxmdnxztbNFYX4 d71WyFvKSdcLjQevZaleAK4BLv4MTxzE/5SO6Gut4QgFaJHeujz9s8QHvgah6rN4 Pa81I1CFexmrNwi2o+B6tzN+ruhQj0/YUBmgRD1mpussEIU+GOo7vd7t2ahPSJ03 QF2Sw== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-transfer-encoding:content-type :date:from:in-reply-to:message-id:mime-version:references :subject:to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender :x-sasl-enc; s=fm1; bh=X2uGAJogJbE61MC4c1/0d4db+rheQyrn5sPt3B2jf ek=; b=ceINa3joRWtekgoblZo5972H2/CWGk0s4NapKYMpurufILS7qFFxq/pHH OwpcBvGmYSr1RmDdaJpA6rc1DyakAp2X3a+c+Rd5Pvh8CxZVsdsGkaugMMmouI5d ZrAs1Q1qHiuSbdAVAdUkgcF0ZPkySNIyuNJswjUCPyRYkkVgWQTGge85cQogUX8V x3jV72qBq0bTAP79le6LFzc7XWUhpX8YU3EWuixfB4hH1QE1q45SDFutjIMEmGnB uBpvccvUO0INGUGmV1VmbpzArspxsrA+07z6AUpm4ZUsQSFZIz5pZQ+KavOVvAlN PULg4myGj65fW+F/FsSVUrB1emCvQ== X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedujedrgeefgdekjecutefuodetggdotefrodftvf curfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfghnecu uegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmdenuc fjughrpefuvfhfhffkffgfgggjtgfgsehtjeertddtfeehnecuhfhrohhmpefurghmuhgv lhcujfholhhlrghnugcuoehsrghmuhgvlhesshhhohhllhgrnhgurdhorhhgqeenucggtf frrghtthgvrhhnpeettdekiedvgeetfeelueduhefftdfhkeehueejvefhtddvffejleeu hfeuhfevkeenucfkphepjedtrddufeehrddugeekrdduhedunecuvehluhhsthgvrhfuih iivgeptdenucfrrghrrghmpehmrghilhhfrhhomhepshgrmhhuvghlsehshhholhhlrghn ugdrohhrgh X-ME-Proxy: Received: from [70.135.148.151] (70-135-148-151.lightspeed.stlsmo.sbcglobal.net [70.135.148.151]) by mail.messagingengine.com (Postfix) with ESMTPA id E440B306467D; Tue, 6 Oct 2020 00:43:51 -0400 (EDT) Subject: Re: [PATCH 20/25] ASoC: sun8i-codec: Protect the clock rate while streams are open To: Maxime Ripard Cc: Mark Brown , Liam Girdwood , Chen-Yu Tsai , Jaroslav Kysela , Takashi Iwai , Ondrej Jirman , alsa-devel@alsa-project.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org References: <20201001021148.15852-1-samuel@sholland.org> <20201001021148.15852-21-samuel@sholland.org> <20201005120101.igzzwosnq6bzbua6@gilmour.lan> From: Samuel Holland Message-ID: <5ef4351f-e9ed-1a38-b79e-53e62a70437e@sholland.org> Date: Mon, 5 Oct 2020 23:43:51 -0500 User-Agent: Mozilla/5.0 (X11; Linux ppc64; rv:78.0) Gecko/20100101 Thunderbird/78.3.1 MIME-Version: 1.0 In-Reply-To: <20201005120101.igzzwosnq6bzbua6@gilmour.lan> Content-Type: text/plain; charset=windows-1252 Content-Language: en-US Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 10/5/20 7:01 AM, Maxime Ripard wrote: > On Wed, Sep 30, 2020 at 09:11:43PM -0500, Samuel Holland wrote: >> The codec's clock input is shared among all AIFs, and shared with other >> audio-related hardware in the SoC, including I2S and SPDIF controllers. >> To ensure sample rates selected by userspace or by codec2codec DAI links >> are maintained, the clock rate must be protected while it is in use. >> >> Signed-off-by: Samuel Holland >> --- >> sound/soc/sunxi/sun8i-codec.c | 25 ++++++++++++++++++++++--- >> 1 file changed, 22 insertions(+), 3 deletions(-) >> >> diff --git a/sound/soc/sunxi/sun8i-codec.c b/sound/soc/sunxi/sun8i-codec.c >> index 501af64d43a0..86065bee7cd3 100644 >> --- a/sound/soc/sunxi/sun8i-codec.c >> +++ b/sound/soc/sunxi/sun8i-codec.c >> @@ -416,27 +416,32 @@ static int sun8i_codec_get_lrck_div_order(unsigned int slots, >> unsigned int div = slots * slot_width; >> >> if (div < 16 || div > 256) >> return -EINVAL; >> >> return order_base_2(div); >> } >> >> +static unsigned int sun8i_codec_get_sysclk_rate(unsigned int sample_rate) >> +{ >> + return sample_rate % 4000 ? 22579200 : 24576000; >> +} >> + >> static int sun8i_codec_hw_params(struct snd_pcm_substream *substream, >> struct snd_pcm_hw_params *params, >> struct snd_soc_dai *dai) >> { >> struct sun8i_codec *scodec = snd_soc_dai_get_drvdata(dai); >> struct sun8i_codec_aif *aif = &scodec->aifs[dai->id]; >> unsigned int sample_rate = params_rate(params); >> unsigned int slots = aif->slots ?: params_channels(params); >> unsigned int slot_width = aif->slot_width ?: params_width(params); >> - unsigned int sysclk_rate = clk_get_rate(scodec->clk_module); >> - int lrck_div_order, word_size; >> + unsigned int sysclk_rate = sun8i_codec_get_sysclk_rate(sample_rate); >> + int lrck_div_order, ret, word_size; >> u8 bclk_div; >> >> /* word size */ >> switch (params_width(params)) { >> case 8: >> word_size = 0x0; >> break; >> case 16: >> @@ -466,17 +471,30 @@ static int sun8i_codec_hw_params(struct snd_pcm_substream *substream, >> (lrck_div_order - 4) << SUN8I_AIF1CLK_CTRL_AIF1_LRCK_DIV); >> >> /* BCLK divider (SYSCLK/BCLK ratio) */ >> bclk_div = sun8i_codec_get_bclk_div(sysclk_rate, lrck_div_order, sample_rate); >> regmap_update_bits(scodec->regmap, SUN8I_AIF1CLK_CTRL, >> SUN8I_AIF1CLK_CTRL_AIF1_BCLK_DIV_MASK, >> bclk_div << SUN8I_AIF1CLK_CTRL_AIF1_BCLK_DIV); >> >> - if (!aif->open_streams) { >> + /* SYSCLK rate */ >> + if (aif->open_streams) { >> + ret = clk_set_rate(scodec->clk_module, sysclk_rate); >> + if (ret < 0) >> + return ret; >> + } else { >> + ret = clk_set_rate_exclusive(scodec->clk_module, sysclk_rate); > > It's not really clear to me why we wouldn't want to always protect the > clock rate here? >From Documentation/sound/kernel-api/writing-an-alsa-driver.rst: hw_params callback ... Note that this and ``prepare`` callbacks may be called multiple times per initialization. For example, the OSS emulation may call these callbacks at each change via its ioctl. Clock rate protection is reference counted, so we must only take one reference (or at least a known number of references) per stream. >> + if (ret == -EBUSY) >> + dev_err(dai->dev, "%s: clock is busy! Sample rate %u Hz " >> + "conflicts with other audio streams.\n", > > This string creates a checkpatch warning. I will put it on one line, though >100 columns is also a checkpatch warning. > Maxime Cheers, Samuel 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 X-Spam-Level: X-Spam-Status: No, score=-11.3 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, NICE_REPLY_A,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED, USER_AGENT_SANE_1 autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id A9811C41604 for ; Tue, 6 Oct 2020 04:45:01 +0000 (UTC) Received: from alsa0.perex.cz (alsa0.perex.cz [77.48.224.243]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id E76662075A for ; Tue, 6 Oct 2020 04:44:59 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=alsa-project.org header.i=@alsa-project.org header.b="VJ4Z/5vQ"; dkim=temperror (0-bit key) header.d=sholland.org header.i=@sholland.org header.b="C04hKOuP"; dkim=temperror (0-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="ceINa3jo" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org E76662075A Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=sholland.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=alsa-devel-bounces@alsa-project.org Received: from alsa1.perex.cz (alsa1.perex.cz [207.180.221.201]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by alsa0.perex.cz (Postfix) with ESMTPS id 5AC661767; Tue, 6 Oct 2020 06:44:08 +0200 (CEST) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa0.perex.cz 5AC661767 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=alsa-project.org; s=default; t=1601959498; bh=Q5qx/pHdP4nsAKRUfQldmWzTFfrslQOmVWhLwl2WTrU=; h=Subject:To:References:From:Date:In-Reply-To:Cc:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From; b=VJ4Z/5vQdj2f6XYvQS8D3VAPTDMvxB+aXQWhwbdPdpPPN+EJSQrIiBlSPspluKaSW DuVb8Pv3nYxIEoU912ROq/Y8msIKozp2vNbgnRkQFX6/ZveTJSTb0HZ2+hmZUNvo84 tszNbRca4fgj5xOG10Mvz6zqfZOcOIhrCPQCe8fA= Received: from alsa1.perex.cz (localhost.localdomain [127.0.0.1]) by alsa1.perex.cz (Postfix) with ESMTP id 89D7EF80218; Tue, 6 Oct 2020 06:44:07 +0200 (CEST) Received: by alsa1.perex.cz (Postfix, from userid 50401) id 21F19F80246; Tue, 6 Oct 2020 06:44:06 +0200 (CEST) Received: from wout4-smtp.messagingengine.com (wout4-smtp.messagingengine.com [64.147.123.20]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by alsa1.perex.cz (Postfix) with ESMTPS id 178A3F800CB for ; Tue, 6 Oct 2020 06:43:58 +0200 (CEST) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa1.perex.cz 178A3F800CB Authentication-Results: alsa1.perex.cz; dkim=pass (2048-bit key) header.d=sholland.org header.i=@sholland.org header.b="C04hKOuP"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="ceINa3jo" Received: from compute5.internal (compute5.nyi.internal [10.202.2.45]) by mailout.west.internal (Postfix) with ESMTP id B6BA53DF; Tue, 6 Oct 2020 00:43:54 -0400 (EDT) Received: from mailfrontend2 ([10.202.2.163]) by compute5.internal (MEProxy); Tue, 06 Oct 2020 00:43:55 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sholland.org; h= subject:to:cc:references:from:message-id:date:mime-version :in-reply-to:content-type:content-transfer-encoding; s=fm3; bh=X 2uGAJogJbE61MC4c1/0d4db+rheQyrn5sPt3B2jfek=; b=C04hKOuPIfiwZn1kb pvrDuEopDqCt7XKTr2jV16rzmpjCMkhbcvTOWnFw8aGUnMJfO9U47YETUyXoP/VZ fu39eqeyt0PzOuvqeQjPO+dDux86UCRvcirx26Jd8MP8QECyeJhQcU41nKvN5i6r 1/zfjtDn9rfePb4aCbaw2E4ipIdS+0C2Plit5jR2MJzOMFPTLGUxmdnxztbNFYX4 d71WyFvKSdcLjQevZaleAK4BLv4MTxzE/5SO6Gut4QgFaJHeujz9s8QHvgah6rN4 Pa81I1CFexmrNwi2o+B6tzN+ruhQj0/YUBmgRD1mpussEIU+GOo7vd7t2ahPSJ03 QF2Sw== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-transfer-encoding:content-type :date:from:in-reply-to:message-id:mime-version:references :subject:to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender :x-sasl-enc; s=fm1; bh=X2uGAJogJbE61MC4c1/0d4db+rheQyrn5sPt3B2jf ek=; b=ceINa3joRWtekgoblZo5972H2/CWGk0s4NapKYMpurufILS7qFFxq/pHH OwpcBvGmYSr1RmDdaJpA6rc1DyakAp2X3a+c+Rd5Pvh8CxZVsdsGkaugMMmouI5d ZrAs1Q1qHiuSbdAVAdUkgcF0ZPkySNIyuNJswjUCPyRYkkVgWQTGge85cQogUX8V x3jV72qBq0bTAP79le6LFzc7XWUhpX8YU3EWuixfB4hH1QE1q45SDFutjIMEmGnB uBpvccvUO0INGUGmV1VmbpzArspxsrA+07z6AUpm4ZUsQSFZIz5pZQ+KavOVvAlN PULg4myGj65fW+F/FsSVUrB1emCvQ== X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedujedrgeefgdekjecutefuodetggdotefrodftvf curfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfghnecu uegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmdenuc fjughrpefuvfhfhffkffgfgggjtgfgsehtjeertddtfeehnecuhfhrohhmpefurghmuhgv lhcujfholhhlrghnugcuoehsrghmuhgvlhesshhhohhllhgrnhgurdhorhhgqeenucggtf frrghtthgvrhhnpeettdekiedvgeetfeelueduhefftdfhkeehueejvefhtddvffejleeu hfeuhfevkeenucfkphepjedtrddufeehrddugeekrdduhedunecuvehluhhsthgvrhfuih iivgeptdenucfrrghrrghmpehmrghilhhfrhhomhepshgrmhhuvghlsehshhholhhlrghn ugdrohhrgh X-ME-Proxy: Received: from [70.135.148.151] (70-135-148-151.lightspeed.stlsmo.sbcglobal.net [70.135.148.151]) by mail.messagingengine.com (Postfix) with ESMTPA id E440B306467D; Tue, 6 Oct 2020 00:43:51 -0400 (EDT) Subject: Re: [PATCH 20/25] ASoC: sun8i-codec: Protect the clock rate while streams are open To: Maxime Ripard References: <20201001021148.15852-1-samuel@sholland.org> <20201001021148.15852-21-samuel@sholland.org> <20201005120101.igzzwosnq6bzbua6@gilmour.lan> From: Samuel Holland Message-ID: <5ef4351f-e9ed-1a38-b79e-53e62a70437e@sholland.org> Date: Mon, 5 Oct 2020 23:43:51 -0500 User-Agent: Mozilla/5.0 (X11; Linux ppc64; rv:78.0) Gecko/20100101 Thunderbird/78.3.1 MIME-Version: 1.0 In-Reply-To: <20201005120101.igzzwosnq6bzbua6@gilmour.lan> Content-Type: text/plain; charset=windows-1252 Content-Language: en-US Content-Transfer-Encoding: 7bit Cc: Ondrej Jirman , alsa-devel@alsa-project.org, linux-kernel@vger.kernel.org, Takashi Iwai , Liam Girdwood , Chen-Yu Tsai , Mark Brown , linux-arm-kernel@lists.infradead.org X-BeenThere: alsa-devel@alsa-project.org X-Mailman-Version: 2.1.15 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" On 10/5/20 7:01 AM, Maxime Ripard wrote: > On Wed, Sep 30, 2020 at 09:11:43PM -0500, Samuel Holland wrote: >> The codec's clock input is shared among all AIFs, and shared with other >> audio-related hardware in the SoC, including I2S and SPDIF controllers. >> To ensure sample rates selected by userspace or by codec2codec DAI links >> are maintained, the clock rate must be protected while it is in use. >> >> Signed-off-by: Samuel Holland >> --- >> sound/soc/sunxi/sun8i-codec.c | 25 ++++++++++++++++++++++--- >> 1 file changed, 22 insertions(+), 3 deletions(-) >> >> diff --git a/sound/soc/sunxi/sun8i-codec.c b/sound/soc/sunxi/sun8i-codec.c >> index 501af64d43a0..86065bee7cd3 100644 >> --- a/sound/soc/sunxi/sun8i-codec.c >> +++ b/sound/soc/sunxi/sun8i-codec.c >> @@ -416,27 +416,32 @@ static int sun8i_codec_get_lrck_div_order(unsigned int slots, >> unsigned int div = slots * slot_width; >> >> if (div < 16 || div > 256) >> return -EINVAL; >> >> return order_base_2(div); >> } >> >> +static unsigned int sun8i_codec_get_sysclk_rate(unsigned int sample_rate) >> +{ >> + return sample_rate % 4000 ? 22579200 : 24576000; >> +} >> + >> static int sun8i_codec_hw_params(struct snd_pcm_substream *substream, >> struct snd_pcm_hw_params *params, >> struct snd_soc_dai *dai) >> { >> struct sun8i_codec *scodec = snd_soc_dai_get_drvdata(dai); >> struct sun8i_codec_aif *aif = &scodec->aifs[dai->id]; >> unsigned int sample_rate = params_rate(params); >> unsigned int slots = aif->slots ?: params_channels(params); >> unsigned int slot_width = aif->slot_width ?: params_width(params); >> - unsigned int sysclk_rate = clk_get_rate(scodec->clk_module); >> - int lrck_div_order, word_size; >> + unsigned int sysclk_rate = sun8i_codec_get_sysclk_rate(sample_rate); >> + int lrck_div_order, ret, word_size; >> u8 bclk_div; >> >> /* word size */ >> switch (params_width(params)) { >> case 8: >> word_size = 0x0; >> break; >> case 16: >> @@ -466,17 +471,30 @@ static int sun8i_codec_hw_params(struct snd_pcm_substream *substream, >> (lrck_div_order - 4) << SUN8I_AIF1CLK_CTRL_AIF1_LRCK_DIV); >> >> /* BCLK divider (SYSCLK/BCLK ratio) */ >> bclk_div = sun8i_codec_get_bclk_div(sysclk_rate, lrck_div_order, sample_rate); >> regmap_update_bits(scodec->regmap, SUN8I_AIF1CLK_CTRL, >> SUN8I_AIF1CLK_CTRL_AIF1_BCLK_DIV_MASK, >> bclk_div << SUN8I_AIF1CLK_CTRL_AIF1_BCLK_DIV); >> >> - if (!aif->open_streams) { >> + /* SYSCLK rate */ >> + if (aif->open_streams) { >> + ret = clk_set_rate(scodec->clk_module, sysclk_rate); >> + if (ret < 0) >> + return ret; >> + } else { >> + ret = clk_set_rate_exclusive(scodec->clk_module, sysclk_rate); > > It's not really clear to me why we wouldn't want to always protect the > clock rate here? >From Documentation/sound/kernel-api/writing-an-alsa-driver.rst: hw_params callback ... Note that this and ``prepare`` callbacks may be called multiple times per initialization. For example, the OSS emulation may call these callbacks at each change via its ioctl. Clock rate protection is reference counted, so we must only take one reference (or at least a known number of references) per stream. >> + if (ret == -EBUSY) >> + dev_err(dai->dev, "%s: clock is busy! Sample rate %u Hz " >> + "conflicts with other audio streams.\n", > > This string creates a checkpatch warning. I will put it on one line, though >100 columns is also a checkpatch warning. > Maxime Cheers, Samuel 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 X-Spam-Level: X-Spam-Status: No, score=-12.0 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH, MAILING_LIST_MULTI,NICE_REPLY_A,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id CB08CC41604 for ; Tue, 6 Oct 2020 04:45:39 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 4F9962075A for ; Tue, 6 Oct 2020 04:45:39 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="Y/+nUX+M"; dkim=temperror (0-bit key) header.d=sholland.org header.i=@sholland.org header.b="C04hKOuP"; dkim=temperror (0-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="ceINa3jo" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 4F9962075A Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=sholland.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=merlin.20170209; h=Sender:Content-Transfer-Encoding: Content-Type:Cc:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:Date:Message-ID:From: References:To:Subject:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=4GM40/XLEHTGbz6P2NVeHGyjIzV0Kcv/2v+pAjpAzBw=; b=Y/+nUX+M8GtK+9Qwf44oWY/V1 vNSpZX0Cr52nwjeWqQ0aRYlK/VRQ2ThAdY9P8iCsvZx/8oGN2izwwNA1wCBsZs2i5IWKiQ/LKT/IH OX2tRK+FwDfdH8cBqTLg3Q0qOXm3tfp9aUOljcwGBFeAW4Uu30+QHtK5mp4hUEyZh2/H33Et0am3Y a4GiRgyJHcVgko708dN+bQIw4hGMcwyhdLeQownqKLVTzQMYl8wIS3moQIG/4HIRHFbVQ5zJf3YS2 1Q+8DgarbN4T35HFdHt+xG6xSD6iDMPeeSIODtHDMqwn0jyB5StPud69yKL///tx6IVEU3rIedGuX bJ+uO++2A==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1kPepW-0007mH-7R; Tue, 06 Oct 2020 04:44:02 +0000 Received: from wout4-smtp.messagingengine.com ([64.147.123.20]) by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1kPepS-0007lx-Bx for linux-arm-kernel@lists.infradead.org; Tue, 06 Oct 2020 04:43:59 +0000 Received: from compute5.internal (compute5.nyi.internal [10.202.2.45]) by mailout.west.internal (Postfix) with ESMTP id B6BA53DF; Tue, 6 Oct 2020 00:43:54 -0400 (EDT) Received: from mailfrontend2 ([10.202.2.163]) by compute5.internal (MEProxy); Tue, 06 Oct 2020 00:43:55 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sholland.org; h= subject:to:cc:references:from:message-id:date:mime-version :in-reply-to:content-type:content-transfer-encoding; s=fm3; bh=X 2uGAJogJbE61MC4c1/0d4db+rheQyrn5sPt3B2jfek=; b=C04hKOuPIfiwZn1kb pvrDuEopDqCt7XKTr2jV16rzmpjCMkhbcvTOWnFw8aGUnMJfO9U47YETUyXoP/VZ fu39eqeyt0PzOuvqeQjPO+dDux86UCRvcirx26Jd8MP8QECyeJhQcU41nKvN5i6r 1/zfjtDn9rfePb4aCbaw2E4ipIdS+0C2Plit5jR2MJzOMFPTLGUxmdnxztbNFYX4 d71WyFvKSdcLjQevZaleAK4BLv4MTxzE/5SO6Gut4QgFaJHeujz9s8QHvgah6rN4 Pa81I1CFexmrNwi2o+B6tzN+ruhQj0/YUBmgRD1mpussEIU+GOo7vd7t2ahPSJ03 QF2Sw== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-transfer-encoding:content-type :date:from:in-reply-to:message-id:mime-version:references :subject:to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender :x-sasl-enc; s=fm1; bh=X2uGAJogJbE61MC4c1/0d4db+rheQyrn5sPt3B2jf ek=; b=ceINa3joRWtekgoblZo5972H2/CWGk0s4NapKYMpurufILS7qFFxq/pHH OwpcBvGmYSr1RmDdaJpA6rc1DyakAp2X3a+c+Rd5Pvh8CxZVsdsGkaugMMmouI5d ZrAs1Q1qHiuSbdAVAdUkgcF0ZPkySNIyuNJswjUCPyRYkkVgWQTGge85cQogUX8V x3jV72qBq0bTAP79le6LFzc7XWUhpX8YU3EWuixfB4hH1QE1q45SDFutjIMEmGnB uBpvccvUO0INGUGmV1VmbpzArspxsrA+07z6AUpm4ZUsQSFZIz5pZQ+KavOVvAlN PULg4myGj65fW+F/FsSVUrB1emCvQ== X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedujedrgeefgdekjecutefuodetggdotefrodftvf curfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfghnecu uegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmdenuc fjughrpefuvfhfhffkffgfgggjtgfgsehtjeertddtfeehnecuhfhrohhmpefurghmuhgv lhcujfholhhlrghnugcuoehsrghmuhgvlhesshhhohhllhgrnhgurdhorhhgqeenucggtf frrghtthgvrhhnpeettdekiedvgeetfeelueduhefftdfhkeehueejvefhtddvffejleeu hfeuhfevkeenucfkphepjedtrddufeehrddugeekrdduhedunecuvehluhhsthgvrhfuih iivgeptdenucfrrghrrghmpehmrghilhhfrhhomhepshgrmhhuvghlsehshhholhhlrghn ugdrohhrgh X-ME-Proxy: Received: from [70.135.148.151] (70-135-148-151.lightspeed.stlsmo.sbcglobal.net [70.135.148.151]) by mail.messagingengine.com (Postfix) with ESMTPA id E440B306467D; Tue, 6 Oct 2020 00:43:51 -0400 (EDT) Subject: Re: [PATCH 20/25] ASoC: sun8i-codec: Protect the clock rate while streams are open To: Maxime Ripard References: <20201001021148.15852-1-samuel@sholland.org> <20201001021148.15852-21-samuel@sholland.org> <20201005120101.igzzwosnq6bzbua6@gilmour.lan> From: Samuel Holland Message-ID: <5ef4351f-e9ed-1a38-b79e-53e62a70437e@sholland.org> Date: Mon, 5 Oct 2020 23:43:51 -0500 User-Agent: Mozilla/5.0 (X11; Linux ppc64; rv:78.0) Gecko/20100101 Thunderbird/78.3.1 MIME-Version: 1.0 In-Reply-To: <20201005120101.igzzwosnq6bzbua6@gilmour.lan> Content-Language: en-US X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20201006_004358_532161_37D82B80 X-CRM114-Status: GOOD ( 20.76 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Ondrej Jirman , alsa-devel@alsa-project.org, linux-kernel@vger.kernel.org, Takashi Iwai , Liam Girdwood , Chen-Yu Tsai , Mark Brown , Jaroslav Kysela , linux-arm-kernel@lists.infradead.org Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On 10/5/20 7:01 AM, Maxime Ripard wrote: > On Wed, Sep 30, 2020 at 09:11:43PM -0500, Samuel Holland wrote: >> The codec's clock input is shared among all AIFs, and shared with other >> audio-related hardware in the SoC, including I2S and SPDIF controllers. >> To ensure sample rates selected by userspace or by codec2codec DAI links >> are maintained, the clock rate must be protected while it is in use. >> >> Signed-off-by: Samuel Holland >> --- >> sound/soc/sunxi/sun8i-codec.c | 25 ++++++++++++++++++++++--- >> 1 file changed, 22 insertions(+), 3 deletions(-) >> >> diff --git a/sound/soc/sunxi/sun8i-codec.c b/sound/soc/sunxi/sun8i-codec.c >> index 501af64d43a0..86065bee7cd3 100644 >> --- a/sound/soc/sunxi/sun8i-codec.c >> +++ b/sound/soc/sunxi/sun8i-codec.c >> @@ -416,27 +416,32 @@ static int sun8i_codec_get_lrck_div_order(unsigned int slots, >> unsigned int div = slots * slot_width; >> >> if (div < 16 || div > 256) >> return -EINVAL; >> >> return order_base_2(div); >> } >> >> +static unsigned int sun8i_codec_get_sysclk_rate(unsigned int sample_rate) >> +{ >> + return sample_rate % 4000 ? 22579200 : 24576000; >> +} >> + >> static int sun8i_codec_hw_params(struct snd_pcm_substream *substream, >> struct snd_pcm_hw_params *params, >> struct snd_soc_dai *dai) >> { >> struct sun8i_codec *scodec = snd_soc_dai_get_drvdata(dai); >> struct sun8i_codec_aif *aif = &scodec->aifs[dai->id]; >> unsigned int sample_rate = params_rate(params); >> unsigned int slots = aif->slots ?: params_channels(params); >> unsigned int slot_width = aif->slot_width ?: params_width(params); >> - unsigned int sysclk_rate = clk_get_rate(scodec->clk_module); >> - int lrck_div_order, word_size; >> + unsigned int sysclk_rate = sun8i_codec_get_sysclk_rate(sample_rate); >> + int lrck_div_order, ret, word_size; >> u8 bclk_div; >> >> /* word size */ >> switch (params_width(params)) { >> case 8: >> word_size = 0x0; >> break; >> case 16: >> @@ -466,17 +471,30 @@ static int sun8i_codec_hw_params(struct snd_pcm_substream *substream, >> (lrck_div_order - 4) << SUN8I_AIF1CLK_CTRL_AIF1_LRCK_DIV); >> >> /* BCLK divider (SYSCLK/BCLK ratio) */ >> bclk_div = sun8i_codec_get_bclk_div(sysclk_rate, lrck_div_order, sample_rate); >> regmap_update_bits(scodec->regmap, SUN8I_AIF1CLK_CTRL, >> SUN8I_AIF1CLK_CTRL_AIF1_BCLK_DIV_MASK, >> bclk_div << SUN8I_AIF1CLK_CTRL_AIF1_BCLK_DIV); >> >> - if (!aif->open_streams) { >> + /* SYSCLK rate */ >> + if (aif->open_streams) { >> + ret = clk_set_rate(scodec->clk_module, sysclk_rate); >> + if (ret < 0) >> + return ret; >> + } else { >> + ret = clk_set_rate_exclusive(scodec->clk_module, sysclk_rate); > > It's not really clear to me why we wouldn't want to always protect the > clock rate here? >From Documentation/sound/kernel-api/writing-an-alsa-driver.rst: hw_params callback ... Note that this and ``prepare`` callbacks may be called multiple times per initialization. For example, the OSS emulation may call these callbacks at each change via its ioctl. Clock rate protection is reference counted, so we must only take one reference (or at least a known number of references) per stream. >> + if (ret == -EBUSY) >> + dev_err(dai->dev, "%s: clock is busy! Sample rate %u Hz " >> + "conflicts with other audio streams.\n", > > This string creates a checkpatch warning. I will put it on one line, though >100 columns is also a checkpatch warning. > Maxime Cheers, Samuel _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel