On 7/20/21 11:57 PM, Pierre-Louis Bossart wrote: > >> +static int acp5x_dma_open(struct snd_soc_component *component, >> + struct snd_pcm_substream *substream) >> +{ >> + struct snd_pcm_runtime *runtime; >> + struct snd_soc_pcm_runtime *prtd; >> + struct i2s_dev_data *adata; >> + struct i2s_stream_instance *i2s_data; >> + int ret; >> + >> + runtime = substream->runtime; >> + prtd = asoc_substream_to_rtd(substream); >> + component = snd_soc_rtdcom_lookup(prtd, DRV_NAME); >> + adata = dev_get_drvdata(component->dev); >> + >> + i2s_data = kzalloc(sizeof(*i2s_data), GFP_KERNEL); >> + if (!i2s_data) >> + return -EINVAL; > > return -ENOMEM? Will fix it. > >> + >> + if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) >> + runtime->hw = acp5x_pcm_hardware_playback; >> + else >> + runtime->hw = acp5x_pcm_hardware_capture; >> + >> + ret = snd_pcm_hw_constraint_integer(runtime, >> + SNDRV_PCM_HW_PARAM_PERIODS); >> + if (ret < 0) { >> + dev_err(component->dev, "set integer constraint failed\n"); >> + kfree(i2s_data); >> + return ret; >> + } >> + i2s_data->acp5x_base = adata->acp5x_base; >> + runtime->private_data = i2s_data; >> + return ret; >> +} >> + >> +static int acp5x_dma_hw_params(struct snd_soc_component *component, >> + struct snd_pcm_substream *substream, >> + struct snd_pcm_hw_params *params) >> +{ >> + struct i2s_stream_instance *rtd; >> + struct snd_soc_pcm_runtime *prtd; >> + struct snd_soc_card *card; >> + struct acp5x_platform_info *pinfo; >> + struct i2s_dev_data *adata; >> + u64 size; >> + >> + prtd = asoc_substream_to_rtd(substream); >> + card = prtd->card; >> + pinfo = snd_soc_card_get_drvdata(card); >> + adata = dev_get_drvdata(component->dev); >> + rtd = substream->runtime->private_data; >> + >> + if (!rtd) >> + return -EINVAL; >> + >> + if (pinfo) { >> + if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) { >> + rtd->i2s_instance = pinfo->play_i2s_instance; >> + switch (rtd->i2s_instance) { >> + case I2S_HS_INSTANCE: >> + adata->play_stream = substream; >> + break; >> + case I2S_SP_INSTANCE: >> + default: >> + adata->i2ssp_play_stream = substream; >> + } >> + } else { >> + rtd->i2s_instance = pinfo->cap_i2s_instance; >> + switch (rtd->i2s_instance) { >> + case I2S_HS_INSTANCE: >> + adata->capture_stream = substream; >> + break; >> + case I2S_SP_INSTANCE: >> + default: >> + adata->i2ssp_capture_stream = substream; >> + } >> + } >> + } else { >> + dev_err(component->dev, "pinfo failed\n"); > > should you bail here with e.g. return -EINVAL? Will fix it and post the new version. > >> + } >> + size = params_buffer_bytes(params); >> + rtd->dma_addr = substream->dma_buffer.addr; >> + rtd->num_pages = (PAGE_ALIGN(size) >> PAGE_SHIFT); >> + config_acp5x_dma(rtd, substream->stream); >> + return 0; >> +} >