| Message ID | 1334607198-18694-1-git-send-email-grant.likely@secretlab.ca (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
Ben, I can take these two patches via my irqdomain tree if you prefer. g. On Mon, Apr 16, 2012 at 2:13 PM, Grant Likely <grant.likely@secretlab.ca> wrote: > The mpc8xx driver uses a reference to NR_IRQS that is buggy. It uses > NR_IRQs for the array size of the ppc_cached_irq_mask bitmap, but > NR_IRQs could be smaller than the number of hardware irqs that > ppc_cached_irq_mask tracks. > > Also, while fixing that problem, it became apparent that the interrupt > controller only supports 32 interrupt numbers, but it is written as if > it supports multiple register banks which is more complicated. > > This patch pulls out the buggy reference to NR_IRQs and fixes the size > of the ppc_cached_irq_mask to match the number of HW irqs. It also > drops the now-unnecessary code since ppc_cached_irq_mask is no longer > an array. > > Signed-off-by: Grant Likely <grant.likely@secretlab.ca> > Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org> > --- > arch/powerpc/sysdev/mpc8xx_pic.c | 61 +++++++++++++------------------------- > 1 file changed, 21 insertions(+), 40 deletions(-) > > diff --git a/arch/powerpc/sysdev/mpc8xx_pic.c b/arch/powerpc/sysdev/mpc8xx_pic.c > index d5f5416..91cade8 100644 > --- a/arch/powerpc/sysdev/mpc8xx_pic.c > +++ b/arch/powerpc/sysdev/mpc8xx_pic.c > @@ -18,69 +18,47 @@ > extern int cpm_get_irq(struct pt_regs *regs); > > static struct irq_domain *mpc8xx_pic_host; > -#define NR_MASK_WORDS ((NR_IRQS + 31) / 32) > -static unsigned long ppc_cached_irq_mask[NR_MASK_WORDS]; > +static unsigned long ppc_cached_irq_mask; > static sysconf8xx_t __iomem *siu_reg; > > int cpm_get_irq(struct pt_regs *regs); > > -static void mpc8xx_unmask_irq(struct irq_data *d) > +static inline unsigned long mpc8xx_irqd_to_bit(struct irq_data *d) > { > - int bit, word; > - unsigned int irq_nr = (unsigned int)irqd_to_hwirq(d); > - > - bit = irq_nr & 0x1f; > - word = irq_nr >> 5; > + return 0x80000000 >> irqd_to_hwirq(d); > +} > > - ppc_cached_irq_mask[word] |= (1 << (31-bit)); > - out_be32(&siu_reg->sc_simask, ppc_cached_irq_mask[word]); > +static void mpc8xx_unmask_irq(struct irq_data *d) > +{ > + ppc_cached_irq_mask |= mpc8xx_irqd_to_bit(d); > + out_be32(&siu_reg->sc_simask, ppc_cached_irq_mask); > } > > static void mpc8xx_mask_irq(struct irq_data *d) > { > - int bit, word; > - unsigned int irq_nr = (unsigned int)irqd_to_hwirq(d); > - > - bit = irq_nr & 0x1f; > - word = irq_nr >> 5; > - > - ppc_cached_irq_mask[word] &= ~(1 << (31-bit)); > - out_be32(&siu_reg->sc_simask, ppc_cached_irq_mask[word]); > + ppc_cached_irq_mask &= ~mpc8xx_irqd_to_bit(d); > + out_be32(&siu_reg->sc_simask, ppc_cached_irq_mask); > } > > static void mpc8xx_ack(struct irq_data *d) > { > - int bit; > - unsigned int irq_nr = (unsigned int)irqd_to_hwirq(d); > - > - bit = irq_nr & 0x1f; > - out_be32(&siu_reg->sc_sipend, 1 << (31-bit)); > + out_be32(&siu_reg->sc_sipend, mpc8xx_irqd_to_bit(d)); > } > > static void mpc8xx_end_irq(struct irq_data *d) > { > - int bit, word; > - unsigned int irq_nr = (unsigned int)irqd_to_hwirq(d); > - > - bit = irq_nr & 0x1f; > - word = irq_nr >> 5; > - > - ppc_cached_irq_mask[word] |= (1 << (31-bit)); > - out_be32(&siu_reg->sc_simask, ppc_cached_irq_mask[word]); > + ppc_cached_irq_mask |= mpc8xx_irqd_to_bit(d); > + out_be32(&siu_reg->sc_simask, ppc_cached_irq_mask); > } > > static int mpc8xx_set_irq_type(struct irq_data *d, unsigned int flow_type) > { > - if (flow_type & IRQ_TYPE_EDGE_FALLING) { > - irq_hw_number_t hw = (unsigned int)irqd_to_hwirq(d); > + /* only external IRQ senses are programmable */ > + if ((flow_type & IRQ_TYPE_EDGE_FALLING) && !(irqd_to_hwirq(d) & 1)) { > unsigned int siel = in_be32(&siu_reg->sc_siel); > - > - /* only external IRQ senses are programmable */ > - if ((hw & 1) == 0) { > - siel |= (0x80000000 >> hw); > - out_be32(&siu_reg->sc_siel, siel); > - __irq_set_handler_locked(d->irq, handle_edge_irq); > - } > + siel |= mpc8xx_irqd_to_bit(d); > + out_be32(&siu_reg->sc_siel, siel); > + __irq_set_handler_locked(d->irq, handle_edge_irq); > } > return 0; > } > @@ -132,6 +110,9 @@ static int mpc8xx_pic_host_xlate(struct irq_domain *h, struct device_node *ct, > IRQ_TYPE_EDGE_FALLING, > }; > > + if (intspec[0] > 0x1f) > + return 0; > + > *out_hwirq = intspec[0]; > if (intsize > 1 && intspec[1] < 4) > *out_flags = map_pic_senses[intspec[1]]; > -- > 1.7.9.5 >
On Mon, 2012-04-16 at 14:13 -0600, Grant Likely wrote: > The mpc8xx driver uses a reference to NR_IRQS that is buggy. It uses > NR_IRQs for the array size of the ppc_cached_irq_mask bitmap, but > NR_IRQs could be smaller than the number of hardware irqs that > ppc_cached_irq_mask tracks. > > Also, while fixing that problem, it became apparent that the interrupt > controller only supports 32 interrupt numbers, but it is written as if > it supports multiple register banks which is more complicated. > > This patch pulls out the buggy reference to NR_IRQs and fixes the size > of the ppc_cached_irq_mask to match the number of HW irqs. It also > drops the now-unnecessary code since ppc_cached_irq_mask is no longer > an array. Can you rename ppc_cached_irq_mask while at it ? I think it was written that way because it was all copy/pasted from powermac/pic.c which -does- need it to be an array (and has the same bugs btw) :-) I wonder if we can get rid of the cached mask alltogether... I have the feeling that we could just rely on the irq_desc fields nowadays for that... this code is ancient. Cheers, Ben. > Signed-off-by: Grant Likely <grant.likely@secretlab.ca> > Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org> > --- > arch/powerpc/sysdev/mpc8xx_pic.c | 61 +++++++++++++------------------------- > 1 file changed, 21 insertions(+), 40 deletions(-) > > diff --git a/arch/powerpc/sysdev/mpc8xx_pic.c b/arch/powerpc/sysdev/mpc8xx_pic.c > index d5f5416..91cade8 100644 > --- a/arch/powerpc/sysdev/mpc8xx_pic.c > +++ b/arch/powerpc/sysdev/mpc8xx_pic.c > @@ -18,69 +18,47 @@ > extern int cpm_get_irq(struct pt_regs *regs); > > static struct irq_domain *mpc8xx_pic_host; > -#define NR_MASK_WORDS ((NR_IRQS + 31) / 32) > -static unsigned long ppc_cached_irq_mask[NR_MASK_WORDS]; > +static unsigned long ppc_cached_irq_mask; > static sysconf8xx_t __iomem *siu_reg; > > int cpm_get_irq(struct pt_regs *regs); > > -static void mpc8xx_unmask_irq(struct irq_data *d) > +static inline unsigned long mpc8xx_irqd_to_bit(struct irq_data *d) > { > - int bit, word; > - unsigned int irq_nr = (unsigned int)irqd_to_hwirq(d); > - > - bit = irq_nr & 0x1f; > - word = irq_nr >> 5; > + return 0x80000000 >> irqd_to_hwirq(d); > +} > > - ppc_cached_irq_mask[word] |= (1 << (31-bit)); > - out_be32(&siu_reg->sc_simask, ppc_cached_irq_mask[word]); > +static void mpc8xx_unmask_irq(struct irq_data *d) > +{ > + ppc_cached_irq_mask |= mpc8xx_irqd_to_bit(d); > + out_be32(&siu_reg->sc_simask, ppc_cached_irq_mask); > } > > static void mpc8xx_mask_irq(struct irq_data *d) > { > - int bit, word; > - unsigned int irq_nr = (unsigned int)irqd_to_hwirq(d); > - > - bit = irq_nr & 0x1f; > - word = irq_nr >> 5; > - > - ppc_cached_irq_mask[word] &= ~(1 << (31-bit)); > - out_be32(&siu_reg->sc_simask, ppc_cached_irq_mask[word]); > + ppc_cached_irq_mask &= ~mpc8xx_irqd_to_bit(d); > + out_be32(&siu_reg->sc_simask, ppc_cached_irq_mask); > } > > static void mpc8xx_ack(struct irq_data *d) > { > - int bit; > - unsigned int irq_nr = (unsigned int)irqd_to_hwirq(d); > - > - bit = irq_nr & 0x1f; > - out_be32(&siu_reg->sc_sipend, 1 << (31-bit)); > + out_be32(&siu_reg->sc_sipend, mpc8xx_irqd_to_bit(d)); > } > > static void mpc8xx_end_irq(struct irq_data *d) > { > - int bit, word; > - unsigned int irq_nr = (unsigned int)irqd_to_hwirq(d); > - > - bit = irq_nr & 0x1f; > - word = irq_nr >> 5; > - > - ppc_cached_irq_mask[word] |= (1 << (31-bit)); > - out_be32(&siu_reg->sc_simask, ppc_cached_irq_mask[word]); > + ppc_cached_irq_mask |= mpc8xx_irqd_to_bit(d); > + out_be32(&siu_reg->sc_simask, ppc_cached_irq_mask); > } > > static int mpc8xx_set_irq_type(struct irq_data *d, unsigned int flow_type) > { > - if (flow_type & IRQ_TYPE_EDGE_FALLING) { > - irq_hw_number_t hw = (unsigned int)irqd_to_hwirq(d); > + /* only external IRQ senses are programmable */ > + if ((flow_type & IRQ_TYPE_EDGE_FALLING) && !(irqd_to_hwirq(d) & 1)) { > unsigned int siel = in_be32(&siu_reg->sc_siel); > - > - /* only external IRQ senses are programmable */ > - if ((hw & 1) == 0) { > - siel |= (0x80000000 >> hw); > - out_be32(&siu_reg->sc_siel, siel); > - __irq_set_handler_locked(d->irq, handle_edge_irq); > - } > + siel |= mpc8xx_irqd_to_bit(d); > + out_be32(&siu_reg->sc_siel, siel); > + __irq_set_handler_locked(d->irq, handle_edge_irq); > } > return 0; > } > @@ -132,6 +110,9 @@ static int mpc8xx_pic_host_xlate(struct irq_domain *h, struct device_node *ct, > IRQ_TYPE_EDGE_FALLING, > }; > > + if (intspec[0] > 0x1f) > + return 0; > + > *out_hwirq = intspec[0]; > if (intsize > 1 && intspec[1] < 4) > *out_flags = map_pic_senses[intspec[1]];
On Mon, 2012-04-16 at 14:13 -0600, Grant Likely wrote: > Ben, I can take these two patches via my irqdomain tree if you prefer. Let me give them a test run first. Cheers, Ben. > g. > > On Mon, Apr 16, 2012 at 2:13 PM, Grant Likely <grant.likely@secretlab.ca> wrote: > > The mpc8xx driver uses a reference to NR_IRQS that is buggy. It uses > > NR_IRQs for the array size of the ppc_cached_irq_mask bitmap, but > > NR_IRQs could be smaller than the number of hardware irqs that > > ppc_cached_irq_mask tracks. > > > > Also, while fixing that problem, it became apparent that the interrupt > > controller only supports 32 interrupt numbers, but it is written as if > > it supports multiple register banks which is more complicated. > > > > This patch pulls out the buggy reference to NR_IRQs and fixes the size > > of the ppc_cached_irq_mask to match the number of HW irqs. It also > > drops the now-unnecessary code since ppc_cached_irq_mask is no longer > > an array. > > > > Signed-off-by: Grant Likely <grant.likely@secretlab.ca> > > Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org> > > --- > > arch/powerpc/sysdev/mpc8xx_pic.c | 61 +++++++++++++------------------------- > > 1 file changed, 21 insertions(+), 40 deletions(-) > > > > diff --git a/arch/powerpc/sysdev/mpc8xx_pic.c b/arch/powerpc/sysdev/mpc8xx_pic.c > > index d5f5416..91cade8 100644 > > --- a/arch/powerpc/sysdev/mpc8xx_pic.c > > +++ b/arch/powerpc/sysdev/mpc8xx_pic.c > > @@ -18,69 +18,47 @@ > > extern int cpm_get_irq(struct pt_regs *regs); > > > > static struct irq_domain *mpc8xx_pic_host; > > -#define NR_MASK_WORDS ((NR_IRQS + 31) / 32) > > -static unsigned long ppc_cached_irq_mask[NR_MASK_WORDS]; > > +static unsigned long ppc_cached_irq_mask; > > static sysconf8xx_t __iomem *siu_reg; > > > > int cpm_get_irq(struct pt_regs *regs); > > > > -static void mpc8xx_unmask_irq(struct irq_data *d) > > +static inline unsigned long mpc8xx_irqd_to_bit(struct irq_data *d) > > { > > - int bit, word; > > - unsigned int irq_nr = (unsigned int)irqd_to_hwirq(d); > > - > > - bit = irq_nr & 0x1f; > > - word = irq_nr >> 5; > > + return 0x80000000 >> irqd_to_hwirq(d); > > +} > > > > - ppc_cached_irq_mask[word] |= (1 << (31-bit)); > > - out_be32(&siu_reg->sc_simask, ppc_cached_irq_mask[word]); > > +static void mpc8xx_unmask_irq(struct irq_data *d) > > +{ > > + ppc_cached_irq_mask |= mpc8xx_irqd_to_bit(d); > > + out_be32(&siu_reg->sc_simask, ppc_cached_irq_mask); > > } > > > > static void mpc8xx_mask_irq(struct irq_data *d) > > { > > - int bit, word; > > - unsigned int irq_nr = (unsigned int)irqd_to_hwirq(d); > > - > > - bit = irq_nr & 0x1f; > > - word = irq_nr >> 5; > > - > > - ppc_cached_irq_mask[word] &= ~(1 << (31-bit)); > > - out_be32(&siu_reg->sc_simask, ppc_cached_irq_mask[word]); > > + ppc_cached_irq_mask &= ~mpc8xx_irqd_to_bit(d); > > + out_be32(&siu_reg->sc_simask, ppc_cached_irq_mask); > > } > > > > static void mpc8xx_ack(struct irq_data *d) > > { > > - int bit; > > - unsigned int irq_nr = (unsigned int)irqd_to_hwirq(d); > > - > > - bit = irq_nr & 0x1f; > > - out_be32(&siu_reg->sc_sipend, 1 << (31-bit)); > > + out_be32(&siu_reg->sc_sipend, mpc8xx_irqd_to_bit(d)); > > } > > > > static void mpc8xx_end_irq(struct irq_data *d) > > { > > - int bit, word; > > - unsigned int irq_nr = (unsigned int)irqd_to_hwirq(d); > > - > > - bit = irq_nr & 0x1f; > > - word = irq_nr >> 5; > > - > > - ppc_cached_irq_mask[word] |= (1 << (31-bit)); > > - out_be32(&siu_reg->sc_simask, ppc_cached_irq_mask[word]); > > + ppc_cached_irq_mask |= mpc8xx_irqd_to_bit(d); > > + out_be32(&siu_reg->sc_simask, ppc_cached_irq_mask); > > } > > > > static int mpc8xx_set_irq_type(struct irq_data *d, unsigned int flow_type) > > { > > - if (flow_type & IRQ_TYPE_EDGE_FALLING) { > > - irq_hw_number_t hw = (unsigned int)irqd_to_hwirq(d); > > + /* only external IRQ senses are programmable */ > > + if ((flow_type & IRQ_TYPE_EDGE_FALLING) && !(irqd_to_hwirq(d) & 1)) { > > unsigned int siel = in_be32(&siu_reg->sc_siel); > > - > > - /* only external IRQ senses are programmable */ > > - if ((hw & 1) == 0) { > > - siel |= (0x80000000 >> hw); > > - out_be32(&siu_reg->sc_siel, siel); > > - __irq_set_handler_locked(d->irq, handle_edge_irq); > > - } > > + siel |= mpc8xx_irqd_to_bit(d); > > + out_be32(&siu_reg->sc_siel, siel); > > + __irq_set_handler_locked(d->irq, handle_edge_irq); > > } > > return 0; > > } > > @@ -132,6 +110,9 @@ static int mpc8xx_pic_host_xlate(struct irq_domain *h, struct device_node *ct, > > IRQ_TYPE_EDGE_FALLING, > > }; > > > > + if (intspec[0] > 0x1f) > > + return 0; > > + > > *out_hwirq = intspec[0]; > > if (intsize > 1 && intspec[1] < 4) > > *out_flags = map_pic_senses[intspec[1]]; > > -- > > 1.7.9.5 > > > > >
On Mon, Apr 16, 2012 at 5:03 PM, Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote: > On Mon, 2012-04-16 at 14:13 -0600, Grant Likely wrote: >> Ben, I can take these two patches via my irqdomain tree if you prefer. > > Let me give them a test run first. Yes of course. I won't merge before they've been checked out. g.
On Mon, Apr 16, 2012 at 5:03 PM, Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote: > On Mon, 2012-04-16 at 14:13 -0600, Grant Likely wrote: >> The mpc8xx driver uses a reference to NR_IRQS that is buggy. It uses >> NR_IRQs for the array size of the ppc_cached_irq_mask bitmap, but >> NR_IRQs could be smaller than the number of hardware irqs that >> ppc_cached_irq_mask tracks. >> >> Also, while fixing that problem, it became apparent that the interrupt >> controller only supports 32 interrupt numbers, but it is written as if >> it supports multiple register banks which is more complicated. >> >> This patch pulls out the buggy reference to NR_IRQs and fixes the size >> of the ppc_cached_irq_mask to match the number of HW irqs. It also >> drops the now-unnecessary code since ppc_cached_irq_mask is no longer >> an array. > > Can you rename ppc_cached_irq_mask while at it ? I think it was written > that way because it was all copy/pasted from powermac/pic.c which -does- > need it to be an array (and has the same bugs btw) :-) > > I wonder if we can get rid of the cached mask alltogether... I have the > feeling that we could just rely on the irq_desc fields nowadays for > that... this code is ancient. The irq_desc fields aren't ideal because they are per-irq and would need to be &'ed together for each mask/unmask/ack operation. We could store something in the irq_domain I suppose, but I'm not convinced it is worth it for an interrupt controller that will only ever have one instance in the system. If I were to refactor any deeper than I have, then I'm move this driver over to use irq_generic_chip instead. g.
> > On Mon, 2012-04-16 at 14:13 -0600, Grant Likely wrote: > > Ben, I can take these two patches via my irqdomain tree if you prefer. > > Let me give them a test run first. Hi Ben If you are going to test 8xx, could you revert e0908085fc2391c85b85fb814ae1df377c8e0dcb, powerpc/8xx: Fix regression introduced by cache coherency rewrite ? It should not be needed after I fixed up the 8xx TLB code and added a workaround for buggy dcbX instructions. Jocke
On Tue, 2012-04-17 at 02:03 +0200, Joakim Tjernlund wrote: > > If you are going to test 8xx, could you revert > e0908085fc2391c85b85fb814ae1df377c8e0dcb, > powerpc/8xx: Fix regression introduced by cache coherency rewrite ? > > It should not be needed after I fixed up the 8xx TLB code and added a > workaround > for buggy dcbX instructions. Have you actually verified that things still work after reverting it ? Cheers, Ben.
Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote on 2012/04/17 03:00:40: > > On Tue, 2012-04-17 at 02:03 +0200, Joakim Tjernlund wrote: > > > > If you are going to test 8xx, could you revert > > e0908085fc2391c85b85fb814ae1df377c8e0dcb, > > powerpc/8xx: Fix regression introduced by cache coherency rewrite ? > > > > It should not be needed after I fixed up the 8xx TLB code and added a > > workaround > > for buggy dcbX instructions. > > Have you actually verified that things still work after reverting it ? Nope, I haven't moved our 8xx to 3.x. We are still on 2.4 and it works there. Jocke
On Tue, 17 Apr 2012 09:03:02 +1000, Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote: > On Mon, 2012-04-16 at 14:13 -0600, Grant Likely wrote: > > The mpc8xx driver uses a reference to NR_IRQS that is buggy. It uses > > NR_IRQs for the array size of the ppc_cached_irq_mask bitmap, but > > NR_IRQs could be smaller than the number of hardware irqs that > > ppc_cached_irq_mask tracks. > > > > Also, while fixing that problem, it became apparent that the interrupt > > controller only supports 32 interrupt numbers, but it is written as if > > it supports multiple register banks which is more complicated. > > > > This patch pulls out the buggy reference to NR_IRQs and fixes the size > > of the ppc_cached_irq_mask to match the number of HW irqs. It also > > drops the now-unnecessary code since ppc_cached_irq_mask is no longer > > an array. > > Can you rename ppc_cached_irq_mask while at it ? I think it was written > that way because it was all copy/pasted from powermac/pic.c which -does- > need it to be an array (and has the same bugs btw) :-) Done; and I think I fixed the pic.c bugs in patch 2. g.
diff --git a/arch/powerpc/sysdev/mpc8xx_pic.c b/arch/powerpc/sysdev/mpc8xx_pic.c index d5f5416..91cade8 100644 --- a/arch/powerpc/sysdev/mpc8xx_pic.c +++ b/arch/powerpc/sysdev/mpc8xx_pic.c @@ -18,69 +18,47 @@ extern int cpm_get_irq(struct pt_regs *regs); static struct irq_domain *mpc8xx_pic_host; -#define NR_MASK_WORDS ((NR_IRQS + 31) / 32) -static unsigned long ppc_cached_irq_mask[NR_MASK_WORDS]; +static unsigned long ppc_cached_irq_mask; static sysconf8xx_t __iomem *siu_reg; int cpm_get_irq(struct pt_regs *regs); -static void mpc8xx_unmask_irq(struct irq_data *d) +static inline unsigned long mpc8xx_irqd_to_bit(struct irq_data *d) { - int bit, word; - unsigned int irq_nr = (unsigned int)irqd_to_hwirq(d); - - bit = irq_nr & 0x1f; - word = irq_nr >> 5; + return 0x80000000 >> irqd_to_hwirq(d); +} - ppc_cached_irq_mask[word] |= (1 << (31-bit)); - out_be32(&siu_reg->sc_simask, ppc_cached_irq_mask[word]); +static void mpc8xx_unmask_irq(struct irq_data *d) +{ + ppc_cached_irq_mask |= mpc8xx_irqd_to_bit(d); + out_be32(&siu_reg->sc_simask, ppc_cached_irq_mask); } static void mpc8xx_mask_irq(struct irq_data *d) { - int bit, word; - unsigned int irq_nr = (unsigned int)irqd_to_hwirq(d); - - bit = irq_nr & 0x1f; - word = irq_nr >> 5; - - ppc_cached_irq_mask[word] &= ~(1 << (31-bit)); - out_be32(&siu_reg->sc_simask, ppc_cached_irq_mask[word]); + ppc_cached_irq_mask &= ~mpc8xx_irqd_to_bit(d); + out_be32(&siu_reg->sc_simask, ppc_cached_irq_mask); } static void mpc8xx_ack(struct irq_data *d) { - int bit; - unsigned int irq_nr = (unsigned int)irqd_to_hwirq(d); - - bit = irq_nr & 0x1f; - out_be32(&siu_reg->sc_sipend, 1 << (31-bit)); + out_be32(&siu_reg->sc_sipend, mpc8xx_irqd_to_bit(d)); } static void mpc8xx_end_irq(struct irq_data *d) { - int bit, word; - unsigned int irq_nr = (unsigned int)irqd_to_hwirq(d); - - bit = irq_nr & 0x1f; - word = irq_nr >> 5; - - ppc_cached_irq_mask[word] |= (1 << (31-bit)); - out_be32(&siu_reg->sc_simask, ppc_cached_irq_mask[word]); + ppc_cached_irq_mask |= mpc8xx_irqd_to_bit(d); + out_be32(&siu_reg->sc_simask, ppc_cached_irq_mask); } static int mpc8xx_set_irq_type(struct irq_data *d, unsigned int flow_type) { - if (flow_type & IRQ_TYPE_EDGE_FALLING) { - irq_hw_number_t hw = (unsigned int)irqd_to_hwirq(d); + /* only external IRQ senses are programmable */ + if ((flow_type & IRQ_TYPE_EDGE_FALLING) && !(irqd_to_hwirq(d) & 1)) { unsigned int siel = in_be32(&siu_reg->sc_siel); - - /* only external IRQ senses are programmable */ - if ((hw & 1) == 0) { - siel |= (0x80000000 >> hw); - out_be32(&siu_reg->sc_siel, siel); - __irq_set_handler_locked(d->irq, handle_edge_irq); - } + siel |= mpc8xx_irqd_to_bit(d); + out_be32(&siu_reg->sc_siel, siel); + __irq_set_handler_locked(d->irq, handle_edge_irq); } return 0; } @@ -132,6 +110,9 @@ static int mpc8xx_pic_host_xlate(struct irq_domain *h, struct device_node *ct, IRQ_TYPE_EDGE_FALLING, }; + if (intspec[0] > 0x1f) + return 0; + *out_hwirq = intspec[0]; if (intsize > 1 && intspec[1] < 4) *out_flags = map_pic_senses[intspec[1]];
The mpc8xx driver uses a reference to NR_IRQS that is buggy. It uses NR_IRQs for the array size of the ppc_cached_irq_mask bitmap, but NR_IRQs could be smaller than the number of hardware irqs that ppc_cached_irq_mask tracks. Also, while fixing that problem, it became apparent that the interrupt controller only supports 32 interrupt numbers, but it is written as if it supports multiple register banks which is more complicated. This patch pulls out the buggy reference to NR_IRQs and fixes the size of the ppc_cached_irq_mask to match the number of HW irqs. It also drops the now-unnecessary code since ppc_cached_irq_mask is no longer an array. Signed-off-by: Grant Likely <grant.likely@secretlab.ca> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org> --- arch/powerpc/sysdev/mpc8xx_pic.c | 61 +++++++++++++------------------------- 1 file changed, 21 insertions(+), 40 deletions(-)