public inbox for netdev@vger.kernel.org 
 help / color / mirror / Atom feed
From: Sascha Herrmann <sascha@ps•nvbi.de>
To: Werner Almesberger <werner@almesberger•net>
Cc: netdev@vger•kernel.org, linux-zigbee-devel@lists•sourceforge.net
Subject: Re: [PATCH 2/2] at86rf230: change irq handling to prevent lockups with edge type irq
Date: Sun, 07 Apr 2013 22:52:38 +0200	[thread overview]
Message-ID: <5161DC96.6030905@ps.nvbi.de> (raw)
In-Reply-To: <20130406142035.GG28141@ws>

On 06.04.2013 16:20, Werner Almesberger wrote:

> Sascha Herrmann wrote:
>> The trigger level isn't configurable in the rf230,
> 
> Right, I should have mentioned that the polarity can only be set on
> the AT86RF231. The 230 has to make do with rising edge or high level.


Oh, I didn't realized, that this is possible at all with the rf231...

 
>> I fear the sollution to read the interrupt status register in a loop
>> (as suggested in your earlier message) would leave chances for race
>> conditions or spurious interrupts, depending on wheter interrupts are
>> enabled before or after reading the status register. 
> 
> I don't think the analysis is worse than for any other solution.
> There are also tools and methods that can help if it becomes too
> much of a headache.
> 
> If you don't like the loop, a double read without loop would work in
> this case as well:
> 
> 	irq = read_and_clear_interrupt();
> 	enable_irq();
> 	irq |= read_and_clear_interrupt();
> 	...


Maybe one way to eliminate the extra latency of the second register read
would be to split the interrupt handling function into a generic part
and two different functions to handle the different types of interrupts:

	static void at86rf230_irqwork_level(void) {
		__at86rf230_irqwork();

		spin_lock_irqsave(&lp->lock, flags);
		lp->irq_busy = 0;
		enable_irq()
		spin_unlock_irqrestore(&lp->lock, flags);
	}

For edge type configuration the call to enable_irq() must be removed.
The same would be required for the isr function. The probe function
could than decide, which isr function should be registered for the
interrupt.

> By the way, once we switch to early RX_ON, I think we'll have the
> problem that two TRX_END interrupts may be generated without any
> host action between them, in which case the first will be
> interpreted as the end of sending and the second will be ignored,
> leaving a received frame in the buffer, which in turn leaves
> dynamic buffer protection on and thus prevents any further
> reception.


I think this is right, we should have an eye on this when working on the
early RX_ON...

> Not sure yet how to solve this. I also don't know how bad our
> latencies are, but I fear that they can at times be substantial.
> Already a single register access with spi-gpio takes some 80 us
> (measured on an otherwise idle Ben, 336 MHz MIPS).
> 
>> Surely for this option, the assumption that (at least nearly) every
>> platform supports edge type interrupts should hold.
> 
> I think you're on relatively safe ground with the assumption that
> most relevant platforms per se can do it. But if the interrupt line
> happens to be shared with some other device, then level trigger is
> quite popular.


If you think the solution above would be ok, I could try to send a
version which allows the configuration of trigger type and level.

Thanks,
Sascha

-- 
Hi! I'm a .signature virus! Copy me into your ~/.signature to help me
spread!

  reply	other threads:[~2013-04-07 20:52 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-04-04 21:01 [PATCH 0/2] changed irq handling and some cleanup for at86rf230 Sascha Herrmann
2013-04-04 21:02 ` [PATCH 1/2] at86rf230: remove unnecessary / dead code Sascha Herrmann
2013-04-08 16:01   ` David Miller
2013-04-04 21:02 ` [PATCH 2/2] at86rf230: change irq handling to prevent lockups with edge type irq Sascha Herrmann
     [not found]   ` <57c67195742f5e7482dc57f5a05b6d69156b00d2.1365107512.git.sascha-2k69yqSu1NaELgA04lAiVw@public.gmane.org>
2013-04-05  3:59     ` Werner Almesberger
2013-04-05 10:51       ` [Linux-zigbee-devel] " Werner Almesberger
2013-04-05 15:59         ` Sascha Herrmann
2013-04-06 14:20           ` Werner Almesberger
2013-04-07 20:52             ` Sascha Herrmann [this message]
2013-04-08 15:35               ` Werner Almesberger

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5161DC96.6030905@ps.nvbi.de \
    --to=sascha@ps$(echo .)nvbi.de \
    --cc=linux-zigbee-devel@lists$(echo .)sourceforge.net \
    --cc=netdev@vger$(echo .)kernel.org \
    --cc=werner@almesberger$(echo .)net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox