Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
In-Reply-To: <1644850708-11099-4-git-send-email-quic_srivasam@quicinc.com>
References: <1644850708-11099-1-git-send-email-quic_srivasam@quicinc.com> <1644850708-11099-4-git-send-email-quic_srivasam@quicinc.com>
From:   Stephen Boyd <swboyd@chromium.org>
User-Agent: alot/0.10
Date:   Mon, 14 Feb 2022 17:05:39 -0800
Message-ID: <CAE-0n53dXfwsJ=mh7cNH6Lq0Um+h+oTJDvqsXALh8L=wfONLjw@mail.gmail.com>
Subject: Re: [RESEND v13 03/10] ASoC: qcom: lpass: Add dma fields for codec
 dma lpass interface
To:     Srinivasa Rao Mandadapu <quic_srivasam@quicinc.com>,
        agross@kernel.org, alsa-devel@alsa-project.org,
        bgoswami@codeaurora.org, bjorn.andersson@linaro.org,
        broonie@kernel.org, devicetree@vger.kernel.org,
        judyhsiao@chromium.org, lgirdwood@gmail.com,
        linux-arm-msm@vger.kernel.org, linux-kernel@vger.kernel.org,
        perex@perex.cz, quic_plai@quicinc.com, robh+dt@kernel.org,
        rohitkr@codeaurora.org, srinivas.kandagatla@linaro.org,
        tiwai@suse.com
Cc:     Venkata Prasad Potturu <quic_potturu@quicinc.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Quoting Srinivasa Rao Mandadapu (2022-02-14 06:58:21)
> Add lpass interface memebers to support audio path over codec dma.
>
> Signed-off-by: Srinivasa Rao Mandadapu <quic_srivasam@quicinc.com>
> Co-developed-by: Venkata Prasad Potturu <quic_potturu@quicinc.com>
> Signed-off-by: Venkata Prasad Potturu <quic_potturu@quicinc.com>
> Reviewed-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> ---
>  sound/soc/qcom/lpass.h | 116 +++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 116 insertions(+)
>
> diff --git a/sound/soc/qcom/lpass.h b/sound/soc/qcom/lpass.h
> index f0d21cd..7cc3763 100644
> --- a/sound/soc/qcom/lpass.h
> +++ b/sound/soc/qcom/lpass.h
> @@ -234,12 +294,66 @@ struct lpass_variant {
>         struct reg_field wrdma_enable;
>         struct reg_field wrdma_dyncclk;
>
> +       /*CDC RXTX RD_DMA */

Please add space after /*

> +       struct reg_field rxtx_rdma_intf;
> +       struct reg_field rxtx_rdma_bursten;
> +       struct reg_field rxtx_rdma_wpscnt;
> +       struct reg_field rxtx_rdma_fifowm;
> +       struct reg_field rxtx_rdma_enable;
> +       struct reg_field rxtx_rdma_dyncclk;
> +       struct reg_field rxtx_rdma_burst8;
> +       struct reg_field rxtx_rdma_burst16;
> +       struct reg_field rxtx_rdma_dynburst;
> +       struct reg_field rxtx_rdma_codec_enable;
> +       struct reg_field rxtx_rdma_codec_pack;
> +       struct reg_field rxtx_rdma_codec_intf;
> +       struct reg_field rxtx_rdma_codec_fs_sel;
> +       struct reg_field rxtx_rdma_codec_ch;
> +       struct reg_field rxtx_rdma_codec_fs_delay;
> +
> +       /*CDC RXTX WR_DMA */

Same

> +       struct reg_field rxtx_wrdma_intf;
> +       struct reg_field rxtx_wrdma_bursten;
> +       struct reg_field rxtx_wrdma_wpscnt;
> +       struct reg_field rxtx_wrdma_fifowm;
> +       struct reg_field rxtx_wrdma_enable;
> +       struct reg_field rxtx_wrdma_dyncclk;
> +       struct reg_field rxtx_wrdma_burst8;
> +       struct reg_field rxtx_wrdma_burst16;
> +       struct reg_field rxtx_wrdma_dynburst;
> +       struct reg_field rxtx_wrdma_codec_enable;
> +       struct reg_field rxtx_wrdma_codec_pack;
> +       struct reg_field rxtx_wrdma_codec_intf;
> +       struct reg_field rxtx_wrdma_codec_fs_sel;
> +       struct reg_field rxtx_wrdma_codec_ch;
> +       struct reg_field rxtx_wrdma_codec_fs_delay;
> +
> +       /*CDC VA WR_DMA */
> +       struct reg_field va_wrdma_intf;
> +       struct reg_field va_wrdma_bursten;
> +       struct reg_field va_wrdma_wpscnt;
> +       struct reg_field va_wrdma_fifowm;
> +       struct reg_field va_wrdma_enable;
> +       struct reg_field va_wrdma_dyncclk;
> +       struct reg_field va_wrdma_burst8;
> +       struct reg_field va_wrdma_burst16;
> +       struct reg_field va_wrdma_dynburst;
> +       struct reg_field va_wrdma_codec_enable;
> +       struct reg_field va_wrdma_codec_pack;
> +       struct reg_field va_wrdma_codec_intf;
> +       struct reg_field va_wrdma_codec_fs_sel;
> +       struct reg_field va_wrdma_codec_ch;
> +       struct reg_field va_wrdma_codec_fs_delay;
> +
>         /**

This shouldn't have two stars as it isn't kerneldoc

>          * on SOCs like APQ8016 the channel control bits start
>          * at different offset to ipq806x
>          **/
>         u32     dmactl_audif_start;
>         u32     wrdma_channel_start;
> +       u32     rxtx_wrdma_channel_start;
> +       u32     va_wrdma_channel_start;
> +
>         /* SOC specific initialization like clocks */
>         int (*init)(struct platform_device *pdev);
>         int (*exit)(struct platform_device *pdev);
> @@ -251,10 +365,12 @@ struct lpass_variant {
>         int num_dai;
>         const char * const *dai_osr_clk_names;
>         const char * const *dai_bit_clk_names;
> +       const char * const *cdc_dma_clk_names;
>
>         /* SOC specific clocks configuration */
>         const char **clk_name;
>         int num_clks;
> +       int cdc_dma_num_clks;

Why not size_t? Negative numbers are useful here?