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

[Mark Brown <broonie@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 1888 bytes --]

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()?

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]