lkml.org 
[lkml]   [2012]   [Apr]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectRe: [PATCH v5 06/27] irq_domain/powerpc: eliminate irq_map; use irq_alloc_desc() instead
    Date
    On Wed, 11 Apr 2012 15:29:42 +1000, Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:
    > On Wed, 2012-04-11 at 11:33 +1000, Benjamin Herrenschmidt wrote:
    > > On Wed, 2012-04-11 at 11:13 +1000, Benjamin Herrenschmidt wrote:
    > > > On Sat, 2012-04-07 at 14:27 +0200, Andreas Schwab wrote:
    > > > > Benjamin Herrenschmidt <benh@kernel.crashing.org> writes:
    > > > >
    > > > > > It's arguable that this irq_set_irq_type(,NONE) shouln't be there but
    > > > > > still ... it's been around for ever and things worked :-) So something
    > > > > > -else- is causing the problem and I'd like to understand what exactly.
    > > > >
    > > > > AFAICS before a09b659cd68c10ec6a30cb91ebd2c327fcd5bfe5
    > > > > irq_set_irq_type(,NONE) was actually a no-op.
    > > >
    > > > So I'm still a bit baffled... ie, I understand some of what's happening
    > > > but not why it breaks things, I haven't yet managed to reproduce but I
    > > > haven't tried too hard just yet (was away from the HW) :
    > >
    > > Allright, I have a repro-case, I'll dig.
    >
    > Ok, so it's Grant's fault :-)

    I pretty much expected it would be. :-p

    > So basically, it's quite subtle and I'm only 99% sure of the details but
    > I believe what happens is:
    >
    > - The audio interrupts get virq 64 and 65 (so above NR_IRQS)
    >
    > - The reverse map isn't pre-filled at map time (we should probably do
    > it nowadays), we do it lazily so ...

    Hmmm... I though I had merged a patch that does that.

    /me goes to look again...

    Okay, I did write that patch, but I never merged it because it didn't
    get much review and I was already nervous about the other irq_domain
    changes. I'll post it again and ask for feedback.

    > - The whole business with irq_virq_count needs fixing. Basically the
    > default value shouldn't be NR_IRQ. I suggest making it 0 and have all
    > the use sites do something like:
    >
    > max = irq_virq_count ? irq_virq_count : nr_irqs;
    >
    > (Grant, can you take care of that ?)

    Yeah, I've got a different way to fix it though. There is exactly one
    user of irq_virq_count in-tree right now: PS3. Also, irq_virq_count
    is only useful for the NOMAP mapping. So, instead of having a single
    global irq_virq_count values, I've dropped it entirely and added a
    max_irq argument to irq_domain_add_nomap(). That makes it a property
    of an individual nomap irq domain instead of a global system settting.

    Hopefully I'll have a draft patch ready today.

    g.


    \
     
     \ /
      Last update: 2012-04-11 22:59    [W:0.032 / U:152.752 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site