[alsa-devel] [PATCH 11/13] ASoC: AMD: add AMD ASoC ACP 2.x DMA driver

[Mark Brown]

On Fri, Dec 04, 2015 at 06:40:38PM -0500, Alex Deucher wrote:

> +static irqreturn_t dma_irq_handler(int irq, void *arg)
> +{
> +	u16 dscr_idx;
> +	u32 intr_flag;
> +
> +	int priority_level = 0;
> +	struct device *dev = arg;
> +
> +	struct audio_drv_data *irq_data;
> +	void __iomem *acp_mmio;
> +
> +	irq_data = dev_get_drvdata(dev);
> +	acp_mmio = irq_data->acp_mmio;
> +
> +	intr_flag = acp_get_intr_flag(acp_mmio);
> +
> +	if ((intr_flag & BIT(ACP_TO_I2S_DMA_CH_NUM)) != 0) {

> +	if ((intr_flag & BIT(I2S_TO_ACP_DMA_CH_NUM)) != 0) {

> +	if ((intr_flag & BIT(ACP_TO_SYSRAM_CH_NUM)) != 0) {
> +		snd_pcm_period_elapsed(irq_data->capture_stream);
> +		acp_ext_stat_clear_dmaioc(acp_mmio, ACP_TO_SYSRAM_CH_NUM);
> +	}
> +
> +	return IRQ_HANDLED;

What if none of those flags were set?

> +		mul = (dscr == PLAYBACK_START_DMA_DESCR_CH13) ? 0 : 1;
> +		pos =  (mul * period_bytes);

Please just write normal if statements, they're a lot more legible than
the ternery operator.

> +	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
> +	case SNDRV_PCM_TRIGGER_RESUME:
> +		if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
> +			acp_dma_start(rtd->acp_mmio,
> +						SYSRAM_TO_ACP_CH_NUM, false);
> +			while (acp_reg_read(rtd->acp_mmio, mmACP_DMA_CH_STS) &
> +						BIT(SYSRAM_TO_ACP_CH_NUM))
> +				cpu_relax();

This needs a timeout in case things go wrong, otherwise we lock up.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: not available
URL: <http://mailman.alsa-project.org/pipermail/alsa-devel/attachments/20151218/9b5f1660/attachment.sig>