[kv2019i]

Write back OUTSTATx bits whenever interrupt is raised and
not just when ROR_BIT is set.

Signed-off-by: Kai Vehmanen kai.vehmanen@linux.intel.com

[kv2019i]

Not sure this is correct thing to do, but this avoids the IRQ floods I get with multiple-pause-resume.sh sof-test on Zephyr builds.

[lgirdwood]

@kv2019i this look right to me as a "Read to Clear" and should be in rc3/final. @keyonjie fyi.
@singalsu any comments ?

[singalsu]

Nice finding! Thanks!

[plbossart]

this changes the sequence for error handling, should you move the two writes before the tests and factor the two dmic_stop?

e.g.

needs_stop = false;
val0 = dai_read(dai, OUTSTAT0);
val1 = dai_read(dai, OUTSTAT1);

dai_write(dai, OUTSTAT0, val0);
dai_write(dai, OUTSTAT1, val1);

if (val0 & OUTSTAT0_ROR_BIT) {
    dai_err(dai, "dmic_irq_handler(): full fifo A or PDM overrun");
    needs_stop = true;
}

if (val1 & OUTSTAT1_ROR_BIT) {
    dai_err(dai, "dmic_irq_handler(): full fifo B or PDM overrun");
    needs_stop = true;
}

if (needs_stop) {
    dmic_stop(dai, dmic->state == COMP_STATE_ACTIVE);
    dmic->state = COMP_STATE_PREPARE;
}

[kv2019i]

Ack, thanks @plbossart . I'll do a non-draft cleanup version. I'm also not 100% sure calling dmic_stop() is safe here. To me it seems possible that a dmic interrupt is raised while a DSP core is already running dmic_stop() (for normal pipeline stop).

I think the primary reason to stop the dmic here (rather than relying on pipeline xrun handling) is to avoid spamming the trace log when a xrun happens.

[keyonjie]

@kv2019i @singalsu sorry I don't understand the logic here, only the xrun bit of the whole register is writable, if the xrun bit is not set which means there is no this kind of xrun actually, why we should write back it? and write back bit 0 actually won't clear any interrupts for us no?

[keyonjie]

if we do hit interrupt storm, I would suspect if the previous interrupt clearing or dmic_stop() work actually correctly, e.g. add a polling to the xrun bit or the SIP/FINIT bits to double confirm the clearing and stopping do success.

@singalsu one more point is that we should check (and clear) the OUTSTAT for the corresponding dai only, e.g.

static void dmic_irq_handler(void *data)
{
	struct dai *dai = data;
        uint32_t reg = dai->index ? OUTSTAT1 : OUTSTAT0;
	uint32_t val;

	/* Trace OUTSTAT register */
	val= dai_read(dai, reg);
	dai_info(dai, "dmic_irq_handler(), OUTSTAT = 0x%x",
		 val);

	if (val & BIT(27)) {
		dai_err(dai, "dmic_irq_handler(): full fifo or PDM xrun for dai %d", index);
		dai_write(dai, reg, val);
		dmic_stop(dai);
	}
}

[kv2019i]

Cross-checked with hw spec and this change is not needed. ROR is the only writable bit, so original logic is correct.

The problem I was seeing is fixed by #4297 instead. Closing this one.