public inbox for linuxppc-dev@ozlabs.org 
 help / color / mirror / Atom feed
From: Valentine Barshak <vbarshak@ru•mvista.com>
To: benh@kernel•crashing.org
Cc: linuxppc-dev@ozlabs•org, David Gibson <dwg@au1•ibm.com>
Subject: Re: [PATCH 2/2] PowerPC: make 4xx uic use generic edge and level irq handlers
Date: Wed, 14 Nov 2007 15:31:13 +0300	[thread overview]
Message-ID: <473AEA91.8090109@ru.mvista.com> (raw)
In-Reply-To: <1195011796.28865.28.camel@pasglop>

Benjamin Herrenschmidt wrote:
> On Wed, 2007-11-14 at 13:13 +1100, David Gibson wrote:
>> Hrm.  I *think* I'm convinced this is safe, although acking in a
>> callback which doesn't say it acks is rather yucky.  Essentially this
>> code is trading flow readability (because just reading
>> handle_level_irq will tell you something other than what it does in
>> our case) for smaller code size.  I'm not sure if this is a good trade
>> or not.
>>

It's not just code size. Actually, I was having problems with Ingo's 
real-time patch, that works on all platforms that use generic hard irq 
handlers and doesn't work just on 4xx since we use a custom one here.
And I thought that using generic handlers would be easier to maintain.
I agree that ack'ing in a callback which doesn't say it ack's looks odd, 
but ack'ing level-triggered interrupts is quirky on UIC itself. So, I 
just thought that adding a couple of quirks to mask_ack and unmask 
callbacks was not that bad.

>> There's also one definite problem: according to the discussions I had
>> with Thomas Gleixner when I wrote uic.c, handle_edge_irq is not what
>> we want for edge interrupts.
>>
>> Apparently handle_edge_irq is only for edge interrupts on "broken"
>> PICs which won't latch new interrupts while the irq is masked.  UIC is
>> not in this category, so handle_level_irq is actually what we want,
>> even for an edge irq.
>>
>> Yes, I thought the naming was more than a little confusing, too.
> 
> Hrm... handle_edge_irq works for both and you have a small performance
> benefit in not masking, and thus using handle_edge_irq, so I don't
> totally agree here. Basically, what handle_edge_irq() does is lazy
> masking. Now there -is- an issue here is that if you do lazy masking,
> you need to be able to re-emit in some convenient way.

With the ack quirks added we can use handle_level_irq for edge-triggered 
interrupts. I'll test and resubmit the patch.

> 
> Ben.
> 
> 

Thanks,
Valentine.

  reply	other threads:[~2007-11-14 12:32 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-11-13 20:15 [PATCH 0/2] PowerPC: 4xx uic updates Valentine Barshak
2007-11-13 20:25 ` [PATCH 1/2] PowerPC: 4xx uic: add mask_ack callback Valentine Barshak
2007-11-14  0:57   ` David Gibson
2007-11-13 20:27 ` [PATCH 2/2] PowerPC: make 4xx uic use generic edge and level irq handlers Valentine Barshak
2007-11-14  2:13   ` David Gibson
2007-11-14  3:43     ` Benjamin Herrenschmidt
2007-11-14 12:31       ` Valentine Barshak [this message]
2007-11-14 14:00         ` [PATCH 2/2] PowerPC: make 4xx uic use generic level irq handler Valentine Barshak
2007-11-14  2:05 ` [PATCH 0/2] PowerPC: 4xx uic updates Josh Boyer
2007-11-14  2:14   ` David Gibson
2007-11-14  3:40   ` Benjamin Herrenschmidt

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=473AEA91.8090109@ru.mvista.com \
    --to=vbarshak@ru$(echo .)mvista.com \
    --cc=benh@kernel$(echo .)crashing.org \
    --cc=dwg@au1$(echo .)ibm.com \
    --cc=linuxppc-dev@ozlabs$(echo .)org \
    /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