Re: [alsa-devel] [PATCH 2/4] ASoC sst: Add mid platform driver

...

• case SNDRV_PCM_TRIGGER_START:

• pr_debug("sst: Trigger Start\n");
  

• ret_val = stream->sstdrv_ops->control_set(
  

• 				SST_SND_START, &str_id);
  

• if (ret_val)
  

• 	return ret_val;
  

• stream->stream_status = RUNNING;
  

• stream->stream_info.mad_substream = substream;
  

• break;
  

You've no locking in the rest of the driver protecting the variables you're updating here.

Will fix this

...

• stream = substream->runtime->private_data;
• if (stream->stream_status == INIT)

• return 0;
  

• str_info = &stream->stream_info;
• ret_val = stream->sstdrv_ops->control_set(

• 		SST_SND_BUFFER_POINTER, &str_info);
  

• if (ret_val) {

• pr_err("sst: error code = 0x%x\n", ret_val);
  

• return ret_val;
  

• }
• return stream->stream_info.buffer_ptr;

I suspect you need to return bytes_to_frames() of this...

No as the DSP FW returns the frame value for the pointer

...

• pr_debug("intelmid_platform_probe called\n");
• ret = snd_soc_register_platform(&pdev->dev, &intelmid_soc_platform_drv);
• if (ret) {

• pr_err("registering soc platform failed\n");
  

• return ret;
  

• }
• return snd_soc_register_dais(&pdev->dev,

• 		mfld_dai, ARRAY_SIZE(mfld_dai));
  

You're not unwinding the platform register if the DAI registration fail.s

Will fix

...

+MODULE_LICENSE("GPL v2"); +MODULE_ALIAS("mid-platform");

Same issue as with the CODEC driver here.

Okay

...

+int snd_intelmad_alloc_stream(struct snd_pcm_substream *substream); +int snd_intelmad_init_stream(struct snd_pcm_substream *substream);

Why are these exposed here?

These function are implemented in platform_lib.c and called in mid_platform.c

...

+enum mid_drv_status {

• INIT = 1,
• STARTED,
• RUNNING,
• PAUSED,
• DROPPED,

+};

Namespacing.

Will fix

~Vinod