Re: [PATCH v2 10/16] ASoC: qcom: audioreach: add bedai support

+static int q6dma_hw_params(struct snd_pcm_substream *substream,

• 	   struct snd_pcm_hw_params *params,
  

• 	   struct snd_soc_dai *dai)
  

+{

• struct q6apm_bedai_data *dai_data = dev_get_drvdata(dai->dev);
• struct q6apm_cdc_dma_cfg *cfg = &dai_data->port_config[dai->id].dma_cfg;
• cfg->bit_width = params_width(params);
• cfg->sample_rate = params_rate(params);
• cfg->num_channels = params_channels(params);
• switch (params_format(params)) {
• case SNDRV_PCM_FORMAT_S16_LE:

• dai_data->bits_per_sample[dai->id] = 16;
  

• break;
  

• case SNDRV_PCM_FORMAT_S24_LE:

• dai_data->bits_per_sample[dai->id] = 24;
  

• break;
  

default: return -EINVAL;

?

• }
• return 0;

+}

+static void q6apm_bedai_shutdown(struct snd_pcm_substream *substream,

• 		struct snd_soc_dai *dai)
  

+{

• struct q6apm_bedai_data *dai_data = dev_get_drvdata(dai->dev);
• int rc;
• if (!dai_data->is_port_started[dai->id])

• return;
  

• rc = q6apm_graph_stop(dai_data->graph[dai->id]);
• if (rc < 0)

• dev_err(dai->dev, "fail to close APM port (%d)\n", rc);
  

• q6apm_graph_close(dai_data->graph[dai->id]);
• dai_data->is_port_started[dai->id] = false;

difficult to follow, this mixes '!start', 'stop', 'close'

+}

+static int q6apm_bedai_prepare(struct snd_pcm_substream *substream,

• 	       struct snd_soc_dai *dai)
  

+{

• struct q6apm_bedai_data *dai_data = dev_get_drvdata(dai->dev);
• struct q6apm_cdc_dma_cfg *cfg = &dai_data->port_config[dai->id].dma_cfg;
• int graph_id = dai->id;
• int rc;
• int ret;
• struct q6apm_graph *graph;
• if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {

• graph = q6apm_graph_open(dai->dev, NULL, dai->dev, graph_id);
  

• if (IS_ERR(graph)) {
  

• 	dev_err(dai->dev, "Failed to open graph (%d)\n",
  

• 		graph_id);
  

• 	ret = PTR_ERR(graph);
  

• 	return ret;
  

• }
  

• dai_data->graph[graph_id] = graph;
  

• }
• rc = q6apm_graph_media_format_pcm(dai_data->graph[dai->id],

• 			  substream->stream, cfg->sample_rate,
  

• 			  cfg->num_channels, NULL, cfg->bit_width);
  

• rc = q6apm_graph_prepare(dai_data->graph[dai->id]);

any good static analyzer would tell you you didn't test rc before overriding it..

this looks rather questionable.

• rc = q6apm_graph_start(dai_data->graph[dai->id]);
• if (rc < 0) {

• dev_err(dai->dev, "fail to start APM port %x\n", dai->id);
  

• return rc;
  

• }
• dai_data->is_port_started[dai->id] = true;
• return 0;

+}