Re: [PATCH 5/9] ASoC: ipq806x: Add I2S PCM platform driver

On Wed, Nov 19, 2014 at 10:52:45AM -0800, Kenneth Westfield wrote:

+	.periods_min		=	LPASS_MI2S_NO_OF_PERIODS,
+	.periods_max		=	LPASS_MI2S_NO_OF_PERIODS,

As Pierre said this is really odd - it appears to be just a single
possible value.

+static irqreturn_t lpass_pcm_mi2s_irq(int intrsrc, void *data)
+{
+	int dma_ch;
+	uint32_t ret = IRQ_NONE;
+	uint32_t has_xrun, pending;
+
+	struct snd_pcm_substream *substream = data;
+	struct snd_pcm_runtime *runtime = substream->runtime;
+	struct lpass_runtime_data_t *prtd = runtime->private_data;

Here we rely on runtime.

+
+	if (prtd) {
+		dma_ch = prtd->lpaif_info.dma_ch;
+	} else {
+		pr_debug("%s: received interrupt w/o runtime\n", __func__);
+		return IRQ_NONE;
+	}

Here we print (as a debug message, not a dev_err()) an error saying we
lack a runtime (actually it's private data).

+	if (unlikely(has_xrun) && substream->runtime &&
+			snd_pcm_running(substream)) {

Here we check if runtime (which we already dereferenced) is non-NULL.

+	if (pending & LPAIF_UNDER_CH(dma_ch)) {
+		snd_pcm_period_elapsed(substream);
+		pr_debug("%s: xrun warning\n", __func__);
+		ret = IRQ_HANDLED;
+	}
+
+	if (pending & LPAIF_ERR_CH(dma_ch)) {
+		pr_debug("%s: Bus access warning\n", __func__);
+		ret = IRQ_HANDLED;
+	}

These errors should be logged as such.

+static int lpass_pcm_mi2s_hw_params(struct snd_pcm_substream *substream,
+					struct snd_pcm_hw_params *params)
+{
+	struct snd_pcm_runtime *runtime = substream->runtime;
+	struct lpass_runtime_data_t *prtd = runtime->private_data;
+
+	snd_pcm_set_runtime_buffer(substream, &substream->dma_buffer);
+	prtd->pcm_stream_info.pcm_prepare_start = 0;
+	prtd->pcm_stream_info.period_index = 0;
+	return 0;
+}

This appears to ignore the params passed so hw_params() seems like the
wrong place - open()?