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?

On 2/15/2022 6:40 AM, Stephen Boyd wrote: Thanks for your time Stephen!!!

Quoting Srinivasa Rao Mandadapu (2022-02-14 06:58:22)

...

Add support function to get dma control and lpaif handle to avoid repeated code in platform driver

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-platform.c | 113 +++++++++++++++++++++++----------------- 1 file changed, 66 insertions(+), 47 deletions(-)

diff --git a/sound/soc/qcom/lpass-platform.c b/sound/soc/qcom/lpass-platform.c index a44162c..5d77240 100644 --- a/sound/soc/qcom/lpass-platform.c +++ b/sound/soc/qcom/lpass-platform.c @@ -177,6 +177,49 @@ static int lpass_platform_pcmops_close(struct snd_soc_component *component, return 0; }

+static void __lpass_get_lpaif_handle(struct snd_pcm_substream *substream,

const?

Okay. will add const to substream pointer.

...

•                                struct snd_soc_component *component,
  

const?

Here const is giving compilation errors in below code.

...

•                                struct lpaif_dmactl **dmactl, int *id, struct regmap **map)
  

+{

•   struct snd_soc_pcm_runtime *soc_runtime = asoc_substream_to_rtd(substream);
  

•   struct snd_soc_dai *cpu_dai = asoc_rtd_to_cpu(soc_runtime, 0);
  

•   struct lpass_data *drvdata = snd_soc_component_get_drvdata(component);
  

•   struct snd_pcm_runtime *rt = substream->runtime;
  

•   struct lpass_pcm_data *pcm_data = rt->private_data;
  

•   struct lpass_variant *v = drvdata->variant;
  

•   int dir = substream->stream;
  

•   unsigned int dai_id = cpu_dai->driver->id;
  

•   struct lpaif_dmactl *l_dmactl = NULL;
  

•   struct regmap *l_map = NULL;
  

•   int l_id = 0;
  

•   switch (dai_id) {
  

•   case MI2S_PRIMARY ... MI2S_QUINARY:
  

•           if (dir == SNDRV_PCM_STREAM_PLAYBACK) {
  

Please write if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) and drop 'dir' local variable.

Okay. will change it.

...

•                   l_id = pcm_data->dma_ch;
  

•                   l_dmactl = drvdata->rd_dmactl;
  

•           } else {
  

•                   l_dmactl = drvdata->wr_dmactl;
  

•                   l_id = pcm_data->dma_ch - v->wrdma_channel_start;
  

•           }
  

•           l_map = drvdata->lpaif_map;
  

•           break;
  

•   case LPASS_DP_RX:
  

•           l_id = pcm_data->dma_ch;
  

•           l_dmactl = drvdata->hdmi_rd_dmactl;
  

•           l_map = drvdata->hdmiif_map;
  

•           break;
  

•   default:
  

•           break;
  

•   }
  

•   if (dmactl)
  

•           *dmactl = l_dmactl;
  

•   if (id)
  

•           *id = l_id;
  

•   if (map)
  

•           *map = l_map;
  

Why not 'return 0' here and return -EINVAL in the default case above? Then we can skip the checks for !map or !dmactl below and simply bail out if it failed with an error value.

Here the check is for input params. some users call for only damctl or only map.

so check seems mandatory for updating only valid fields. Will remove default case which may never occurs.

...

+}

• static int lpass_platform_pcmops_hw_params(struct snd_soc_component *component, struct snd_pcm_substream *substream, struct snd_pcm_hw_params *params)

@@ -191,21 +234,15 @@ static int lpass_platform_pcmops_hw_params(struct snd_soc_component *component, unsigned int channels = params_channels(params); unsigned int regval; struct lpaif_dmactl *dmactl;

•   int id, dir = substream->stream;
  

•   int id;
     int bitwidth;
     int ret, dma_port = pcm_data->i2s_port + v->dmactl_audif_start;
     unsigned int dai_id = cpu_dai->driver->id;
  
  

•   if (dir ==  SNDRV_PCM_STREAM_PLAYBACK) {
  

•           id = pcm_data->dma_ch;
  

•           if (dai_id == LPASS_DP_RX)
  

•                   dmactl = drvdata->hdmi_rd_dmactl;
  

•           else
  

•                   dmactl = drvdata->rd_dmactl;
  

•   } else {
  

•           dmactl = drvdata->wr_dmactl;
  

•           id = pcm_data->dma_ch - v->wrdma_channel_start;
  

•   __lpass_get_lpaif_handle(substream, component, &dmactl, &id, NULL);
  

•   if (!dmactl) {
  

•           dev_err(soc_runtime->dev, "failed to get dmactl handle\n");
  

•           return -EINVAL;
     }
  
     bitwidth = snd_pcm_format_width(format);
  

@@ -350,10 +387,11 @@ static int lpass_platform_pcmops_hw_free(struct snd_soc_component *component, struct regmap *map; unsigned int dai_id = cpu_dai->driver->id;

•   if (dai_id == LPASS_DP_RX)
  

•           map = drvdata->hdmiif_map;
  

•   else
  

•           map = drvdata->lpaif_map;
  

•   __lpass_get_lpaif_handle(substream, component, NULL, NULL, &map);
  

•   if (!map) {
  

•           dev_err(soc_runtime->dev, "failed to get dmactl handle\n");
  

•           return -EINVAL;
  

•   }
  
     reg = LPAIF_DMACTL_REG(v, pcm_data->dma_ch, substream->stream, dai_id);
     ret = regmap_write(map, reg, 0);
  

@@ -379,22 +417,12 @@ static int lpass_platform_pcmops_prepare(struct snd_soc_component *component, int ret, id, ch, dir = substream->stream; unsigned int dai_id = cpu_dai->driver->id;

•    ch = pcm_data->dma_ch;
  

•   if (dir ==  SNDRV_PCM_STREAM_PLAYBACK) {
  

•           if (dai_id == LPASS_DP_RX) {
  

•                   dmactl = drvdata->hdmi_rd_dmactl;
  

•                   map = drvdata->hdmiif_map;
  

•           } else {
  

•                   dmactl = drvdata->rd_dmactl;
  

•                   map = drvdata->lpaif_map;
  

•           }
  
  

•           id = pcm_data->dma_ch;
  

•   } else {
  

•           dmactl = drvdata->wr_dmactl;
  

•           id = pcm_data->dma_ch - v->wrdma_channel_start;
  

•           map = drvdata->lpaif_map;
  

•   __lpass_get_lpaif_handle(substream, component, &dmactl, &id, &map);
  

•   if (!dmactl) {
  

•           dev_err(soc_runtime->dev, "failed to get dmactl handle\n");
  

•           return -EINVAL;
     }
  
     ret = regmap_write(map, LPAIF_DMABASE_REG(v, ch, dir, dai_id),
  

@@ -444,25 +472,15 @@ static int lpass_platform_pcmops_trigger(struct snd_soc_component *component, struct lpaif_dmactl *dmactl; struct regmap *map; int ret, ch, id;

•   int dir = substream->stream;
     unsigned int reg_irqclr = 0, val_irqclr = 0;
     unsigned int  reg_irqen = 0, val_irqen = 0, val_mask = 0;
     unsigned int dai_id = cpu_dai->driver->id;
  
     ch = pcm_data->dma_ch;
  

•   if (dir ==  SNDRV_PCM_STREAM_PLAYBACK) {
  

•           id = pcm_data->dma_ch;
  

•           if (dai_id == LPASS_DP_RX) {
  

•                   dmactl = drvdata->hdmi_rd_dmactl;
  

•                   map = drvdata->hdmiif_map;
  

•           } else {
  

•                   dmactl = drvdata->rd_dmactl;
  

•                   map = drvdata->lpaif_map;
  

•           }
  

•   } else {
  

•           dmactl = drvdata->wr_dmactl;
  

•           id = pcm_data->dma_ch - v->wrdma_channel_start;
  

•           map = drvdata->lpaif_map;
  

•   __lpass_get_lpaif_handle(substream, component, &dmactl, &id, &map);
  

•   if (!dmactl) {
  

•           dev_err(soc_runtime->dev, "failed to get dmactl handle\n");
  

•           return -EINVAL;
     }
  
     switch (cmd) {
  

@@ -597,10 +615,11 @@ static snd_pcm_uframes_t lpass_platform_pcmops_pointer( struct regmap *map; unsigned int dai_id = cpu_dai->driver->id;

•   if (dai_id == LPASS_DP_RX)
  

•           map = drvdata->hdmiif_map;
  

•   else
  

•           map = drvdata->lpaif_map;
  

•   __lpass_get_lpaif_handle(substream, component, NULL, NULL, &map);
  

•   if (!map) {
  

•           dev_err(soc_runtime->dev, "failed to get dmactl handle\n");
  

•           return -EINVAL;
  

•   }
  
     ch = pcm_data->dma_ch;
  
  

-- 2.7.4

On 2/18/2022 1:11 AM, Stephen Boyd wrote: Thanks for Your time Stephen!!!

Quoting Srinivasa Rao Mandadapu (2022-02-15 21:11:29)

...

On 2/15/2022 6:40 AM, Stephen Boyd wrote: Thanks for your time Stephen!!!

...

Quoting Srinivasa Rao Mandadapu (2022-02-14 06:58:22)

...

Add support function to get dma control and lpaif handle to avoid repeated code in platform driver

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-platform.c | 113 +++++++++++++++++++++++----------------- 1 file changed, 66 insertions(+), 47 deletions(-)

diff --git a/sound/soc/qcom/lpass-platform.c b/sound/soc/qcom/lpass-platform.c index a44162c..5d77240 100644 --- a/sound/soc/qcom/lpass-platform.c +++ b/sound/soc/qcom/lpass-platform.c @@ -177,6 +177,49 @@ static int lpass_platform_pcmops_close(struct snd_soc_component *component, return 0; }

+static void __lpass_get_lpaif_handle(struct snd_pcm_substream *substream,

const?

Okay. will add const to substream pointer.

...

...

•                                struct snd_soc_component *component,
  

const?

Here const is giving compilation errors in below code.

Ok

...

...

...

•                   l_id = pcm_data->dma_ch;
  

•                   l_dmactl = drvdata->rd_dmactl;
  

•           } else {
  

•                   l_dmactl = drvdata->wr_dmactl;
  

•                   l_id = pcm_data->dma_ch - v->wrdma_channel_start;
  

•           }
  

•           l_map = drvdata->lpaif_map;
  

•           break;
  

•   case LPASS_DP_RX:
  

•           l_id = pcm_data->dma_ch;
  

•           l_dmactl = drvdata->hdmi_rd_dmactl;
  

•           l_map = drvdata->hdmiif_map;
  

•           break;
  

•   default:
  

•           break;
  

•   }
  

•   if (dmactl)
  

•           *dmactl = l_dmactl;
  

•   if (id)
  

•           *id = l_id;
  

•   if (map)
  

•           *map = l_map;
  

Why not 'return 0' here and return -EINVAL in the default case above? Then we can skip the checks for !map or !dmactl below and simply bail out if it failed with an error value.

Here the check is for input params. some users call for only damctl or only map.

Yes the check is to make sure there's a pointer to store the value into. I get that. The users are all internal to this driver though because the function is static. If the function returned an error because it couldn't find something then we wouldn't have to test the resulting pointers for success, instead we could check for a return value. Similarly, it may be clearer to have a single get for each item and then return a pointer from the function vs. passing it in to extract something. It may duplicate some code but otherwise the code would be clearer because we're getting one thing and can pass an error value through the pointer with PTR_ERR().

Okay. Agreed. But in initial review comments, asked to make common function for code readability,

and to avoid duplicate code.

Anyway will change accordingly and re post it.

Quoting Srinivasa Rao Mandadapu (2022-02-14 06:58:23)

This patch adds register definitions for codec read dma and write dma

git grep "This patch" -- Documentation/process/

lpass interface.

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

diff --git a/sound/soc/qcom/lpass.h b/sound/soc/qcom/lpass.h index 7cc3763..e059c4a 100644 --- a/sound/soc/qcom/lpass.h +++ b/sound/soc/qcom/lpass.h @@ -39,6 +39,29 @@ return -EINVAL; \ } while (0)

+static inline bool is_cdc_dma_port(int dai_id) +{

•   switch (dai_id) {
  

•   case LPASS_CDC_DMA_RX0 ... LPASS_CDC_DMA_RX9:
  

•   case LPASS_CDC_DMA_TX0 ... LPASS_CDC_DMA_TX8:
  

•   case LPASS_CDC_DMA_VA_TX0 ... LPASS_CDC_DMA_VA_TX8:
  

•           return true;
  

•   default:
  

Drop case

•           return false;
  

•   }
  

return false;

would be shorter.

+}

+static inline bool is_rxtx_cdc_dma_port(int dai_id) +{

•   switch (dai_id) {
  

•   case LPASS_CDC_DMA_RX0 ... LPASS_CDC_DMA_RX9:
  

•   case LPASS_CDC_DMA_TX0 ... LPASS_CDC_DMA_TX8:
  

•           return true;
  

•   default:
  

•           return false;
  

•   }
  

Same.

+}

struct lpaif_i2sctl { struct regmap_field *loopback; struct regmap_field *spken;

Quoting Srinivasa Rao Mandadapu (2022-02-14 06:58:25)

Upadate lpass cpu and platform driver to support audio over codec dma in ADSP bypass use case.

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 Reported-by: kernel test robot lkp@intel.com

---

sound/soc/qcom/lpass-cpu.c | 59 ++++- sound/soc/qcom/lpass-platform.c | 499 ++++++++++++++++++++++++++++++++++++++-- 2 files changed, 537 insertions(+), 21 deletions(-)

diff --git a/sound/soc/qcom/lpass-cpu.c b/sound/soc/qcom/lpass-cpu.c index 4fb9669..a5a46bc 100644 --- a/sound/soc/qcom/lpass-cpu.c +++ b/sound/soc/qcom/lpass-cpu.c @@ -1042,7 +1042,9 @@ static void of_lpass_cpu_parse_dai_data(struct device *dev, } if (id == LPASS_DP_RX) { data->hdmi_port_enable = 1;

•           } else {
  

•           } else if (is_cdc_dma_port(id))
  

Please add braces if any of the other arms of the if statement have braces

} else if (is_cdc_dma_port(id)) {

•                   data->codec_dma_enable = 1;
  

•           else {
  

} else {

                    data->mi2s_playback_sd_mode[id] =
                            of_lpass_cpu_parse_sd_lines(dev, node,
                                                        "qcom,playback-sd-lines");

@@ -1057,6 +1059,7 @@ int asoc_qcom_lpass_cpu_platform_probe(struct platform_device *pdev) { struct lpass_data *drvdata; struct device_node *dsp_of_node;

•   struct resource *res;
    struct lpass_variant *variant;
    struct device *dev = &pdev->dev;
    const struct of_device_id *match;
  

@@ -1082,6 +1085,58 @@ int asoc_qcom_lpass_cpu_platform_probe(struct platform_device *pdev)

    of_lpass_cpu_parse_dai_data(dev, drvdata);

•   drvdata->num_clks =  variant->num_clks;
  

•   if (drvdata->codec_dma_enable) {
  

•           drvdata->rxtx_lpaif =
  

•                           devm_platform_ioremap_resource_byname(pdev, "lpass-rxtx-lpaif");
  

•           if (IS_ERR(drvdata->rxtx_lpaif))
  

•                   return PTR_ERR(drvdata->rxtx_lpaif);
  

•           drvdata->va_lpaif = devm_platform_ioremap_resource_byname(pdev, "lpass-va-lpaif");
  

•           if (IS_ERR(drvdata->va_lpaif))
  

•                   return PTR_ERR(drvdata->va_lpaif);
  

•           lpass_rxtx_regmap_config.max_register = LPAIF_CDC_RXTX_WRDMAPER_REG(variant,
  

•                                   variant->rxtx_wrdma_channels +
  

•                                   variant->rxtx_wrdma_channel_start, LPASS_CDC_DMA_TX3);
  

•           drvdata->rxtx_lpaif_map = devm_regmap_init_mmio(dev, drvdata->rxtx_lpaif,
  

•                                   &lpass_rxtx_regmap_config);
  

•           if (IS_ERR(drvdata->rxtx_lpaif_map)) {
  

•                   dev_err(dev, "error initializing rxtx regmap: %ld\n",
  

•                           PTR_ERR(drvdata->rxtx_lpaif_map));
  

The regmap core already prints when things go wrong. Let's just use that instead of adding another error message.

•                   return PTR_ERR(drvdata->rxtx_lpaif_map);
  

•           }
  

•           lpass_va_regmap_config.max_register = LPAIF_CDC_VA_WRDMAPER_REG(variant,
  

•                                   variant->va_wrdma_channels +
  

•                                   variant->va_wrdma_channel_start, LPASS_CDC_DMA_VA_TX0);
  

•           drvdata->va_lpaif_map = devm_regmap_init_mmio(dev, drvdata->va_lpaif,
  

•                                   &lpass_va_regmap_config);
  

•           if (IS_ERR(drvdata->va_lpaif_map)) {
  

•                   dev_err(dev, "error initializing va regmap: %ld\n",
  

•                           PTR_ERR(drvdata->va_lpaif_map));
  

•                   return PTR_ERR(drvdata->va_lpaif_map);
  

•           }
  

•           drvdata->cdc_clks = devm_kcalloc(dev, variant->cdc_dma_num_clks,
  

•                                   sizeof(*drvdata->cdc_clks), GFP_KERNEL);
  

•           drvdata->cdc_num_clks = variant->cdc_dma_num_clks;
  

•           for (i = 0; i < drvdata->cdc_num_clks; i++)
  

•                   drvdata->cdc_clks[i].id = variant->cdc_dma_clk_names[i];
  

•           ret = devm_clk_bulk_get(dev, drvdata->cdc_num_clks, drvdata->cdc_clks);
  

Do you care about particular clks? If not please use devm_clk_bulk_get_all() and stop storing clk names and counts.

•           if (ret) {
  

•                   dev_err(dev, "Failed to get clocks %d\n", ret);
  

•                   return ret;
  

•           }
  

•           res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "lpass-rxtx-cdc-dma-lpm");
  

•           drvdata->rxtx_cdc_dma_lpm_buf = res->start;
  

•           res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "lpass-va-cdc-dma-lpm");
  

•           drvdata->va_cdc_dma_lpm_buf = res->start;
  

•   }
    drvdata->lpaif = devm_platform_ioremap_resource_byname(pdev, "lpass-lpaif");
    if (IS_ERR(drvdata->lpaif))
            return PTR_ERR(drvdata->lpaif);
  

@@ -1124,7 +1179,7 @@ int asoc_qcom_lpass_cpu_platform_probe(struct platform_device *pdev)

    for (i = 0; i < variant->num_dai; i++) {
            dai_id = variant->dai_driver[i].id;

•           if (dai_id == LPASS_DP_RX)
  

•           if (dai_id == LPASS_DP_RX || is_cdc_dma_port(dai_id))
                    continue;
  
            drvdata->mi2s_osr_clk[dai_id] = devm_clk_get_optional(dev,
  

diff --git a/sound/soc/qcom/lpass-platform.c b/sound/soc/qcom/lpass-platform.c index 5d77240..12b8d40 100644 --- a/sound/soc/qcom/lpass-platform.c +++ b/sound/soc/qcom/lpass-platform.c @@ -20,6 +20,9 @@

#define LPASS_PLATFORM_BUFFER_SIZE (24 * 2 * 1024) #define LPASS_PLATFORM_PERIODS 2 +#define LPASS_RXTX_CDC_DMA_LPM_BUFF_SIZE (8 * 1024) +#define LPASS_VA_CDC_DMA_LPM_BUFF_SIZE (12 * 1024) +#define LPASS_CDC_DMA_REGISTER_FIELDS_MAX 15

static const struct snd_pcm_hardware lpass_platform_pcm_hardware = { .info = SNDRV_PCM_INFO_MMAP | @@ -45,6 +48,99 @@ static const struct snd_pcm_hardware lpass_platform_pcm_hardware = { .fifo_size = 0, };

+static const struct snd_pcm_hardware lpass_platform_rxtx_hardware = {

•   .info                   =       SNDRV_PCM_INFO_MMAP |
  

•                                   SNDRV_PCM_INFO_MMAP_VALID |
  

•                                   SNDRV_PCM_INFO_INTERLEAVED |
  

•                                   SNDRV_PCM_INFO_PAUSE |
  

•                                   SNDRV_PCM_INFO_RESUME,
  

•   .formats                =       SNDRV_PCM_FMTBIT_S16 |
  

•                                   SNDRV_PCM_FMTBIT_S24 |
  

•                                   SNDRV_PCM_FMTBIT_S32,
  

•   .rates                  =       SNDRV_PCM_RATE_8000_192000,
  

•   .rate_min               =       8000,
  

•   .rate_max               =       192000,
  

•   .channels_min           =       1,
  

•   .channels_max           =       8,
  

•   .buffer_bytes_max       =       LPASS_RXTX_CDC_DMA_LPM_BUFF_SIZE,
  

•   .period_bytes_max       =       LPASS_RXTX_CDC_DMA_LPM_BUFF_SIZE /
  

•                                           LPASS_PLATFORM_PERIODS,
  

•   .period_bytes_min       =       LPASS_RXTX_CDC_DMA_LPM_BUFF_SIZE /
  

•                                           LPASS_PLATFORM_PERIODS,
  

•   .periods_min            =       LPASS_PLATFORM_PERIODS,
  

•   .periods_max            =       LPASS_PLATFORM_PERIODS,
  

•   .fifo_size              =       0,
  

+};

+static const struct snd_pcm_hardware lpass_platform_va_hardware = {

•   .info                   =       SNDRV_PCM_INFO_MMAP |
  

•                                   SNDRV_PCM_INFO_MMAP_VALID |
  

•                                   SNDRV_PCM_INFO_INTERLEAVED |
  

•                                   SNDRV_PCM_INFO_PAUSE |
  

•                                   SNDRV_PCM_INFO_RESUME,
  

•   .formats                =       SNDRV_PCM_FMTBIT_S16 |
  

•                                   SNDRV_PCM_FMTBIT_S24 |
  

•                                   SNDRV_PCM_FMTBIT_S32,
  

•   .rates                  =       SNDRV_PCM_RATE_8000_192000,
  

•   .rate_min               =       8000,
  

•   .rate_max               =       192000,
  

•   .channels_min           =       1,
  

•   .channels_max           =       8,
  

•   .buffer_bytes_max       =       LPASS_VA_CDC_DMA_LPM_BUFF_SIZE,
  

•   .period_bytes_max       =       LPASS_VA_CDC_DMA_LPM_BUFF_SIZE /
  

•                                           LPASS_PLATFORM_PERIODS,
  

•   .period_bytes_min       =       LPASS_VA_CDC_DMA_LPM_BUFF_SIZE /
  

•                                           LPASS_PLATFORM_PERIODS,
  

•   .periods_min            =       LPASS_PLATFORM_PERIODS,
  

•   .periods_max            =       LPASS_PLATFORM_PERIODS,
  

•   .fifo_size              =       0,
  

+};

+static int lpass_platform_alloc_rxtx_dmactl_fields(struct device *dev,

•                                    struct regmap *map)
  

+{

•   struct lpass_data *drvdata = dev_get_drvdata(dev);
  

•   struct lpass_variant *v = drvdata->variant;
  

•   struct lpaif_dmactl *rd_dmactl, *wr_dmactl;
  

•   int rval;
  

•   rd_dmactl = devm_kzalloc(dev, sizeof(*rd_dmactl), GFP_KERNEL);
  

•   if (rd_dmactl == NULL)
  

if (!rd_dmactl)

is kernel idiomatic. Please follow that style.

•           return -ENOMEM;
  

•   wr_dmactl = devm_kzalloc(dev, sizeof(*wr_dmactl), GFP_KERNEL);
  

•   if (wr_dmactl == NULL)
  

•           return -ENOMEM;
  

•   drvdata->rxtx_rd_dmactl = rd_dmactl;
  

•   drvdata->rxtx_wr_dmactl = wr_dmactl;
  

•   rval = devm_regmap_field_bulk_alloc(dev, map, &rd_dmactl->intf,
  

•                                       &v->rxtx_rdma_intf, LPASS_CDC_DMA_REGISTER_FIELDS_MAX);
  

•   if (rval)
  

•           return rval;
  

•   return devm_regmap_field_bulk_alloc(dev, map, &wr_dmactl->intf,
  

•                                       &v->rxtx_wrdma_intf, LPASS_CDC_DMA_REGISTER_FIELDS_MAX);
  

+}

+static int lpass_platform_alloc_va_dmactl_fields(struct device *dev,

•                                    struct regmap *map)
  

+{

•   struct lpass_data *drvdata = dev_get_drvdata(dev);
  

•   struct lpass_variant *v = drvdata->variant;
  

•   struct lpaif_dmactl *wr_dmactl;
  

•   wr_dmactl = devm_kzalloc(dev, sizeof(*wr_dmactl), GFP_KERNEL);
  

•   if (wr_dmactl == NULL)
  

•           return -ENOMEM;
  

•   drvdata->va_wr_dmactl = wr_dmactl;
  

•   return devm_regmap_field_bulk_alloc(dev, map, &wr_dmactl->intf,
  

•                                       &v->va_wrdma_intf, LPASS_CDC_DMA_REGISTER_FIELDS_MAX);
  

+}

static int lpass_platform_alloc_dmactl_fields(struct device *dev, struct regmap *map) { @@ -123,25 +219,55 @@ static int lpass_platform_pcmops_open(struct snd_soc_component *component, return dma_ch; }

•   if (cpu_dai->driver->id == LPASS_DP_RX) {
  

•           map = drvdata->hdmiif_map;
  

•           drvdata->hdmi_substream[dma_ch] = substream;
  

•   } else {
  

•   switch (dai_id) {
  

•   case MI2S_PRIMARY ... MI2S_QUINARY:
            map = drvdata->lpaif_map;
            drvdata->substream[dma_ch] = substream;
  

•           break;
  

•   case LPASS_DP_RX:
  

•           map = drvdata->hdmiif_map;
  

•           drvdata->hdmi_substream[dma_ch] = substream;
  

•           break;
  

•   case LPASS_CDC_DMA_RX0 ... LPASS_CDC_DMA_RX9:
  

•   case LPASS_CDC_DMA_TX0 ... LPASS_CDC_DMA_TX8:
  

•           map = drvdata->rxtx_lpaif_map;
  

•           drvdata->rxtx_substream[dma_ch] = substream;
  

•           break;
  

•   case LPASS_CDC_DMA_VA_TX0 ... LPASS_CDC_DMA_VA_TX8:
  

•           map = drvdata->va_lpaif_map;
  

•           drvdata->va_substream[dma_ch] = substream;
  

•           break;
  

•   default:
  

•           break;
    }
  

•   data->dma_ch = dma_ch;
  

•   ret = regmap_write(map,
  

•                   LPAIF_DMACTL_REG(v, dma_ch, dir, data->i2s_port), 0);
  

•   if (ret) {
  

•           dev_err(soc_runtime->dev,
  

•                   "error writing to rdmactl reg: %d\n", ret);
  

•           return ret;
  

•   switch (dai_id) {
  

•   case MI2S_PRIMARY ... MI2S_QUINARY:
  

•   case LPASS_DP_RX:
  

•           ret = regmap_write(map, LPAIF_DMACTL_REG(v, dma_ch, dir, data->i2s_port), 0);
  

•           if (ret) {
  

•                   kfree(data);
  

•                   dev_err(soc_runtime->dev, "error writing to rdmactl reg: %d\n", ret);
  

•                   return ret;
  

•           }
  

•           snd_soc_set_runtime_hwparams(substream, &lpass_platform_pcm_hardware);
  

•           runtime->dma_bytes = lpass_platform_pcm_hardware.buffer_bytes_max;
  

•           break;
  

•   case LPASS_CDC_DMA_RX0 ... LPASS_CDC_DMA_RX9:
  

•   case LPASS_CDC_DMA_TX0 ... LPASS_CDC_DMA_TX8:
  

•           snd_soc_set_runtime_hwparams(substream, &lpass_platform_rxtx_hardware);
  

•           runtime->dma_bytes = lpass_platform_rxtx_hardware.buffer_bytes_max;
  

•           snd_pcm_set_runtime_buffer(substream, &substream->dma_buffer);
  

•           break;
  

•   case LPASS_CDC_DMA_VA_TX0 ... LPASS_CDC_DMA_VA_TX8:
  

•           snd_soc_set_runtime_hwparams(substream, &lpass_platform_va_hardware);
  

•           runtime->dma_bytes = lpass_platform_va_hardware.buffer_bytes_max;
  

•           snd_pcm_set_runtime_buffer(substream, &substream->dma_buffer);
  

•           break;
  

•   default:
  

•           break;
    }
  

•   snd_soc_set_runtime_hwparams(substream, &lpass_platform_pcm_hardware);
  

•   runtime->dma_bytes = lpass_platform_pcm_hardware.buffer_bytes_max;
  

•   ret = snd_pcm_hw_constraint_integer(runtime,
                    SNDRV_PCM_HW_PARAM_PERIODS);
    if (ret < 0) {
  

@@ -166,10 +292,25 @@ static int lpass_platform_pcmops_close(struct snd_soc_component *component, unsigned int dai_id = cpu_dai->driver->id;

    data = runtime->private_data;

•   if (dai_id == LPASS_DP_RX)
  

•           drvdata->hdmi_substream[data->dma_ch] = NULL;
  

•   else
  

•   switch (dai_id) {
  

•   case MI2S_PRIMARY ... MI2S_QUINARY:
            drvdata->substream[data->dma_ch] = NULL;
  

•           break;
  

•   case LPASS_DP_RX:
  

•           drvdata->hdmi_substream[data->dma_ch] = NULL;
  

•           break;
  

•   case LPASS_CDC_DMA_RX0 ... LPASS_CDC_DMA_RX9:
  

•   case LPASS_CDC_DMA_TX0 ... LPASS_CDC_DMA_TX8:
  

•           drvdata->rxtx_substream[data->dma_ch] = NULL;
  

•           break;
  

•   case LPASS_CDC_DMA_VA_TX0 ... LPASS_CDC_DMA_VA_TX8:
  

•           drvdata->va_substream[data->dma_ch] = NULL;
  

•           break;
  

•   default:
  

•           break;
  

•   }
  

•   if (v->free_dma_channel)
            v->free_dma_channel(drvdata, data->dma_ch, dai_id);
  
  

@@ -209,9 +350,25 @@ static void __lpass_get_lpaif_handle(struct snd_pcm_substream *substream, l_dmactl = drvdata->hdmi_rd_dmactl; l_map = drvdata->hdmiif_map; break;

•   case LPASS_CDC_DMA_RX0 ... LPASS_CDC_DMA_RX9:
  

•           l_id = pcm_data->dma_ch;
  

•           l_dmactl = drvdata->rxtx_rd_dmactl;
  

•           l_map = drvdata->rxtx_lpaif_map;
  

•           break;
  

•   case LPASS_CDC_DMA_TX0 ... LPASS_CDC_DMA_TX8:
  

•           l_id = pcm_data->dma_ch - v->rxtx_wrdma_channel_start;
  

•           l_dmactl = drvdata->rxtx_wr_dmactl;
  

•           l_map = drvdata->rxtx_lpaif_map;
  

•           break;
  

•   case LPASS_CDC_DMA_VA_TX0 ... LPASS_CDC_DMA_VA_TX8:
  

•           l_id = pcm_data->dma_ch - v->va_wrdma_channel_start;
  

•           l_dmactl = drvdata->va_wr_dmactl;
  

•           l_map = drvdata->va_lpaif_map;
  

•           break;
    default:
            break;
    }
  

•   if (dmactl)
            *dmactl = l_dmactl;
    if (id)
  

@@ -299,6 +456,10 @@ static int lpass_platform_pcmops_hw_params(struct snd_soc_component *component, }

            break;

•   case LPASS_CDC_DMA_RX0 ... LPASS_CDC_DMA_RX9:
  

•   case LPASS_CDC_DMA_TX0 ... LPASS_CDC_DMA_TX8:
  

•   case LPASS_CDC_DMA_VA_TX0 ... LPASS_CDC_DMA_VA_TX0:
  

•           break;
    default:
            dev_err(soc_runtime->dev, "%s: invalid  interface: %d\n", __func__, dai_id);
            break;
  

@@ -387,6 +548,9 @@ static int lpass_platform_pcmops_hw_free(struct snd_soc_component *component, struct regmap *map; unsigned int dai_id = cpu_dai->driver->id;

•   if (is_cdc_dma_port(dai_id))
  

•           return 0;
  

•   __lpass_get_lpaif_handle(substream, component, NULL, NULL, &map);
    if (!map) {
            dev_err(soc_runtime->dev, "failed to get dmactl handle\n");
  

@@ -449,6 +613,14 @@ static int lpass_platform_pcmops_prepare(struct snd_soc_component *component, return ret; }

•   if (is_cdc_dma_port(dai_id)) {
  

•           ret = regmap_fields_write(dmactl->fifowm, id, LPAIF_DMACTL_FIFOWM_8);
  

•           if (ret) {
  

•                   dev_err(soc_runtime->dev, "error writing fifowm field to dmactl reg: %d, id: %d\n",
  

•                           ret, id);
  

•                   return ret;
  

•           }
  

•   }
    ret = regmap_fields_write(dmactl->enable, id, LPAIF_DMACTL_ENABLE_ON);
    if (ret) {
            dev_err(soc_runtime->dev, "error writing to rdmactl reg: %d\n",
  

@@ -532,6 +704,35 @@ static int lpass_platform_pcmops_trigger(struct snd_soc_component *component, val_mask = LPAIF_IRQ_ALL(ch); val_irqen = LPAIF_IRQ_ALL(ch); break;

•           case LPASS_CDC_DMA_RX0 ... LPASS_CDC_DMA_RX9:
  

•           case LPASS_CDC_DMA_TX0 ... LPASS_CDC_DMA_TX8:
  

•                   ret = regmap_fields_write(dmactl->dyncclk, id, LPAIF_DMACTL_DYNCLK_ON);
  

•                   if (ret) {
  

•                           dev_err(soc_runtime->dev,
  

•                                   "error writing to rdmactl reg field: %d\n", ret);
  

•                           return ret;
  

•                   }
  

•                   reg_irqclr = LPAIF_RXTX_IRQCLEAR_REG(v, LPAIF_IRQ_PORT_HOST);
  

•                   val_irqclr = LPAIF_IRQ_ALL(ch);
  

•                   reg_irqen = LPAIF_RXTX_IRQEN_REG(v, LPAIF_IRQ_PORT_HOST);
  

•                   val_mask = LPAIF_IRQ_ALL(ch);
  

•                   val_irqen = LPAIF_IRQ_ALL(ch);
  

•                   break;
  

•           case LPASS_CDC_DMA_VA_TX0 ... LPASS_CDC_DMA_VA_TX8:
  

•                   ret = regmap_fields_write(dmactl->dyncclk, id, LPAIF_DMACTL_DYNCLK_ON);
  

•                   if (ret) {
  

•                           dev_err(soc_runtime->dev,
  

•                                   "error writing to rdmactl reg field: %d\n", ret);
  

•                           return ret;
  

•                   }
  

•                   reg_irqclr = LPAIF_VA_IRQCLEAR_REG(v, LPAIF_IRQ_PORT_HOST);
  

•                   val_irqclr = LPAIF_IRQ_ALL(ch);
  

•                   reg_irqen = LPAIF_VA_IRQEN_REG(v, LPAIF_IRQ_PORT_HOST);
  

•                   val_mask = LPAIF_IRQ_ALL(ch);
  

•                   val_irqen = LPAIF_IRQ_ALL(ch);
  

•                   break;
            default:
                    dev_err(soc_runtime->dev, "%s: invalid %d interface\n", __func__, dai_id);
                    return -EINVAL;
  

@@ -583,6 +784,37 @@ static int lpass_platform_pcmops_trigger(struct snd_soc_component *component, val_mask = LPAIF_IRQ_ALL(ch); val_irqen = 0; break;

•           case LPASS_CDC_DMA_RX0 ... LPASS_CDC_DMA_RX9:
  

•           case LPASS_CDC_DMA_TX0 ... LPASS_CDC_DMA_TX8:
  

•                   ret = regmap_fields_write(dmactl->dyncclk, id, LPAIF_DMACTL_DYNCLK_OFF);
  

•                   if (ret) {
  

•                           dev_err(soc_runtime->dev,
  

•                                   "error writing to rdmactl reg field: %d\n", ret);
  

•                           return ret;
  

•                   }
  

•                   reg_irqclr = LPAIF_RXTX_IRQCLEAR_REG(v, LPAIF_IRQ_PORT_HOST);
  

•                   val_irqclr = LPAIF_IRQ_ALL(ch);
  

•                   reg_irqen = LPAIF_RXTX_IRQEN_REG(v, LPAIF_IRQ_PORT_HOST);
  

•                   val_mask = LPAIF_IRQ_ALL(ch);
  

•                   val_irqen = LPAIF_IRQ_ALL(ch);
  

•                   break;
  

•           case LPASS_CDC_DMA_VA_TX0 ... LPASS_CDC_DMA_VA_TX8:
  

•                   ret = regmap_fields_write(dmactl->dyncclk, id, LPAIF_DMACTL_DYNCLK_OFF);
  

•                   if (ret) {
  

•                           dev_err(soc_runtime->dev,
  

•                                   "error writing to rdmactl reg field: %d\n", ret);
  

•                           return ret;
  

•                   }
  

•                   reg_irqclr = LPAIF_VA_IRQCLEAR_REG(v, LPAIF_IRQ_PORT_HOST);
  

•                   val_irqclr = LPAIF_IRQ_ALL(ch);
  

•                   reg_irqen = LPAIF_VA_IRQEN_REG(v, LPAIF_IRQ_PORT_HOST);
  

•                   val_mask = LPAIF_IRQ_ALL(ch);
  

•                   val_irqen = LPAIF_IRQ_ALL(ch);
  

•                   break;
            default:
                    dev_err(soc_runtime->dev, "%s: invalid %d interface\n", __func__, dai_id);
                    return -EINVAL;
  

@@ -642,6 +874,39 @@ static snd_pcm_uframes_t lpass_platform_pcmops_pointer( return bytes_to_frames(substream->runtime, curr_addr - base_addr); }

+static int lpass_platform_cdc_dma_mmap(struct snd_soc_component *component,

'component' is not used.

•                                  struct snd_pcm_substream *substream,
  

•                                  struct vm_area_struct *vma)
  

+{

•   struct snd_pcm_runtime *runtime = substream->runtime;
  

•   unsigned long size, offset;
  

•   vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
  

•   size = vma->vm_end - vma->vm_start;
  

•   offset = vma->vm_pgoff << PAGE_SHIFT;
  

•   return io_remap_pfn_range(vma, vma->vm_start,
  

•                   (runtime->dma_addr + offset) >> PAGE_SHIFT,
  

•                   size, vma->vm_page_prot);
  

+}

+static int lpass_platform_pcmops_mmap(struct snd_soc_component *component,

•                                 struct snd_pcm_substream *substream,
  

•                                 struct vm_area_struct *vma)
  

+{

•   struct snd_soc_pcm_runtime *soc_runtime = asoc_substream_to_rtd(substream);
  

•   struct snd_soc_dai *cpu_dai = asoc_rtd_to_cpu(soc_runtime, 0);
  

•   unsigned int dai_id = cpu_dai->driver->id;
  

•   int err;
  

•   if (is_cdc_dma_port(dai_id))
  

•           err = lpass_platform_cdc_dma_mmap(component, substream, vma);
  

return lpass_platform_cdc_dma_mmap(...)

•   else
  

Drop else

•           err = snd_pcm_lib_default_mmap(substream, vma);
  

return snd_pcm_lib_default_mmap(...)

•   return err;
  

err is no longer required.

+}

static irqreturn_t lpass_dma_interrupt_handler( struct snd_pcm_substream *substream, struct lpass_data *drvdata, @@ -674,6 +939,17 @@ static irqreturn_t lpass_dma_interrupt_handler( reg = LPAIF_IRQCLEAR_REG(v, LPAIF_IRQ_PORT_HOST); val = 0; break;

•   case LPASS_CDC_DMA_RX0 ... LPASS_CDC_DMA_RX9:
  

•   case LPASS_CDC_DMA_TX0 ... LPASS_CDC_DMA_TX8:
  

•           map = drvdata->rxtx_lpaif_map;
  

•           reg = LPAIF_RXTX_IRQCLEAR_REG(v, LPAIF_IRQ_PORT_HOST);
  

•           val = 0;
  

•   break;
  

•   case LPASS_CDC_DMA_VA_TX0 ... LPASS_CDC_DMA_VA_TX8:
  

•           map = drvdata->va_lpaif_map;
  

•           reg = LPAIF_VA_IRQCLEAR_REG(v, LPAIF_IRQ_PORT_HOST);
  

•           val = 0;
  

•   break;
    default:
    dev_err(soc_runtime->dev, "%s: invalid  %d interface\n", __func__, dai_id);
    return -EINVAL;
  

@@ -781,18 +1057,122 @@ static irqreturn_t lpass_platform_hdmiif_irq(int irq, void *data) return rv; } }

•   return IRQ_HANDLED;
  

+}

+static irqreturn_t lpass_platform_rxtxif_irq(int irq, void *data) +{

•   struct lpass_data *drvdata = data;
  

•   struct lpass_variant *v = drvdata->variant;
  

•   unsigned int irqs;
  

•   int rv, chan;
  

Why is 'rv' int vs. irqreturn_t? Is something that isn't an irqreturn_t type being returned from lpass_dma_interrupt_handler()? That's bad. If the regmap read fails in the irq handler we are in pretty bad shape. I'd say that we don't care to handle that case, or at the least, don't care about the return value and can simply get out of this irq handler as fast as possible.

•   rv = regmap_read(drvdata->rxtx_lpaif_map,
  

•                   LPAIF_RXTX_IRQSTAT_REG(v, LPAIF_IRQ_PORT_HOST), &irqs);
  

•   if (rv)
  

•           return IRQ_NONE;
  

•   /* Handle per channel interrupts */
  

•   for (chan = 0; chan < LPASS_MAX_CDC_DMA_CHANNELS; chan++) {
  

•           if (irqs & LPAIF_IRQ_ALL(chan) && drvdata->rxtx_substream[chan]) {
  

•                   rv = lpass_dma_interrupt_handler(
  

•                                           drvdata->rxtx_substream[chan],
  

•                                           drvdata, chan, irqs);
  

•                   if (rv != IRQ_HANDLED)
  

•                           return rv;
  

•           }
  

•   }
  
    return IRQ_HANDLED;
  

}

+static irqreturn_t lpass_platform_vaif_irq(int irq, void *data) +{

•   struct lpass_data *drvdata = data;
  

•   struct lpass_variant *v = drvdata->variant;
  

•   unsigned int irqs;
  

•   int rv, chan;
  

•   rv = regmap_read(drvdata->va_lpaif_map,
  

•                   LPAIF_VA_IRQSTAT_REG(v, LPAIF_IRQ_PORT_HOST), &irqs);
  

•   if (rv)
  

•           return IRQ_NONE;
  

•   /* Handle per channel interrupts */
  

•   for (chan = 0; chan < LPASS_MAX_VA_CDC_DMA_CHANNELS; chan++) {
  

•           if (irqs & LPAIF_IRQ_ALL(chan) && drvdata->va_substream[chan]) {
  

•                   rv = lpass_dma_interrupt_handler(
  

•                                           drvdata->va_substream[chan],
  

•                                           drvdata, chan, irqs);
  

•                   if (rv != IRQ_HANDLED)
  

•                           return rv;
  

•           }
  

•   }
  

•   return IRQ_HANDLED;
  

+}

+static int lpass_platform_prealloc_cdc_dma_buffer(struct snd_soc_component *component,

•                                             struct snd_pcm *pcm, int dai_id)
  

+{

•   struct lpass_data *drvdata = snd_soc_component_get_drvdata(component);
  

•   struct snd_pcm_substream *substream;
  

•   struct snd_dma_buffer *buf;
  

•   int ret;
  

•   if (pcm->streams[SNDRV_PCM_STREAM_PLAYBACK].substream)
  

•           substream = pcm->streams[SNDRV_PCM_STREAM_PLAYBACK].substream;
  

•   else
  

•           substream = pcm->streams[SNDRV_PCM_STREAM_CAPTURE].substream;
  

•   ret = dma_coerce_mask_and_coherent(pcm->card->dev, DMA_BIT_MASK(64));
  

Why isn't the dma mask setup for this device?

•   if (ret)
  

•           return ret;
  

•   buf = &substream->dma_buffer;
  

•   buf->dev.dev = pcm->card->dev;
  

•   buf->private_data = NULL;
  

•   /* Assign Codec DMA buffer pointers */
  

•   buf->dev.type = SNDRV_DMA_TYPE_CONTINUOUS;
  

•   switch (dai_id) {
  

•   case LPASS_CDC_DMA_RX0 ... LPASS_CDC_DMA_RX9:
  

•           buf->bytes = lpass_platform_rxtx_hardware.buffer_bytes_max;
  

•           buf->addr = drvdata->rxtx_cdc_dma_lpm_buf;
  

•           break;
  

•   case LPASS_CDC_DMA_TX0 ... LPASS_CDC_DMA_TX8:
  

•           buf->bytes = lpass_platform_rxtx_hardware.buffer_bytes_max;
  

•           buf->addr = drvdata->rxtx_cdc_dma_lpm_buf + LPASS_RXTX_CDC_DMA_LPM_BUFF_SIZE;
  

•           break;
  

•   case LPASS_CDC_DMA_VA_TX0 ... LPASS_CDC_DMA_VA_TX8:
  

•           buf->bytes = lpass_platform_va_hardware.buffer_bytes_max;
  

•           buf->addr = drvdata->va_cdc_dma_lpm_buf;
  

•           break;
  

•   default:
  

•           break;
  

•   }
  

•   buf->area = (unsigned char * __force)ioremap(buf->addr, buf->bytes);
  

Why aren't we using the DMA mapping framework?

•   return 0;
  

+}

static int lpass_platform_pcm_new(struct snd_soc_component *component, struct snd_soc_pcm_runtime *soc_runtime) { struct snd_pcm *pcm = soc_runtime->pcm;

•   struct snd_soc_dai *cpu_dai = asoc_rtd_to_cpu(soc_runtime, 0);
  

•   unsigned int dai_id = cpu_dai->driver->id;
  

•   size_t size = lpass_platform_pcm_hardware.buffer_bytes_max;
  
  

•   return snd_pcm_set_fixed_buffer_all(pcm, SNDRV_DMA_TYPE_DEV,
  

•                                       component->dev, size);
  

•   /*
  

•    * Lpass codec dma can access only lpass lpm hardware memory.
  

•    * ioremap is for HLOS to access hardware memory.
  

•    */
  

•   if (is_cdc_dma_port(dai_id))
  

•           return lpass_platform_prealloc_cdc_dma_buffer(component, pcm, dai_id);
  

•   else
  

•           return snd_pcm_set_fixed_buffer_all(pcm, SNDRV_DMA_TYPE_DEV,
  

•                                           component->dev, size);
  

else return is an anti-pattern. Please remove the else and deindent the return.

}

static int lpass_platform_pcmops_suspend(struct snd_soc_component *component) @@ -827,6 +1207,31 @@ static int lpass_platform_pcmops_resume(struct snd_soc_component *component) return regcache_sync(map); }

+static int lpass_platform_copy(struct snd_soc_component *component,

•                          struct snd_pcm_substream *substream, int channel,
  

•                          unsigned long pos, void __user *buf, unsigned long bytes)
  

+{

•   struct snd_pcm_runtime *rt = substream->runtime;
  

•   unsigned int dai_id = component->id;
  

•   int ret = 0;
  

•   void __iomem *dma_buf = rt->dma_area + pos +
  

•                           channel * (rt->dma_bytes / rt->channels);
  

•   if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
  

•           if (is_cdc_dma_port(dai_id))
  

•                   ret = copy_from_user_toio(dma_buf, buf, bytes);
  

•           else
  

•                   ret = copy_from_user((void __force *)dma_buf, buf, bytes);
  

•   } else if (substream->stream == SNDRV_PCM_STREAM_CAPTURE) {
  

•           if (is_cdc_dma_port(dai_id))
  

•                   ret = copy_to_user_fromio(buf, dma_buf, bytes);
  

•           else
  

•                   ret = copy_to_user(buf, (void __force *)dma_buf, bytes);
  

Having __force in here highlights the lack of DMA API usage. I guess there's a sound dma wrapper library in sound/core/memalloc.c? Why can't that be used?

•   }
  

•   return ret;
  

+}

static const struct snd_soc_component_driver lpass_component_driver = { .name = DRV_NAME, @@ -837,9 +1242,11 @@ static const struct snd_soc_component_driver lpass_component_driver = { .prepare = lpass_platform_pcmops_prepare, .trigger = lpass_platform_pcmops_trigger, .pointer = lpass_platform_pcmops_pointer,

•   .mmap           = lpass_platform_pcmops_mmap,
    .pcm_construct  = lpass_platform_pcm_new,
    .suspend                = lpass_platform_pcmops_suspend,
    .resume                 = lpass_platform_pcmops_resume,
  

•   .copy_user              = lpass_platform_copy,
  
  

};

@@ -877,6 +1284,60 @@ int asoc_qcom_lpass_platform_register(struct platform_device *pdev) return ret; }

•   if (drvdata->codec_dma_enable) {
  

•           ret = regmap_write(drvdata->rxtx_lpaif_map,
  

•                   LPAIF_RXTX_IRQEN_REG(v, LPAIF_IRQ_PORT_HOST), 0x0);
  

•           if (ret) {
  

•                   dev_err(&pdev->dev, "error writing to rxtx irqen reg: %d\n", ret);
  

•                   return ret;
  

•           }
  

•           ret = regmap_write(drvdata->va_lpaif_map,
  

•                   LPAIF_VA_IRQEN_REG(v, LPAIF_IRQ_PORT_HOST), 0x0);
  

•           if (ret) {
  

•                   dev_err(&pdev->dev, "error writing to rxtx irqen reg: %d\n", ret);
  

•                   return ret;
  

•           }
  

•           drvdata->rxtxif_irq = platform_get_irq_byname(pdev, "lpass-irq-rxtxif");
  

•           if (drvdata->rxtxif_irq < 0)
  

•                   return -ENODEV;
  

•           ret = devm_request_irq(&pdev->dev, drvdata->rxtxif_irq,
  

•                           lpass_platform_rxtxif_irq, IRQF_TRIGGER_RISING,
  

Drop flags and get it from firmware please.

•                           "lpass-irq-rxtxif", drvdata);
  

•           if (ret) {
  

•                   dev_err(&pdev->dev, "rxtx irq request failed: %d\n", ret);
  

•                   return ret;
  

•           }
  

•           ret = lpass_platform_alloc_rxtx_dmactl_fields(&pdev->dev,
  

•                                            drvdata->rxtx_lpaif_map);
  

•           if (ret) {
  

•                   dev_err(&pdev->dev,
  

•                           "error initializing rxtx dmactl fields: %d\n", ret);
  

•                   return ret;
  

•           }
  

•           drvdata->vaif_irq = platform_get_irq_byname(pdev, "lpass-irq-vaif");
  

•           if (drvdata->vaif_irq < 0)
  

•                   return -ENODEV;
  

•           ret = devm_request_irq(&pdev->dev, drvdata->vaif_irq,
  

•                           lpass_platform_vaif_irq, IRQF_TRIGGER_RISING,
  

Drop flags and get it from firmware please.

•                           "lpass-irq-vaif", drvdata);
  

•           if (ret) {
  

•                   dev_err(&pdev->dev, "va irq request failed: %d\n", ret);
  

•                   return ret;
  

•           }
  

•           ret = lpass_platform_alloc_va_dmactl_fields(&pdev->dev,
  

•                                            drvdata->va_lpaif_map);
  

•           if (ret) {
  

•                   dev_err(&pdev->dev,
  

•                           "error initializing va dmactl fields: %d\n", ret);
  

•                   return ret;
  

•           }
  

•   }
  

•   if (drvdata->hdmi_port_enable) {
            drvdata->hdmiif_irq = platform_get_irq_byname(pdev, "lpass-irq-hdmi");
            if (drvdata->hdmiif_irq < 0)
  

-- 2.7.4

Quoting Srinivasa Rao Mandadapu (2022-02-15 22:53:11)

On 2/15/2022 6:57 AM, Stephen Boyd wrote: Thanks for your time and valuable review comments Stephen!!!

...

Quoting Srinivasa Rao Mandadapu (2022-02-14 06:58:25)

...

diff --git a/sound/soc/qcom/lpass-platform.c b/sound/soc/qcom/lpass-platform.c index 5d77240..12b8d40 100644 --- a/sound/soc/qcom/lpass-platform.c +++ b/sound/soc/qcom/lpass-platform.c

[...]

...

...

•   if (ret)
  

•           return ret;
  

•   buf = &substream->dma_buffer;
  

•   buf->dev.dev = pcm->card->dev;
  

•   buf->private_data = NULL;
  

•   /* Assign Codec DMA buffer pointers */
  

•   buf->dev.type = SNDRV_DMA_TYPE_CONTINUOUS;
  

•   switch (dai_id) {
  

•   case LPASS_CDC_DMA_RX0 ... LPASS_CDC_DMA_RX9:
  

•           buf->bytes = lpass_platform_rxtx_hardware.buffer_bytes_max;
  

•           buf->addr = drvdata->rxtx_cdc_dma_lpm_buf;
  

•           break;
  

•   case LPASS_CDC_DMA_TX0 ... LPASS_CDC_DMA_TX8:
  

•           buf->bytes = lpass_platform_rxtx_hardware.buffer_bytes_max;
  

•           buf->addr = drvdata->rxtx_cdc_dma_lpm_buf + LPASS_RXTX_CDC_DMA_LPM_BUFF_SIZE;
  

•           break;
  

•   case LPASS_CDC_DMA_VA_TX0 ... LPASS_CDC_DMA_VA_TX8:
  

•           buf->bytes = lpass_platform_va_hardware.buffer_bytes_max;
  

•           buf->addr = drvdata->va_cdc_dma_lpm_buf;
  

•           break;
  

•   default:
  

•           break;
  

•   }
  

•   buf->area = (unsigned char * __force)ioremap(buf->addr, buf->bytes);
  

Why aren't we using the DMA mapping framework?

Here, Need to use hardware memory, that is LPASS LPM region for codec DMA.

It does not look like iomem, so the usage of ioremap() is wrong. I understand that it is some place inside the audio subsystem used to DMA. ioremap() memory should be accessed through the io accessors, readl/writel, ioread/iowrite.

...

...

@@ -827,6 +1207,31 @@ static int lpass_platform_pcmops_resume(struct snd_soc_component *component) return regcache_sync(map); }

+static int lpass_platform_copy(struct snd_soc_component *component,

•                          struct snd_pcm_substream *substream, int channel,
  

•                          unsigned long pos, void __user *buf, unsigned long bytes)
  

+{

•   struct snd_pcm_runtime *rt = substream->runtime;
  

•   unsigned int dai_id = component->id;
  

•   int ret = 0;
  

•   void __iomem *dma_buf = rt->dma_area + pos +
  

•                           channel * (rt->dma_bytes / rt->channels);
  

•   if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
  

•           if (is_cdc_dma_port(dai_id))
  

•                   ret = copy_from_user_toio(dma_buf, buf, bytes);
  

•           else
  

•                   ret = copy_from_user((void __force *)dma_buf, buf, bytes);
  

•   } else if (substream->stream == SNDRV_PCM_STREAM_CAPTURE) {
  

•           if (is_cdc_dma_port(dai_id))
  

•                   ret = copy_to_user_fromio(buf, dma_buf, bytes);
  

•           else
  

•                   ret = copy_to_user(buf, (void __force *)dma_buf, bytes);
  

Having __force in here highlights the lack of DMA API usage. I guess there's a sound dma wrapper library in sound/core/memalloc.c? Why can't that be used?

Didn't see any memcopy wrapper functions in memalloc.c. Could You please elaborate or share some example.

Can you add some memcpy wrappers to memalloc.c? Or implement the copy wrapper you need?

...

...

•   }
  

•   return ret;
  

+}

static const struct snd_soc_component_driver lpass_component_driver = { .name = DRV_NAME, @@ -837,9 +1242,11 @@ static const struct snd_soc_component_driver lpass_component_driver = { .prepare = lpass_platform_pcmops_prepare, .trigger = lpass_platform_pcmops_trigger, .pointer = lpass_platform_pcmops_pointer,

•   .mmap           = lpass_platform_pcmops_mmap,
     .pcm_construct  = lpass_platform_pcm_new,
     .suspend                = lpass_platform_pcmops_suspend,
     .resume                 = lpass_platform_pcmops_resume,
  

•   .copy_user              = lpass_platform_copy,
  
  

  };

@@ -877,6 +1284,60 @@ int asoc_qcom_lpass_platform_register(struct platform_device *pdev) return ret; }

•   if (drvdata->codec_dma_enable) {
  

•           ret = regmap_write(drvdata->rxtx_lpaif_map,
  

•                   LPAIF_RXTX_IRQEN_REG(v, LPAIF_IRQ_PORT_HOST), 0x0);
  

•           if (ret) {
  

•                   dev_err(&pdev->dev, "error writing to rxtx irqen reg: %d\n", ret);
  

•                   return ret;
  

•           }
  

•           ret = regmap_write(drvdata->va_lpaif_map,
  

•                   LPAIF_VA_IRQEN_REG(v, LPAIF_IRQ_PORT_HOST), 0x0);
  

•           if (ret) {
  

•                   dev_err(&pdev->dev, "error writing to rxtx irqen reg: %d\n", ret);
  

•                   return ret;
  

•           }
  

•           drvdata->rxtxif_irq = platform_get_irq_byname(pdev, "lpass-irq-rxtxif");
  

•           if (drvdata->rxtxif_irq < 0)
  

•                   return -ENODEV;
  

•           ret = devm_request_irq(&pdev->dev, drvdata->rxtxif_irq,
  

•                           lpass_platform_rxtxif_irq, IRQF_TRIGGER_RISING,
  

Drop flags and get it from firmware please.

Same is followed in existing for other i2s and HDMI interrupts. Could You please give some example if it's really matters?

It matters in the case that the hardware team decides to change the pin to falling. DT already has the flags encoded, so having a zero here avoids conflicting with what DT has set and also alleviates us from having to set different flags on different devices. Everyone wins. Look around for drivers that pass 0 in place of IRQF_TRIGGER_RISING, there are many examples.

Hi Srinivasa,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on broonie-sound/for-next] [also build test WARNING on v5.17-rc4 next-20220215] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch]

url: https://github.com/0day-ci/linux/commits/Srinivasa-Rao-Mandadapu/Add-support... base: https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next config: csky-randconfig-s032-20220216 (https://download.01.org/0day-ci/archive/20220216/202202161407.5MOObZwm-lkp@i...) compiler: csky-linux-gcc (GCC) 11.2.0 reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # apt-get install sparse # sparse version: v0.6.4-dirty # https://github.com/0day-ci/linux/commit/e81c7e5d842d2b8039700a71557683e88ce0... git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Srinivasa-Rao-Mandadapu/Add-support-for-audio-on-SC7280-based-targets/20220214-230256 git checkout e81c7e5d842d2b8039700a71557683e88ce0162d # save the config file to linux build tree mkdir build_dir COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=csky SHELL=/bin/bash sound/soc/qcom/

If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot lkp@intel.com

sparse warnings: (new ones prefixed by >>)

...

sound/soc/qcom/lpass-platform.c:1218:52: sparse: sparse: incorrect type in initializer (different address spaces) @@ expected void [noderef] __iomem *dma_buf @@ got unsigned char * @@

sound/soc/qcom/lpass-platform.c:1218:52: sparse: expected void [noderef] __iomem *dma_buf sound/soc/qcom/lpass-platform.c:1218:52: sparse: got unsigned char *

vim +1218 sound/soc/qcom/lpass-platform.c

1209 1210 static int lpass_platform_copy(struct snd_soc_component *component, 1211 struct snd_pcm_substream *substream, int channel, 1212 unsigned long pos, void __user *buf, unsigned long bytes) 1213 { 1214 struct snd_pcm_runtime *rt = substream->runtime; 1215 unsigned int dai_id = component->id; 1216 int ret = 0; 1217

1218 void __iomem *dma_buf = rt->dma_area + pos +

1219 channel * (rt->dma_bytes / rt->channels); 1220 1221 if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) { 1222 if (is_cdc_dma_port(dai_id)) 1223 ret = copy_from_user_toio(dma_buf, buf, bytes); 1224 else 1225 ret = copy_from_user((void __force *)dma_buf, buf, bytes); 1226 } else if (substream->stream == SNDRV_PCM_STREAM_CAPTURE) { 1227 if (is_cdc_dma_port(dai_id)) 1228 ret = copy_to_user_fromio(buf, dma_buf, bytes); 1229 else 1230 ret = copy_to_user(buf, (void __force *)dma_buf, bytes); 1231 } 1232 1233 return ret; 1234 } 1235

--- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

On 2/15/2022 7:03 AM, Stephen Boyd wrote: Thanks for your time Stephen!!!

Quoting Srinivasa Rao Mandadapu (2022-02-14 06:58:26)

...

diff --git a/sound/soc/qcom/lpass-cdc-dma.c b/sound/soc/qcom/lpass-cdc-dma.c new file mode 100644 index 0000000..4a50baa --- /dev/null +++ b/sound/soc/qcom/lpass-cdc-dma.c @@ -0,0 +1,304 @@ +// SPDX-License-Identifier: GPL-2.0-only +/*

• • Copyright (c) 2021 The Linux Foundation. All rights reserved.
• • lpass-cdc-dma.c -- ALSA SoC CDC DMA CPU DAI driver for QTi LPASS
• */

+#include <linux/clk.h> +#include <linux/module.h>

export.h for EXPORT_SYMBOL usage.

...

+#include <sound/soc.h> +#include <sound/soc-dai.h>

+#include "lpass-lpaif-reg.h" +#include "lpass.h"

+#define CODEC_MEM_FREQ_NORMAL 153600000

Is this in Hz? CODEC_MEM_HZ_NORMAL?

Okay. Will change accordingly.

...

+enum codec_dma_interfaces {

•   LPASS_CDC_DMA_INTERFACE1 = 1,
  

•   LPASS_CDC_DMA_INTERFACE2,
  

•   LPASS_CDC_DMA_INTERFACE3,
  

•   LPASS_CDC_DMA_INTERFACE4,
  

•   LPASS_CDC_DMA_INTERFACE5,
  

•   LPASS_CDC_DMA_INTERFACE6,
  

•   LPASS_CDC_DMA_INTERFACE7,
  

•   LPASS_CDC_DMA_INTERFACE8,
  

•   LPASS_CDC_DMA_INTERFACE9,
  

•   LPASS_CDC_DMA_INTERFACE10,
  

+};

+static void __lpass_get_dmactl_handle(struct snd_pcm_substream *substream, struct snd_soc_dai *dai,

•                                 struct lpaif_dmactl **dmactl, int *id)
  

+{

•   struct snd_soc_pcm_runtime *soc_runtime = asoc_substream_to_rtd(substream);
  

•   struct snd_soc_dai *cpu_dai = asoc_rtd_to_cpu(soc_runtime, 0);
  

•   struct lpass_data *drvdata = snd_soc_dai_get_drvdata(dai);
  

•   struct snd_pcm_runtime *rt = substream->runtime;
  

•   struct lpass_pcm_data *pcm_data = rt->private_data;
  

•   struct lpass_variant *v = drvdata->variant;
  

•   unsigned int dai_id = cpu_dai->driver->id;
  

•   switch (dai_id) {
  

•   case LPASS_CDC_DMA_RX0 ... LPASS_CDC_DMA_RX9:
  

•           *dmactl = drvdata->rxtx_rd_dmactl;
  

•           *id = pcm_data->dma_ch;
  

•           break;
  

•   case LPASS_CDC_DMA_TX0 ... LPASS_CDC_DMA_TX8:
  

•           *dmactl = drvdata->rxtx_wr_dmactl;
  

•           *id = pcm_data->dma_ch - v->rxtx_wrdma_channel_start;
  

•           break;
  

•   case LPASS_CDC_DMA_VA_TX0 ... LPASS_CDC_DMA_VA_TX8:
  

•           *dmactl = drvdata->va_wr_dmactl;
  

•           *id = pcm_data->dma_ch - v->va_wrdma_channel_start;
  

•           break;
  

•   default:
  

•           dev_err(soc_runtime->dev, "invalid dai id for dma ctl: %d\n", dai_id);
  

•           break;
  

•   }
  

+}

+static int __lpass_get_codec_dma_intf_type(int dai_id) +{

•   int ret;
  

•   switch (dai_id) {
  

•   case LPASS_CDC_DMA_RX0:
  

•   case LPASS_CDC_DMA_TX0:
  

•   case LPASS_CDC_DMA_VA_TX0:
  

•           ret = LPASS_CDC_DMA_INTERFACE1;
  

•           break;
  

•   case LPASS_CDC_DMA_RX1:
  

•   case LPASS_CDC_DMA_TX1:
  

•   case LPASS_CDC_DMA_VA_TX1:
  

•           ret = LPASS_CDC_DMA_INTERFACE2;
  

•           break;
  

•   case LPASS_CDC_DMA_RX2:
  

•   case LPASS_CDC_DMA_TX2:
  

•   case LPASS_CDC_DMA_VA_TX2:
  

•           ret = LPASS_CDC_DMA_INTERFACE3;
  

•           break;
  

•   case LPASS_CDC_DMA_RX3:
  

•   case LPASS_CDC_DMA_TX3:
  

•   case LPASS_CDC_DMA_VA_TX3:
  

•           ret = LPASS_CDC_DMA_INTERFACE4;
  

•           break;
  

•   case LPASS_CDC_DMA_RX4:
  

•   case LPASS_CDC_DMA_TX4:
  

•   case LPASS_CDC_DMA_VA_TX4:
  

•           ret = LPASS_CDC_DMA_INTERFACE5;
  

•           break;
  

•   case LPASS_CDC_DMA_RX5:
  

•   case LPASS_CDC_DMA_TX5:
  

•   case LPASS_CDC_DMA_VA_TX5:
  

•           ret = LPASS_CDC_DMA_INTERFACE6;
  

•           break;
  

•   case LPASS_CDC_DMA_RX6:
  

•   case LPASS_CDC_DMA_TX6:
  

•   case LPASS_CDC_DMA_VA_TX6:
  

•           ret = LPASS_CDC_DMA_INTERFACE7;
  

•           break;
  

•   case LPASS_CDC_DMA_RX7:
  

•   case LPASS_CDC_DMA_TX7:
  

•   case LPASS_CDC_DMA_VA_TX7:
  

•           ret = LPASS_CDC_DMA_INTERFACE8;
  

•           break;
  

•   case LPASS_CDC_DMA_RX8:
  

•   case LPASS_CDC_DMA_TX8:
  

•   case LPASS_CDC_DMA_VA_TX8:
  

•           ret = LPASS_CDC_DMA_INTERFACE9;
  

•           break;
  

•   case LPASS_CDC_DMA_RX9:
  

•           ret  = LPASS_CDC_DMA_INTERFACE10;
  

•           break;
  

•   default:
  

•           ret = -EINVAL;
  

•           break;
  

•   }
  

•   return ret;
  

+}

+static int __lpass_platform_codec_intf_init(struct snd_soc_dai *dai,

•                                       struct snd_pcm_substream *substream)
  

+{

•   struct snd_soc_pcm_runtime *soc_runtime = asoc_substream_to_rtd(substream);
  

•   struct snd_soc_dai *cpu_dai = asoc_rtd_to_cpu(soc_runtime, 0);
  

•   struct lpaif_dmactl *dmactl = NULL;
  

•   struct device *dev = soc_runtime->dev;
  

•   int ret, id, codec_intf;
  

•   unsigned int dai_id = cpu_dai->driver->id;
  

•   codec_intf = __lpass_get_codec_dma_intf_type(dai_id);
  

•   if (codec_intf < 0) {
  

•           dev_err(dev, "failed to get codec_intf: %d\n", codec_intf);
  

•           return codec_intf;
  

•   }
  

•   __lpass_get_dmactl_handle(substream, dai, &dmactl, &id);
  

•   if (!dmactl) {
  

•           dev_err(dev, "failed to get dmactl handle for dai_id: %d\n", dai_id);
  

•           return -EINVAL;
  

•   }
  

•   ret = regmap_fields_write(dmactl->codec_intf, id, codec_intf);
  

•   if (ret) {
  

•           dev_err(dev, "error writing to dmactl codec_intf reg field: %d\n", ret);
  

•           return ret;
  

•   }
  

•   ret = regmap_fields_write(dmactl->codec_fs_sel, id, 0x0);
  

•   if (ret) {
  

•           dev_err(dev, "error writing to dmactl codec_fs_sel reg field: %d\n", ret);
  

•           return ret;
  

•   }
  

•   ret = regmap_fields_write(dmactl->codec_fs_delay, id, 0x0);
  

•   if (ret) {
  

•           dev_err(dev, "error writing to dmactl codec_fs_delay reg field: %d\n", ret);
  

•           return ret;
  

•   }
  

•   ret = regmap_fields_write(dmactl->codec_pack, id, 0x1);
  

•   if (ret) {
  

•           dev_err(dev, "error writing to dmactl codec_pack reg field: %d\n", ret);
  

•           return ret;
  

•   }
  

•   ret = regmap_fields_write(dmactl->codec_enable, id, LPAIF_DMACTL_ENABLE_ON);
  

•   if (ret) {
  

•           dev_err(dev, "error writing to dmactl codec_enable reg field: %d\n", ret);
  

•           return ret;
  

•   }
  

•   return 0;
  

+}

+static int lpass_cdc_dma_daiops_startup(struct snd_pcm_substream *substream,

•                               struct snd_soc_dai *dai)
  

+{

•   struct lpass_data *drvdata = snd_soc_dai_get_drvdata(dai);
  

•   struct snd_soc_pcm_runtime *soc_runtime = asoc_substream_to_rtd(substream);
  

•   switch (dai->id) {
  

•   case LPASS_CDC_DMA_RX0 ... LPASS_CDC_DMA_RX9:
  

•   case LPASS_CDC_DMA_TX0 ... LPASS_CDC_DMA_TX8:
  

•           clk_set_rate(drvdata->cdc_clks[2].clk, CODEC_MEM_FREQ_NORMAL);
  

•           clk_prepare_enable(drvdata->cdc_clks[2].clk);
  

Where do '2' and '5' come from? Why are they part of the bulk clk array? Why not get them separately and give them real named pointer values?

Okay. will change to real named pointers.

...

•           break;
  

•   case LPASS_CDC_DMA_VA_TX0 ... LPASS_CDC_DMA_VA_TX0:
  

•           clk_set_rate(drvdata->cdc_clks[5].clk, CODEC_MEM_FREQ_NORMAL);
  

•           clk_prepare_enable(drvdata->cdc_clks[5].clk);
  

•           break;
  

•   default:
  

•           dev_err(soc_runtime->dev, "%s: invalid  interface: %d\n", __func__, dai->id);
  

•           break;
  

•   }
  

•   return 0;
  

+}

+static void lpass_cdc_dma_daiops_shutdown(struct snd_pcm_substream *substream,

•                                 struct snd_soc_dai *dai)
  

+{

•   struct lpass_data *drvdata = snd_soc_dai_get_drvdata(dai);
  

•   struct snd_soc_pcm_runtime *soc_runtime = asoc_substream_to_rtd(substream);
  

•   switch (dai->id) {
  

•   case LPASS_CDC_DMA_RX0 ... LPASS_CDC_DMA_RX9:
  

•   case LPASS_CDC_DMA_TX0 ... LPASS_CDC_DMA_TX8:
  

•           clk_disable_unprepare(drvdata->cdc_clks[2].clk);
  

•           break;
  

•   case LPASS_CDC_DMA_VA_TX0 ... LPASS_CDC_DMA_VA_TX0:
  

•           clk_disable_unprepare(drvdata->cdc_clks[5].clk);
  

•           break;
  

•   default:
  

•           dev_err(soc_runtime->dev, "%s: invalid  interface: %d\n", __func__, dai->id);
  

•           break;
  

•   }
  

+}

+static int lpass_cdc_dma_daiops_hw_params(struct snd_pcm_substream *substream,

•                                 struct snd_pcm_hw_params *params,
  

•                                 struct snd_soc_dai *dai)
  

+{

•   struct snd_soc_pcm_runtime *soc_runtime = asoc_substream_to_rtd(substream);
  

•   struct lpaif_dmactl *dmactl = NULL;
  

•   unsigned int ret, regval;
  

•   unsigned int channels = params_channels(params);
  

•   int id;
  

•   switch (channels) {
  

•   case 1:
  

•           regval = LPASS_CDC_DMA_INTF_ONE_CHANNEL;
  

•           break;
  

•   case 2:
  

•           regval = LPASS_CDC_DMA_INTF_TWO_CHANNEL;
  

•           break;
  

•   case 4:
  

•           regval = LPASS_CDC_DMA_INTF_FOUR_CHANNEL;
  

•           break;
  

•   case 6:
  

•           regval = LPASS_CDC_DMA_INTF_SIX_CHANNEL;
  

•           break;
  

•   case 8:
  

•           regval = LPASS_CDC_DMA_INTF_EIGHT_CHANNEL;
  

•           break;
  

•   default:
  

•           dev_err(soc_runtime->dev, "invalid PCM config\n");
  

•           return -EINVAL;
  

•   }
  

•   __lpass_get_dmactl_handle(substream, dai, &dmactl, &id);
  

•   if (!dmactl) {
  

•           dev_err(soc_runtime->dev, "failed to get dmactl handle\n");
  

•           return -EINVAL;
  

•   }
  

•   ret = regmap_fields_write(dmactl->codec_channel, id, regval);
  

•   if (ret) {
  

•           dev_err(soc_runtime->dev,
  

•                   "error writing to dmactl codec_channel reg field: %d\n", ret);
  

•           return ret;
  

•   }
  

•   return 0;
  

+}

+static int lpass_cdc_dma_daiops_trigger(struct snd_pcm_substream *substream,

•                               int cmd, struct snd_soc_dai *dai)
  

+{

•   struct snd_soc_pcm_runtime *soc_runtime = asoc_substream_to_rtd(substream);
  

•   struct lpaif_dmactl *dmactl;
  

•   int ret = 0, id;
  

•   switch (cmd) {
  

•   case SNDRV_PCM_TRIGGER_START:
  

•   case SNDRV_PCM_TRIGGER_RESUME:
  

•   case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
  

•           __lpass_platform_codec_intf_init(dai, substream);
  

•           break;
  

•   case SNDRV_PCM_TRIGGER_STOP:
  

•   case SNDRV_PCM_TRIGGER_SUSPEND:
  

•   case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
  

•           __lpass_get_dmactl_handle(substream, dai, &dmactl, &id);
  

•           if (!dmactl) {
  

•                   dev_err(soc_runtime->dev, "failed to get dmactl handle\n");
  

This same message is in many places. I really hope it never gets printed because finding out which line it got printed at is going to be impossible.