Re: [PATCH v3 7/8] ASoC: wm_adsp: Add a handler for the compressed IRQ

[Mark Brown <broonie@kernel.org>]

[-- Attachment #1.1: Type: text/plain, Size: 1123 bytes --]

On Tue, Dec 15, 2015 at 11:29:48AM +0000, Charles Keepax wrote:

> +static irqreturn_t wm5110_adsp2_irq(int irq, void *data)
> +{
> +	struct wm5110_priv *florida = data;
> +
> +	wm_adsp_compr_handle_irq(&florida->core.adsp[2]);
> +
> +	return IRQ_HANDLED;
> +}

We unconditionally handle the IRQ...

> +int wm_adsp_compr_handle_irq(struct wm_adsp *dsp)
> +{
> +	struct wm_adsp_compr_buf *buf = dsp->buffer;
> +	int ret = 0;
> +
> +	mutex_lock(&dsp->pwr_lock);
> +
> +	if (!buf) {
> +		adsp_err(dsp, "Spurious buffer IRQ\n");
> +		ret = -EINVAL;
> +		goto out;
> +	}

...though we even have code to handle spurious IRQs.  I'd expect
IRQ_NONE if the interrupt wasn't handled, allowing genirq's error
handling and diagnostics to take effect.

> +static int wm_adsp_buffer_ack_irq(struct wm_adsp_compr_buf *buf)
> +{
> +	if (buf->irq_ack & 0x01)
> +		return 0;
> +
> +	adsp_dbg(buf->dsp, "Acking buffer IRQ(0x%x)\n", buf->irq_ack);
> +
> +	buf->irq_ack |= 0x01;
> +
> +	return wm_adsp_buffer_write(buf, HOST_BUFFER_FIELD(irq_ack),
> +				    buf->irq_ack);
> +}

This is confusing, this isn't actually in the interrupt path...

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]