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=-5.3 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,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 D8018C3A5A2 for ; Fri, 20 Sep 2019 23:26:27 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id A5DE02070C for ; Fri, 20 Sep 2019 23:26:27 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="sv0ycOVZ" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2394077AbfITX00 (ORCPT ); Fri, 20 Sep 2019 19:26:26 -0400 Received: from mail-pl1-f193.google.com ([209.85.214.193]:42825 "EHLO mail-pl1-f193.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2394035AbfITX00 (ORCPT ); Fri, 20 Sep 2019 19:26:26 -0400 Received: by mail-pl1-f193.google.com with SMTP id e5so3890035pls.9; Fri, 20 Sep 2019 16:26:25 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=CA6mH2s7/mQqbWPAIxBYkadhzeqMoOn4kmubZpmIYQA=; b=sv0ycOVZPEhTBxFw+PhY1TKXnOl5DVN2Av+2c6JW39gGUGeaaPf9JWdZpzhfhjyDvO tHo5pFIxpbHshEplJahmcB28BZVO66DXVbj2Etn1qVpN/UyRaEys8guhMCBUGWa3Pdps RwEUfv849KjqGBzz5YzRqXMYHsE2hTKKND/lC1Ak9mBG11lytDDhN+YuEfWxOa7G7DG2 70IUKQm0VRf3zc5GtNZG2Sz8i3JA1ZgYokOXQL3Rp2Nl2Ri/jFFw2/ckRuCEF/dQ2nF8 FRjZ4M/463RnpHE6i+2XVRXIkNXaaQzojkcpObFvr65CCoiwweYS2QoO8s5RYoJnpHod fXoA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=CA6mH2s7/mQqbWPAIxBYkadhzeqMoOn4kmubZpmIYQA=; b=X4xmH9wG7OZheSo0Xoe0A7Yjr6BTNedD3wA8iPHOCPtIF5g3Jrfb8PpremP2DlcxU3 Fa84WGYUcWaCpwb2XilhJjP7Q3kuh3VPUThV/KyYYtb3YwohsgeJvKhWDLapAZXSZXUf /9ja0umVQ751udj774jirDjzsvHTEnWqdoEH5368T26VBqs/QN5LTdVU7vzfOqJeaCSv Kd+CUQBqlWHzFAdppWpOniKtf8fcLUdQSt/REYGEtKYttECPbtARZdjOuYuFnYEaZ2Wk DEJrMSvCYa7aZRiQ2NSIJnmBctxkG8mOlFNOXtzCD0FHykrrdvR+sdA+j00/lQAOk71i 0mXQ== X-Gm-Message-State: APjAAAV68LfyBGDJz1QN/LX/9GCRE4Oncufwxy5b5lg8F4XgGcKPspGK UWCSOzV68I8zA34rC2OimTc= X-Google-Smtp-Source: APXvYqwAW5gxV9v80zvEtG/jnR4oL2FMp5gqh40w1OPjkDMijYj5VxPtPws76h5RT0qlX41fhMzS0g== X-Received: by 2002:a17:902:8f8c:: with SMTP id z12mr17751680plo.2.1569021985410; Fri, 20 Sep 2019 16:26:25 -0700 (PDT) Received: from Asurada-Nvidia.nvidia.com (thunderhill.nvidia.com. [216.228.112.22]) by smtp.gmail.com with ESMTPSA id k5sm3716946pfp.109.2019.09.20.16.26.24 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Fri, 20 Sep 2019 16:26:25 -0700 (PDT) Date: Fri, 20 Sep 2019 16:25:33 -0700 From: Nicolin Chen To: Shengjiu Wang Cc: timur@kernel.org, Xiubo.Lee@gmail.com, festevam@gmail.com, lgirdwood@gmail.com, broonie@kernel.org, perex@perex.cz, tiwai@suse.com, alsa-devel@alsa-project.org, linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org, robh+dt@kernel.org, mark.rutland@arm.com, devicetree@vger.kernel.org, lars@metafoo.de Subject: Re: [PATCH V3 4/4] ASoC: fsl_asrc: Fix error with S24_3LE format bitstream in i.MX8 Message-ID: <20190920232533.GA29851@Asurada-Nvidia.nvidia.com> References: <0fe619f4c8f0898cf51c7324c9a0784c5782ed91.1568861098.git.shengjiu.wang@nxp.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <0fe619f4c8f0898cf51c7324c9a0784c5782ed91.1568861098.git.shengjiu.wang@nxp.com> User-Agent: Mutt/1.9.4 (2018-02-28) Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hello Shengjiu, One issue for error-out and some nit-pickings inline. Thanks. On Thu, Sep 19, 2019 at 08:11:42PM +0800, Shengjiu Wang wrote: > There is error "aplay: pcm_write:2023: write error: Input/output error" > on i.MX8QM/i.MX8QXP platform for S24_3LE format. > > In i.MX8QM/i.MX8QXP, the DMA is EDMA, which don't support 24bit > sample, but we didn't add any constraint, that cause issues. > > So we need to query the caps of dma, then update the hw parameters > according to the caps. > > Signed-off-by: Shengjiu Wang > --- > sound/soc/fsl/fsl_asrc.c | 4 +-- > sound/soc/fsl/fsl_asrc.h | 3 +++ > sound/soc/fsl/fsl_asrc_dma.c | 52 +++++++++++++++++++++++++++++++----- > 3 files changed, 50 insertions(+), 9 deletions(-) > > @@ -276,6 +274,11 @@ static int fsl_asrc_dma_startup(struct snd_pcm_substream *substream) > struct device *dev = component->dev; > struct fsl_asrc *asrc_priv = dev_get_drvdata(dev); > struct fsl_asrc_pair *pair; > + bool tx = substream->stream == SNDRV_PCM_STREAM_PLAYBACK; > + u8 dir = tx ? OUT : IN; > + struct dma_chan *tmp_chan; > + struct snd_dmaengine_dai_dma_data *dma_data; Nit: would it be possible to reorganize these a bit? Usually we put struct things together unless there is a dependency, similar to fsl_asrc_dma_hw_params(). > @@ -285,9 +288,44 @@ static int fsl_asrc_dma_startup(struct snd_pcm_substream *substream) > > runtime->private_data = pair; > > + /* Request a temp pair, which is release in the end */ Nit: "which will be released later" or "and will release it later"? And could we use a work like "dummy"? Or at least I would love to see the comments explaining the parameter "1" in the function call below. > + ret = fsl_asrc_request_pair(1, pair); > + if (ret < 0) { > + dev_err(dev, "failed to request asrc pair\n"); > + return ret; > + } > + > + tmp_chan = fsl_asrc_get_dma_channel(pair, dir); > + if (!tmp_chan) { > + dev_err(dev, "can't get dma channel\n"); Could we align with other error messages using "failed to"? > + ret = snd_soc_set_runtime_hwparams(substream, &snd_imx_hardware); > + if (ret) > + return ret; > + [...] > + dma_release_channel(tmp_chan); > + fsl_asrc_release_pair(pair); I think we need an "out:" here for those error-out routines to goto. Otherwise, it'd be a pair leak? > + Could we drop this? There is a blank line below already :) > > return 0; > } > -- > 2.21.0 > 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=-5.2 required=3.0 tests=DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED,DKIM_VALID,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,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 9DC57C3A5A2 for ; Fri, 20 Sep 2019 23:27:25 +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 976B92070C for ; Fri, 20 Sep 2019 23:27:24 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=alsa-project.org header.i=@alsa-project.org header.b="PiCnN1wS"; dkim=fail reason="signature verification failed" (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="sv0ycOVZ" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 976B92070C Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=gmail.com 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 3216D1666; Sat, 21 Sep 2019 01:26:32 +0200 (CEST) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa0.perex.cz 3216D1666 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=alsa-project.org; s=default; t=1569022042; bh=IqMMpsnjsMWqEwMdkmeq2G8vB+al8PlJg2qKmTCkXT8=; h=Date:From:To:References:In-Reply-To:Cc:Subject:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From; b=PiCnN1wSP5UagRJoMQ1+mQdL0o/w7+mICNBOuKKKJswuAuo1hZfBJcE7nYswZVEk8 /77zCPx/pzpPnm1FiX4HZFUOGz/CcctOs8I5TczgYqsyhTolUolDy9ejAptnTCSm1O oNl3PfvoyIYQjnXMjndGlNFHDSm+I8Ol8uYCqoLg= Received: from alsa1.perex.cz (localhost.localdomain [127.0.0.1]) by alsa1.perex.cz (Postfix) with ESMTP id A6059F804FF; Sat, 21 Sep 2019 01:26:31 +0200 (CEST) Received: by alsa1.perex.cz (Postfix, from userid 50401) id 53532F80506; Sat, 21 Sep 2019 01:26:30 +0200 (CEST) Received: from mail-pl1-x641.google.com (mail-pl1-x641.google.com [IPv6:2607:f8b0:4864:20::641]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by alsa1.perex.cz (Postfix) with ESMTPS id B117FF800C7 for ; Sat, 21 Sep 2019 01:26:27 +0200 (CEST) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa1.perex.cz B117FF800C7 Authentication-Results: alsa1.perex.cz; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="sv0ycOVZ" Received: by mail-pl1-x641.google.com with SMTP id w10so3895987plq.5 for ; Fri, 20 Sep 2019 16:26:27 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=CA6mH2s7/mQqbWPAIxBYkadhzeqMoOn4kmubZpmIYQA=; b=sv0ycOVZPEhTBxFw+PhY1TKXnOl5DVN2Av+2c6JW39gGUGeaaPf9JWdZpzhfhjyDvO tHo5pFIxpbHshEplJahmcB28BZVO66DXVbj2Etn1qVpN/UyRaEys8guhMCBUGWa3Pdps RwEUfv849KjqGBzz5YzRqXMYHsE2hTKKND/lC1Ak9mBG11lytDDhN+YuEfWxOa7G7DG2 70IUKQm0VRf3zc5GtNZG2Sz8i3JA1ZgYokOXQL3Rp2Nl2Ri/jFFw2/ckRuCEF/dQ2nF8 FRjZ4M/463RnpHE6i+2XVRXIkNXaaQzojkcpObFvr65CCoiwweYS2QoO8s5RYoJnpHod fXoA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=CA6mH2s7/mQqbWPAIxBYkadhzeqMoOn4kmubZpmIYQA=; b=riTgztQJBvGUCbO0Hg7MNeomVWBPqfZKLXFyzpiWnJ6jBPl3nRfb3F5OKhLB/q5J38 nUtqF1VmEMvlCq8ZoIdc2injoHdQZ0v56GK7Fhc4LD9gyx/sEwVxKEvX2Y3S7M4zm9wy 2zXTZAzERyV0I7m3Zm8fewsPEsrDvpURLOE61WBH9BkAgsppy9rYxWweYNBcsCpc9gQN tDJe6vN0gfNb/ZEaDFHHKR73i+eki7CNQrE/SP1XpfPlsanZxKeKaVyzfzpKq6i4xx9C KHH0z/3EVXO385PIPYZXvVWBEmuGOFfJ7vKJLYWlJC67njIXMIzwYT5ukqmXPB/MJQEx ZVJw== X-Gm-Message-State: APjAAAVD/Nd4TyQtejpIJelr7Qy1SX7ttCQLqsAdQ7jozCL14ngyaQ09 CP6frzv1y3Dku7IUuo7LikY= X-Google-Smtp-Source: APXvYqwAW5gxV9v80zvEtG/jnR4oL2FMp5gqh40w1OPjkDMijYj5VxPtPws76h5RT0qlX41fhMzS0g== X-Received: by 2002:a17:902:8f8c:: with SMTP id z12mr17751680plo.2.1569021985410; Fri, 20 Sep 2019 16:26:25 -0700 (PDT) Received: from Asurada-Nvidia.nvidia.com (thunderhill.nvidia.com. [216.228.112.22]) by smtp.gmail.com with ESMTPSA id k5sm3716946pfp.109.2019.09.20.16.26.24 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Fri, 20 Sep 2019 16:26:25 -0700 (PDT) Date: Fri, 20 Sep 2019 16:25:33 -0700 From: Nicolin Chen To: Shengjiu Wang Message-ID: <20190920232533.GA29851@Asurada-Nvidia.nvidia.com> References: <0fe619f4c8f0898cf51c7324c9a0784c5782ed91.1568861098.git.shengjiu.wang@nxp.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <0fe619f4c8f0898cf51c7324c9a0784c5782ed91.1568861098.git.shengjiu.wang@nxp.com> User-Agent: Mutt/1.9.4 (2018-02-28) Cc: mark.rutland@arm.com, devicetree@vger.kernel.org, alsa-devel@alsa-project.org, lars@metafoo.de, timur@kernel.org, Xiubo.Lee@gmail.com, linuxppc-dev@lists.ozlabs.org, tiwai@suse.com, lgirdwood@gmail.com, robh+dt@kernel.org, broonie@kernel.org, festevam@gmail.com, linux-kernel@vger.kernel.org Subject: Re: [alsa-devel] [PATCH V3 4/4] ASoC: fsl_asrc: Fix error with S24_3LE format bitstream in i.MX8 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: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: alsa-devel-bounces@alsa-project.org Sender: "Alsa-devel" Hello Shengjiu, One issue for error-out and some nit-pickings inline. Thanks. On Thu, Sep 19, 2019 at 08:11:42PM +0800, Shengjiu Wang wrote: > There is error "aplay: pcm_write:2023: write error: Input/output error" > on i.MX8QM/i.MX8QXP platform for S24_3LE format. > > In i.MX8QM/i.MX8QXP, the DMA is EDMA, which don't support 24bit > sample, but we didn't add any constraint, that cause issues. > > So we need to query the caps of dma, then update the hw parameters > according to the caps. > > Signed-off-by: Shengjiu Wang > --- > sound/soc/fsl/fsl_asrc.c | 4 +-- > sound/soc/fsl/fsl_asrc.h | 3 +++ > sound/soc/fsl/fsl_asrc_dma.c | 52 +++++++++++++++++++++++++++++++----- > 3 files changed, 50 insertions(+), 9 deletions(-) > > @@ -276,6 +274,11 @@ static int fsl_asrc_dma_startup(struct snd_pcm_substream *substream) > struct device *dev = component->dev; > struct fsl_asrc *asrc_priv = dev_get_drvdata(dev); > struct fsl_asrc_pair *pair; > + bool tx = substream->stream == SNDRV_PCM_STREAM_PLAYBACK; > + u8 dir = tx ? OUT : IN; > + struct dma_chan *tmp_chan; > + struct snd_dmaengine_dai_dma_data *dma_data; Nit: would it be possible to reorganize these a bit? Usually we put struct things together unless there is a dependency, similar to fsl_asrc_dma_hw_params(). > @@ -285,9 +288,44 @@ static int fsl_asrc_dma_startup(struct snd_pcm_substream *substream) > > runtime->private_data = pair; > > + /* Request a temp pair, which is release in the end */ Nit: "which will be released later" or "and will release it later"? And could we use a work like "dummy"? Or at least I would love to see the comments explaining the parameter "1" in the function call below. > + ret = fsl_asrc_request_pair(1, pair); > + if (ret < 0) { > + dev_err(dev, "failed to request asrc pair\n"); > + return ret; > + } > + > + tmp_chan = fsl_asrc_get_dma_channel(pair, dir); > + if (!tmp_chan) { > + dev_err(dev, "can't get dma channel\n"); Could we align with other error messages using "failed to"? > + ret = snd_soc_set_runtime_hwparams(substream, &snd_imx_hardware); > + if (ret) > + return ret; > + [...] > + dma_release_channel(tmp_chan); > + fsl_asrc_release_pair(pair); I think we need an "out:" here for those error-out routines to goto. Otherwise, it'd be a pair leak? > + Could we drop this? There is a blank line below already :) > > return 0; > } > -- > 2.21.0 > _______________________________________________ Alsa-devel mailing list Alsa-devel@alsa-project.org https://mailman.alsa-project.org/mailman/listinfo/alsa-devel 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=-5.0 required=3.0 tests=DKIM_ADSP_CUSTOM_MED, DKIM_INVALID,DKIM_SIGNED,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE, SPF_PASS,USER_AGENT_SANE_1 autolearn=no 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 4A5DEC3A5A2 for ; Fri, 20 Sep 2019 23:28:11 +0000 (UTC) Received: from lists.ozlabs.org (lists.ozlabs.org [203.11.71.2]) (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 462CE2070C for ; Fri, 20 Sep 2019 23:28:10 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="sv0ycOVZ" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 462CE2070C Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=gmail.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=linuxppc-dev-bounces+linuxppc-dev=archiver.kernel.org@lists.ozlabs.org Received: from bilbo.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3]) by lists.ozlabs.org (Postfix) with ESMTP id 46Zqdg3yZrzF4Mp for ; Sat, 21 Sep 2019 09:28:07 +1000 (AEST) Authentication-Results: lists.ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=gmail.com (client-ip=2607:f8b0:4864:20::641; helo=mail-pl1-x641.google.com; envelope-from=nicoleotsuka@gmail.com; receiver=) Authentication-Results: lists.ozlabs.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: lists.ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="sv0ycOVZ"; dkim-atps=neutral Received: from mail-pl1-x641.google.com (mail-pl1-x641.google.com [IPv6:2607:f8b0:4864:20::641]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 46Zqbp1M4DzF3Nl for ; Sat, 21 Sep 2019 09:26:27 +1000 (AEST) Received: by mail-pl1-x641.google.com with SMTP id q24so3870482plr.13 for ; Fri, 20 Sep 2019 16:26:27 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=CA6mH2s7/mQqbWPAIxBYkadhzeqMoOn4kmubZpmIYQA=; b=sv0ycOVZPEhTBxFw+PhY1TKXnOl5DVN2Av+2c6JW39gGUGeaaPf9JWdZpzhfhjyDvO tHo5pFIxpbHshEplJahmcB28BZVO66DXVbj2Etn1qVpN/UyRaEys8guhMCBUGWa3Pdps RwEUfv849KjqGBzz5YzRqXMYHsE2hTKKND/lC1Ak9mBG11lytDDhN+YuEfWxOa7G7DG2 70IUKQm0VRf3zc5GtNZG2Sz8i3JA1ZgYokOXQL3Rp2Nl2Ri/jFFw2/ckRuCEF/dQ2nF8 FRjZ4M/463RnpHE6i+2XVRXIkNXaaQzojkcpObFvr65CCoiwweYS2QoO8s5RYoJnpHod fXoA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=CA6mH2s7/mQqbWPAIxBYkadhzeqMoOn4kmubZpmIYQA=; b=EpEx9VzpdpTNCAsTeP005l3vSL2Vf//bhlTj1Eci6otGr9YZ2PC4YK3/9225EQFG8f CfzSpc4kfaG0iD34SCkLKqurG6CnCIJuMyO9bL6FqElOR29djpiVCrskz1Iqy0uzeWbI 7+ruskph/gM60Xa5ifrCszJuqXGXHmiFq2HDATsb+kNC5NCe3l+pcFqjS9tdBKi0IIj5 8sZqc0b8pcJr8v8Cj23wXD3ibz4+g50/3SepYcK15erD+yo2AmK00yAvH5mKvJ/HZ+Pm da2/z6IBPFejuZsemzK5dUOGPtp1M45QY+MASmHM+kU7gWAyMn+u69N1oMY1i/M58z1z 73JA== X-Gm-Message-State: APjAAAVe3qIajbuzihmd/tJme8357XgG63TfYgp/CB5wbd7nvhvDaOED mvjLc565MT5y5R4ad0EYH/4= X-Google-Smtp-Source: APXvYqwAW5gxV9v80zvEtG/jnR4oL2FMp5gqh40w1OPjkDMijYj5VxPtPws76h5RT0qlX41fhMzS0g== X-Received: by 2002:a17:902:8f8c:: with SMTP id z12mr17751680plo.2.1569021985410; Fri, 20 Sep 2019 16:26:25 -0700 (PDT) Received: from Asurada-Nvidia.nvidia.com (thunderhill.nvidia.com. [216.228.112.22]) by smtp.gmail.com with ESMTPSA id k5sm3716946pfp.109.2019.09.20.16.26.24 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Fri, 20 Sep 2019 16:26:25 -0700 (PDT) Date: Fri, 20 Sep 2019 16:25:33 -0700 From: Nicolin Chen To: Shengjiu Wang Subject: Re: [PATCH V3 4/4] ASoC: fsl_asrc: Fix error with S24_3LE format bitstream in i.MX8 Message-ID: <20190920232533.GA29851@Asurada-Nvidia.nvidia.com> References: <0fe619f4c8f0898cf51c7324c9a0784c5782ed91.1568861098.git.shengjiu.wang@nxp.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <0fe619f4c8f0898cf51c7324c9a0784c5782ed91.1568861098.git.shengjiu.wang@nxp.com> User-Agent: Mutt/1.9.4 (2018-02-28) X-BeenThere: linuxppc-dev@lists.ozlabs.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: mark.rutland@arm.com, devicetree@vger.kernel.org, alsa-devel@alsa-project.org, lars@metafoo.de, timur@kernel.org, Xiubo.Lee@gmail.com, linuxppc-dev@lists.ozlabs.org, tiwai@suse.com, lgirdwood@gmail.com, robh+dt@kernel.org, perex@perex.cz, broonie@kernel.org, festevam@gmail.com, linux-kernel@vger.kernel.org Errors-To: linuxppc-dev-bounces+linuxppc-dev=archiver.kernel.org@lists.ozlabs.org Sender: "Linuxppc-dev" Hello Shengjiu, One issue for error-out and some nit-pickings inline. Thanks. On Thu, Sep 19, 2019 at 08:11:42PM +0800, Shengjiu Wang wrote: > There is error "aplay: pcm_write:2023: write error: Input/output error" > on i.MX8QM/i.MX8QXP platform for S24_3LE format. > > In i.MX8QM/i.MX8QXP, the DMA is EDMA, which don't support 24bit > sample, but we didn't add any constraint, that cause issues. > > So we need to query the caps of dma, then update the hw parameters > according to the caps. > > Signed-off-by: Shengjiu Wang > --- > sound/soc/fsl/fsl_asrc.c | 4 +-- > sound/soc/fsl/fsl_asrc.h | 3 +++ > sound/soc/fsl/fsl_asrc_dma.c | 52 +++++++++++++++++++++++++++++++----- > 3 files changed, 50 insertions(+), 9 deletions(-) > > @@ -276,6 +274,11 @@ static int fsl_asrc_dma_startup(struct snd_pcm_substream *substream) > struct device *dev = component->dev; > struct fsl_asrc *asrc_priv = dev_get_drvdata(dev); > struct fsl_asrc_pair *pair; > + bool tx = substream->stream == SNDRV_PCM_STREAM_PLAYBACK; > + u8 dir = tx ? OUT : IN; > + struct dma_chan *tmp_chan; > + struct snd_dmaengine_dai_dma_data *dma_data; Nit: would it be possible to reorganize these a bit? Usually we put struct things together unless there is a dependency, similar to fsl_asrc_dma_hw_params(). > @@ -285,9 +288,44 @@ static int fsl_asrc_dma_startup(struct snd_pcm_substream *substream) > > runtime->private_data = pair; > > + /* Request a temp pair, which is release in the end */ Nit: "which will be released later" or "and will release it later"? And could we use a work like "dummy"? Or at least I would love to see the comments explaining the parameter "1" in the function call below. > + ret = fsl_asrc_request_pair(1, pair); > + if (ret < 0) { > + dev_err(dev, "failed to request asrc pair\n"); > + return ret; > + } > + > + tmp_chan = fsl_asrc_get_dma_channel(pair, dir); > + if (!tmp_chan) { > + dev_err(dev, "can't get dma channel\n"); Could we align with other error messages using "failed to"? > + ret = snd_soc_set_runtime_hwparams(substream, &snd_imx_hardware); > + if (ret) > + return ret; > + [...] > + dma_release_channel(tmp_chan); > + fsl_asrc_release_pair(pair); I think we need an "out:" here for those error-out routines to goto. Otherwise, it'd be a pair leak? > + Could we drop this? There is a blank line below already :) > > return 0; > } > -- > 2.21.0 >