[PATCH v2 1/2] powerpc/8xx: Fix NR_IRQ bugs and refactor 8xx interrupt controller
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.
v2: - Rename ppc_cached_irq_mask to mpc8xx_cached_irq_mask
- Drop duplicate forward declaration
Signed-off-by: Grant Likely
Cc: Benjamin Herrenschmidt
---
arch/powerpc/sysdev/mpc8xx_pic.c | 61 +-
1 file changed, 20 insertions(+), 41 deletions(-)
diff --git a/arch/powerpc/sysdev/mpc8xx_pic.c b/arch/powerpc/sysdev/mpc8xx_pic.c
index d5f5416..b724622 100644
--- a/arch/powerpc/sysdev/mpc8xx_pic.c
+++ b/arch/powerpc/sysdev/mpc8xx_pic.c
@@ -18,69 +18,45 @@
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 mpc8xx_cached_irq_mask;
static sysconf8xx_t __iomem *siu_reg;
-int cpm_get_irq(struct pt_regs *regs);
+static inline unsigned long mpc8xx_irqd_to_bit(struct irq_data *d)
+{
+ return 0x8000 >> irqd_to_hwirq(d);
+}
static void mpc8xx_unmask_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]);
+ mpc8xx_cached_irq_mask |= mpc8xx_irqd_to_bit(d);
+ out_be32(&siu_reg->sc_simask, mpc8xx_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]);
+ mpc8xx_cached_irq_mask &= ~mpc8xx_irqd_to_bit(d);
+ out_be32(&siu_reg->sc_simask, mpc8xx_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]);
+ mpc8xx_cached_irq_mask |= mpc8xx_irqd_to_bit(d);
+ out_be32(&siu_reg->sc_simask, mpc8xx_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 |= (0x8000 >> 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 +108,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
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v2 1/2] powerpc/8xx: Fix NR_IRQ bugs and refactor 8xx interrupt controller
On Mon, 2012-04-23 at 16:30 -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.
Joakim, any chance you can test this ASAP ? :-)
Thanks !
Cheers,
Ben.
> 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.
>
> v2: - Rename ppc_cached_irq_mask to mpc8xx_cached_irq_mask
> - Drop duplicate forward declaration
>
> Signed-off-by: Grant Likely
> Cc: Benjamin Herrenschmidt
> ---
> arch/powerpc/sysdev/mpc8xx_pic.c | 61
> +-
> 1 file changed, 20 insertions(+), 41 deletions(-)
>
> diff --git a/arch/powerpc/sysdev/mpc8xx_pic.c
> b/arch/powerpc/sysdev/mpc8xx_pic.c
> index d5f5416..b724622 100644
> --- a/arch/powerpc/sysdev/mpc8xx_pic.c
> +++ b/arch/powerpc/sysdev/mpc8xx_pic.c
> @@ -18,69 +18,45 @@
> 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 mpc8xx_cached_irq_mask;
> static sysconf8xx_t __iomem *siu_reg;
>
> -int cpm_get_irq(struct pt_regs *regs);
> +static inline unsigned long mpc8xx_irqd_to_bit(struct irq_data *d)
> +{
> + return 0x8000 >> irqd_to_hwirq(d);
> +}
>
> static void mpc8xx_unmask_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]);
> + mpc8xx_cached_irq_mask |= mpc8xx_irqd_to_bit(d);
> + out_be32(&siu_reg->sc_simask, mpc8xx_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]);
> + mpc8xx_cached_irq_mask &= ~mpc8xx_irqd_to_bit(d);
> + out_be32(&siu_reg->sc_simask, mpc8xx_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]);
> + mpc8xx_cached_irq_mask |= mpc8xx_irqd_to_bit(d);
> + out_be32(&siu_reg->sc_simask, mpc8xx_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 |= (0x8000 >> 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 +108,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]];
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v2 1/2] powerpc/8xx: Fix NR_IRQ bugs and refactor 8xx interrupt controller
Benjamin Herrenschmidt wrote on 2012/04/24 05:13:15: > > On Mon, 2012-04-23 at 16:30 -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. > > Joakim, any chance you can test this ASAP ? :-) Sorry, but no. We have not moved our 8xx boards to 3.x as it in maintenance and takes too much space. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v2 1/2] powerpc/8xx: Fix NR_IRQ bugs and refactor 8xx interrupt controller
On Tue, 2012-04-24 at 09:41 +0200, Joakim Tjernlund wrote: > Benjamin Herrenschmidt wrote on 2012/04/24 > 05:13:15: > > > > On Mon, 2012-04-23 at 16:30 -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. > > > > Joakim, any chance you can test this ASAP ? :-) > > Sorry, but no. We have not moved our 8xx boards to 3.x as it in maintenance > and takes too much space. Ah ok... Anybody else on the list who still has some 8xx gear ? Cheers, Ben. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v2 1/2] powerpc/8xx: Fix NR_IRQ bugs and refactor 8xx interrupt controller
On 04/24/2012 06:22 AM, Benjamin Herrenschmidt wrote: > On Tue, 2012-04-24 at 09:41 +0200, Joakim Tjernlund wrote: >> Benjamin Herrenschmidt wrote on 2012/04/24 >> 05:13:15: >>> >>> On Mon, 2012-04-23 at 16:30 -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. >>> >>> Joakim, any chance you can test this ASAP ? :-) >> >> Sorry, but no. We have not moved our 8xx boards to 3.x as it in maintenance >> and takes too much space. > > Ah ok... Anybody else on the list who still has some 8xx gear ? Boots OK on ep88xc. -Scott ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
