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=-7.9 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI,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 E9BA3C433E0 for ; Fri, 5 Feb 2021 23:01:59 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id ACC0D64E43 for ; Fri, 5 Feb 2021 23:01:59 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232242AbhBEXBQ (ORCPT ); Fri, 5 Feb 2021 18:01:16 -0500 Received: from mail.kernel.org ([198.145.29.99]:42858 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232775AbhBEOiV (ORCPT ); Fri, 5 Feb 2021 09:38:21 -0500 Received: by mail.kernel.org (Postfix) with ESMTPSA id 220B864FBC; Fri, 5 Feb 2021 14:59:05 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1612537146; bh=pcoVC9g6Ql5brnm8ZYrxM8gXd4vKE+CdwztAMluc3jc=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=dAu9XINGXWMXKM/v4VSMEpR5UUCt36txoacXH7ka0tNv+vG8Uhvnlhf5OOFcYyRyK tKUvQdjoSvGBnoTZ1QDZRsluTJ4fRW+i0tlc+VKsUF9R5bkMFO921B8eKwzT9xS89+ BxgNKjCkkKuVdr2Xb16fv/oWk2wF/IpowhfTwmRmdl/hk0xAPKiy6q/JPbOV9rihec DwKrhy22qVj5ArEmPBouXYoyduoc9mEyFyB8W2cvPw7M1ank7v970HjrN9h2zkMlCC yhP5EQx8DqzkpHfzgFM25WJfLei1aGv86v8U7bPoZpWQE7W4pd9B5eIU9O0dmMTnAy 8ir6e/XFyhIKg== Date: Fri, 5 Feb 2021 14:58:16 +0000 From: Mark Brown To: Shengjiu Wang Cc: lgirdwood@gmail.com, perex@perex.cz, tiwai@suse.com, alsa-devel@alsa-project.org, linux-kernel@vger.kernel.org, timur@kernel.org, nicoleotsuka@gmail.com, Xiubo.Lee@gmail.com, festevam@gmail.com, linuxppc-dev@lists.ozlabs.org, robh+dt@kernel.org, devicetree@vger.kernel.org Subject: Re: [PATCH 5/7] ASoC: imx-pcm-rpmsg: Add platform driver for audio base on rpmsg Message-ID: <20210205145816.GD4720@sirena.org.uk> References: <1612508250-10586-1-git-send-email-shengjiu.wang@nxp.com> <1612508250-10586-6-git-send-email-shengjiu.wang@nxp.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="k4f25fnPtRuIRUb3" Content-Disposition: inline In-Reply-To: <1612508250-10586-6-git-send-email-shengjiu.wang@nxp.com> X-Cookie: Huh? User-Agent: Mutt/1.10.1 (2018-07-13) Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --k4f25fnPtRuIRUb3 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Fri, Feb 05, 2021 at 02:57:28PM +0800, Shengjiu Wang wrote: > + if (params_format(params) == SNDRV_PCM_FORMAT_S16_LE) > + msg->s_msg.param.format = RPMSG_S16_LE; > + else if (params_format(params) == SNDRV_PCM_FORMAT_S24_LE) Again this should be a switch statement. > + if (params_channels(params) == 1) > + msg->s_msg.param.channels = RPMSG_CH_LEFT; > + else > + msg->s_msg.param.channels = RPMSG_CH_STEREO; Shouldn't this be reporting an error if the number of channels is more than 2? > + /* > + * if the data in the buffer is less than one period > + * send message immediately. > + * if there is more than one period data, delay one > + * period (timer) to send the message. > + */ > + if ((avail - writen_num * period_size) <= period_size) { > + imx_rpmsg_insert_workqueue(substream, msg, info); > + } else if (rpmsg->force_lpa && !timer_pending(timer)) { > + int time_msec; > + > + time_msec = (int)(runtime->period_size * 1000 / runtime->rate); > + mod_timer(timer, jiffies + msecs_to_jiffies(time_msec)); > + } The comment here is at least confusing - why would we not send a full buffer immediately if we have one? This sounds like it's the opposite way round to what we'd do if we were trying to cut down the number of messages. It might help to say which buffer and where? > + /** > + * Every work in the work queue, first we check if there /** comments are only for kerneldoc. --k4f25fnPtRuIRUb3 Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQEzBAABCgAdFiEEreZoqmdXGLWf4p/qJNaLcl1Uh9AFAmAdXQcACgkQJNaLcl1U h9ACUwf9EcHPKiRzzRa6Atb6PHhaM1oBK/2zYZcdmLDejwBct/KltZywmVsBQv0o JmeLnKw7/jLk3Sph4Pqk6J2lyizC5nik/w7NjFO5CIUyNTQnFRZtaDILcnVr7vNk 28HX0/XoPM54EbfyncrP41lr/L4EYgHmjIMqi/TjVtFnfyOt1Pq99Rj02lKDVnV3 ERmOguBociG3yf9kV/wcrZzJ4hOg7Lw468CHtxoeCpPKsJovmByQ0I78JQJlJ1Jj TRjC06zUmRhscWFCrWiOkItqPpTcrv5TxMVh5Ko5zE1rYslk8XURTFpnKDxWoxkR MtylP+v1qS4G4STsZObKZtcso3D9hA== =hzt6 -----END PGP SIGNATURE----- --k4f25fnPtRuIRUb3-- 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.5 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,MAILING_LIST_MULTI,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 EA32AC433DB for ; Fri, 5 Feb 2021 15:00:06 +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 E29B9650DE for ; Fri, 5 Feb 2021 15:00:05 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org E29B9650DE Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.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 0FF451660; Fri, 5 Feb 2021 15:59:14 +0100 (CET) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa0.perex.cz 0FF451660 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=alsa-project.org; s=default; t=1612537204; bh=pcoVC9g6Ql5brnm8ZYrxM8gXd4vKE+CdwztAMluc3jc=; h=Date:From:To:Subject:References:In-Reply-To:Cc:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From; b=DFkSmSH4wRIEKLgm50fFYJmDMmrZUDsYlNZjvrBVN1KfJICp17EwZBDKQf7sx4ssg bOt6iE0t7oJLTnkkMZsN29Twb2pC9i1CCTuU2w+ICN/GjHs2FXbAJUZciZKI7MK0pE uksZDDjKSf2Nb6VOOxkB0h3Fbn51XD4sS87VpUrA= Received: from alsa1.perex.cz (localhost.localdomain [127.0.0.1]) by alsa1.perex.cz (Postfix) with ESMTP id 92BF6F80152; Fri, 5 Feb 2021 15:59:13 +0100 (CET) Received: by alsa1.perex.cz (Postfix, from userid 50401) id DA014F8015A; Fri, 5 Feb 2021 15:59:11 +0100 (CET) Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by alsa1.perex.cz (Postfix) with ESMTPS id DFB85F800C0 for ; Fri, 5 Feb 2021 15:59:08 +0100 (CET) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa1.perex.cz DFB85F800C0 Authentication-Results: alsa1.perex.cz; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="dAu9XING" Received: by mail.kernel.org (Postfix) with ESMTPSA id 220B864FBC; Fri, 5 Feb 2021 14:59:05 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1612537146; bh=pcoVC9g6Ql5brnm8ZYrxM8gXd4vKE+CdwztAMluc3jc=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=dAu9XINGXWMXKM/v4VSMEpR5UUCt36txoacXH7ka0tNv+vG8Uhvnlhf5OOFcYyRyK tKUvQdjoSvGBnoTZ1QDZRsluTJ4fRW+i0tlc+VKsUF9R5bkMFO921B8eKwzT9xS89+ BxgNKjCkkKuVdr2Xb16fv/oWk2wF/IpowhfTwmRmdl/hk0xAPKiy6q/JPbOV9rihec DwKrhy22qVj5ArEmPBouXYoyduoc9mEyFyB8W2cvPw7M1ank7v970HjrN9h2zkMlCC yhP5EQx8DqzkpHfzgFM25WJfLei1aGv86v8U7bPoZpWQE7W4pd9B5eIU9O0dmMTnAy 8ir6e/XFyhIKg== Date: Fri, 5 Feb 2021 14:58:16 +0000 From: Mark Brown To: Shengjiu Wang Subject: Re: [PATCH 5/7] ASoC: imx-pcm-rpmsg: Add platform driver for audio base on rpmsg Message-ID: <20210205145816.GD4720@sirena.org.uk> References: <1612508250-10586-1-git-send-email-shengjiu.wang@nxp.com> <1612508250-10586-6-git-send-email-shengjiu.wang@nxp.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="k4f25fnPtRuIRUb3" Content-Disposition: inline In-Reply-To: <1612508250-10586-6-git-send-email-shengjiu.wang@nxp.com> X-Cookie: Huh? User-Agent: Mutt/1.10.1 (2018-07-13) Cc: devicetree@vger.kernel.org, alsa-devel@alsa-project.org, timur@kernel.org, lgirdwood@gmail.com, linuxppc-dev@lists.ozlabs.org, Xiubo.Lee@gmail.com, linux-kernel@vger.kernel.org, tiwai@suse.com, nicoleotsuka@gmail.com, robh+dt@kernel.org, festevam@gmail.com 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" --k4f25fnPtRuIRUb3 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Fri, Feb 05, 2021 at 02:57:28PM +0800, Shengjiu Wang wrote: > + if (params_format(params) == SNDRV_PCM_FORMAT_S16_LE) > + msg->s_msg.param.format = RPMSG_S16_LE; > + else if (params_format(params) == SNDRV_PCM_FORMAT_S24_LE) Again this should be a switch statement. > + if (params_channels(params) == 1) > + msg->s_msg.param.channels = RPMSG_CH_LEFT; > + else > + msg->s_msg.param.channels = RPMSG_CH_STEREO; Shouldn't this be reporting an error if the number of channels is more than 2? > + /* > + * if the data in the buffer is less than one period > + * send message immediately. > + * if there is more than one period data, delay one > + * period (timer) to send the message. > + */ > + if ((avail - writen_num * period_size) <= period_size) { > + imx_rpmsg_insert_workqueue(substream, msg, info); > + } else if (rpmsg->force_lpa && !timer_pending(timer)) { > + int time_msec; > + > + time_msec = (int)(runtime->period_size * 1000 / runtime->rate); > + mod_timer(timer, jiffies + msecs_to_jiffies(time_msec)); > + } The comment here is at least confusing - why would we not send a full buffer immediately if we have one? This sounds like it's the opposite way round to what we'd do if we were trying to cut down the number of messages. It might help to say which buffer and where? > + /** > + * Every work in the work queue, first we check if there /** comments are only for kerneldoc. --k4f25fnPtRuIRUb3 Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQEzBAABCgAdFiEEreZoqmdXGLWf4p/qJNaLcl1Uh9AFAmAdXQcACgkQJNaLcl1U h9ACUwf9EcHPKiRzzRa6Atb6PHhaM1oBK/2zYZcdmLDejwBct/KltZywmVsBQv0o JmeLnKw7/jLk3Sph4Pqk6J2lyizC5nik/w7NjFO5CIUyNTQnFRZtaDILcnVr7vNk 28HX0/XoPM54EbfyncrP41lr/L4EYgHmjIMqi/TjVtFnfyOt1Pq99Rj02lKDVnV3 ERmOguBociG3yf9kV/wcrZzJ4hOg7Lw468CHtxoeCpPKsJovmByQ0I78JQJlJ1Jj TRjC06zUmRhscWFCrWiOkItqPpTcrv5TxMVh5Ko5zE1rYslk8XURTFpnKDxWoxkR MtylP+v1qS4G4STsZObKZtcso3D9hA== =hzt6 -----END PGP SIGNATURE----- --k4f25fnPtRuIRUb3-- 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=BAYES_00,DKIM_INVALID, DKIM_SIGNED,MAILING_LIST_MULTI,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 7E083C433DB for ; Fri, 5 Feb 2021 15:00:48 +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 9DB18650DD for ; Fri, 5 Feb 2021 15:00:47 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 9DB18650DD Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org 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 4DXJWc0SWzzDvbp for ; Sat, 6 Feb 2021 02:00:44 +1100 (AEDT) Authentication-Results: lists.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=kernel.org (client-ip=198.145.29.99; helo=mail.kernel.org; envelope-from=broonie@kernel.org; receiver=) Authentication-Results: lists.ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=kernel.org header.i=@kernel.org header.a=rsa-sha256 header.s=k20201202 header.b=dAu9XING; dkim-atps=neutral Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 4DXJTn2rn7zDvbl for ; Sat, 6 Feb 2021 01:59:09 +1100 (AEDT) Received: by mail.kernel.org (Postfix) with ESMTPSA id 220B864FBC; Fri, 5 Feb 2021 14:59:05 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1612537146; bh=pcoVC9g6Ql5brnm8ZYrxM8gXd4vKE+CdwztAMluc3jc=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=dAu9XINGXWMXKM/v4VSMEpR5UUCt36txoacXH7ka0tNv+vG8Uhvnlhf5OOFcYyRyK tKUvQdjoSvGBnoTZ1QDZRsluTJ4fRW+i0tlc+VKsUF9R5bkMFO921B8eKwzT9xS89+ BxgNKjCkkKuVdr2Xb16fv/oWk2wF/IpowhfTwmRmdl/hk0xAPKiy6q/JPbOV9rihec DwKrhy22qVj5ArEmPBouXYoyduoc9mEyFyB8W2cvPw7M1ank7v970HjrN9h2zkMlCC yhP5EQx8DqzkpHfzgFM25WJfLei1aGv86v8U7bPoZpWQE7W4pd9B5eIU9O0dmMTnAy 8ir6e/XFyhIKg== Date: Fri, 5 Feb 2021 14:58:16 +0000 From: Mark Brown To: Shengjiu Wang Subject: Re: [PATCH 5/7] ASoC: imx-pcm-rpmsg: Add platform driver for audio base on rpmsg Message-ID: <20210205145816.GD4720@sirena.org.uk> References: <1612508250-10586-1-git-send-email-shengjiu.wang@nxp.com> <1612508250-10586-6-git-send-email-shengjiu.wang@nxp.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="k4f25fnPtRuIRUb3" Content-Disposition: inline In-Reply-To: <1612508250-10586-6-git-send-email-shengjiu.wang@nxp.com> X-Cookie: Huh? User-Agent: Mutt/1.10.1 (2018-07-13) 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: devicetree@vger.kernel.org, alsa-devel@alsa-project.org, timur@kernel.org, lgirdwood@gmail.com, linuxppc-dev@lists.ozlabs.org, Xiubo.Lee@gmail.com, linux-kernel@vger.kernel.org, tiwai@suse.com, nicoleotsuka@gmail.com, robh+dt@kernel.org, perex@perex.cz, festevam@gmail.com Errors-To: linuxppc-dev-bounces+linuxppc-dev=archiver.kernel.org@lists.ozlabs.org Sender: "Linuxppc-dev" --k4f25fnPtRuIRUb3 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Fri, Feb 05, 2021 at 02:57:28PM +0800, Shengjiu Wang wrote: > + if (params_format(params) == SNDRV_PCM_FORMAT_S16_LE) > + msg->s_msg.param.format = RPMSG_S16_LE; > + else if (params_format(params) == SNDRV_PCM_FORMAT_S24_LE) Again this should be a switch statement. > + if (params_channels(params) == 1) > + msg->s_msg.param.channels = RPMSG_CH_LEFT; > + else > + msg->s_msg.param.channels = RPMSG_CH_STEREO; Shouldn't this be reporting an error if the number of channels is more than 2? > + /* > + * if the data in the buffer is less than one period > + * send message immediately. > + * if there is more than one period data, delay one > + * period (timer) to send the message. > + */ > + if ((avail - writen_num * period_size) <= period_size) { > + imx_rpmsg_insert_workqueue(substream, msg, info); > + } else if (rpmsg->force_lpa && !timer_pending(timer)) { > + int time_msec; > + > + time_msec = (int)(runtime->period_size * 1000 / runtime->rate); > + mod_timer(timer, jiffies + msecs_to_jiffies(time_msec)); > + } The comment here is at least confusing - why would we not send a full buffer immediately if we have one? This sounds like it's the opposite way round to what we'd do if we were trying to cut down the number of messages. It might help to say which buffer and where? > + /** > + * Every work in the work queue, first we check if there /** comments are only for kerneldoc. --k4f25fnPtRuIRUb3 Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQEzBAABCgAdFiEEreZoqmdXGLWf4p/qJNaLcl1Uh9AFAmAdXQcACgkQJNaLcl1U h9ACUwf9EcHPKiRzzRa6Atb6PHhaM1oBK/2zYZcdmLDejwBct/KltZywmVsBQv0o JmeLnKw7/jLk3Sph4Pqk6J2lyizC5nik/w7NjFO5CIUyNTQnFRZtaDILcnVr7vNk 28HX0/XoPM54EbfyncrP41lr/L4EYgHmjIMqi/TjVtFnfyOt1Pq99Rj02lKDVnV3 ERmOguBociG3yf9kV/wcrZzJ4hOg7Lw468CHtxoeCpPKsJovmByQ0I78JQJlJ1Jj TRjC06zUmRhscWFCrWiOkItqPpTcrv5TxMVh5Ko5zE1rYslk8XURTFpnKDxWoxkR MtylP+v1qS4G4STsZObKZtcso3D9hA== =hzt6 -----END PGP SIGNATURE----- --k4f25fnPtRuIRUb3--