Re: [PATCH 2/2] at86rf230: change irq handling to prevent lockups with edge type irq

[Werner Almesberger <werner-SEdMjqphH88wryQfseakQg@public.gmane.org>]

Sascha Herrmann wrote:
> -	rc = request_irq(spi->irq, at86rf230_isr, IRQF_SHARED,
> +	rc = request_irq(spi->irq, at86rf230_isr,
> +			 IRQF_SHARED | IRQF_TRIGGER_RISING,

To clarify the purpose of your patch, I think it's only needed
for platforms that don't support level-triggered interrupts.
I.e., if you used IRQF_TRIGGER_HIGH (and happened to have a
platform that supports it, which you said on IRC you don't),
the existing code should work fine. Correct ?

I wonder if it wouldn't be better to make the code work with
both edge and level interrupts instead of having to choose.

E.g., this sort of loop in at86rf230_irqwork should work with
either:

	while (1) {
		irq = read_and_clear_interrupt();
		...
		if (!test_and_clear_bit(0, &lp->irq_disabled))
			break;
		enable_irq();
	}
	lp->irq_busy = 0; /* allow _xmit */

Disadvantage: you get extra IRQ_STATUS reads, which has a
slight penalty, given that all this is on SPI.

To achieve perfection, at86rf230_probe could try all four
possible trigger modes, pick one the platform supports, and
set TRX_CTRL_1.IRQ_POLARITY accordingly.

In any case, I gave your patch very light testing on ATBEN and
it performed as good as the original code.

- Werner

------------------------------------------------------------------------------
Minimize network downtime and maximize team effectiveness.
Reduce network management and security costs.Learn how to hire 
the most talented Cisco Certified professionals. Visit the 
Employer Resources Portal
http://www.cisco.com/web/learning/employer_resources/index.html